Skip to content

refactor(window): consume explicitly sorted input#378

Merged
KKould merged 1 commit into
feat/spillfrom
feat/window-external-sort-spill
Jul 21, 2026
Merged

refactor(window): consume explicitly sorted input#378
KKould merged 1 commit into
feat/spillfrom
feat/window-external-sort-spill

Conversation

@KKould

@KKould KKould commented Jul 21, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue link:

Window execution currently performs its own in-memory sort and materializes the full input. This couples Window to one sorting implementation and prevents the existing spill-backed ExternalSort from satisfying Window ordering.

What is changed and how it works?

Window now consumes explicitly sorted input instead of sorting internally. The binder inserts a Sort on PARTITION BY + ORDER BY; native builds with the spill feature therefore use the existing ExternalSort executor, while non-spill and WASM builds continue to use the in-memory Sort implementation.

The Window executor processes ordered input incrementally and retains only the rows required by the current function set: one row, one peer group, or one partition. Partition and peer boundaries are detected from cached sort values, and a single pending row carries a boundary across buffered output.

Code changes

  • Has Rust code change
  • Has CI related scripts change

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)
  • No code

Window unit coverage verifies row-, peer-, and partition-retention behavior. SQL logic coverage verifies ranking, aggregate windows, multiple window groups, partition boundaries, peer ties, views, and explain output. The spill-enabled build also compiles successfully.

Side effects

  • Performance regression: Consumes more CPU
  • Performance regression: Consumes more Memory
  • Breaking backward compatibility

Note for reviewer

The primary goal is to decouple Window from Sort. Window only requires ordered input; with spill enabled, the existing Sort execution path selects ExternalSort and provides spill support without adding spill-specific code to Window.

The repository-wide clippy target remains blocked by pre-existing warnings outside this change: an unused parser import, unused optimizer pattern/matcher code, and manual_is_multiple_of in the existing ExternalSort test.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.21260% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.90%. Comparing base (506bd78) to head (449219b).

Files with missing lines Patch % Lines
src/execution/dql/window.rs 99.05% 1 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff               @@
##           feat/spill     #378      +/-   ##
==============================================
- Coverage       92.91%   92.90%   -0.01%     
==============================================
  Files             253      253              
  Lines           43141    43190      +49     
==============================================
+ Hits            40084    40126      +42     
- Misses           3057     3064       +7     
Flag Coverage Δ
rust 92.90% <99.21%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
src/binder/window.rs 96.07% <100.00%> (+0.18%) ⬆️
src/execution/dql/window/function.rs 97.29% <100.00%> (+0.07%) ⬆️
src/execution/dql/window.rs 99.28% <99.05%> (+0.29%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KKould KKould added the enhancement New feature or request label Jul 21, 2026
@KKould KKould self-assigned this Jul 21, 2026
@KKould
KKould merged commit 16265d0 into feat/spill Jul 21, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant