SQL: Fix nullable DATE, TIMESTAMP reduction. (#16915)

Reduction of nullable DATE and TIMESTAMP expressions did not perform
a necessary null check, so would in some cases reduce to
1970-01-01 00:00:00 (epoch) rather than NULL.
This commit is contained in:
Gian Merlino 2024-08-16 22:41:12 -07:00 committed by GitHub
parent 422183ee70
commit 806649f8af
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 103 additions and 24 deletions

View File

@ -90,33 +90,41 @@ public class DruidRexExecutor implements RexExecutor
literal = rexBuilder.makeLiteral(exprResult.asBoolean(), constExp.getType(), true);
}
} else if (sqlTypeName == SqlTypeName.DATE) {
// It is possible for an expression to have a non-null String value but it can return null when parsed
// as a primitive long/float/double.
// ExprEval.isNumericNull checks whether the parsed primitive value is null or not.
if (!constExp.getType().isNullable() && exprResult.isNumericNull()) {
if (exprResult.isNumericNull()) {
if (constExp.getType().isNullable()) {
literal = rexBuilder.makeNullLiteral(constExp.getType());
} else {
// There can be implicit casts of VARCHAR to TIMESTAMP where the VARCHAR is an invalid timestamp, but the
// TIMESTAMP type is not nullable. In this case it's best to throw an error, since it likely means the
// user's SQL query contains an invalid literal.
throw InvalidSqlInput.exception("Illegal DATE constant [%s]", constExp);
}
} else {
literal = rexBuilder.makeDateLiteral(
Calcites.jodaToCalciteDateString(
DateTimes.utc(exprResult.asLong()),
plannerContext.getTimeZone()
)
);
}
} else if (sqlTypeName == SqlTypeName.TIMESTAMP) {
// It is possible for an expression to have a non-null String value but it can return null when parsed
// as a primitive long/float/double.
// ExprEval.isNumericNull checks whether the parsed primitive value is null or not.
if (!constExp.getType().isNullable() && exprResult.isNumericNull()) {
if (exprResult.isNumericNull()) {
if (constExp.getType().isNullable()) {
literal = rexBuilder.makeNullLiteral(constExp.getType());
} else {
// There can be implicit casts of VARCHAR to TIMESTAMP where the VARCHAR is an invalid timestamp, but the
// TIMESTAMP type is not nullable. In this case it's best to throw an error, since it likely means the
// user's SQL query contains an invalid literal.
throw InvalidSqlInput.exception("Illegal TIMESTAMP constant [%s]", constExp);
}
} else {
literal = Calcites.jodaToCalciteTimestampLiteral(
rexBuilder,
DateTimes.utc(exprResult.asLong()),
plannerContext.getTimeZone(),
constExp.getType().getPrecision()
);
}
} else if (SqlTypeName.NUMERIC_TYPES.contains(sqlTypeName)) {
final BigDecimal bigDecimal;

View File

@ -37,6 +37,7 @@ import org.apache.calcite.sql.type.BasicSqlType;
import org.apache.calcite.sql.type.SqlTypeFactoryImpl;
import org.apache.calcite.sql.type.SqlTypeFamily;
import org.apache.calcite.sql.type.SqlTypeName;
import org.apache.druid.java.util.common.DateTimes;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.segment.column.ColumnType;
import org.apache.druid.segment.column.RowSignature;
@ -46,6 +47,7 @@ import org.apache.druid.sql.calcite.expression.DruidExpression;
import org.apache.druid.sql.calcite.expression.Expressions;
import org.apache.druid.sql.calcite.expression.OperatorConversions;
import org.apache.druid.sql.calcite.expression.builtin.MultiValueStringOperatorConversions;
import org.apache.druid.sql.calcite.expression.builtin.TimeParseOperatorConversion;
import org.apache.druid.sql.calcite.schema.DruidSchema;
import org.apache.druid.sql.calcite.schema.DruidSchemaCatalog;
import org.apache.druid.sql.calcite.schema.NamedDruidSchema;
@ -57,6 +59,7 @@ import org.apache.druid.sql.calcite.util.CalciteTests;
import org.apache.druid.sql.hook.DruidHookDispatcher;
import org.apache.druid.testing.InitializedNullHandlingTest;
import org.easymock.EasyMock;
import org.joda.time.DateTimeZone;
import org.junit.Assert;
import org.junit.Test;
@ -139,6 +142,74 @@ public class DruidRexExecutorTest extends InitializedNullHandlingTest
Assert.assertEquals(new BigDecimal(30L), ((RexLiteral) reduced.get(0)).getValue());
}
@Test
public void testCastDateReduced()
{
// CAST('2010-01-01' AS DATE)
RexNode call = rexBuilder.makeCall(
rexBuilder.getTypeFactory().createSqlType(SqlTypeName.DATE),
SqlStdOperatorTable.CAST,
Collections.singletonList(rexBuilder.makeLiteral("2010-01-01"))
);
DruidRexExecutor rexy = new DruidRexExecutor(PLANNER_CONTEXT);
List<RexNode> reduced = new ArrayList<>();
rexy.reduce(rexBuilder, ImmutableList.of(call), reduced);
Assert.assertEquals(1, reduced.size());
Assert.assertEquals(SqlKind.LITERAL, reduced.get(0).getKind());
Assert.assertEquals(
rexBuilder.makeDateLiteral(
Calcites.jodaToCalciteDateString(
DateTimes.of("2010-01-01"),
DateTimeZone.UTC
)
),
reduced.get(0)
);
}
@Test
public void testTimeParseReduced()
{
// TIME_PARSE('2010-01-01T02:03:04Z')
RexNode call = rexBuilder.makeCall(
new TimeParseOperatorConversion().calciteOperator(),
rexBuilder.makeLiteral("2010-01-01T02:03:04Z")
);
DruidRexExecutor rexy = new DruidRexExecutor(PLANNER_CONTEXT);
List<RexNode> reduced = new ArrayList<>();
rexy.reduce(rexBuilder, ImmutableList.of(call), reduced);
Assert.assertEquals(1, reduced.size());
Assert.assertEquals(SqlKind.LITERAL, reduced.get(0).getKind());
Assert.assertEquals(
Calcites.jodaToCalciteTimestampLiteral(
rexBuilder,
DateTimes.of("2010-01-01T02:03:04Z"),
DateTimeZone.UTC,
DruidTypeSystem.DEFAULT_TIMESTAMP_PRECISION
),
reduced.get(0)
);
}
@Test
public void testTimeParseUnparseableReduced()
{
// TIME_PARSE('not a timestamp')
RexNode call = rexBuilder.makeCall(
new TimeParseOperatorConversion().calciteOperator(),
rexBuilder.makeLiteral("not a timestamp")
);
DruidRexExecutor rexy = new DruidRexExecutor(PLANNER_CONTEXT);
List<RexNode> reduced = new ArrayList<>();
rexy.reduce(rexBuilder, ImmutableList.of(call), reduced);
Assert.assertEquals(1, reduced.size());
Assert.assertEquals(SqlKind.LITERAL, reduced.get(0).getKind());
Assert.assertTrue(RexLiteral.isNullLiteral(reduced.get(0)));
}
@Test
public void testComplexNotReduced()
{