fix(isthmus)!: apply the projection a read relation carries - #1280
Open
alexandrefimov wants to merge 1 commit into
Open
fix(isthmus)!: apply the projection a read relation carries#1280alexandrefimov wants to merge 1 commit into
alexandrefimov wants to merge 1 commit into
Conversation
`AbstractReadRel` applies a read's projection when it derives the record type, so the relation produces the columns the mask keeps. The conversion built the scan from the initial schema and gave the Calcite node every column of it, mask or no mask.
An emit mapping selects by index, and so does every field reference a parent relation makes. Both index the columns the relation produces, so against that node they landed on the wrong columns, with no error. A `VirtualTableScan` carrying a projection was refused outright rather than converted against those columns.
```
NamedScan t(a i64, b string, c fp64), projection = mask[a, c], emit = [1]
record type Struct{[FP64]} -- column c
Calcite row type RecordType(VARCHAR b) -- column b
```
The mask now becomes a projection above the scan and below the emit mapping, for a named table and a virtual one alike. Converting back gives that projection rather than the mask -- a `Project` over the whole schema, selecting the same columns in the same order -- so a round trip keeps the record type and not the encoding.
A mask that selects inside a column -- some of a struct's fields, some of a list's elements -- throws `UnsupportedOperationException`: applying it would mean rebuilding the column's value, which this conversion does not do.
The masked columns come out in the order the mask lists them. Spec v0.102.0 does not settle whether a mask may reorder at all: `expressions/field_references.md` describes a mask as removing columns and raises reordering as an open question. `MaskExpressionTypeProjector` already derives the record type in that order, though, and refusing a mask that reorders would reject plans the model accepts.
This addresses the projection part of substrait-io#1204, which also names `filter` and `best_effort_filter`. substrait-io#1260 applies `filter`; `best_effort_filter` is still dropped.
BREAKING CHANGE: a `NamedScan` carrying a projection now converts to Calcite with the mask applied, where it used to convert with the mask dropped and give a tree whose columns were not the relation's. A `VirtualTableScan` carrying one converts as well, where it used to be refused. A `NamedScan` whose mask selects inside a column now throws `UnsupportedOperationException`, where that projection used to be dropped in silence along with the rest.
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
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.
AbstractReadRelapplies a read's projection when it derives the record type, so the relation produces the columns the mask keeps. The conversion built the scan from the initial schema and gave the Calcite node every column of it, mask or no mask.An emit mapping selects by index, and so does every field reference a parent relation makes. Both index the columns the relation produces, so against that node they landed on the wrong columns, with no error. A
VirtualTableScancarrying a projection was refused outright rather than converted against those columns.The mask now becomes a projection above the scan and below the emit mapping, for a named table and a virtual one alike. Converting back gives that projection rather than the mask -- a
Projectover the whole schema, selecting the same columns in the same order -- so a round trip keeps the record type and not the encoding.A mask that selects inside a column -- some of a struct's fields, some of a list's elements -- throws
UnsupportedOperationException: applying it would mean rebuilding the column's value, which this conversion does not do.The masked columns come out in the order the mask lists them. Spec v0.102.0 does not settle whether a mask may reorder at all:
expressions/field_references.mddescribes a mask as removing columns and raises reordering as an open question.MaskExpressionTypeProjectoralready derives the record type in that order, though, and refusing a mask that reorders would reject plans the model accepts.This addresses the projection part of #1204, which also names
filterandbest_effort_filter. #1260 appliesfilter;best_effort_filteris still dropped.BREAKING CHANGE: a
NamedScancarrying a projection now converts to Calcite with the mask applied, where it used to convert with the mask dropped and give a tree whose columns were not the relation's. AVirtualTableScancarrying one converts as well, where it used to be refused. ANamedScanwhose mask selects inside a column now throwsUnsupportedOperationException, where that projection used to be dropped in silence along with the rest.