Repository navigation
fix: Correct two bugs when formatting decimal values - #10869
Merged
Merged
Conversation
`format_decimal_str` truncated the digit string to the declared precision before placing the decimal point, so a value exceeding its declared precision displayed as a different number (12345 at Decimal128(3, 1), i.e. 1234.5, displayed as "12.3"), as a malformed string (12345 at (3, 3) displayed as ".123"), or panicked on a subtraction overflow when the scale exceeds the precision (constructible via `with_data_type` and reachable from IPC/FFI input). Arrays are not validated against their declared precision, and this path backs `value_as_string`, casts to strings, pretty printing, and the CSV and JSON writers. Always format the full value; precision no longer affects formatting. Also format a zero value at negative scale as "0" instead of zero-padding it to "000", which is not a valid number and produced invalid JSON writer output. Closes apache#10866. Closes apache#10865.
github-actions
Bot
added
arrow
cakeni
approved these changes
Aug 27, 2026
cakeni
left a comment
cakeni
left a comment
Contributor
There was a problem hiding this comment.
Looks good to me. Removing the precision-based truncation fixes both reported cases at the shared formatter, and the regression tests cover them.
Jefffrey
approved these changes
Aug 27, 2026
Contributor
|
thanks @neilconway & @cakeni |
Rich-T-kid
pushed a commit
to Rich-T-kid/arrow-rs
that referenced
this pull request
Sep 2, 2026
# Which issue does this PR close? - Closes apache#10866. - Closes apache#10865. # Rationale for this change By default, decimal values are not validated against their type's declared precision. Such out-of-precision values must therefore be handled correctly through arrow-rs. `format_decimal_str` attempted to do so by truncating the digit string to the declared precision, but that is not correct: for example, the value `12345` in a `Decimal128(3, 1)` context was formatted as `12.3`, i.e., it silently produced incorrect values. There are two principled ways to handle such values: format them in their full precision or return an error. Returning an error is impractical: (1) the formatting code doesn't have an easy way to surface such errors (2) this would be inconsistent with how out-of-precision decimals are handled throughout the rest of the Arrow ecosystem. Hence, this PR arranges to format out-of-precision values without truncation; the declared precision no longer affects formatting. Along the way, fix a second bug: a `0` decimal value with a negative scale was zero-padded; for example, formatting with scale `-2` resulted in the string "000". That is not a legal number in JSON, and is also just a surprising and likely unexpected behavior. # What changes are included in this PR? * Fix decimal formatting bugs as described above * Add/update tests # Are these changes tested? Yes; new tests added. # Are there any user-facing changes? Yes, but only in corner-cases. Both scenarios are rare to begin with, and the implemented behavior is unlikely to be what a user would desire. `format_decimal_str` still takes a `precision` parameter that is now unused. I've kept that in the function signature to avoid breaking a public API.
Rich-T-kid
pushed a commit
to Rich-T-kid/arrow-rs
that referenced
this pull request
Sep 18, 2026
# Which issue does this PR close? - Closes apache#10866. - Closes apache#10865. # Rationale for this change By default, decimal values are not validated against their type's declared precision. Such out-of-precision values must therefore be handled correctly through arrow-rs. `format_decimal_str` attempted to do so by truncating the digit string to the declared precision, but that is not correct: for example, the value `12345` in a `Decimal128(3, 1)` context was formatted as `12.3`, i.e., it silently produced incorrect values. There are two principled ways to handle such values: format them in their full precision or return an error. Returning an error is impractical: (1) the formatting code doesn't have an easy way to surface such errors (2) this would be inconsistent with how out-of-precision decimals are handled throughout the rest of the Arrow ecosystem. Hence, this PR arranges to format out-of-precision values without truncation; the declared precision no longer affects formatting. Along the way, fix a second bug: a `0` decimal value with a negative scale was zero-padded; for example, formatting with scale `-2` resulted in the string "000". That is not a legal number in JSON, and is also just a surprising and likely unexpected behavior. # What changes are included in this PR? * Fix decimal formatting bugs as described above * Add/update tests # Are these changes tested? Yes; new tests added. # Are there any user-facing changes? Yes, but only in corner-cases. Both scenarios are rare to begin with, and the implemented behavior is unlikely to be what a user would desire. `format_decimal_str` still takes a `precision` parameter that is now unused. I've kept that in the function signature to avoid breaking a public API.
Rich-T-kid
pushed a commit
to Rich-T-kid/arrow-rs
that referenced
this pull request
Sep 21, 2026
# Which issue does this PR close? - Closes apache#10866. - Closes apache#10865. # Rationale for this change By default, decimal values are not validated against their type's declared precision. Such out-of-precision values must therefore be handled correctly through arrow-rs. `format_decimal_str` attempted to do so by truncating the digit string to the declared precision, but that is not correct: for example, the value `12345` in a `Decimal128(3, 1)` context was formatted as `12.3`, i.e., it silently produced incorrect values. There are two principled ways to handle such values: format them in their full precision or return an error. Returning an error is impractical: (1) the formatting code doesn't have an easy way to surface such errors (2) this would be inconsistent with how out-of-precision decimals are handled throughout the rest of the Arrow ecosystem. Hence, this PR arranges to format out-of-precision values without truncation; the declared precision no longer affects formatting. Along the way, fix a second bug: a `0` decimal value with a negative scale was zero-padded; for example, formatting with scale `-2` resulted in the string "000". That is not a legal number in JSON, and is also just a surprising and likely unexpected behavior. # What changes are included in this PR? * Fix decimal formatting bugs as described above * Add/update tests # Are these changes tested? Yes; new tests added. # Are there any user-facing changes? Yes, but only in corner-cases. Both scenarios are rare to begin with, and the implemented behavior is unlikely to be what a user would desire. `format_decimal_str` still takes a `precision` parameter that is now unused. I've kept that in the function signature to avoid breaking a public API.
Rich-T-kid
pushed a commit
to Rich-T-kid/arrow-rs
that referenced
this pull request
Sep 25, 2026
# Which issue does this PR close? - Closes apache#10866. - Closes apache#10865. # Rationale for this change By default, decimal values are not validated against their type's declared precision. Such out-of-precision values must therefore be handled correctly through arrow-rs. `format_decimal_str` attempted to do so by truncating the digit string to the declared precision, but that is not correct: for example, the value `12345` in a `Decimal128(3, 1)` context was formatted as `12.3`, i.e., it silently produced incorrect values. There are two principled ways to handle such values: format them in their full precision or return an error. Returning an error is impractical: (1) the formatting code doesn't have an easy way to surface such errors (2) this would be inconsistent with how out-of-precision decimals are handled throughout the rest of the Arrow ecosystem. Hence, this PR arranges to format out-of-precision values without truncation; the declared precision no longer affects formatting. Along the way, fix a second bug: a `0` decimal value with a negative scale was zero-padded; for example, formatting with scale `-2` resulted in the string "000". That is not a legal number in JSON, and is also just a surprising and likely unexpected behavior. # What changes are included in this PR? * Fix decimal formatting bugs as described above * Add/update tests # Are these changes tested? Yes; new tests added. # Are there any user-facing changes? Yes, but only in corner-cases. Both scenarios are rare to begin with, and the implemented behavior is unlikely to be what a user would desire. `format_decimal_str` still takes a `precision` parameter that is now unused. I've kept that in the function signature to avoid breaking a public API.
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
3 participants
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.
Which issue does this PR close?
Rationale for this change
By default, decimal values are not validated against their type's declared precision. Such out-of-precision values must therefore be handled correctly through arrow-rs.
format_decimal_strattempted to do so by truncating the digit string to the declared precision, but that is not correct: for example, the value12345in aDecimal128(3, 1)context was formatted as12.3, i.e., it silently produced incorrect values.There are two principled ways to handle such values: format them in their full precision or return an error. Returning an error is impractical: (1) the formatting code doesn't have an easy way to surface such errors (2) this would be inconsistent with how out-of-precision decimals are handled throughout the rest of the Arrow ecosystem. Hence, this PR arranges to format out-of-precision values without truncation; the declared precision no longer affects formatting.
Along the way, fix a second bug: a
0decimal value with a negative scale was zero-padded; for example, formatting with scale-2resulted in the string "000". That is not a legal number in JSON, and is also just a surprising and likely unexpected behavior.What changes are included in this PR?
Are these changes tested?
Yes; new tests added.
Are there any user-facing changes?
Yes, but only in corner-cases. Both scenarios are rare to begin with, and the implemented behavior is unlikely to be what a user would desire.
format_decimal_strstill takes aprecisionparameter that is now unused. I've kept that in the function signature to avoid breaking a public API.