From 7fd16116cf95b239a789d8d954266f4dc8e22cb8 Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Wed, 23 Sep 2026 10:40:40 +0300 Subject: [PATCH 1/5] fix(isthmus)!: convert precision_time and precision_timestamp up to nanoseconds SubstraitTypeSystem capped TIME, TIMESTAMP and TIMESTAMP_WITH_LOCAL_TIME_ZONE at microseconds, so a plan carrying a nanosecond precision_timestamp was refused although Calcite 1.42 builds such a type the moment the ceiling allows it, and TimestampString carries the digits. Raise the ceiling to nanoseconds. Precisions 10 to 12, which Substrait allows and Calcite clamps rather than reports, stay refused, naming the bound. Converting back, a value is a 64-bit count of its own unit, so nanoseconds reach only 2262 where a TimestampString reaches 9999: a timestamp outside that range is now reported instead of wrapping. Closes #995 --- .../isthmus/SubstraitTypeSystem.java | 7 ++-- .../isthmus/expression/LiteralConverter.java | 36 +++++++++++++++++-- .../substrait/isthmus/CalciteLiteralTest.java | 32 +++++++++++++++-- .../io/substrait/isthmus/CalciteTypeTest.java | 21 +++++++++-- .../isthmus/ExpressionConvertabilityTest.java | 22 +++++------- .../isthmus/SubstraitTypeSystemTest.java | 8 +++-- 6 files changed, 101 insertions(+), 25 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java index 849be1526..dcc10631f 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java @@ -106,13 +106,14 @@ public int getMaxPrecision(final SqlTypeName typeName) { case BINARY: case VARBINARY: return Integer.MAX_VALUE; + case TIME: + case TIMESTAMP: + case TIMESTAMP_WITH_LOCAL_TIME_ZONE: + return 9; case INTERVAL_DAY: case INTERVAL_YEAR: case INTERVAL_YEAR_MONTH: - case TIME: case TIME_WITH_LOCAL_TIME_ZONE: - case TIMESTAMP: - case TIMESTAMP_WITH_LOCAL_TIME_ZONE: return 6; case DECIMAL: return 38; 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 ebd14d978..18266f8ca 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -296,8 +296,11 @@ public Expression.Literal convert(RexLiteral literal, RelDataType resultType) { LocalDateTime.parse(timestamp.toString(), CALCITE_LOCAL_DATETIME_FORMATTER); int precision = TypeConverter.precisionOf(resultType); long value = - localDateTime.toEpochSecond(ZoneOffset.UTC) * LongMath.pow(10, precision) - + rescaleNanos(localDateTime.getNano(), precision); + epochUnits( + localDateTime.toEpochSecond(ZoneOffset.UTC), + rescaleNanos(localDateTime.getNano(), precision), + precision, + timestamp); // toEpochSecond floors, and the nanosecond part it leaves behind is always positive, so // a pre-epoch timestamp narrowed to a coarser precision moves back in time rather than // towards the epoch. That is what a timestamp wants — 1969-12-31 23:59:59.5 at second @@ -400,6 +403,35 @@ public static byte[] padRightIfNeeded( return padRightIfNeeded(bytes.getBytes(), length); } + /** + * Returns a timestamp as a count of 10^-precision seconds since the epoch. + * + *

