Skip to content

Commit 2c46787

Browse files
authored
Fix filters on TIMESTAMP_TO_MILLIS over TIME_FLOOR. (#17871)
* Fix filters on TIMESTAMP_TO_MILLIS over TIME_FLOOR. It is possible for toQueryGranularity to return nonnull for BIGINT rhs, in the case where the time floor expression is wrapped in TIMESTAMP_TO_MILLIS. Prior to this patch, that would throw an (incorrect) error about expecting a literal. This patch fixes the defensive error to have the correct message (in Calcites) and also fixes the filter translation code to not hit that defensive check (in Expressions). * Fix error message.
1 parent f1c6f26 commit 2c46787

File tree

3 files changed

+37
-8
lines changed

3 files changed

+37
-8
lines changed

sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java

+11-4
Original file line numberDiff line numberDiff line change
@@ -648,12 +648,19 @@ private static DimFilter toSimpleLeafFilter(
648648
return null;
649649
}
650650

651-
// Special handling for filters on FLOOR(__time TO granularity).
651+
// Special handling for filters like FLOOR(__time TO granularity).
652652
final Granularity queryGranularity =
653653
toQueryGranularity(lhsExpression, plannerContext.getExpressionParser());
654-
if (queryGranularity != null) {
655-
// lhs is FLOOR(__time TO granularity); rhs must be a timestamp
656-
final long rhsMillis = Calcites.calciteDateTimeLiteralToJoda(rhs, plannerContext.getTimeZone()).getMillis();
654+
if (queryGranularity != null && !RexLiteral.isNullLiteral(rhs)) {
655+
// lhs is a time-floor expression; rhs must be a timestamp or millis
656+
final long rhsMillis;
657+
658+
if (rhs.getType().getSqlTypeName() == SqlTypeName.BIGINT) {
659+
rhsMillis = ((Number) RexLiteral.value(rhs)).longValue();
660+
} else {
661+
rhsMillis = Calcites.calciteDateTimeLiteralToJoda(rhs, plannerContext.getTimeZone()).getMillis();
662+
}
663+
657664
return buildTimeFloorFilter(
658665
ColumnHolder.TIME_COLUMN_NAME,
659666
queryGranularity,

sql/src/main/java/org/apache/druid/sql/calcite/planner/Calcites.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@
4040
import org.apache.calcite.util.DateString;
4141
import org.apache.calcite.util.TimeString;
4242
import org.apache.calcite.util.TimestampString;
43+
import org.apache.druid.error.DruidException;
4344
import org.apache.druid.java.util.common.DateTimes;
44-
import org.apache.druid.java.util.common.IAE;
4545
import org.apache.druid.java.util.common.StringUtils;
4646
import org.apache.druid.math.expr.ExpressionProcessing;
4747
import org.apache.druid.math.expr.ExpressionProcessingConfig;
@@ -449,8 +449,8 @@ public static DateString jodaToCalciteDateString(final DateTime dateTime, final
449449
public static DateTime calciteDateTimeLiteralToJoda(final RexNode literal, final DateTimeZone timeZone)
450450
{
451451
final SqlTypeName typeName = literal.getType().getSqlTypeName();
452-
if (literal.getKind() != SqlKind.LITERAL || (typeName != SqlTypeName.TIMESTAMP && typeName != SqlTypeName.DATE)) {
453-
throw new IAE("Expected literal but got[%s]", literal.getKind());
452+
if (literal.getKind() != SqlKind.LITERAL) {
453+
throw DruidException.defensive("Expected literal but got[%s]", literal.getKind());
454454
}
455455

456456
if (typeName == SqlTypeName.TIMESTAMP) {
@@ -460,7 +460,7 @@ public static DateTime calciteDateTimeLiteralToJoda(final RexNode literal, final
460460
final DateString dateString = (DateString) RexLiteral.value(literal);
461461
return CALCITE_DATE_PARSER.parse(dateString.toString()).withZoneRetainFields(timeZone);
462462
} else {
463-
throw new IAE("Expected TIMESTAMP or DATE but got[%s]", typeName);
463+
throw DruidException.defensive("Expected TIMESTAMP or DATE but got[%s]", typeName);
464464
}
465465
}
466466

sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java

+22
Original file line numberDiff line numberDiff line change
@@ -6106,6 +6106,28 @@ public void testCountStarWithFloorTimeFilter()
61066106
);
61076107
}
61086108

6109+
@Test
6110+
public void testCountStarWithFloorTimeFilterUsingMilliseconds()
6111+
{
6112+
testQuery(
6113+
"SELECT COUNT(*) FROM druid.foo "
6114+
+ "WHERE TIMESTAMP_TO_MILLIS(FLOOR(__time TO DAY)) >= 946684800000 AND "
6115+
+ "TIMESTAMP_TO_MILLIS(FLOOR(__time TO DAY)) < 978307200000",
6116+
ImmutableList.of(
6117+
Druids.newTimeseriesQueryBuilder()
6118+
.dataSource(CalciteTests.DATASOURCE1)
6119+
.intervals(querySegmentSpec(Intervals.of("2000-01-01/2001-01-01")))
6120+
.granularity(Granularities.ALL)
6121+
.aggregators(aggregators(new CountAggregatorFactory("a0")))
6122+
.context(QUERY_CONTEXT_DEFAULT)
6123+
.build()
6124+
),
6125+
ImmutableList.of(
6126+
new Object[]{3L}
6127+
)
6128+
);
6129+
}
6130+
61096131
@Test
61106132
public void testCountStarWithMisalignedFloorTimeFilter()
61116133
{

0 commit comments

Comments
 (0)