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

feat(over window): generate EOWC stream plan from LogicalOverAgg #9597

Merged
merged 7 commits into from
May 6, 2023

Conversation

stdrc
Copy link
Member

@stdrc stdrc commented May 4, 2023

I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.

What's changed and what's your intention?

This PR enables Over Window feature with EMIT ON WINDOW CLOSE. Tracked in #9124.

Checklist For Contributors

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • I have demonstrated that backward compatibility is not broken by breaking changes and created issues to track deprecated features to be removed in the future. (Please refer to the issue)
  • All checks passed in ./risedev check (or alias, ./risedev c)

Checklist For Reviewers

  • I have requested macro/micro-benchmarks as this PR can affect performance substantially, and the results are shown.

Documentation

Types of user-facing changes

  • SQL commands, functions, and operators

Release note

Support Over Window (a.k.a Over Aggregation or Window Function) with EMIT ON WINDOW CLOSE. Currently only support windows partitioned by at least one column and ordered by only single watermark column.

Example:

CREATE SOURCE t (a TIMESTAMP, b INT, WATERMARK FOR a AS a - INTERVAL '5 minutes') WITH ...;
CREATE MATERIALIZED VIEW mv EMIT ON WINDOW CLOSE AS
SELECT
    window_start,
    lag(b) OVER (PARTITION BY window_start ORDER BY a),
    max(b) OVER (PARTITION BY window_start ORDER BY a ROWS BETWEEN 10 PRECEDING AND CURRENT ROW)
FROM tumble(t, a, INTERVAL '1 hour');

Window functions supported by far:

  • lag and lead with constant offset
  • aggregate functions

Window frame types supported by far:

  • ROWS without exclusion clause

@stdrc stdrc force-pushed the rc/logical-over-window-to-stream branch from f2d92c4 to 5744289 Compare May 5, 2023 13:29
@stdrc stdrc changed the base branch from main to rc/eowc-keyword May 5, 2023 13:29
@stdrc stdrc changed the title feat(over window): generate stream plan from LogicalOverAgg feat(over window): generate EOWC stream plan from LogicalOverAgg May 6, 2023
@stdrc stdrc force-pushed the rc/eowc-keyword branch 2 times, most recently from c1b8d83 to df3448c Compare May 6, 2023 07:19
@stdrc stdrc force-pushed the rc/eowc-keyword branch from df3448c to a69c1a3 Compare May 6, 2023 07:46
@stdrc stdrc force-pushed the rc/logical-over-window-to-stream branch from 5744289 to 11b4f17 Compare May 6, 2023 07:59
@github-actions github-actions bot added the user-facing-changes Contains changes that are visible to users label May 6, 2023
@stdrc stdrc marked this pull request as ready for review May 6, 2023 08:17
@codecov
Copy link

codecov bot commented May 6, 2023

Codecov Report

Merging #9597 (29f4cad) into main (2ff33a7) will increase coverage by 0.07%.
The diff coverage is 81.91%.

@@            Coverage Diff             @@
##             main    #9597      +/-   ##
==========================================
+ Coverage   70.83%   70.90%   +0.07%     
==========================================
  Files        1238     1238              
  Lines      207367   207449      +82     
==========================================
+ Hits       146879   147090     +211     
+ Misses      60488    60359     -129     
Flag Coverage Δ
rust 70.90% <81.91%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
src/stream/src/from_proto/eowc_over_window.rs 0.00% <0.00%> (ø)
...ontend/src/optimizer/plan_node/logical_over_agg.rs 88.25% <78.78%> (+2.65%) ⬆️
...end/src/optimizer/plan_node/generic/over_window.rs 89.84% <94.44%> (+15.06%) ⬆️
...rc/frontend/src/optimizer/plan_node/stream_sort.rs 98.64% <100.00%> (+3.06%) ⬆️

... and 13 files with indirect coverage changes

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

Copy link
Contributor

@soundOfDestiny soundOfDestiny left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -56,7 +56,8 @@ impl ExecutorBuilder for EowcOverWindowExecutorBuilder {
.expect("vnodes not set for EOWC over window"),
));
let state_table =
StateTable::from_table_catalog(node.get_state_table()?, store, vnodes).await;
StateTable::from_table_catalog_inconsistent_op(node.get_state_table()?, store, vnodes)
Copy link
Contributor

Choose a reason for hiding this comment

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

yet another inconsistent op

@chenzl25
Copy link
Contributor

chenzl25 commented May 6, 2023

richardchien wants to merge 6 commits into rc/eowc-keyword from rc/logical-over-window-to-stream

Why not merge into the main branch?

@stdrc
Copy link
Member Author

stdrc commented May 6, 2023

Why not merge into the main branch?

Because this PR depends on #9622. Will merge that first, then this PR will automatically based on main.

Copy link
Contributor

@chenzl25 chenzl25 left a comment

Choose a reason for hiding this comment

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

LGTM

@stdrc stdrc force-pushed the rc/logical-over-window-to-stream branch 3 times, most recently from ede66b0 to 5147f02 Compare May 6, 2023 11:16
Base automatically changed from rc/eowc-keyword to main May 6, 2023 11:58
stdrc added 4 commits May 6, 2023 20:52
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
stdrc added 3 commits May 6, 2023 20:52
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
@stdrc stdrc force-pushed the rc/logical-over-window-to-stream branch from 5147f02 to 29f4cad Compare May 6, 2023 12:52
@stdrc stdrc enabled auto-merge May 6, 2023 12:54
@stdrc stdrc added this pull request to the merge queue May 6, 2023
Merged via the queue into main with commit 83eebed May 6, 2023
@stdrc stdrc deleted the rc/logical-over-window-to-stream branch May 6, 2023 13:16
@CharlieSYH CharlieSYH added the 📖✓ Covered or will be covered in the user docs. label May 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/feature user-facing-changes Contains changes that are visible to users 📖✓ Covered or will be covered in the user docs.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants