jayzhan211 commented on code in PR #24008:
URL: https://github.com/apache/datafusion/pull/24008#discussion_r3837657837
##########
datafusion/physical-plan/src/aggregates/ordered_final_stream.rs:
##########
@@ -245,13 +218,13 @@ impl OrderedFinalSpillContext {
baseline_metrics.clone(),
group_by_metrics,
None,
- )?;
- Ok(Box::pin(replay))
+ )?
+ .into_stream())
Review Comment:
```suggestion
// NOTE: deliberately NOT `into_stream()`: the replay shares
// `baseline_metrics` with the outer stream, whose `ObservedStream`
// already records every forwarded batch. Wrapping the replay in a
// second `ObservedStream` would count `output_rows` twice.
let replay = OrderedFinalAggregateStream::new_with_input_and_metrics(
&agg,
&context,
partition,
merged,
&InputOrderMode::Sorted,
baseline_metrics.clone(),
group_by_metrics,
None,
)?;
let replay_schema = Arc::clone(&replay.schema);
Ok(Box::pin(RecordBatchStreamAdapter::new(
replay_schema,
replay.aggregate(),
)))
```
##########
datafusion/physical-plan/src/aggregates/ordered_final_stream.rs:
##########
@@ -245,13 +218,13 @@ impl OrderedFinalSpillContext {
baseline_metrics.clone(),
group_by_metrics,
None,
- )?;
- Ok(Box::pin(replay))
+ )?
+ .into_stream())
Review Comment:
The PR drops the manual batch.record_output(...) calls and lets
ObservedStream record metrics instead — correct for the in-memory path. But the
spill path nests two ObservedStreams over the same BaselineMetrics:
into_replay_stream builds the replay with .into_stream(), which wraps it, and
then every replayed batch gets forwarded through emitter.emit(...) and recorded
a second time by the outer wrapper.
--
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]
