fix(isthmus): convert pre-epoch temporal literals instead of throwing

isthmus/src/main/java/io/substrait/isthmus/expression/ExpressionRexConverter.java

@@ -305,36 +305,16 @@ public RexNode visit(PrecisionTimeLiteral expr, Context context) throws RuntimeE
    * @return a TimeString representing the time value
    */
   protected TimeString createTimeString(long value, int precision) {
-    switch (precision) {
-      case 0:
-        return TimeString.fromMillisOfDay((int) TimeUnit.SECONDS.toMillis(value));
-      case 3:
-        {
-          long seconds = TimeUnit.MILLISECONDS.toSeconds(value);
-          int fracSecondsInNano =
-              (int) (TimeUnit.MILLISECONDS.toNanos(value) - TimeUnit.SECONDS.toNanos(seconds));
-          return TimeString.fromMillisOfDay((int) TimeUnit.SECONDS.toMillis(seconds))
-              .withNanos(fracSecondsInNano);
-        }
-      case 6:
-        {
-          long seconds = TimeUnit.MICROSECONDS.toSeconds(value);
-          int fracSecondsInNano =
-              (int) (TimeUnit.MICROSECONDS.toNanos(value) - TimeUnit.SECONDS.toNanos(seconds));
-          return TimeString.fromMillisOfDay((int) TimeUnit.SECONDS.toMillis(seconds))
-              .withNanos(fracSecondsInNano);
-        }
-      case 9:
-        {
-          long seconds = TimeUnit.NANOSECONDS.toSeconds(value);
-          int fracSecondsInNano = (int) (value - TimeUnit.SECONDS.toNanos(seconds));
-          return TimeString.fromMillisOfDay((int) TimeUnit.SECONDS.toMillis(seconds))
-              .withNanos(fracSecondsInNano);
-        }
-      default:
-        throw new IllegalArgumentException(
-            String.format("Cannot handle PrecisionTime with precision %d.", precision));
+    long unitsPerSecond = unitsPerSecond(precision, "PrecisionTime");
+    if (value < 0 || value >= 86_400L * unitsPerSecond) {
+      // 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));
     }
+    return TimeString.fromMillisOfDay(
+            (int) TimeUnit.SECONDS.toMillis(secondsOf(value, unitsPerSecond)))
+        .withNanos(nanosOf(value, unitsPerSecond));
   }
 
   @Override
@@ -384,38 +364,60 @@ 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)))
+        .withNanos(nanosOf(value, unitsPerSecond));
+  }
+
+  /**
+   * Returns how many units of a temporal value at the given precision make up one second.
+   *
+   * @param precision the fractional-second precision
+   * @param typeName the Substrait type being converted, for the failure message
+   * @return the number of units per second
+   * @throws IllegalArgumentException if the precision is not one Calcite can be given
+   */
+  private static long unitsPerSecond(int precision, String typeName) {
     switch (precision) {
       case 0:
-        return TimestampString.fromMillisSinceEpoch(TimeUnit.SECONDS.toMillis(value));
+        return 1L;
       case 3:
-        {
-          long seconds = TimeUnit.MILLISECONDS.toSeconds(value);
-          int fracSecondsInNano =
-              (int) (TimeUnit.MILLISECONDS.toNanos(value) - TimeUnit.SECONDS.toNanos(seconds));
-          return TimestampString.fromMillisSinceEpoch(TimeUnit.SECONDS.toMillis(seconds))
-              .withNanos(fracSecondsInNano);
-        }
+        return 1_000L;
       case 6:
-        {
-          long seconds = TimeUnit.MICROSECONDS.toSeconds(value);
-          int fracSecondsInNano =
-              (int) (TimeUnit.MICROSECONDS.toNanos(value) - TimeUnit.SECONDS.toNanos(seconds));
-          return TimestampString.fromMillisSinceEpoch(TimeUnit.SECONDS.toMillis(seconds))
-              .withNanos(fracSecondsInNano);
-        }
+        return 1_000_000L;
       case 9:
-        {
-          long seconds = TimeUnit.NANOSECONDS.toSeconds(value);
-          int fracSecondsInNano = (int) (value - TimeUnit.SECONDS.toNanos(seconds));
-          return TimestampString.fromMillisSinceEpoch(TimeUnit.SECONDS.toMillis(seconds))
-              .withNanos(fracSecondsInNano);
-        }
+        return 1_000_000_000L;
       default:
         throw new IllegalArgumentException(
-            String.format("Cannot handle PrecisionTimestamp with precision %d.", precision));
+            String.format("Cannot handle %s with precision %d.", typeName, precision));
     }
   }
 
