Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Window function #884

Merged
merged 10 commits into from
Nov 22, 2022
Merged

Window function #884

merged 10 commits into from
Nov 22, 2022

Conversation

yliuuuu
Copy link
Contributor

@yliuuuu yliuuuu commented Nov 15, 2022

Relevant Issues

Description

Key points:

  • Ability to parsing window function
  • AST, Logical, Logically resolved, Physical representation of window function.
  • Sort based framework.
  • Lag and Lead implementation and test cases.
  • Optimization pass to merge window functions that operate on the same window partition to a single window operator (at the level of physical pass).
  • Experimental Tag.

Points to address:

Other Information

  • Updated Unreleased Section in CHANGELOG: [YES/NO]
    • Yes.
  • Any backward-incompatible changes? [YES/NO]
    • No
  • Any new external dependencies? [YES/NO]
  • No

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
Copy link

codecov-commenter commented Nov 15, 2022

Codecov Report

Base: 80.45% // Head: 80.64% // Increases project coverage by +0.19% 🎉

Coverage data is based on head (2e74645) compared to base (664b025).
Patch coverage: 90.97% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main     #884      +/-   ##
============================================
+ Coverage     80.45%   80.64%   +0.19%     
- Complexity     2124     2161      +37     
============================================
  Files           252      259       +7     
  Lines         16213    16493     +280     
  Branches       2893     2922      +29     
============================================
+ Hits          13044    13301     +257     
- Misses         2307     2318      +11     
- Partials        862      874      +12     
Flag Coverage Δ
CLI 22.41% <ø> (ø)
EXAMPLES 78.63% <ø> (ø)
EXTENSIONS 69.23% <ø> (ø)
LANG 83.06% <90.97%> (+0.16%) ⬆️
PARTIQL_PTS ∅ <ø> (∅)
PARTIQL_TEST_SCRIPT 80.90% <ø> (ø)

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

Impacted Files Coverage Δ
...iql/lang/eval/physical/PhysicalPlanCompilerImpl.kt 83.35% <ø> (ø)
...l/physical/operators/SortOperatorFactoryDefault.kt 83.33% <ø> (ø)
...c/org/partiql/lang/prettyprint/ASTPrettyPrinter.kt 80.62% <0.00%> (-0.15%) ⬇️
...lang/eval/physical/window/BuiltInWindowFunction.kt 60.00% <60.00%> (ø)
...g/src/org/partiql/lang/eval/physical/window/Lag.kt 70.00% <70.00%> (ø)
.../src/org/partiql/lang/eval/physical/window/Lead.kt 75.00% <75.00%> (ø)
lang/src/org/partiql/lang/errors/ErrorCode.kt 87.16% <80.00%> (-0.05%) ⬇️
...ang/eval/physical/PhysicalBexprToThunkConverter.kt 86.82% <80.00%> (-0.90%) ⬇️
...sforms/LogicalToLogicalResolvedVisitorTransform.kt 93.61% <85.71%> (-0.51%) ⬇️
...planner/transforms/AstToLogicalVisitorTransform.kt 86.30% <91.80%> (+2.04%) ⬆️
... and 13 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@yliuuuu yliuuuu marked this pull request as ready for review November 15, 2022 19:34
@yliuuuu yliuuuu requested a review from am357 November 15, 2022 21:01
am357
am357 previously approved these changes Nov 18, 2022
Copy link
Contributor

@am357 am357 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the great effort in putting together this experimental feature; I know the bits and pieces of this have already been reviewed by others in other PRs. Considering that I had a cursory review and left some comments related to style, duplicate code reduction, etc. Overall LGTM.

Copy link
Member

@johnedquinn johnedquinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks really good. Great job pulling in the changes from the last PRs. Great job.

