From 0850e615b2fbb0a2b2813dd48a1d0a8fc61ee0da Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Sun, 24 Sep 2023 23:04:24 -0700 Subject: [PATCH] Remove istrue, isfalse vectorized impls. (#14991) These were added in #14977, but the implementations are incorrect, because they return null when the input arg is null. They should return false when the input is null. Remove them for now, rather than fixing them, since they're so new that they might as well never have existed. --- .../org/apache/druid/math/expr/Function.java | 27 ------------------- .../druid/sql/calcite/CalciteQueryTest.java | 12 ++++++--- 2 files changed, 9 insertions(+), 30 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/math/expr/Function.java b/processing/src/main/java/org/apache/druid/math/expr/Function.java index cabeb557792c..3af7aa181da1 100644 --- a/processing/src/main/java/org/apache/druid/math/expr/Function.java +++ b/processing/src/main/java/org/apache/druid/math/expr/Function.java @@ -28,7 +28,6 @@ import org.apache.druid.java.util.common.UOE; import org.apache.druid.math.expr.vector.CastToTypeVectorProcessor; import org.apache.druid.math.expr.vector.ExprVectorProcessor; -import org.apache.druid.math.expr.vector.VectorComparisonProcessors; import org.apache.druid.math.expr.vector.VectorMathProcessors; import org.apache.druid.math.expr.vector.VectorProcessors; import org.apache.druid.math.expr.vector.VectorStringProcessors; @@ -2422,19 +2421,6 @@ public ExpressionType getOutputType(Expr.InputBindingInspector inspector, List args) - { - final Expr expr = args.get(0); - return inspector.areNumeric(expr) && expr.canVectorize(inspector); - } - - @Override - public ExprVectorProcessor asVectorProcessor(Expr.VectorInputBindingInspector inspector, List args) - { - return VectorComparisonProcessors.lessThanOrEqual(inspector, args.get(0), ExprEval.of(0L).toExpr()); - } } /** @@ -2467,19 +2453,6 @@ public ExpressionType getOutputType(Expr.InputBindingInspector inspector, List args) - { - final Expr expr = args.get(0); - return inspector.areNumeric(expr) && expr.canVectorize(inspector); - } - - @Override - public ExprVectorProcessor asVectorProcessor(Expr.VectorInputBindingInspector inspector, List args) - { - return VectorComparisonProcessors.greaterThan(inspector, args.get(0), ExprEval.of(0L).toExpr()); - } } class IsNullFunc implements Function diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java index 99b974aa7110..74800270bc4b 100644 --- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java +++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java @@ -2470,6 +2470,12 @@ public void testExactCountDistinctWithFilter() ); requireMergeBuffers(3); + + if (NullHandling.sqlCompatible()) { + // Cannot vectorize due to "istrue" operator. + cannotVectorize(); + } + testQuery( PLANNER_CONFIG_NO_HLL.withOverrides( ImmutableMap.of( @@ -2489,9 +2495,9 @@ public void testExactCountDistinctWithFilter() .setGranularity(Granularities.ALL) .setVirtualColumns(expressionVirtualColumn( "v0", - NullHandling.replaceWithDefault() - ? "(\"cnt\" == 1)" - : "istrue((\"cnt\" == 1))", + NullHandling.sqlCompatible() + ? "istrue((\"cnt\" == 1))" + : "(\"cnt\" == 1)", ColumnType.LONG )) .setDimensions(dimensions(