Jackie-Jiang commented on code in PR #19788:
URL: https://github.com/apache/pinot/pull/19788#discussion_r4265398018
##########
pinot-query-planner/src/main/java/org/apache/pinot/calcite/rel/rules/PinotEvaluateLiteralRule.java:
##########
@@ -251,6 +264,112 @@ private static RexNode
evaluateLiteralOnlyFunction(RexCall rexCall, RexBuilder r
}
}
+ /// Returns whether the call is `datetime + interval` (Calcite's
`DATETIME_PLUS`) or `datetime - interval`
+ /// (`MINUS_DATE`) with a `TIMESTAMP` or `DATE` result. Calcite puts the
interval second for both.
+ private static boolean isDatetimeIntervalArithmetic(RexCall rexCall) {
+ SqlKind kind = rexCall.getKind();
+ SqlTypeName resultType = rexCall.getType().getSqlTypeName();
+ List<RexNode> operands = rexCall.getOperands();
+ return (kind == SqlKind.PLUS || kind == SqlKind.MINUS) && (resultType ==
SqlTypeName.TIMESTAMP
+ || resultType == SqlTypeName.DATE) && operands.size() == 2 &&
SqlTypeUtil.isInterval(operands.get(1).getType());
+ }
+
+ /// Evaluates `datetime + interval` or `datetime - interval` with the same
semantics as Calcite. It cannot use Pinot's
+ /// `plus` / `minus` functions, because Calcite stores a day-time interval
as milliseconds but a year-month interval
+ /// as months: they would add `INTERVAL '1' MONTH` as 1 millisecond. Months
are added with the same calendar
+ /// arithmetic as `TIMESTAMPADD(MONTH, ...)`, so `2019-01-31 + INTERVAL '1'
MONTH` is `2019-02-28`. Like Calcite, only
+ /// the whole days of a day-time interval are added to a `DATE`.
+ private static RexNode evaluateDatetimeIntervalArithmetic(RexCall rexCall,
RexBuilder rexBuilder) {
+ RexLiteral datetime = (RexLiteral) rexCall.getOperands().get(0);
+ RexLiteral interval = (RexLiteral) rexCall.getOperands().get(1);
+ RelDataType resultType = rexCall.getType();
+ if (datetime.isNull() || interval.isNull()) {
+ return rexBuilder.makeNullLiteral(resultType);
+ }
+ Object resultValue;
+ try {
+ long millis =
Objects.requireNonNull(datetime.getValueAs(Calendar.class)).getTimeInMillis();
+ // Number of months for a year-month interval, number of milliseconds
for a day-time interval
+ long intervalValue =
Objects.requireNonNull(interval.getValueAs(BigDecimal.class)).longValueExact();
+ if (rexCall.getKind() == SqlKind.MINUS) {
+ intervalValue = Math.negateExact(intervalValue);
+ }
+ boolean isDate = resultType.getSqlTypeName() == SqlTypeName.DATE;
+ long resultMillis;
+ if
(SqlTypeName.YEAR_INTERVAL_TYPES.contains(interval.getType().getSqlTypeName()))
{
+ resultMillis = DateTimeFunctions.timestampAdd("MONTH", intervalValue,
millis);
+ } else {
+ if (isDate) {
+ intervalValue = intervalValue / DateTimeUtils.MILLIS_PER_DAY *
DateTimeUtils.MILLIS_PER_DAY;
+ }
+ resultMillis = Math.addExact(millis, intervalValue);
+ }
+ resultValue = isDate ? DateString.fromDaysSinceEpoch(
+ Math.toIntExact(Math.floorDiv(resultMillis,
DateTimeUtils.MILLIS_PER_DAY))) : resultMillis;
+ } catch (Exception e) {
+ throw new SqlCompilationException(
+ "Caught exception while evaluating: " + rexCall + ": " +
e.getMessage(), e);
+ }
+ return rexBuilder.makeLiteral(resultValue, resultType, false);
+ }
+
+ /// Returns whether the call is `+`, `-`, `*` or `/` with an interval result
and only interval or numeric operands,
+ /// e.g. `INTERVAL '1' MONTH * 2`.
+ private static boolean isIntervalArithmetic(RexCall rexCall) {
+ SqlKind kind = rexCall.getKind();
+ List<RexNode> operands = rexCall.getOperands();
+ return (kind == SqlKind.PLUS || kind == SqlKind.MINUS || kind ==
SqlKind.TIMES || kind == SqlKind.DIVIDE)
+ && SqlTypeUtil.isInterval(rexCall.getType()) && operands.size() == 2
&& operands.stream()
+ .allMatch(operand -> SqlTypeUtil.isInterval(operand.getType()) ||
SqlTypeUtil.isNumeric(operand.getType()));
+ }
+
+ /// Evaluates interval arithmetic exactly. Pinot's `times` / `divide`
functions compute in DOUBLE, so
+ /// `INTERVAL '100' MONTH * 0.29` would be 28.999999999999996 months instead
of 29, and `INTERVAL '1' DAY * 0.7` would
+ /// be 1 millisecond short.
+ private static RexNode evaluateIntervalArithmetic(RexCall rexCall,
RexBuilder rexBuilder) {
+ RexLiteral left = (RexLiteral) rexCall.getOperands().get(0);
+ RexLiteral right = (RexLiteral) rexCall.getOperands().get(1);
+ RelDataType resultType = rexCall.getType();
+ if (left.isNull() || right.isNull()) {
+ return rexBuilder.makeNullLiteral(resultType);
+ }
+ BigDecimal resultValue;
+ try {
+ // An interval literal holds its number of months or milliseconds
+ BigDecimal leftValue =
Objects.requireNonNull(left.getValueAs(BigDecimal.class));
+ BigDecimal rightValue =
Objects.requireNonNull(right.getValueAs(BigDecimal.class));
Review Comment:
Non-blocking precision edge case: for a DOUBLE literal, Calcite's
`getValueAs(BigDecimal.class)` uses `MathContext.DECIMAL64`, so it rounds
`1.9999999999999998` to `2` before the interval arithmetic runs.
```sql
SELECT TIMESTAMP '2019-01-01 00:00:00.000'
+ INTERVAL '1' SECOND * 1.9999999999999998E0
FROM a
```
This now folds to `2019-01-01 00:00:02.000`; both the original base and the
previous PR revision fold to `2019-01-01 00:00:01.999`. The same issue occurs
with `CAST(1.9999999999999998 AS DOUBLE)`. Please preserve the numeric
operand's precision when converting it to BigDecimal and add a regression case
around this millisecond boundary. Confirmed with isolated planner probes
against all three versions.
--
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]
