diff --git a/processing/src/main/java/org/apache/druid/math/expr/vector/DoubleOutLongsInFunctionVectorValueProcessor.java b/processing/src/main/java/org/apache/druid/math/expr/vector/DoubleOutLongsInFunctionVectorValueProcessor.java index 699f66f53fe3..d9fa3fe2856b 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/vector/DoubleOutLongsInFunctionVectorValueProcessor.java +++ b/processing/src/main/java/org/apache/druid/math/expr/vector/DoubleOutLongsInFunctionVectorValueProcessor.java @@ -45,7 +45,7 @@ public DoubleOutLongsInFunctionVectorValueProcessor( @Override public ExpressionType getOutputType() { - return ExpressionType.LONG; + return ExpressionType.DOUBLE; } @Override diff --git a/processing/src/main/java/org/apache/druid/math/expr/vector/VectorProcessors.java b/processing/src/main/java/org/apache/druid/math/expr/vector/VectorProcessors.java index 1a16d99d1831..2e2c5ffee3ca 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/vector/VectorProcessors.java +++ b/processing/src/main/java/org/apache/druid/math/expr/vector/VectorProcessors.java @@ -544,9 +544,11 @@ public void processIndex( outputNulls[i] = rightNulls[i]; } else { output[i] = rightInput[i]; + outputNulls[i] = false; } } else { output[i] = leftInput[i]; + outputNulls[i] = false; } } @@ -580,9 +582,11 @@ public void processIndex( outputNulls[i] = rightNulls[i]; } else { output[i] = rightInput[i]; + outputNulls[i] = false; } } else { output[i] = leftInput[i]; + outputNulls[i] = false; } } @@ -744,6 +748,7 @@ public void processIndex( } } output[i] = Evals.asLong(Evals.asBoolean(leftInput[i]) || Evals.asBoolean(rightInput[i])); + outputNulls[i] = false; } @Override @@ -793,6 +798,7 @@ public void processIndex( } } output[i] = Evals.asLong(Evals.asBoolean(leftInput[i]) || Evals.asBoolean(rightInput[i])); + outputNulls[i] = false; } @Override @@ -839,6 +845,7 @@ public void processIndex( return; } output[i] = Evals.asLong(Evals.asBoolean((String) leftInput[i]) || Evals.asBoolean((String) rightInput[i])); + outputNulls[i] = false; } @Override @@ -907,6 +914,7 @@ public void processIndex( } } output[i] = Evals.asLong(Evals.asBoolean(leftInput[i]) && Evals.asBoolean(rightInput[i])); + outputNulls[i] = false; } @Override @@ -916,7 +924,7 @@ public ExprEvalVector asEval() } }, () -> new BivariateFunctionVectorProcessor( - ExpressionType.DOUBLE, + ExpressionType.LONG, left.asVectorProcessor(inputTypes), right.asVectorProcessor(inputTypes) ) @@ -956,6 +964,7 @@ public void processIndex( } } output[i] = Evals.asLong(Evals.asBoolean(leftInput[i]) && Evals.asBoolean(rightInput[i])); + outputNulls[i] = false; } @Override @@ -965,7 +974,7 @@ public ExprEvalVector asEval() } }, () -> new BivariateFunctionVectorProcessor( - ExpressionType.STRING, + ExpressionType.LONG, left.asVectorProcessor(inputTypes), right.asVectorProcessor(inputTypes) ) @@ -1004,6 +1013,7 @@ public void processIndex( output[i] = Evals.asLong( Evals.asBoolean((String) leftInput[i]) && Evals.asBoolean((String) rightInput[i]) ); + outputNulls[i] = false; } @Override diff --git a/processing/src/test/java/org/apache/druid/math/expr/VectorExprSanityTest.java b/processing/src/test/java/org/apache/druid/math/expr/VectorExprSanityTest.java index a032e993db0f..d25220f0929c 100644 --- a/processing/src/test/java/org/apache/druid/math/expr/VectorExprSanityTest.java +++ b/processing/src/test/java/org/apache/druid/math/expr/VectorExprSanityTest.java @@ -25,6 +25,7 @@ import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.math.expr.vector.ExprEvalVector; +import org.apache.druid.math.expr.vector.ExprVectorProcessor; import org.apache.druid.testing.InitializedNullHandlingTest; import org.junit.Assert; import org.junit.Test; @@ -113,7 +114,7 @@ public void testUnaryLogicOperators() public void testBinaryLogicOperators() { final String[] functions = new String[]{"&&", "||"}; - final String[] templates = new String[]{"d1 %s d2", "l1 %s l2", "boolString1 %s boolString2"}; + final String[] templates = new String[]{"d1 %s d2", "l1 %s l2", "boolString1 %s boolString2", "(d1 == d2) %s (l1 == l2)"}; testFunctions(types, templates, functions); } @@ -283,21 +284,17 @@ static void testExpression(String expr, Map types) log.debug("[%s]", expr); Expr parsed = Parser.parse(expr, ExprMacroTable.nil()); - NonnullPair bindings; - for (int iterations = 0; iterations < NUM_ITERATIONS; iterations++) { - bindings = makeRandomizedBindings(VECTOR_SIZE, types); - testExpressionWithBindings(expr, parsed, bindings); - } - bindings = makeSequentialBinding(VECTOR_SIZE, types); - testExpressionWithBindings(expr, parsed, bindings); + testExpression(expr, parsed, types, NUM_ITERATIONS); + testSequentialBinding(expr, parsed, types); } - public static void testExpressionWithBindings( + public static void testSequentialBinding( String expr, Expr parsed, - NonnullPair bindings + Map types ) { + NonnullPair bindings = makeSequentialBinding(VECTOR_SIZE, types); Assert.assertTrue(StringUtils.format("Cannot vectorize %s", expr), parsed.canVectorize(bindings.rhs)); ExpressionType outputType = parsed.getOutputType(bindings.rhs); ExprEvalVector vectorEval = parsed.asVectorProcessor(bindings.rhs).evalVector(bindings.rhs); @@ -320,6 +317,55 @@ public static void testExpressionWithBindings( } } + public static void testExpression( + String expr, + Expr parsed, + Map types, + int numIterations + ) + { + Expr.InputBindingInspector inspector = InputBindings.inspectorFromTypeMap(types); + Expr.VectorInputBindingInspector vectorInputBindingInspector = new Expr.VectorInputBindingInspector() + { + @Override + public int getMaxVectorSize() + { + return VECTOR_SIZE; + } + + @Nullable + @Override + public ExpressionType getType(String name) + { + return inspector.getType(name); + } + }; + Assert.assertTrue(StringUtils.format("Cannot vectorize %s", expr), parsed.canVectorize(inspector)); + ExpressionType outputType = parsed.getOutputType(inspector); + final ExprVectorProcessor processor = parsed.asVectorProcessor(vectorInputBindingInspector); + // 'null' expressions can have an output type of null, but still evaluate in default mode, so skip type checks + if (outputType != null) { + Assert.assertEquals(expr, outputType, processor.getOutputType()); + } + for (int iterations = 0; iterations < numIterations; iterations++) { + NonnullPair bindings = makeRandomizedBindings(VECTOR_SIZE, types); + ExprEvalVector vectorEval = processor.evalVector(bindings.rhs); + final Object[] vectorVals = vectorEval.getObjectVector(); + for (int i = 0; i < VECTOR_SIZE; i++) { + ExprEval eval = parsed.eval(bindings.lhs[i]); + // 'null' expressions can have an output type of null, but still evaluate in default mode, so skip type checks + if (outputType != null && !eval.isNumericNull()) { + Assert.assertEquals(eval.type(), outputType); + } + Assert.assertEquals( + StringUtils.format("Values do not match for row %s for expression %s", i, expr), + eval.valueOrDefault(), + vectorVals[i] + ); + } + } + } + public static NonnullPair makeRandomizedBindings( int vectorSize, Map types @@ -332,7 +378,7 @@ public static NonnullPair makeRan types, () -> r.nextLong(Integer.MAX_VALUE - 1), r::nextDouble, - r::nextBoolean, + () -> r.nextDouble(0, 1.0) > 0.9, () -> String.valueOf(r.nextInt()) ); } diff --git a/processing/src/test/java/org/apache/druid/query/expression/VectorExpressionsSanityTest.java b/processing/src/test/java/org/apache/druid/query/expression/VectorExpressionsSanityTest.java index ba23a54a7ca7..c8cfdf100659 100644 --- a/processing/src/test/java/org/apache/druid/query/expression/VectorExpressionsSanityTest.java +++ b/processing/src/test/java/org/apache/druid/query/expression/VectorExpressionsSanityTest.java @@ -22,7 +22,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.apache.druid.java.util.common.DateTimes; -import org.apache.druid.java.util.common.NonnullPair; import org.apache.druid.java.util.common.logger.Logger; import org.apache.druid.math.expr.Expr; import org.apache.druid.math.expr.ExprEval; @@ -62,13 +61,8 @@ public class VectorExpressionsSanityTest extends InitializedNullHandlingTest static void testExpression(String expr, Expr parsed, Map types) { log.debug("[%s]", expr); - NonnullPair bindings; - for (int iterations = 0; iterations < NUM_ITERATIONS; iterations++) { - bindings = VectorExprSanityTest.makeRandomizedBindings(VECTOR_SIZE, types); - VectorExprSanityTest.testExpressionWithBindings(expr, parsed, bindings); - } - bindings = VectorExprSanityTest.makeSequentialBinding(VECTOR_SIZE, types); - VectorExprSanityTest.testExpressionWithBindings(expr, parsed, bindings); + VectorExprSanityTest.testExpression(expr, parsed, types, NUM_ITERATIONS); + VectorExprSanityTest.testSequentialBinding(expr, parsed, types); } @Test