{@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. */ @@ -106,13 +112,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/ExpressionRexConverter.java b/isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java index 55e22bc63..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))) @@ -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 %s.", value)); + } + return TimestampString.fromMillisSinceEpoch(TimeUnit.SECONDS.toMillis(seconds)) .withNanos(nanosOf(value, unitsPerSecond)); } 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 ebd14d978..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()); @@ -296,8 +297,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 +404,42 @@ 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) { + long unitsPerSecond = LongMath.pow(10, precision); + try { + // 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( + Locale.ROOT, + "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..484ee589f 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; @@ -143,9 +144,84 @@ void tTimeWithMicroSecond() { @Test void tTimeWithNanoSecond() { + 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")); + } + + /** + * 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( - rex.makeTimeLiteral(new TimeString("14:22:47.123456789"), 9), - rex.makeTimeLiteral(new TimeString("14:22:47.123456"), 6)); + 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")); + } + + /** + * 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( + ExpressionCreator.precisionTimestamp(false, 1_704_067_200_123_456_789L, 9), + rex.makeTimestampLiteral(new TimestampString("2024-01-01 00:00:00.123456789"), 9)); } @Test @@ -188,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) @@ -239,7 +345,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); @@ -250,7 +356,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); @@ -261,7 +367,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 @@ -376,10 +482,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 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