Skip to content

Zachmu/insert into select #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Nov 8, 2019
Merged

Zachmu/insert into select #24

merged 6 commits into from
Nov 8, 2019

Conversation

zachmu
Copy link
Member

@zachmu zachmu commented Nov 7, 2019

First draft. Works, but error messages could be better. Working on an extension to the Type so that we can compare column types for compatibility (right now type mismatches fail at execution with e.g. "expecting int64 but got string" messages)

…or checking for compatibility

Signed-off-by: Zach Musgrave <[email protected]>
… bug: select col1, col2 from table gets mapped as a ResolvedTable, rather than a Project. This means the columns come back in the wrong order.

Signed-off-by: Zach Musgrave <[email protected]>
…ect were being inappropriately pruned (a Project turned into a ResolvedTable because none of the columns were referenced by a return).

Signed-off-by: Zach Musgrave <[email protected]>
Signed-off-by: Zach Musgrave <[email protected]>
@zachmu zachmu requested a review from Hydrocharged November 7, 2019 22:14
Copy link
Contributor

@Hydrocharged Hydrocharged left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! Such a simple change. Good idea to test joins too, I probably would have forgotten about those.

I hope your Type extensions don't conflict too much with my changing of the types to allow for parameters.

Signed-off-by: Zach Musgrave <[email protected]>
@zachmu zachmu merged commit 17eca8e into ld-master Nov 8, 2019
@Hydrocharged Hydrocharged deleted the zachmu/insert-into-select branch December 8, 2019 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants