fix bug with CastOperatorConversion with types which cannot be mapped to native druid types (#17011)

This commit is contained in:
Clint Wylie 2024-09-06 17:07:32 -07:00 committed by GitHub
parent 48a758ee08
commit b0f36c1b89
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 83 additions and 43 deletions

View File

@ -91,10 +91,7 @@ public class ExpressionTypeConversion
return type; return type;
} }
if (type.isArray() || other.isArray()) { if (type.isArray() || other.isArray()) {
if (!Objects.equals(type, other)) { return leastRestrictiveType(type, other);
throw new Types.IncompatibleTypeException(type, other);
}
return type;
} }
if (type.is(ExprType.COMPLEX) || other.is(ExprType.COMPLEX)) { if (type.is(ExprType.COMPLEX) || other.is(ExprType.COMPLEX)) {
if (type.getComplexTypeName() == null) { if (type.getComplexTypeName() == null) {
@ -134,12 +131,8 @@ public class ExpressionTypeConversion
if (other == null) { if (other == null) {
return type; return type;
} }
// arrays cannot be auto converted
if (type.isArray() || other.isArray()) { if (type.isArray() || other.isArray()) {
if (!Objects.equals(type, other)) { return leastRestrictiveType(type, other);
throw new Types.IncompatibleTypeException(type, other);
}
return type;
} }
if (type.is(ExprType.COMPLEX) || other.is(ExprType.COMPLEX)) { if (type.is(ExprType.COMPLEX) || other.is(ExprType.COMPLEX)) {
if (type.getComplexTypeName() == null) { if (type.getComplexTypeName() == null) {

View File

@ -20,13 +20,10 @@
package org.apache.druid.math.expr; package org.apache.druid.math.expr;
import com.google.common.collect.ImmutableMap; 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.segment.column.ColumnType;
import org.apache.druid.testing.InitializedNullHandlingTest; import org.apache.druid.testing.InitializedNullHandlingTest;
import org.junit.Assert; import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test; import org.junit.Test;
import org.junit.rules.ExpectedException;
import java.util.Map; import java.util.Map;
@ -48,9 +45,6 @@ public class OutputTypeTest extends InitializedNullHandlingTest
.build() .build()
); );
@Rule
public ExpectedException expectedException = ExpectedException.none();
@Test @Test
public void testConstantsAndIdentifiers() public void testConstantsAndIdentifiers()
{ {
@ -541,7 +535,6 @@ public class OutputTypeTest extends InitializedNullHandlingTest
ExpressionType.DOUBLE, ExpressionType.DOUBLE,
ExpressionTypeConversion.operator(ExpressionType.LONG, ExpressionType.STRING) ExpressionTypeConversion.operator(ExpressionType.LONG, ExpressionType.STRING)
); );
// unless it is an array, and those have to be the same
Assert.assertEquals( Assert.assertEquals(
ExpressionType.LONG_ARRAY, ExpressionType.LONG_ARRAY,
ExpressionTypeConversion.operator(ExpressionType.LONG_ARRAY, ExpressionType.LONG_ARRAY) ExpressionTypeConversion.operator(ExpressionType.LONG_ARRAY, ExpressionType.LONG_ARRAY)
@ -554,7 +547,30 @@ public class OutputTypeTest extends InitializedNullHandlingTest
ExpressionType.STRING_ARRAY, ExpressionType.STRING_ARRAY,
ExpressionTypeConversion.operator(ExpressionType.STRING_ARRAY, 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); ExpressionType nested = ExpressionType.fromColumnType(ColumnType.NESTED_DATA);
Assert.assertEquals( Assert.assertEquals(
nested, nested,
@ -619,7 +635,6 @@ public class OutputTypeTest extends InitializedNullHandlingTest
ExpressionType.STRING, ExpressionType.STRING,
ExpressionTypeConversion.function(ExpressionType.STRING, ExpressionType.STRING) ExpressionTypeConversion.function(ExpressionType.STRING, ExpressionType.STRING)
); );
// unless it is an array, and those have to be the same
Assert.assertEquals( Assert.assertEquals(
ExpressionType.LONG_ARRAY, ExpressionType.LONG_ARRAY,
ExpressionTypeConversion.function(ExpressionType.LONG_ARRAY, ExpressionType.LONG_ARRAY) ExpressionTypeConversion.function(ExpressionType.LONG_ARRAY, ExpressionType.LONG_ARRAY)
@ -632,6 +647,30 @@ public class OutputTypeTest extends InitializedNullHandlingTest
ExpressionType.STRING_ARRAY, ExpressionType.STRING_ARRAY,
ExpressionTypeConversion.function(ExpressionType.STRING_ARRAY, 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); ExpressionType nested = ExpressionType.fromColumnType(ColumnType.NESTED_DATA);
Assert.assertEquals( Assert.assertEquals(
nested, 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) private void assertOutputType(String expression, Expr.InputBindingInspector inspector, ExpressionType outputType)
{ {
final Expr expr = Parser.parse(expression, ExprMacroTable.nil(), false); final Expr expr = Parser.parse(expression, ExprMacroTable.nil(), false);

View File

@ -93,11 +93,17 @@ public class CastOperatorConversion implements SqlOperatorConversion
: ExpressionType.fromColumnType(toDruidType); : ExpressionType.fromColumnType(toDruidType);
if (toExpressionType == null) { if (toExpressionType == null) {
// We have no runtime type for these SQL types. // We have no runtime type for to SQL type.
return null; return null;
} }
if (fromExpressionType == 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; final DruidExpression typeCastExpression;

View File

@ -218,6 +218,9 @@ public class Calcites
if (elementType != null) { if (elementType != null) {
return ColumnType.ofArray(elementType); return ColumnType.ofArray(elementType);
} }
if (type.getComponentType().getSqlTypeName() == SqlTypeName.NULL) {
return ColumnType.LONG_ARRAY;
}
return null; return null;
} else { } else {
return null; return null;

View File

@ -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 @Test
public void testSelectConstantArrayExpressionFromTable() 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}
)
);
}
} }