From 7fd16116cf95b239a789d8d954266f4dc8e22cb8 Mon Sep 17 00:00:00 2001 From: Aleksandr Efimov Date: Wed, 23 Sep 2026 10:40:40 +0300 Subject: [PATCH 1/2] 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/2] 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(