From a4d3319b3db9492590aeecb40201b994eec16a69 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Tue, 3 Mar 2026 12:58:50 +0100 Subject: [PATCH 1/2] feat(isthmus): migrate to PrecisionTime / PrecisionTimestamp Signed-off-by: Niels Pardon --- .../expression/ExpressionCreator.java | 39 +++++++++++++++++++ .../io/substrait/isthmus/TypeConverter.java | 2 +- .../expression/ExpressionRexConverter.java | 35 ++++++++++++++--- .../isthmus/expression/LiteralConverter.java | 15 +++++-- .../io/substrait/isthmus/CalciteCallTest.java | 2 +- .../substrait/isthmus/CalciteLiteralTest.java | 23 ++++++++--- .../io/substrait/isthmus/CalciteTypeTest.java | 2 +- .../isthmus/ExpressionConvertabilityTest.java | 3 +- .../isthmus/FunctionConversionTest.java | 6 +-- 9 files changed, 103 insertions(+), 24 deletions(-) diff --git a/core/src/main/java/io/substrait/expression/ExpressionCreator.java b/core/src/main/java/io/substrait/expression/ExpressionCreator.java index 8f276d142..9203796af 100644 --- a/core/src/main/java/io/substrait/expression/ExpressionCreator.java +++ b/core/src/main/java/io/substrait/expression/ExpressionCreator.java @@ -10,6 +10,7 @@ import java.nio.ByteBuffer; import java.time.Instant; import java.time.LocalDateTime; +import java.time.LocalTime; import java.time.ZoneOffset; import java.util.Arrays; import java.util.List; @@ -89,6 +90,23 @@ public static Expression.PrecisionTimeLiteral precisionTime( .build(); } + /** + * Creates a precision time literal from a LocalTime value. + * + *

This method converts a Java {@link LocalTime} to a Substrait precision time literal with + * nanosecond precision (precision = 9). The time value is represented as nanoseconds since + * midnight (00:00:00). + * + * @param nullable whether the literal can be null + * @param value the LocalTime value to convert + * @return a PrecisionTimeLiteral with nanosecond precision representing the given time + * @see #precisionTime(boolean, long, int) for creating precision time with custom precision + */ + public static Expression.PrecisionTimeLiteral precisionTime(boolean nullable, LocalTime value) { + long epochNano = value.toNanoOfDay(); + return precisionTime(nullable, epochNano, 9); + } + /** * @deprecated Timestamp is deprecated in favor of PrecisionTimestamp */ @@ -155,6 +173,27 @@ public static Expression.PrecisionTimestampLiteral precisionTimestamp( .build(); } + /** + * Creates a precision timestamp literal from a LocalDateTime value. + * + *

This method converts a Java {@link LocalDateTime} to a Substrait precision timestamp literal + * with nanosecond precision (precision = 9). The timestamp value is represented as nanoseconds + * since the Unix epoch (1970-01-01 00:00:00 UTC), assuming the LocalDateTime is in UTC timezone. + * + * @param nullable whether the literal can be null + * @param value the LocalDateTime value to convert (interpreted as UTC) + * @return a PrecisionTimestampLiteral with nanosecond precision representing the given timestamp + * @see #precisionTimestamp(boolean, long, int) for creating precision timestamp with custom + * precision + */ + public static Expression.PrecisionTimestampLiteral precisionTimestamp( + boolean nullable, LocalDateTime value) { + long epochNano = + TimeUnit.SECONDS.toNanos(value.toEpochSecond(ZoneOffset.UTC)) + + value.toLocalTime().getNano(); + return precisionTimestamp(nullable, epochNano, 9); + } + public static Expression.PrecisionTimestampTZLiteral precisionTimestampTZ( boolean nullable, long value, int precision) { return Expression.PrecisionTimestampTZLiteral.builder() diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index 932b8f6d8..53b610044 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -106,7 +106,7 @@ private Type toSubstrait(RelDataType type, List names) { case DATE: return creator.DATE; case TIME: - return creator.TIME; + return creator.precisionTime(type.getPrecision()); case TIMESTAMP: return creator.precisionTimestamp(type.getPrecision()); case TIMESTAMP_WITH_LOCAL_TIME_ZONE: 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 5fd9efdcb..db91537a0 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java @@ -53,6 +53,7 @@ import org.apache.calcite.sql.SqlOperator; import org.apache.calcite.sql.fun.SqlStdOperatorTable; import org.apache.calcite.sql.parser.SqlParserPos; +import org.apache.calcite.sql.type.SqlTypeName; import org.apache.calcite.util.TimeString; import org.apache.calcite.util.TimestampString; @@ -212,9 +213,16 @@ public RexNode visit(Expression.TimeLiteral expr, Context context) throws Runtim @Override public RexNode visit(PrecisionTimeLiteral expr, Context context) throws RuntimeException { - return rexBuilder.makeLiteral( - createTimeString(expr.value(), expr.precision()), - typeConverter.toCalcite(typeFactory, expr.getType())); + int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIME); + if (expr.precision() > maxPrecision) { + throw new UnsupportedOperationException( + String.format( + "unsupported precision_time precision %s, max precision in Calcite type system is set to %s", + expr.precision(), maxPrecision)); + } + + return rexBuilder.makeTimeLiteral( + createTimeString(expr.value(), expr.precision()), expr.precision()); } protected TimeString createTimeString(long value, int precision) { @@ -278,13 +286,28 @@ public RexNode visit(TimestampTZLiteral expr, Context context) throws RuntimeExc @Override public RexNode visit(PrecisionTimestampLiteral expr, Context context) throws RuntimeException { - return rexBuilder.makeLiteral( - getTimestampString(expr.value(), expr.precision()), - typeConverter.toCalcite(typeFactory, expr.getType())); + int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); + if (expr.precision() > maxPrecision) { + throw new UnsupportedOperationException( + String.format( + "unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s", + expr.precision(), maxPrecision)); + } + + return rexBuilder.makeTimestampLiteral( + getTimestampString(expr.value(), expr.precision()), expr.precision()); } @Override public RexNode visit(PrecisionTimestampTZLiteral expr, Context context) throws RuntimeException { + int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_TZ); + if (expr.precision() > maxPrecision) { + throw new UnsupportedOperationException( + String.format( + "unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s", + expr.precision(), maxPrecision)); + } + return rexBuilder.makeLiteral( getTimestampString(expr.value(), expr.precision()), typeConverter.toCalcite(typeFactory, expr.getType())); 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 2e1cad6ca..fbaf2b3fb 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -170,16 +170,23 @@ public Expression.Literal convert(RexLiteral literal, boolean nullable) { { TimeString time = literal.getValueAs(TimeString.class); LocalTime localTime = LocalTime.parse(time.toString(), CALCITE_LOCAL_TIME_FORMATTER); - return ExpressionCreator.time( - nullable, TimeUnit.NANOSECONDS.toMicros(localTime.toNanoOfDay())); + // Calcite supports up to microsecond precision (6), convert nanoseconds to microseconds + long nanos = localTime.toNanoOfDay(); + long micros = TimeUnit.NANOSECONDS.toMicros(nanos); + return ExpressionCreator.precisionTime(nullable, micros, 6); } case TIMESTAMP: case TIMESTAMP_WITH_LOCAL_TIME_ZONE: { TimestampString timestamp = literal.getValueAs(TimestampString.class); - LocalDateTime ldt = + LocalDateTime localDateTime = LocalDateTime.parse(timestamp.toString(), CALCITE_LOCAL_DATETIME_FORMATTER); - return ExpressionCreator.timestamp(nullable, ldt); + // Calcite supports up to microsecond precision (6), convert nanoseconds to microseconds + long epochNanos = + TimeUnit.SECONDS.toNanos(localDateTime.toEpochSecond(java.time.ZoneOffset.UTC)) + + localDateTime.toLocalTime().getNano(); + long epochMicros = TimeUnit.NANOSECONDS.toMicros(epochNanos); + return ExpressionCreator.precisionTimestamp(nullable, epochMicros, 6); } case INTERVAL_YEAR: case INTERVAL_YEAR_MONTH: diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java index fe87d5fa1..d03239408 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteCallTest.java @@ -35,7 +35,7 @@ class CalciteCallTest extends CalciteObjs { @Test void extract() { test( - "extract:req_ts", + "extract:req_pts", rex.makeCall( t(SqlTypeName.INTEGER), SqlStdOperatorTable.EXTRACT, diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index 2b29d89d0..c3360f873 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -8,7 +8,7 @@ import io.substrait.expression.Expression.IntervalDayLiteral; import io.substrait.expression.Expression.IntervalYearLiteral; import io.substrait.expression.Expression.Literal; -import io.substrait.expression.Expression.TimestampLiteral; +import io.substrait.expression.Expression.PrecisionTimestampLiteral; import io.substrait.expression.ExpressionCreator; import io.substrait.extension.DefaultExtensionCatalog; import io.substrait.extension.SimpleExtension; @@ -21,6 +21,8 @@ import java.math.BigDecimal; import java.nio.charset.StandardCharsets; import java.time.LocalDate; +import java.time.LocalDateTime; +import java.time.ZoneOffset; import java.util.Arrays; import java.util.Collections; import java.util.List; @@ -106,7 +108,7 @@ void tBinary() { @Test void tTime() { bitest( - ExpressionCreator.time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000), + ExpressionCreator.precisionTime(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000, 6), rex.makeTimeLiteral(new TimeString(14, 22, 47), 6)); } @@ -122,7 +124,8 @@ void tTimeWithMicroSecond() { new TimeString("14:22:47.123456")); bitest( - ExpressionCreator.time(false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000 + 123456), + ExpressionCreator.precisionTime( + false, (14L * 60 * 60 + 22 * 60 + 47) * 1000 * 1000 + 123456, 6), rex.makeTimeLiteral(new TimeString("14:22:47.123456"), 6)); } @@ -142,15 +145,23 @@ void tDate() { @Test void tTimestamp() { - TimestampLiteral ts = ExpressionCreator.timestamp(false, 2002, 2, 14, 16, 20, 47, 123); + long epochMicro = + TimeUnit.SECONDS.toMicros( + LocalDateTime.of(2002, 2, 14, 16, 20, 47).toEpochSecond(ZoneOffset.UTC)) + + 123; + PrecisionTimestampLiteral ts = ExpressionCreator.precisionTimestamp(false, epochMicro, 6); int nano = (int) TimeUnit.MICROSECONDS.toNanos(123); TimestampString tsx = new TimestampString(2002, 2, 14, 16, 20, 47).withNanos(nano); bitest(ts, rex.makeTimestampLiteral(tsx, 6)); } @Test - void tTimestampWithMilliMacroSeconds() { - TimestampLiteral ts = ExpressionCreator.timestamp(false, 2002, 2, 14, 16, 20, 47, 123456); + void tTimestampWithMilliMicroSeconds() { + long epochMicro = + TimeUnit.SECONDS.toMicros( + LocalDateTime.of(2002, 2, 14, 16, 20, 47).toEpochSecond(ZoneOffset.UTC)) + + 123456; + PrecisionTimestampLiteral ts = ExpressionCreator.precisionTimestamp(false, epochMicro, 6); int nano = (int) TimeUnit.MICROSECONDS.toNanos(123456); TimestampString tsx = new TimestampString(2002, 2, 14, 16, 20, 47).withNanos(nano); bitest(ts, rex.makeTimestampLiteral(tsx, 6)); diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index 176552f7d..f0fead6de 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -104,7 +104,7 @@ void date(boolean nullable) { @ParameterizedTest @ValueSource(booleans = {true, false}) void time(boolean nullable) { - testType(Type.withNullability(nullable).TIME, SqlTypeName.TIME, nullable, 6); + testType(Type.withNullability(nullable).precisionTime(6), SqlTypeName.TIME, nullable, 6); } @ParameterizedTest diff --git a/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java b/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java index 00316b5be..cb1ae6076 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java @@ -154,7 +154,6 @@ void assertPrecisionTimestampLiteral(int precision) { void supportedPrecisionForPrecisionTimestampTZLiteral() { assertPrecisionTimestampTZLiteral(0); assertPrecisionTimestampTZLiteral(3); - assertPrecisionTimestampTZLiteral(6); } void assertPrecisionTimestampTZLiteral(int precision) { @@ -207,7 +206,7 @@ void unsupportedPrecisionPrecisionTimestampTZLiteral() { assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(4); assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(5); - + assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(6); assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(7); assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(8); diff --git a/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java b/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java index f0b761967..ff9cb3ab5 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/FunctionConversionTest.java @@ -103,7 +103,7 @@ void extractPrecisionTimestampTzScalarFunction() { "extract:req_ptstz_str", TypeCreator.REQUIRED.I64, EnumArg.builder().value("MONTH").build(), - Expression.PrecisionTimestampTZLiteral.builder().value(0).precision(6).build(), + Expression.PrecisionTimestampTZLiteral.builder().value(0).precision(3).build(), Expression.StrLiteral.builder().value("GMT").build()); RexNode calciteExpr = reqPtstzFn.accept(expressionRexConverter, Context.newContext()); @@ -112,7 +112,7 @@ void extractPrecisionTimestampTzScalarFunction() { RexCall extract = (RexCall) calciteExpr; assertEquals( - "EXTRACT(FLAG(MONTH), 1970-01-01 00:00:00:TIMESTAMP_WITH_LOCAL_TIME_ZONE(6), 'GMT':VARCHAR)", + "EXTRACT(FLAG(MONTH), 1970-01-01 00:00:00:TIMESTAMP_WITH_LOCAL_TIME_ZONE(3), 'GMT':VARCHAR)", extract.toString()); } @@ -300,7 +300,7 @@ private static Stream strptimeTestCases() { "strptime_time:str_str", "12:34:56", "%H:%M:%S", - TypeCreator.REQUIRED.TIME, + TypeCreator.REQUIRED.precisionTime(6), "PARSE_TIME"), Arguments.of( "strptime_timestamp:str_str", From 655a26b6ba52cf579c11308cc8064b5b8d9a52df Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Thu, 5 Mar 2026 11:59:34 +0100 Subject: [PATCH 2/2] fix(ishtmus): use illegalargumentexception Signed-off-by: Niels Pardon --- .../main/java/io/substrait/isthmus/TypeConverter.java | 6 +++--- .../isthmus/expression/ExpressionRexConverter.java | 10 +++++----- .../isthmus/ExpressionConvertabilityTest.java | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java index 53b610044..ac2faf119 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/TypeConverter.java @@ -256,7 +256,7 @@ public RelDataType visit(Type.Timestamp expr) { public RelDataType visit(Type.PrecisionTime expr) { int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIME); if (expr.precision() > maxPrecision) { - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format( "unsupported precision_time precision %s, max precision in Calcite type system is set to %s", expr.precision(), maxPrecision)); @@ -268,7 +268,7 @@ public RelDataType visit(Type.PrecisionTime expr) { public RelDataType visit(Type.PrecisionTimestamp expr) { int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); if (expr.precision() > maxPrecision) { - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format( "unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s", expr.precision(), maxPrecision)); @@ -281,7 +281,7 @@ public RelDataType visit(Type.PrecisionTimestampTZ expr) throws RuntimeException int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE); if (expr.precision() > maxPrecision) { - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format( "unsupported precision_timestamp_tz precision %s, max precision in Calcite type system is set to %s", expr.precision(), maxPrecision)); 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 db91537a0..aba780f7d 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java @@ -215,7 +215,7 @@ public RexNode visit(Expression.TimeLiteral expr, Context context) throws Runtim public RexNode visit(PrecisionTimeLiteral expr, Context context) throws RuntimeException { int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIME); if (expr.precision() > maxPrecision) { - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format( "unsupported precision_time precision %s, max precision in Calcite type system is set to %s", expr.precision(), maxPrecision)); @@ -253,7 +253,7 @@ protected TimeString createTimeString(long value, int precision) { .withNanos(fracSecondsInNano); } default: - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format("Cannot handle PrecisionTime with precision %d.", precision)); } } @@ -288,7 +288,7 @@ public RexNode visit(TimestampTZLiteral expr, Context context) throws RuntimeExc public RexNode visit(PrecisionTimestampLiteral expr, Context context) throws RuntimeException { int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP); if (expr.precision() > maxPrecision) { - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format( "unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s", expr.precision(), maxPrecision)); @@ -302,7 +302,7 @@ public RexNode visit(PrecisionTimestampLiteral expr, Context context) throws Run public RexNode visit(PrecisionTimestampTZLiteral expr, Context context) throws RuntimeException { int maxPrecision = typeFactory.getTypeSystem().getMaxPrecision(SqlTypeName.TIMESTAMP_TZ); if (expr.precision() > maxPrecision) { - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format( "unsupported precision_timestamp precision %s, max precision in Calcite type system is set to %s", expr.precision(), maxPrecision)); @@ -345,7 +345,7 @@ private TimestampString getTimestampString(long value, int precision) { .withNanos(fracSecondsInNano); } default: - throw new UnsupportedOperationException( + throw new IllegalArgumentException( String.format("Cannot handle PrecisionTimestamp with precision %d.", precision)); } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java b/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java index cb1ae6076..cda5192df 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java @@ -230,6 +230,6 @@ void assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(int precision) void assertThrowsExpressionLiteral(Expression.Literal expr) { assertThrows( - UnsupportedOperationException.class, () -> expr.accept(converter, Context.newContext())); + IllegalArgumentException.class, () -> expr.accept(converter, Context.newContext())); } }