Repository navigation
SQL: ClickHouse's lagInFrame window function needs window-frame modeling #231
Description
Summary
The bgp_jan2024_workload 200-query corpus (#219) has 5 occurrences of ClickHouse's lagInFrame, which currently fails to lower — lower_window_func_kind (crates/frontend-sql/src/sql/mod.rs, ~line 1391) is a closed hand-match over WindowFunctionDefinition::WindowUDF names (row_number, rank, dense_rank, lag, lead, first_value, last_value, nth_value) that rejects anything else via LoweringError::UnsupportedFeature.
Why this isn't a simple catalog addition (unlike #230)
lagInFrame's actual semantic difference from plain LAG is that it respects the window frame bounds (returns NULL/a default when the lookback would cross the frame boundary, rather than reaching arbitrarily far back like ordinary LAG). QueryExpr::SQLWindowFunc / WindowFuncKind (crates/types/src/pre_asap/query_expr.rs) has no frame representation at all — its own doc comment says "Window frames are not modelled yet."
So this needs a design decision before it's mechanical:
- Model window frames properly first, then give
lagInFrameits own correct semantics (bigger: touchesSQLWindowFunc's shape,output_schema, and downstream binding). This is a prerequisite for correctly supporting ClickHouse's whole*InFramefunction family, not just this one name, and possibly needed for SQL's ownOVER (... ROWS BETWEEN ...)frame clauses generally, which are presumably in the same boat. - Map
lagInFrame→WindowFuncKind::Lagas an approximation and accept the semantic gap (silently wrong at the frame boundary) until (1) happens. Cheap, but a correctness compromise that should be a deliberate, documented decision, not a quiet shortcut.
Scope of this issue
Settle which of the two directions above (or another) to take, and size the "model window frames properly" work if that's the chosen direction. Not asking for an implementation yet.
Related
- feat: add bgp_jan2024_workload SQL corpus test (200 queries, ASAPQuery PR #561) #219 —
bgp_jan2024_workloadcorpus test, where this gap was found - SQL: generalize the ClickHouse-builtin stub-registration mechanism from AggregateUDF to ScalarUDF #230 — the sibling scalar-function catalog issue this is not the same shape as
crates/types/src/pre_asap/query_expr.rs—QueryExpr::SQLWindowFunc,WindowFuncKindcrates/frontend-sql/src/sql/mod.rs—lower_window_func_kind
Activity
milindsrivastava1997 commentedon Aug 23, 2026
Split into two independent issues:
Neither depends on the other. #267 alone unblocks the 5 `lagInFrame` corpus failures (all default-frame usages, so it's exact for the corpus as-is); #268 is the general fix for the wider silent gap affecting every frame-sensitive `WindowFuncKind` (`Sum`/`Avg`/`Count`/`Min`/`Max`/`FirstValue`/`LastValue`/`NthValue`), not just `lagInFrame`.
feat(sql): give lagInFrame/leadInFrame their own WindowFuncKind varia…
zzylol commentedon Aug 26, 2026
Related open work
GROUPSframe representation.lagInFrame/leadInFrame-specific requirement.zzylol commentedon Aug 26, 2026
Closing because this issue's stated scope was to choose a direction and size/split the window-frame work, not to implement an execution engine. That work is complete:
lagInFrameandleadInFramevariants instead of silently conflating them with ANSIlag/lead.ROWSandRANGEframe data to canonical pre-ASAP IR and SQL lowering.GROUPSrepresentation is explicitly tracked by SQL: model GROUPS window frames #280.Keeping this design/splitting issue open would duplicate those focused follow-ups.