Skip to content

Preserve magnitude in BigInteger and BigDecimal locale converters (1.X)#403

Merged
garydgregory merged 1 commit into
apache:1.Xfrom
rootvector2:locale-bigdecimal-parse-1.x
Jun 22, 2026
Merged

Preserve magnitude in BigInteger and BigDecimal locale converters (1.X)#403
garydgregory merged 1 commit into
apache:1.Xfrom
rootvector2:locale-bigdecimal-parse-1.x

Conversation

@rootvector2

Copy link
Copy Markdown
Contributor

Port of #401 to the 1.X branch.

DecimalLocaleConverter.parse builds a DecimalFormat but never enables setParseBigDecimal, so DecimalFormat.parse returns a clamped Long (when the value fits in long) or a lossy Double (out of long range). BigIntegerLocaleConverter and BigDecimalLocaleConverter then derive their result from that Number, so a value past long/double range comes back silently wrong: 9999999999999999999 becomes 9223372036854775807 for BigInteger and 1.0E+19 for BigDecimal. The non-locale BigIntegerConverter/BigDecimalConverter avoid this by parsing the string directly.

Added a protected isParseBigDecimal() hook (default false) on DecimalLocaleConverter and pass it to formatter.setParseBigDecimal(...); only BigIntegerLocaleConverter and BigDecimalLocaleConverter override it to true, so DecimalFormat returns a lossless BigDecimal for them while the narrowing Byte/Short/Integer/Long/Float/Double converters keep the Long/Double path. BigIntegerLocaleConverter converts the BigDecimal via toBigInteger().

Added regression tests to BigIntegerLocaleConverterTest and BigDecimalLocaleConverterTest; both fail without the runtime change (9223372036854775807 and 1.0E+19 respectively).

mvn test -Dtest='*LocaleConverterTest' is green (106 tests). The only failure in a full mvn run is LocaleBeanificationTest.testContextClassloaderIndependence, a pre-existing flake unrelated to this change (it fails the same way on a clean 1.X checkout).

  • Read the contribution guidelines for this project.
  • Read the ASF Generative Tooling Guidance if you use Artificial Intelligence (AI).
  • I used AI to create any part of, or all of, this pull request. Which AI tool was used to create this pull request, and to what extent did it contribute?
  • Run a successful build using the default Maven goal with mvn; that's mvn on the command line by itself.
  • Write unit tests that match behavioral changes, where the tests fail if the changes to the runtime are not applied. This may not always be possible, but it is a best practice.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Each commit in the pull request should have a meaningful subject line and body. Note that a maintainer may squash commits during the merge process.

DecimalLocaleConverter.parse builds a DecimalFormat without enabling
setParseBigDecimal, so a value past long/double range comes back from
DecimalFormat.parse as a clamped Long or a lossy Double; the BigInteger
and BigDecimal converters then derive their result from that Number and
return it silently wrong (9999999999999999999 -> 9223372036854775807 for
BigInteger, -> 1.0E+19 for BigDecimal).

Add a protected isParseBigDecimal() hook (default false) and pass it to
formatter.setParseBigDecimal; only BigIntegerLocaleConverter and
BigDecimalLocaleConverter override it to true, so DecimalFormat returns a
lossless BigDecimal for them while the narrowing Byte/Short/Integer/Long/
Float/Double converters stay on the Long/Double path. BigIntegerLocaleConverter
converts the BigDecimal via toBigInteger().

Found by comparing the locale converters with the non-locale
BigInteger/BigDecimal converters that parse the string directly.
@garydgregory garydgregory changed the title preserve magnitude in BigInteger and BigDecimal locale converters (1.X) Preserve magnitude in BigInteger and BigDecimal locale converters (1.X) Jun 22, 2026
@garydgregory garydgregory merged commit 65b6f8a into apache:1.X Jun 22, 2026
8 checks passed
@garydgregory

Copy link
Copy Markdown
Member

Thank you @rootvector2 , merged 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants