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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
