-
Notifications
You must be signed in to change notification settings - Fork 328
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
feat: support Create Table ... Like
#3372
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3372 +/- ##
==========================================
- Coverage 85.07% 84.60% -0.47%
==========================================
Files 889 889
Lines 146377 146454 +77
==========================================
- Hits 124525 123906 -619
- Misses 21852 22548 +696 |
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.
Great work! Thank you. I have a few suggestions, please take a look.
show_create_table::create_table_stmt(&table_ref.table_info(), quote_style) | ||
.context(error::ParseQuerySnafu)?; | ||
create_stmt.name = stmt.name; | ||
create_stmt.if_not_exists = false; |
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.
Looks like create table like
can support if_not_exists
too, just like
CREATE TABLE IF NOT EXISTS snapshot LIKE source_data;
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.
In fact, I thought so at first, but I checked MySQL's Create Table ... Like
and it seems that it is not supported, so I chose to keep it unified. Of course, this is up to you to decide.
ref: https://dev.mysql.com/doc/refman/8.0/en/create-table-like.html
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.
Got it. Let's leave it currently.
my grandmother passed away yesterday, and I will handle the review suggestions you provided in a few days |
Sorry to hear that. May she rest in peace. |
It seems you didn’t notice the content in Tips. please take a look |
…d make `Create Table ... Like` testcase more complicated
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.
LGTM
Signed-off-by: tison <[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.
Thanks for your contribution @KKould! Excellent patch.
I suggest you try to check out a different branch (rather than main
) the next time you submit a PR so that we can avoid some subtle issues like:
$ git push
fatal: The upstream branch of your current branch does not match
the name of your current branch. To push to the upstream branch
on the remote, use
git push https://github.com/KKould/greptimedb.git HEAD:main
To push to the branch of the same name on the remote, use
git push https://github.com/KKould/greptimedb.git HEAD
To choose either option permanently, see push.default in 'git help config'.
To avoid automatically configuring an upstream branch when its name
won't match the local branch, see option 'simple' of branch.autoSetupMerge
in 'git help config'.
Although we can use git push https://github.com/KKould/greptimedb.git HEAD:main
as suggested, it's easy to make mistake.
I hereby agree to the terms of the GreptimeDB CLA
What's changed and what's your intention?
Support:
create table xxx like yyy
ref: https://dev.mysql.com/doc/refman/8.0/en/create-table-like.html
Tips:
I was using this case:
CREATE TABLE t1 ( a INT, b STRING, c INT, ts timestamp TIME INDEX);
then runCREATE TABLE t2 LIKE t1;
, thecreate_partitions_stmt
method returned Some and caused the index to go out of bounds, so I did additional checks. Is this a bug? Because this table does not specifyPartitions
ref: https://github.com/GreptimeTeam/greptimedb/pull/3372/files#diff-2563d4135f7e232aaec6a39204e6e9afc21801dff0c957772ce4dd7d3bd271d4R109
Checklist
Refer to a related PR or issue link (optional)
#2509