soumitra-st commented on code in PR #12673:
URL: https://github.com/apache/pinot/pull/12673#discussion_r1530610070
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/ArrayFunctions.java:
##########
@@ -230,7 +230,7 @@ public static String arrayElementAtString(String[] arr, int
idx) {
@ScalarFunction(names = {"array", "arrayValueConstructor"}, isVarArg = true)
public static Object arrayValueConstructor(Object... arr) {
- if (arr.length == 0) {
+ if (arr == null || arr.length == 0 || arr[0] == null) {
Review Comment:
Firstly, to give the context:
foo(String... args) {
args == null // foo(null)
args.length == 0 // foo()
args[0] == null // String bar = null; foo(bar)
In this specific case, if arr[0] == null, the code below
(`arr[0].getClass()`) will cause null pointer exception, hence the additional
checks is an improvement. I didn't follow why it would change the semantics.
I have tried to only add the additional checks in places which can cause
NPE, vs changing all varargs usages in the codebase.
--
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]
