impl alter database rename #4984
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Thanks for the contribution! Please review the labels and make any necessary changes. |
mergify
bot
added
pr-feature
| @@ -451,6 +453,110 @@ impl StateMachine { | |||
| Ok(AppliedState::DatabaseMeta(Change::new(None, None))) | |||
| } | |||
|
|
|||
| fn apply_rename_database_cmd( | |||
There was a problem hiding this comment.
Should we reuse apply_drop_database_cmd and apply_create_database_cmd code? Like apply_rename_table_cmd.
There was a problem hiding this comment.
Should we reuse
apply_drop_database_cmdandapply_create_database_cmdcode? Likeapply_rename_table_cmd.
I think renamed database should use old database's db_id.
create database old;
alter database old rename to new not equal with:
create database old;
drop database old;
create database new;But maybe we can try to extract the same part.
There was a problem hiding this comment.
I think renamed database should use old database's db_id.
I'm not sure, it seems that the id and name are bound now.
There was a problem hiding this comment.
Yep, actually in apply_rename_database_cmd
table_id is table_name's id and table_meta also from old table( &prev.as_ref().unwrap().data).
There was a problem hiding this comment.
I think renamed database should use old database's db_id.
I'm not sure, it seems that the id and name are bound now.
The db_id and table_id need to remain the same when changing the db/table meta
There was a problem hiding this comment.
The db_id and table_id need to remain the same when changing the db/table meta
Thanks, I found it #4838
There was a problem hiding this comment.
@TCeason It looks like there is no conflict between reusing the code and not changing the db_id, It can also be refactored in another PR.
There was a problem hiding this comment.
@TCeason It looks like there is no conflict between reusing the code and not changing the db_id, It can also be refactored in another PR.
Agree with you.
|
@TCeason |
|
@TCeason |
Got. By the way, does the refactor have PR or ISSUE? I want to link it here. |
|
@TCeason |
Okay, I got it |
I hereby agree to the terms of the CLA available at: https://databend.rs/dev/policies/cla/
Summary
Support alter database .
Changelog
Related Issues
Fixes #4839