Jackie-Jiang commented on code in PR #18972: URL: https://github.com/apache/pinot/pull/18972#discussion_r3609431112
########## pinot-common/src/main/java/org/apache/pinot/common/function/StreamingJsonPathExtractor.java: ########## @@ -0,0 +1,326 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.common.function; + +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Preconditions; +import com.jayway.jsonpath.InvalidJsonException; +import java.io.IOException; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; + + +/// Resolves one or more [SimpleJsonPath]s against a JSON document in a single forward pass of a Jackson +/// [JsonParser], materializing only the addressed values. Subtrees that no path descends into are consumed +/// with `skipChildren()`, so no intermediate `Map` / `List` is ever built for them. This replaces +/// `JsonPath.parse(json).read(path)`, which builds a full Jackson DOM of the document and only then walks to +/// the field. +/// +/// **Semantics.** The results are byte-for-byte identical to Jayway configured as Pinot configures it +/// (`JacksonJsonProvider` + `JacksonMappingProvider` + `Option.SUPPRESS_EXCEPTIONS`), including: +/// - a missing key, an out-of-range index, indexing a non-array, descending into a scalar, and an explicit +/// JSON `null` all resolve to `null`; +/// - duplicate object keys resolve to the **last** occurrence, and a later occurrence discards whatever an +/// earlier one resolved to, at every depth; +/// - a container leaf is returned as a `LinkedHashMap` / `ArrayList` with the same element types; +/// - numbers become `Integer` / `Long` / `BigInteger` by magnitude, and `Double` (or `BigDecimal` when +/// `useBigDecimal` is set) for floating-point literals; +/// - a document that is malformed anywhere inside its root value raises [InvalidJsonException], even when the +/// addressed field was already seen; content *after* the root value is ignored, exactly as +/// `ObjectMapper.readValue` ignores it (`FAIL_ON_TRAILING_TOKENS` is off by default). +/// +/// **The one place it deliberately differs.** Jayway builds a DOM of the *whole* document, so it materializes +/// and validates every value in it - including fields the path never addresses. This extractor skips those +/// subtrees, so a value that Jackson can lex but cannot *materialize* raises in Jayway and not here. Two such +/// values exist, and both only matter when they sit **outside** the addressed path: +/// 1. a string longer than `StreamReadConstraints` `maxStringLength` (20,000,000 chars); +/// 2. when `useBigDecimal` is set, a float literal whose exponent overflows an `int` (e.g. `1e999999999999`), +/// which `BigDecimal` cannot represent. +/// Jayway rejects such a document outright; this extractor returns the requested value. When the offending +/// value *is* the addressed one, both raise, so the results still agree. +/// +/// This is inherent to the optimization - not materializing what was not asked for is precisely what makes it +/// fast, and matching Jayway here would mean materializing every string in every document. The extractor never +/// returns a *different value* for the addressed path; it only declines to fail on values the caller did not +/// ask about, and it is the safer of the two (it never allocates the 20 MB string or the pathological +/// `BigDecimal`). +/// +/// **Early exit.** When the caller passes `earlyExit`, the pass stops as soon as every requested path has +/// resolved to a non-null value. That is much faster when the target fields appear early in the document, at +/// the cost of two behavior changes, since the tail is never read: +/// 1. duplicate keys resolve to the **first** occurrence rather than the last; +/// 2. a document malformed strictly after the last addressed field yields the extracted value instead of +/// raising [InvalidJsonException]. +/// Both inputs are undefined (RFC 8259) or corrupt, and the switch is off by default. +/// +/// **Applicability.** Callers must first check [#canExtract]: the document's first non-whitespace character +/// has to be `{` or `[`. Bare scalar documents, `null`, empty / whitespace-only input, a leading byte-order +/// mark and plain text all take the Jayway path, which reproduces its exact exceptions for them. +/// +/// Stateless and thread-safe; the shared [JsonFactory] and [ObjectMapper]s are safe for concurrent use. +public final class StreamingJsonPathExtractor { + /// The live-path set is tracked as a bitmask, so a single call cannot resolve more paths than a `long` has + /// bits. Callers with more paths must batch them. + public static final int MAX_PATHS = Long.SIZE; + + /// Deliberately the plain, unconfigured factory and mappers that Jayway's `JacksonJsonProvider` builds, so + /// that stream-read constraints (nesting depth, string length) and number handling match exactly. + private static final JsonFactory FACTORY = new JsonFactory(); + private static final ObjectMapper MAPPER = new ObjectMapper(); + private static final ObjectMapper MAPPER_WITH_BIG_DECIMAL = + new ObjectMapper().configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true); + + private StreamingJsonPathExtractor() { Review Comment: (nit) Put this next to the declaration ########## pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java: ########## @@ -105,6 +107,31 @@ public static Object jsonPath(Object object, String jsonPath) { return PARSE_CONTEXT.parse(object).read(jsonPath, NO_PREDICATES); } + /// Resolves `jsonPath` against `object` using the streaming extractor ([StreamingJsonPathExtractor]) when + /// `object` is a JSON `String` and the path is a simple linear chain, and falls back to the Jayway + /// [#jsonPath] implementation otherwise, so a caller sees the same value and the same exceptions either way. + /// Backs the opt-in streaming scalar functions below. See [StreamingJsonPathExtractor] for the one edge case + /// where the streaming and Jayway results differ. + @Nullable + private static Object jsonPathStreaming(Object object, String jsonPath, boolean useBigDecimal, boolean earlyExit) { Review Comment: `Streaming` might not be the right term. `Fast` feels better. Same for other places ########## pinot-common/src/main/java/org/apache/pinot/common/function/StreamingJsonPathExtractor.java: ########## @@ -0,0 +1,326 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.common.function; + +import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonParser; +import com.fasterxml.jackson.core.JsonToken; +import com.fasterxml.jackson.databind.DeserializationFeature; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.base.Preconditions; +import com.jayway.jsonpath.InvalidJsonException; +import java.io.IOException; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; + + +/// Resolves one or more [SimpleJsonPath]s against a JSON document in a single forward pass of a Jackson +/// [JsonParser], materializing only the addressed values. Subtrees that no path descends into are consumed +/// with `skipChildren()`, so no intermediate `Map` / `List` is ever built for them. This replaces +/// `JsonPath.parse(json).read(path)`, which builds a full Jackson DOM of the document and only then walks to +/// the field. +/// +/// **Semantics.** The results are byte-for-byte identical to Jayway configured as Pinot configures it +/// (`JacksonJsonProvider` + `JacksonMappingProvider` + `Option.SUPPRESS_EXCEPTIONS`), including: +/// - a missing key, an out-of-range index, indexing a non-array, descending into a scalar, and an explicit +/// JSON `null` all resolve to `null`; +/// - duplicate object keys resolve to the **last** occurrence, and a later occurrence discards whatever an +/// earlier one resolved to, at every depth; +/// - a container leaf is returned as a `LinkedHashMap` / `ArrayList` with the same element types; +/// - numbers become `Integer` / `Long` / `BigInteger` by magnitude, and `Double` (or `BigDecimal` when +/// `useBigDecimal` is set) for floating-point literals; +/// - a document that is malformed anywhere inside its root value raises [InvalidJsonException], even when the +/// addressed field was already seen; content *after* the root value is ignored, exactly as +/// `ObjectMapper.readValue` ignores it (`FAIL_ON_TRAILING_TOKENS` is off by default). +/// +/// **The one place it deliberately differs.** Jayway builds a DOM of the *whole* document, so it materializes +/// and validates every value in it - including fields the path never addresses. This extractor skips those +/// subtrees, so a value that Jackson can lex but cannot *materialize* raises in Jayway and not here. Two such +/// values exist, and both only matter when they sit **outside** the addressed path: +/// 1. a string longer than `StreamReadConstraints` `maxStringLength` (20,000,000 chars); +/// 2. when `useBigDecimal` is set, a float literal whose exponent overflows an `int` (e.g. `1e999999999999`), +/// which `BigDecimal` cannot represent. +/// Jayway rejects such a document outright; this extractor returns the requested value. When the offending +/// value *is* the addressed one, both raise, so the results still agree. +/// +/// This is inherent to the optimization - not materializing what was not asked for is precisely what makes it +/// fast, and matching Jayway here would mean materializing every string in every document. The extractor never +/// returns a *different value* for the addressed path; it only declines to fail on values the caller did not +/// ask about, and it is the safer of the two (it never allocates the 20 MB string or the pathological +/// `BigDecimal`). +/// +/// **Early exit.** When the caller passes `earlyExit`, the pass stops as soon as every requested path has +/// resolved to a non-null value. That is much faster when the target fields appear early in the document, at +/// the cost of two behavior changes, since the tail is never read: +/// 1. duplicate keys resolve to the **first** occurrence rather than the last; +/// 2. a document malformed strictly after the last addressed field yields the extracted value instead of +/// raising [InvalidJsonException]. +/// Both inputs are undefined (RFC 8259) or corrupt, and the switch is off by default. +/// +/// **Applicability.** Callers must first check [#canExtract]: the document's first non-whitespace character +/// has to be `{` or `[`. Bare scalar documents, `null`, empty / whitespace-only input, a leading byte-order +/// mark and plain text all take the Jayway path, which reproduces its exact exceptions for them. +/// +/// Stateless and thread-safe; the shared [JsonFactory] and [ObjectMapper]s are safe for concurrent use. +public final class StreamingJsonPathExtractor { Review Comment: Streaming might not be the right term as regular json parser can also stream. Probably `FastJsonPathExtractor`? -- 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]
