gortiz commented on code in PR #11546:
URL: https://github.com/apache/pinot/pull/11546#discussion_r1319438573
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunction.java:
##########
@@ -45,4 +48,37 @@ int getLogicalFuncResult(int left, int right) {
}
return 0;
}
+
+ @Nullable
+ @Override
+ public RoaringBitmap getNullBitmap(ValueBlock valueBlock) {
+ int numDocs = valueBlock.getNumDocs();
+ int numArguments = _arguments.size();
+ int[][] intValuesSVs = new int[numArguments][numDocs];
Review Comment:
I think this is incorrect from performance perspective. This sentence
instantiates the whole matrix (it allocates an array of `numArguments` elements
pointing to a new array of `numDocs` ints). Few lines later elements on
`intValuesSVs` are changed to
`_arguments.get(i).transformToIntValuesSV(valueBlock);`. Therefore we are
creating the internal arrays for no reason.
What you should do here is `new int[numArguments][];`
You can verify that in jshell:
```
╰─➤ jshell
microk8s
| Welcome to JShell -- Version 11.0.19
| For an introduction type: /help intro
jshell> var a = new int[2][3];
a ==> int[2][] { int[3] { 0, 0, 0 }, int[3] { 0, 0, 0 } }
jshell> a[0]
$2 ==> int[3] { 0, 0, 0 }
jshell> var b = new int[2][];
b ==> int[2][] { null, null }
jshell> b[0]
$4 ==> null
```
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunction.java:
##########
@@ -45,4 +48,37 @@ int getLogicalFuncResult(int left, int right) {
}
return 0;
}
+
+ @Nullable
+ @Override
+ public RoaringBitmap getNullBitmap(ValueBlock valueBlock) {
+ int numDocs = valueBlock.getNumDocs();
+ int numArguments = _arguments.size();
+ int[][] intValuesSVs = new int[numArguments][numDocs];
Review Comment:
I think this is incorrect from performance perspective. This sentence
instantiates the whole matrix (it allocates an array of `numArguments` elements
pointing to a new array of `numDocs` ints). Few lines later elements on
`intValuesSVs` are changed to
`_arguments.get(i).transformToIntValuesSV(valueBlock);`. Therefore we are
creating the internal arrays for no reason.
What you should do here is `new int[numArguments][];`
You can verify that in jshell:
```
╰─➤ jshell
microk8s
| Welcome to JShell -- Version 11.0.19
| For an introduction type: /help intro
jshell> var a = new int[2][3];
a ==> int[2][] { int[3] { 0, 0, 0 }, int[3] { 0, 0, 0 } }
jshell> a[0]
$2 ==> int[3] { 0, 0, 0 }
jshell> var b = new int[2][];
b ==> int[2][] { null, null }
jshell> b[0]
$4 ==> null
```
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
