Adds RANK, DENSE_RANK, LAG, LEAD, & ROW_NUMBER Window Functions & Operator #1746

johnedquinn · 2025-03-10T18:51:51Z

Description

Adds parsing, planning, and evaluation support for windows and window functions
- Parsing support for the window clause itself (and window references) has not been implemented, however, their modeling has been carved out for future support.
Adds end-to-end support for the window clause of a SFW query. Window functions can reference the user-defined window -- allowing them to share the computation of the windows across multiple window functions (bringing down the computational cost several-fold).
Adds RANK, DENSE_RANK, LAG, LEAD, & ROW_NUMBER window functions

Deprecated

Deprecated the previous AST modeling of window functions. We weren't using it, so it slipped by the 1.0 package audit. It does not follow the SQL-spec and is not able to be updated to support the SQL spec.

Window Design

Researched implementations previously implemented in PLK (WindowFunction, Window Plan Node), Trino (Plan WindowNode, Compiled WindowOperator, Compiled WindowFunction), and PostgreSQL (Compiled window functions, Compiled Lead/Lag func).
This PR, given optional ORDER BY and PARTITION BY clauses on a window specification, will inject a RelOpSort as input to the compiled RelOpWindow. The RelOpSort will sort by both the partition keys and sort keys -- which allows our implementation of the RelOpWindow to grab contiguous rows of a particular partition and peer group (sort group). This makes creation of a WindowPartition much easier (and fairly performant).
- With the modeling exposed, we can always provide additional metadata to indicate whether some columns are pre-sorted. This may be added in the future for further optimization.
In the future, we may add a new WindowFunction API that takes in frame information as well.
This PR does NOT create a mechanism for loading SPI implementations of a window function. This PR intentionally creates public interfaces in the plan and evaluator. We may, in the future, add the ability to load SPI-provided window functions and perform a conversion between SPI's definition of a WindowFunction and the plan and evaluator's definition of a WindowFunctionNode and WindowFunction. This PR acts this way to maintain a separation of concerns. For now, we are only modeling the built-ins as provided by the database system, however, the APIs can be extended in the future to accept and converts SPI-provided window functions.
- This was and still is my personal opinion on tables, scalar functions, and aggregate functions. It allows for a tiny SPI package in the future if we'd like.

Future Changes

I anticipate that the definition of a RelWindow, WindowFunctionNode, and WindowFunction will be need to be updated to support frames, which is totally fine. For WindowFunction, we'll likely need to add a new method that takes as input some frame information, which by default should call the existing (probably deprecated in the future) eval method. This will not break the public API, nor would it break the existing customer flow.

Other Information

Updated Unreleased Section in CHANGELOG: NO: Not yet
Any backward-incompatible changes? NO: Not technically.
Any new external dependencies? NO
Do your changes comply with the Contributing Guidelines and Code Style Guidelines? YES

License Information

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

codecov-commenter · 2025-03-11T21:20:58Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 31.84%. Comparing base (2c17795) to head (bf7b73c).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1746      +/-   ##
============================================
+ Coverage     31.10%   31.84%   +0.74%     
- Complexity       99      102       +3     
============================================
  Files            31       31              
  Lines          2016     2013       -3     
  Branches        152      151       -1     
============================================
+ Hits            627      641      +14     
+ Misses         1366     1350      -16     
+ Partials         23       22       -1

Flag	Coverage Δ
CLI	`15.66% <100.00%> (+0.95%)`	⬆️
EXAMPLES	`80.03% <ø> (ø)`

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

☔ View full report in Codecov by Sentry.
📢 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.

yliuuuu

Finished Parser and Ast Package. As always, very delightful code : ).

One preliminary comment:

Perhaps it is worth to track the feature gap between components if anything exists.

i.e., if something is supported in parsing but not supported in planning, etc.

partiql-parser/src/main/antlr/PartiQLParser.g4

partiql-ast/src/main/java/org/partiql/ast/QueryBody.java

yliuuuu · 2025-03-15T02:46:30Z

partiql-ast/src/main/java/org/partiql/ast/WindowFunctionType.java

