Do not implicit cast for BOOLEAN and TIMESTAMP (#9385)

apache · Sep 19, 2022 · 985d0b5 · 985d0b5
1 parent 974e1ea
commit 985d0b5
Show file tree

Hide file tree

Showing 29 changed files with 1,081 additions and 973 deletions.
diff --git a/...on/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java b/...on/src/main/java/org/apache/pinot/common/function/scalar/DataTypeConversionFunctions.java
@@ -43,6 +43,7 @@ private DataTypeConversionFunctions() {
   public static Object cast(Object value, String targetTypeLiteral) {
     try {
       Class<?> clazz = value.getClass();
+      // TODO: Support cast for MV
       Preconditions.checkArgument(!clazz.isArray() | clazz == byte[].class, "%s must not be an array type", clazz);
       PinotDataType sourceType = PinotDataType.getSingleValueType(clazz);
       String transformed = targetTypeLiteral.toUpperCase();

diff --git a/pinot-core/src/main/java/org/apache/pinot/core/common/DataFetcher.java b/pinot-core/src/main/java/org/apache/pinot/core/common/DataFetcher.java
diff --git a/...rc/main/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunction.java b/...rc/main/java/org/apache/pinot/core/operator/transform/function/BaseTransformFunction.java
diff --git a/...rc/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java b/...rc/main/java/org/apache/pinot/core/operator/transform/function/CaseTransformFunction.java
@@ -436,7 +436,7 @@ public byte[][] transformToBytesValuesSV(ProjectionBlock projectionBlock) {
         TransformFunction transformFunction = _elseThenStatements.get(i);
         byte[][] bytesValues = transformFunction.transformToBytesValuesSV(projectionBlock);
         if (_numSelections == 1) {
-          System.arraycopy(bytesValues, 0, _byteValuesSV, 0, numDocs);
+          System.arraycopy(bytesValues, 0, _bytesValuesSV, 0, numDocs);
         } else {
           for (int j = 0; j < numDocs; j++) {
             if (selected[j] == i) {

diff --git a/...rc/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java b/...rc/main/java/org/apache/pinot/core/operator/transform/function/CastTransformFunction.java
diff --git a/...a/org/apache/pinot/core/operator/transform/function/LogicalOperatorTransformFunction.java b/...a/org/apache/pinot/core/operator/transform/function/LogicalOperatorTransformFunction.java
@@ -24,7 +24,6 @@
 import org.apache.pinot.core.operator.blocks.ProjectionBlock;
 import org.apache.pinot.core.operator.transform.TransformResultMetadata;
 import org.apache.pinot.segment.spi.datasource.DataSource;
-import org.apache.pinot.spi.utils.ArrayCopyUtils;
 
 
 /**
@@ -33,15 +32,15 @@
  */
 public abstract class LogicalOperatorTransformFunction extends BaseTransformFunction {
   protected List<TransformFunction> _arguments;
-  protected int[] _results;
 
   @Override
   public void init(List<TransformFunction> arguments, Map<String, DataSource> dataSourceMap) {
     _arguments = arguments;
     int numArguments = arguments.size();
     if (numArguments <= 1) {
-      throw new IllegalArgumentException("Expect more than 1 argument for logical operator [" + getName() + "], args ["
-          + Arrays.toString(arguments.toArray()) + "].");
+      throw new IllegalArgumentException(
+          "Expect more than 1 argument for logical operator [" + getName() + "], args [" + Arrays.toString(
+              arguments.toArray()) + "].");
     }
     for (int i = 0; i < numArguments; i++) {
       TransformResultMetadata argumentMetadata = arguments.get(i).getResultMetadata();
@@ -60,20 +59,19 @@ public TransformResultMetadata getResultMetadata() {
   @Override
   public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
     int numDocs = projectionBlock.getNumDocs();
-
-    if (_results == null || _results.length < numDocs) {
-      _results = new int[numDocs];
+    if (_intValuesSV == null) {
+      _intValuesSV = new int[numDocs];
     }
-    ArrayCopyUtils.copy(_arguments.get(0).transformToIntValuesSV(projectionBlock), _results, numDocs);
+    System.arraycopy(_arguments.get(0).transformToIntValuesSV(projectionBlock), 0, _intValuesSV, 0, numDocs);
     int numArguments = _arguments.size();
     for (int i = 1; i < numArguments; i++) {
       TransformFunction transformFunction = _arguments.get(i);
       int[] results = transformFunction.transformToIntValuesSV(projectionBlock);
       for (int j = 0; j < numDocs; j++) {
-        _results[j] = getLogicalFuncResult(_results[j], results[j]);
+        _intValuesSV[j] = getLogicalFuncResult(_intValuesSV[j], results[j]);
       }
     }
-    return _results;
+    return _intValuesSV;
   }
 
   abstract int getLogicalFuncResult(int left, int right);

diff --git a/.../main/java/org/apache/pinot/core/operator/transform/function/LookupTransformFunction.java b/.../main/java/org/apache/pinot/core/operator/transform/function/LookupTransformFunction.java
@@ -227,8 +227,11 @@ private void lookup(ProjectionBlock projectionBlock, ValueAcceptor valueAcceptor
 
   @Override
   public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.INT) {
+      return super.transformToIntValuesSV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_intValuesSV == null || _intValuesSV.length < numDocs) {
+    if (_intValuesSV == null) {
       _intValuesSV = new int[numDocs];
     }
     lookup(projectionBlock, this::setIntSV);
@@ -237,8 +240,11 @@ public int[] transformToIntValuesSV(ProjectionBlock projectionBlock) {
 
   @Override
   public long[] transformToLongValuesSV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.LONG) {
+      return super.transformToLongValuesSV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_longValuesSV == null || _longValuesSV.length < numDocs) {
+    if (_longValuesSV == null) {
       _longValuesSV = new long[numDocs];
     }
     lookup(projectionBlock, this::setLongSV);
@@ -247,8 +253,11 @@ public long[] transformToLongValuesSV(ProjectionBlock projectionBlock) {
 
   @Override
   public float[] transformToFloatValuesSV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.FLOAT) {
+      return super.transformToFloatValuesSV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_floatValuesSV == null || _floatValuesSV.length < numDocs) {
+    if (_floatValuesSV == null) {
       _floatValuesSV = new float[numDocs];
     }
     lookup(projectionBlock, this::setFloatSV);
@@ -257,8 +266,11 @@ public float[] transformToFloatValuesSV(ProjectionBlock projectionBlock) {
 
   @Override
   public double[] transformToDoubleValuesSV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.DOUBLE) {
+      return super.transformToDoubleValuesSV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_doubleValuesSV == null || _doubleValuesSV.length < numDocs) {
+    if (_doubleValuesSV == null) {
       _doubleValuesSV = new double[numDocs];
     }
     lookup(projectionBlock, this::setDoubleSV);
@@ -267,8 +279,11 @@ public double[] transformToDoubleValuesSV(ProjectionBlock projectionBlock) {
 
   @Override
   public String[] transformToStringValuesSV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.STRING) {
+      return super.transformToStringValuesSV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_stringValuesSV == null || _stringValuesSV.length < numDocs) {
+    if (_stringValuesSV == null) {
       _stringValuesSV = new String[numDocs];
     }
     lookup(projectionBlock, this::setStringSV);
@@ -277,18 +292,24 @@ public String[] transformToStringValuesSV(ProjectionBlock projectionBlock) {
 
   @Override
   public byte[][] transformToBytesValuesSV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.BYTES) {
+      return super.transformToBytesValuesSV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_byteValuesSV == null || _byteValuesSV.length < numDocs) {
-      _byteValuesSV = new byte[numDocs][];
+    if (_bytesValuesSV == null) {
+      _bytesValuesSV = new byte[numDocs][];
     }
     lookup(projectionBlock, this::setBytesSV);
-    return _byteValuesSV;
+    return _bytesValuesSV;
   }
 
   @Override
   public int[][] transformToIntValuesMV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.INT) {
+      return super.transformToIntValuesMV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_intValuesMV == null || _intValuesMV.length < numDocs) {
+    if (_intValuesMV == null) {
       _intValuesMV = new int[numDocs][];
     }
     lookup(projectionBlock, this::setIntMV);
@@ -297,8 +318,11 @@ public int[][] transformToIntValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public long[][] transformToLongValuesMV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.LONG) {
+      return super.transformToLongValuesMV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_longValuesMV == null || _longValuesMV.length < numDocs) {
+    if (_longValuesMV == null) {
       _longValuesMV = new long[numDocs][];
     }
     lookup(projectionBlock, this::setLongMV);
@@ -307,8 +331,11 @@ public long[][] transformToLongValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public float[][] transformToFloatValuesMV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.FLOAT) {
+      return super.transformToFloatValuesMV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_floatValuesMV == null || _floatValuesMV.length < numDocs) {
+    if (_floatValuesMV == null) {
       _floatValuesMV = new float[numDocs][];
     }
     lookup(projectionBlock, this::setFloatMV);
@@ -317,8 +344,11 @@ public float[][] transformToFloatValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public double[][] transformToDoubleValuesMV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.DOUBLE) {
+      return super.transformToDoubleValuesMV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_doubleValuesMV == null || _doubleValuesMV.length < numDocs) {
+    if (_doubleValuesMV == null) {
       _doubleValuesMV = new double[numDocs][];
     }
     lookup(projectionBlock, this::setDoubleMV);
@@ -327,8 +357,11 @@ public double[][] transformToDoubleValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public String[][] transformToStringValuesMV(ProjectionBlock projectionBlock) {
+    if (_lookupColumnFieldSpec.getDataType().getStoredType() != DataType.STRING) {
+      return super.transformToStringValuesMV(projectionBlock);
+    }
     int numDocs = projectionBlock.getNumDocs();
-    if (_stringValuesMV == null || _stringValuesMV.length < numDocs) {
+    if (_stringValuesMV == null) {
       _stringValuesMV = new String[numDocs][];
     }
     lookup(projectionBlock, this::setStringMV);
@@ -377,9 +410,9 @@ private void setStringSV(int index, Object value) {
 
   private void setBytesSV(int index, Object value) {
     if (value instanceof byte[]) {
-      _byteValuesSV[index] = (byte[]) value;
+      _bytesValuesSV[index] = (byte[]) value;
     } else {
-      _byteValuesSV[index] = EMPTY_BYTES;
+      _bytesValuesSV[index] = EMPTY_BYTES;
     }
   }
 

diff --git a/...main/java/org/apache/pinot/core/operator/transform/function/ValueInTransformFunction.java b/...main/java/org/apache/pinot/core/operator/transform/function/ValueInTransformFunction.java
@@ -43,7 +43,7 @@
 import org.apache.pinot.core.operator.transform.TransformResultMetadata;
 import org.apache.pinot.segment.spi.datasource.DataSource;
 import org.apache.pinot.segment.spi.index.reader.Dictionary;
-import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.FieldSpec.DataType;
 
 
 public class ValueInTransformFunction extends BaseTransformFunction {
@@ -132,7 +132,7 @@ public int[][] transformToDictIdsMV(ProjectionBlock projectionBlock) {
 
   @Override
   public int[][] transformToIntValuesMV(ProjectionBlock projectionBlock) {
-    if (_dictionary != null || _resultMetadata.getDataType() != FieldSpec.DataType.INT) {
+    if (_dictionary != null || _resultMetadata.getDataType().getStoredType() != DataType.INT) {
       return super.transformToIntValuesMV(projectionBlock);
     }
 
@@ -155,7 +155,7 @@ public int[][] transformToIntValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public long[][] transformToLongValuesMV(ProjectionBlock projectionBlock) {
-    if (_dictionary != null || _resultMetadata.getDataType() != FieldSpec.DataType.LONG) {
+    if (_dictionary != null || _resultMetadata.getDataType().getStoredType() != DataType.LONG) {
       return super.transformToLongValuesMV(projectionBlock);
     }
 
@@ -178,7 +178,7 @@ public long[][] transformToLongValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public float[][] transformToFloatValuesMV(ProjectionBlock projectionBlock) {
-    if (_dictionary != null || _resultMetadata.getDataType() != FieldSpec.DataType.FLOAT) {
+    if (_dictionary != null || _resultMetadata.getDataType().getStoredType() != DataType.FLOAT) {
       return super.transformToFloatValuesMV(projectionBlock);
     }
 
@@ -201,7 +201,7 @@ public float[][] transformToFloatValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public double[][] transformToDoubleValuesMV(ProjectionBlock projectionBlock) {
-    if (_dictionary != null || _resultMetadata.getDataType() != FieldSpec.DataType.DOUBLE) {
+    if (_dictionary != null || _resultMetadata.getDataType().getStoredType() != DataType.DOUBLE) {
       return super.transformToDoubleValuesMV(projectionBlock);
     }
 
@@ -224,7 +224,7 @@ public double[][] transformToDoubleValuesMV(ProjectionBlock projectionBlock) {
 
   @Override
   public String[][] transformToStringValuesMV(ProjectionBlock projectionBlock) {
-    if (_dictionary != null || _resultMetadata.getDataType() != FieldSpec.DataType.STRING) {
+    if (_dictionary != null || _resultMetadata.getDataType().getStoredType() != DataType.STRING) {
       return super.transformToStringValuesMV(projectionBlock);
     }
 

diff --git a/...a/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunctionTest.java b/...a/org/apache/pinot/core/operator/transform/function/AndOperatorTransformFunctionTest.java
@@ -18,17 +18,18 @@
  */
 package org.apache.pinot.core.operator.transform.function;
 
+import org.apache.pinot.common.function.TransformFunctionType;
+
+
 public class AndOperatorTransformFunctionTest extends LogicalOperatorTransformFunctionTest {
+
   @Override
-  int getExpectedValue(boolean left, boolean right) {
-    if (left && right) {
-      return 1;
-    }
-    return 0;
+  boolean getExpectedValue(boolean left, boolean right) {
+    return left && right;
   }
 
   @Override
-  String getFuncName() {
-    return new AndOperatorTransformFunction().getName();
+  String getFunctionName() {
+    return TransformFunctionType.AND.getName();
   }
 }