A Substrait temporal value is a 64-bit count of its own unit, and the finer the unit the + * narrower the range it spans: nanoseconds reach only to the year 2262, where Calcite's own + * {@code TimestampString} reaches 9999. A timestamp outside the range is reported rather than + * wrapped into a different instant. + * + * @param epochSeconds whole seconds since the epoch, floored + * @param subSecondUnits the sub-second part, already in units of 10^-precision seconds + * @param precision the fractional-second precision + * @param timestamp the timestamp being converted, for the failure message + * @return the value of the Substrait literal + * @throws IllegalArgumentException if the value does not fit in 64 bits + */ + private static long epochUnits( + long epochSeconds, long subSecondUnits, int precision, TimestampString timestamp) { + try { + return Math.addExact( + Math.multiplyExact(epochSeconds, LongMath.pow(10, precision)), subSecondUnits); + } catch (ArithmeticException e) { + throw new IllegalArgumentException( + String.format( + "timestamp %s does not fit in a 64-bit count of 10^-%d seconds", + timestamp, precision), + e); + } + } + /** * Rescales a nanosecond count to the fractional-second unit a Substrait temporal literal of the * given precision is expressed in. diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index a2db08544..f6895e5f6 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -143,9 +143,35 @@ void tTimeWithMicroSecond() { @Test void tTimeWithNanoSecond() { - assertEquals( - rex.makeTimeLiteral(new TimeString("14:22:47.123456789"), 9), - rex.makeTimeLiteral(new TimeString("14:22:47.123456"), 6)); + bitest( + ExpressionCreator.precisionTime( + false, (14L * 60 * 60 + 22 * 60 + 47) * 1_000_000_000L + 123_456_789, 9), + rex.makeTimeLiteral(new TimeString("14:22:47.123456789"), 9)); + } + + /** + * A Substrait temporal value is a 64-bit count of its own unit, so the finer the unit the + * narrower the range: nanoseconds run out in 2262, where a Calcite TimestampString reaches 9999. + * A timestamp past that is reported rather than wrapped into a different instant. + */ + @Test + void aTimestampTooLargeForItsPrecisionIsReported() { + RexLiteral literal = + rex.makeTimestampLiteral(new TimestampString("9999-12-31 23:59:59.999999999"), 9); + + IllegalArgumentException error = + assertThrows( + IllegalArgumentException.class, + () -> new LiteralConverter(TypeConverter.DEFAULT).convert(literal)); + + assertTrue(error.getMessage().contains("does not fit in a 64-bit count of 10^-9 seconds")); + } + + @Test + void tPrecisionTimestampAtNanosecondPrecision() { + bitest( + ExpressionCreator.precisionTimestamp(false, 1_704_067_200_123_456_789L, 9), + rex.makeTimestampLiteral(new TimestampString("2024-01-01 00:00:00.123456789"), 9)); } @Test diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java index 138b99cd3..d35196674 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteTypeTest.java @@ -113,7 +113,7 @@ void time(boolean nullable) { @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimeStamp(boolean nullable) { - for (int precision : new int[] {0, 3, 6}) { + for (int precision : new int[] {0, 3, 6, 9}) { testType( Type.withNullability(nullable).precisionTimestamp(precision), SqlTypeName.TIMESTAMP, @@ -125,7 +125,7 @@ void precisionTimeStamp(boolean nullable) { @ParameterizedTest @ValueSource(booleans = {true, false}) void precisionTimestamptz(boolean nullable) { - for (int precision : new int[] {0, 3, 6}) { + for (int precision : new int[] {0, 3, 6, 9}) { testType( Type.withNullability(nullable).precisionTimestampTZ(precision), SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE, @@ -134,6 +134,23 @@ void precisionTimestamptz(boolean nullable) { } } + /** + * Substrait allows 0 to 12; Calcite carries nanoseconds, and builds a type at 12 as one at 9 + * rather than reporting that it cannot. A precision past what it can hold is refused, naming the + * bound, instead of being narrowed in silence. + */ + @ParameterizedTest + @ValueSource(ints = {10, 12}) + void aPrecisionFinerThanNanosecondsIsRefused(int precision) { + IllegalArgumentException error = + assertThrows( + IllegalArgumentException.class, + () -> + TypeConverter.DEFAULT.toCalcite( + type, TypeCreator.REQUIRED.precisionTimestamp(precision), null)); + assertTrue(error.getMessage().contains("max precision in Calcite type system is set to 9")); + } + @ParameterizedTest @ValueSource(booleans = {true, false}) void intervalYear(boolean nullable) { diff --git a/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java b/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java index 86d405888..bbc9fd47c 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/ExpressionConvertabilityTest.java @@ -152,6 +152,9 @@ void supportedPrecisionForPrecisionTimestampLiteral() { assertPrecisionTimestampLiteral(4); assertPrecisionTimestampLiteral(5); assertPrecisionTimestampLiteral(6); + assertPrecisionTimestampLiteral(7); + assertPrecisionTimestampLiteral(8); + assertPrecisionTimestampLiteral(9); } void assertPrecisionTimestampLiteral(int precision) { @@ -166,7 +169,7 @@ void assertPrecisionTimestampLiteral(int precision) { @Test void supportedPrecisionForPrecisionTimestampTZLiteral() { - // The same set the non-TZ literal supports: SubstraitTypeSystem configures a maximum of 6 for + // The same set the non-TZ literal supports: SubstraitTypeSystem configures a maximum of 9 for // TIMESTAMP_WITH_LOCAL_TIME_ZONE, and the TZ literal is checked against that name too. assertPrecisionTimestampTZLiteral(0); assertPrecisionTimestampTZLiteral(1); @@ -175,6 +178,9 @@ void supportedPrecisionForPrecisionTimestampTZLiteral() { assertPrecisionTimestampTZLiteral(4); assertPrecisionTimestampTZLiteral(5); assertPrecisionTimestampTZLiteral(6); + assertPrecisionTimestampTZLiteral(7); + assertPrecisionTimestampTZLiteral(8); + assertPrecisionTimestampTZLiteral(9); } void assertPrecisionTimestampTZLiteral(int precision) { @@ -192,12 +198,7 @@ void unsupportedPrecisionForPrecisionTimestampLiteral() { // test different edge case precision values assertThrowsUnsupportedPrecisionPrecisionTimestampLiteral(-1); - assertThrowsUnsupportedPrecisionPrecisionTimestampLiteral(7); - assertThrowsUnsupportedPrecisionPrecisionTimestampLiteral(8); - - // this would be nanoseconds which are supported in Substrait but not in Calcite - assertThrowsUnsupportedPrecisionPrecisionTimestampLiteral(9); - + // finer than a nanosecond, which is the finest unit a Calcite TimestampString carries assertThrowsUnsupportedPrecisionPrecisionTimestampLiteral(10); assertThrowsUnsupportedPrecisionPrecisionTimestampLiteral(11); @@ -218,12 +219,7 @@ void unsupportedPrecisionPrecisionTimestampTZLiteral() { // test different edge case precision values assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(-1); - assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(7); - assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(8); - - // this would be nanoseconds which are supported in Substrait but not in Calcite - assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(9); - + // finer than a nanosecond, which is the finest unit a Calcite TimestampString carries assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(10); assertThrowsUnsupportedPrecisionPrecisionTimestampTZLiteral(11); diff --git a/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java b/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java index 03dd4c77f..56d00ace8 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/SubstraitTypeSystemTest.java @@ -45,12 +45,16 @@ void decimalDefaultScale() { @Test void timestampMaxPrecision() { - assertEquals(6, typeSystem.getMaxPrecision(SqlTypeName.TIMESTAMP)); + // Nanoseconds: the finest unit a Calcite TimestampString carries, and what the type factory + // builds a TIMESTAMP at once the ceiling allows it. Picoseconds clamp rather than throw, so + // Substrait's 10 to 12 stay out. + assertEquals(9, typeSystem.getMaxPrecision(SqlTypeName.TIMESTAMP)); + assertEquals(9, typeSystem.getMaxPrecision(SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE)); } @Test void timeMaxPrecision() { - assertEquals(6, typeSystem.getMaxPrecision(SqlTypeName.TIME)); + assertEquals(9, typeSystem.getMaxPrecision(SqlTypeName.TIME)); } @Test From b9a83fd7ea2b6fdcf2067f0c641e8a5b41e09ed8 Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Wed, 23 Sep 2026 16:47:56 +0300 Subject: [PATCH 2/5] fix(isthmus): convert the first second of the nanosecond timestamp range The floored seconds of 1677-09-21 00:12:43 overflow when scaled to nanoseconds although the value fits once the sub-second part is added back, so a negative timestamp borrows that second before scaling. Pin both ends of the range and the nanosecond past each. --- .../isthmus/expression/LiteralConverter.java | 9 ++++-- .../substrait/isthmus/CalciteLiteralTest.java | 29 +++++++++++++++++++ 2 files changed, 36 insertions(+), 2 deletions(-) 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 18266f8ca..6fb42baf0 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -420,9 +420,14 @@ public static byte[] padRightIfNeeded( */ private static long epochUnits( long epochSeconds, long subSecondUnits, int precision, TimestampString timestamp) { + long unitsPerSecond = LongMath.pow(10, precision); try { - return Math.addExact( - Math.multiplyExact(epochSeconds, LongMath.pow(10, precision)), subSecondUnits); + // The floored seconds of the range's first second overflow on their own although the value + // fits once the sub-second part is added back, so a negative value borrows that second. + return epochSeconds < 0 && subSecondUnits > 0 + ? Math.addExact( + Math.multiplyExact(epochSeconds + 1, unitsPerSecond), subSecondUnits - unitsPerSecond) + : Math.addExact(Math.multiplyExact(epochSeconds, unitsPerSecond), subSecondUnits); } catch (ArithmeticException e) { throw new IllegalArgumentException( String.format( diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index f6895e5f6..3035503d1 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -167,6 +167,35 @@ void aTimestampTooLargeForItsPrecisionIsReported() { assertTrue(error.getMessage().contains("does not fit in a 64-bit count of 10^-9 seconds")); } + /** + * Both ends of the nanosecond range convert, and one nanosecond past either is reported. The + * lower end is the one a floored split can get wrong: its seconds alone do not fit in 64 bits. + */ + @ParameterizedTest + @CsvSource({ + "1677-09-21 00:12:43.145224192, -9223372036854775808", + "2262-04-11 23:47:16.854775807, 9223372036854775807", + }) + void theEndsOfTheNanosecondRangeConvert(String timestamp, long nanos) { + assertEquals( + ExpressionCreator.precisionTimestamp(false, nanos, 9), + new LiteralConverter(TypeConverter.DEFAULT) + .convert(rex.makeTimestampLiteral(new TimestampString(timestamp), 9))); + } + + @ParameterizedTest + @ValueSource(strings = {"1677-09-21 00:12:43.145224191", "2262-04-11 23:47:16.854775808"}) + void oneNanosecondPastTheRangeIsReported(String timestamp) { + RexLiteral literal = rex.makeTimestampLiteral(new TimestampString(timestamp), 9); + + IllegalArgumentException error = + assertThrows( + IllegalArgumentException.class, + () -> new LiteralConverter(TypeConverter.DEFAULT).convert(literal)); + + assertTrue(error.getMessage().contains("does not fit in a 64-bit count of 10^-9 seconds")); + } + @Test void tPrecisionTimestampAtNanosecondPrecision() { bitest( From 67869223f55c5984e66c0719b675353b0717dcb7 Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Mon, 28 Sep 2026 23:45:51 +0300 Subject: [PATCH 3/5] fix(isthmus): pin far-date comparisons on a nanosecond column A TIMESTAMP(9) column widens a compared literal to nanoseconds, so a sentinel date outside 1677-2262 has no value of that type, and the conversion reports it, as DuckDB and Arrow do. Also pass Locale.ROOT to the overflow message, cover precisions 7 to 9 in the value round trips, and fix the comment on the date minus interval result. --- .../isthmus/expression/FunctionMappings.java | 5 ++- .../isthmus/expression/LiteralConverter.java | 4 ++- .../substrait/isthmus/CalciteLiteralTest.java | 33 +++++++++++++++---- 3 files changed, 32 insertions(+), 10 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionMappings.java b/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionMappings.java index 8ae286e7d..72b01a2db 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionMappings.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/FunctionMappings.java @@ -267,9 +267,8 @@ private static RelDataType inferDatetimeSubtractType(SqlOperatorBinding binding) && intervalType.getFamily() == SqlTypeFamily.INTERVAL_DAY_TIME) { // The spec declares subtract(date, interval_day

) -> precision_timestamp

(spec // v0.101.0), and the interval operand carries P as its fractional-second scale. The result - // is capped at the timestamp maximum, which is lower than the interval one, so an - // interval_day<9> still yields precision_timestamp<6> until the Substrait->Calcite - // conversion accepts a precision_timestamp above 6. + // is capped at the timestamp maximum, which now agrees with the interval one at 9, so an + // interval_day<9> yields precision_timestamp<9> as the spec declares. int precision = Math.max( 0, 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 6fb42baf0..accce3ffb 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -431,8 +431,10 @@ private static long epochUnits( } catch (ArithmeticException e) { throw new IllegalArgumentException( String.format( + Locale.ROOT, "timestamp %s does not fit in a 64-bit count of 10^-%d seconds", - timestamp, precision), + timestamp, + precision), e); } } diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index 3035503d1..cccb24dc9 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -34,6 +34,7 @@ import java.util.List; import java.util.concurrent.TimeUnit; import org.apache.calcite.adapter.java.JavaTypeFactory; +import org.apache.calcite.prepare.Prepare; import org.apache.calcite.rel.RelRoot; import org.apache.calcite.rel.core.Project; import org.apache.calcite.rel.type.RelDataType; @@ -196,6 +197,26 @@ void oneNanosecondPastTheRangeIsReported(String timestamp) { assertTrue(error.getMessage().contains("does not fit in a 64-bit count of 10^-9 seconds")); } + /** + * Compared with a nanosecond column, a literal is widened to the column's precision, so a + * sentinel date outside the nanosecond range has no value of that type and is reported. DuckDB's + * {@code TIMESTAMP_NS} and Arrow's {@code timestamp[ns]} refuse the same comparison. + */ + @ParameterizedTest + @ValueSource(strings = {"9999-12-31 23:59:59", "1600-01-01 00:00:00"}) + void aFarDateComparedWithANanosecondColumnIsReported(String timestamp) throws Exception { + Prepare.CatalogReader catalog = + SubstraitCreateStatementParser.processCreateStatementsToCatalog( + "CREATE TABLE v (ts TIMESTAMP(9))"); + String query = "SELECT * FROM v WHERE ts > TIMESTAMP '" + timestamp + "'"; + + IllegalArgumentException error = + assertThrows( + IllegalArgumentException.class, () -> new SqlToSubstrait().convert(query, catalog)); + + assertTrue(error.getMessage().contains("does not fit in a 64-bit count of 10^-9 seconds")); + } + @Test void tPrecisionTimestampAtNanosecondPrecision() { bitest( @@ -294,7 +315,7 @@ void tTimestampWithMilliMicroSeconds() { } @ParameterizedTest - @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6}) + @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}) void tPrecisionTimeKeepsItsPrecision(int precision) { Expression.PrecisionTimeLiteral time = ExpressionCreator.precisionTime(false, timeValue(precision), precision); @@ -305,7 +326,7 @@ void tPrecisionTimeKeepsItsPrecision(int precision) { } @ParameterizedTest - @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6}) + @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}) void tPrecisionTimestampKeepsItsPrecision(int precision) { PrecisionTimestampLiteral timestamp = ExpressionCreator.precisionTimestamp(false, timestampValue(precision), precision); @@ -316,7 +337,7 @@ void tPrecisionTimestampKeepsItsPrecision(int precision) { } @ParameterizedTest - @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6}) + @ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9}) void tPrecisionTimestampTZStaysTimeZoned(int precision) { // Calcite has no dedicated timestamp-with-time-zone literal, but it does have the // TIMESTAMP_WITH_LOCAL_TIME_ZONE type that TypeConverter maps precision_timestamp_tz to, so a @@ -431,10 +452,10 @@ private static long timestampValue(int precision) { + subseconds(precision); } - /** 123 milliseconds expressed at the given precision, or none at all when there is no room. */ + /** .123456789 seconds cut to the given precision, or none at all when there is no room. */ private static long subseconds(int precision) { - // The first `precision` digits of .123456, so no precision is left with an empty fraction. - return 123_456L / LongMath.pow(10, 6 - precision); + // The first `precision` digits of .123456789, so no precision is left with an empty fraction. + return 123_456_789L / LongMath.pow(10, 9 - precision); } @Test From 3f883790d6a50491d8145aaa83aa22cb5de171e0 Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Tue, 29 Sep 2026 11:32:59 +0300 Subject: [PATCH 4/5] fix(isthmus): reject a precision_timestamp outside the TimestampString range A TimestampString spans years 0000 to 9999, and past that DateTimeUtils renders the year modulo 10000, so precision_timestamp<7> 3000000000000000000 became 1476-08-15 05:20:00. The value is now checked the way a precision_time already is. Also documents the IllegalArgumentException of LiteralConverter.convert(RexLiteral) and why the time ceilings stop at 9. --- .../isthmus/SubstraitTypeSystem.java | 6 ++++ .../expression/ExpressionRexConverter.java | 11 +++++-- .../isthmus/expression/LiteralConverter.java | 1 + .../substrait/isthmus/CalciteLiteralTest.java | 30 +++++++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) diff --git a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java index dcc10631f..c313b5a12 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java +++ b/isthmus/src/main/java/io/substrait/isthmus/SubstraitTypeSystem.java @@ -95,6 +95,12 @@ public static void requireSupportedPrecision( * VARBINARY(65536)} -- narrower than one of its own inputs, and the cap this method removes * reimposed. * + *

{@link SqlTypeName#TIME}, {@link SqlTypeName#TIMESTAMP} and {@link + * SqlTypeName#TIMESTAMP_WITH_LOCAL_TIME_ZONE} stop at 9, nanoseconds: that is the finest unit a + * Calcite {@code TimeString} or {@code TimestampString} carries. Substrait's precisions 10 to 12 + * stay out because the type factory clamps a finer precision to the ceiling rather than reporting + * it. + * * @param typeName The {@link SqlTypeName} for which precision is requested. * @return Maximum precision for the type. */ 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 55e22bc63..57ca1dfa1 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java @@ -355,8 +355,15 @@ public RexNode visit(PrecisionTimestampTZLiteral expr, Context context) throws R private TimestampString getTimestampString(long value, int precision) { long unitsPerSecond = unitsPerSecond(precision, "PrecisionTimestamp"); - return TimestampString.fromMillisSinceEpoch( - TimeUnit.SECONDS.toMillis(secondsOf(value, unitsPerSecond))) + long seconds = secondsOf(value, unitsPerSecond); + // A TimestampString spans 0000-01-01 00:00:00 to 9999-12-31 23:59:59. Without this an + // out-of-range value reaches DateTimeUtils, which renders the year modulo 10000, so the literal + // names a different instant rather than reporting the value. + if (seconds < -62_167_219_200L || seconds > 253_402_300_799L) { + throw new IllegalArgumentException( + String.format("Cannot handle PrecisionTimestamp with out-of-range value %d.", value)); + } + return TimestampString.fromMillisSinceEpoch(TimeUnit.SECONDS.toMillis(seconds)) .withNanos(nanosOf(value, unitsPerSecond)); } 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 accce3ffb..5a16383e5 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/LiteralConverter.java @@ -185,6 +185,7 @@ private static BigDecimal bd(RexLiteral literal) { * @param literal the Calcite literal to convert * @return the corresponding Substrait literal * @throws UnsupportedOperationException if the literal type/value cannot be handled + * @throws IllegalArgumentException if the literal's value cannot be expressed at its type */ public Expression.Literal convert(RexLiteral literal) { return convert(literal, literal.getType()); diff --git a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java index cccb24dc9..484ee589f 100644 --- a/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java +++ b/isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java @@ -264,6 +264,36 @@ void tPrecisionTimeRejectsAnOutOfRangeValue() { assertEquals(new TimeString("00:00:00"), timeStringOf(0L, 0)); } + @Test + void tPrecisionTimestampRejectsAnOutOfRangeValue() { + // A TimestampString spans 0000-01-01 to 9999-12-31, and past it Calcite renders the year + // modulo 10000: year 11476 at precision 7 would convert to 1476-08-15 05:20:00 with nothing + // said. Rejected by value instead, at every precision. + assertEquals( + "Cannot handle PrecisionTimestamp with out-of-range value 3000000000000000000.", + timestampRejectionOf(3_000_000_000_000_000_000L, 7)); + assertEquals( + "Cannot handle PrecisionTimestamp with out-of-range value -9223372036854775808.", + timestampRejectionOf(Long.MIN_VALUE, 8)); + assertEquals( + "Cannot handle PrecisionTimestamp with out-of-range value 253402300800.", + timestampRejectionOf(253_402_300_800L, 0)); + assertEquals( + "Cannot handle PrecisionTimestamp with out-of-range value -62167219201.", + timestampRejectionOf(-62_167_219_201L, 0)); + + // The last second a TimestampString holds, and the first, still convert. + assertEquals( + new TimestampString("9999-12-31 23:59:59"), timestampStringOf(253_402_300_799L, 0)); + assertEquals( + new TimestampString("0000-01-01 00:00:00"), timestampStringOf(-62_167_219_200L, 0)); + } + + private String timestampRejectionOf(long value, int precision) { + return assertThrows(IllegalArgumentException.class, () -> timestampStringOf(value, precision)) + .getMessage(); + } + private TimestampString timestampStringOf(long value, int precision) { RexNode converted = ExpressionCreator.precisionTimestamp(false, value, precision) From b5ef9b1e56b2dada79d2b40afbe0574b55b1fdbf Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Tue, 29 Sep 2026 12:39:18 +0300 Subject: [PATCH 5/5] fix(isthmus): format out-of-range temporal values independent of locale %d formats in the default locale, so under a non-Latin numbering system the precision_time and precision_timestamp messages carried other digits and the exact-match test assertions failed. %s does not. --- .../substrait/isthmus/expression/ExpressionRexConverter.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 57ca1dfa1..446fef62c 100644 --- a/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java +++ b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java @@ -301,7 +301,7 @@ protected TimeString createTimeString(long value, int precision) { // A precision_time is a time of day. Without this an out-of-range value reaches // TimeString.fromMillisOfDay, which reports a corrupt time string rather than the value. throw new IllegalArgumentException( - String.format("Cannot handle PrecisionTime with out-of-range value %d.", value)); + String.format("Cannot handle PrecisionTime with out-of-range value %s.", value)); } return TimeString.fromMillisOfDay( (int) TimeUnit.SECONDS.toMillis(secondsOf(value, unitsPerSecond))) @@ -361,7 +361,7 @@ private TimestampString getTimestampString(long value, int precision) { // names a different instant rather than reporting the value. if (seconds < -62_167_219_200L || seconds > 253_402_300_799L) { throw new IllegalArgumentException( - String.format("Cannot handle PrecisionTimestamp with out-of-range value %d.", value)); + String.format("Cannot handle PrecisionTimestamp with out-of-range value %s.", value)); } return TimestampString.fromMillisSinceEpoch(TimeUnit.SECONDS.toMillis(seconds)) .withNanos(nanosOf(value, unitsPerSecond));