fix: formatter function casing#5903
Open
albertosuman-1k5 wants to merge 10 commits into
Open
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Adds test_format_model_expressions_normalize_functions to lock down expected behavior of format_model_expressions across three modes: - default: preserve original spellings in audit references and query body - normalize_functions='upper': upper-case both audit refs and query funcs - normalize_functions='lower': lower-case both audit refs and query funcs The default assertion currently FAILS (red), confirming the regression: audit names (unique_combination_of_columns, not_null) are unconditionally upper-cased instead of being left as-is. This test is intentionally left failing until the production fix is applied. Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…inal casing FormatConfig.normalize_functions now accepts str | bool | None and defaults to False instead of None. False passes normalize_functions=False to the SQLGlot generator, preserving the original casing of custom/anonymous function names (such as SQLMesh audit references like not_null, accepted_range). format_model_expressions gains an explicit normalize_functions parameter with the same False default, ensuring the behaviour is consistent whether the formatter is called directly (tests) or via Context using generator_options. The dialect=None code path for meta expressions (MODEL/AUDIT/METRIC) is unchanged; only the normalize_functions kwarg is now explicitly forwarded. Update test_format_model_expressions expected output to reflect lowercase audit names (not_null, unique_values, accepted_range, accepted_values) and add test_format_config_normalize_functions_false to validate the config default. Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
… path The early-return path in format_model_expressions for a single meta expression (MODEL/AUDIT/METRIC) was calling expression.sql() without forwarding normalize_functions, so that argument was silently ignored for standalone MODEL-only inputs. Pass normalize_functions=normalize_functions explicitly alongside the existing dialect=None so the single-expression path behaves identically to the multi-expression loop path. Also update the test_format_model_expressions_normalize_functions docstring to describe the approved behavior (custom/audit names preserved; SQLGlot built-in functions canonicalize to uppercase because the parser discards original spelling), and add assertions that exercise the single-expression code path to prevent regression. Pre-commit mypy hook fails on 9 pre-existing infrastructure errors (sqlmesh._version missing stub, sqlglot.generators.athena missing stub, etc.) that are unrelated to this change and were present before it (documented in task-2-report.md). Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…vior Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
The FormatConfig.normalize_functions field accepts str | bool | None, but the reference table listed only 'string | boolean'. Explicit null defers to SQLGlot's generator default, which uppercases all function names (including custom ones), making it behaviorally distinct from False (preserve casing). Add null to the type column and one clause to the description so the user-facing docs match the actual accepted values. Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…tests - Expand normalize_functions docstrings in FormatConfig and format_model_expressions to explicitly document True and None as aliases that defer to SQLGlot's default (uppercase custom functions), contrasting them with False (preserve) and the string modes. - Add None regression assertions to the existing normalize_functions test: one for the multi-expression path and one for the single-meta-expression path, both proving that lowercase audit names are uppercased when normalize_functions=None is passed explicitly. - Update docs/reference/configuration.md to mention 'true' alongside 'null' as a valid value that defers to the SQLGlot default. mypy skipped: errors are pre-existing in unrelated files (cache.py, macros.py, state_sync/base.py) and were present before this branch. Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
…not SQLGlot default At the FormatConfig/YAML layer, normalize_functions=None is excluded from generator_options by PydanticModel.dict(exclude_none=True). format_model_expressions therefore uses its own False default, so YAML null behaves like false — not like SQLGlot's direct None deferral. Changes: - FormatConfig docstring: distinguish None (excluded -> False path) from True (defers to SQLGlot default -> uppercase); remove the incorrect claim they are equivalent - format_model_expressions docstring: clarify that direct None passes None to the SQLGlot generator (dialect-dependent) while the config path never reaches this because None is excluded before the call - docs/reference/configuration.md: describe null as taking the false-default path; true remains the value that defers to SQLGlot's generator default - tests: add test_format_config_normalize_functions_none asserting that FormatConfig(normalize_functions=None).generator_options omits the key and that the resulting format call preserves custom function casing (False behaviour) Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Signed-off-by: Alberto Suman <alberto.suman@1komma5grad.com> Co-authored-by: Cursor <cursoragent@cursor.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
1 participant
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
Fixes formatter casing regression introduced by SQLGlot 30.8.x.
normalize_functionstoFalse, preserving custom function and audit reference casing.upper/lowernormalization modes unchanged.normalize_functionsthrough the standalone metadata formatting path while retainingdialect=None.False,True,null,upper, andlowerconfiguration behavior.Note: SQLGlot canonicalizes built-in functions such as
counttoCOUNTbecause the parsed AST does not retain their original source casing.Testing
ruff format --check sqlmesh/core/config/format.py sqlmesh/core/dialect.py tests/core/test_dialect.pypytest tests/core/test_dialect.py tests/core/test_format.py -v -m "not slow and not docker"Result: 162 passed.
Checklist
make styleand fixed any issuesmake fast-test)git commit -s) per the DCO