-
Notifications
You must be signed in to change notification settings - Fork 590
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(meta): rename replace_table to replace_stream_job where appropriate #19537
refactor(meta): rename replace_table to replace_stream_job where appropriate #19537
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
#[derive(Debug, Clone)] | ||
pub struct ReplaceTablePlan { | ||
pub struct ReplaceStreamJobPlan { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can see no fields here is strongly related with Table
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the contrary, the proto contains catalog.Table
, so it's need more refactoring and cannot be simply renamed.
risingwave/proto/ddl_service.proto
Lines 356 to 369 in cd83140
message ReplaceTablePlan { | |
// The new table catalog, with the correct (old) table ID and a new version. | |
// If the new version does not match the subsequent version in the meta service's | |
// catalog, this request will be rejected. | |
catalog.Table table = 1; | |
// The new materialization plan, where all schema are updated. | |
stream_plan.StreamFragmentGraph fragment_graph = 2; | |
// The mapping from the old columns to the new columns of the table. | |
// If no column modifications occur (such as for sinking into table), this will be None. | |
catalog.ColIndexMapping table_col_index_mapping = 3; | |
// Source catalog of table's associated source | |
catalog.Source source = 4; | |
TableJobType job_type = 5; | |
} |
710029a
to
792f5e3
Compare
792f5e3
to
669ef4f
Compare
Signed-off-by: xxchan <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
match streaming_job { | ||
StreamingJob::Table(_source, table, _table_job_type) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shall we let .. else { .. }
to avoid indent change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will add other branch soon...
Signed-off-by: xxchan <[email protected]>
I hereby agree to the terms of the RisingWave Labs, Inc. Contributor License Agreement.
What's changed and what's your intention?
Although it was only for sink into table/ alter table now, actually in most places,
replace_table
is as general asreplace_stream_job
.I noticed this after renaming
TableFragments
toStreamJobFragments
in #19510.This can pave way for #18935
Also include some minor style refactoring.
Checklist
./risedev check
(or alias,./risedev c
)Documentation
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.