feat: eliminate GlobalLimitExec when input statistics prove limit is already satisfied#22150
feat: eliminate GlobalLimitExec when input statistics prove limit is already satisfied#22150xiedeyantu wants to merge 3 commits into
Conversation
…already satisfied
There was a problem hiding this comment.
@xiedeyantu
Thanks for the cleanup and improvements here. I went through the changes and only had one small suggestion around clarifying the invariant in the helper logic.
| Ok(num_rows <= fetch) | ||
| } | ||
|
|
||
| /// Returns exact row counts only for operators whose cardinality guarantee is |
There was a problem hiding this comment.
The helper comment mentions returning exact row counts for operators whose guarantees are strong enough, but the implementation is intentionally a pretty small whitelist (EmptyExec, PlaceholderRowExec, and exact zero stats). It might help to clarify in the comment that this is meant to be a conservative whitelist.
Another option would be adding a small test or note explaining why a general Precision::Exact(n <= fetch) is not considered safe enough here. I think that would make the invariant a bit clearer for future extensions.
There was a problem hiding this comment.
I removed the related description. Frankly, I am unsure whether Precision::Exact(n <= fetch) can serve as a reliable basis; specifically, if a FILTER is present, would the row count become imprecise? Therefore, I opted for a conservative approach.
Which issue does this PR close?
Rationale for this change
GlobalLimitExec(andLocalLimitExec) are sometimes redundant: if the input can be proven via exact statistics to produce no more rows than the fetch value, the limit node does nothing and should be removed entirely.Previously, the
LimitPushdownrule had no mechanism to eliminate such trivially-satisfied limits. A query likeSELECT * FROM (VALUES ...) LIMIT 10— where the input is a single-rowPlaceholderRowExec— still carried an unnecessaryGlobalLimitExecin the physical plan. Similarly, aLIMIT Nover anEmptyExecor any zero-row plan was retained.What changes are included in this PR?
limit_satisfied_by_input()inlimit_pushdown.rs: checks whether a plan's child provably produces at mostfetchrows (requiresskip == 0and a single output partition).limit_eliminable_exact_num_rows(): iteratively unwrapsProjectionExecwrappers and recognisesEmptyExec(0 rows),PlaceholderRowExec(1 row), and any plan reportingPrecision::Exact(0)rows as eliminable producers.global_state.satisfied = trueand returns early — without resettingfetch/skip— so nested limit nodes still receive the correct outer constraints to merge against.merges_local_limit_with_local_limitsnapshot: the result is now bareEmptyExec(limit eliminated).union.slt:ProjectionExecoverPlaceholderRowExec(1 row) withfetch=3no longer carries a redundantGlobalLimitExec.explain_tree.slttest:SELECT count(*) … LIMIT 10over a two-row VALUES clause is correctly reduced toProjectionExec → PlaceholderRowExecwith no limit node.fetch=10is now correctly pushed all the way down toDataSourceExec.Are these changes tested?
Yes.
cargo fmt --allcargo clippy --all-targets --all-features -- -D warningscargo test -p datafusion-core --test physical_optimizer limitcargo test --features backtrace,parquet_encryption --profile ci --package datafusion-sqllogictest --test sqllogictests -- copy.slt union.slt explain_tree.sltAre there any user-facing changes?
No API changes. Physical plans for queries with
LIMITover statically small inputs (EmptyExec,PlaceholderRowExec, or zero-row tables) will now have the redundantGlobalLimitExec/LocalLimitExecnodes eliminated, resulting in simpler and slightly more efficient plans.