sunchao commented on code in PR #5409:
URL: https://github.com/apache/datafusion-comet/pull/5409#discussion_r3839055178
##########
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:
[P2] Preserve the explode metrics through the new wrapper
Replacing `UnnestExec` as the `SparkPlan` root also drops its SQL metrics.
`to_native_metric_node` reads this root's `metrics()` and then its `SparkPlan`
children; it never visits the wrapped native child. `BatchSplitExec::metrics()`
returns only `batches_split`, which `CometExplodeExec` does not expose, so its
existing input/output row and batch counters and `elapsed_compute` now remain
zero for every native explode, even when no split is needed. A focused Spark
4.0.4 probe over 8,192 short-string rows returned all 16,384 exploded rows, but
all five metrics were zero with the matching PR native artifact; the control
runtime reported `input_rows=8192` and `output_rows=16384`. The exact-source
Rust probe independently confirms that the populated unnest metrics are hidden
by the wrapper. Please preserve those metrics, including `output_rows` (the
existing additional-plan aggregation deliberately skips that field), and add a
metric regression check.
--
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]
