Skip to content

[multistage] Initial (phase 1) query planner support for window functions #10228

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.

Sign up for GitHub

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

Jump to bottom
Merged
siddharthteotia merged 4 commits into from
Feb 8, 2023
Merged

[multistage] Initial (phase 1) query planner support for window functions #10228

siddharthteotia merged 4 commits into from
Feb 8, 2023

Conversation

@somandal
Copy link
Contributor

@somandal somandal commented Feb 4, 2023 •
edited
Loading

This PR introduces the multi-stage planner changes to support Phase 1 of Window Functions. Planning support has been added for the following classes of window queries:

  1. Empty OVER()
  2. OVER(ORDER BY)
  3. OVER(PARTITION BY)
  4. OVER(PARTITION BY ORDER BY)

The window functions supported as part of Phase 1 are: SUM, AVG, MIN, MAX, and COUNT

This PR also adds JSON files for planner test cases which can be extended for an exhaustive set of query types.

This PR does not include support for:

  • Execution engine changes for Phase 1 (will be the next PR for this feature)
  • Custom frames
  • Other window functions related to rank and values
  • Multiple window groups (basically multiple OVER clauses with different PARTITION BY, ORDER BY and/or FRAME specifications, if these specifications are the same they get grouped into a single window group)

The above will be part of future changes for window function support

cc @siddharthteotia @walterddr @Jackie-Jiang

@siddharthteotia siddharthteotia changed the title [multi stage] Initial (phase 1) planner changes for window functions in multi-stage engine [multistage] Initial (phase 1) query planner support for window functions Feb 4, 2023
@siddharthteotia siddharthteotia added the multi-stage Related to the multi-stage query engine label Feb 4, 2023
@codecov-commenter
Copy link

codecov-commenter commented Feb 4, 2023 •
edited
Loading

Codecov Report

Merging #10228 (6564aaa) into master (0bbb945) will decrease coverage by 0.01%.
The diff coverage is 56.33%.

@@             Coverage Diff              @@
##             master   #10228      +/-   ##
============================================
- Coverage     70.43%   70.42%   -0.01%     
+ Complexity     5759     5100     -659     
============================================
  Files          2016     2017       +1     
  Lines        109154   109153       -1     
  Branches      16583    16594      +11     
============================================
- Hits          76881    76874       -7     
+ Misses        26894    26892       -2     
- Partials       5379     5387       +8
Flag Coverage Δ
integration1 24.73% <0.00%> (+0.13%) ⬆️
integration2 24.36% <0.00%> (-0.18%) ⬇️
unittests1 67.66% <56.33%> (-0.07%) ⬇️
unittests2 13.71% <0.00%> (+0.01%) ⬆️

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

Impacted Files Coverage Δ
...e/pinot/query/planner/ExplainPlanStageVisitor.java 0.00% <0.00%> (ø)
...t/query/planner/logical/ShuffleRewriteVisitor.java 0.00% <0.00%> (ø)
...hysical/colocated/GreedyShuffleRewriteVisitor.java 0.00% <0.00%> (ø)
...lanner/stage/DefaultPostOrderTraversalVisitor.java 0.00% <0.00%> (ø)
.../pinot/query/runtime/plan/PhysicalPlanVisitor.java 92.30% <0.00%> (-4.99%) ⬇️
...t/query/runtime/plan/ServerRequestPlanVisitor.java 84.25% <0.00%> (-0.79%) ⬇️
...g/apache/pinot/query/planner/stage/WindowNode.java 55.10% <55.10%> (ø)
...not/query/planner/logical/RelToStageConverter.java 85.33% <75.00%> (+0.61%) ⬆️
...che/pinot/query/planner/logical/RexExpression.java 90.76% <100.00%> (+0.76%) ⬆️
...ot/query/planner/logical/StageMetadataVisitor.java 100.00% <100.00%> (ø)
... and 50 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

walterddr
walterddr approved these changes Feb 7, 2023
View reviewed changes
Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

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

lgtm overall. I am sure the runtime PR will require some changes on this so let's follow up on the runtime PR once finishes.

@somandal somandal requested a review from walterddr February 7, 2023 23:25
@somandal somandal requested review from siddharthteotia and removed request for walterddr February 8, 2023 04:59
@siddharthteotia siddharthteotia merged commit 3c9a9df into apache:master Feb 8, 2023
@somandal somandal deleted the window-functions-planner-phase1 branch February 8, 2023 18:30
@somandal somandal mentioned this pull request Feb 15, 2023
Merged
@somandal somandal mentioned this pull request Mar 10, 2023
Merged
@somandal somandal mentioned this pull request Mar 21, 2023
Merged
This was referenced Apr 4, 2023
Merged
Merged
@somandal somandal mentioned this pull request Apr 28, 2023
Merged
@yashmayya yashmayya added feature window-functions Related to SQL window functions on the multi-stage query engine labels Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@siddharthteotia siddharthteotia siddharthteotia approved these changes

@walterddr walterddr walterddr approved these changes

Assignees

No one assigned

Labels

feature multi-stage Related to the multi-stage query engine window-functions Related to SQL window functions on the multi-stage query engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@somandal @codecov-commenter @siddharthteotia @walterddr @yashmayya