Skip to content

Support equality with one in opt-in WindowTopN rule - #26160

Open
osipovartem wants to merge 1 commit into
apache:mainfrom
Embucket:upstream-window-topn-eq-one
Open

osipovartem wants to merge 1 commit into
apache:mainfrom
Embucket:upstream-window-topn-eq-one

Conversation

@osipovartem

Copy link
Copy Markdown
Contributor

What changes

  • Recognize rank_col = 1 and 1 = rank_col in the opt-in WindowTopN rule. Other equalities remain untouched.
  • Add physical-plan and result regressions for ROW_NUMBER, RANK (including ties), and DENSE_RANK. Preserve the existing sibling-window safety guard.
  • Leave enable_window_topn disabled by default.

Validation

  • cargo +stable test --profile ci -p datafusion-sqllogictest --test sqllogictests -- window_topn.slt --test-threads 1
  • cargo +stable test --profile ci -p datafusion-physical-optimizer --lib (38 passed)
  • cargo +stable clippy --profile ci -p datafusion-physical-optimizer --all-targets -- -D warnings
  • cargo +stable fmt --all --check
  • Independent read-only review approved correctness, regressions, performance, API compatibility, and tests.

Performance scope

Five local synthetic 100k-row runs (median query elapsed): 100 groups, rule off/on 31/18 ms; 90k groups, rule off/on 120/140 ms. High-cardinality plans can regress, which is why this remains opt-in. These numbers are not a Snowplow benchmark. A pushed-down FilterExec projection can also prevent this rewrite. This PR does not change either behavior.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.77%. Comparing base (06aa131) to head (c60ed5a).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #26160      +/-   ##
==========================================
- Coverage   82.77%   82.77%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      450909   450930      +21     
  Branches   450909   450930      +21     
==========================================
+ Hits       373252   373264      +12     
- Misses      54934    54938       +4     
- Partials    22723    22728       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants