You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2022/02/01 18:48:40 UTC
[pinot] branch master updated: Upgrade to JsonPath 2.7.0 (#7819)
This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 3f76154 Upgrade to JsonPath 2.7.0 (#7819)
3f76154 is described below
commit 3f761544e2fb6532558c9882f18bfd48e4c927e6
Author: Richard Startin <ri...@startree.ai>
AuthorDate: Tue Feb 1 18:48:20 2022 +0000
Upgrade to JsonPath 2.7.0 (#7819)
---
LICENSE-binary | 2 +-
.../common/function/scalar/JsonFunctions.java | 60 +++++++++----------
.../pinot/common/function/JsonFunctionsTest.java | 67 +++++++++++++++++++---
.../scalar/ArrayAwareJacksonJsonProviderTest.java | 13 ++---
.../evaluators/DefaultJsonPathEvaluator.java | 49 ++--------------
pom.xml | 2 +-
6 files changed, 100 insertions(+), 93 deletions(-)
diff --git a/LICENSE-binary b/LICENSE-binary
index eb25b51..2f0beb8 100644
--- a/LICENSE-binary
+++ b/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
diff --git a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
index 0285f7b..e2f9403 100644
--- a/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
+++ b/pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
@@ -22,6 +22,7 @@ import com.fasterxml.jackson.core.JsonProcessingException;
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 class JsonFunctions {
* 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,11 +105,8 @@ public class JsonFunctions {
@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) {
@@ -115,6 +114,8 @@ public class JsonFunctions {
return ((List) arrayObject).toArray();
} else if (arrayObject instanceof Object[]) {
return (Object[]) arrayObject;
+ } else if (arrayObject == null) {
+ return null;
}
return new Object[]{arrayObject};
}
@@ -129,7 +130,7 @@ public class JsonFunctions {
if (jsonValue instanceof String) {
return (String) jsonValue;
}
- return JsonUtils.objectToString(jsonValue);
+ return jsonValue == null ? null : JsonUtils.objectToString(jsonValue);
}
/**
@@ -137,9 +138,13 @@ public class JsonFunctions {
*/
@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,14 +154,7 @@ public class JsonFunctions {
*/
@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);
}
/**
@@ -164,11 +162,14 @@ public class JsonFunctions {
*/
@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());
}
/**
@@ -176,11 +177,7 @@ public class JsonFunctions {
*/
@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);
}
/**
@@ -188,11 +185,14 @@ public class JsonFunctions {
*/
@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
diff --git a/pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java b/pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java
index de1fb24..f9376af 100644
--- a/pinot-common/src/test/java/org/apache/pinot/common/function/JsonFunctionsTest.java
+++ b/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 class JsonFunctionsTest {
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 class JsonFunctionsTest {
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 class JsonFunctionsTest {
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]);
+ }
+ }
+ }
}
diff --git a/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/ArrayAwareJacksonJsonProviderTest.java b/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/ArrayAwareJacksonJsonProviderTest.java
index 3f4b71e..3f16ed8 100644
--- a/pinot-common/src/test/java/org/apache/pinot/common/function/scalar/ArrayAwareJacksonJsonProviderTest.java
+++ b/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 class ArrayAwareJacksonJsonProviderTest {
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
diff --git a/pinot-core/src/main/java/org/apache/pinot/core/common/evaluators/DefaultJsonPathEvaluator.java b/pinot-core/src/main/java/org/apache/pinot/core/common/evaluators/DefaultJsonPathEvaluator.java
index c399b76..b45acec 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/common/evaluators/DefaultJsonPathEvaluator.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/common/evaluators/DefaultJsonPathEvaluator.java
@@ -25,7 +25,6 @@ import com.jayway.jsonpath.Option;
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 final class DefaultJsonPathEvaluator implements JsonPathEvaluator {
}
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 @@ public final class DefaultJsonPathEvaluator implements JsonPathEvaluator {
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);
- }
}
diff --git a/pom.xml b/pom.xml
index 1d38cf3..dec133e 100644
--- a/pom.xml
+++ b/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>
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org