Support Modify Column for MySQL dialect
#1216
Conversation
(cherry picked from commit 6c27f05)
| options, | ||
| column_position, | ||
| } => { | ||
| write!(f, "MODIFY COLUMN {col_name} {data_type}")?; |
There was a problem hiding this comment.
It seems we'll need to track whether the COLUMN keyword was specified? Otherwise the following wont fulfill round trip ALTER TABLE foo MODIFY mycolumn int since the serialized version becomes ALTER TABLE foo MODIFY COLUMN mycolumn int
There was a problem hiding this comment.
CHANGE COLUMN seems to have the same problem? I'm not sure if I need to address it all together here
There was a problem hiding this comment.
oh I see, yeah no need to tackle that one in this PR unless you feel its straight-forward and want to. for this PR main would be to ensure the behavior for modify column
There was a problem hiding this comment.
@KKould double checking: did you plan to update the PR for the MODIFY COLUMN variant introduced?
There was a problem hiding this comment.
@KKould double checking: did you plan to update the PR for the
MODIFY COLUMNvariant introduced?
nope
There was a problem hiding this comment.
I agree it would be great to ensure this type of syntax roundtrips but no need to do it in this PR
|
cc @alamb changes LGTM for review |
| options, | ||
| column_position, | ||
| } => { | ||
| write!(f, "MODIFY COLUMN {col_name} {data_type}")?; |
There was a problem hiding this comment.
I agree it would be great to ensure this type of syntax roundtrips but no need to do it in this PR
Pull Request Test Coverage Report for Build 8679082248Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
(see https://dev.mysql.com/doc/refman/8.0/en/alter-table.html).