gabotechs commented on code in PR #26890:
URL: https://github.com/apache/datafusion/pull/26890#discussion_r4525757400
##########
docs/source/library-user-guide/upgrading/56.0.0.md:
##########
@@ -1336,3 +1306,33 @@ without declaring one, use
`FileScanConfigBuilder::with_preserve_order(true)`.
Code that reads `FileScanConfig::preserve_order` to decide whether files may be
reordered should call `FileScanConfig::preserves_file_order()` instead, which
also accounts for the output ordering.
+
+### Substrait consumers use explicit scope context
+
+Relation and expression callbacks on `SubstraitConsumer` now take a final
+`&SubstraitConsumerContext` argument. Forward that context when delegating to
the
+standard conversion helpers or consuming child relations and expressions.
+`from_substrait_plan` and `from_substrait_plan_with_consumer` still create the
root
+context for you. When starting a new conversion directly through a helper, use
+`SubstraitConsumerContext::default()`; nested conversions must receive the
current
+context.
+
+Remove implementations of `push_outer_schema`, `pop_outer_schema`,
+`get_outer_schema`, `push_lambda_parameters`, `pop_lambda_parameters`, and
+`lambda_variable`, and remove embedded `DefaultSubstraitLambdaConsumer` values.
+The standard handlers resolve scopes through the context. Custom handlers that
+introduce scopes should use `context.with_outer_schema(...)` or
+`context.with_lambda_parameters(...)` and pass the resulting child context to
+nested conversions. There is no matching pop operation; the parent context
stays
+unchanged on success, error, and cancellation.
+
+Input consumption remains sequential. Custom consumers
Review Comment:
Also, `branch-56` was already cut, and this PR is not there, so this might
belong to the 57 upgrade guide instead.
##########
docs/source/library-user-guide/upgrading/56.0.0.md:
##########
@@ -1336,3 +1306,33 @@ without declaring one, use
`FileScanConfigBuilder::with_preserve_order(true)`.
Code that reads `FileScanConfig::preserve_order` to decide whether files may be
reordered should call `FileScanConfig::preserves_file_order()` instead, which
also accounts for the output ordering.
+
+### Substrait consumers use explicit scope context
+
+Relation and expression callbacks on `SubstraitConsumer` now take a final
+`&SubstraitConsumerContext` argument. Forward that context when delegating to
the
+standard conversion helpers or consuming child relations and expressions.
+`from_substrait_plan` and `from_substrait_plan_with_consumer` still create the
root
+context for you. When starting a new conversion directly through a helper, use
+`SubstraitConsumerContext::default()`; nested conversions must receive the
current
+context.
+
+Remove implementations of `push_outer_schema`, `pop_outer_schema`,
+`get_outer_schema`, `push_lambda_parameters`, `pop_lambda_parameters`, and
+`lambda_variable`, and remove embedded `DefaultSubstraitLambdaConsumer` values.
+The standard handlers resolve scopes through the context. Custom handlers that
+introduce scopes should use `context.with_outer_schema(...)` or
+`context.with_lambda_parameters(...)` and pass the resulting child context to
+nested conversions. There is no matching pop operation; the parent context
stays
+unchanged on success, error, and cancellation.
+
+Input consumption remains sequential. Custom consumers
Review Comment:
This looks like the classical LLM correction over a previous state of this
PR that is not going to be shipped. I'd revisit the upgrade guide.
##########
datafusion/substrait/tests/cases/logical_plans.rs:
##########
@@ -545,7 +558,7 @@ mod tests {
assert_snapshot!(
plan,
@"
- Projection: array_transform2(make_array(make_array(data3.p1)), (p0,
p2) -> array_concat(array_transform2(p0, (p3, p4) -> p3 * p2 * p4),
array_transform2(p0, (p5, p6) -> p5 * p2 * p6))) AS
array_transform2(make_array(make_array(data3.p1)),(v, i) ->
array_concat(array_transform2(v,(v, j) -> v * i * j),array_transform2(v,(v, j)
-> v * i * j)))
+ Projection: array_transform2(make_array(make_array(data3.p1)), (p0,
p2) -> array_concat(array_transform2(p0, (p3, p4) -> p3 * p2 * p4),
array_transform2(p0, (p3, p4) -> p3 * p2 * p4))) AS
array_transform2(make_array(make_array(data3.p1)),(v, i) ->
array_concat(array_transform2(v,(v, j) -> v * i * j),array_transform2(v,(v, j)
-> v * i * j)))
TableScan: data3
Review Comment:
🤔 at first I thought there was something wrong with this snapshot change,
but it does look correct, just that the argument numeration is not global to
whole plan, but to the specific plan branch in which that `array_transform`
happened to fall.
--
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]
