shenyu0127 commented on code in PR #10594:
URL: https://github.com/apache/pinot/pull/10594#discussion_r1176875098


##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java:
##########
@@ -241,226 +266,768 @@ public TransformResultMetadata getResultMetadata() {
   }
 
   /**
-   * Evaluate the ValueBlock for the WHEN statements, returns an array with the
-   * index(1 to N) of matched WHEN clause ordered by match priority, 0 means 
nothing
-   * matched, so go to ELSE.
+   * Evaluate the ValueBlock for the WHEN statements, returns an array with 
the index(1 to N) of matched WHEN clause
+   * -1 means there is no match.
    */
-  private int[] getSelectedArray(ValueBlock valueBlock) {
+  private int[] getSelectedArray(ValueBlock valueBlock, boolean 
nullHandlingEnabled) {
     int numDocs = valueBlock.getNumDocs();
     if (_selectedResults == null || _selectedResults.length < numDocs) {
       _selectedResults = new int[numDocs];
-    } else {
-      Arrays.fill(_selectedResults, 0, numDocs, 0);
-      Arrays.fill(_selections, false);
     }
+    Arrays.fill(_selectedResults, 0, numDocs, -1);
+    Arrays.fill(_selections, false);
+    BitSet unselectedDocs = new BitSet();
+    unselectedDocs.set(0, numDocs);
     int numWhenStatements = _whenStatements.size();
-    for (int i = numWhenStatements - 1; i >= 0; i--) {
+    for (int i = 0; i < numWhenStatements; i++) {
       TransformFunction whenStatement = _whenStatements.get(i);
-      int[] conditions = whenStatement.transformToIntValuesSV(valueBlock);
-      for (int j = 0; j < numDocs & j < conditions.length; j++) {
-        _selectedResults[j] = Math.max(conditions[j] * (i + 1), 
_selectedResults[j]);
+      int[] conditions = getWhenConditions(whenStatement, valueBlock, 
nullHandlingEnabled);
+      for (int docId = unselectedDocs.nextSetBit(0); docId >= 0; docId = 
unselectedDocs.nextSetBit(docId + 1)) {
+        if (conditions[docId] == 1) {
+          unselectedDocs.clear(docId);
+          _selectedResults[docId] = i;
+        }
+      }
+      if (unselectedDocs.isEmpty()) {
+        break;
       }
     }
     // try to prune clauses now
     for (int i = 0; i < numDocs; i++) {
-      _selections[_selectedResults[i]] = true;
-    }
-    int numSelections = 0;
-    for (boolean selection : _selections) {
-      if (selection) {
-        numSelections++;
+      if (_selectedResults[i] != -1) {
+        _selections[_selectedResults[i]] = true;
       }
     }
-    _numSelections = numSelections;
     return _selectedResults;
   }
 
+  // Returns an array of valueBlock length to indicate whether a row is 
selected or not.
+  // When nullHandlingEnabled is set to true, we also check whether the row is 
null and set to false if null.
+  private static int[] getWhenConditions(TransformFunction whenStatement, 
ValueBlock valueBlock,
+      boolean nullHandlingEnabled) {
+    if (!nullHandlingEnabled) {
+      return whenStatement.transformToIntValuesSV(valueBlock);
+    }
+    Pair<int[], RoaringBitmap> result = 
whenStatement.transformToIntValuesSVWithNull(valueBlock);
+    RoaringBitmap bitmap = result.getRight();
+    int[] intResult = result.getLeft();
+    if (bitmap != null) {
+      bitmap.forEach((IntConsumer) (i) -> {
+        if (bitmap.contains(i)) {
+          intResult[i] = 0;
+        }
+      });

Review Comment:
   ```
   if (bitmap != null) {
     for (int i : bitmap) {
       intResult[i] = 0;
     };
   }
   ```



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java:
##########
@@ -241,226 +266,768 @@ public TransformResultMetadata getResultMetadata() {
   }
 
   /**
-   * Evaluate the ValueBlock for the WHEN statements, returns an array with the
-   * index(1 to N) of matched WHEN clause ordered by match priority, 0 means 
nothing
-   * matched, so go to ELSE.
+   * Evaluate the ValueBlock for the WHEN statements, returns an array with 
the index(1 to N) of matched WHEN clause
+   * -1 means there is no match.
    */
-  private int[] getSelectedArray(ValueBlock valueBlock) {
+  private int[] getSelectedArray(ValueBlock valueBlock, boolean 
nullHandlingEnabled) {
     int numDocs = valueBlock.getNumDocs();
     if (_selectedResults == null || _selectedResults.length < numDocs) {
       _selectedResults = new int[numDocs];
-    } else {
-      Arrays.fill(_selectedResults, 0, numDocs, 0);
-      Arrays.fill(_selections, false);
     }
+    Arrays.fill(_selectedResults, 0, numDocs, -1);
+    Arrays.fill(_selections, false);
+    BitSet unselectedDocs = new BitSet();
+    unselectedDocs.set(0, numDocs);
     int numWhenStatements = _whenStatements.size();
-    for (int i = numWhenStatements - 1; i >= 0; i--) {
+    for (int i = 0; i < numWhenStatements; i++) {
       TransformFunction whenStatement = _whenStatements.get(i);
-      int[] conditions = whenStatement.transformToIntValuesSV(valueBlock);
-      for (int j = 0; j < numDocs & j < conditions.length; j++) {
-        _selectedResults[j] = Math.max(conditions[j] * (i + 1), 
_selectedResults[j]);
+      int[] conditions = getWhenConditions(whenStatement, valueBlock, 
nullHandlingEnabled);
+      for (int docId = unselectedDocs.nextSetBit(0); docId >= 0; docId = 
unselectedDocs.nextSetBit(docId + 1)) {
+        if (conditions[docId] == 1) {
+          unselectedDocs.clear(docId);
+          _selectedResults[docId] = i;
+        }
+      }
+      if (unselectedDocs.isEmpty()) {
+        break;
       }
     }
     // try to prune clauses now
     for (int i = 0; i < numDocs; i++) {
-      _selections[_selectedResults[i]] = true;
-    }
-    int numSelections = 0;
-    for (boolean selection : _selections) {
-      if (selection) {
-        numSelections++;
+      if (_selectedResults[i] != -1) {
+        _selections[_selectedResults[i]] = true;
       }
     }
-    _numSelections = numSelections;
     return _selectedResults;
   }
 
+  // Returns an array of valueBlock length to indicate whether a row is 
selected or not.
+  // When nullHandlingEnabled is set to true, we also check whether the row is 
null and set to false if null.
+  private static int[] getWhenConditions(TransformFunction whenStatement, 
ValueBlock valueBlock,
+      boolean nullHandlingEnabled) {
+    if (!nullHandlingEnabled) {
+      return whenStatement.transformToIntValuesSV(valueBlock);
+    }
+    Pair<int[], RoaringBitmap> result = 
whenStatement.transformToIntValuesSVWithNull(valueBlock);
+    RoaringBitmap bitmap = result.getRight();
+    int[] intResult = result.getLeft();
+    if (bitmap != null) {
+      bitmap.forEach((IntConsumer) (i) -> {
+        if (bitmap.contains(i)) {
+          intResult[i] = 0;
+        }
+      });
+    }
+    return intResult;
+  }
+
   @Override
   public int[] transformToIntValuesSV(ValueBlock valueBlock) {
     if (_resultMetadata.getDataType().getStoredType() != DataType.INT) {
       return super.transformToIntValuesSV(valueBlock);
     }
-    int[] selected = getSelectedArray(valueBlock);
+    int[] selected = getSelectedArray(valueBlock, false);
     int numDocs = valueBlock.getNumDocs();
     initIntValuesSV(numDocs);
-    int numElseThenStatements = _elseThenStatements.size();
-    for (int i = 0; i < numElseThenStatements; i++) {
+    int numThenStatements = _thenStatements.size();
+    BitSet unselectedDocs = new BitSet();
+    unselectedDocs.set(0, numDocs);
+    for (int i = 0; i < numThenStatements; i++) {
       if (_selections[i]) {
-        TransformFunction transformFunction = _elseThenStatements.get(i);
+        TransformFunction transformFunction = _thenStatements.get(i);
         int[] intValues = transformFunction.transformToIntValuesSV(valueBlock);
-        if (_numSelections == 1) {
-          System.arraycopy(intValues, 0, _intValuesSV, 0, numDocs);
-        } else {
-          for (int j = 0; j < numDocs; j++) {
-            if (selected[j] == i) {
-              _intValuesSV[j] = intValues[j];
-            }
+        for (int docId = unselectedDocs.nextSetBit(0); docId >= 0; docId = 
unselectedDocs.nextSetBit(docId + 1)) {

Review Comment:
   I meant we compute all the "then statements" that need to be computed. 
   
   (The previous comment implicitly meant that because it suggested to use a 
map. The map's size may be smaller than `numThenStatements`.)



##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/CoalesceTransformFunction.java:
##########
@@ -116,24 +108,24 @@ private int[] getIntTransformResults(ValueBlock 
valueBlock) {
     int width = _transformFunctions.length;
     RoaringBitmap[] nullBitMaps = getNullBitMaps(valueBlock, 
_transformFunctions);
     int[][] data = new int[width][length];
-    RoaringBitmap filledData = new RoaringBitmap();
+    BitSet filledData = new BitSet(); // indicates whether certain column has 
be filled in data.
     for (int i = 0; i < length; i++) {
       boolean hasNonNullValue = false;
       for (int j = 0; j < width; j++) {
         // Consider value as null only when null option is enabled.
         if (nullBitMaps[j] != null && nullBitMaps[j].contains(i)) {
           continue;
         }
-        if (!filledData.contains(j)) {
-          filledData.add(j);
+        if (!filledData.get(j)) {
+          filledData.set(j);
           data[j] = _transformFunctions[j].transformToIntValuesSV(valueBlock);
         }

Review Comment:
   Also s/hasNonNullValue/valueIsNull for better readability.



-- 
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

Reply via email to