discord9 opened a new pull request, #25069: URL: https://github.com/apache/datafusion/pull/25069
## Which issue does this PR close? No linked issue. Independently reproduced physical-rule correctness fix. ## Rationale for this change WindowTopN accepts projections between a filter and its window, but interprets the filter's column index in the window's schema without remapping it. A projection can place a data column at the original ranking-column index, causing the rule to remove a real data filter and return incorrect rows. For example, window output `[pk, val, rn]` projected to `[pk, rn, val]` makes `val <= 1` refer to index 2. With input `pk=1, val=10`, the original plan returns no rows; the rule mistakes that index for `rn`, inserts top-1, removes the filter, and returns the row. This is reproduced through the direct physical-rule API with WindowTopN enabled. The option defaults to false. The equivalent SQL tested retains the correct filter after earlier normalization; this PR does not claim a default-SQL failure. ## What changes are included in this PR? Remap a local copy of the predicate through intermediate projections, in filter-to-window order, using the existing positional `unproject_expr` helper. Only then identify a supported ranking-column bound. Unsupported remaps or computed predicates leave the original plan unchanged. Intermediate reconstruction remains unchanged. No new mapping framework, public API, or configuration is introduced. This is independent of the TopK aggregation window-boundary fix in #25066. ## What is the testing strategy for this PR? Native execution coverage checks: - a reordered data predicate that must return no rows; - a reordered data predicate retaining two rows, rather than truncating them to top-1; - a ranking predicate through two reordered/aliased projections that still optimizes and returns the correct top-K; - a computed ranking expression that must remain unsupported and preserve its result. Removing only predicate remapping makes the first regression return one row instead of zero. The source was restored byte-exactly, and all 20 WindowTopN tests passed afterward. Verified locally: - `cargo check -p datafusion-physical-optimizer` - `cargo test -p datafusion --test core_integration physical_optimizer::window_topn::` — 20 passed - `cargo fmt --all --check` - `cargo clippy --workspace --all-targets --all-features -- -D warnings` The full workspace runtime test suite was not run. ## Are there any user-facing changes? Correct results when the enabled physical WindowTopN rule encounters projected predicates. No public API or configuration changes. -- 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]
