fix null transform bound check (apache#9495)

* fix bound check * add test Co-authored-by: Rong Rong <[email protected]>
61yao · Oct 3, 2022 · a213bb8 · a213bb8
1 parent 0926bfe
commit a213bb8
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 16 deletions.
diff --git a/...in/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java b/...in/java/org/apache/pinot/core/operator/transform/function/IsNotNullTransformFunction.java
@@ -77,12 +77,14 @@ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
       int currentDocIdIndex = 0;
       while (_nullValueVectorIterator.hasNext() & currentDocIdIndex < length) {
         _nullValueVectorIterator.advanceIfNeeded(docIds[currentDocIdIndex]);
-        currentDocIdIndex = Arrays.binarySearch(docIds, currentDocIdIndex, length, _nullValueVectorIterator.next());
-        if (currentDocIdIndex >= 0) {
-          _results[currentDocIdIndex] = 0;
-          currentDocIdIndex++;
-        } else {
-          currentDocIdIndex = -currentDocIdIndex - 1;
+        if (_nullValueVectorIterator.hasNext()) {
+          currentDocIdIndex = Arrays.binarySearch(docIds, currentDocIdIndex, length, _nullValueVectorIterator.next());
+          if (currentDocIdIndex >= 0) {
+            _results[currentDocIdIndex] = 0;
+            currentDocIdIndex++;
+          } else {
+            currentDocIdIndex = -currentDocIdIndex - 1;
+          }
         }
       }
     }

diff --git a/.../main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java b/.../main/java/org/apache/pinot/core/operator/transform/function/IsNullTransformFunction.java
@@ -76,12 +76,14 @@ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
       int currentDocIdIndex = 0;
       while (_nullValueVectorIterator.hasNext() & currentDocIdIndex < length) {
         _nullValueVectorIterator.advanceIfNeeded(docIds[currentDocIdIndex]);
-        currentDocIdIndex = Arrays.binarySearch(docIds, currentDocIdIndex, length, _nullValueVectorIterator.next());
-        if (currentDocIdIndex >= 0) {
-          _results[currentDocIdIndex] = 1;
-          currentDocIdIndex++;
-        } else {
-          currentDocIdIndex = -currentDocIdIndex - 1;
+        if (_nullValueVectorIterator.hasNext()) {
+          currentDocIdIndex = Arrays.binarySearch(docIds, currentDocIdIndex, length, _nullValueVectorIterator.next());
+          if (currentDocIdIndex >= 0) {
+            _results[currentDocIdIndex] = 1;
+            currentDocIdIndex++;
+          } else {
+            currentDocIdIndex = -currentDocIdIndex - 1;
+          }
         }
       }
     }

diff --git a/.../org/apache/pinot/core/operator/transform/function/NullHandlingTransformFunctionTest.java b/.../org/apache/pinot/core/operator/transform/function/NullHandlingTransformFunctionTest.java
@@ -86,17 +86,18 @@ public class NullHandlingTransformFunctionTest {
   protected final double[] _doubleSVValues = new double[NUM_ROWS];
   protected final String[] _stringSVValues = new String[NUM_ROWS];
   protected final byte[][] _bytesSVValues = new byte[NUM_ROWS][];
+  protected final boolean[] _nullValues = new boolean[NUM_ROWS];
 
   protected Map<String, DataSource> _dataSourceMap;
   protected ProjectionBlock _projectionBlock;
-  protected static final int NULL_VALUE_MOD = 10;
 
   @BeforeClass
   public void setup()
       throws Exception {
     FileUtils.deleteQuietly(new File(INDEX_DIR_PATH));
     DecimalFormat df = new DecimalFormat("0", DecimalFormatSymbols.getInstance(Locale.ENGLISH));
     df.setMaximumFractionDigits(340); // 340 = DecimalFormat.DOUBLE_FRACTION_DIGITS
+    Random nullValueRandom = new Random(42);
     long currentTimeMs = System.currentTimeMillis();
     for (int i = 0; i < NUM_ROWS; i++) {
       _intSVValues[i] = RANDOM.nextInt();
@@ -107,12 +108,13 @@ public void setup()
       _bytesSVValues[i] = RandomStringUtils.randomAlphanumeric(26).getBytes();
 
       _timeValues[i] = currentTimeMs - RANDOM.nextInt(365 * 24 * 3600) * 1000L;
+      _nullValues[i] = nullValueRandom.nextInt(2) > 0;
     }
 
     List<GenericRow> rows = new ArrayList<>(NUM_ROWS);
     for (int i = 0; i < NUM_ROWS; i++) {
       Map<String, Object> map = new HashMap<>();
-      if (i % NULL_VALUE_MOD != 0) {
+      if (!_nullValues[i]) {
         map.put(INT_SV_COLUMN, _intSVValues[i]);
         map.put(LONG_SV_COLUMN, _longSVValues[i]);
         map.put(FLOAT_SV_COLUMN, _floatSVValues[i]);
@@ -186,7 +188,7 @@ public void testIsNullTransformFunction(String columnName)
     assertFalse(resultMetadata.hasDictionary());
     boolean[] expectedValues = new boolean[NUM_ROWS];
     for (int i = 0; i < NUM_ROWS; i++) {
-      expectedValues[i] = i % NULL_VALUE_MOD == 0;
+      expectedValues[i] = _nullValues[i];
     }
     testTransformFunction(expression, expectedValues);
   }
@@ -214,7 +216,7 @@ public void testIsNotNullTransformFunction(String columnName)
     assertFalse(resultMetadata.hasDictionary());
     boolean[] expectedValues = new boolean[NUM_ROWS];
     for (int i = 0; i < NUM_ROWS; i++) {
-      expectedValues[i] = i % NULL_VALUE_MOD != 0;
+      expectedValues[i] = !_nullValues[i];
     }
     testTransformFunction(expression, expectedValues);
   }