This is an automated email from the ASF dual-hosted git repository.
gian 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 2c46787d8c3 Fix filters on TIMESTAMP_TO_MILLIS over TIME_FLOOR.
(#17871)
2c46787d8c3 is described below
commit 2c46787d8c399b96f88dcf3a14c68b6753b12464
Author: Gian Merlino <[email protected]>
AuthorDate: Mon Apr 28 08:27:29 2025 -0700
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.
---
.../druid/sql/calcite/expression/Expressions.java | 15 +++++++++++----
.../apache/druid/sql/calcite/planner/Calcites.java | 8 ++++----
.../apache/druid/sql/calcite/CalciteQueryTest.java | 22 ++++++++++++++++++++++
3 files changed, 37 insertions(+), 8 deletions(-)
diff --git
a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
index 8ed39a7de95..cf9c5748670 100644
--- a/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
+++ b/sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java
@@ -648,12 +648,19 @@ public class Expressions
return null;
}
- // Special handling for filters on FLOOR(__time TO granularity).
+ // Special handling for filters like FLOOR(__time TO granularity).
final Granularity queryGranularity =
toQueryGranularity(lhsExpression,
plannerContext.getExpressionParser());
- if (queryGranularity != null) {
- // lhs is FLOOR(__time TO granularity); rhs must be a timestamp
- final long rhsMillis = Calcites.calciteDateTimeLiteralToJoda(rhs,
plannerContext.getTimeZone()).getMillis();
+ if (queryGranularity != null && !RexLiteral.isNullLiteral(rhs)) {
+ // lhs is a time-floor expression; rhs must be a timestamp or millis
+ final long rhsMillis;
+
+ if (rhs.getType().getSqlTypeName() == SqlTypeName.BIGINT) {
+ rhsMillis = ((Number) RexLiteral.value(rhs)).longValue();
+ } else {
+ rhsMillis = Calcites.calciteDateTimeLiteralToJoda(rhs,
plannerContext.getTimeZone()).getMillis();
+ }
+
return buildTimeFloorFilter(
ColumnHolder.TIME_COLUMN_NAME,
queryGranularity,
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 8da1c2dd0d4..42d6af58573 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
@@ -40,8 +40,8 @@ import org.apache.calcite.util.ConversionUtil;
import org.apache.calcite.util.DateString;
import org.apache.calcite.util.TimeString;
import org.apache.calcite.util.TimestampString;
+import org.apache.druid.error.DruidException;
import org.apache.druid.java.util.common.DateTimes;
-import org.apache.druid.java.util.common.IAE;
import org.apache.druid.java.util.common.StringUtils;
import org.apache.druid.math.expr.ExpressionProcessing;
import org.apache.druid.math.expr.ExpressionProcessingConfig;
@@ -449,8 +449,8 @@ public class Calcites
public static DateTime calciteDateTimeLiteralToJoda(final RexNode literal,
final DateTimeZone timeZone)
{
final SqlTypeName typeName = literal.getType().getSqlTypeName();
- if (literal.getKind() != SqlKind.LITERAL || (typeName !=
SqlTypeName.TIMESTAMP && typeName != SqlTypeName.DATE)) {
- throw new IAE("Expected literal but got[%s]", literal.getKind());
+ if (literal.getKind() != SqlKind.LITERAL) {
+ throw DruidException.defensive("Expected literal but got[%s]",
literal.getKind());
}
if (typeName == SqlTypeName.TIMESTAMP) {
@@ -460,7 +460,7 @@ public class Calcites
final DateString dateString = (DateString) RexLiteral.value(literal);
return
CALCITE_DATE_PARSER.parse(dateString.toString()).withZoneRetainFields(timeZone);
} else {
- throw new IAE("Expected TIMESTAMP or DATE but got[%s]", typeName);
+ throw DruidException.defensive("Expected TIMESTAMP or DATE but got[%s]",
typeName);
}
}
diff --git
a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
index a9faffec769..98759560eef 100644
--- a/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
+++ b/sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java
@@ -6106,6 +6106,28 @@ public class CalciteQueryTest extends
BaseCalciteQueryTest
);
}
+ @Test
+ public void testCountStarWithFloorTimeFilterUsingMilliseconds()
+ {
+ testQuery(
+ "SELECT COUNT(*) FROM druid.foo "
+ + "WHERE TIMESTAMP_TO_MILLIS(FLOOR(__time TO DAY)) >= 946684800000 AND
"
+ + "TIMESTAMP_TO_MILLIS(FLOOR(__time TO DAY)) < 978307200000",
+ ImmutableList.of(
+ Druids.newTimeseriesQueryBuilder()
+ .dataSource(CalciteTests.DATASOURCE1)
+
.intervals(querySegmentSpec(Intervals.of("2000-01-01/2001-01-01")))
+ .granularity(Granularities.ALL)
+ .aggregators(aggregators(new CountAggregatorFactory("a0")))
+ .context(QUERY_CONTEXT_DEFAULT)
+ .build()
+ ),
+ ImmutableList.of(
+ new Object[]{3L}
+ )
+ );
+ }
+
@Test
public void testCountStarWithMisalignedFloorTimeFilter()
{
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
