fix(core): re-derive field reference types when copy-on-write replaces a relation

[nielspardon]

A FieldReference caches the type of the field it references. RelCopyOnWriteVisitor copied those references over verbatim, so replacing a subtree with one that emits a different record type left every reference above it carrying the type of the relation that is no longer there — and the record types derived from those references, such as a Project's, were wrong in turn.

The visitor now tracks the record type that each relation's own expressions resolve against and re-derives the cached type of every field reference it rewrites from it. Inputs are rewritten before their relation's expressions so the scope is the type a replaced input emits, and enclosing scopes are tracked per subquery boundary so an outer reference re-derives against the relation it steps out to.

Each scope is the one the reference actually resolves against, which is not always the relation's inputs concatenated:

• a join condition, post-join filter and residual expression resolve against the two inputs combined;
• a hash or merge join key resolves against the single side it selects from — its offsets are side-relative, which is why proto conversion types each side with its own converter;
• the filter of a read relation resolves against the schema being read, not against any enclosing scope.

References that resolve against something outside the tracked scopes are left as they are: a lambda parameter, and an outer reference identified by rel anchor rather than by stepping out (which is how a lateral join's right input references the current left row — resolving an anchor needs plan-wide context this visitor does not have).

Resolving a reference is total

Re-deriving a type must not become a new way for a rewrite to fail. A rewrite that drops a column, reshapes a nested type, or changes a container kind can leave a reference selecting something its input no longer has; the resulting tree is invalid either way, so such a reference keeps its cached type instead.

Delivering that needed the resolution itself to be total, not a guard in front of a throwing derivation — a guard that checks only the outermost segment still lets a nested reference reach the derivation and throw. FieldReference.resolveType reports the type a chain of segments selects, or nothing, at any depth and for every kind of segment. It sits beside the finders whose rules it mirrors so the two cannot drift apart silently, and it mirrors them exactly, including the two asymmetries that matter: a list element offset is not bounds-checked, because the length of a list is not part of its type, and a map key type is compared exactly, nullability included. A parity test asserts resolveType selects something exactly when ofRoot/ofExpression do not throw, which is what keeps the two in step.

Not fixed here

The types cached on function invocations are not re-derived — that needs the function declarations, which the visitor does not have — so a relation whose record type comes from a measure or window function can still be stale. Expression.ScalarSubquery caches its type the same way. Whether these types should be cached on the POJOs at all is the broader question the issue raises; this is the short-term fix it asks for.

Two neighbouring bugs are deliberately left alone rather than folded in, and filed separately: the bound check in StructFieldFinder is off by one (#1068 — the exception it produces is part of what ProtoExpressionConverter reports for a malformed plan, so changing it is not a free fix), and MergeJoin.deriveRecordType reads its right input for both sides (#1067).

Behaviour worth calling out

• Where a narrowing rewrite previously threw from inside the visitor, it now yields a plan in which the unresolvable reference keeps its stale type. Anyone relying on that exception as a validation signal loses it — validation belongs in a validator, not in a copy-on-write rewrite.
• Re-derivation is unconditional, so a reference whose cached type disagreed with its input is corrected even when nothing in the subtree was replaced.
• Tracking the scope makes a visitor instance stateful for the duration of a traversal, so an instance can no longer be used to visit several relation trees concurrently. Sequential reuse is unaffected.
• visitComparisonJoinKey takes the two side record types now. It could not previously return a usable reference at all, so nothing can have depended on the old signature.

Three further bugs surfaced while wiring this up and are fixed as well:

• visitFieldReference built its replacement without copying the original, dropping the segments and the type. Since the type is mandatory, rewriting any reference rooted at an expression threw instead of returning the rewritten reference.
• The input of a ConsistentPartitionWindow was never visited, so no subtree beneath a window relation could be replaced.
• MultiBucketExchange reported no change when only its expression had been rewritten, discarding that rewrite.

Closes #185

🤖 Generated with AI