sql: preserve integer digits in decimal division - #174719
Open
Alignyx wants to merge 1 commit into
Open
Conversation
Decimal division previously used a fixed 20-significant-digit context. When a quotient had more than 20 integer digits, this rounded digits to the left of the decimal point. A variance-equivalent expression could then subtract that rounded quotient from an exact sum and return a false non-zero residual. Select additional division precision only when an inexact result has a positive exponent, preserving the existing 20-digit behavior for ordinary quotients. Use the same helper for row evaluation, constant resolution, and generated vectorized projections. Add evaluator and SQL regressions for cockroachdb#168373. Release note (bug fix): Fixed decimal division rounding integer digits in large quotients, which could produce incorrect results in expressions that combine exact aggregates with division.
|
Thank you for contributing to CockroachDB. Please ensure you have followed the guidelines for creating a PR. My owl senses detect your PR is good for review. Please keep an eye out for any test failures in CI. 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
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
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.
Fixes #168373.
Root cause
SQL decimal division always used the fixed 20-significant-digit
DecimalCtx. In the reported expression,5845710769898530048144 / 44requires 21 integer digits, so the exact quotient132857062952239319276was rounded to1.3285706295223931928E+20. Subtracting that rounded quotient from the exact sum left-4, and the final division produced-1/11even though every input in the group was constant and the variance-equivalent expression must be zero.How I tracked it down
I reproduced the result for both constants and all three reported groups, then compared the expression with
VAR_POP, a translation to zero, and an algebraically equivalent form that performs the division last; all three metamorphic oracles returned zero. Changing only the row count showed the precision boundary: counts 4 and 11 returned zero, while 44 and 101 returned-1/n. Optimizer and distributed plans had the same aggregation levels, and row and vectorized execution failed identically. Inspecting the final scalar render then exposed the rounded quotient and exact-4residual, locating the problem in the shared decimal division policy rather than aggregation.Fix
Added
tree.DecimalQuo, which first divides with the existing 20-digit context and retries only when an inexact finite result has a positive exponent, adding just enough precision to retain the quotient's integer portion. The helper preserves aliased operands across a retry. Row evaluation, numeric constant resolution, and generated vectorized decimal/integer division now use the same helper. Ordinary fractional results retain the established 20-significant-digit behavior, so this does not raise the global decimal context or change unrelated decimal operations.Test coverage
10/3retaining the existing 20-digit result.VAR_POPas the zero oracle.