Skip to content

fix(window): give RANGE frames peer-group semantics (P24) - #56

Merged
TimelordUK merged 2 commits into
mainfrom
fix/p24-range-window-frames
Aug 30, 2026
Merged

fix(window): give RANGE frames peer-group semantics (P24)#56
TimelordUK merged 2 commits into
mainfrom
fix/p24-range-window-frames

Conversation

@TimelordUK

Copy link
Copy Markdown
Owner

A RANGE frame was evaluated as ROWS. get_frame_rows carried the comment "RANGE frame - based on ORDER BY values (not yet fully implemented) / For now, treat like ROWS" above a verbatim copy of the ROWS arm.

RANGE is defined over peer groups - all rows tying on every ORDER BY key - where ROWS counts physical rows. On distinct keys the two coincide, so this only shows up against ties, which is why it survived until null_edges.csv existed. With SUM(score) OVER (ORDER BY score) over scores 10,20,30,50,50,... the first 50 saw 110 instead of 160: its own peer excluded from its own frame.

The damaging half is the default frame. SUM(x) OVER (ORDER BY y) with no explicit frame means RANGE UNBOUNDED PRECEDING .. CURRENT ROW, and is the common way to write a running total - silently wrong wherever y has duplicates.

Both halves were one defect. The parser already synthesised the correct default frame, so the entire divergence lived in the evaluator and one fix closed both corpus cases.

  • OrderedPartition gains peer_bounds, computed in a single linear pass over the already-sorted rows, so all-equal keys cost no more than all-distinct.
  • CURRENT ROW resolves to the first row of the peer group as a start bound and the last as an end bound. That asymmetry is the whole mechanism, so win_range_frame_peer_start covers the start side the original cases missed.
  • Sorting and peer detection must agree exactly or frames land mid-group; both now route through one compare_by_sort_cols, and peers are the rows it calls Equal. With no ORDER BY that makes the partition a single peer group, which is what the standard specifies, with no special case.
  • RANGE with a numeric offset is value-based, not positional, and is now rejected at WindowContext construction instead of silently returning the ROWS answer. Filed as P33; ROWS offsets are untouched.

Parity 129 -> 133 AGREE (+2 fixed, +2 new coverage), GAP 13 -> 14 for P33. Explicit ROWS frames verified unchanged. Four unit tests cover peer groups from both bound sides, the ROWS contrast, and the offset rejection.

TimelordUK and others added 2 commits August 30, 2026 12:08
A RANGE frame was evaluated as ROWS. `get_frame_rows` carried the comment
"RANGE frame - based on ORDER BY values (not yet fully implemented) / For now,
treat like ROWS" above a verbatim copy of the ROWS arm.

RANGE is defined over *peer groups* - all rows tying on every ORDER BY key -
where ROWS counts physical rows. On distinct keys the two coincide, so this
only shows up against ties, which is why it survived until null_edges.csv
existed. With `SUM(score) OVER (ORDER BY score)` over scores 10,20,30,50,50,...
the first 50 saw 110 instead of 160: its own peer excluded from its own frame.

The damaging half is the default frame. `SUM(x) OVER (ORDER BY y)` with no
explicit frame means RANGE UNBOUNDED PRECEDING .. CURRENT ROW, and is the
common way to write a running total - silently wrong wherever y has duplicates.

Both halves were one defect. The parser already synthesised the correct default
frame, so the entire divergence lived in the evaluator and one fix closed both
corpus cases.

- OrderedPartition gains `peer_bounds`, computed in a single linear pass over
  the already-sorted rows, so all-equal keys cost no more than all-distinct.
- CURRENT ROW resolves to the first row of the peer group as a start bound and
  the last as an end bound. That asymmetry is the whole mechanism, so
  win_range_frame_peer_start covers the start side the original cases missed.
- Sorting and peer detection must agree exactly or frames land mid-group; both
  now route through one `compare_by_sort_cols`, and peers are the rows it calls
  Equal. With no ORDER BY that makes the partition a single peer group, which
  is what the standard specifies, with no special case.
- RANGE with a numeric offset is value-based, not positional, and is now
  rejected at WindowContext construction instead of silently returning the ROWS
  answer. Filed as P33; ROWS offsets are untouched.

Parity 129 -> 133 AGREE (+2 fixed, +2 new coverage), GAP 13 -> 14 for P33.
Explicit ROWS frames verified unchanged. Four unit tests cover peer groups from
both bound sides, the ROWS contrast, and the offset rejection.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…P24 bug

The Examples Test Suite was the only failing CI job. Its formal test
window_functions compares against examples/expectations/window_functions.json,
captured back when RANGE was evaluated as ROWS, so it had frozen the defect:
query 9's LAST_VALUE(sales_amount) OVER (PARTITION BY region ORDER BY month)
recorded each row's OWN amount.

With no explicit frame the default is RANGE UNBOUNDED PRECEDING AND CURRENT
ROW, so the frame ends at the last PEER - every row tying on month - and both
salespeople in a region/month report the same value. Checked against DuckDB
before touching anything: it returns the new output on all 24 rows, so the
capture was wrong and the fix is right. Re-captured on that basis.

The diff is confined to last_sale_in_frame (12 values); no other field moved, so
the re-capture bakes in nothing else. The suite's other failures are pre-existing
smoke-test noise (missing fixtures, unreachable URLs) and do not fail the job -
only FORMAL mismatches do, which is why this job passed on main.

- Re-capture examples/expectations/window_functions.json.
- Correct the comment above query 9, which documented the old row-at-a-time
  reading and would have sent the next reader the wrong way.
- Add corpus case win_last_value_default_frame: SUM reads the whole frame,
  LAST_VALUE only its final row, so it pins the end bound where the SUM cases
  cannot. Parity 133 -> 134 AGREE.
- Record the pattern in SQL_PARITY.md: when a fix breaks a captured test,
  establish which side is right against the reference engine before touching
  either. Re-capturing reflexively would have silently re-frozen the bug.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@TimelordUK
TimelordUK merged commit 421ac71 into main Aug 30, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant