gruuya commented on code in PR #21293:
URL: https://github.com/apache/datafusion/pull/21293#discussion_r3029913320
##########
datafusion/physical-plan/src/spill/in_progress_spill_file.rs:
##########
@@ -138,3 +142,81 @@ impl InProgressSpillFile {
Ok(self.in_progress_file.take())
}
}
+
+#[cfg(test)]
Review Comment:
I was actually of the opposite opinion—the e2e test is slightly unwieldy to
me, and convoluted in a way, since it tries to reproduce the bug using
non-standard primitives and hit the right spill conditions in an unnatural way
(because otherwise it's hard to repro). For instance union/repartition exec
used directly instead of sql/dataframe API, special task ctx, sort_batch called
directly on batches instead of wrapping with SortExec.
Also `InProgressSpillFile` spill file unit tests were missing anyway.
Either way, I'm fine with dropping either one (or none) of those, let me
know what you think.
--
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]
