Snowflake drop column fixes #1618
Conversation
Merge updates on main from sqlfluff repo
Update main with sqlfluff main
| OneOf("ALTER", "MODIFY", "DROP"), | ||
| OptionallyBracketed( | ||
| Delimited( | ||
| OneOf( | ||
| # Add things | ||
| Sequence( | ||
| Ref.keyword("COLUMN", optional=True), | ||
| Ref("SingleIdentifierGrammar"), | ||
| ), | ||
| Sequence( | ||
| Ref.keyword("COLUMN", optional=True), | ||
| Ref("SingleIdentifierGrammar"), |
There was a problem hiding this comment.
This is interesting. The DROP syntax is supported further down but insists on column name before.
Should we either:
- Go with what you have, but remove the
DROPfrom line 804? That would allowALTER TABLE ALTER column1which is not technically correct. - Revert your change and make the column before it on line 802 optional? But that would allow
ALTER TABLE ALTER SET NOT NULLwhich is not correct as column is missing. - Wrap whole thing in a
OneOfand leaveALTERandMODIFYalone but pullDROPinto it's own path?
Option 3 Is probably the most correct to be honest.
What's your thoughts?
There was a problem hiding this comment.
Hm. I see your point. Will wrap the whole thing into an OneOf statement.
There was a problem hiding this comment.
@tunetheweb I think it is now fixed. Could you have a look again. Because we wrapped everything into a OneOf statement, I also had to create a sequence for the 'old' path.
| Ref.keyword("COLUMN", optional=True), | ||
| Ref("SingleIdentifierGrammar"), | ||
| OneOf( | ||
| Sequence( |
There was a problem hiding this comment.
This is the added sequence for simple drop column statement
| Ref.keyword("COLUMN", optional=True), | ||
| Ref("SingleIdentifierGrammar"), | ||
| ), | ||
| Sequence( |
There was a problem hiding this comment.
I created a new sequence here, so that the OneOf wrapper 'understands' the steps following after the step OneOf("ALTER", "MODIFY") are related.
|
Thanks and cool @tunetheweb! Thanks for the fast replies again. |
Codecov Report
@@ Coverage Diff @@
## main #1618 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 131 131
Lines 9208 9208
=========================================
Hits 9208 9208
Continue to review full report at Codecov.
|
Brief summary of the change made
Fixes #1617. Linting an 'alter table drop dolumn statement' raised an unparsable section error. Including this specific sequence fixes the issue.
Are there any other side effects of this change that we should be aware of?
Not that I'm aware of.
Pull Request checklist