fix(macros): keep SAFE_ADD, SAFE_SUB and SAFE_DIV grouped inside surrounding operators

[Rodrigo-Palma]

Description

Closes #5649.

@SAFE_ADD and @SAFE_SUB build CASE WHEN <all fields> IS NULL THEN NULL ELSE <arithmetic> END. When a field is a literal, the optimizer resolves 1 IS NULL to false and sqlglot's simplify_conditionals replaces the whole CASE with its ELSE branch. The branch is inserted as a bare Sub/Add node, so an enclosing operator swallows its left operand. Parentheses written around the macro call don't help: simplify_parens drops them in the same pass because the node they wrap is still a CASE at that point.

With the model from the issue, the projection renders as

CASE WHEN ... END * COALESCE(1, 0) - COALESCE("test_data"."percent_off" / 100, 0)

and DuckDB returns 99.8, 175 and 489 instead of 80, 175 and 0.

The collapse itself is sqlglot behaviour and reproduces without SQLMesh on 30.8.0 (the current pin), 30.18.0 and sqlglot main:

simplify(parse_one("SELECT x * CASE WHEN FALSE THEN NULL ELSE a - b END FROM t")).sql()
# 'SELECT x * a - b FROM t'

That deserves an upstream fix (wrapping the promoted branch when its new parent is an operator), and it also affects hand written CASE expressions. This PR makes the macros correct under the current pin regardless of that: the ELSE arithmetic is emitted as an explicit Paren, which is what the AST should hold for a grouped value, and the optimizer keeps it wherever it is needed.

@SAFE_DIV had a related defect that does not involve the optimizer. It returns a bare Div, and after the evaluator serializes and reparses the query, the quotient is no longer a single operand: x / @SAFE_DIV(a, b) rendered as x / a / NULLIF(b, 0), which is (x / a) / b. The macro now returns the quotient wrapped in a Paren.

Output changes to be aware of:

• @SAFE_ADD/@SAFE_SUB render ELSE (COALESCE(a, 0) + COALESCE(b, 0)) END in every usage.
• @SAFE_DIV used as a standalone projection keeps its outer parentheses when the query is not optimized (for example, dependencies missing from the schema). The optimizer removes them otherwise.
• Fingerprints are unaffected: the data hash uses the unrendered query.

The doctests and the three examples in docs/concepts/macros/sqlmesh_macros.md were updated to the new output.

I also tried adding the parentheses generically in MacroEvaluator.transform for any binary result placed under an operator. It changes unrelated output (for example test_merge_filter_macro gains parentheses around a predicate) and would need full precedence handling, so it is left out of this fix.

Test Plan

• New test_safe_arithmetic_macros_keep_precedence_after_optimization in tests/core/test_macros.py, parametrized over the issue example and sibling cases (x * @SAFE_SUB(1, y), -@SAFE_SUB(1, y), x - @SAFE_ADD(1, y), x * @SAFE_ADD(1, y), nested @SAFE_DIV(@SAFE_SUB(...), x), x / @SAFE_DIV(...), x * @SAFE_DIV(...), plus two standalone usages). It renders a model through the optimizer and asserts the projection SQL. 9 of the 10 cases fail on main; the remaining standalone @SAFE_DIV case passes on both and guards against the optimizer keeping redundant parentheses.
• Ran the issue model on DuckDB: 80, 175, 0 after the change (99.8, 175, 489 before).
• make py-style: ruff, ruff-format, mypy and migrations check pass.
• tests/core/test_macros.py: 152 passed (143 passed, 9 failed against main).
• make doc-test: 29 passed.
• make fast-test: 2634 passed, 4 skipped, then 5, 1 and 161 passed in the isolated steps.
• make cicd-test (Python 3.12, macOS): 3393 passed, 22 skipped in the main step. The isolated, registry and dialect steps pass (10, 1, 161). The pyspark step passes (3) once SPARK_HOME points at pyspark's bundled Spark instead of a missing local install. It fails the same way on main with that local setting.

Checklist

• I have run make style and fixed any issues
• I have added tests for my changes (if applicable)
• All existing tests pass (make fast-test)
• My commits are signed off (git commit -s) per the DCO

[Rodrigo-Palma]

Upstream now has a fix for the root cause. tobymao/sqlglot#8384, against the issue I filed there (tobymao/sqlglot#8380), parenthesizes the promoted branch when a collapsed CASE/IF lands under an operator. Its fixtures cover the shapes these macros produce: x * CASE WHEN FALSE THEN NULL ELSE a - b END, the same with explicit parentheses around the CASE, the IF form, and the unary -CASE .... It is still open, not merged.

Two things that don't change because of it:

1. The pin. pyproject.toml requires sqlglot~=30.8.0 and the latest sqlglot release is 30.18.0, so the upstream fix would land in 30.19.0, outside the current range. @SAFE_ADD and @SAFE_SUB stay wrong on every sqlglot version SQLMesh currently accepts until that bump happens.

2. @SAFE_DIV. That defect does not involve the optimizer: the macro returns a bare Div, and once the evaluator serializes and reparses the query, x / @SAFE_DIV(a, b) renders as x / a / NULLIF(b, 0). fix(optimizer): preserve precedence when collapsing CASE/IF branches [BIG-PICKLE] tobymao/sqlglot#8384 does not touch that path, so the Paren in this PR is what fixes it.

