fix(isthmus)!: round-trip a virtual table whose rows are not all literals

[alexandrefimov]

Calcite has no relation that carries expressions the way a Substrait virtual table does, so a VirtualTableScan whose rows contain a call was expanded into a UNION ALL of one single-row projection per row, each over an empty Values. Nothing in that shape says it was a table, so the plan came back as a Set of Projects over empty virtual tables -- a different relation than the one that went in. The names went with it: the schema's names rode on a renaming projection above the union, and that projection is merged into whatever projection sits above the table.

Emit an isthmus-owned relation instead. VirtualTable holds the rows as RexNodes and the schema as its row type, and visitOther recognises it by type, the way CreateTable and CreateView already are.

Recognition by type cannot over-match: a union someone wrote out of the same parts stays a union, and so does one whose arms differ only in nullability -- a shape match takes the rows from the arms and the schema from the union's row type, so anything the least-restrictive type widened is rejected against the schema. And no planner rule can erase it: UNION_REMOVE strips the one-input union a single-row table expands to, and UNION_MERGE flattens a nested union of single-row projections into exactly that shape (both measured).

Carrying the schema on the relation is also what reaches a struct column: a schema names every field at every level, depth-first, which cannot be paired with a row type after the fact.

For a consumer whose planner only knows Calcite's own relations, VirtualTableExpansionRule expands the table into the union of projections. It is opt-in -- isthmus never runs it -- and one-way: the expansion is a plan like any other and converts back as the relation it is.

The first two commits here are #1153; that one stands on its own and should go in first.

Closes #631

BREAKING CHANGE: a virtual table whose rows are not all literals now converts to io.substrait.isthmus.calcite.rel.VirtualTable instead of a UNION ALL of single-row projections over an empty table. Add VirtualTableExpansionRule to a planner to get that shape back.

[nielspardon]

Thanks for digging into this. I'd like to propose a different approach before you put more into this one, because I think the shape matcher will keep costing us.

What I'd like to try instead: an isthmus-owned Calcite relation carrying the virtual table's rows as RexNodes, emitted by SubstraitRelNodeConverter and recognized by instanceof in visitOther — the same pair CreateTable/CreateView already form. Recognition becomes identity, and the schema rides on the node instead of being rebuilt from a derived row type.

Then, for consumers who need stock Calcite, a HepPlanner rule that expands the node into LogicalValues / LogicalUnion of LogicalProjects — opt-in, never run by isthmus itself, and explicitly one-way: expand and convert back and you get the Project over an empty table again. Most of the forward-conversion work in this PR moves into that rule rather than being thrown away.

The reason I'd rather not refine the matcher: it accepts a strictly larger set of unions than the emitter produces, and that gap is doing real damage — arms differing only in nullability or width now throw IllegalArgumentException out of visit(Union) where they used to convert to a Set, which Set.deriveRecordType explicitly tolerates. handWrittenUnionOfSingleRowProjectionsBecomesAVirtualTable doesn't catch that because it never reaches the new code (literal-only arms get folded into one LogicalValues, so the reverse goes through visit(Values) — reverting both files leaves it green). And the recognition doesn't survive planning: UNION_REMOVE deletes the one-input union a single-row table emits, restoring the exact shape this fixes, while UNION_MERGE rewrites into the matching shape. Carrying the schema on the node also reaches the nested-struct case #631 is actually about, which the current NamedStruct.names()-against-top-level-types pairing can't express in either direction.

[alexandrefimov]

All three check out; measurements below so nobody has to redo them.

The test is vacuous as you say. Reverting both main files leaves handWrittenUnionOfSingleRowProjectionsBecomesAVirtualTable green, and the reason is the one you gave — the hand-written union converts to a single LogicalValues(tuples=[[{ 1 }, { 2 }]]), so the reverse goes through visit(Values) and never reaches the matcher. expressionVirtualTableUnderAProject and expressionContainingVirtualTable do fail on revert, so the PR is not uncovered, just not where that test claims it is.

The nullability gap is a live regression rather than a corner. A union of two single-row projections differing only in nullability:

origin/main:   Set
this PR:       IllegalArgumentException: Row field type (I32{nullable=false})
               does not match schema field type (I32{nullable=true})

The matcher takes the schema from union.getRowType() and the rows from the arms' own expressions, so anything the least-restrictive type widened trips VirtualTableScan's check.

And it does not survive planning. A single-row virtual table emits a one-input union:

LogicalUnion(all=[true])
  LogicalProject(c=[CAST(7:BIGINT):INTEGER NOT NULL])
    LogicalValues(tuples=[[{  }]])

UNION_REMOVE strips it, and what is left converts back to a Project — the shape this PR set out to fix.

So I would rather rework it your way than refine the matcher: recognition by identity cannot over-match, a rule cannot erase it, and carrying the schema on the node is the part that reaches the nested-struct case. I will take that on.

One thing I would like your read on before I start. #1153 changes SubstraitRelNodeConverter.visit(VirtualTableScan) — the same method the new emitter would replace — to fix a bare AssertionError on a virtual table with a struct column. Does that still stand on its own against the new shape, or should it fold into the rework?

[nielspardon]

One thing I would like your read on before I start. #1153 changes SubstraitRelNodeConverter.visit(VirtualTableScan) — the same method the new emitter would replace — to fix a bare AssertionError on a virtual table with a struct column. Does that still stand on its own against the new shape, or should it fold into the rework?

I did post review comments on #1153 just now. We can tackle that on its own.

[alexandrefimov]

Reworked the way you proposed and pushed. The branch sits on top of #1153, so the first two commits here are that one.

Two things the description does not carry. One cost the shape match never had: a relation holding RexNodes has to pass a RexShuttle on, or every expression rewrite walks past the rows -- including the scan that finds the subqueries binding outer references, which the old expansion got for free from visit(Project). A shuttle applied to the node found zero subqueries and rebuilt nothing before I added it. VirtualTable.accept(RexShuttle) and a case in OuterReferenceResolver cover it, pinned by correlationInsideASubqueryInAVirtualTableRow.

The other is a gain: the struct-column round trip comes back here. #1153 has to give it up, since a row literal cannot sit in a Values tuple and the column takes the projection encoding there, while on this branch structColumnConverts and severalStructColumnsConvert are back on assertFullRoundTrip.