Skip to content

Commit

Permalink
SQL: Fix nullable DATE, TIMESTAMP reduction. (#16915)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@gianm
gianm authored Aug 17, 2024
1 parent 422183e commit 806649f
Show file tree
Hide file tree
Showing 2 changed files with 103 additions and 24 deletions.
56 changes: 32 additions & 24 deletions sql/src/main/java/org/apache/druid/sql/calcite/planner/DruidRexExecutor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,33 +90,41 @@ public void reduce(
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()) {
throw InvalidSqlInput.exception("Illegal DATE constant [%s]", constExp);
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()
)
);
}

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()) {
throw InvalidSqlInput.exception("Illegal TIMESTAMP constant [%s]", constExp);
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()
);
}

literal = Calcites.jodaToCalciteTimestampLiteral(
rexBuilder,
DateTimes.utc(exprResult.asLong()),
plannerContext.getTimeZone(),
constExp.getType().getPrecision()
);
} else if (SqlTypeName.NUMERIC_TYPES.contains(sqlTypeName)) {
final BigDecimal bigDecimal;

Expand Down
71 changes: 71 additions & 0 deletions sql/src/test/java/org/apache/druid/sql/calcite/planner/DruidRexExecutorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
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;
Expand All @@ -46,6 +47,7 @@
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;
Expand All @@ -57,6 +59,7 @@
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;

Expand Down Expand Up @@ -139,6 +142,74 @@ public void testLongsReduced()
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()
{
Expand Down

0 comments on commit 806649f

Please sign in to comment.