zhuqi-lucas commented on code in PR #16641: URL: https://github.com/apache/datafusion/pull/16641#discussion_r2181454354
########## datafusion/physical-optimizer/src/enforce_sorting/sort_pushdown.rs: ########## @@ -216,7 +218,25 @@ fn pushdown_sorts_helper( fn pushdown_requirement_to_children( plan: &Arc<dyn ExecutionPlan>, parent_required: OrderingRequirements, + parent_fetch: Option<usize>, ) -> Result<Option<Vec<Option<OrderingRequirements>>>> { + // Only attempt to push down TopK when there is an upstream LIMIT + if parent_fetch.is_some() { + // 1) Never push a new TopK below an operator that already has its own fetch + if plan.fetch().is_some() { + return Ok(None); + } + // 2) Only allow pushdown through operators that do not increase row count + // (equal cardinality). Any other operator (including joins, filter, + // sort-with-limit, or UDTFs that may expand rows) must stop the pushdown. + let effect = plan.cardinality_effect(); + if !matches!(effect, CardinalityEffect::Equal) { + return Ok(None); + } + // At this point, only single-input, non-expanding operators + // such as ProjectionExec, CoalesceBatchesExec, are allowed to receive TopK. + } Review Comment: Thank you @adriangb , exactly more concise and accurate solution! Merged your suggestion now. -- 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: github-unsubscr...@datafusion.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org For additional commands, e-mail: github-h...@datafusion.apache.org