diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java index 1b58cfb5..35ae1577 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/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 diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java index ace2f9f7..47f63908 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/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,7 +94,7 @@ 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) { @@ -104,8 +102,9 @@ public Expression.Literal convert(RexLiteral literal) { } 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 -> { diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index d6279226..0e3e7072 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/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)