JingsongLi commented on code in PR #10376:
URL: https://github.com/apache/paimon/pull/10376#discussion_r4177892114
##########
paimon-format/src/main/java/org/apache/paimon/format/json/JsonFileReader.java:
##########
@@ -64,12 +70,34 @@ public JsonFileReader(
throws IOException {
super(fileIO, filePath, rowType, options.getLineDelimiter(), offset,
length);
this.options = options;
+ // Parse floating-point numbers as BigDecimal only when needed, so
DECIMAL values keep
+ // their precision instead of being rounded through double.
+ this.jsonReader =
+ containsDecimal(rowType)
+ ? JsonSerdeUtil.OBJECT_MAPPER_INSTANCE
+ .reader()
+
.with(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
Review Comment:
Withdrawn after the maintainer excluded signed-zero equivalence/sign
observations from this review scope. No change is requested for this finding.
Original observation, retained for history:
[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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