@yliuuuu yliuuuu merged commit 734257d into main Nov 22, 2022
lziq added a commit that referenced this pull request Nov 28, 2022
commit 8da40ba
Author: R. C. Howell <[email protected]>
Date:   Mon Nov 28 10:04:52 2022 -0800

    Places sources in the conventional Gradle sourceset layout (#901)

    * Places sources in the conventional Gradle sourceset layout
    * Uncomment test that randomly failed

commit 6d58eca
Author: yliuuuu <[email protected]>
Date:   Fri Nov 25 11:16:27 2022 -0800

    add window function doc (#900)

    * add window function doc

commit 666ba52
Author: lziq <[email protected]>
Date:   Tue Nov 22 12:28:33 2022 -0800

    Updated `Abstract Syntax Tree.md` (#897)

commit 734257d
Author: yliuuuu <[email protected]>
Date:   Mon Nov 21 18:04:53 2022 -0800

    Window function (#884)

    Experimental implementation `LAG` `LEAD`

commit dbc089d
Author: yliuuuu <[email protected]>
Date:   Fri Nov 18 15:24:49 2022 -0800

    Fix Distinct(#887)

    * fix distinct issue

commit 4071231
Author: lziq <[email protected]>
Date:   Thu Nov 17 16:43:35 2022 -0800

    Changed default `TypedOpBehvaior` to `HONOR_PARAMETERS` for `PartiQLParser` (#888)

commit 2facc92
Author: John Ed Quinn <[email protected]>
Date:   Thu Nov 17 15:12:10 2022 -0800

    Removes use of passed context to improve latency of parse (#890)

commit b409242
Author: John Ed Quinn <[email protected]>
Date:   Thu Nov 17 12:11:24 2022 -0800

    Adds SLL prediction mode to parser to reduce latency (#886)

commit 328caf8
Author: yliuuuu <[email protected]>
Date:   Tue Nov 15 15:02:51 2022 -0800

    Rename to randomized (#885)

commit 664b025
Author: yliuuuu <[email protected]>
Date:   Mon Nov 14 18:14:38 2022 -0800

    Separate randomized tests  (#882)
lziq added a commit that referenced this pull request Nov 28, 2022
commit 8da40ba
Author: R. C. Howell <[email protected]>
Date:   Mon Nov 28 10:04:52 2022 -0800

    Places sources in the conventional Gradle sourceset layout (#901)

    * Places sources in the conventional Gradle sourceset layout
    * Uncomment test that randomly failed

commit 6d58eca
Author: yliuuuu <[email protected]>
Date:   Fri Nov 25 11:16:27 2022 -0800

    add window function doc (#900)

    * add window function doc

commit 666ba52
Author: lziq <[email protected]>
Date:   Tue Nov 22 12:28:33 2022 -0800

    Updated `Abstract Syntax Tree.md` (#897)

commit 734257d
Author: yliuuuu <[email protected]>
Date:   Mon Nov 21 18:04:53 2022 -0800

    Window function (#884)

    Experimental implementation `LAG` `LEAD`

commit dbc089d
Author: yliuuuu <[email protected]>
Date:   Fri Nov 18 15:24:49 2022 -0800

    Fix Distinct(#887)

    * fix distinct issue

commit 4071231
Author: lziq <[email protected]>
Date:   Thu Nov 17 16:43:35 2022 -0800

    Changed default `TypedOpBehvaior` to `HONOR_PARAMETERS` for `PartiQLParser` (#888)

commit 2facc92
Author: John Ed Quinn <[email protected]>
Date:   Thu Nov 17 15:12:10 2022 -0800

    Removes use of passed context to improve latency of parse (#890)

commit b409242
Author: John Ed Quinn <[email protected]>
Date:   Thu Nov 17 12:11:24 2022 -0800

    Adds SLL prediction mode to parser to reduce latency (#886)
@yliuuuu yliuuuu mentioned this pull request Jan 30, 2023
3 tasks
@RCHowell RCHowell deleted the window-function branch December 20, 2023 19:36
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.

4 participants