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

refactor(frontend): create streaming job handling #7101

Merged
merged 16 commits into from
Dec 29, 2022

Conversation

BugenZhao
Copy link
Member

@BugenZhao BugenZhao commented Dec 28, 2022

I hereby agree to the terms of the Singularity Data, Inc. Contributor License Agreement.

What's changed and what's your intention?

Refactor the path of handling creating streaming jobs.

  • Split gen_materialize_plan into another gen_table_plan. Those table-specific arguments are not necessary for gen_materialize_plan.
  • Handle user columns like mv_or_sink(v1, v2) in the PlanRoot.
  • Split StreamMaterialize::create into multiple methods.
    • create: for mview/sink/index, no table-specific arguments, columns are derived from the input.
    • create_for_table: for table, columns are passed-in and consistent with the bind_sql_columns in the first step of the handler. This is a must-have for schema change that do not let the StreamMaterialize::create handle the column ID generation.
  • No need to create 2 materialize node when create table.
  • Store the CREATE TABLE SQL for future schema change usages.

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • All checks passed in ./risedev check (or alias, ./risedev c)

Refer to a related PR or issue link (optional)

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@codecov
Copy link

codecov bot commented Dec 28, 2022

Codecov Report

Merging #7101 (5376d94) into main (fe8ff49) will decrease coverage by 0.00%.
The diff coverage is 90.45%.

@@            Coverage Diff             @@
##             main    #7101      +/-   ##
==========================================
- Coverage   73.19%   73.19%   -0.01%     
==========================================
  Files        1051     1051              
  Lines      167386   167423      +37     
==========================================
+ Hits       122515   122539      +24     
- Misses      44871    44884      +13     
Flag Coverage Δ
rust 73.19% <90.45%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
src/frontend/src/handler/create_table_as.rs 0.00% <0.00%> (ø)
src/frontend/src/handler/create_source.rs 60.56% <62.50%> (+0.32%) ⬆️
src/frontend/src/handler/create_sink.rs 97.10% <66.66%> (+0.52%) ⬆️
src/frontend/src/handler/create_table.rs 89.43% <81.81%> (-0.47%) ⬇️
...tend/src/optimizer/plan_node/stream_materialize.rs 93.00% <92.47%> (+3.00%) ⬆️
src/frontend/src/optimizer/mod.rs 95.77% <95.08%> (-1.27%) ⬇️
src/frontend/src/catalog/table_catalog.rs 97.22% <100.00%> (+<0.01%) ⬆️
src/frontend/src/handler/create_index.rs 88.52% <100.00%> (ø)
src/frontend/src/handler/create_mv.rs 95.83% <100.00%> (+1.36%) ⬆️
src/frontend/src/handler/mod.rs 61.90% <100.00%> (ø)
... and 14 more

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

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
…eate-table

Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
@BugenZhao BugenZhao marked this pull request as ready for review December 28, 2022 14:42
Signed-off-by: Bugen Zhao <i@bugenzhao.com>
Copy link
Contributor

@st1page st1page left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@xx01cyx xx01cyx left a comment

Choose a reason for hiding this comment

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

LGTM

@mergify
Copy link
Contributor

mergify bot commented Dec 29, 2022

Hey @BugenZhao, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by clicking "Update branch" or pushing an empty commit with git commit --allow-empty -m "rerun" && git push.

@mergify mergify bot merged commit ad5b15a into main Dec 29, 2022
@mergify mergify bot deleted the bz/refactor-handle-create-table branch December 29, 2022 12:14
@BugenZhao BugenZhao mentioned this pull request Dec 30, 2022
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants