-
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 ALTER TABLE ... MODIFY COLUMN ... ...
#3796
feat: support ALTER TABLE ... MODIFY COLUMN ... ...
#3796
Conversation
ALTER TABLE xxx ALTER COLUMN xxx TYPE xxx
ALTER TABLE ... ALTER COLUMN ... TYPE ...
b8fd671
to
6307c7c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3796 +/- ##
==========================================
- Coverage 85.64% 85.29% -0.35%
==========================================
Files 954 954
Lines 163037 163253 +216
==========================================
- Hits 139625 139246 -379
- Misses 23412 24007 +595 |
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 looks good.
Good job! But why not use the common SQL syntax? ALTER TABLE table_name
MODIFY COLUMN column_name datatype; It would be better that the user doesn't need to learn a new syntax. |
This is the syntax PostgreSQL currently uses. ALTER TABLE distributors
ALTER COLUMN address TYPE varchar(80),
ALTER COLUMN name TYPE varchar(100); |
Yes, but MySQL, Oracle, and SQLServer are using |
tips: sqlparser-rs already supports modify column: apache/datafusion-sqlparser-rs#1216 |
Create an issue here for reminding us the docs after this whole feature is done. |
Hi @KKould There are some conflicts |
f1db676
to
f7668c6
Compare
Is it possible to upgrade the |
I created PR: GreptimeTeam/sqlparser-rs#11 to support |
ddd689f
to
4bd503f
Compare
If the CI failure in GreptimeTeam/sqlparser-rs#11 is easy to fix. We could implement the final syntax in this PR. @KKould Could you please take a look? |
I checked before and it seems that these errors are caused by other PRs, not this PR. |
The pr should be open on v0.44.x instead of the main branch, because the sqlparser code will be synchronized with datafusion' sqlparser code regularly. However, I still recommend not to change the keywords and use parser directly to identify the token to avoid conflict when sync upstream sqlparser later. |
2af1e83
to
83efb1c
Compare
ALTER TABLE ... ALTER COLUMN ... TYPE ...
ALTER TABLE ... MODIFY COLUMN ... ...
I hereby agree to the terms of the GreptimeDB CLA.
Refer to a related PR or issue link (optional)
#3517
GreptimeTeam/greptime-proto#149
What's changed and what's your intention?
Support:
alter table xxx modify column xxx xxx
https://dev.mysql.com/doc/refman/8.0/en/alter-table.html
Checklist