clflushopt opened a new pull request, #23488:
URL: https://github.com/apache/datafusion/pull/23488

   ## Rationale for this change
   
   The Substrait producer errors on `Expr::OuterReferenceColumn`, so any plan 
containing a correlated subquery cannot be serialized currently. DataFusion's 
round-trip tests don't hit this because the optimizer decorrelates subqueries 
into joins before serialization, but any workflow that serializes *unoptimized* 
plans (e.g. sending raw plans between systems for later optimization in my 
case) fails on queries like TPC-H q2/q4/q17/q20/q21/q22, and ~26 cases in 
`joins.slt` fail in `--substrait-round-trip` mode.
   
   A previous attempt (#18987) was closed because it introduced a non-standard 
mechanism for outer references, producing plans only DataFusion would be able 
consume. Substrait represents correlated references natively: a 
`FieldReference` with an `OuterReference` root type and a `steps_out` depth. 
The consumer side of exactly this representation was implemented in #20439, 
which resolves `OuterReference` field references against a stack of outer 
schemas. This PR implements the producing half, symmetric with that design, so 
correlated plans round-trip using only standard Substrait.
   
   ## What changes are included in this PR?
   
   - `SubstraitProducer` gains outer-schema-stack methods mirroring 
`SubstraitConsumer`: `push_outer_schema` / `pop_outer_schema` (default no-ops) 
and `get_outer_schema(steps_out)` (default `None`), plus a 
`handle_outer_reference_column` method so custom producers can override the 
behaviour like every other expression kind. Defaults are backward compatible: 
existing custom producers are unaffected unless a plan actually contains an 
outer reference, in which case they now get an actionable error instead of 
`not_impl_err`.
   - `DefaultSubstraitProducer` maintains the stack in a `Vec<DFSchemaRef>`.
   - The four subquery producers (`from_in_subquery`, `from_scalar_subquery`, 
`from_exists`, `from_set_comparison`) push the enclosing query's schema around 
the subquery plan conversion (via a shared `produce_subquery_rel`, analogous to 
the consumer's `consume_subquery_rel`).
   - `from_outer_reference_column` (previously unused and emitting an incorrect 
plain `RootReference`) now resolves the column against the outer-schema stack, 
innermost first, and emits a `FieldReference` with an `OuterReference` root and 
the corresponding `steps_out`.
   - `to_substrait_rex` dispatches `Expr::OuterReferenceColumn` to the new 
handler instead of erroring.
   
   ## Are these changes tested?
   
   Yes:
   
   - New round-trip tests in `roundtrip_logical_plan.rs` covering correlated 
`EXISTS`, correlated `IN` subquery, correlated scalar subquery, and a nested 
correlated subquery that crosses two subquery boundaries (`steps_out = 2`). 
Each test asserts the produced plan contains an `OuterReference` at the 
expected depth and that the plan round-trips through the existing consumer with 
its schema intact.
   - Consumer-side resolution was already covered by the tests added in #20439; 
these tests now exercise both halves together.
   - `joins.slt` in `--substrait-round-trip` mode goes from 38 failures to 12; 
the remaining failures are pre-existing gaps unrelated to outer references 
(`USING` join constraint, plan-level lateral `LogicalPlan::Subquery`, duplicate 
unqualified field names).
   
   ## Are there any user-facing changes?
   
   - Plans containing correlated subqueries now serialize instead of returning 
"not implemented", emitting spec-standard `OuterReference` field references.
   - `SubstraitProducer` has three new provided methods and 
`handle_outer_reference_column`; all have defaults, so existing implementations 
continue to compile.
   - The signature of the public helper `from_outer_reference_column` changed 
(it now takes the producer and the outer field) — its previous form resolved 
against the wrong schema and emitted a plain `RootReference`, and it was not 
called from anywhere in the crate.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to