Skip to content

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

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
merged 7 commits into from
May 6, 2023
Merged

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

merged 7 commits into from
May 6, 2023

Conversation

@stdrc
Copy link
Collaborator

@stdrc stdrc commented May 4, 2023 •
edited
Loading

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 mentioned this pull request May 4, 2023
Open
38 tasks
@github-actions github-actions bot added the type/feature Type: New feature. label May 4, 2023
@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 mentioned this pull request May 6, 2023
Merged
6 tasks
@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 •
edited
Loading

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

soundOfDestiny
soundOfDestiny approved these changes May 6, 2023
View reviewed changes
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

src/stream/src/from_proto/eowc_over_window.rs
));
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

@soundOfDestiny soundOfDestiny May 6, 2023

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
Collaborator Author

stdrc commented May 6, 2023 •
edited
Loading

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.

chenzl25
chenzl25 approved these changes May 6, 2023
View reviewed changes
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
@stdrc
fix window function display
c084946 
Signed-off-by: Richard Chien <[email protected]>
@stdrc
fix LogicalOverAgg::rewrite_with_input
e2ad009 
Signed-off-by: Richard Chien <[email protected]>
@stdrc
fix state table from proto
330dcbc 
Signed-off-by: Richard Chien <[email protected]>
@stdrc
fix sort table dist key
ca1004d 
Signed-off-by: Richard Chien <[email protected]>
stdrc added 3 commits May 6, 2023 20:52
@stdrc
LogicalOverAgg::to_stream
122938b 
Signed-off-by: Richard Chien <[email protected]>
@stdrc
update planner tests
846b004 
Signed-off-by: Richard Chien <[email protected]>
@stdrc
add e2e test
29f4cad 
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
This was referenced May 6, 2023
Closed
Closed
@emile-00 emile-00 mentioned this pull request May 13, 2023
Closed
@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

Reviewers

@chenzl25 chenzl25 chenzl25 approved these changes

@TennyZhuang TennyZhuang Awaiting requested review from TennyZhuang

@Honeta Honeta Awaiting requested review from Honeta

@st1page st1page Awaiting requested review from st1page

+1 more reviewer

@soundOfDestiny soundOfDestiny soundOfDestiny approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

type/feature Type: New feature. user-facing-changes Contains changes that are visible to users 📖✓ Covered or will be covered in the user docs.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@stdrc @chenzl25 @soundOfDestiny @CharlieSYH