You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by jo...@apache.org on 2020/10/03 22:30:57 UTC
[druid] branch master updated: fix array types from escaping into
wider query engine (#10460)
This is an automated email from the ASF dual-hosted git repository.
jonwei pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 9ec5c08 fix array types from escaping into wider query engine (#10460)
9ec5c08 is described below
commit 9ec5c08e2a3c1210fefc78e26fbafe75702c7c2f
Author: Clint Wylie <cw...@apache.org>
AuthorDate: Sat Oct 3 15:30:34 2020 -0700
fix array types from escaping into wider query engine (#10460)
* fix array types from escaping into wider query engine
* oops
* adjust
* fix lgtm
---
.../apache/druid/math/expr/BinaryOperatorExpr.java | 8 +-
.../org/apache/druid/math/expr/ConstantExpr.java | 11 ++
.../main/java/org/apache/druid/math/expr/Expr.java | 6 +
.../java/org/apache/druid/math/expr/ExprType.java | 104 --------------
.../apache/druid/math/expr/ExprTypeConversion.java | 159 +++++++++++++++++++++
.../java/org/apache/druid/math/expr/Function.java | 33 +++--
.../org/apache/druid/math/expr/OutputTypeTest.java | 142 ++++++++++--------
.../druid/segment/virtual/ExpressionPlanner.java | 12 +-
.../segment/virtual/ExpressionVirtualColumn.java | 10 +-
.../druid/sql/calcite/expression/Expressions.java | 17 ---
.../builtin/ReductionOperatorConversionHelper.java | 3 +-
.../apache/druid/sql/calcite/planner/Calcites.java | 9 +-
.../apache/druid/sql/calcite/rel/QueryMaker.java | 16 ++-
.../apache/druid/sql/calcite/CalciteQueryTest.java | 30 +++-
14 files changed, 354 insertions(+), 206 deletions(-)
diff --git a/core/src/main/java/org/apache/druid/math/expr/BinaryOperatorExpr.java b/core/src/main/java/org/apache/druid/math/expr/BinaryOperatorExpr.java
index 128a780..20ecc5d 100644
--- a/core/src/main/java/org/apache/druid/math/expr/BinaryOperatorExpr.java
+++ b/core/src/main/java/org/apache/druid/math/expr/BinaryOperatorExpr.java
@@ -83,7 +83,13 @@ abstract class BinaryOpExprBase implements Expr
@Override
public ExprType getOutputType(InputBindingTypes inputTypes)
{
- return ExprType.operatorAutoTypeConversion(left.getOutputType(inputTypes), right.getOutputType(inputTypes));
+ if (left.isNullLiteral()) {
+ return right.getOutputType(inputTypes);
+ }
+ if (right.isNullLiteral()) {
+ return left.getOutputType(inputTypes);
+ }
+ return ExprTypeConversion.operator(left.getOutputType(inputTypes), right.getOutputType(inputTypes));
}
@Override
diff --git a/core/src/main/java/org/apache/druid/math/expr/ConstantExpr.java b/core/src/main/java/org/apache/druid/math/expr/ConstantExpr.java
index 279600d..57ae900 100644
--- a/core/src/main/java/org/apache/druid/math/expr/ConstantExpr.java
+++ b/core/src/main/java/org/apache/druid/math/expr/ConstantExpr.java
@@ -99,6 +99,11 @@ abstract class NullNumericConstantExpr extends ConstantExpr
}
+ @Override
+ public boolean isNullLiteral()
+ {
+ return true;
+ }
}
class LongExpr extends ConstantExpr
@@ -429,6 +434,12 @@ class StringExpr extends ConstantExpr
}
@Override
+ public boolean isNullLiteral()
+ {
+ return value == null;
+ }
+
+ @Override
public String toString()
{
return value;
diff --git a/core/src/main/java/org/apache/druid/math/expr/Expr.java b/core/src/main/java/org/apache/druid/math/expr/Expr.java
index be0a32e..ff646fe 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Expr.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Expr.java
@@ -53,6 +53,12 @@ public interface Expr
return false;
}
+ default boolean isNullLiteral()
+ {
+ // Overridden by things that are null literals.
+ return false;
+ }
+
/**
* Returns the value of expr if expr is a literal, or throws an exception otherwise.
*
diff --git a/core/src/main/java/org/apache/druid/math/expr/ExprType.java b/core/src/main/java/org/apache/druid/math/expr/ExprType.java
index e11b8ace..ebdf64a 100644
--- a/core/src/main/java/org/apache/druid/math/expr/ExprType.java
+++ b/core/src/main/java/org/apache/druid/math/expr/ExprType.java
@@ -19,7 +19,6 @@
package org.apache.druid.math.expr;
-import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.ISE;
import org.apache.druid.segment.column.ValueType;
@@ -169,107 +168,4 @@ public enum ExprType
return elementType;
}
- /**
- * Given 2 'input' types, choose the most appropriate combined type, if possible
- *
- * arrays must be the same type
- * if both types are {@link #STRING}, the output type will be preserved as string
- * if both types are {@link #LONG}, the output type will be preserved as long
- *
- */
- @Nullable
- public static ExprType operatorAutoTypeConversion(@Nullable ExprType type, @Nullable ExprType other)
- {
- if (type == null || other == null) {
- // cannot auto conversion unknown types
- return null;
- }
- // arrays cannot be auto converted
- if (isArray(type) || isArray(other)) {
- if (!type.equals(other)) {
- throw new IAE("Cannot implicitly cast %s to %s", type, other);
- }
- return type;
- }
- // if both arguments are a string, type becomes a string
- if (STRING.equals(type) && STRING.equals(other)) {
- return STRING;
- }
-
- // otherwise a decimal or integer number
- return numericAutoTypeConversion(type, other);
- }
-
- /**
- * Given 2 'input' types, choose the most appropriate combined type, if possible
- *
- * arrays must be the same type
- * if either type is {@link #STRING}, the output type will be preserved as string
- * if both types are {@link #LONG}, the output type will be preserved as long, otherwise {@link #DOUBLE}
- */
- @Nullable
- public static ExprType doubleMathFunctionAutoTypeConversion(@Nullable ExprType type, @Nullable ExprType other)
- {
- if (type == null || other == null) {
- // cannot auto conversion unknown types
- return null;
- }
- // arrays cannot be auto converted
- if (isArray(type) || isArray(other)) {
- if (!type.equals(other)) {
- throw new IAE("Cannot implicitly cast %s to %s", type, other);
- }
- return type;
- }
- // if either argument is a string, type becomes a string
- if (STRING.equals(type) || STRING.equals(other)) {
- return STRING;
- }
-
- return numericAutoTypeConversion(type, other);
- }
-
- /**
- * Given 2 'input' types, choose the most appropriate combined type, if possible
- *
- * arrays must be the same type
- * if either type is {@link #STRING}, the output type will be preserved as string
- * any number will be coerced to {@link #LONG}
- */
- @Nullable
- public static ExprType integerMathFunctionAutoTypeConversion(@Nullable ExprType type, @Nullable ExprType other)
- {
- if (type == null || other == null) {
- // cannot auto conversion unknown types
- return null;
- }
- // arrays cannot be auto converted
- if (isArray(type) || isArray(other)) {
- if (!type.equals(other)) {
- throw new IAE("Cannot implicitly cast %s to %s", type, other);
- }
- return type;
- }
- // if either argument is a string, type becomes a string
- if (STRING.equals(type) || STRING.equals(other)) {
- return STRING;
- }
-
- // any number is long
- return LONG;
- }
-
- /**
- * Default best effort numeric type conversion. If both types are {@link #LONG}, returns {@link #LONG}, else
- * {@link #DOUBLE}
- */
- public static ExprType numericAutoTypeConversion(ExprType type, ExprType other)
- {
- // all numbers win over longs
- if (LONG.equals(type) && LONG.equals(other)) {
- return LONG;
- }
- // floats vs doubles would be handled here, but we currently only support doubles...
- return DOUBLE;
- }
}
diff --git a/core/src/main/java/org/apache/druid/math/expr/ExprTypeConversion.java b/core/src/main/java/org/apache/druid/math/expr/ExprTypeConversion.java
new file mode 100644
index 0000000..2fc4577
--- /dev/null
+++ b/core/src/main/java/org/apache/druid/math/expr/ExprTypeConversion.java
@@ -0,0 +1,159 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.math.expr;
+
+import org.apache.druid.java.util.common.IAE;
+
+import javax.annotation.Nullable;
+import java.util.List;
+
+public class ExprTypeConversion
+{
+
+ /**
+ * Infer the output type of a list of possible 'conditional' expression outputs (where any of these could be the
+ * output expression if the corresponding case matching expression evaluates to true)
+ */
+ static ExprType conditional(Expr.InputBindingTypes inputTypes, List<Expr> args)
+ {
+ ExprType type = null;
+ for (Expr arg : args) {
+ if (arg.isNullLiteral()) {
+ continue;
+ }
+ if (type == null) {
+ type = arg.getOutputType(inputTypes);
+ } else {
+ type = doubleMathFunction(type, arg.getOutputType(inputTypes));
+ }
+ }
+ return type;
+ }
+
+ /**
+ * Given 2 'input' types, choose the most appropriate combined type, if possible
+ *
+ * arrays must be the same type
+ * if both types are {@link ExprType#STRING}, the output type will be preserved as string
+ * if both types are {@link ExprType#LONG}, the output type will be preserved as long
+ *
+ */
+ @Nullable
+ public static ExprType operator(@Nullable ExprType type, @Nullable ExprType other)
+ {
+ if (type == null || other == null) {
+ // cannot auto conversion unknown types
+ return null;
+ }
+ // arrays cannot be auto converted
+ if (ExprType.isArray(type) || ExprType.isArray(other)) {
+ if (!type.equals(other)) {
+ throw new IAE("Cannot implicitly cast %s to %s", type, other);
+ }
+ return type;
+ }
+ // if both arguments are a string, type becomes a string
+ if (ExprType.STRING.equals(type) && ExprType.STRING.equals(other)) {
+ return ExprType.STRING;
+ }
+
+ // otherwise a decimal or integer number
+ return numeric(type, other);
+ }
+
+ /**
+ * Given 2 'input' types, choose the most appropriate combined type, if possible
+ *
+ * arrays must be the same type
+ * if either type is {@link ExprType#STRING}, the output type will be preserved as string
+ * if both types are {@link ExprType#LONG}, the output type will be preserved as long, otherwise
+ * {@link ExprType#DOUBLE}
+ */
+ @Nullable
+ public static ExprType doubleMathFunction(@Nullable ExprType type, @Nullable ExprType other)
+ {
+ if (type == null || other == null) {
+ // cannot auto conversion unknown types
+ return null;
+ }
+ // arrays cannot be auto converted
+ if (ExprType.isArray(type) || ExprType.isArray(other)) {
+ if (!type.equals(other)) {
+ throw new IAE("Cannot implicitly cast %s to %s", type, other);
+ }
+ return type;
+ }
+ // if either argument is a string, type becomes a string
+ if (ExprType.STRING.equals(type) || ExprType.STRING.equals(other)) {
+ return ExprType.STRING;
+ }
+
+ return numeric(type, other);
+ }
+
+ /**
+ * Given 2 'input' types, choose the most appropriate combined type, if possible
+ *
+ * arrays must be the same type
+ * if either type is {@link ExprType#STRING}, the output type will be preserved as string
+ * any number will be coerced to {@link ExprType#LONG}
+ */
+ @Nullable
+ public static ExprType integerMathFunction(@Nullable ExprType type, @Nullable ExprType other)
+ {
+ if (type == null || other == null) {
+ // cannot auto conversion unknown types
+ return null;
+ }
+ // arrays cannot be auto converted
+ if (ExprType.isArray(type) || ExprType.isArray(other)) {
+ if (!type.equals(other)) {
+ throw new IAE("Cannot implicitly cast %s to %s", type, other);
+ }
+ return type;
+ }
+ // if either argument is a string, type becomes a string
+ if (ExprType.STRING.equals(type) || ExprType.STRING.equals(other)) {
+ return ExprType.STRING;
+ }
+
+ // any number is long
+ return ExprType.LONG;
+ }
+
+ /**
+ * Default best effort numeric type conversion. If both types are {@link ExprType#LONG}, returns
+ * {@link ExprType#LONG}, else {@link ExprType#DOUBLE}
+ */
+ public static ExprType numeric(ExprType type, ExprType other)
+ {
+ // all numbers win over longs
+ if (ExprType.LONG.equals(type) && ExprType.LONG.equals(other)) {
+ return ExprType.LONG;
+ }
+ // floats vs doubles would be handled here, but we currently only support doubles...
+ return ExprType.DOUBLE;
+ }
+
+ private ExprTypeConversion()
+ {
+ // no instantiation
+ }
+}
diff --git a/core/src/main/java/org/apache/druid/math/expr/Function.java b/core/src/main/java/org/apache/druid/math/expr/Function.java
index d812fa8..488a35c 100644
--- a/core/src/main/java/org/apache/druid/math/expr/Function.java
+++ b/core/src/main/java/org/apache/druid/math/expr/Function.java
@@ -279,7 +279,7 @@ public interface Function
@Override
public ExprType getOutputType(Expr.InputBindingTypes inputTypes, List<Expr> args)
{
- return ExprType.doubleMathFunctionAutoTypeConversion(args.get(0).getOutputType(inputTypes), args.get(1).getOutputType(inputTypes));
+ return ExprTypeConversion.doubleMathFunction(args.get(0).getOutputType(inputTypes), args.get(1).getOutputType(inputTypes));
}
@Override
@@ -444,7 +444,7 @@ public interface Function
{
ExprType outputType = ExprType.LONG;
for (Expr expr : args) {
- outputType = ExprType.doubleMathFunctionAutoTypeConversion(outputType, expr.getOutputType(inputTypes));
+ outputType = ExprTypeConversion.doubleMathFunction(outputType, expr.getOutputType(inputTypes));
}
return outputType;
}
@@ -465,7 +465,7 @@ public interface Function
ExprType exprType = exprEval.type();
if (isValidType(exprType)) {
- outputType = ExprType.doubleMathFunctionAutoTypeConversion(outputType, exprType);
+ outputType = ExprTypeConversion.doubleMathFunction(outputType, exprType);
}
if (exprEval.value() != null) {
@@ -843,7 +843,7 @@ public interface Function
@Override
public ExprType getOutputType(Expr.InputBindingTypes inputTypes, List<Expr> args)
{
- return ExprType.integerMathFunctionAutoTypeConversion(args.get(0).getOutputType(inputTypes), args.get(1).getOutputType(inputTypes));
+ return ExprTypeConversion.integerMathFunction(args.get(0).getOutputType(inputTypes), args.get(1).getOutputType(inputTypes));
}
@Override
@@ -1700,8 +1700,7 @@ public interface Function
@Override
public ExprType getOutputType(Expr.InputBindingTypes inputTypes, List<Expr> args)
{
- // output type is defined by else
- return args.get(2).getOutputType(inputTypes);
+ return ExprTypeConversion.conditional(inputTypes, args.subList(1, 3));
}
}
@@ -1744,8 +1743,13 @@ public interface Function
@Override
public ExprType getOutputType(Expr.InputBindingTypes inputTypes, List<Expr> args)
{
- // output type is defined by else
- return args.get(args.size() - 1).getOutputType(inputTypes);
+ List<Expr> results = new ArrayList<>();
+ for (int i = 1; i < args.size(); i += 2) {
+ results.add(args.get(i));
+ }
+ // add else
+ results.add(args.get(args.size() - 1));
+ return ExprTypeConversion.conditional(inputTypes, results);
}
}
@@ -1788,8 +1792,13 @@ public interface Function
@Override
public ExprType getOutputType(Expr.InputBindingTypes inputTypes, List<Expr> args)
{
- // output type is defined by else
- return args.get(args.size() - 1).getOutputType(inputTypes);
+ List<Expr> results = new ArrayList<>();
+ for (int i = 2; i < args.size(); i += 2) {
+ results.add(args.get(i));
+ }
+ // add else
+ results.add(args.get(args.size() - 1));
+ return ExprTypeConversion.conditional(inputTypes, results);
}
}
@@ -1820,7 +1829,7 @@ public interface Function
@Override
public ExprType getOutputType(Expr.InputBindingTypes inputTypes, List<Expr> args)
{
- return args.get(0).getOutputType(inputTypes);
+ return ExprTypeConversion.conditional(inputTypes, args);
}
}
@@ -2619,7 +2628,7 @@ public interface Function
{
ExprType type = ExprType.LONG;
for (Expr arg : args) {
- type = ExprType.doubleMathFunctionAutoTypeConversion(type, arg.getOutputType(inputTypes));
+ type = ExprTypeConversion.doubleMathFunction(type, arg.getOutputType(inputTypes));
}
return ExprType.asArrayType(type);
}
diff --git a/core/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java b/core/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
index c28fdb8..441a421 100644
--- a/core/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
+++ b/core/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
@@ -195,6 +195,17 @@ public class OutputTypeTest extends InitializedNullHandlingTest
);
assertOutputType(
+ "case_simple(y,2,2,3,3.0,4)",
+ inputTypes,
+ ExprType.DOUBLE
+ );
+ assertOutputType(
+ "case_simple(z,2.0,2.0,3.0,3.0,null)",
+ inputTypes,
+ ExprType.DOUBLE
+ );
+
+ assertOutputType(
"case_searched(x=='baz','is baz',x=='foo','is foo','is other')",
inputTypes,
ExprType.STRING
@@ -209,10 +220,27 @@ public class OutputTypeTest extends InitializedNullHandlingTest
inputTypes,
ExprType.DOUBLE
);
+ assertOutputType(
+ "case_searched(y==1,1,y==2,2.0,0)",
+ inputTypes,
+ ExprType.DOUBLE
+ );
+ assertOutputType(
+ "case_searched(z==1.0,1,z==2.0,2,null)",
+ inputTypes,
+ ExprType.LONG
+ );
+ assertOutputType(
+ "case_searched(z==1.0,1.0,z==2.0,2.0,null)",
+ inputTypes,
+ ExprType.DOUBLE
+ );
assertOutputType("nvl(x, 'foo')", inputTypes, ExprType.STRING);
assertOutputType("nvl(y, 1)", inputTypes, ExprType.LONG);
+ assertOutputType("nvl(y, 1.1)", inputTypes, ExprType.DOUBLE);
assertOutputType("nvl(z, 2.0)", inputTypes, ExprType.DOUBLE);
+ assertOutputType("nvl(y, 2.0)", inputTypes, ExprType.DOUBLE);
assertOutputType("isnull(x)", inputTypes, ExprType.LONG);
assertOutputType("isnull(y)", inputTypes, ExprType.LONG);
assertOutputType("isnull(z)", inputTypes, ExprType.LONG);
@@ -365,33 +393,33 @@ public class OutputTypeTest extends InitializedNullHandlingTest
public void testOperatorAutoConversion()
{
// nulls output nulls
- Assert.assertNull(ExprType.operatorAutoTypeConversion(ExprType.LONG, null));
- Assert.assertNull(ExprType.operatorAutoTypeConversion(null, ExprType.LONG));
- Assert.assertNull(ExprType.operatorAutoTypeConversion(ExprType.DOUBLE, null));
- Assert.assertNull(ExprType.operatorAutoTypeConversion(null, ExprType.DOUBLE));
- Assert.assertNull(ExprType.operatorAutoTypeConversion(ExprType.STRING, null));
- Assert.assertNull(ExprType.operatorAutoTypeConversion(null, ExprType.STRING));
+ Assert.assertNull(ExprTypeConversion.operator(ExprType.LONG, null));
+ Assert.assertNull(ExprTypeConversion.operator(null, ExprType.LONG));
+ Assert.assertNull(ExprTypeConversion.operator(ExprType.DOUBLE, null));
+ Assert.assertNull(ExprTypeConversion.operator(null, ExprType.DOUBLE));
+ Assert.assertNull(ExprTypeConversion.operator(ExprType.STRING, null));
+ Assert.assertNull(ExprTypeConversion.operator(null, ExprType.STRING));
// only long stays long
- Assert.assertEquals(ExprType.LONG, ExprType.operatorAutoTypeConversion(ExprType.LONG, ExprType.LONG));
+ Assert.assertEquals(ExprType.LONG, ExprTypeConversion.operator(ExprType.LONG, ExprType.LONG));
// only string stays string
- Assert.assertEquals(ExprType.STRING, ExprType.operatorAutoTypeConversion(ExprType.STRING, ExprType.STRING));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.operator(ExprType.STRING, ExprType.STRING));
// for operators, doubles is the catch all
- Assert.assertEquals(ExprType.DOUBLE, ExprType.operatorAutoTypeConversion(ExprType.LONG, ExprType.DOUBLE));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.operatorAutoTypeConversion(ExprType.DOUBLE, ExprType.LONG));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.operatorAutoTypeConversion(ExprType.DOUBLE, ExprType.DOUBLE));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.operatorAutoTypeConversion(ExprType.DOUBLE, ExprType.STRING));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.operatorAutoTypeConversion(ExprType.STRING, ExprType.DOUBLE));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.operatorAutoTypeConversion(ExprType.STRING, ExprType.LONG));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.operatorAutoTypeConversion(ExprType.LONG, ExprType.STRING));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.operator(ExprType.LONG, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.operator(ExprType.DOUBLE, ExprType.LONG));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.operator(ExprType.DOUBLE, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.operator(ExprType.DOUBLE, ExprType.STRING));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.operator(ExprType.STRING, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.operator(ExprType.STRING, ExprType.LONG));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.operator(ExprType.LONG, ExprType.STRING));
// unless it is an array, and those have to be the same
- Assert.assertEquals(ExprType.LONG_ARRAY, ExprType.operatorAutoTypeConversion(ExprType.LONG_ARRAY, ExprType.LONG_ARRAY));
+ Assert.assertEquals(ExprType.LONG_ARRAY, ExprTypeConversion.operator(ExprType.LONG_ARRAY, ExprType.LONG_ARRAY));
Assert.assertEquals(
ExprType.DOUBLE_ARRAY,
- ExprType.operatorAutoTypeConversion(ExprType.DOUBLE_ARRAY, ExprType.DOUBLE_ARRAY)
+ ExprTypeConversion.operator(ExprType.DOUBLE_ARRAY, ExprType.DOUBLE_ARRAY)
);
Assert.assertEquals(
ExprType.STRING_ARRAY,
- ExprType.operatorAutoTypeConversion(ExprType.STRING_ARRAY, ExprType.STRING_ARRAY)
+ ExprTypeConversion.operator(ExprType.STRING_ARRAY, ExprType.STRING_ARRAY)
);
}
@@ -399,33 +427,33 @@ public class OutputTypeTest extends InitializedNullHandlingTest
public void testFunctionAutoConversion()
{
// nulls output nulls
- Assert.assertNull(ExprType.doubleMathFunctionAutoTypeConversion(ExprType.LONG, null));
- Assert.assertNull(ExprType.doubleMathFunctionAutoTypeConversion(null, ExprType.LONG));
- Assert.assertNull(ExprType.doubleMathFunctionAutoTypeConversion(ExprType.DOUBLE, null));
- Assert.assertNull(ExprType.doubleMathFunctionAutoTypeConversion(null, ExprType.DOUBLE));
- Assert.assertNull(ExprType.doubleMathFunctionAutoTypeConversion(ExprType.STRING, null));
- Assert.assertNull(ExprType.doubleMathFunctionAutoTypeConversion(null, ExprType.STRING));
+ Assert.assertNull(ExprTypeConversion.doubleMathFunction(ExprType.LONG, null));
+ Assert.assertNull(ExprTypeConversion.doubleMathFunction(null, ExprType.LONG));
+ Assert.assertNull(ExprTypeConversion.doubleMathFunction(ExprType.DOUBLE, null));
+ Assert.assertNull(ExprTypeConversion.doubleMathFunction(null, ExprType.DOUBLE));
+ Assert.assertNull(ExprTypeConversion.doubleMathFunction(ExprType.STRING, null));
+ Assert.assertNull(ExprTypeConversion.doubleMathFunction(null, ExprType.STRING));
// only long stays long
- Assert.assertEquals(ExprType.LONG, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.LONG, ExprType.LONG));
+ Assert.assertEquals(ExprType.LONG, ExprTypeConversion.doubleMathFunction(ExprType.LONG, ExprType.LONG));
// any double makes all doubles
- Assert.assertEquals(ExprType.DOUBLE, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.LONG, ExprType.DOUBLE));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.DOUBLE, ExprType.LONG));
- Assert.assertEquals(ExprType.DOUBLE, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.DOUBLE, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.doubleMathFunction(ExprType.LONG, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.doubleMathFunction(ExprType.DOUBLE, ExprType.LONG));
+ Assert.assertEquals(ExprType.DOUBLE, ExprTypeConversion.doubleMathFunction(ExprType.DOUBLE, ExprType.DOUBLE));
// any string makes become string
- Assert.assertEquals(ExprType.STRING, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.LONG, ExprType.STRING));
- Assert.assertEquals(ExprType.STRING, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.STRING, ExprType.LONG));
- Assert.assertEquals(ExprType.STRING, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.DOUBLE, ExprType.STRING));
- Assert.assertEquals(ExprType.STRING, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.STRING, ExprType.DOUBLE));
- Assert.assertEquals(ExprType.STRING, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.STRING, ExprType.STRING));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.doubleMathFunction(ExprType.LONG, ExprType.STRING));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.doubleMathFunction(ExprType.STRING, ExprType.LONG));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.doubleMathFunction(ExprType.DOUBLE, ExprType.STRING));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.doubleMathFunction(ExprType.STRING, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.doubleMathFunction(ExprType.STRING, ExprType.STRING));
// unless it is an array, and those have to be the same
- Assert.assertEquals(ExprType.LONG_ARRAY, ExprType.doubleMathFunctionAutoTypeConversion(ExprType.LONG_ARRAY, ExprType.LONG_ARRAY));
+ Assert.assertEquals(ExprType.LONG_ARRAY, ExprTypeConversion.doubleMathFunction(ExprType.LONG_ARRAY, ExprType.LONG_ARRAY));
Assert.assertEquals(
ExprType.DOUBLE_ARRAY,
- ExprType.doubleMathFunctionAutoTypeConversion(ExprType.DOUBLE_ARRAY, ExprType.DOUBLE_ARRAY)
+ ExprTypeConversion.doubleMathFunction(ExprType.DOUBLE_ARRAY, ExprType.DOUBLE_ARRAY)
);
Assert.assertEquals(
ExprType.STRING_ARRAY,
- ExprType.doubleMathFunctionAutoTypeConversion(ExprType.STRING_ARRAY, ExprType.STRING_ARRAY)
+ ExprTypeConversion.doubleMathFunction(ExprType.STRING_ARRAY, ExprType.STRING_ARRAY)
);
}
@@ -433,32 +461,32 @@ public class OutputTypeTest extends InitializedNullHandlingTest
public void testIntegerFunctionAutoConversion()
{
// nulls output nulls
- Assert.assertNull(ExprType.integerMathFunctionAutoTypeConversion(ExprType.LONG, null));
- Assert.assertNull(ExprType.integerMathFunctionAutoTypeConversion(null, ExprType.LONG));
- Assert.assertNull(ExprType.integerMathFunctionAutoTypeConversion(ExprType.DOUBLE, null));
- Assert.assertNull(ExprType.integerMathFunctionAutoTypeConversion(null, ExprType.DOUBLE));
- Assert.assertNull(ExprType.integerMathFunctionAutoTypeConversion(ExprType.STRING, null));
- Assert.assertNull(ExprType.integerMathFunctionAutoTypeConversion(null, ExprType.STRING));
+ Assert.assertNull(ExprTypeConversion.integerMathFunction(ExprType.LONG, null));
+ Assert.assertNull(ExprTypeConversion.integerMathFunction(null, ExprType.LONG));
+ Assert.assertNull(ExprTypeConversion.integerMathFunction(ExprType.DOUBLE, null));
+ Assert.assertNull(ExprTypeConversion.integerMathFunction(null, ExprType.DOUBLE));
+ Assert.assertNull(ExprTypeConversion.integerMathFunction(ExprType.STRING, null));
+ Assert.assertNull(ExprTypeConversion.integerMathFunction(null, ExprType.STRING));
// all numbers are longs
- Assert.assertEquals(ExprType.LONG, ExprType.integerMathFunctionAutoTypeConversion(ExprType.LONG, ExprType.LONG));
- Assert.assertEquals(ExprType.LONG, ExprType.integerMathFunctionAutoTypeConversion(ExprType.LONG, ExprType.DOUBLE));
- Assert.assertEquals(ExprType.LONG, ExprType.integerMathFunctionAutoTypeConversion(ExprType.DOUBLE, ExprType.LONG));
- Assert.assertEquals(ExprType.LONG, ExprType.integerMathFunctionAutoTypeConversion(ExprType.DOUBLE, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.LONG, ExprTypeConversion.integerMathFunction(ExprType.LONG, ExprType.LONG));
+ Assert.assertEquals(ExprType.LONG, ExprTypeConversion.integerMathFunction(ExprType.LONG, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.LONG, ExprTypeConversion.integerMathFunction(ExprType.DOUBLE, ExprType.LONG));
+ Assert.assertEquals(ExprType.LONG, ExprTypeConversion.integerMathFunction(ExprType.DOUBLE, ExprType.DOUBLE));
// any string makes become string
- Assert.assertEquals(ExprType.STRING, ExprType.integerMathFunctionAutoTypeConversion(ExprType.LONG, ExprType.STRING));
- Assert.assertEquals(ExprType.STRING, ExprType.integerMathFunctionAutoTypeConversion(ExprType.STRING, ExprType.LONG));
- Assert.assertEquals(ExprType.STRING, ExprType.integerMathFunctionAutoTypeConversion(ExprType.DOUBLE, ExprType.STRING));
- Assert.assertEquals(ExprType.STRING, ExprType.integerMathFunctionAutoTypeConversion(ExprType.STRING, ExprType.DOUBLE));
- Assert.assertEquals(ExprType.STRING, ExprType.integerMathFunctionAutoTypeConversion(ExprType.STRING, ExprType.STRING));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.integerMathFunction(ExprType.LONG, ExprType.STRING));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.integerMathFunction(ExprType.STRING, ExprType.LONG));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.integerMathFunction(ExprType.DOUBLE, ExprType.STRING));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.integerMathFunction(ExprType.STRING, ExprType.DOUBLE));
+ Assert.assertEquals(ExprType.STRING, ExprTypeConversion.integerMathFunction(ExprType.STRING, ExprType.STRING));
// unless it is an array
- Assert.assertEquals(ExprType.LONG_ARRAY, ExprType.integerMathFunctionAutoTypeConversion(ExprType.LONG_ARRAY, ExprType.LONG_ARRAY));
+ Assert.assertEquals(ExprType.LONG_ARRAY, ExprTypeConversion.integerMathFunction(ExprType.LONG_ARRAY, ExprType.LONG_ARRAY));
Assert.assertEquals(
ExprType.DOUBLE_ARRAY,
- ExprType.integerMathFunctionAutoTypeConversion(ExprType.DOUBLE_ARRAY, ExprType.DOUBLE_ARRAY)
+ ExprTypeConversion.integerMathFunction(ExprType.DOUBLE_ARRAY, ExprType.DOUBLE_ARRAY)
);
Assert.assertEquals(
ExprType.STRING_ARRAY,
- ExprType.integerMathFunctionAutoTypeConversion(ExprType.STRING_ARRAY, ExprType.STRING_ARRAY)
+ ExprTypeConversion.integerMathFunction(ExprType.STRING_ARRAY, ExprType.STRING_ARRAY)
);
}
@@ -466,21 +494,21 @@ public class OutputTypeTest extends InitializedNullHandlingTest
public void testAutoConversionArrayMismatchArrays()
{
expectedException.expect(IAE.class);
- ExprType.doubleMathFunctionAutoTypeConversion(ExprType.DOUBLE_ARRAY, ExprType.LONG_ARRAY);
+ ExprTypeConversion.doubleMathFunction(ExprType.DOUBLE_ARRAY, ExprType.LONG_ARRAY);
}
@Test
public void testAutoConversionArrayMismatchArrayScalar()
{
expectedException.expect(IAE.class);
- ExprType.doubleMathFunctionAutoTypeConversion(ExprType.DOUBLE_ARRAY, ExprType.LONG);
+ ExprTypeConversion.doubleMathFunction(ExprType.DOUBLE_ARRAY, ExprType.LONG);
}
@Test
public void testAutoConversionArrayMismatchScalarArray()
{
expectedException.expect(IAE.class);
- ExprType.doubleMathFunctionAutoTypeConversion(ExprType.STRING, ExprType.LONG_ARRAY);
+ ExprTypeConversion.doubleMathFunction(ExprType.STRING, ExprType.LONG_ARRAY);
}
private void assertOutputType(String expression, Expr.InputBindingTypes inputTypes, ExprType outputType)
diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlanner.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlanner.java
index dc6361e..45122f0 100644
--- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlanner.java
+++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlanner.java
@@ -150,8 +150,16 @@ public class ExpressionPlanner
outputType = expression.getOutputType(inspector);
}
- // if analysis, inferred output type, or implicit mapping is in play, output will be multi-valued
- if (analysis.isOutputArray() || ExprType.isArray(outputType) || ExpressionPlan.is(traits, ExpressionPlan.Trait.NEEDS_APPLIED)) {
+ // if analysis predicts output, or inferred output type is array, output will be multi-valued
+ if (analysis.isOutputArray() || ExprType.isArray(outputType)) {
+ traits.add(ExpressionPlan.Trait.NON_SCALAR_OUTPUT);
+
+ // single input mappable may not produce array output explicitly, only through implicit mapping
+ traits.remove(ExpressionPlan.Trait.SINGLE_INPUT_MAPPABLE);
+ }
+
+ // if implicit mapping is in play, output will be multi-valued but may still use SINGLE_INPUT_MAPPABLE optimization
+ if (ExpressionPlan.is(traits, ExpressionPlan.Trait.NEEDS_APPLIED)) {
traits.add(ExpressionPlan.Trait.NON_SCALAR_OUTPUT);
}
diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java
index 3260afa..4a7635c 100644
--- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java
+++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionVirtualColumn.java
@@ -175,6 +175,12 @@ public class ExpressionVirtualColumn implements VirtualColumn
// are unable to compute the output type of the expression, either due to incomplete type information of the
// inputs or because of unimplemented methods on expression implementations themselves, or, because a
// ColumnInspector is not available
+
+ // array types must not currently escape from the expression system
+ if (outputType != null && outputType.isArray()) {
+ return new ColumnCapabilitiesImpl().setType(ValueType.STRING).setHasMultipleValues(true);
+ }
+
return new ColumnCapabilitiesImpl().setType(outputType == null ? ValueType.FLOAT : outputType);
}
@@ -185,7 +191,8 @@ public class ExpressionVirtualColumn implements VirtualColumn
if (plan.getOutputType() != null) {
- if (outputType != null && ExprType.fromValueType(outputType) != plan.getOutputType()) {
+ final ExprType inferredOutputType = plan.getOutputType();
+ if (outputType != null && ExprType.fromValueType(outputType) != inferredOutputType) {
log.warn(
"Projected output type %s of expression %s does not match provided type %s",
plan.getOutputType(),
@@ -193,7 +200,6 @@ public class ExpressionVirtualColumn implements VirtualColumn
outputType
);
}
- final ExprType inferredOutputType = plan.getOutputType();
final ValueType valueType = ExprType.toValueType(inferredOutputType);
if (valueType.isNumeric()) {
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
index e456474..3ddddd9 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
@@ -37,7 +37,6 @@ import org.apache.druid.java.util.common.ISE;
import org.apache.druid.java.util.common.granularity.Granularity;
import org.apache.druid.math.expr.Expr;
import org.apache.druid.math.expr.ExprMacroTable;
-import org.apache.druid.math.expr.ExprType;
import org.apache.druid.math.expr.Parser;
import org.apache.druid.query.aggregation.PostAggregator;
import org.apache.druid.query.expression.TimestampFloorExprMacro;
@@ -54,7 +53,6 @@ import org.apache.druid.query.ordering.StringComparators;
import org.apache.druid.segment.VirtualColumn;
import org.apache.druid.segment.column.ColumnHolder;
import org.apache.druid.segment.column.RowSignature;
-import org.apache.druid.segment.column.ValueType;
import org.apache.druid.sql.calcite.filtration.BoundRefKey;
import org.apache.druid.sql.calcite.filtration.Bounds;
import org.apache.druid.sql.calcite.filtration.Filtration;
@@ -666,21 +664,6 @@ public class Expressions
: null;
}
- public static ExprType exprTypeForValueType(final ValueType valueType)
- {
- switch (valueType) {
- case LONG:
- return ExprType.LONG;
- case FLOAT:
- case DOUBLE:
- return ExprType.DOUBLE;
- case STRING:
- return ExprType.STRING;
- default:
- throw new ISE("No ExprType for valueType[%s]", valueType);
- }
- }
-
/**
* Converts an expression to a Granularity, if possible. This is possible if, and only if, the expression
* is a timestamp_floor function on the __time column with literal parameters for period, origin, and timeZone.
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ReductionOperatorConversionHelper.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ReductionOperatorConversionHelper.java
index 1356de9..3fa5f36 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ReductionOperatorConversionHelper.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/ReductionOperatorConversionHelper.java
@@ -24,6 +24,7 @@ import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.sql.type.SqlReturnTypeInference;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.druid.java.util.common.IAE;
+import org.apache.druid.math.expr.ExprTypeConversion;
import org.apache.druid.segment.column.ValueType;
import org.apache.druid.sql.calcite.planner.Calcites;
@@ -38,7 +39,7 @@ class ReductionOperatorConversionHelper
* https://dev.mysql.com/doc/refman/8.0/en/comparison-operators.html#function_least
*
* @see org.apache.druid.math.expr.Function.ReduceFunction#apply
- * @see org.apache.druid.math.expr.ExprType#doubleMathFunctionAutoTypeConversion
+ * @see ExprTypeConversion#doubleMathFunction
*/
static final SqlReturnTypeInference TYPE_INFERENCE =
opBinding -> {
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
index c8364fd..0f38dd7 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java
@@ -142,12 +142,15 @@ public class Calcites
} else if (sqlTypeName == SqlTypeName.ARRAY) {
SqlTypeName componentType = type.getComponentType().getSqlTypeName();
if (isDoubleType(componentType)) {
- return ValueType.DOUBLE_ARRAY;
+ // in the future return ValueType.DOUBLE_ARRAY;
+ return ValueType.STRING;
}
if (isLongType(componentType)) {
- return ValueType.LONG_ARRAY;
+ // in the future we will return ValueType.LONG_ARRAY;
+ return ValueType.STRING;
}
- return ValueType.STRING_ARRAY;
+ // in the future we will return ValueType.STRING_ARRAY;
+ return ValueType.STRING;
} else {
return null;
}
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/rel/QueryMaker.java b/sql/src/main/java/org/apache/druid/sql/calcite/rel/QueryMaker.java
index f8c3dc2..5528d91 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/rel/QueryMaker.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/rel/QueryMaker.java
@@ -307,11 +307,17 @@ public class QueryMaker
coercedValue = value.getClass().getName();
}
} else if (sqlType == SqlTypeName.ARRAY) {
- try {
- coercedValue = jsonMapper.writeValueAsString(value);
- }
- catch (IOException e) {
- throw new RuntimeException(e);
+ if (value instanceof String) {
+ coercedValue = NullHandling.nullToEmptyIfNeeded((String) value);
+ } else if (value instanceof NlsString) {
+ coercedValue = ((NlsString) value).getValue();
+ } else {
+ try {
+ coercedValue = jsonMapper.writeValueAsString(value);
+ }
+ catch (IOException e) {
+ throw new RuntimeException(e);
+ }
}
} else {
throw new ISE("Cannot coerce[%s] to %s", value.getClass().getName(), sqlType);
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 c232aa8..a87c951 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
@@ -14684,7 +14684,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
newScanQueryBuilder()
.dataSource(CalciteTests.DATASOURCE1)
.intervals(querySegmentSpec(Filtration.eternity()))
- .virtualColumns(expressionVirtualColumn("v0", "array(1,2)", ValueType.LONG_ARRAY))
+ .virtualColumns(expressionVirtualColumn("v0", "array(1,2)", ValueType.STRING))
.columns("dim1", "v0")
.resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
.limit(1)
@@ -14698,6 +14698,32 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
}
@Test
+ public void testGroupByArrayFromCase() throws Exception
+ {
+ cannotVectorize();
+ testQuery(
+ "SELECT CASE WHEN dim4 = 'a' THEN ARRAY['foo','bar','baz'] END as mv_value, count(1) from numfoo GROUP BY 1",
+ ImmutableList.of(
+ GroupByQuery.builder()
+ .setDataSource(CalciteTests.DATASOURCE3)
+ .setInterval(querySegmentSpec(Filtration.eternity()))
+ .setVirtualColumns(expressionVirtualColumn("v0", "case_searched((\"dim4\" == 'a'),array('foo','bar','baz'),null)", ValueType.STRING))
+ .setDimensions(new DefaultDimensionSpec("v0", "_d0"))
+ .setGranularity(Granularities.ALL)
+ .setAggregatorSpecs(new CountAggregatorFactory("a0"))
+ .setContext(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{null, 3L},
+ new Object[]{"bar", 3L},
+ new Object[]{"baz", 3L},
+ new Object[]{"foo", 3L}
+ )
+ );
+ }
+
+ @Test
public void testSelectNonConstantArrayExpressionFromTable() throws Exception
{
testQuery(
@@ -14706,7 +14732,7 @@ public class CalciteQueryTest extends BaseCalciteQueryTest
newScanQueryBuilder()
.dataSource(CalciteTests.DATASOURCE1)
.intervals(querySegmentSpec(Filtration.eternity()))
- .virtualColumns(expressionVirtualColumn("v0", "array(concat(\"dim1\",'word'),'up')", ValueType.STRING_ARRAY))
+ .virtualColumns(expressionVirtualColumn("v0", "array(concat(\"dim1\",'word'),'up')", ValueType.STRING))
.columns("dim1", "v0")
.resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
.limit(5)
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org