rluvaton commented on code in PR #23407:
URL: https://github.com/apache/datafusion/pull/23407#discussion_r3564825793
##########
datafusion/physical-plan/src/sorts/merge.rs:
##########
@@ -171,24 +152,40 @@ impl<C: CursorValues> SortPreservingMergeStream<C> {
in_progress: BatchBuilder::new(schema, stream_count, batch_size,
reservation),
streams,
metrics,
- done: false,
- drain_in_progress_on_done: false,
cursors: (0..stream_count).map(|_| None).collect(),
prev_cursors: (0..stream_count).map(|_| None).collect(),
round_robin_tie_breaker_mode: false,
num_of_polled_with_same_value: vec![0; stream_count],
current_reset_epoch: 0,
poll_reset_epochs: vec![0; stream_count],
loser_tree: vec![],
- loser_tree_adjusted: false,
batch_size,
fetch,
produced: 0,
- uninitiated_partitions: (0..stream_count).collect(),
enable_round_robin_tie_breaker,
}
}
+ pub(crate) fn into_stream(self) -> SendableRecordBatchStream
+ where
+ C: 'static,
+ {
+ let schema_clone = Arc::clone(self.in_progress.schema());
+
+ let cloned_metrics = self.metrics.clone();
+
+ let mut this = self;
+ let stream = Gen::new(|co| async move {
Review Comment:
@stuhood
> [...] both for the error handling differences,
If you don't handle the result you will have:
```
unused Result that must be used
this Result may be an Err variant, which should be handled
#[warn(unused_must_use)] (part of #[warn(unused)]) on by default (rustc
unused_must_use)
```
> [...] as well as because it does not introduce the requirement that the
stream be `Send`.
`SendableRecordBatchStream` which is what actually being used is required to
be `Send` anyway.
https://github.com/apache/datafusion/blob/9ec28bda1f7e410f1872c51021bbc910f6ef3ac5/datafusion/execution/src/stream.rs#L60
And the one most important thing for me is that you can't create a function
that does yield, for example, one of the optimization I'm doing for sort is
passthrough last stream, having all the logic in the same code block makes it
harder to read, this pr is for making it more readable and easier to extend,
not less
--
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]
