FrankChen021 commented on code in PR #19906:
URL: https://github.com/apache/druid/pull/19906#discussion_r3735950561
##########
sql/src/main/java/org/apache/druid/sql/calcite/expression/Expressions.java:
##########
@@ -371,7 +373,14 @@ public static DruidLiteral calciteLiteralToDruidLiteral(
retVal = new DruidLiteral(ExpressionType.DOUBLE, number == null ? null :
number.doubleValue());
} else if (SqlTypeFamily.INTERVAL_DAY_TIME == sqlTypeName.getFamily()) {
// Calcite represents DAY-TIME intervals in milliseconds.
- final long milliseconds = ((Number)
RexLiteral.value(rexNode)).longValue();
+ long milliseconds = ((Number) RexLiteral.value(rexNode)).longValue();
+ // Calcite has a known quirk where WEEK interval literals are stored as
1 hour in the
+ // INTERVAL_DAY_TIME family (see
https://github.com/apache/druid/issues/18665).
+ // Detect the WEEK qualifier and convert the value to 7 days.
+ final SqlIntervalQualifier intervalQualifier =
rexNode.getType().getIntervalQualifier();
+ if (intervalQualifier != null && intervalQualifier.getStartUnit() ==
TimeUnit.WEEK) {
+ milliseconds = milliseconds * 7 * 24;
Review Comment:
[P1] Avoid rescaling already-correct WEEK literals
This branch uses Calcite 1.41, which already converts WEEK intervals to
millisecond values. A narrow probe produced 604800000 for INTERVAL '1' WEEK;
multiplying by 7 * 24 changes it to 101606400000 ms, making existing quoted and
unquoted WEEK expressions 168 times too large. Apply the workaround only to the
pre-1.38 representation or remove it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
