-
Notifications
You must be signed in to change notification settings - Fork 315
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 parsing the RENAME TABLE statements in the parser #780
feat: support parsing the RENAME TABLE statements in the parser #780
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #780 +/- ##
===========================================
+ Coverage 85.42% 85.49% +0.06%
===========================================
Files 414 414
Lines 54230 53904 -326
===========================================
- Hits 46328 46085 -243
+ Misses 7902 7819 -83
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
@evenyag PTAL |
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 @e1ijah1. Looks like there is no test covering the parser's change, do you mind adding some?
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.
Let us leave the table
and mito
crates unchanged and add the AlterKind::RenameTable
variant in the PR implementing rename table for the mito table engine. So this PR only affects the parser.
d424ffe
to
f745a90
Compare
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.
I think it's fine to revert these changes.
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.
Mostly LGTM. This should be the last suggestion 😄
Co-authored-by: Yingwen <[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.
Looks good to me ❤️
…timeTeam#780) * feat: add parsing `alter rename table` syntax to the parser * chore: fix clippy * chore: add test for parser * fix: add test for parsing RENAME keyword * chore: remove unused code * fix: parse table name object Co-authored-by: Yingwen <[email protected]> * chore: fmt code Co-authored-by: Yingwen <[email protected]>
I hereby agree to the terms of the GreptimeDB CLA
What's changed and what's your intention?
support parsing the RENAME TABLE statements in the parser
Checklist
Refer to a related PR or issue link (optional)
#723