Fix #3224 - conversion for TIME '24:00' to LocalTime breaks in binary…

…-mode (#3225)

* test: add tests for `TIME '24:00'` / LocalTime.MAX in binary-mode

add tests which verify, that conversion of binary-transferred TIME/TIMETZ `24:00` to LocalTime currently fails

* fix: handling of `TIME '24:00'` / LocalTime in binary-mode

fixes conversion of TIME/TIMETZ `24:00` to LocalTime from binary.
we now convert `24:00` to `LocalTime.MAX` instead of failing with `Invalid value for NanoOfDay`, which is now consistent with conversion from text.

* test: remove superfluous delete at end of test

`#setUp()` and `#tearDown()` manage the test-table's lifecycle. the test-table gets dropped at end of test anyway.

pgjdbc/src/main/java/org/postgresql/jdbc/TimestampUtils.java

@@ -60,6 +60,7 @@ public class TimestampUtils {
   // LocalTime.MAX is 23:59:59.999_999_999, and it wraps to 24:00:00 when nanos exceed 999_999_499
   // since PostgreSQL has microsecond resolution only
   private static final LocalTime MAX_TIME = LocalTime.MAX.minus(Duration.ofNanos(500));
+  private static final long MAX_TIME_NANOS = MAX_TIME.toNanoOfDay();
   private static final OffsetDateTime MAX_OFFSET_DATETIME = OffsetDateTime.MAX.minus(Duration.ofMillis(500));
   private static final LocalDateTime MAX_LOCAL_DATETIME = LocalDateTime.MAX.minus(Duration.ofMillis(500));
   // low value for dates is   4713 BC
@@ -1437,7 +1438,13 @@ public LocalTime toLocalTimeBin(byte[] bytes) throws PSQLException {
       micros = ByteConverter.int8(bytes, 0);
     }
 
-    return LocalTime.ofNanoOfDay(Math.multiplyExact(micros, 1000L));
+    long nanos = Math.multiplyExact(micros, 1000L);
+
+    if (nanos > MAX_TIME_NANOS) {
+      return LocalTime.MAX;
+    } else {
+      return LocalTime.ofNanoOfDay(nanos);
+    }
   }
 
   /**

pgjdbc/src/test/java/org/postgresql/test/jdbc42/GetObject310Test.java

@@ -210,7 +210,24 @@ public void testGetLocalTime() throws SQLException {
         assertDataTypeMismatch(rs, "time_without_time_zone_column", LocalDate.class);
         assertDataTypeMismatch(rs, "time_without_time_zone_column", LocalDateTime.class);
       }
-      stmt.executeUpdate("DELETE FROM table1");
+    }
+  }
+
+  /**
+   * Test the behavior getObject for time columns with value "24:00", which isn't supported by
+   * {@link LocalTime}, and thus gets converted to {@link LocalTime#MAX}
+   */
+  @Test
+  public void testGetLocalTimeMax() throws SQLException {
+    try (Statement stmt = con.createStatement() ) {
+      stmt.executeUpdate(TestUtil.insertSQL("table1", "time_without_time_zone_column", "TIME '24:00'"));
+
+      try (ResultSet rs = stmt.executeQuery(TestUtil.selectSQL("table1", "time_without_time_zone_column"))) {
+        assertTrue(rs.next());
+        LocalTime localTime = LocalTime.MAX;
+        assertEquals(localTime, rs.getObject("time_without_time_zone_column", LocalTime.class));
+        assertEquals(localTime, rs.getObject(1, LocalTime.class));
+      }
     }
   }
 
@@ -227,7 +244,6 @@ public void testGetLocalTimeNull() throws SQLException {
         assertNull(rs.getObject("time_without_time_zone_column", LocalTime.class));
         assertNull(rs.getObject(1, LocalTime.class));
       }
-      stmt.executeUpdate("DELETE FROM table1");
     }
   }
 
@@ -245,7 +261,6 @@ public void testGetLocalTimeInvalidType() throws SQLException {
         assertDataTypeMismatch(rs, "time_with_time_zone_column", LocalDateTime.class);
         assertDataTypeMismatch(rs, "time_with_time_zone_column", LocalDate.class);
       }
-      stmt.executeUpdate("DELETE FROM table1");
     }
   }
 

pgjdbc/src/test/java/org/postgresql/test/jdbc42/TimestampUtilsTest.java

@@ -8,6 +8,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import org.postgresql.jdbc.TimestampUtils;
+import org.postgresql.util.ByteConverter;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -22,7 +23,7 @@ class TimestampUtilsTest {
 
   @BeforeEach
   void setUp() {
-    timestampUtils = new TimestampUtils(true, TimeZone::getDefault);
+    timestampUtils = new TimestampUtils(false, TimeZone::getDefault);
   }
 
   @Test
@@ -91,6 +92,37 @@ private void assertToLocalTime(String expectedOutput, String inputTime, String m
             + (message == null ? ": " + message : ""));
   }
 
+  @Test
+  void toLocalTimeBin() throws SQLException {
+    assertToLocalTimeBin("00:00:00", 0L);
+
+    assertToLocalTimeBin("00:00:00.1", 100_000L);
+    assertToLocalTimeBin("00:00:00.12", 120_000L);
+    assertToLocalTimeBin("00:00:00.123", 123_000L);
+    assertToLocalTimeBin("00:00:00.1234", 123_400L);
+    assertToLocalTimeBin("00:00:00.12345", 123_450L);
+    assertToLocalTimeBin("00:00:00.123456", 123_456L);
+    assertToLocalTimeBin("00:00:00.999999", 999_999L);
+
+    assertToLocalTimeBin("23:59:59", 86_399_000_000L);
+    assertToLocalTimeBin("23:59:59.999999", 86_399_999_999L);
+    assertToLocalTimeBin(LocalTime.MAX.toString(), 86_400_000_000L, "LocalTime can't represent 24:00:00");
+  }
+
+  private void assertToLocalTimeBin(String expectedOutput, long inputMicros) throws SQLException {
+    assertToLocalTimeBin(expectedOutput, inputMicros, null);
+  }
+
+  private void assertToLocalTimeBin(String expectedOutput, long inputMicros, String message) throws SQLException {
+    final byte[] bytes = new byte[8];
+    ByteConverter.int8(bytes, 0, inputMicros);
+    assertEquals(
+        LocalTime.parse(expectedOutput),
+        timestampUtils.toLocalTimeBin(bytes),
+        "timestampUtils.toLocalTime(" + inputMicros + ")"
+        + (message == null ? ": " + message : ""));
+  }
+
   @Test
   void toStringOfOffsetTime() {
     assertToStringOfOffsetTime("00:00:00+00", "00:00:00+00:00");