[GitHub] [pinot] Jackie-Jiang commented on a diff in pull request #10376: [feature] [null support # 1] selection only literal in broker null support

["Jackie-Jiang (via GitHub)" <gi...@apache.org>]

Jackie-Jiang commented on code in PR #10376:
URL: https://github.com/apache/pinot/pull/10376#discussion_r1148110788


##########
pinot-common/src/main/java/org/apache/pinot/common/utils/request/RequestUtils.java:
##########
@@ -173,7 +175,16 @@ public static Expression getLiteralExpression(byte[] value) {
     return expression;
   }
 
+  public static Expression getUnknownLiteralExpression() {

Review Comment:
   (minor)
   ```suggestion
     public static Expression getNullLiteralExpression() {
   ```



##########
pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java:
##########
@@ -743,6 +746,10 @@ private void verifyDataIsSame(DataTable newDataTable, DataSchema.ColumnDataType[
               Assert.assertEquals(ObjectSerDeUtils.deserialize(customObject), OBJECTS[rowId], ERROR_MESSAGE);
             }
             break;
+          case UNKNOWN:
+            Object nulValue = newDataTable.getCustomObject(rowId, colId);
+            Assert.assertNull(nulValue, ERROR_MESSAGE);

Review Comment:
   (nit)
   ```suggestion
               Assert.assertNull(newDataTable.getCustomObject(rowId, colId), ERROR_MESSAGE);
   ```



##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/rewriter/CompileTimeFunctionsInvoker.java:
##########
@@ -74,7 +75,12 @@ protected static Expression invokeCompileTimeFunctionExpression(@Nullable Expres
       if (functionInfo != null) {
         Object[] arguments = new Object[numOperands];
         for (int i = 0; i < numOperands; i++) {
-          arguments[i] = function.getOperands().get(i).getLiteral().getFieldValue();
+          Literal literal = function.getOperands().get(i).getLiteral();
+          if (literal.getSetField() == Literal._Fields.NULL_VALUE) {

Review Comment:
   (minor)
   ```suggestion
             if (literal.isSetNullValue) {
   ```



##########
pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java:
##########
@@ -743,6 +746,10 @@ private void verifyDataIsSame(DataTable newDataTable, DataSchema.ColumnDataType[
               Assert.assertEquals(ObjectSerDeUtils.deserialize(customObject), OBJECTS[rowId], ERROR_MESSAGE);
             }
             break;
+          case UNKNOWN:

Review Comment:
   (nit) Move it to the end



##########
pinot-core/src/test/java/org/apache/pinot/core/common/datatable/DataTableSerDeTest.java:
##########
@@ -614,6 +614,9 @@ private void fillDataTableWithRandomData(DataTableBuilder dataTableBuilder,
             OBJECTS[rowId] = isNull ? null : RANDOM.nextDouble();
             dataTableBuilder.setColumn(colId, OBJECTS[rowId]);
             break;
+          case UNKNOWN:

Review Comment:
   (nit) Move it to the end



-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org