fix(isthmus): sql day/time interval conversion

RexLiteral day/time intervals were converted to Substrait using their
scale. Despite all day/time interval RexLiteral created from SQL by
Calcite having a (default) scale of 6, their value is always in
milliseconds. This resulted in intervals generated by Calcite from SQL
being wrong by a factor of 1000 seconds.

This change treats day/time interval RexLiteral values as milliseconds
regardless of their claimed scale.

Signed-off-by: Mark S. Lewis <Mark.S.Lewis@outlook.com>

isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java

@@ -97,10 +97,8 @@ public RexNode visit(Expression.NullLiteral expr) throws RuntimeException {
   @Override
   public RexNode visit(Expression.UserDefinedLiteral expr) throws RuntimeException {
     var binaryLiteral = rexBuilder.makeBinaryLiteral(new ByteString(expr.value().toByteArray()));
-    return rexBuilder.makeReinterpretCast(
-        typeConverter.toCalcite(typeFactory, expr.getType()),
-        binaryLiteral,
-        rexBuilder.makeLiteral(false));
+    var type = typeConverter.toCalcite(typeFactory, expr.getType());
+    return rexBuilder.makeReinterpretCast(type, binaryLiteral, rexBuilder.makeLiteral(false));
   }
 
   @Override

isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java

@@ -15,8 +15,8 @@
 import java.time.format.DateTimeFormatter;
 import java.time.format.DateTimeFormatterBuilder;
 import java.util.List;
+import java.util.Objects;
 import java.util.Optional;
-import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import org.apache.calcite.rel.type.RelDataType;
 import org.apache.calcite.rex.RexLiteral;
@@ -32,8 +32,6 @@ public LiteralConverter(TypeConverter typeConverter) {
     this.typeConverter = typeConverter;
   }
 
-  private static final long MICROS_IN_DAY = TimeUnit.DAYS.toMicros(1);
-
   static final DateTimeFormatter CALCITE_LOCAL_DATE_FORMATTER = DateTimeFormatter.ISO_LOCAL_DATE;
   static final DateTimeFormatter CALCITE_LOCAL_TIME_FORMATTER =
       new DateTimeFormatterBuilder()
@@ -96,16 +94,17 @@ public Expression.Literal convert(RexLiteral literal) {
       case SMALLINT -> i16(n, i(literal).intValue());
       case INTEGER -> i32(n, i(literal).intValue());
       case BIGINT -> i64(n, i(literal).longValue());
-      case BOOLEAN -> bool(n, (Boolean) literal.getValue());
+      case BOOLEAN -> bool(n, literal.getValueAs(Boolean.class));
       case CHAR -> {
         var val = literal.getValue();
         if (val instanceof NlsString nls) {
           yield fixedChar(n, nls.getValue());
         }
         throw new UnsupportedOperationException("Unable to handle char type: " + val);
       }
-      case FLOAT, DOUBLE -> fp64(n, ((Number) literal.getValue()).doubleValue());
-      case REAL -> fp32(n, ((Number) literal.getValue()).floatValue());
+      case FLOAT, DOUBLE -> fp64(n, literal.getValueAs(Double.class));
+      case REAL -> fp32(n, literal.getValueAs(Float.class));
+
       case DECIMAL -> {
         BigDecimal bd = bd(literal);
         yield decimal(n, bd, literal.getType().getPrecision(), literal.getType().getScale());
@@ -155,7 +154,7 @@ public Expression.Literal convert(RexLiteral literal) {
         yield timestamp(n, ldt);
       }
       case INTERVAL_YEAR, INTERVAL_YEAR_MONTH, INTERVAL_MONTH -> {
-        var intervalLength = literal.getValueAs(BigDecimal.class).longValue();
+        long intervalLength = Objects.requireNonNull(literal.getValueAs(Long.class));
         var years = intervalLength / 12;
         var months = intervalLength - years * 12;
         yield intervalYear(n, (int) years, (int) months);
@@ -170,19 +169,15 @@ public Expression.Literal convert(RexLiteral literal) {
           INTERVAL_MINUTE,
           INTERVAL_MINUTE_SECOND,
           INTERVAL_SECOND -> {
-        // we need to convert to microseconds.
-        // TODO: don't need to anymore
-        int scale = literal.getType().getScale();
-        var intervalLength = literal.getValueAs(BigDecimal.class).longValue();
-        var adjustedLength =
-            scale > 6
-                ? intervalLength / ((int) Math.pow(10, scale - 6))
-                : intervalLength * ((int) Math.pow(10, 6 - scale));
-        var days = adjustedLength / MICROS_IN_DAY;
-        var totalMicroseconds = adjustedLength - days * MICROS_IN_DAY;
-        var seconds = totalMicroseconds / 1_000_000;
-        var microseconds = totalMicroseconds - 1_000_000 * seconds;
-        yield intervalDay(n, (int) days, (int) seconds, microseconds, 6 /* micros */);
+        // Calcite represents day/time intervals in milliseconds, despite a default scale of 6.
+        var totalMillis = Objects.requireNonNull(literal.getValueAs(Long.class));
+        var interval = Duration.ofMillis(totalMillis);
+
+        var days = interval.toDays();
+        var seconds = interval.minusDays(days).toSeconds();
+        var micros = interval.toMillisPart() * 1000;
+
+        yield intervalDay(n, (int) days, (int) seconds, micros, 6);
       }
 
       case ROW -> {

isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java

@@ -215,6 +215,33 @@ void tIntervalMillisecond() {
     bitest(intervalDaySecondExpr, intervalDaySecond);
   }
 
+  @Test
+  void tIntervalDay() {
+    // Calcite always uses milliseconds
+    BigDecimal bd = new BigDecimal(TimeUnit.DAYS.toMillis(5));
+    RexLiteral intervalDayLiteral =
+        rex.makeIntervalLiteral(
+            bd,
+            new SqlIntervalQualifier(
+                org.apache.calcite.avatica.util.TimeUnit.DAY,
+                -1,
+                null,
+                -1,
+                SqlParserPos.ZERO));
+    var intervalDayExpr = intervalDay(false, 5, 0, 0, 6);
+
+    // rex --> expression
+    var convertedExpr = intervalDayLiteral.accept(rexExpressionConverter);
+    assertEquals(intervalDayExpr, convertedExpr);
+
+    // expression -> rex
+    RexLiteral convertedRex = (RexLiteral) intervalDayExpr.accept(expressionRexConverter);
+
+    // Compare value only. Ignore the precision in SqlIntervalQualifier in comparison.
+    assertEquals(
+        intervalDayLiteral.getValueAs(BigDecimal.class), convertedRex.getValueAs(BigDecimal.class));
+  }
+
   @Test
   void tIntervalYear() {
     BigDecimal bd = new BigDecimal(123 * 12); // '123' year(3)