Repository navigation
Conversation
jat255
added
bug
Contributor
|
Preview root: https://posit-dev.github.io/commons/pr-442/ Python site preview: https://posit-dev.github.io/commons/pr-442/py/ Built from the latest commit on this branch. The R links in it point at the published R site, which no pull request rebuilds. |
The guarded division rendered both operands at the lowest precedence and joined them with /, so amount / (ratio * 2) became "amount" / "ratio" * 2. The quotient now wraps each operand at division's precedence, as every other infix does. The zero guards keep the bare operands, since a comparison binds looser than arithmetic. The shared warehouse fixture now pins the quotient of the corpus's negative quotient for both targets. Closes #439
jat255
force-pushed
the
jat255/439-divisor-parentheses
branch
from
October 22, 2026 02:46
6806780 to
44d353a
Compare
This branch has not been deployed
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.
Closes #439
This PR fixes the Snowflake and Databricks SQL for a division, which dropped the parentheses around a compound operand.
amount / (ratio * 2)compiled to"amount" / "ratio" * 2, so the query ran without an error and returned a wrong value.Details (agent-written)
The guarded division now builds its quotient with the same precedence rule as every other infix operator, in the Python emitter and the R emitter. The zero guards keep the bare operands, because a comparison binds more loosely than arithmetic.
The shared warehouse fixture now pins the quotient of the corpus definition
negative quotientfor both targets. I did not add a corpus definition with a product divisor. It goes through the same code, and a new corpus entry means regeneratingdefinitions.jsonfrom data-dict and updating its hand-maintained sections and the counts that tests pin.Testing: the full Python and R suites pass. On live Snowflake, with the rest of this stack, every corpus definition returns the same values as DuckDB. Databricks is not verified, because no workspace is available yet.
Kata: whhh (the live warehouse work that found this).
R changes
The R SQL emitter for Snowflake and Databricks now keeps the parentheses around a compound numerator or divisor (
definition-emit-sql.R). A definition such asamount / (ratio * 2)or(a + b) / cused to compile to SQL that computed a different value, and now computes the value that data-dict defines. The DuckDB output does not change. NEWS has a bullet.