fix: Swap parser override order for macro IF under T-SQL (Issue #5823)#5828
fix: Swap parser override order for macro IF under T-SQL (Issue #5823)#5828jagannalla wants to merge 1 commit into
Conversation
noezhiya-dot
left a comment
There was a problem hiding this comment.
The fix is a simple line swap but the issue it addresses is important. Two comments:
-
The core change (swapping lines 1125-1126) is correct — registering the base Parser override before the TSQL-specific one ensures the custom _parse_if wrapper is applied in the right order.
-
The test for PRINT strips the quotes from 'hello' — the expected output shows PRINT hello without quotes. Is this intentional? It seems like the T-SQL parser might be stripping string literal quotes from PRINT statements. If this is expected behavior, a comment in the test would help future readers understand why.
-
Consider adding a test that verifies the original error case from issue #5823 (the Required keyword: 'true' missing error) no longer occurs, to make the regression test more explicit.
|
Clean fix — the root cause explanation makes sense, and swapping the override order is the minimal correct change. Tests cover the specific T-SQL case well. LGTM. |
90abae3 to
2d07241
Compare
|
Thanks for the review, @noezhiya-dot! I have updated the PR to address all comments and successfully force-pushed:
Ready for review/merge! |
2d07241 to
304dfc5
Compare
…sh#5823) Signed-off-by: Jagan Nalla <jagannalla1@gmail.com>
304dfc5 to
97e9db2
Compare
Summary
This PR fixes a bug where conditional macro statements (e.g.
@IF(cond, statement)) using the T-SQL / MSSQL dialect would fail validation with:Required keyword: 'true' missing for <class 'sqlglot.expressions.functions.If'>Root Cause
In
extend_sqlglot()withinsqlmesh/core/dialect.py, the overrides were registered in the wrong order:Solution
sqlmesh/core/dialect.pyso the custom parser wrapper is registered on the baseParserfirst.tests/core/test_dialect.pyfor T-SQL-specific statements parsed within@IF.Link to Issue
Closes #5823