fix(tsql): ALTER COLUMN with a nullability clause is parsed as a Command - #5937
Open
sravankumarkunadi wants to merge 1 commit into
Open
fix(tsql): ALTER COLUMN with a nullability clause is parsed as a Command#5937sravankumarkunadi wants to merge 1 commit into
sravankumarkunadi wants to merge 1 commit into
Conversation
Signed-off-by: sravankumarkunadi <sravankumarkunadi@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
ALTER TABLE t ALTER COLUMN c INT NOT NULLdoesn't parse in T-SQL and falls back to aCommand. Macros aren't resolved inside aCommand, so@SQL('@this_model')reaches SQL Server verbatim and fails withIncorrect syntax near '@SQL'.sqlglot's
_parse_alter_table_alterreturns as soon as it has the type, so T-SQL's trailingNOT NULLis left unconsumed and parsing raises. TheADD CONSTRAINTvariant in the issue works because it parses cleanly.Adds a T-SQL-scoped parser override that consumes the trailing
NULL/NOT NULLintoallow_null, plus a generator override to emit it — sqlglot'saltercolumn_sqlreturns at thedtypebranch before it reachesallow_null. Both only engage when adtypeis present, soSET NOT NULL/DROP NOT NULLkeep their existing path.Post-statement from the issue,
creatingstage:Doesn't address the reporter's aside about declaring a column
NOT NULLwithout a post-statement.Closes #5932
Test Plan
test_tsql_alter_column_nullability(test_dialect.py) — parse/round-trip forINT NOT NULL,INT NULL, bareINT, the@IFform from the issue, andDROP NOT NULL. Fails onmain.test_tsql_alter_column_post_statement(test_model.py) —render_post_statementsresolves@this_modelat thecreatingstage.pytest tests/core/test_dialect.py tests/core/test_model.py tests/core/test_macros.py→ 609 passed vs 607 onmain; both runs hit the same 26 pre-existingsushi_contextfixture errors (Materialization strategy ... was not found).Checklist
make styleand fixed any issues —ruff format/ruff check/mypyon the changed files, notpre-commit run --all-filesmake fast-test) — ran the three files above, see the pre-existing local errors noted theregit commit -s) per the DCO