geoffreyclaude commented on code in PR #26118:
URL: https://github.com/apache/datafusion/pull/26118#discussion_r4378998269
##########
datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs:
##########
@@ -1085,12 +1072,128 @@ fn handle_custom_pushdown(
})
.collect();
+ // A custom node can preserve row positions while reversing value order
+ // (e.g. `-a AS a`). Keep the outer sort unless the proposed child
sorts
+ // satisfy the node's input requirements and give its output the
+ // requested ordering.
+ if !is_custom_sort_pushdown_valid(plan, &result, requirement)? {
+ return Ok(None);
+ }
Ok(Some(result))
} else {
Ok(None)
}
}
+/// Checks that proposed child orderings satisfy the node's input requirements
+/// and produce the requested output ordering.
+fn is_custom_sort_pushdown_valid(
+ plan: &Arc<dyn ExecutionPlan>,
+ child_requirements: &[Option<OrderingRequirements>],
+ output_requirement: LexRequirement,
+) -> Result<bool> {
+ let plan_children = plan.children();
+
+ // Test the proposed child orderings without moving the real sorts yet.
+ let mut probe_children = Vec::with_capacity(plan_children.len());
+ for (child, required) in plan_children.iter().zip(child_requirements) {
+ let child = if let Some(required) = required {
+ // Other order-maintaining inputs may have narrower schemas.
+ if required.first().iter().any(|sort| {
+ collect_columns(&sort.expr)
+ .iter()
+ .any(|column| column.index() >=
child.schema().fields().len())
+ }) {
+ return Ok(false);
+ }
+ // A bounded operator can stop an unbounded input; a new blocking
+ // sort below it cannot. An already ordered input needs no new
sort.
+ if child.boundedness().is_unbounded()
Review Comment:
Added both cases in a separate commit. I checked that removing either guard
makes its corresponding test fail.
--
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]
