Jackie-Jiang commented on code in PR #18972:
URL: https://github.com/apache/pinot/pull/18972#discussion_r3589962854
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java:
##########
@@ -100,11 +103,26 @@ public static String jsonFormat(Object object)
@ScalarFunction
public static Object jsonPath(Object object, String jsonPath) {
if (object instanceof String) {
- return PARSE_CONTEXT.parse((String) object).read(jsonPath,
NO_PREDICATES);
+ String json = (String) object;
+ SimpleJsonPath simpleJsonPath = simpleJsonPath(jsonPath);
Review Comment:
Could this ever throw exception? Should we do a try catch and fall back?
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java:
##########
@@ -140,13 +158,12 @@ private static boolean canExtractJsonPath(@Nullable
Object object) {
/**
* Extract object array based on Json path
*/
+ // Routed through jsonPath() rather than PARSE_CONTEXT directly: the two are
equivalent, and this way the
Review Comment:
(minor) Fix the javadoc
##########
pinot-common/src/main/java/org/apache/pinot/common/function/JsonPathFastPathMode.java:
##########
@@ -0,0 +1,60 @@
+/**
+ * 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 org.apache.pinot.spi.utils.CommonConstants;
+
+
+/// Global runtime switches for the streaming JsonPath fast path (see
[StreamingJsonPathExtractor]).
+///
+/// Both switches default to `false` and are seeded from the corresponding JVM
system property, so tests and
+/// JMH benchmarks can drive them without a running server. Server / broker /
controller / minion startup
+/// overrides them from cluster config via
`ServiceStartableUtils.applyClusterConfig`.
+///
+/// - [#isEnabled()] gates the fast path entirely. When off, every extraction
goes through Jayway.
+/// - [#isEarlyExitEnabled()] additionally lets the extractor stop at the leaf
instead of reading the whole
+/// root value. It trades two documented behavior changes for a large
speedup on documents whose target
+/// field appears early; see [StreamingJsonPathExtractor] for the exact
divergences. It has no effect
+/// unless [#isEnabled()] is also true.
+///
+/// Thread-safe: both fields are `volatile` and only ever flipped at startup
(or from a test).
+public class JsonPathFastPathMode {
+ private static volatile boolean _enabled =
Boolean.getBoolean(CommonConstants.CONFIG_OF_JSONPATH_FASTPATH_ENABLED);
+ private static volatile boolean _earlyExitEnabled =
+
Boolean.getBoolean(CommonConstants.CONFIG_OF_JSONPATH_FASTPATH_EARLY_EXIT_ENABLED);
+
+ private JsonPathFastPathMode() {
Review Comment:
(nit) Put private constructor next to class declaration for util classes
--
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]
