Sitelet https://github.com/apache/paimon/pull/10376
Skip to content

[format] Keep DECIMAL precision when reading JSON numbers - #10376

Open
thswlsqls wants to merge 1 commit into
apache:masterfrom
thswlsqls:fix/json-decimal-precision
Open

thswlsqls wants to merge 1 commit into
apache:masterfrom
thswlsqls:fix/json-decimal-precision

Conversation

@thswlsqls

Copy link
Copy Markdown
Contributor

Purpose

fix #10374

  • JsonFileReader parsed JSON numbers as double, rounding numeric DECIMAL values (12345678901234567.89 read as ...568.00).
  • When the read type contains a DECIMAL (including nested), floats are now parsed as BigDecimal; other columns keep their previous double text.
  • fileformat.md maps DECIMAL to a JSON number; Paimon's writer stores DECIMAL as a string, so round-trip tests missed this.
  • Trade-off: in rows with a DECIMAL column, a JSON number -0.0 for FLOAT/DOUBLE reads as 0.0 (same conditional approach as Flink's flink-json). Tables without DECIMAL are unchanged.

Tests

  • Added to JsonFileFormatTest: testReadNumericDecimalKeepsPrecision (top-level, ARRAY, MAP, ROW; fails on master) and regression guards testReadNumericNonDecimalFieldsInDecimalRowUnchanged, testReadNegativeZeroDoubleWithoutDecimal.
  • mvn -pl paimon-format clean install: BUILD SUCCESS, 770 tests, 0 failures.

JsonFileReader parsed every JSON number through double, so numeric
DECIMAL values beyond double precision were silently rounded. When the
read row type contains a DECIMAL, parse floats as BigDecimal; other
types keep the text they got from double before.

Generated-by: Claude Code

@JingsongLi JingsongLi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requirement fit: SUPPORTED. Implementation: FINDINGS.

The numeric DECIMAL precision fix has a real end-to-end benefit. I ran JsonFileFormatTest on JDK 8 with the normal Maven checks: 25 passed and 1 existing test was skipped. Two additional mixed-schema regression tests fail on this head and pass with the baseline JsonFileReader; details are inline.

containsDecimal(rowType)
? JsonSerdeUtil.OBJECT_MAPPER_INSTANCE
.reader()
.with(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Keep non-DECIMAL values independent of the projection. This reader-wide flag irreversibly loses the sign of every numeric zero before the non-DECIMAL conversion at lines 140–144. For example, reading {"s":-0.0,"d":1.5} with s STRING, d DECIMAL(20,2) now returns s = "0.0", whereas reading only s (or using the baseline reader) returns "-0.0"; the same projection change turns a DOUBLE's raw bits from Long.MIN_VALUE into 0. This means adding an unrelated projected DECIMAL column changes existing string values as well as floating-point values. I reproduced both cases with actual JSON files: 2/2 tests fail on this commit and 2/2 pass against the baseline reader. Please preserve the original numeric representation for non-DECIMAL fields, e.g. by retaining raw numeric tokens or applying exact decimal parsing per field, and cover mixed-schema/projection reads. The trade-off mentioned in the PR body does not cover the STRING value change or make projection-dependent results safe.

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.

[Bug] JSON format reader rounds numeric DECIMAL values through double

2 participants