martin-g commented on code in PR #21744:
URL: https://github.com/apache/datafusion/pull/21744#discussion_r3110784460
##########
datafusion/physical-optimizer/src/pushdown_sort.rs:
##########
@@ -149,14 +152,19 @@ impl PhysicalOptimizerRule for PushdownSort {
match sort_input.try_pushdown_sort(required_ordering)? {
SortOrderPushdownResult::Exact { inner } => {
// Data source guarantees perfect ordering - remove the
Sort operator.
- // Preserve the fetch (LIMIT) from the original SortExec
so the
- // data source can stop reading early.
- let inner = if let Some(fetch) = sort_exec.fetch() {
- inner.with_fetch(Some(fetch)).unwrap_or(inner)
+ //
+ // If the SortExec carried a fetch (LIMIT), we must
preserve it.
+ // First try pushing the limit into the source via
`with_fetch()`.
+ // If the source doesn't support `with_fetch`, fall back to
+ // wrapping with GlobalLimitExec.
+ if let Some(fetch) = sort_exec.fetch() {
+ let inner =
inner.with_fetch(Some(fetch)).unwrap_or_else(|| {
+ Arc::new(GlobalLimitExec::new(inner, 0,
Some(fetch)))
+ });
+ Ok(Transformed::yes(inner))
} else {
- inner
- };
- Ok(Transformed::yes(inner))
+ Ok(Transformed::yes(inner))
Review Comment:
Why this (old code) returns `Transformed::yes` ?
If there is no fetch/limit then there is no transformation.
##########
datafusion/physical-optimizer/src/pushdown_sort.rs:
##########
@@ -105,7 +106,9 @@ impl PhysicalOptimizerRule for PushdownSort {
match sort_input.try_pushdown_sort(required_ordering)? {
SortOrderPushdownResult::Exact { inner } => {
let inner = if let Some(fetch) = sort_child.fetch() {
- inner.with_fetch(Some(fetch)).unwrap_or(inner)
+ inner.with_fetch(Some(fetch)).unwrap_or_else(|| {
+ Arc::new(GlobalLimitExec::new(inner, 0,
Some(fetch)))
Review Comment:
Line 102 mentions multi-partitioning (`sort_child.preserve_partitioning()`)
but GlobalLimitExec requires single partitioning -
https://docs.rs/datafusion-physical-plan/53.1.0/src/datafusion_physical_plan/limit.rs.html#170
--
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]
