Skip to content

feat(frontend-sql): model SQL window frame clauses (ROWS/RANGE BETWEEN ...) - #270

Merged
zzylol merged 3 commits into
mainfrom
issue-268-window-frame
Aug 25, 2026
Merged

zzylol merged 3 commits into
mainfrom
issue-268-window-frame

Conversation

@milindsrivastava1997

@milindsrivastava1997 milindsrivastava1997 commented Aug 24, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • SQLWindowFunc previously dropped the window-frame clause entirely. A default frame, ROWS BETWEEN 2 PRECEDING AND CURRENT ROW, and ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING therefore lowered to structurally identical IR.
  • This PR adds first-class WindowFrame, WindowFrameUnits, WindowFrameBound, and typed WindowFrameOffset values to the canonical IR.
  • lower_window now preserves DataFusion's resolved frame, including its context-dependent default.
  • Offset RANGE ... INTERVAL bounds are normalized to month/day/nanosecond components instead of remaining opaque DataFusion 43 strings such as "1 HOUR".
  • Legacy serialized SQLWindowFunc nodes that predate the frame field remain readable through #[serde(default)] frame: Option<WindowFrame>; newly lowered SQL always produces Some(frame).

These defaults follow DataFusion's documented behavior: with ORDER BY, the default ends at the current row; without ORDER BY, it covers the entire partition. None is reserved for legacy IR because one static fallback could not reconstruct both cases safely.

Before / after IR

Example:

SELECT service, SUM(latency) OVER (
  PARTITION BY service ORDER BY ts
  ROWS BETWEEN 2 PRECEDING AND CURRENT ROW
) FROM metrics

Before — no machine-readable frame:

SQLWindowFunc {
    func: Sum,
    args: [Column(2)],
    partition_by: GroupKeys { keys: [1], without: false },
    order_by: [SortKey { expr: Column(0), ascending: true, nulls_first: false }],
    output_name: "... ROWS BETWEEN 2 PRECEDING AND CURRENT ROW",
    child: Scan { .. },
}

After — the resolved frame is part of the IR:

SQLWindowFunc {
    func: Sum,
    args: [Column(2)],
    partition_by: GroupKeys { keys: [1], without: false },
    order_by: [SortKey { expr: Column(0), ascending: true, nulls_first: false }],
    frame: Some(WindowFrame {
        units: Rows,
        start_bound: Preceding(Scalar(Int64(2))),
        end_bound: CurrentRow,
    }),
    output_name: "... ROWS BETWEEN 2 PRECEDING AND CURRENT ROW",
    child: Scan { .. },
}

An interval RANGE bound is typed as:

Preceding(Interval {
    months: 0,
    days: 0,
    nanoseconds: 3_600_000_000_000,
})

for INTERVAL '1' HOUR.

Scope

  • ROWS and RANGE are modeled.
  • GROUPS is explicitly rejected rather than silently mis-lowered. DataFusion supports it, but it remains outside this PR's declared scope.
  • This PR preserves frame semantics for downstream consumers; it does not add a post-ASAP frame-execution rule.

Tests

  • window_frame_is_captured_not_dropped
  • range_interval_frame_is_preserved
  • groups_frame_is_rejected
  • legacy_window_json_without_frame_deserializes_as_unspecified
  • cargo test -p asap-frontend-sql -p asap-types
  • CI format-and-lint and test

Fixes #268

…N ...)

SQLWindowFunc previously dropped the frame clause entirely — a default
frame, ROWS BETWEEN 2 PRECEDING AND CURRENT ROW, and ROWS BETWEEN
CURRENT ROW AND UNBOUNDED FOLLOWING all lowered to identical IR, so
every frame-sensitive window function (SUM, AVG, COUNT, MIN, MAX,
FIRST_VALUE, LAST_VALUE, NTH_VALUE) with a non-default frame was
silently wrong. Adds WindowFrame/WindowFrameUnits/WindowFrameBound to
the canonical IR and threads it through lowering, CSE, resolve, and
DAG export. GROUPS frames are rejected explicitly (unused in this
repo's SQL corpora, unsupported downstream).

Fixes #268

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol

zzylol commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Fixed the review findings in commit 724279d.

What changed:

  • Added a typed WindowFrameOffset representation. DataFusion 43 exposes SQL frame interval literals such as INTERVAL '1' HOUR as normalized UTF-8 text ("1 HOUR"), so lowering now parses that text with Arrow’s interval parser and stores canonical month/day/nanosecond components. Native DataFusion interval scalar variants are normalized into the same representation.
  • Changed SQLWindowFunc.frame to Option<WindowFrame> with #[serde(default)]. Newly lowered SQL always writes Some(frame); legacy serialized IR without the field now reads as None, preserving the fact that the old planner did not retain enough context to reconstruct its implicit frame safely.
  • Added regression tests for an offset RANGE BETWEEN INTERVAL '1' HOUR PRECEDING ... frame and for deserializing legacy frame-less window JSON.

Validation: cargo test -p asap-frontend-sql -p asap-types passes (69 SQL lowering tests and 99 asap-types tests, plus the corpus/unit suites).

@zzylol
zzylol merged commit c7ba69a into main Aug 25, 2026
3 checks passed
@zzylol
zzylol deleted the issue-268-window-frame branch September 21, 2026 18:44
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.

SQL: SQLWindowFunc doesn't model window frames (ROWS/RANGE ... PRECEDING/FOLLOWING)

2 participants