xiangfu0 commented on code in PR #18756:
URL: https://github.com/apache/pinot/pull/18756#discussion_r4213811356
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -783,6 +783,115 @@ public static List<Map<String, String>> flatten(String
jsonString, JsonIndexConf
}
}
+ /// Flattens an already-parsed JSON value ({@link Map} / {@link List} /
{@link JsonNode}) for the JSON index, avoiding
+ /// the string tokenization that {@link #flatten(String, JsonIndexConfig)}
performs. Used by the realtime JSON index
+ /// when the source value is already a parsed object (e.g. cached on the
`GenericRow` before it is serialized to a
+ /// string for the forward index), so the document is parsed once at
ingestion instead of being serialized and
+ /// re-parsed here. The result must match {@link #flatten(String,
JsonIndexConfig)} on the serialized form (both go
+ /// through `DEFAULT_MAPPER`). A {@link String} input is delegated to {@link
#flatten(String, JsonIndexConfig)}.
+ public static List<Map<String, String>> flattenParsed(Object jsonValue,
JsonIndexConfig jsonIndexConfig)
+ throws IOException {
+ if (jsonValue instanceof String) {
+ return flatten((String) jsonValue, jsonIndexConfig);
+ }
+ JsonNode jsonNode = jsonValue instanceof JsonNode ? (JsonNode) jsonValue :
DEFAULT_MAPPER.valueToTree(jsonValue);
Review Comment:
Fixed in 136c53b7b6. The private parsed-value mapper preserves BigDecimal
token scale before leaf normalization; isolated 2.0 and large scale-zero
regressions now match the actual forward string and immutable-index results.
JsonUtilsTest and JsonIndexTest cover both without Float/binary masking.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/DataTypeTransformer.java:
##########
@@ -131,8 +167,39 @@ public void transform(GenericRow record) {
&& value instanceof CharSequence) {
validateCanonicalUuidPrimaryKey(column, value.toString());
}
- value = DataTypeTransformerUtils.transformValue(column, value,
entry.getValue());
+ // For a JSON column with a JSON index, cache the parsed value so the
index flattens it directly instead of
+ // re-parsing the string the value is serialized into for the forward
index. The parsed value is cached AFTER
+ // the forward value is written (below), because putValue invalidates
the parsed-JSON cache: a later transformer
+ // that rewrites the value (e.g. sanitization trimming an over-length
JSON string) must not leave a stale node.
+ Object parsedToCache = null;
+ if (value != null && !_jsonCacheColumns.isEmpty() &&
_jsonCacheColumns.contains(column)) {
+ if (value instanceof Map || value instanceof List || value
instanceof JsonNode) {
+ // Already-parsed JSON: serialize it for the forward index (below)
and cache the parsed value for the index.
+ parsedToCache = value;
Review Comment:
Fixed in 136c53b7b6. Cached Map conversion now sorts Map keys recursively,
matching MapUtils.toString; List/JsonNode inputs retain their existing
serialization order. The dotted-key collision regressions compare the
transformed forward value and real immutable JSON index with the cached mutable
path.
##########
pinot-spi/src/main/java/org/apache/pinot/spi/utils/JsonUtils.java:
##########
@@ -783,6 +783,115 @@ public static List<Map<String, String>> flatten(String
jsonString, JsonIndexConf
}
}
+ /// Flattens an already-parsed JSON value ({@link Map} / {@link List} /
{@link JsonNode}) for the JSON index, avoiding
+ /// the string tokenization that {@link #flatten(String, JsonIndexConfig)}
performs. Used by the realtime JSON index
+ /// when the source value is already a parsed object (e.g. cached on the
`GenericRow` before it is serialized to a
+ /// string for the forward index), so the document is parsed once at
ingestion instead of being serialized and
+ /// re-parsed here. The result must match {@link #flatten(String,
JsonIndexConfig)} on the serialized form (both go
+ /// through `DEFAULT_MAPPER`). A {@link String} input is delegated to {@link
#flatten(String, JsonIndexConfig)}.
+ public static List<Map<String, String>> flattenParsed(Object jsonValue,
JsonIndexConfig jsonIndexConfig)
+ throws IOException {
+ if (jsonValue instanceof String) {
+ return flatten((String) jsonValue, jsonIndexConfig);
+ }
+ JsonNode jsonNode = jsonValue instanceof JsonNode ? (JsonNode) jsonValue :
DEFAULT_MAPPER.valueToTree(jsonValue);
+ int classification = classifyForFlatten(jsonNode);
+ if (classification == FLATTEN_UNSAFE) {
+ // A leaf renders differently than the string path (e.g. a Float or
byte[] from a non-JSON RecordReader). Fall
+ // back to serialize+reparse so the flattened records are byte-for-byte
identical. Rare: the JSON decoders never
+ // produce these types.
+ return flatten(objectToString(jsonValue), jsonIndexConfig);
Review Comment:
Fixed in 136c53b7b6. Float/binary values decline caching before tree
conversion, so indexing reuses the already-produced forward string. The
Avro-style benchmark measures both preflight and fallback: the short JDK25 GC
run reduced allocation from 22,688 to 17,952 B/op (20.9%); timing was noisy, so
I am making no throughput claim.
--
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]
