fix: keep a division's operands grouped on Snowflake and Databricks

pkg-py/src/commons/_definitions/_emit_sql.py

@@ -327,8 +327,13 @@ def _divide(node: Ir, state: _State, selected: str | None) -> str:
     null, so the cases are written out.
     """
     dialect = state.dialect
+    # The guards compare each operand, which binds looser than arithmetic, so
+    # only the quotient needs the operands wrapped at division's precedence.
     numerator = _child(node.attrs["lhs"], 0, "free", state, selected)
     divisor = _child(node.attrs["rhs"], 0, "free", state, selected)
+    quotient = _infix(
+        node.attrs["lhs"], node.attrs["rhs"], "/", _MULTIPLICATIVE, state, selected
+    )
     nan = _non_finite_literal("nan", dialect)
     infinity = _non_finite_literal("inf", dialect)
     negative_infinity = _non_finite_literal("-inf", dialect)
@@ -339,7 +344,7 @@ def _divide(node: Ir, state: _State, selected: str | None) -> str:
         f"WHEN {numerator} IS NULL THEN NULL "
         f"WHEN {is_nan} OR {numerator} = 0 THEN {nan} "
         f"WHEN {numerator} < 0 THEN {negative_infinity} ELSE {infinity} END "
-        f"ELSE {numerator} / {divisor} END"
+        f"ELSE {quotient} END"
     )
 
 

pkg-r/NEWS.md

@@ -1,5 +1,7 @@
 # commons (development version)
 
+* A definition that divides by a compound expression, such as `amount / (ratio * 2)`, now compiles to the correct Snowflake and Databricks SQL. The parentheses around the divisor were dropped, so the query divided by `ratio` alone (#439).
+
 * `commons()` gains a `mode` argument. With `mode = "trusted only"`, the agent answers only with trusted calculations. It never writes its own SQL or R, and it tells the user when no trusted calculation answers a question. These answers carry no provenance markers.
 
 * `describe_table` reports complete table schemas and documentation without a sampled-row summary. DuckDB descriptions use database-native types. Snowflake and Databricks continue to use native metadata, while other DBI connections infer R column classes from five sampled rows, or zero rows in trusted-only mode (#319).

pkg-r/R/definition-emit-sql.R

@@ -396,6 +396,17 @@ definition_sql_divide <- function(node, state, dialect, selected) {
     dialect,
     selected
   )
+  # The guards compare each operand, which binds looser than arithmetic, so
+  # only the quotient needs the operands wrapped at division's precedence.
+  quotient <- definition_sql_infix(
+    node$lhs,
+    node$rhs,
+    "/",
+    6L,
+    state,
+    dialect,
+    selected
+  )
   nan <- definition_sql_non_finite_literal("nan", dialect)
   inf <- definition_sql_non_finite_literal("inf", dialect)
   neg_inf <- definition_sql_non_finite_literal("-inf", dialect)
@@ -407,7 +418,7 @@ definition_sql_divide <- function(node, state, dialect, selected) {
       "WHEN %s IS NULL THEN NULL ",
       "WHEN %s OR %s = 0 THEN %s ",
       "WHEN %s < 0 THEN %s ELSE %s END ",
-      "ELSE %s / %s END"
+      "ELSE %s END"
     ),
     divisor,
     numerator,
@@ -417,8 +428,7 @@ definition_sql_divide <- function(node, state, dialect, selected) {
     numerator,
     neg_inf,
     inf,
-    numerator,
-    divisor
+    quotient
   )
 }
 

pkg-r/tests/testthat/fixtures/shared/definition-warehouse-sql.json

@@ -74,8 +74,16 @@
         "SQL(snowflake)": { "notes_contain": ["newline characters"] }
       },
       "survey::negative quotient": {
-        "SQL(snowflake)": { "code_contains": ["CASE WHEN"] },
-        "SQL(databricks)": { "notes_contain": ["negative zero divisor"] }
+        "SQL(snowflake)": {
+          "code_contains": [
+            "CASE WHEN \"ratio\" + 1.0 = 0 THEN",
+            "ELSE -(\"amount\" - 2) / (\"ratio\" + 1.0) END"
+          ]
+        },
+        "SQL(databricks)": {
+          "code_contains": ["ELSE -(`amount` - 2) / (`ratio` + 1.0) END"],
+          "notes_contain": ["negative zero divisor"]
+        }
       }
     }
   },

tests/shared/definition-warehouse-sql.json

@@ -74,8 +74,16 @@
         "SQL(snowflake)": { "notes_contain": ["newline characters"] }
       },
       "survey::negative quotient": {
-        "SQL(snowflake)": { "code_contains": ["CASE WHEN"] },
-        "SQL(databricks)": { "notes_contain": ["negative zero divisor"] }
+        "SQL(snowflake)": {
+          "code_contains": [
+            "CASE WHEN \"ratio\" + 1.0 = 0 THEN",
+            "ELSE -(\"amount\" - 2) / (\"ratio\" + 1.0) END"
+          ]
+        },
+        "SQL(databricks)": {
+          "code_contains": ["ELSE -(`amount` - 2) / (`ratio` + 1.0) END"],
+          "notes_contain": ["negative zero divisor"]
+        }
       }
     }
   },