Repository navigation
fix: handle negative-scale Decimals in increment, decrement and truncate_decimal - #4285
Open
AP24110011695 wants to merge 1 commit into
Open
AP24110011695 wants to merge 1 commit into
AP24110011695 wants to merge 1 commit into
Conversation
…ate_decimal Decimal.as_tuple().exponent carries the negated scale, so a Decimal with a non-negative exponent (1E+2) has a negative scale. abs() turned that exponent into a positive scale, so increment()/decrement() returned values on the wrong order of magnitude (1E+2 -> 0.02 instead of 2E+2) and truncate_decimal() landed on a wrong scale. Same root cause as apache#3997, remaining call sites. Signed-off-by: Ayush Kumar Saha <225407166+AP24110011695@users.noreply.github.com>
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
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 #4284
Problem
Decimal.as_tuple().exponentcarries the negated scale, soDecimal("1E+2")(100) has exponent+2, i.e. scale-2. Three remaining call sites still read the scale asabs(int(exponent)), which turns that-2into+2:DecimalLiteral.increment():1E+2stepped to0.02instead of2E+2DecimalLiteral.decrement():1E+2stepped to0.00instead of0E+2truncate_decimal():1.5E+3truncated to0.12instead of1.2E+3Same root cause as #3997 (which fixed the
to(DecimalType)/ serialize sites); this PR fixes the remaining ones. For all positive-scale Decimals the two formulas are identical, so behavior only changes for negative-scale values, where it was wrong.Tests
Failing-first regression tests, all verified locally (Python 3.12, Windows):
test_decimal_literal_increment_negative_scale/test_decimal_literal_decrement_negative_scale(extendedtests/expressions/test_literals.py, mirroring the existing increment/decrement tests)test_truncate_decimal(first coverage for this helper's normal path, including the floor-toward-negative-infinity case) +test_truncate_decimal_negative_scaleintests/utils/test_decimal.pyAll 4 tests fail on
mainand pass with this change. Fulltests/expressions/test_literals.py(182 passed),tests/utils/test_decimal.py, andtests/test_transforms.py(275 passed; the 17 pyarrow-parametrized failures in that file fail identically on unmodifiedmainin this environment).ruff(pinned 0.14.3) format + check clean on all touched files;mypy(1.18.2, repo config) clean on the changed module.Transparent disclosure: developed and verified with AI assistance under human direction, per the repo's agent guidelines.