You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by ab...@apache.org on 2022/08/29 09:45:11 UTC
[druid] branch 24.0.0 updated: fix issue with SQL planner and null array constants (#12971) (#12985)
This is an automated email from the ASF dual-hosted git repository.
abhishek pushed a commit to branch 24.0.0
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/24.0.0 by this push:
new e2f7038485 fix issue with SQL planner and null array constants (#12971) (#12985)
e2f7038485 is described below
commit e2f703848595a1cb8821297978fef5572408b319
Author: Clint Wylie <cw...@apache.org>
AuthorDate: Mon Aug 29 02:45:04 2022 -0700
fix issue with SQL planner and null array constants (#12971) (#12985)
---
.../org/apache/druid/math/expr/FunctionTest.java | 16 ++++-
.../calcite/expression/OperatorConversions.java | 42 +++++++++---
.../MultiValueStringToArrayOperatorConversion.java | 2 +-
.../builtin/NestedDataOperatorConversions.java | 4 +-
.../builtin/StringToArrayOperatorConversion.java | 2 +-
.../sql/calcite/planner/DruidRexExecutor.java | 62 ++++++++---------
.../sql/calcite/CalciteNestedDataQueryTest.java | 78 ++++++++++++++++++++++
.../druid/sql/calcite/CalciteSelectQueryTest.java | 4 +-
8 files changed, 162 insertions(+), 48 deletions(-)
diff --git a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
index 03d2e50d1c..26deec240c 100644
--- a/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
+++ b/core/src/test/java/org/apache/druid/math/expr/FunctionTest.java
@@ -347,12 +347,13 @@ public class FunctionTest extends InitializedNullHandlingTest
public void testStringToArray()
{
assertArrayExpr("string_to_array('1,2,3', ',')", new String[]{"1", "2", "3"});
+ assertArrayExpr("string_to_array(null, ',')", null);
assertArrayExpr("string_to_array('1', ',')", new String[]{"1"});
assertArrayExpr("string_to_array(array_to_string(a, ','), ',')", new String[]{"foo", "bar", "baz", "foobar"});
}
@Test
- public void testArrayCast()
+ public void testArrayCastLegacy()
{
assertArrayExpr("cast([1, 2, 3], 'STRING_ARRAY')", new String[]{"1", "2", "3"});
assertArrayExpr("cast([1, 2, 3], 'DOUBLE_ARRAY')", new Double[]{1.0, 2.0, 3.0});
@@ -364,6 +365,19 @@ public class FunctionTest extends InitializedNullHandlingTest
assertArrayExpr("cast(['1.0', '2.0', '3.0'], 'LONG_ARRAY')", new Long[]{1L, 2L, 3L});
}
+ @Test
+ public void testArrayCast()
+ {
+ assertArrayExpr("cast([1, 2, 3], 'ARRAY<STRING>')", new String[]{"1", "2", "3"});
+ assertArrayExpr("cast([1, 2, 3], 'ARRAY<DOUBLE>')", new Double[]{1.0, 2.0, 3.0});
+ assertArrayExpr("cast(c, 'ARRAY<LONG>')", new Long[]{3L, 4L, 5L});
+ assertArrayExpr(
+ "cast(string_to_array(array_to_string(b, ','), ','), 'ARRAY<LONG>')",
+ new Long[]{1L, 2L, 3L, 4L, 5L}
+ );
+ assertArrayExpr("cast(['1.0', '2.0', '3.0'], 'ARRAY<LONG>')", new Long[]{1L, 2L, 3L});
+ }
+
@Test
public void testArraySlice()
{
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
index 978460f099..c1b0b1b81f 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java
@@ -318,8 +318,8 @@ public class OperatorConversions
* operator should never, ever, return null.
*
* One of {@link #returnTypeNonNull}, {@link #returnTypeNullable}, {@link #returnTypeCascadeNullable(SqlTypeName)}
- * {@link #returnTypeNullableArray}, or {@link #returnTypeInference(SqlReturnTypeInference)} must be used before
- * calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
+ * {@link #returnTypeNullableArrayWithNullableElements}, or {@link #returnTypeInference(SqlReturnTypeInference)}
+ * must be used before calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
*/
public OperatorBuilder returnTypeNonNull(final SqlTypeName typeName)
{
@@ -335,8 +335,8 @@ public class OperatorConversions
* Sets the return type of the operator to "typeName", marked as nullable.
*
* One of {@link #returnTypeNonNull}, {@link #returnTypeNullable}, {@link #returnTypeCascadeNullable(SqlTypeName)}
- * {@link #returnTypeNullableArray}, or {@link #returnTypeInference(SqlReturnTypeInference)} must be used before
- * calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
+ * {@link #returnTypeNullableArrayWithNullableElements}, or {@link #returnTypeInference(SqlReturnTypeInference)}
+ * must be used before calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
*/
public OperatorBuilder returnTypeNullable(final SqlTypeName typeName)
{
@@ -352,8 +352,8 @@ public class OperatorConversions
* Sets the return type of the operator to "typeName", marked as nullable if any of its operands are nullable.
*
* One of {@link #returnTypeNonNull}, {@link #returnTypeNullable}, {@link #returnTypeCascadeNullable(SqlTypeName)}
- * {@link #returnTypeNullableArray}, or {@link #returnTypeInference(SqlReturnTypeInference)} must be used before
- * calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
+ * {@link #returnTypeNullableArrayWithNullableElements}, or {@link #returnTypeInference(SqlReturnTypeInference)}
+ * must be used before calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
*/
public OperatorBuilder returnTypeCascadeNullable(final SqlTypeName typeName)
{
@@ -366,10 +366,10 @@ public class OperatorConversions
* Sets the return type of the operator to an array type with elements of "typeName", marked as nullable.
*
* One of {@link #returnTypeNonNull}, {@link #returnTypeNullable}, {@link #returnTypeCascadeNullable(SqlTypeName)}
- * {@link #returnTypeNullableArray}, or {@link #returnTypeInference(SqlReturnTypeInference)} must be used before
- * calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
+ * {@link #returnTypeArrayWithNullableElements}, or {@link #returnTypeInference(SqlReturnTypeInference)} must be
+ * used before calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
*/
- public OperatorBuilder returnTypeNullableArray(final SqlTypeName elementTypeName)
+ public OperatorBuilder returnTypeArrayWithNullableElements(final SqlTypeName elementTypeName)
{
Preconditions.checkState(this.returnTypeInference == null, "Cannot set return type multiple times");
@@ -379,13 +379,33 @@ public class OperatorConversions
return this;
}
+ /**
+ * Sets the return type of the operator to an array type with elements of "typeName", marked as nullable.
+ *
+ * One of {@link #returnTypeNonNull}, {@link #returnTypeNullable}, {@link #returnTypeCascadeNullable(SqlTypeName)}
+ * {@link #returnTypeArrayWithNullableElements}, or {@link #returnTypeInference(SqlReturnTypeInference)} must be
+ * used before calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
+ */
+ public OperatorBuilder returnTypeNullableArrayWithNullableElements(final SqlTypeName elementTypeName)
+ {
+ this.returnTypeInference = ReturnTypes.cascade(
+ opBinding -> Calcites.createSqlArrayTypeWithNullability(
+ opBinding.getTypeFactory(),
+ elementTypeName,
+ true
+ ),
+ SqlTypeTransforms.FORCE_NULLABLE
+ );
+ return this;
+ }
+
/**
* Provides customized return type inference logic.
*
* One of {@link #returnTypeNonNull}, {@link #returnTypeNullable}, {@link #returnTypeCascadeNullable(SqlTypeName)}
- * {@link #returnTypeNullableArray}, or {@link #returnTypeInference(SqlReturnTypeInference)} must be used before
- * calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
+ * {@link #returnTypeNullableArrayWithNullableElements}, or {@link #returnTypeInference(SqlReturnTypeInference)}
+ * must be used before calling {@link #build()}. These methods cannot be mixed; you must call exactly one.
*/
public OperatorBuilder returnTypeInference(final SqlReturnTypeInference returnTypeInference)
{
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/MultiValueStringToArrayOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/MultiValueStringToArrayOperatorConversion.java
index 1f5fd10656..f8245ddeed 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/MultiValueStringToArrayOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/MultiValueStringToArrayOperatorConversion.java
@@ -38,7 +38,7 @@ public class MultiValueStringToArrayOperatorConversion extends DirectOperatorCon
.operatorBuilder("MV_TO_ARRAY")
.operandTypeChecker(OperandTypes.family(SqlTypeFamily.STRING))
.functionCategory(SqlFunctionCategory.STRING)
- .returnTypeNullableArray(SqlTypeName.VARCHAR)
+ .returnTypeNullableArrayWithNullableElements(SqlTypeName.VARCHAR)
.build();
public MultiValueStringToArrayOperatorConversion()
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/NestedDataOperatorConversions.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/NestedDataOperatorConversions.java
index 89cad74142..045f4dc4d3 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/NestedDataOperatorConversions.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/NestedDataOperatorConversions.java
@@ -165,7 +165,7 @@ public class NestedDataOperatorConversions
.operatorBuilder("JSON_PATHS")
.operandTypeChecker(OperandTypes.ANY)
.functionCategory(SqlFunctionCategory.USER_DEFINED_FUNCTION)
- .returnTypeNullableArray(SqlTypeName.VARCHAR)
+ .returnTypeArrayWithNullableElements(SqlTypeName.VARCHAR)
.build();
@Override
@@ -207,7 +207,7 @@ public class NestedDataOperatorConversions
)
)
.functionCategory(SqlFunctionCategory.USER_DEFINED_FUNCTION)
- .returnTypeNullableArray(SqlTypeName.VARCHAR)
+ .returnTypeNullableArrayWithNullableElements(SqlTypeName.VARCHAR)
.build();
@Override
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/StringToArrayOperatorConversion.java b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/StringToArrayOperatorConversion.java
index a95c55443c..087aada4a2 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/StringToArrayOperatorConversion.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/StringToArrayOperatorConversion.java
@@ -40,7 +40,7 @@ public class StringToArrayOperatorConversion extends DirectOperatorConversion
)
)
.functionCategory(SqlFunctionCategory.STRING)
- .returnTypeNullableArray(SqlTypeName.VARCHAR)
+ .returnTypeNullableArrayWithNullableElements(SqlTypeName.VARCHAR)
.build();
public StringToArrayOperatorConversion()
diff --git a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
index dfa068d847..7ab6fd0b04 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
@@ -34,9 +34,9 @@ import org.apache.druid.sql.calcite.expression.DruidExpression;
import org.apache.druid.sql.calcite.expression.Expressions;
import java.math.BigDecimal;
+import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
-import java.util.stream.Collectors;
/**
* A Calcite {@code RexExecutor} that reduces Calcite expressions by evaluating them using Druid's own built-in
@@ -130,7 +130,7 @@ public class DruidRexExecutor implements RexExecutor
double exprResultDouble = exprResult.asDouble();
if (Double.isNaN(exprResultDouble) || Double.isInfinite(exprResultDouble)) {
String expression = druidExpression.getExpression();
- throw new UnsupportedSQLQueryException("'%s' evaluates to '%s' that is not supported in SQL. You can either cast the expression as bigint ('cast(%s as bigint)') or char ('cast(%s as char)') or change the expression itself",
+ throw new UnsupportedSQLQueryException("'%s' evaluates to '%s' that is not supported in SQL. You can either cast the expression as BIGINT ('CAST(%s as BIGINT)') or VARCHAR ('CAST(%s as VARCHAR)') or change the expression itself",
expression,
Double.toString(exprResultDouble),
expression,
@@ -142,40 +142,42 @@ public class DruidRexExecutor implements RexExecutor
}
} else if (sqlTypeName == SqlTypeName.ARRAY) {
assert exprResult.isArray();
- if (SqlTypeName.NUMERIC_TYPES.contains(constExp.getType().getComponentType().getSqlTypeName())) {
+ final Object[] array = exprResult.asArray();
+ if (array == null) {
+ literal = rexBuilder.makeNullLiteral(constExp.getType());
+ } else if (SqlTypeName.NUMERIC_TYPES.contains(constExp.getType().getComponentType().getSqlTypeName())) {
if (exprResult.type().getElementType().is(ExprType.LONG)) {
- List<BigDecimal> resultAsBigDecimalList = Arrays.stream(exprResult.asArray())
- .map(val -> {
- final Number longVal = (Number) val;
- if (longVal == null) {
- return null;
- }
- return BigDecimal.valueOf(longVal.longValue());
- })
- .collect(Collectors.toList());
+ List<BigDecimal> resultAsBigDecimalList = new ArrayList<>(array.length);
+ for (Object val : array) {
+ final Number longVal = (Number) val;
+ if (longVal == null) {
+ resultAsBigDecimalList.add(null);
+ } else {
+ resultAsBigDecimalList.add(BigDecimal.valueOf(longVal.longValue()));
+ }
+ }
literal = rexBuilder.makeLiteral(resultAsBigDecimalList, constExp.getType(), true);
} else {
- List<BigDecimal> resultAsBigDecimalList = Arrays.stream(exprResult.asArray()).map(
- val -> {
- final Number doubleVal = (Number) val;
- if (doubleVal == null) {
- return null;
- }
- if (Double.isNaN(doubleVal.doubleValue()) || Double.isInfinite(doubleVal.doubleValue())) {
- String expression = druidExpression.getExpression();
- throw new UnsupportedSQLQueryException(
- "'%s' contains an element that evaluates to '%s' which is not supported in SQL. You can either cast the element in the array to bigint or char or change the expression itself",
- expression,
- Double.toString(doubleVal.doubleValue())
- );
- }
- return BigDecimal.valueOf(doubleVal.doubleValue());
- }
- ).collect(Collectors.toList());
+ List<BigDecimal> resultAsBigDecimalList = new ArrayList<>(array.length);
+ for (Object val : array) {
+ final Number doubleVal = (Number) val;
+ if (doubleVal == null) {
+ resultAsBigDecimalList.add(null);
+ } else if (Double.isNaN(doubleVal.doubleValue()) || Double.isInfinite(doubleVal.doubleValue())) {
+ String expression = druidExpression.getExpression();
+ throw new UnsupportedSQLQueryException(
+ "'%s' contains an element that evaluates to '%s' which is not supported in SQL. You can either cast the element in the ARRAY to BIGINT or VARCHAR or change the expression itself",
+ expression,
+ Double.toString(doubleVal.doubleValue())
+ );
+ } else {
+ resultAsBigDecimalList.add(BigDecimal.valueOf(doubleVal.doubleValue()));
+ }
+ }
literal = rexBuilder.makeLiteral(resultAsBigDecimalList, constExp.getType(), true);
}
} else {
- literal = rexBuilder.makeLiteral(Arrays.asList(exprResult.asArray()), constExp.getType(), true);
+ literal = rexBuilder.makeLiteral(Arrays.asList(array), constExp.getType(), true);
}
} else if (sqlTypeName == SqlTypeName.OTHER) {
// complex constant is not reducible, so just leave it as an expression
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
index 8a05ee74f8..eb4cf5ba7b 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteNestedDataQueryTest.java
@@ -2506,4 +2506,82 @@ public class CalciteNestedDataQueryTest extends BaseCalciteQueryTest
);
}
+
+ @Test
+ public void testJsonPathsNonJsonInput()
+ {
+ testQuery(
+ "SELECT JSON_PATHS(string), JSON_PATHS(1234), JSON_PATHS('1234'), JSON_PATHS(1.1), JSON_PATHS(null)\n"
+ + "FROM druid.nested",
+ ImmutableList.of(
+ Druids.newScanQueryBuilder()
+ .dataSource(DATA_SOURCE)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(
+ expressionVirtualColumn("v0", "json_paths(\"string\")", ColumnType.STRING_ARRAY),
+ expressionVirtualColumn("v1", "array('$')", ColumnType.STRING_ARRAY)
+ )
+ .columns("v0", "v1")
+ .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+ .legacy(false)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{"[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]"},
+ new Object[]{"[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]"},
+ new Object[]{"[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]"},
+ new Object[]{"[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]"},
+ new Object[]{"[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]"},
+ new Object[]{"[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]"},
+ new Object[]{"[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]", "[\"$\"]"}
+ ),
+ RowSignature.builder()
+ .add("EXPR$0", ColumnType.STRING_ARRAY)
+ .add("EXPR$1", ColumnType.STRING_ARRAY)
+ .add("EXPR$2", ColumnType.STRING_ARRAY)
+ .add("EXPR$3", ColumnType.STRING_ARRAY)
+ .add("EXPR$4", ColumnType.STRING_ARRAY)
+ .build()
+
+ );
+ }
+
+ @Test
+ public void testJsonKeysNonJsonInput()
+ {
+ testQuery(
+ "SELECT JSON_KEYS(string, '$'), JSON_KEYS(1234, '$'), JSON_KEYS('1234', '$'), JSON_KEYS(1.1, '$'), JSON_KEYS(null, '$')\n"
+ + "FROM druid.nested",
+ ImmutableList.of(
+ Druids.newScanQueryBuilder()
+ .dataSource(DATA_SOURCE)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(
+ expressionVirtualColumn("v0", "json_keys(\"string\",'$')", ColumnType.STRING_ARRAY),
+ expressionVirtualColumn("v1", "null", ColumnType.STRING_ARRAY)
+ )
+ .columns("v0", "v1")
+ .resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+ .legacy(false)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{null, null, null, null, null},
+ new Object[]{null, null, null, null, null},
+ new Object[]{null, null, null, null, null},
+ new Object[]{null, null, null, null, null},
+ new Object[]{null, null, null, null, null},
+ new Object[]{null, null, null, null, null},
+ new Object[]{null, null, null, null, null}
+ ),
+ RowSignature.builder()
+ .add("EXPR$0", ColumnType.STRING_ARRAY)
+ .add("EXPR$1", ColumnType.STRING_ARRAY)
+ .add("EXPR$2", ColumnType.STRING_ARRAY)
+ .add("EXPR$3", ColumnType.STRING_ARRAY)
+ .add("EXPR$4", ColumnType.STRING_ARRAY)
+ .build()
+
+ );
+ }
}
diff --git a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java
index a50e7212ed..0982f17807 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteSelectQueryTest.java
@@ -313,7 +313,7 @@ public class CalciteSelectQueryTest extends BaseCalciteQueryTest
public void testSelectConstantExpressionEquivalentToNaN()
{
expectedException.expectMessage(
- "'(log10(0) - log10(0))' evaluates to 'NaN' that is not supported in SQL. You can either cast the expression as bigint ('cast((log10(0) - log10(0)) as bigint)') or char ('cast((log10(0) - log10(0)) as char)') or change the expression itself");
+ "'(log10(0) - log10(0))' evaluates to 'NaN' that is not supported in SQL. You can either cast the expression as BIGINT ('CAST((log10(0) - log10(0)) as BIGINT)') or VARCHAR ('CAST((log10(0) - log10(0)) as VARCHAR)') or change the expression itself");
testQuery(
"SELECT log10(0) - log10(0), dim1 FROM foo LIMIT 1",
ImmutableList.of(),
@@ -325,7 +325,7 @@ public class CalciteSelectQueryTest extends BaseCalciteQueryTest
public void testSelectConstantExpressionEquivalentToInfinity()
{
expectedException.expectMessage(
- "'log10(0)' evaluates to '-Infinity' that is not supported in SQL. You can either cast the expression as bigint ('cast(log10(0) as bigint)') or char ('cast(log10(0) as char)') or change the expression itself");
+ "'log10(0)' evaluates to '-Infinity' that is not supported in SQL. You can either cast the expression as BIGINT ('CAST(log10(0) as BIGINT)') or VARCHAR ('CAST(log10(0) as VARCHAR)') or change the expression itself");
testQuery(
"SELECT log10(0), dim1 FROM foo LIMIT 1",
ImmutableList.of(),
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org