fix(isthmus)!: apply the emit mapping a virtual table carries - #1189
Open
alexandrefimov wants to merge 2 commits into
Open
fix(isthmus)!: apply the emit mapping a virtual table carries#1189alexandrefimov wants to merge 2 commits into
alexandrefimov wants to merge 2 commits into
Conversation
…he ones that cannot be A VirtualTableScan routed neither its emit mapping nor its hint, so a scan whose mapping drops a column converted carrying all of them, with a row type that reported one. It goes through applyRelCommon now, like every other relation whose conversion applies one. A NamedWrite and a NamedDdl cannot: a TableModify's row type is a single ROWCOUNT column, and a CreateTable or a CreateView produces the object it creates. They refuse a mapping the way visit(NamedUpdate) already does, rather than dropping it. Closes substrait-io#1160
Nothing covered the case a reader asks about first: with no mapping there is no projection for the names to land on, so the table keeps the names its schema gives it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
visit(VirtualTableScan)routed neither half of the relation'sRelCommon: a scan whose emit mapping drops a column converted carrying all of them, under a row type that reported one, and the names of its hint were dropped with it. It goes throughapplyRelCommonnow, like every relation whose conversion applies a mapping.A
NamedWriteand aNamedDdlcannot apply one. ATableModify's row type is a single ROWCOUNT column, and aCreateTableor aCreateViewproduces the object it creates, so there are no columns for a mapping to select;visit(NamedUpdate)already refuses one for that reason, and these two now do the same instead of dropping it.A mapping is materialised as a projection, so a scan carrying one comes back from Calcite as a projection over a virtual table rather than as the scan it went in as -- the shape every relation with a mapping returns as. Without a mapping there is no projection for the hint's names to land on, and the table keeps the names its schema gives it.
Closes #1160
BREAKING CHANGE: a NamedWrite or a NamedDdl carrying an emit mapping no longer converts to Calcite. It used to convert with the mapping dropped, which produced a plan that did not describe the relation it came from.