Upgrade to JsonPath 2.7.0

LICENSE-binary

@@ -241,7 +241,7 @@ com.google.http-client:google-http-client-jackson2:1.32.1
 com.google.http-client:google-http-client:1.32.1
 com.google.j2objc:j2objc-annotations:1.3
 com.google.oauth-client:google-oauth-client:1.30.3
-com.jayway.jsonpath:json-path:2.4.0
+com.jayway.jsonpath:json-path:2.7.0
 com.lmax:disruptor:3.3.4
 com.ning:async-http-client:1.9.21
 com.squareup.okio:okio:1.6.0

pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java

@@ -22,6 +22,7 @@
 import com.google.common.annotations.VisibleForTesting;
 import com.jayway.jsonpath.Configuration;
 import com.jayway.jsonpath.JsonPath;
+import com.jayway.jsonpath.Option;
 import com.jayway.jsonpath.ParseContext;
 import com.jayway.jsonpath.Predicate;
 import com.jayway.jsonpath.spi.cache.CacheProvider;
@@ -50,10 +51,12 @@ public class JsonFunctions {
   private JsonFunctions() {
   }
 
+  private static final Object[] EMPTY = new Object[0];
   private static final Predicate[] NO_PREDICATES = new Predicate[0];
   private static final ParseContext PARSE_CONTEXT = JsonPath.using(
       new Configuration.ConfigurationBuilder().jsonProvider(new ArrayAwareJacksonJsonProvider())
-          .mappingProvider(new JacksonMappingProvider()).build());
+          .mappingProvider(new JacksonMappingProvider()).options(Option.SUPPRESS_EXCEPTIONS)
+          .build());
 
   static {
     // Set the JsonPath cache before the cache is accessed
@@ -93,8 +96,7 @@ public static Object jsonPath(Object object, String jsonPath) {
    * Extract object array based on Json path
    */
   @ScalarFunction
-  public static Object[] jsonPathArray(Object object, String jsonPath)
-      throws JsonProcessingException {
+  public static Object[] jsonPathArray(Object object, String jsonPath) {
     if (object instanceof String) {
       return convertObjectToArray(PARSE_CONTEXT.parse((String) object).read(jsonPath, NO_PREDICATES));
     }
@@ -103,18 +105,17 @@ public static Object[] jsonPathArray(Object object, String jsonPath)
 
   @ScalarFunction
   public static Object[] jsonPathArrayDefaultEmpty(Object object, String jsonPath) {
-    try {
-      return jsonPathArray(object, jsonPath);
-    } catch (Exception e) {
-      return new Object[0];
-    }
+    Object[] result = jsonPathArray(object, jsonPath);
+    return result == null ? EMPTY : result;
   }
 
   private static Object[] convertObjectToArray(Object arrayObject) {
     if (arrayObject instanceof List) {
       return ((List) arrayObject).toArray();
     } else if (arrayObject instanceof Object[]) {
       return (Object[]) arrayObject;
+    } else if (arrayObject == null) {
+      return null;
     }
     return new Object[]{arrayObject};
   }
@@ -129,17 +130,21 @@ public static String jsonPathString(Object object, String jsonPath)
     if (jsonValue instanceof String) {
       return (String) jsonValue;
     }
-    return JsonUtils.objectToString(jsonValue);
+    return jsonValue == null ? null : JsonUtils.objectToString(jsonValue);
   }
 
   /**
    * Extract from Json with path to String
    */
   @ScalarFunction
   public static String jsonPathString(Object object, String jsonPath, String defaultValue) {
+    Object jsonValue = jsonPath(object, jsonPath);
+    if (jsonValue instanceof String) {
+      return (String) jsonValue;
+    }
     try {
-      return jsonPathString(object, jsonPath);
-    } catch (Exception e) {
+      return jsonValue == null ? defaultValue : JsonUtils.objectToString(jsonValue);
+    } catch (JsonProcessingException e) {
       return defaultValue;
     }
   }
@@ -149,50 +154,45 @@ public static String jsonPathString(Object object, String jsonPath, String defau
    */
   @ScalarFunction
   public static long jsonPathLong(Object object, String jsonPath) {
-    final Object jsonValue = jsonPath(object, jsonPath);
-    if (jsonValue == null) {
-      return Long.MIN_VALUE;
-    }
-    if (jsonValue instanceof Number) {
-      return ((Number) jsonValue).longValue();
-    }
-    return Long.parseLong(jsonValue.toString());
+    return jsonPathLong(object, jsonPath, Long.MIN_VALUE);
   }
 
   /**
    * Extract from Json with path to Long
    */
   @ScalarFunction
   public static long jsonPathLong(Object object, String jsonPath, long defaultValue) {
-    try {
-      return jsonPathLong(object, jsonPath);
-    } catch (Exception e) {
+    Object jsonValue = jsonPath(object, jsonPath);
+    if (jsonValue == null) {
       return defaultValue;
     }
+    if (jsonValue instanceof Number) {
+      return ((Number) jsonValue).longValue();
+    }
+    return Long.parseLong(jsonValue.toString());
   }
 
   /**
    * Extract from Json with path to Double
    */
   @ScalarFunction
   public static double jsonPathDouble(Object object, String jsonPath) {
-    final Object jsonValue = jsonPath(object, jsonPath);
-    if (jsonValue instanceof Number) {
-      return ((Number) jsonValue).doubleValue();
-    }
-    return Double.parseDouble(jsonValue.toString());
+    return jsonPathDouble(object, jsonPath, Double.NaN);
   }
 
   /**
    * Extract from Json with path to Double
    */
   @ScalarFunction
   public static double jsonPathDouble(Object object, String jsonPath, double defaultValue) {
-    try {
-      return jsonPathDouble(object, jsonPath);
-    } catch (Exception e) {
+    Object jsonValue = jsonPath(object, jsonPath);
+    if (jsonValue == null) {
       return defaultValue;
     }
+    if (jsonValue instanceof Number) {
+      return ((Number) jsonValue).doubleValue();
+    }
+    return Double.parseDouble(jsonValue.toString());
   }
 
   @VisibleForTesting

pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java

@@ -19,18 +19,20 @@
 package org.apache.pinot.common.function;
 
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.ObjectMapper;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
-import com.jayway.jsonpath.PathNotFoundException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 import org.apache.pinot.common.function.scalar.JsonFunctions;
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
 import static org.testng.Assert.assertEquals;
-import static org.testng.Assert.fail;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
 
 
 public class JsonFunctionsTest {
@@ -74,7 +76,9 @@ public void testJsonFunction()
     assertEquals(JsonFunctions.jsonPathDouble(jsonString, "$.actor.id"), 33500718.0);
     assertEquals(JsonFunctions.jsonPathString(jsonString, "$.actor.aaa", "null"), "null");
     assertEquals(JsonFunctions.jsonPathLong(jsonString, "$.actor.aaa", 100L), 100L);
+    assertEquals(JsonFunctions.jsonPathLong(jsonString, "$.actor.aaa"), Long.MIN_VALUE);
     assertEquals(JsonFunctions.jsonPathDouble(jsonString, "$.actor.aaa", 53.2), 53.2);
+    assertTrue(Double.isNaN(JsonFunctions.jsonPathDouble(jsonString, "$.actor.aaa")));
   }
 
   @Test
@@ -111,12 +115,7 @@ public void testJsonFunctionExtractingArrayWithMissingField()
       throws JsonProcessingException {
     String jsonString = "{\"name\": \"Pete\", \"age\": 24}";
 
-    try {
-      assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].name"), new String[]{});
-      fail();
-    } catch (PathNotFoundException e) {
-      assertEquals(e.getMessage(), "Missing property in path $['subjects']");
-    }
+    assertEquals(JsonFunctions.jsonPathArray(jsonString, "$.subjects[*].name"), new String[]{});
     assertEquals(JsonFunctions.jsonPathArrayDefaultEmpty(jsonString, "$.subjects[*].name"), new String[]{});
     assertEquals(JsonFunctions.jsonPathArrayDefaultEmpty(jsonString, "$.subjects[*].grade"), new String[]{});
     assertEquals(JsonFunctions.jsonPathArrayDefaultEmpty(jsonString, "$.subjects[*].homework_grades"), new Object[]{});
@@ -245,4 +244,56 @@ public void testJsonFunctionOnObjectArray()
         new Object[]{Arrays.asList(80, 85, 90, 95, 100), Arrays.asList(60, 65, 70, 85, 90)});
     assertEquals(JsonFunctions.jsonPathArray(rawData, "$.[*].score"), new Integer[]{90, 50});
   }
+
+  private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
+
+  @DataProvider
+  public static Object[][] jsonPathStringTestCases() {
+    return new Object[][]{
+        {ImmutableMap.of("foo", "x", "bar", ImmutableMap.of("foo", "y")), "$.foo", "x"},
+        {ImmutableMap.of("foo", "x", "bar", ImmutableMap.of("foo", "y")), "$.qux", null},
+        {ImmutableMap.of("foo", "x", "bar", ImmutableMap.of("foo", "y")), "$.bar", "{\"foo\":\"y\"}"},
+    };
+  }
+
+  @Test(dataProvider = "jsonPathStringTestCases")
+  public void testJsonPathString(Map<String, Object> map, String path, String expected)
+      throws JsonProcessingException {
+    String value = JsonFunctions.jsonPathString(OBJECT_MAPPER.writeValueAsString(map), path);
+    assertEquals(value, expected);
+  }
+
+  @Test(dataProvider = "jsonPathStringTestCases")
+  public void testJsonPathStringWithDefaultValue(Map<String, Object> map, String path, String expected)
+      throws JsonProcessingException {
+    String value = JsonFunctions.jsonPathString(OBJECT_MAPPER.writeValueAsString(map), path, expected);
+    assertEquals(value, expected);
+  }
+
+  @DataProvider
+  public static Object[][] jsonPathArrayTestCases() {
+    return new Object[][]{
+        {ImmutableMap.of("foo", "x", "bar", ImmutableMap.of("foo", "y")), "$.foo", new Object[]{"x"}},
+        {ImmutableMap.of("foo", "x", "bar", ImmutableMap.of("foo", "y")), "$.qux", null},
+        {
+            ImmutableMap.of("foo", "x", "bar", ImmutableMap.of("foo", "y")), "$.bar", new Object[]{
+            ImmutableMap.of("foo", "y")
+        }
+        },
+    };
+  }
+
+  @Test(dataProvider = "jsonPathArrayTestCases")
+  public void testJsonPathArray(Map<String, Object> map, String path, Object[] expected)
+      throws JsonProcessingException {
+    Object[] value = JsonFunctions.jsonPathArray(OBJECT_MAPPER.writeValueAsString(map), path);
+    if (expected == null) {
+      assertNull(value);
+    } else {
+      assertEquals(value.length, expected.length);
+      for (int i = 0; i < value.length; i++) {
+        assertEquals(value[i], expected[i]);
+      }
+    }
+  }
 }

pinot-common/src/test/java/org/apache/pinot/common/function/scalar/ArrayAwareJacksonJsonProviderTest.java

@@ -18,6 +18,7 @@
  */
 package org.apache.pinot.common.function.scalar;
 
+import com.jayway.jsonpath.JsonPathException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -48,15 +49,11 @@ public void testArrayLength() {
 
     List<Object> dataInList = Arrays.asList("abc", "efg", "hij");
     assertEquals(jp.length(dataInList), 3);
+  }
 
-    try {
-      jp.length(null);
-      fail();
-    } catch (NullPointerException e) {
-      // It's supposed to get a JsonPathException, but JsonPath library actually
-      // has a bug leading to NullPointerException while creating the JsonPathException.
-      assertNull(e.getMessage());
-    }
+  @Test(expectedExceptions = JsonPathException.class)
+  public void testArrayLengthThrowsForNullArray() {
+    new JsonFunctions.ArrayAwareJacksonJsonProvider().length(null);
   }
 
   @Test

pinot-core/src/main/java/org/apache/pinot/core/common/evaluators/DefaultJsonPathEvaluator.java

@@ -25,7 +25,6 @@
 import com.jayway.jsonpath.ParseContext;
 import com.jayway.jsonpath.spi.json.JacksonJsonProvider;
 import com.jayway.jsonpath.spi.mapper.JacksonMappingProvider;
-import java.nio.charset.StandardCharsets;
 import java.util.List;
 import javax.annotation.Nullable;
 import org.apache.pinot.common.function.JsonPathCache;
@@ -390,57 +389,21 @@ public <T extends ForwardIndexReaderContext> void evaluateBlock(int[] docIds, in
   }
 
   private <T> T extractFromBytes(Dictionary dictionary, int dictId) {
-    try {
-      // TODO make JsonPath accept byte[] - Jackson can
-      return JSON_PARSER_CONTEXT.parse(new String(dictionary.getBytesValue(dictId), StandardCharsets.UTF_8))
-          .read(_jsonPath);
-    } catch (Exception e) {
-      // TODO JsonPath 2.7.0 will not throw here but produce null when path not found
-      if (_defaultValue == null) {
-        throwPathNotFoundException(e);
-      }
-      return null;
-    }
+    return JSON_PARSER_CONTEXT.parseUtf8(dictionary.getBytesValue(dictId)).read(_jsonPath);
   }
 
   private <T, R extends ForwardIndexReaderContext> T extractFromBytes(ForwardIndexReader<R> reader, R context,
       int docId) {
-    try {
-      // TODO make JsonPath accept byte[] - Jackson can
-      return JSON_PARSER_CONTEXT.parse(new String(reader.getBytes(docId, context), StandardCharsets.UTF_8))
-          .read(_jsonPath);
-    } catch (Exception e) {
-      // TODO JsonPath 2.7.0 will not throw here but produce null when path not found
-      if (_defaultValue == null) {
-        throwPathNotFoundException(e);
-      }
-      return null;
-    }
+    return JSON_PARSER_CONTEXT.parseUtf8(reader.getBytes(docId, context)).read(_jsonPath);
   }
 
   private <T> T extractFromString(Dictionary dictionary, int dictId) {
-    try {
-      return JSON_PARSER_CONTEXT.parse(dictionary.getStringValue(dictId)).read(_jsonPath);
-    } catch (Exception e) {
-      // TODO JsonPath 2.7.0 will not throw here but produce null when path not found
-      if (_defaultValue == null) {
-        throwPathNotFoundException(e);
-      }
-      return null;
-    }
+    return JSON_PARSER_CONTEXT.parse(dictionary.getStringValue(dictId)).read(_jsonPath);
   }
 
   private <T, R extends ForwardIndexReaderContext> T extractFromString(ForwardIndexReader<R> reader, R context,
       int docId) {
-    try {
-      return JSON_PARSER_CONTEXT.parse(reader.getString(docId, context)).read(_jsonPath);
-    } catch (Exception e) {
-      // TODO JsonPath 2.7.0 will not throw here but produce null when path not found
-      if (_defaultValue == null) {
-        throwPathNotFoundException(e);
-      }
-      return null;
-    }
+    return JSON_PARSER_CONTEXT.parseUtf8(reader.getBytes(docId, context)).read(_jsonPath);
   }
 
   private void processValue(int index, Object value, int defaultValue, int[] valueBuffer) {
@@ -582,8 +545,4 @@ private void processList(int index, List<String> value, String[][] valuesBuffer)
   private void throwPathNotFoundException() {
     throw new IllegalArgumentException("Illegal Json Path: " + _jsonPath.getPath() + " does not match document");
   }
-
-  private void throwPathNotFoundException(Exception e) {
-    throw new IllegalArgumentException("Illegal Json Path: " + _jsonPath.getPath() + " does not match document", e);
-  }
 }

pom.xml

@@ -130,7 +130,7 @@
     <hadoop.version>2.7.0</hadoop.version>
     <scala.version>2.13.3</scala.version>
     <antlr.version>4.6</antlr.version>
-    <jsonpath.version>2.4.0</jsonpath.version>
+    <jsonpath.version>2.7.0</jsonpath.version>
     <quartz.version>2.3.2</quartz.version>
     <calcite.version>1.19.0</calcite.version>
     <lucene.version>8.2.0</lucene.version>