diff --git a/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java b/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java index 6b1c9177c..5193c5b53 100644 --- a/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java +++ b/core/src/main/java/io/substrait/expression/AbstractExpressionVisitor.java @@ -94,6 +94,11 @@ public OUTPUT visit(Expression.IntervalDayLiteral expr) throws EXCEPTION { return visitFallback(expr); } + @Override + public OUTPUT visit(Expression.IntervalCompoundLiteral expr) throws EXCEPTION { + return visitFallback(expr); + } + @Override public OUTPUT visit(Expression.UUIDLiteral expr) throws EXCEPTION { return visitFallback(expr); diff --git a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java index 6efc700a3..44a4aa24e 100644 --- a/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java +++ b/core/src/main/java/io/substrait/expression/proto/ProtoExpressionConverter.java @@ -341,8 +341,8 @@ public Expression.Literal from(io.substrait.proto.Expression.Literal literal) { literal.getIntervalYearToMonth().getYears(), literal.getIntervalYearToMonth().getMonths()); case INTERVAL_DAY_TO_SECOND -> { - // Handle deprecated version that doesn't provide precision and that uses microseconds instead - // of subseconds, for backwards compatibility + // Handle deprecated version that doesn't provide precision and that uses microseconds + // instead of subseconds, for backwards compatibility int precision = literal.getIntervalDayToSecond().hasPrecision() ? literal.getIntervalDayToSecond().getPrecision() diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index e4ce67009..1adf5b861 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -102,7 +102,7 @@ private Type toSubstrait(RelDataType type, List names) { INTERVAL_HOUR_SECOND, INTERVAL_MINUTE, INTERVAL_MINUTE_SECOND, - INTERVAL_SECOND -> creator.INTERVAL_DAY; + INTERVAL_SECOND -> creator.intervalDay(type.getScale()); case VARBINARY -> creator.BINARY; case BINARY -> creator.fixedBinary(type.getPrecision()); case MAP -> { 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 8f0cf24ec..9dd32e072 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java @@ -229,17 +229,17 @@ public RexNode visit(Expression.IntervalYearLiteral expr) throws RuntimeExceptio new BigDecimal(expr.years() * 12 + expr.months()), YEAR_MONTH_INTERVAL); } - private static final long MICROS_IN_DAY = TimeUnit.DAYS.toMicros(1); + private static final long MILLIS_IN_DAY = TimeUnit.DAYS.toMillis(1); @Override public RexNode visit(Expression.IntervalDayLiteral expr) throws RuntimeException { + long milliseconds = + expr.precision() > 3 + ? (expr.subseconds() / (int) Math.pow(10, expr.precision() - 3)) + : (expr.subseconds() * (int) Math.pow(10, 3 - expr.precision())); return rexBuilder.makeIntervalLiteral( // Current Calcite behavior is to store milliseconds since Epoch - // microseconds version: new BigDecimal(expr.days() * MICROS_IN_DAY + expr.seconds() * - // 100000L + expr.microseconds()), DAY_SECOND_INTERVAL); - new BigDecimal( - (expr.days() * MICROS_IN_DAY + expr.seconds() * 1_000_000L + expr.microseconds()) - / 1000L), + new BigDecimal((expr.days() * MILLIS_IN_DAY + expr.seconds() * 1_000L + milliseconds)), DAY_SECOND_INTERVAL); } diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java b/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java index a170268ff..34a2b591b 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/IgnoreNullableAndParameters.java @@ -85,7 +85,14 @@ public Boolean visit(Type.IntervalYear type) { @Override public Boolean visit(Type.IntervalDay type) { - return typeToMatch instanceof Type.IntervalDay; + return typeToMatch instanceof Type.IntervalDay + || typeToMatch instanceof ParameterizedType.IntervalDay; + } + + @Override + public Boolean visit(Type.IntervalCompound type) { + return typeToMatch instanceof Type.IntervalCompound + || typeToMatch instanceof ParameterizedType.IntervalCompound; } @Override @@ -171,6 +178,18 @@ public Boolean visit(ParameterizedType.Decimal expr) throws RuntimeException { return typeToMatch instanceof Type.Decimal || typeToMatch instanceof ParameterizedType.Decimal; } + @Override + public Boolean visit(ParameterizedType.IntervalDay expr) throws RuntimeException { + return typeToMatch instanceof Type.IntervalDay + || typeToMatch instanceof ParameterizedType.IntervalDay; + } + + @Override + public Boolean visit(ParameterizedType.IntervalCompound expr) throws RuntimeException { + return typeToMatch instanceof Type.IntervalCompound + || typeToMatch instanceof ParameterizedType.IntervalCompound; + } + @Override public Boolean visit(ParameterizedType.PrecisionTimestamp expr) throws RuntimeException { return typeToMatch instanceof Type.PrecisionTimestamp 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 433284b24..777ad33ea 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -172,6 +172,7 @@ public Expression.Literal convert(RexLiteral literal) { 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 = @@ -182,7 +183,7 @@ public Expression.Literal convert(RexLiteral literal) { 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, (int) microseconds); + yield intervalDay(n, (int) days, (int) seconds, microseconds, 6 /* micros */); } 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 5029b678f..d6279226f 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -211,7 +211,7 @@ void tIntervalMillisecond() { org.apache.calcite.avatica.util.TimeUnit.SECOND, 3, SqlParserPos.ZERO)); - var intervalDaySecondExpr = intervalDay(false, 3, 5 * 3600 + 7 * 60 + 9, 500_000); + var intervalDaySecondExpr = intervalDay(false, 3, 5 * 3600 + 7 * 60 + 9, 500_000, 6); bitest(intervalDaySecondExpr, intervalDaySecond); } diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index 20cbd962f..731c9fef4 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -118,7 +118,7 @@ void intervalYear(boolean nullable) { @ValueSource(booleans = {true, false}) void intervalDay(boolean nullable) { testType( - Type.withNullability(nullable).INTERVAL_DAY, + Type.withNullability(nullable).intervalDay(6), type.createSqlIntervalType(SubstraitTypeSystem.DAY_SECOND_INTERVAL), nullable); }