mustafasrepo commented on code in PR #5661:
URL: https://github.com/apache/arrow-datafusion/pull/5661#discussion_r1142949196
##########
datafusion/core/src/physical_optimizer/sort_enforcement.rs:
##########
@@ -439,62 +426,64 @@ fn ensure_sorting(
let physical_ordering = child.output_ordering();
match (required_ordering, physical_ordering) {
(Some(required_ordering), Some(physical_ordering)) => {
- let is_ordering_satisfied = ordering_satisfy_concrete(
+ if !ordering_satisfy_requirement_concrete(
physical_ordering,
- required_ordering,
+ &required_ordering,
|| child.equivalence_properties(),
- );
- if !is_ordering_satisfied {
+ ) {
// Make sure we preserve the ordering requirements:
update_child_to_remove_unnecessary_sort(child,
sort_onwards, &plan)?;
- let sort_expr = required_ordering.to_vec();
+ let sort_expr =
make_sort_exprs_from_requirements(&required_ordering);
add_sort_above(child, sort_expr)?;
- *sort_onwards = Some(ExecTree::new(child.clone(), idx,
vec![]));
- }
- if let Some(tree) = sort_onwards {
- // For window expressions, we can remove some sorts when
we can
- // calculate the result in reverse:
- if plan.as_any().is::<WindowAggExec>()
- || plan.as_any().is::<BoundedWindowAggExec>()
- {
- if let Some(result) =
analyze_window_sort_removal(tree, &plan)? {
- return Ok(Some(result));
- }
+ if is_sort(child) {
+ *sort_onwards = Some(ExecTree::new(child.clone(), idx,
vec![]));
+ } else {
+ *sort_onwards = None;
}
}
}
(Some(required), None) => {
// Ordering requirement is not met, we should add a `SortExec`
to the plan.
- add_sort_above(child, required.to_vec())?;
+ let sort_expr = make_sort_exprs_from_requirements(&required);
+ add_sort_above(child, sort_expr)?;
*sort_onwards = Some(ExecTree::new(child.clone(), idx,
vec![]));
}
(None, Some(_)) => {
// We have a `SortExec` whose effect may be neutralized by
- // another order-imposing operator. Remove or update this sort:
- if !plan.maintains_input_order()[idx] {
- let count = plan.output_ordering().map_or(0, |e| e.len());
- if (count > 0) && !is_sort(&plan) {
- update_child_to_change_finer_sort(child, sort_onwards,
count)?;
Review Comment:
Because of pushdown routine we no longer need to change below sorts with
finer sorts. This handling can be removed from here.
--
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]
