Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Support aggregate window functions #946

Conversation

dai-chen
Copy link
Member

@dai-chen dai-chen commented Dec 18, 2020

Issue #, if available: #770

Description of changes:

To reviewers, most file changes are due to 1 and 2a renaming. You can focus on core logic in PeerRowsWindowFrame and AggregateWindowFunction.

  1. Use NamedExpression in logical and physical window operator: this is required because after expression analysis window function lost info in OVER clause, ex. RANK() OVER(expr1), RANK(expr2)
  2. Refactor window frame
    a. Rename existing frame to CurrentRowWindowFrame which only maintains current row (and previous) for ranking window functions.
    b. Add new PeerRowsWindowFrame which preloads all peer rows for aggregate window function (Please check the design doc for more details)
  3. Add AggregateWindowFunction adapter class to reuse existing aggregator

Documentation:

  1. Design doc: https://github.com/dai-chen/sql/blob/support-aggregate-window-functions-2/docs/dev/AggregateWindowFunction.md
  2. User manual: https://github.com/dai-chen/sql/blob/support-aggregate-window-functions-2/docs/user/dql/window.rst#aggregate-functions
  3. Limitations: https://github.com/dai-chen/sql/blob/support-aggregate-window-functions-2/docs/user/limitations/limitations.rst#limitations-on-window-functions

TODOs: (covered in limitation doc)

  1. Optimize reference to other expression in SELECT clause, ex. SELECT SUM(a), SUM(SUM(a)) OVER(...) ...
  2. [Low] window function nested in other expression, ex. CASE WHEN SUM(a) OVER(...) ...
  3. Enable circuit break for window operator in case of worst case happen (elaborated in design doc above).

Testing: Added UT and comparison test.

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

@dai-chen dai-chen added enhancement New feature or request SQL labels Dec 18, 2020
@dai-chen dai-chen self-assigned this Dec 18, 2020
@codecov
Copy link

codecov bot commented Dec 18, 2020

Codecov Report

Merging #946 (59c7582) into develop (280482c) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             develop     #946   +/-   ##
==========================================
  Coverage      99.87%   99.87%           
- Complexity      2350     2381   +31     
==========================================
  Files            232      234    +2     
  Lines           5413     5477   +64     
  Branches         349      357    +8     
==========================================
+ Hits            5406     5470   +64     
  Misses             5        5           
  Partials           2        2           
Impacted Files Coverage Δ Complexity Δ
...l/expression/window/ranking/DenseRankFunction.java 100.00% <ø> (ø) 4.00 <0.00> (ø)
...ch/sql/expression/window/ranking/RankFunction.java 100.00% <ø> (ø) 4.00 <0.00> (ø)
...l/expression/window/ranking/RowNumberFunction.java 100.00% <ø> (ø) 3.00 <0.00> (ø)
...sticsearch/sql/planner/logical/LogicalPlanDSL.java 100.00% <ø> (ø) 16.00 <0.00> (ø)
...asticsearch/sql/planner/logical/LogicalWindow.java 100.00% <ø> (ø) 2.00 <0.00> (ø)
...icsearch/sql/planner/physical/PhysicalPlanDSL.java 100.00% <ø> (ø) 15.00 <0.00> (ø)
...rch/sql/sql/parser/context/QuerySpecification.java 100.00% <ø> (ø) 15.00 <0.00> (ø)
...elasticsearch/sql/analysis/ExpressionAnalyzer.java 100.00% <100.00%> (ø) 33.00 <2.00> (+1.00)
...rch/sql/analysis/ExpressionReferenceOptimizer.java 100.00% <100.00%> (ø) 14.00 <2.00> (+2.00)
...csearch/sql/analysis/SelectExpressionAnalyzer.java 100.00% <100.00%> (ø) 9.00 <0.00> (ø)
... and 10 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 280482c...59c7582. Read the comment docs.

@dai-chen dai-chen marked this pull request as ready for review December 18, 2020 21:16
@dai-chen dai-chen merged commit 9a08770 into opendistro-for-elasticsearch:develop Jan 7, 2021
@dai-chen dai-chen deleted the support-aggregate-window-functions-2 branch January 7, 2021 19:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request SQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants