Fix SortExec bench case and Add SortExec input cases to bench for SortPreservingMergeExec #5308
Conversation
jaylmiller
changed the title
| let plan = Arc::new(SortExec::try_new(sort, Arc::new(exec), None).unwrap()); | ||
| let exec = Arc::new(CoalescePartitionsExec::new(Arc::new(exec))); | ||
| let plan = Arc::new(SortExec::try_new(sort, exec, None).unwrap()); |
There was a problem hiding this comment.
without this change it was only ever benchmarking the case where the execute receives a single record batch to sort.
|
Benchmark runs are scheduled for baseline = 222205d and contender = e13c653. e13c653 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
…tPreservingMergeExec (apache#5308) * add SortExec input case to each merge bench case * fix lil typo error in sort bench * fix sort bench to actually use full data set in non-preserve partition case
Which issue does this PR close?
#5230 (does not close it.)
Rationale for this change
The current SortPreservingMergeExec benchmarks take MemoryExec as input. In #5230 we want to modify SortPreservingMergeExec and SortExec so that when row encoding is used within the sort node, it is preserved for usage by the merge node.
Which means, we need bench cases on main for SortPreservingMergeExec when it has SortExec as input to see if the changes for #5230 are helping perf.
Additionally, the logic in the SortExec bench for the case where
preserve_partitioning=falsewas flawed, so that has been fixed too.What changes are included in this PR?
Are these changes tested?
n/a
Are there any user-facing changes?
no