yonipeleg33 commented on code in PR #23606:
URL: https://github.com/apache/datafusion/pull/23606#discussion_r3594247036
##########
datafusion/physical-plan/src/sorts/merge.rs:
##########
@@ -375,7 +385,10 @@ impl<C: CursorValues> SortPreservingMergeStream<C> {
let _ = cursor.advance();
if cursor.is_finished() {
// Take the current cursor, leaving `None` in its place
- self.prev_cursors[stream_idx] =
self.cursors[stream_idx].take();
+ let taken = self.cursors[stream_idx].take();
+ if self.enable_round_robin_tie_breaker {
Review Comment:
IIUC, perv_cursors is only used when tie breaking is enabled, right? If so,
consider making `self.prev_cursors` optional so that it'll be `None` when tie
breaking is disabled; this will make the code in this file more readable in
general, and specifically this fix will be more semantic.
If not, at least consider adding a comment here:
```rust
// `perv_cursors` is not used when tie-breaking is disabled; avoid saving
unnecessary cursors which inflate memory
```
--
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]