Three ways to go, whichever you prefer:

• Merge as is. The macros emit a Paren for a value that is grouped, which is what the AST should hold regardless of the optimizer, and it is correct before and after any bump. The cost is the output change listed above: ELSE (COALESCE(a, 0) + COALESCE(b, 0)) END in every usage.
• Keep only the @SAFE_DIV change and let the sqlglot bump fix @SAFE_ADD/@SAFE_SUB. I can trim the PR and its tests to that scope.
• Close this and track it behind the bump. In that case I'd leave @SAFE_SUB gets optimized during model rendering, causing improper calculations #5649 open with the upstream link, since it reproduces on every currently pinned version.

Happy to do any of the three.

[Rodrigo-Palma]

Correcting my comment above: tobymao/sqlglot#8384 was closed without merging, about four hours after I linked it. georgesittas closed it with "I don't think this is the right approach. Closing and will take this on myself." My issue there (tobymao/sqlglot#8380) is still open, and there is no replacement PR open at the moment.

I checked the current state rather than assuming, against main at 59e9702 (2026-09-18):

input	simplify() on sqlglot main today
x * CASE WHEN FALSE THEN NULL ELSE a - b END	x * a - b
x * IF(FALSE, NULL, a - b)	x * a - b
-CASE WHEN FALSE THEN NULL ELSE a - b END	-a - b
x * (CASE WHEN FALSE THEN NULL ELSE a - b END)	x * (a - b) (correct)

So the root cause is still live upstream, with no version to wait for: no open PR, and the maintainer reimplementing it on his own schedule. That removes the timing argument from my third option (close this and track it behind the bump), since there is nothing concrete to track it behind.

The rest of the comment stands. @SAFE_DIV never depended on the optimizer anyway, and sqlglot~=30.8.0 would keep @SAFE_ADD/@SAFE_SUB broken on every version this project currently accepts even after an upstream fix ships. Still happy to merge as is, or to trim this down to the @SAFE_DIV change alone, whichever you prefer.

[Rodrigo-Palma]

Update: the root cause is fixed upstream. georgesittas opened tobymao/sqlglot#8389 himself and merged it today (2026-09-19, 06c7b3d), closing my issue tobymao/sqlglot#8380 as completed. It touches optimizer/simplify.py and the simplify fixtures.

I verified it rather than taking the title for it, on sqlglot main at 99940ea (which contains that commit):

input	simplify() on sqlglot main today
x * CASE WHEN FALSE THEN NULL ELSE a - b END	x * (a - b)
x * IF(FALSE, NULL, a - b)	x * (a - b)
-CASE WHEN FALSE THEN NULL ELSE a - b END	-(a - b)
x * (CASE WHEN FALSE THEN NULL ELSE a - b END)	x * (a - b)

All four are correct now, so the timing argument I withdrew yesterday is back: there is something concrete to wait for.

Two things still hold:

1. The pin. pyproject.toml requires sqlglot~=30.8.0, which accepts 30.8.x only. The fix is not in any release yet (the latest tag is v30.18.0, from 2026-09-03, and main is 75 commits ahead of it), so it ships in 30.19.0 at the earliest, and it is marked breaking (fix(optimizer)!), which makes the bump a decision of its own rather than a routine one. @SAFE_ADD and @SAFE_SUB stay wrong on every sqlglot version this project currently accepts until that happens.

2. @SAFE_DIV. Unchanged by any of this. The macro returns a bare Div, and once the evaluator serializes and reparses the query, x / @SAFE_DIV(a, b) renders as x / a / NULLIF(b, 0). sqlglot#8389 does not touch that path; the Paren in this PR is what fixes it.

So the same three options, with the third one now viable:

• Merge as is. The macros emit a Paren for a value that is grouped, correct before and after any bump, at the cost of the output change listed in the description.
• Trim to the @SAFE_DIV change and let the sqlglot bump fix @SAFE_ADD/@SAFE_SUB. I can cut the PR and its tests to that scope.
• Close this and track it behind the bump, keeping @SAFE_SUB gets optimized during model rendering, causing improper calculations #5649 open with the upstream link, since it reproduces on every currently pinned version.

Whichever you prefer. Note that CI here never ran: the workflows are sitting in action_required awaiting approval, so the checks on this PR are empty rather than failing.

[Rodrigo-Palma]

Ran this repository's full CI on my fork, on the same commit as this PR (8bd2483), since the runs here are still awaiting approval:

https://github.com/Rodrigo-Palma/sqlmesh/actions/runs/35449729558

28 jobs, all green: style-and-cicd-tests on Python 3.9 through 3.13, cicd-tests-windows, migration-test, doc-tests, test-dbt-versions 1.3 through 1.12, and engine-tests-docker across duckdb, postgres, mysql, mssql, trino, spark, clickhouse, starrocks and risingwave. The only skipped jobs are the ones the changes job switches off for areas this PR does not touch.

This supersedes the last line of my previous comment, which said CI had never run here.