8000 feat(generated_columns): support select generated columns from source by yuhao-su · Pull Request #8841 · risingwavelabs/risingwave · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat(generated_columns): support select generated columns from source #8841

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 14 commits into from
Mar 31, 2023
Merged

feat(generated_columns): support select generated columns from source #8841

merged 14 commits into from
Mar 31, 2023

Conversation

yuhao-su
Copy link
Contributor
@yuhao-su yuhao-su commented Mar 28, 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?

  • support select generated columns from source

Checklist For Contributors

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • 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

Click here for Documentation

Types of user-facing changes

Please keep the types that apply to your changes, and remove the others.

  • SQL commands, functions, and operators

Release note

Allow select from a source created with generated columns.

@github-actions github-actions bot added the type/feature Type: New feature. label Mar 28, 2023
@yuhao-su yuhao-su mentioned this pull request Mar 28, 2023
Open
8 tasks
@yuhao-su yuhao-su marked this pull request as ready for review March 29, 2023 06:10
xx01cyx
xx01cyx reviewed Mar 29, 2023
View reviewed changes
@yuhao-su yuhao-su marked this pull request as draft March 29, 2023 07:53
@yuhao-su yuhao-su marked this pull request as ready for review March 30, 2023 05:58
@yuhao-su yuhao-su requested a review from xx01cyx March 30, 2023 07:29
xx01cyx
xx01cyx reviewed Mar 30, 2023
View reviewed changes
@yuhao-su yuhao-su requested a review from xx01cyx March 30, 2023 09:07
xx01cyx
xx01cyx approved these changes Mar 30, 2023
View reviewed changes
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

@BugenZhao
Copy link
Member

Is there any planner test for this feature?

@BugenZhao BugenZhao added the user-facing-changes Contains changes that are visible to users label Mar 30, 2023
BugenZhao
BugenZhao reviewed Mar 30, 2023
View reviewed changes
@codecov
Copy link
codecov bot commented Mar 30, 2023 •
edited
Loading

Codecov Report

Merging #8841 (924a7de) into main (e51b240) will decrease coverage by 0.01%.
The diff coverage is 80.00%.

@@            Coverage Diff             @@
##             main    #8841      +/-   ##
==========================================
- Coverage   70.80%   70.79%   -0.01%     
==========================================
  Files        1181     1181              
  Lines      195845   195915      +70     
==========================================
+ Hits       138660   138707      +47     
- Misses      57185    57208      +23
Flag Coverage Δ
rust 70.79% <80.00%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
src/frontend/src/binder/insert.rs 85.97% <ø> (ø)
...c/frontend/src/optimizer/plan_node/batch_source.rs 63.33% <0.00%> (-2.19%) ⬇️
src/frontend/src/optimizer/plan_node/stream.rs 14.54% <ø> (+0.03%) ⬆️
src/stream/src/from_proto/source.rs 0.00% <ø> (ø)
src/frontend/src/handler/create_source.rs 49.49% <50.00%> (ø)
src/frontend/src/handler/create_table.rs 90.20% <60.00%> (-0.59%) ⬇️
...frontend/src/optimizer/plan_node/logical_source.rs 71.73% <74.35%> (-1.38%) ⬇️
src/source/src/source_desc.rs 55.55% <92.30%> (+5.20%) ⬆️
src/common/src/catalog/column.rs 92.46% <100.00%> (ø)
src/frontend/src/optimizer/mod.rs 92.48% <100.00%> (+0.33%) ⬆️
... and 6 more

... and 7 files with indirect coverage changes

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

st1page
st1page suggested changes Mar 31, 2023
View reviewed changes
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.

offline discussion with @yuhao-su and we all agree that the source should not care about anything with "pk" and the plan has no pk constraint before the MaterializeExec with "handle conflict". We need to remove the pk in source first.