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.

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

feat(frontend): support alter add column for shared source #19649

Merged
merged 2 commits into from
Dec 11, 2024
Merged

feat(frontend): support alter add column for shared source #19649

merged 2 commits into from
Dec 11, 2024
+371 −49

Conversation

xxchan
Copy link
Member

@xxchan xxchan commented Dec 3, 2024 •
edited
Loading

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

What's changed and what's your intention?

fe part of #19063

Checklist

  • I have written necessary rustdoc comments
  • I have added necessary unit tests and integration tests
  • I have added test labels as necessary. See details.
  • I have added fuzzing tests or opened an issue to track them. (Optional, recommended for new SQL features Sqlsmith: Sql feature generation #7934).
  • My PR contains breaking changes. (If it deprecates some features, please create a tracking issue to remove them in the future).
  • All checks passed in ./risedev check (or alias, ./risedev c)
  • My PR changes performance-critical code. (Please run macro/micro-benchmarks and show the results.)
  • My PR contains critical fixes that are necessary to be merged into the latest release. (Please check out the details)

Documentation

  • My PR needs documentation updates. (Please use the Release note section below to summarize the impact on users)

Release note

If this PR includes changes that directly affect users or other significant modifications relevant to the community, kindly draft a release note to provide a concise summary of these changes. Please prioritize highlighting the impact these changes will have on users.

This was referenced Dec 3, 2024
Merged
Merged
@xxchan Graphite App
Copy link
Member Author

xxchan commented Dec 3, 2024 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@xxchan xxchan force-pushed the 09-17-feat_support_alter_shared_source branch from 3698c6d to dcf9e2b Compare December 3, 2024 04:44
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from 2830087 to e8f3f89 Compare December 3, 2024 04:44
@xxchan xxchan force-pushed the 09-17-feat_support_alter_shared_source branch from dcf9e2b to 5b06546 Compare December 3, 2024 05:16
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from e8f3f89 to e68963c Compare December 3, 2024 05:16
@xxchan xxchan force-pushed the 09-17-feat_support_alter_shared_source branch 4 times, most recently from 1d9faeb to 2a4081e Compare December 3, 2024 05:45
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from e68963c to ae050b3 Compare December 3, 2024 05:45
@xxchan xxchan changed the base branch from 09-17-feat_support_alter_shared_source to 12-03-feat_meta_support_replace_source December 3, 2024 05:45
@xxchan xxchan mentioned this pull request Dec 3, 2024
Merged
9 tasks
@xxchan xxchan force-pushed the 12-03-feat_meta_support_replace_source branch from e7756d6 to c3219ea Compare December 3, 2024 08:29
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from ae050b3 to f32b9a5 Compare December 3, 2024 08:29
@xxchan xxchan force-pushed the 12-03-feat_meta_support_replace_source branch from c3219ea to 89ea6eb Compare December 3, 2024 09:53
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from f32b9a5 to 4cd88e3 Compare December 3, 2024 09:54
@xxchan xxchan force-pushed the 12-03-feat_meta_support_replace_source branch from 89ea6eb to d56b1da Compare December 4, 2024 01:07
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from 4cd88e3 to e46f9ae Compare December 4, 2024 01:07
@xxchan xxchan force-pushed the 12-03-feat_meta_support_replace_source branch from d56b1da to 0ab300f Compare December 4, 2024 01:17
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch 2 times, most recently from c03e79d to 7c0ac3f Compare December 4, 2024 02:27
@xxchan xxchan force-pushed the 12-03-feat_meta_support_replace_source branch from 0ab300f to 55aa35f Compare December 4, 2024 10:28
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch 2 times, most recently from 4a9a63d to fe53106 Compare December 4, 2024 10:29
@xxchan xxchan force-pushed the 12-03-feat_meta_support_replace_source branch from 55aa35f to 8a37656 Compare December 5, 2024 08:35
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from fe53106 to 122a819 Compare December 5, 2024 08:35
@xxchan xxchan marked this pull request as ready for review December 5, 2024 08:37
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch 3 times, most recently from 4bd5815 to 2bacad6 Compare December 5, 2024 09:50
@xxchan xxchan force-pushed the 12-03-feat_meta_support_replace_source branch from 835348d to ffb51b8 Compare December 10, 2024 08:24
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from 2bacad6 to a52d3bd Compare December 10, 2024 08:25
@xxchan xxchan mentioned this pull request Dec 10, 2024
Merged
9 tasks
@BugenZhao BugenZhao requested a review from stdrc December 10, 2024 08:34
@xxchan xxchan requested review from BugenZhao and removed request for BugenZhao December 10, 2024 08:36
stdrc
stdrc approved these changes Dec 10, 2024
View reviewed changes
Copy link
Member

@stdrc stdrc left a comment

Choose a reason for hiding this comment

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

lgtm

Base automatically changed from 12-03-feat_meta_support_replace_source to main December 10, 2024 09:44
@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from a52d3bd to 423935c Compare December 10, 2024 09:47
@xxchan xxchan changed the title feat(frontend): support alter shared source feat(frontend): support alter add column for shared source Dec 10, 2024
@graphite-app graphite-app bot requested a review from a team December 10, 2024 10:26
xxchan
xxchan commented Dec 10, 2024
View reviewed changes
src/frontend/src/handler/alter_source_column.rs
Copy link
Member Author

@xxchan xxchan Dec 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

My concern is the old implementation for ALTER SOURCE ADD COLUMN for non-shared source is already problematic. (date back to #17293) You may get a rough idea from the fact that SourceVersion doesn't have a last_column_id like TableVersion. cc @BugenZhao

But seemingly we haven't make it worse now...

Copy link
Member Author

Choose a reason for hiding this comment

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

#17340

@xxchan xxchan force-pushed the 12-03-feat_frontend_support_alter_shared_source branch from 423935c to d2a8cc2 Compare December 11, 2024 02:09
@xxchan xxchan added this pull request to the merge queue Dec 11, 2024
Merged via the queue into main with commit 11dac77 Dec 11, 2024
34 of 35 checks passed
@xxchan xxchan deleted the 12-03-feat_frontend_support_alter_shared_source branch December 11, 2024 07:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BugenZhao BugenZhao BugenZhao approved these changes

@stdrc stdrc stdrc approved these changes

@chenzl25 chenzl25 Awaiting requested review from chenzl25

@st1page st1page Awaiting requested review from st1page

@xiangjinwu xiangjinwu Awaiting requested review from xiangjinwu

Assignees
No one assigned
Labels
ci/run-e2e-single-node-tests ci/run-e2e-test-other-backends type/feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@xxchan @stdrc @BugenZhao