You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "Vamsi-klu (via GitHub)" <gi...@apache.org> on 2026/07/11 21:39:38 UTC
[PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Vamsi-klu opened a new pull request, #18976:
URL: https://github.com/apache/pinot/pull/18976
Fixes #12312
## What changed
This PR preserves exact decimal precision when Pinot ingests JSON records through the built-in JSON ingestion path.
The change adds BigDecimal-aware parsing helpers to `JsonUtils` and routes the default JSON record reader, message decoder, and record extractor through those helpers. Numeric JSON values that cannot be represented exactly as `double` now remain `BigDecimal` until Pinot's downstream type conversion logic maps them into the target column type.
The implementation also keeps the custom extractor compatibility boundary explicit:
- The built-in `JSONRecordExtractor` now receives BigDecimal-preserving maps by default.
- A user-supplied custom `recordExtractorClass` keeps the historical `Double` parsing behavior by default, so existing custom extractor code is not surprised by new numeric object types.
- Custom extractors can opt into BigDecimal-preserving parsing with `preserveDecimalPrecision=true`.
## Why it matters
JSON ingestion previously parsed decimal literals through Jackson's default floating-point path. That silently rounded values such as high-precision monetary amounts, identifiers encoded as decimal strings, or scientific measurements before Pinot had a chance to apply the configured schema conversion.
That behavior is especially risky because the corruption happens during ingestion and can be hard to detect later: queries see a stable but already-rounded value. Preserving the JSON decimal as `BigDecimal` keeps the source value intact until Pinot decides how to store it for the configured column.
The custom extractor compatibility guard matters because many deployments use custom extractors that may inspect raw map values directly. Changing those extractors from `Double` to `BigDecimal` without an opt-in could break otherwise unrelated ingestion jobs.
## Impact
Built-in JSON ingestion now keeps decimal precision for values that need it.
Existing ingestion pipelines that use Pinot's built-in JSON extractor get safer precision behavior without requiring table config changes.
Existing custom JSON record extractors keep their previous raw numeric value types unless they explicitly opt into the new precision-preserving map parsing mode.
No query execution behavior is changed by this PR. The change is limited to JSON input decoding and extraction.
## Testing
- `./mvnw -Dmaven.repo.local=/tmp/m2-pinot-12312 -pl pinot-spi,pinot-plugins/pinot-input-format/pinot-json -Dtest=JsonUtilsTest,JSONRecordReaderTest,JSONMessageDecoderTest,JSONRecordExtractorTest test`
- `./mvnw -Dmaven.repo.local=/tmp/m2-pinot-12312 -pl pinot-spi,pinot-plugins/pinot-input-format/pinot-json spotless:apply checkstyle:check license:check`
- `git diff --check`
The tests cover BigDecimal parsing from strings and byte slices, JSON record reader precision preservation, default message-decoder precision preservation, built-in extractor behavior, legacy custom extractor behavior, and explicit custom extractor opt-in.
---
Drafted-by: Codex (GPT-5) (no human review before posting)
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on PR #18976:
URL: https://github.com/apache/pinot/pull/18976#issuecomment-5249500598
Pushed `7388fdf`. The Unit Test Set 2 failure was real and caused by this change: `JSONRecordReader` now returns `BigDecimal("3.0")` for floating JSON literals, where `PinotSegmentConverterTest.testJsonConverter` asserted a `Double`. That is why the message read "expected [3.0] but found [3.0]", same string, different type.
The float and double assertions compare numerically now, element-wise for the multi-value cases, which documents the new type-agnostic contract rather than hiding the behavior change. INT and LONG assertions are untouched since integral literals are unaffected. Rebased onto latest master, and the reader, extractor and JsonUtils suites are green locally.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on PR #18976:
URL: https://github.com/apache/pinot/pull/18976#issuecomment-5186565159
@swaminathanmanish @krishan1390 Please help review this change
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on PR #18976:
URL: https://github.com/apache/pinot/pull/18976#issuecomment-4949200047
Review request: @snleee @swaminathanmanish
GitHub did not allow this account/integration to add requested reviewers through the reviewer section, so I am tagging the relevant reviewers here for visibility.
This PR fixes #12312 by preserving JSON decimal values as `BigDecimal` through Pinot's built-in JSON ingestion path, while keeping custom `recordExtractorClass` implementations on the legacy `Double` parsing behavior unless they opt in with `preserveDecimalPrecision=true`. Review from the ingestion/Kafka-decoder side would be especially useful to confirm the compatibility boundary and whether the opt-in behavior is appropriate for custom extractors.
---
Drafted-by: Codex (GPT-5); human-reviewed by Vamsi-klu before posting
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #18976:
URL: https://github.com/apache/pinot/pull/18976#issuecomment-5200258022
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/18976?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `28.57143%` with `10 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 57.11%. Comparing base ([`cd2538d`](https://app.codecov.io/gh/apache/pinot/commit/cd2538d4b7bd0d54c722173db66c0ccaed646ee2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`680075e`](https://app.codecov.io/gh/apache/pinot/commit/680075eefd21f465dd0dbea581dcf6cbb53274d9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/18976?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...ot/plugin/inputformat/json/JSONMessageDecoder.java](https://app.codecov.io/gh/apache/pinot/pull/18976?src=pr&el=tree&filepath=pinot-plugins%2Fpinot-input-format%2Fpinot-json%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fplugin%2Finputformat%2Fjson%2FJSONMessageDecoder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtcGx1Z2lucy9waW5vdC1pbnB1dC1mb3JtYXQvcGlub3QtanNvbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvcGx1Z2luL2lucHV0Zm9ybWF0L2pzb24vSlNPTk1lc3NhZ2VEZWNvZGVyLmphdmE=) | 0.00% | [10 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/18976?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (cd2538d) and HEAD (680075e). Click for more details.
>
> <details><summary>HEAD has 16 uploads less than BASE</summary>
>
>| Flag | BASE (cd2538d) | HEAD (680075e) |
>|------|------|------|
>|unittests|2|1|
>|java-25|5|1|
>|temurin|5|1|
>|unittests2|1|0|
>|integration|3|0|
>|integration1|1|0|
>|custom-integration1|1|0|
>|integration2|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #18976 +/- ##
============================================
- Coverage 66.59% 57.11% -9.49%
+ Complexity 1423 1 -1422
============================================
Files 3443 2646 -797
Lines 218536 158894 -59642
Branches 34780 26115 -8665
============================================
- Hits 145538 90752 -54786
+ Misses 61273 60361 -912
+ Partials 11725 7781 -3944
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.11% <28.57%> (-9.49%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.11% <28.57%> (-9.49%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.11% <28.57%> (-9.48%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.11% <28.57%> (-0.01%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/18976/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/18976?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r3804055004
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -71,13 +74,24 @@ public void init(Map<String, String> props, Set<String> fieldsToRead, String top
if (recordExtractorClass == null) {
recordExtractorClass = JSON_RECORD_EXTRACTOR_CLASS;
}
+ String preserveDecimalPrecision = null;
+ if (props != null) {
+ preserveDecimalPrecision = props.get(PRESERVE_DECIMAL_PRECISION_CONFIG_KEY);
+ }
+
_jsonRecordExtractor = PluginManager.get().createInstance(recordExtractorClass);
_jsonRecordExtractor.init(fieldsToRead, null);
_fieldsToRead = CollectionUtils.isNotEmpty(fieldsToRead) ? Set.copyOf(fieldsToRead) : null;
// Direct parsing implements JSONRecordExtractor's conversion contract and bypasses extract(). Require the
// exact default class so a configured extractor or subclass cannot lose custom extraction behavior.
_usesDefaultRecordExtractor = _jsonRecordExtractor.getClass() == JSONRecordExtractor.class;
- _parser = JsonPayloadFormat.fromConfig(jsonFormat).getParser();
+ JsonPayloadFormat format = JsonPayloadFormat.fromConfig(jsonFormat);
+ _parser = format.getParser();
+ // BigDecimal-preserving parsing goes through Pinot's BigDecimal-aware text JSON reader, so it applies only
+ // to the TEXT format (the historical default); the binary formats encode floating point natively.
+ _preserveDecimalPrecision = format == JsonPayloadFormat.TEXT && (preserveDecimalPrecision != null
Review Comment:
is this applicable only to json extractor and not to other extractors like avro/proto etc. I guess the text gets rounded off to double.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -52,12 +53,14 @@ public class JSONMessageDecoder implements StreamMessageDecoder<byte[]> {
private static final String JSON_RECORD_EXTRACTOR_CLASS =
"org.apache.pinot.plugin.inputformat.json.JSONRecordExtractor";
+ private static final String PRESERVE_DECIMAL_PRECISION_CONFIG_KEY = "preserveDecimalPrecision";
Review Comment:
We are doing opt-in only for backwards compatibility ?
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r3755548345
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -46,6 +48,14 @@ public void init(Map<String, String> props, Set<String> fieldsToRead, String top
if (recordExtractorClass == null) {
Review Comment:
This one is pre-existing on master rather than something this PR introduces: `JSONMessageDecoder.init` has always called `_jsonRecordExtractor.init(fieldsToRead, null)`, and wiring `RECORD_EXTRACTOR_CONFIG_CONFIG_KEY` through the way `SimpleAvroMessageDecoder` does is orthogonal to BigDecimal precision. I would rather keep this PR scoped and handle it as a follow-up.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r3717366003
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -46,6 +48,14 @@ public void init(Map<String, String> props, Set<String> fieldsToRead, String top
if (recordExtractorClass == null) {
Review Comment:
JSONMessageDecoder.init() currently always calls _jsonRecordExtractor.init(fieldsToRead, null), ignoring StreamMessageDecoder.RECORD_EXTRACTOR_CONFIG_CONFIG_KEY (recordExtractorConfigClass). Other decoders (e.g., Avro) honor this and pass a RecordExtractorConfig initialized from props; leaving it null here prevents configuring custom JSON extractors and makes decoder behavior inconsistent across formats.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "swaminathanmanish (via GitHub)" <gi...@apache.org>.
swaminathanmanish commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r3804060391
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -52,12 +53,14 @@ public class JSONMessageDecoder implements StreamMessageDecoder<byte[]> {
private static final String JSON_RECORD_EXTRACTOR_CLASS =
"org.apache.pinot.plugin.inputformat.json.JSONRecordExtractor";
+ private static final String PRESERVE_DECIMAL_PRECISION_CONFIG_KEY = "preserveDecimalPrecision";
Review Comment:
We are doing opt-in only for backwards compatibility ? This is the correct behavior right. Asking because this is yet another flag to remember.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Always preserve JSON decimal text as BigDecimal on ingest [pinot]
Posted by "Jackie-Jiang (via GitHub)" <gi...@apache.org>.
Jackie-Jiang commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r4383602578
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/format/JacksonPayloadParser.java:
##########
@@ -109,17 +115,24 @@ public boolean parse(byte[] payload, int offset, int length, GenericRow destinat
/// Materializes the value at the parser's current token in [JSONRecordExtractor]'s converted shape. Scalars
/// are read straight off the parser rather than through databind, which allocates a fresh
/// `DeserializationContext` per `readValue` call. `getNumberValue()` keeps the binary formats' `Float`
- /// (never upcast to `Double`) and text JSON's `Double`, matching Jackson's untyped materialization.
+ /// (never upcast to `Double`). Text-backed formats use `getDecimalValue()` for floats so decimal literals
+ /// are not rounded to `Double` before Pinot's schema conversion.
@Nullable
private Object readValue(JsonParser parser, JsonToken valueToken)
throws IOException {
switch (valueToken) {
case VALUE_STRING:
return parser.getText();
case VALUE_NUMBER_INT:
- case VALUE_NUMBER_FLOAT:
// Oversized ints widen to BigDecimal via the shared contract (Pinot has no BigInteger type)
return JSONRecordExtractor.convert(parser.getNumberValue());
+ case VALUE_NUMBER_FLOAT:
+ // JSON text stores decimals as decimal *text*. Jackson's default getNumberValue() rounds that text
+ // to Double. Avro and Protobuf extractors do not have this problem: they carry typed numeric
+ // values (Avro's decimal logical type is already BigDecimal). Binary JSON formats encode IEEE
+ // floats natively, so they keep getNumberValue().
+ return JSONRecordExtractor.convert(
+ _preserveDecimalFloats ? parser.getDecimalValue() : parser.getNumberValue());
Review Comment:
[P1] Preserve false when converting decimal zero to BOOLEAN
For a BOOLEAN schema column, `{"flag":0.0}` previously decoded to `Double` and converted to false. This now produces `BigDecimal("0.0")`, but `PinotDataType.BIG_DECIMAL.toBoolean()` uses `!value.equals(BigDecimal.ZERO)`. Because `BigDecimal.equals` compares scale, the value is silently stored as true. Batch ingestion is affected as well.
Please change the BigDecimal boolean conversion to numerical zero comparison (`signum() != 0` or `compareTo(BigDecimal.ZERO) != 0`) alongside this change, and cover `0.0`, `-0.0`, and exponent-form zero through decoding and schema conversion.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONRecordReader.java:
##########
@@ -49,7 +49,7 @@ private void init()
throws IOException {
_inputStream = RecordReaderUtils.getBufferedInputStream(_dataFile);
try {
- _iterator = JsonUtils.DEFAULT_READER.forType(new TypeReference<Map<String, Object>>() {
+ _iterator = JsonUtils.READER_WITH_BIG_DECIMAL.forType(new TypeReference<Map<String, Object>>() {
}).readValues(_inputStream);
Review Comment:
[P1] Preserve mixed numeric arrays when converting to MV STRING
An input such as `{"v":[1.5,2]}` previously ingested into a multi-value STRING column as `["1.5","2"]`. With this reader, the array contains a BigDecimal followed by an Integer. `DataTypeTransformerUtils.transformValue()` chooses `BIG_DECIMAL_ARRAY` from the first element, and `PinotDataType.toStringArray()` applies `BIG_DECIMAL.toString()` to every element. The cast of the Integer second element to BigDecimal throws `ClassCastException`, rejecting the row or nulling the column depending on error handling. The previous `DOUBLE.toString()` simply called `value.toString()`, so this input worked. Streaming ingestion has the same regression.
Please dispatch STRING-array conversion using each element's actual type, preserving `toPlainString()` for BigDecimal elements, and add mixed-array regression coverage.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/format/TextJsonPayloadParser.java:
##########
@@ -22,11 +22,12 @@
/// Parses UTF-8 text JSON, the historical [org.apache.pinot.plugin.inputformat.json.JSONMessageDecoder]
-/// behavior. Uses the same Jackson map/value materialization contract as the binary JSON parsers.
+/// format. Floating literals are `BigDecimal` so high-precision decimals survive ingestion; this is
+/// always on because it is the correct behavior for decimal text, not an opt-in.
class TextJsonPayloadParser extends JacksonPayloadParser {
TextJsonPayloadParser() {
- super(JsonUtils.DEFAULT_READER);
+ super(JsonUtils.READER_WITH_BIG_DECIMAL);
Review Comment:
[P1] Keep built-in nullIf transforms matching decimal values
For JSON `{"metric":1.5}`, an ingestion transform `nullIf(metric, 1.5)` previously returned null. The source metric now becomes BigDecimal while the expression literal remains Double. `FunctionInvoker` leaves the Object arguments unchanged, and `ObjectFunctions.nullIf()` compares them using `equals`, so it returns 1.5 instead of null. Expression transforms run before schema conversion, so a DOUBLE target column does not prevent this regression. Comparing source fields containing `1.5` and `1.50` also stops returning null because BigDecimal equality includes scale.
Please make built-in `nullIf` compare numeric operands by value and add ingestion-transform coverage before changing the default decoded type.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r3994749955
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -71,13 +74,24 @@ public void init(Map<String, String> props, Set<String> fieldsToRead, String top
if (recordExtractorClass == null) {
recordExtractorClass = JSON_RECORD_EXTRACTOR_CLASS;
}
+ String preserveDecimalPrecision = null;
+ if (props != null) {
+ preserveDecimalPrecision = props.get(PRESERVE_DECIMAL_PRECISION_CONFIG_KEY);
+ }
+
_jsonRecordExtractor = PluginManager.get().createInstance(recordExtractorClass);
_jsonRecordExtractor.init(fieldsToRead, null);
_fieldsToRead = CollectionUtils.isNotEmpty(fieldsToRead) ? Set.copyOf(fieldsToRead) : null;
// Direct parsing implements JSONRecordExtractor's conversion contract and bypasses extract(). Require the
// exact default class so a configured extractor or subclass cannot lose custom extraction behavior.
_usesDefaultRecordExtractor = _jsonRecordExtractor.getClass() == JSONRecordExtractor.class;
- _parser = JsonPayloadFormat.fromConfig(jsonFormat).getParser();
+ JsonPayloadFormat format = JsonPayloadFormat.fromConfig(jsonFormat);
+ _parser = format.getParser();
+ // BigDecimal-preserving parsing goes through Pinot's BigDecimal-aware text JSON reader, so it applies only
+ // to the TEXT format (the historical default); the binary formats encode floating point natively.
+ _preserveDecimalPrecision = format == JsonPayloadFormat.TEXT && (preserveDecimalPrecision != null
Review Comment:
JSON-only (plus PostgreSQL jsonb, whose body is text JSON). Avro and Protobuf already deliver schema-typed numbers. Avro decimal / big-decimal is already BigDecimal. Proto double is IEEE. CSV is untyped strings. ORC/Parquet/Arrow/BSON already convert typed DECIMAL to BigDecimal. Smile/CBOR keep native floats. We did not change those readers.
The rounding you mentioned is Jackson treating JSON number tokens as Double. That does not exist on Avro/proto.
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -52,12 +53,14 @@ public class JSONMessageDecoder implements StreamMessageDecoder<byte[]> {
private static final String JSON_RECORD_EXTRACTOR_CLASS =
"org.apache.pinot.plugin.inputformat.json.JSONRecordExtractor";
+ private static final String PRESERVE_DECIMAL_PRECISION_CONFIG_KEY = "preserveDecimalPrecision";
Review Comment:
Yes. The flag was only a compatibility guard, and preserving decimal text is the correct default. We removed preserveDecimalPrecision so operators do not have another knob. Built-in and custom extractors now both see BigDecimal for floating JSON literals until DataTypeTransformer. Typed DOUBLE / FLOAT / BIG_DECIMAL columns still convert via PinotDataType.
The real upgrade breaks are custom extractors / Groovy that assume Double, and STRING columns (toPlainString(), so 1.23e10 becomes 12300000000). Existing DOUBLE segments are unchanged. Happy to add a short-lived opt-out if you want one. Otherwise I would rather keep the JSON plugin README upgrade note than keep the flag.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Preserve BigDecimal precision during JSON ingestion [pinot]
Posted by "Vamsi-klu (via GitHub)" <gi...@apache.org>.
Vamsi-klu commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r3725704764
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONMessageDecoder.java:
##########
@@ -46,6 +48,14 @@ public void init(Map<String, String> props, Set<String> fieldsToRead, String top
if (recordExtractorClass == null) {
Review Comment:
The observation is accurate, but this behavior is pre-existing rather than introduced here. The _jsonRecordExtractor.init(fieldsToRead, null) call is unchanged from master; this PR's changes to init() only add the preserveDecimalPrecision handling. Wiring up recordExtractorConfigClass would mean instantiating the config class and initializing it from props the way SimpleAvroMessageDecoder does, which is orthogonal to BigDecimal precision. I would prefer to keep this PR focused and address the extractor config support in a separate follow-up PR if the maintainers want it.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] Always preserve JSON decimal text as BigDecimal on ingest [pinot]
Posted by "Jackie-Jiang (via GitHub) via commits" <co...@pinot.apache.org>.
Jackie-Jiang commented on code in PR #18976:
URL: https://github.com/apache/pinot/pull/18976#discussion_r4498969993
##########
pinot-plugins/pinot-input-format/pinot-json/src/main/java/org/apache/pinot/plugin/inputformat/json/JSONRecordReader.java:
##########
@@ -49,7 +49,7 @@ private void init()
throws IOException {
_inputStream = RecordReaderUtils.getBufferedInputStream(_dataFile);
try {
- _iterator = JsonUtils.DEFAULT_READER.forType(new TypeReference<Map<String, Object>>() {
+ _iterator = JsonUtils.READER_WITH_BIG_DECIMAL.forType(new TypeReference<Map<String, Object>>() {
}).readValues(_inputStream);
Review Comment:
[P1] Handle mixed numeric arrays when converting to MV BIG_DECIMAL
Batch ingestion of `{"v":[1.5,2]}` into an MV BIG_DECIMAL column previously succeeded, but now throws `ClassCastException`. The decoded array contains a BigDecimal followed by an Integer. `DataTypeTransformerUtils.transformValue()` infers `BIG_DECIMAL_ARRAY` from the first element, and `PinotDataType.toBigDecimalArray()` applies `BIG_DECIMAL.toBigDecimal()` to every element. Its direct cast fails on the Integer second element. Before this PR, the inferred source was `DOUBLE_ARRAY`, whose `toBigDecimal()` uses `new BigDecimal(value.toString())` and accepts both numbers.
MV BIG_DECIMAL is a supported offline ingestion path, including dictionary and raw columns. Depending on error handling, this change fails ingestion or nulls the column.
Please add per-element source-type dispatch or fallback to `toBigDecimalArray()`, analogous to the STRING fix, and regression coverage for converting `{BigDecimal("1.5"), 2}` to `BIG_DECIMAL_ARRAY`.
--
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: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org