sam-1112 commented on code in PR #5409:
URL: https://github.com/apache/datafusion-comet/pull/5409#discussion_r3841796438
##########
native/core/src/execution/planner.rs:
##########
@@ -2116,11 +2116,20 @@ impl PhysicalPlanner {
output_schema,
unnest_options,
)?);
+ // DataFusion 54.1.0's UnnestExec can emit more than the
runtime batch size.
+ // Bound batches before downstream native projections until
Comet upgrades to a
+ // DataFusion version containing
https://github.com/apache/datafusion/pull/24384.
+ let bounded_unnest: Arc<dyn ExecutionPlan> =
+ Arc::new(BatchSplitExec::new(unnest_exec));
Ok((
scans,
shuffle_scans,
- Arc::new(SparkPlan::new(spark_plan.plan_id, unnest_exec,
vec![child])),
+ Arc::new(SparkPlan::new(
+ spark_plan.plan_id,
+ bounded_unnest,
+ vec![child],
Review Comment:
Thanks for catching this. I kept `BatchSplitExec` as the execution root, but
made it transparent to metric collection: `BatchSplitExec::metrics()` now
returns its own split metrics together with all metrics from the wrapped
`UnnestExec`.
This preserves `input_rows`, `output_rows`, `input_batches`,
`output_batches`, and `elapsed_compute`. It does not use
`additional_native_plans`, so `output_rows` is not dropped by that path’s
deliberate filtering. The wrapper’s `batches_split` metric is retained as well.
I added two regression checks:
- The Spark reproducer with 8,192 input rows expanding to 16,384 rows now
asserts `input_rows == 8192`, `output_rows == 16384`, and non-zero input/output
batch counters on `CometExplodeExec`.
- The Rust unit test wraps an input with an `output_rows` metric, executes
it through `BatchSplitExec`, and verifies that `BatchSplitExec::metrics()`
exposes `output_rows == 10`.
The focused Rust test and the Spark 4.1 reproducer both pass.
--
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]
