Jackie-Jiang commented on a change in pull request #8382:
URL: https://github.com/apache/pinot/pull/8382#discussion_r834590900
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -84,7 +84,7 @@ public static String jsonFormat(Object object)
/**
* Extract object based on Json path
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
Review comment:
No need to annotate this function because when input is `null`, output
is also `null`. The null check on line 92 can actually be removed with the
change in this PR because that will never be hit
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -103,7 +103,7 @@ public static Object jsonPath(Object object, String
jsonPath) {
return convertObjectToArray(PARSE_CONTEXT.parse(object).read(jsonPath,
NO_PREDICATES));
}
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
public static Object[] jsonPathArrayDefaultEmpty(Object object, String
jsonPath) {
Review comment:
`object` should be annotated as `Nullable`
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -127,7 +127,7 @@ public static Object jsonPath(Object object, String
jsonPath) {
/**
* Extract from Json with path to String
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
Review comment:
Similarly, we don't need to annotate this function, and the null check
can be removed
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -66,7 +66,7 @@ private JsonFunctions() {
/**
* Convert Map to Json String
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
public static String toJsonMapStr(Map map)
Review comment:
Let's annotate the nullable parameter as `Nullable`, same for other
annotated functions
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -183,15 +183,15 @@ public static long jsonPathLong(Object object, String
jsonPath, long defaultValu
/**
* Extract from Json with path to Double
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
public static double jsonPathDouble(Object object, String jsonPath) {
return jsonPathDouble(object, jsonPath, Double.NaN);
}
/**
* Extract from Json with path to Double
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
public static double jsonPathDouble(Object object, String jsonPath, double
defaultValue) {
Review comment:
`object` should be annotated as `Nullable`
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -183,15 +183,15 @@ public static long jsonPathLong(Object object, String
jsonPath, long defaultValu
/**
* Extract from Json with path to Double
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
Review comment:
Similarly, don't annotate this function
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -156,15 +156,15 @@ public static String jsonPathString(Object object, String
jsonPath, String defau
/**
* Extract from Json with path to Long
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
Review comment:
Let's not annotate this function. When the input is null, we want to
return null so that the actual default value defined in the schema can be filled
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/JsonFunctions.java
##########
@@ -156,15 +156,15 @@ public static String jsonPathString(Object object, String
jsonPath, String defau
/**
* Extract from Json with path to Long
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
public static long jsonPathLong(Object object, String jsonPath) {
return jsonPathLong(object, jsonPath, Long.MIN_VALUE);
}
/**
* Extract from Json with path to Long
*/
- @ScalarFunction
+ @ScalarFunction(nullableParameters = true)
public static long jsonPathLong(Object object, String jsonPath, long
defaultValue) {
Review comment:
`object` should be annotated as `Nullable`
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/function/InbuiltFunctionEvaluator.java
##########
@@ -108,11 +108,13 @@ public Object evaluate(Object[] values) {
private static class FunctionExecutionNode implements ExecutableNode {
final FunctionInvoker _functionInvoker;
+ private final FunctionInfo _functionInfo;
Review comment:
(minor) remove `private` as it is already in a private class
--
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]