+     */
+    @Builder(builderClassName = "Builder")
+    @EqualsAndHashCode(callSuper = false)
+    public static final class NoArg extends WindowFunctionType {


Hmmm, mind evaluating why the choice of NoArg vs say Ranking and RowNumber.

So seemingly for one we are modeling using number of args, for the other we are modeling using "function characteristic" for lack of better words.

I understand that both Ranking and RowNumber must not contains argument... And no arg is, well, no arg... and you don't have to worry about argument type....

But perhaps using NoArg is bit over-smart, in that it might cause difficulty when (especially a user is) constructing the ast.

Yeah, well, a user can just use a factory method that we provide. We could do what you're proposing, but also the enum is also a small footprint. I'm not sold in either direction. If you feel strongly, I don't mind changing it to be that way.

Decided to go ahead with your observation. Trying to thread the needle between your comments and @alancai98's.

partiql-ast/src/main/java/org/partiql/ast/WindowReference.java

partiql-parser/src/main/kotlin/org/partiql/parser/internal/PartiQLParserDefault.kt

johnedquinn · 2025-03-19T15:31:02Z

@yliuuuu added a commit to fix the nullable references to lag/lead arguments.

Regarding any potential feature gaps, I've cut: #1748. The only other thing not supported is the IGNORE/RESPECT NULLS for the LAG/LEAD functions. See the test in the planner -- that should be sufficient for tracking.

CHANGELOG.md

alancai98

Leaving some initial comments for the parsing + AST modeling. I'll need some more time to read through the rest of the PR

CHANGELOG.md

partiql-eval/src/test/kotlin/org/partiql/eval/internal/WindowTests.kt

partiql-parser/src/main/antlr/PartiQLParser.g4

alancai98 · 2025-03-19T20:26:05Z

partiql-parser/src/main/antlr/PartiQLParser.g4

+ * EBNF 2023:
+ * <window function> ::= <window function type> OVER <window name or specification>
+ */
+windowFunction: funcType=windowFunctionType OVER spec=windowNameOrSpecification;
+
+/**
+ * EBNF 2023:
+ * <window function type> ::=
+ *   <rank function type> <left paren> <right paren>
+ *   | ROW_NUMBER <left paren> <right paren>
+ *   | <lead or lag function>
+ *   | and more...
+ */
+windowFunctionType
+    : rankFunctionType PAREN_LEFT PAREN_RIGHT # WindowFunctionTypeRank
+    | ROW_NUMBER PAREN_LEFT PAREN_RIGHT       # WindowFunctionTypeRowNumber
+    | leadOrLagFunction                       # WindowFunctionTypeLeadOrLag
+    ;


Rather than have the window function type be hard-coded, could we do something similar to what we do for scalar and aggregations (i.e. the modeling of functionCall uses a qualifiedName rather than any hard-coded names)? Doing so will make it a lot easier to add new window functions and also user-defined window functions in the future.

Relevant prior PRs to get rid of hard-coded scalar and aggregations from the antlr grammar

Add to non-reserved keywords; rework functionName and symbolPrimitive parse rules #1457

Remove hard-coded aggregations from parser and ast #1464

I'd say let's do that when we get there. The EBNF does not contribute to API breaking changes. WindowFunctionType also is not a sealed interface. If we want to make it more explicit, I can make NoArg specific to RowNumber and Rank. Then, if we ever decided to allow user-defined window functions, we can do so by adding a variant specifically for this.

I'm still not fully-convinced that the modeling for the window functions needs to be hard-coded in the parser and AST. In the future if we are to support more window functions, we would need additional rules in the parser and variants within the WindowFunctionType, which may be redundant. Trino, for instance, defines all scalar, aggregate, and window functions as a FunctionCall (see their grammar).

Since we've marked all the public APIs as experimental though, I'm still fine w/ proceeding and experimenting further as we get feedback from customers.

alancai98 · 2025-03-19T20:44:46Z

partiql-ast/src/main/java/org/partiql/ast/WindowSpecification.java

+    // TODO: In the future: @Nullable private final WindowFrameClause frameClause;
+
+    @Nullable
+    private final Identifier.Simple existingName;


nit: could we just call this name? The SQL2023 spec uses <existing window name> but it just wraps a <window name>

Personally, I like the fact that it matches the EBNF and that it makes it apparent that the WindowSpecification operates against a pre-defined (aka existing) window, effectively creating a new window.

partiql-ast/src/main/java/org/partiql/ast/WindowOrderClause.java

partiql-ast/src/main/java/org/partiql/ast/WindowSpecification.java

alancai98 · 2025-03-19T21:20:58Z

partiql-ast/src/main/java/org/partiql/ast/WindowFunctionType.java

+/**
+ * Represents a window function type.
+ * @see ExprWindowFunction#getFunctionType() 
+ */
+public abstract class WindowFunctionType extends AstNode {


Similar to the parser comment, I'm wondering if we should change the window functions to not be as hard-coded within the AST modeling and follow something more like scalar and aggregate function calls. IMO this can make it a lot more straightforward to add future support for user-defined window functions.

Trino for example models window functions (along with scalars and aggregates) using FunctionsCall. For all of the function names, they just use a QualifiedName rather than any sort of hard-coded names within their AST, leaving the validation to a later stage. I'm not sure if we should lump all three of those under ExprCall but modeling window functions with just an identifier name rather than the WindowFunctionType could be a cleaner modeling as more window functions are added over time.

I'd be more keen to add a user-defined variant in the future. See the 2003 EBNF. Trino doesn't have support for over half of the specified variants of the window functions.

alancai98 · 2025-03-19T21:27:15Z

partiql-ast/src/main/java/org/partiql/ast/WindowReference.java

+     */
+    @Builder(builderClassName = "Builder")
+    @EqualsAndHashCode(callSuper = false)
+    public static final class InLineSpecification extends WindowReference {


It's not clear to me why we need a separate in-line window specification class. Perhaps a simpler modeling could follow what Trino does with their Window interface.

The Window interface is implemented by two separate classes

WindowReference and

WindowSpecification

Following a similar modeling, we could have a Window abstract class that is used by the ExprWindowFunction.

We could put WindowSpecification to be a variant of WindowReference (and make it a nested class, or leave it out separately, doesn't matter to me). Alternatively, extract a WindowReference interface. Did this locally, and while it's nice, I'm really despising how AstNode is an abstract class. Should've just created a public AstNodeBase. I'll publish a rev, and it should be apparent why it's a burden.

I'm actually going to take @yliuuuu's recommendation of removing WindowReference entirely. It removes all the shenanigans, and we get what you're describing also 😄

alancai98 · 2025-03-19T21:29:44Z

partiql-ast/src/main/java/org/partiql/ast/WindowFunctionType.java

+        private final Expr extent;
+        private final Long offset;
+        private final Expr defaultValue;


If window functions were modeled more closely to ExprCall, these could all be function arguments rather than hard-coded properties.

partiql-planner/src/main/kotlin/org/partiql/planner/internal/transforms/RelConverter.kt

...anner/src/main/kotlin/org/partiql/planner/internal/window/WindowFunctionSignatureProvider.kt

partiql-eval/src/test/kotlin/org/partiql/eval/internal/WindowTests.kt

partiql-eval/src/main/kotlin/org/partiql/eval/internal/window/NavigationFunction.kt

yliuuuu

Overall looks good to me. Just one comment on the Typing of Lag/Lead Function.

yliuuuu · 2025-07-29T18:28:19Z

partiql-ast/src/main/java/org/partiql/ast/WindowFunctionType.java

+     */
+    @Builder(builderClassName = "Builder")
+    @EqualsAndHashCode(callSuper = false)
+    public static final class LeadOrLag extends WindowFunctionType {


No objection here, but might argue that by breaking it down into two classes, the API is more consistent.

Yeah, I mean I'm fine with that too 😄 Whatever floats your boat

partiql-eval/src/main/kotlin/org/partiql/eval/internal/operator/rel/RelOpWindow.kt

yliuuuu · 2025-07-29T22:21:32Z

...anner/src/main/kotlin/org/partiql/planner/internal/window/WindowFunctionSignatureProvider.kt

+        returnTyper.accumulate(expr)
+        returnTyper.accumulate(default)


Do we want to accumulate or throw on mismatch?

I might be missing something. The lag/lead's output type will be the expr or default if the offset row doesn't exist, so they don't need to match. In the case of coercible types, maybe we coerce them to their common super type, hence the TODO. But, I don't see any problem with the output type being ANY if the types aren't coercible.

The only validation I'd say is required is that the offset by coercible to BIGINT (or for now just BIGINT).

… clause Adds AST modeling for window functions Adds plan modeling for window functions Deprecates old window function AST APIs due to incompatibility Adds RANK, DENSE_RANK window functions Adds ROW_NUMBER window function Adds LAG and LEAD window functions Adds support for referencing window clause from window functions

Adds children check Leverages different warning type

Adds deprecation notices to APIs

alancai98

Thanks for updating the PR and adding the experimental notices! Mostly left some minor comments/feedback that should be quick to address before we merge. A couple other things to highlight:

I think we should try to use a separate tracking issue for things that are left as TODOs. Otherwise, the TODOs may be hidden in the code and hard to prioritize.
The current modeling of window functions in the parser/ast may need further experimentation as we integrate w/ customers. Since everything that's public will be marked w/ experimental, I think we can still proceed with merging in its current modeling.

alancai98 · 2025-08-25T18:06:02Z

test/partiql-tests

partiql-tests submodule change should be reverted

alancai98 · 2025-08-25T18:06:33Z

CHANGELOG.md

@@ -27,6 +27,7 @@ Thank you to all who have contributed!

 ### Added
 - partiql-ast: add `With` and `WithListElement` to `AstVisitor` and `SqlDialect`
+- Adds end-to-end support for window functions including RANK, DENSE_RANK, LAG, LEAD, and ROW_NUMBER functions alongside the WINDOW clause.


We should call out that the new window clause and window function support is experimental.

alancai98 · 2025-08-29T20:45:54Z

partiql-ast/src/main/java/org/partiql/ast/WindowClause.java

+     */
+    @lombok.Builder(builderClassName = "Builder")
+    @EqualsAndHashCode(callSuper = false)
+    public static final class WindowDefinition extends AstNode {


nit: can unnest from the WindowClause or just call this Definition. Currently the type is WindowClause.WindowDefinition, which is pretty lengthy

also need a deprecated/experimental notice

alancai98 · 2025-08-29T20:57:21Z

partiql-ast/src/main/java/org/partiql/ast/WindowPartition.java

+ * @deprecated This feature is experimental and is subject to change.
+ */
+@Deprecated
+public abstract class WindowPartition extends AstNode {


Why is the WindowPartition abstract? If I'm matching this up properly to the SQL2023 spec, this lines up with <window partition column reference>, which will have a <column reference> and an optional <collate clause>. Perhaps a simpler modeling is just as a final class with a columnReference field.

alancai98 · 2025-08-29T20:58:46Z

CHANGELOG.md

@@ -27,6 +27,7 @@ Thank you to all who have contributed!

 ### Added
 - partiql-ast: add `With` and `WithListElement` to `AstVisitor` and `SqlDialect`
+- Adds end-to-end support for window functions including RANK, DENSE_RANK, LAG, LEAD, and ROW_NUMBER functions alongside the WINDOW clause.
 - **EXPERIMENTAL** partiql-plan: add representation of `RelWith` and `WithListElement` to the plan and the 
 `OperatorVisitor`
 - **EXPERIMENTAL** partiql-planner: add planner builder function to control whether `With` table references are 


Should probably also call out that the prior modeling of window is deprecated and replaced with the newer modeling.

alancai98 · 2025-08-29T21:21:05Z

partiql-spi/src/main/java/org/partiql/spi/errors/PRuntimeException.java

+        // if (t != null) {
+        //     return t;
+        // }
+        return null;


Why are we returning null for the cause here?

alancai98 · 2025-08-29T21:21:48Z

partiql-spi/src/main/java/org/partiql/spi/value/DatumString.java

@@ -35,7 +35,7 @@ public PType getType() {
    public String toString() {
        return "DatumString{" +
                "_type=" + _type +
-                ", _value=" + _value +
+                ", _value='" + _value + '\'' +


nit: for consistency and don't need the escape if we use double-quotes

Suggested change

", _value='" + _value + '\'' +

", _value='" + _value + "'" +

alancai98 · 2025-08-29T21:23:17Z

partiql-ast/src/main/java/org/partiql/ast/AstVisitor.java

@@ -324,6 +325,106 @@ public R visitExprArray(ExprArray node, C ctx) {
        return defaultVisit(node, ctx);
    }

+    /**


For these functions, probably need an experimental notice.

alancai98 · 2025-08-29T21:26:46Z

partiql-ast/src/main/java/org/partiql/ast/WindowSpecification.java

+@EqualsAndHashCode(callSuper = false)
+@Deprecated
+public final class WindowSpecification extends AstNode {
+    // TODO: In the future: @Nullable private final WindowFrameClause frameClause;


Could we create a tracking issue for the TODOs brought up in this PR? In the code and in PR description there are a lot of TODOs mentioned (e.g. window frame clause, unsupported functions in WindowBuiltins) but may be hard to discover unless we do a code search of the repo.

alancai98 · 2025-08-29T21:46:44Z

partiql-parser/src/main/antlr/PartiQLParser.g4

+ * EBNF 2023:
+ * <window function> ::= <window function type> OVER <window name or specification>
+ */
+windowFunction: funcType=windowFunctionType OVER spec=windowNameOrSpecification;
+
+/**
+ * EBNF 2023:
+ * <window function type> ::=
+ *   <rank function type> <left paren> <right paren>
+ *   | ROW_NUMBER <left paren> <right paren>
+ *   | <lead or lag function>
+ *   | and more...
+ */
+windowFunctionType
+    : rankFunctionType PAREN_LEFT PAREN_RIGHT # WindowFunctionTypeRank
+    | ROW_NUMBER PAREN_LEFT PAREN_RIGHT       # WindowFunctionTypeRowNumber
+    | leadOrLagFunction                       # WindowFunctionTypeLeadOrLag
+    ;


I'm still not fully-convinced that the modeling for the window functions needs to be hard-coded in the parser and AST. In the future if we are to support more window functions, we would need additional rules in the parser and variants within the WindowFunctionType, which may be redundant. Trino, for instance, defines all scalar, aggregate, and window functions as a FunctionCall (see their grammar).

Since we've marked all the public APIs as experimental though, I'm still fine w/ proceeding and experimenting further as we get feedback from customers.

johnedquinn force-pushed the window branch 2 times, most recently from 6111dc6 to eb013ab Compare March 11, 2025 21:46

johnedquinn changed the title ~~[DRAFT] Adds RANK, DENSE_RANK, LAG, LEAD, & ROW_NUMBER Window Functions & Operator~~ Adds RANK, DENSE_RANK, LAG, LEAD, & ROW_NUMBER Window Functions & Operator Mar 11, 2025

johnedquinn marked this pull request as ready for review March 11, 2025 22:01

johnedquinn requested review from jpschorr and yliuuuu March 11, 2025 22:02

johnedquinn mentioned this pull request Mar 11, 2025

Adds parsing support for window functions #1744

Closed

yliuuuu reviewed Mar 15, 2025

View reviewed changes

partiql-parser/src/main/kotlin/org/partiql/parser/internal/PartiQLParserDefault.kt Outdated Show resolved Hide resolved

johnedquinn mentioned this pull request Mar 19, 2025

Add support for extending a window definition #1748

Open

johnedquinn requested a review from yliuuuu March 19, 2025 15:31

alancai98 reviewed Mar 19, 2025

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

alancai98 reviewed Mar 19, 2025

View reviewed changes

yliuuuu reviewed Mar 20, 2025

View reviewed changes

johnedquinn force-pushed the window branch from 66daba2 to 94e98ca Compare July 28, 2025 19:33

johnedquinn requested review from yliuuuu and alancai98 July 28, 2025 23:54

yliuuuu reviewed Jul 29, 2025

View reviewed changes

johnedquinn added 10 commits August 18, 2025 13:27

Fixes nullable references to lag/lead arguments

26fa454

Fixes infinite printing

ac2cd05

Updates CHANGELOG

3bafa7a

Addresses minor comments

85063d4

Adds children check Leverages different warning type

Removes WindowReference in favor of WindowSpecification

c88bf6d

Adds three dedicated edge-case tests

d220c86

Removes WindowOrderClause and WindowPartitionClause

0731b02

Flattens the WindowFunctionType to include variants

70eb250

Removes unnecessary reset method

4278b99

johnedquinn added 3 commits August 18, 2025 13:27

Splits out Lag/Lead into own classes

1767018

Adds PError.EXPERIMENTAL, and adds to window parsing/planning

e7bdc78

Adds deprecation notices to APIs

Updates string snapshot tests

93f3c7d

johnedquinn force-pushed the window branch from 17a387f to 93f3c7d Compare August 18, 2025 20:36

johnedquinn requested a review from yliuuuu August 18, 2025 20:37

alancai98 reviewed Aug 29, 2025

View reviewed changes

Adds RANK, DENSE_RANK, LAG, LEAD, & ROW_NUMBER Window Functions & Operator #1746

Are you sure you want to change the base?

Adds RANK, DENSE_RANK, LAG, LEAD, & ROW_NUMBER Window Functions & Operator #1746

Uh oh!

Conversation

johnedquinn commented Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Deprecated

Window Design

Future Changes

Other Information

License Information

Uh oh!

codecov-commenter commented Mar 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yliuuuu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

johnedquinn commented Mar 19, 2025

Uh oh!

Uh oh!

alancai98 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alancai98 Aug 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yliuuuu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnedquinn commented Mar 10, 2025 •

edited

Loading

codecov-commenter commented Mar 11, 2025 •

edited

Loading

alancai98 Aug 29, 2025 •

edited

Loading

alancai98 Aug 29, 2025 •

edited

Loading