Skip to content

fix(isthmus)!: apply the projection a read relation carries - #1280

Open
alexandrefimov wants to merge 1 commit into
substrait-io:mainfrom
alexandrefimov:issue-1204-read-fields
Open

fix(isthmus)!: apply the projection a read relation carries#1280
alexandrefimov wants to merge 1 commit into
substrait-io:mainfrom
alexandrefimov:issue-1204-read-fields

Conversation

@alexandrefimov

Copy link
Copy Markdown
Contributor

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 #1204, which also names filter and best_effort_filter. #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.

`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.
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.

1 participant