+  /**
+   * Returns the whole seconds in a temporal value, rounding towards negative infinity rather than
+   * towards zero. Before the epoch the value is negative, and both {@link TimeString#withNanos} and
+   * {@link TimestampString#withNanos} reject the negative remainder that truncation would leave.
+   *
+   * @param value the temporal value
+   * @param unitsPerSecond the number of units of that value per second
+   * @return the whole seconds
+   */
+  private static long secondsOf(long value, long unitsPerSecond) {
+    return Math.floorDiv(value, unitsPerSecond);
+  }
+
+  /**
+   * Returns the sub-second part of a temporal value in nanoseconds, always in [0, 1_000_000_000).
+   *
+   * @param value the temporal value
+   * @param unitsPerSecond the number of units of that value per second
+   * @return the sub-second part in nanoseconds
+   */
+  private static int nanosOf(long value, long unitsPerSecond) {
+    return (int) (Math.floorMod(value, unitsPerSecond) * (1_000_000_000L / unitsPerSecond));
+  }
+
   @Override
   public RexNode visit(Expression.IntervalYearLiteral expr, Context context)
       throws RuntimeException {

isthmus/src/test/java/io/substrait/isthmus/CalciteLiteralTest.java

@@ -2,6 +2,7 @@
 
 import static io.substrait.isthmus.SubstraitTypeSystem.YEAR_MONTH_INTERVAL;
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import com.google.common.collect.ImmutableMap;
@@ -141,6 +142,65 @@ void tTimeWithNanoSecond() {
         rex.makeTimeLiteral(new TimeString("14:22:47.123456"), 6));
   }
 
+  @Test
+  void tPrecisionTimestampBeforeTheEpoch() {
+    // 1969-12-31 23:59:59.5, where the value is negative and the sub-second part is not. Splitting
+    // it has to floor rather than truncate towards zero: TimestampString.withNanos rejects the
+    // negative remainder truncation leaves behind, so both of these threw before this change.
+    assertEquals(new TimestampString("1969-12-31 23:59:59.5"), timestampStringOf(-500L, 3));
+    assertEquals(new TimestampString("1969-12-31 23:59:59.5"), timestampStringOf(-500_000L, 6));
+
+    // The epoch itself, and a value after it, are unchanged by the same split.
+    assertEquals(new TimestampString("1970-01-01 00:00:00"), timestampStringOf(0L, 3));
+    assertEquals(new TimestampString("1970-01-01 00:00:01.5"), timestampStringOf(1_500L, 3));
+  }
+
+  @Test
+  void tPrecisionTimeKeepsItsSubSecondPart() {
+    // A time of day is never negative, so this only guards that the shared split did not change
+    // what it produced.
+    assertEquals(
+        new TimeString("14:22:47.5"), timeStringOf((14L * 3600 + 22 * 60 + 47) * 1000 + 500, 3));
+  }
+
+  @Test
+  void tPrecisionTimeRejectsAnOutOfRangeValue() {
+    // A precision_time is a time of day, so a day or more is as far outside it as a negative value
+    // is. Left to Calcite that end reports "Hour out of range: [24]" from inside TimeString until
+    // the millisecond count overflows an int, past which the narrowing wraps and 4346734 seconds
+    // converts to 14:22:46 with nothing said. Rejected by value instead.
+    assertEquals(
+        "Cannot handle PrecisionTime with out-of-range value -500.", timeRejectionOf(-500L, 3));
+    assertEquals(
+        "Cannot handle PrecisionTime with out-of-range value 4346734.",
+        timeRejectionOf(4_346_734L, 0));
+    assertEquals(
+        "Cannot handle PrecisionTime with out-of-range value 86400.", timeRejectionOf(86_400L, 0));
+
+    // The last second that is still a time of day, and the first, are not.
+    assertEquals(new TimeString("23:59:59"), timeStringOf(86_399L, 0));
+    assertEquals(new TimeString("00:00:00"), timeStringOf(0L, 0));
+  }
+
+  private TimestampString timestampStringOf(long value, int precision) {
+    RexNode converted =
+        ExpressionCreator.precisionTimestamp(false, value, precision)
+            .accept(expressionRexConverter, Context.newContext());
+    return ((RexLiteral) converted).getValueAs(TimestampString.class);
+  }
+
+  private TimeString timeStringOf(long value, int precision) {
+    RexNode converted =
+        ExpressionCreator.precisionTime(false, value, precision)
+            .accept(expressionRexConverter, Context.newContext());
+    return ((RexLiteral) converted).getValueAs(TimeString.class);
+  }
+
+  private String timeRejectionOf(long value, int precision) {
+    return assertThrows(IllegalArgumentException.class, () -> timeStringOf(value, precision))
+        .getMessage();
+  }
+
   @Test
   void tDate() {
     bitest(