This is an automated email from the ASF dual-hosted git repository.
cwylie 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 b0f36c1b89b fix bug with CastOperatorConversion with types which
cannot be mapped to native druid types (#17011)
b0f36c1b89b is described below
commit b0f36c1b89bced172d60d9f2188f2e054b7f2072
Author: Clint Wylie <[email protected]>
AuthorDate: Fri Sep 6 17:07:32 2024 -0700
fix bug with CastOperatorConversion with types which cannot be mapped to
native druid types (#17011)
---
.../druid/math/expr/ExpressionTypeConversion.java | 11 +--
.../org/apache/druid/math/expr/OutputTypeTest.java | 78 +++++++++++++---------
.../expression/builtin/CastOperatorConversion.java | 10 ++-
.../apache/druid/sql/calcite/planner/Calcites.java | 3 +
.../druid/sql/calcite/CalciteArraysQueryTest.java | 24 ++++++-
5 files changed, 83 insertions(+), 43 deletions(-)
diff --git
a/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
b/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
index efe0328a837..16ae5024830 100644
---
a/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
+++
b/processing/src/main/java/org/apache/druid/math/expr/ExpressionTypeConversion.java
@@ -91,10 +91,7 @@ public class ExpressionTypeConversion
return type;
}
if (type.isArray() || other.isArray()) {
- if (!Objects.equals(type, other)) {
- throw new Types.IncompatibleTypeException(type, other);
- }
- return type;
+ return leastRestrictiveType(type, other);
}
if (type.is(ExprType.COMPLEX) || other.is(ExprType.COMPLEX)) {
if (type.getComplexTypeName() == null) {
@@ -134,12 +131,8 @@ public class ExpressionTypeConversion
if (other == null) {
return type;
}
- // arrays cannot be auto converted
if (type.isArray() || other.isArray()) {
- if (!Objects.equals(type, other)) {
- throw new Types.IncompatibleTypeException(type, other);
- }
- return type;
+ return leastRestrictiveType(type, other);
}
if (type.is(ExprType.COMPLEX) || other.is(ExprType.COMPLEX)) {
if (type.getComplexTypeName() == null) {
diff --git
a/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
b/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
index d313749fef8..4ab7c7be0ed 100644
--- a/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
+++ b/processing/src/test/java/org/apache/druid/math/expr/OutputTypeTest.java
@@ -20,13 +20,10 @@
package org.apache.druid.math.expr;
import com.google.common.collect.ImmutableMap;
-import org.apache.druid.java.util.common.IAE;
import org.apache.druid.segment.column.ColumnType;
import org.apache.druid.testing.InitializedNullHandlingTest;
import org.junit.Assert;
-import org.junit.Rule;
import org.junit.Test;
-import org.junit.rules.ExpectedException;
import java.util.Map;
@@ -48,9 +45,6 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
.build()
);
- @Rule
- public ExpectedException expectedException = ExpectedException.none();
-
@Test
public void testConstantsAndIdentifiers()
{
@@ -541,7 +535,6 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
ExpressionType.DOUBLE,
ExpressionTypeConversion.operator(ExpressionType.LONG,
ExpressionType.STRING)
);
- // unless it is an array, and those have to be the same
Assert.assertEquals(
ExpressionType.LONG_ARRAY,
ExpressionTypeConversion.operator(ExpressionType.LONG_ARRAY,
ExpressionType.LONG_ARRAY)
@@ -554,7 +547,30 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
ExpressionType.STRING_ARRAY,
ExpressionTypeConversion.operator(ExpressionType.STRING_ARRAY,
ExpressionType.STRING_ARRAY)
);
-
+ Assert.assertEquals(
+ ExpressionType.LONG_ARRAY,
+ ExpressionTypeConversion.operator(ExpressionType.LONG_ARRAY,
ExpressionType.LONG)
+ );
+ Assert.assertEquals(
+ ExpressionType.STRING_ARRAY,
+ ExpressionTypeConversion.operator(ExpressionType.STRING,
ExpressionType.LONG_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.DOUBLE_ARRAY,
+ ExpressionTypeConversion.operator(ExpressionType.LONG_ARRAY,
ExpressionType.DOUBLE_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.DOUBLE_ARRAY,
+ ExpressionTypeConversion.operator(ExpressionType.LONG,
ExpressionType.DOUBLE_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.STRING_ARRAY,
+ ExpressionTypeConversion.operator(ExpressionType.LONG_ARRAY,
ExpressionType.STRING_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.STRING_ARRAY,
+ ExpressionTypeConversion.operator(ExpressionType.STRING_ARRAY,
ExpressionType.DOUBLE_ARRAY)
+ );
ExpressionType nested =
ExpressionType.fromColumnType(ColumnType.NESTED_DATA);
Assert.assertEquals(
nested,
@@ -619,7 +635,6 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
ExpressionType.STRING,
ExpressionTypeConversion.function(ExpressionType.STRING,
ExpressionType.STRING)
);
- // unless it is an array, and those have to be the same
Assert.assertEquals(
ExpressionType.LONG_ARRAY,
ExpressionTypeConversion.function(ExpressionType.LONG_ARRAY,
ExpressionType.LONG_ARRAY)
@@ -632,6 +647,30 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
ExpressionType.STRING_ARRAY,
ExpressionTypeConversion.function(ExpressionType.STRING_ARRAY,
ExpressionType.STRING_ARRAY)
);
+ Assert.assertEquals(
+ ExpressionType.DOUBLE_ARRAY,
+ ExpressionTypeConversion.function(ExpressionType.DOUBLE_ARRAY,
ExpressionType.LONG_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.STRING_ARRAY,
+ ExpressionTypeConversion.function(ExpressionType.DOUBLE_ARRAY,
ExpressionType.STRING_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.STRING_ARRAY,
+ ExpressionTypeConversion.function(ExpressionType.STRING_ARRAY,
ExpressionType.LONG_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.STRING_ARRAY,
+ ExpressionTypeConversion.function(ExpressionType.STRING,
ExpressionType.LONG_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.DOUBLE_ARRAY,
+ ExpressionTypeConversion.function(ExpressionType.LONG_ARRAY,
ExpressionType.DOUBLE_ARRAY)
+ );
+ Assert.assertEquals(
+ ExpressionType.DOUBLE_ARRAY,
+ ExpressionTypeConversion.function(ExpressionType.LONG,
ExpressionType.DOUBLE_ARRAY)
+ );
ExpressionType nested =
ExpressionType.fromColumnType(ColumnType.NESTED_DATA);
Assert.assertEquals(
nested,
@@ -719,27 +758,6 @@ public class OutputTypeTest extends
InitializedNullHandlingTest
);
}
- @Test
- public void testAutoConversionArrayMismatchArrays()
- {
- expectedException.expect(IAE.class);
- ExpressionTypeConversion.function(ExpressionType.DOUBLE_ARRAY,
ExpressionType.LONG_ARRAY);
- }
-
- @Test
- public void testAutoConversionArrayMismatchArrayScalar()
- {
- expectedException.expect(IAE.class);
- ExpressionTypeConversion.function(ExpressionType.DOUBLE_ARRAY,
ExpressionType.LONG);
- }
-
- @Test
- public void testAutoConversionArrayMismatchScalarArray()
- {
- expectedException.expect(IAE.class);
- ExpressionTypeConversion.function(ExpressionType.DOUBLE,
ExpressionType.LONG_ARRAY);
- }
-
private void assertOutputType(String expression, Expr.InputBindingInspector
inspector, ExpressionType outputType)
{
final Expr expr = Parser.parse(expression, ExprMacroTable.nil(), false);
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/CastOperatorConversion.java
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/CastOperatorConversion.java
index ac34abe48d1..d82d3ea9546 100644
---
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/CastOperatorConversion.java
+++
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/CastOperatorConversion.java
@@ -93,11 +93,17 @@ public class CastOperatorConversion implements
SqlOperatorConversion
:
ExpressionType.fromColumnType(toDruidType);
if (toExpressionType == null) {
- // We have no runtime type for these SQL types.
+ // We have no runtime type for to SQL type.
return null;
}
if (fromExpressionType == null) {
- return DruidExpression.ofLiteral(toDruidType,
DruidExpression.nullLiteral());
+ // Calcites.getColumnTypeForRelDataType returns null in cases of NULL,
but also any type which cannot be
+ // mapped to a native druid type. in the case of the former, make a
null literal of the toType
+ if (fromType.equals(SqlTypeName.NULL)) {
+ return DruidExpression.ofLiteral(toDruidType,
DruidExpression.nullLiteral());
+ }
+ // otherwise, we have no runtime type for from SQL type.
+ return null;
}
final DruidExpression typeCastExpression;
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 64929c6445e..6310b23543d 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
@@ -218,6 +218,9 @@ public class Calcites
if (elementType != null) {
return ColumnType.ofArray(elementType);
}
+ if (type.getComponentType().getSqlTypeName() == SqlTypeName.NULL) {
+ return ColumnType.LONG_ARRAY;
+ }
return null;
} else {
return null;
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java
index 2cd798e193a..d3453404cd9 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteArraysQueryTest.java
@@ -116,8 +116,6 @@ public class CalciteArraysQueryTest extends
BaseCalciteQueryTest
}
}
- // test some query stuffs, sort of limited since no native array column
types so either need to use constructor or
- // array aggregator
@Test
public void testSelectConstantArrayExpressionFromTable()
{
@@ -7478,4 +7476,26 @@ public class CalciteArraysQueryTest extends
BaseCalciteQueryTest
)
);
}
+
+ @Test
+ public void testNullArray()
+ {
+ testQuery(
+ "SELECT arrayLongNulls = ARRAY[null, null] FROM druid.arrays LIMIT 1",
+ ImmutableList.of(
+ newScanQueryBuilder()
+ .dataSource(CalciteTests.ARRAYS_DATASOURCE)
+ .intervals(querySegmentSpec(Filtration.eternity()))
+ .virtualColumns(expressionVirtualColumn("v0",
"(\"arrayLongNulls\" == array(null,null))", ColumnType.LONG))
+ .columns("v0")
+
.resultFormat(ScanQuery.ResultFormat.RESULT_FORMAT_COMPACTED_LIST)
+ .limit(1)
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{false}
+ )
+ );
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
