From 79b2c5d40eaa6378a90519371f07b2b39e4bbda8 Mon Sep 17 00:00:00 2001
From: Margarit Hakobyan <margarit.hakobyan@improving.com>
Date: Wed, 21 Dec 2022 14:09:21 -0800
Subject: [PATCH 1/5] Fixes ResultSet.getTime returns null bug

Signed-off-by: Margarit Hakobyan <margarit.hakobyan@improving.com>
---
 .../org/opensearch/jdbc/ResultSetImpl.java    | 30 +++++++---
 .../org/opensearch/jdbc/types/TimeType.java   | 57 +++++++++++++------
 .../opensearch/jdbc/types/TypeConverters.java |  2 +-
 .../opensearch/jdbc/types/TimeTypeTest.java   | 57 ++++++++++++++++++-
 4 files changed, 117 insertions(+), 29 deletions(-)

diff --git a/src/main/java/org/opensearch/jdbc/ResultSetImpl.java b/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
index 3cace79..e34a42d 100644
--- a/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
+++ b/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
@@ -347,14 +347,18 @@ private Date getDateX(int columnIndex, Calendar calendar) throws SQLException {
     public Time getTime(int columnIndex) throws SQLException {
         log.debug(() -> logEntry("getTime (%d)", columnIndex));
         checkCursorOperationPossible();
-        Time value = getTimeX(columnIndex);
+        Time value = getTimeX(columnIndex, null);
         log.debug(() -> logExit("getTime", value));
         return value;
     }
 
-    private Time getTimeX(int columnIndex) throws SQLException {
-        // TODO - add/check support
-        return getObjectX(columnIndex, Time.class);
+    private Time getTimeX(int columnIndex, Calendar calendar) throws SQLException {
+        Map<String, Object> conversionParams = null;
+        if (calendar != null) {
+            conversionParams = new HashMap<>();
+            conversionParams.put("calendar", calendar);
+        }
+        return getObjectX(columnIndex, Time.class, conversionParams);
     }
 
     @Override
@@ -494,7 +498,7 @@ public Date getDate(String columnLabel) throws SQLException {
     public Time getTime(String columnLabel) throws SQLException {
         log.debug(() -> logEntry("getTime (%s)", columnLabel));
         checkCursorOperationPossible();
-        Time value = getTimeX(getColumnIndex(columnLabel));
+        Time value = getTimeX(getColumnIndex(columnLabel), null);
         log.debug(() -> logExit("getTime", value));
         return value;
     }
@@ -1071,14 +1075,22 @@ public Date getDate(String columnLabel, Calendar cal) throws SQLException {
 
     @Override
     public Time getTime(int columnIndex, Calendar cal) throws SQLException {
-        // TODO - implement?
-        return null;
+        log.debug(() -> logEntry("getTime (%d, %s)", columnIndex,
+                cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
+        checkCursorOperationPossible();
+        Time value = getTimeX(columnIndex, cal);
+        log.debug(() -> logExit("getTime", value));
+        return value;
     }
 
     @Override
     public Time getTime(String columnLabel, Calendar cal) throws SQLException {
-        // TODO - implement?
-        return null;
+        log.debug(() -> logEntry("getTime (%s, %s)", columnLabel,
+                cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
+        checkCursorOperationPossible();
+        Time value = getTimeX(getColumnIndex(columnLabel), cal);
+        log.debug(() -> logExit("getTime", value));
+        return value;
     }
 
     @Override
diff --git a/src/main/java/org/opensearch/jdbc/types/TimeType.java b/src/main/java/org/opensearch/jdbc/types/TimeType.java
index ad44c7f..74cc51c 100644
--- a/src/main/java/org/opensearch/jdbc/types/TimeType.java
+++ b/src/main/java/org/opensearch/jdbc/types/TimeType.java
@@ -8,7 +8,13 @@
 
 import java.sql.SQLException;
 import java.sql.Time;
-import java.time.LocalTime;
+import java.sql.Timestamp;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.TemporalAccessor;
+import java.util.Calendar;
+import java.util.Locale;
 import java.util.Map;
 
 public class TimeType implements TypeHelper<Time>{
@@ -24,41 +30,56 @@ public Time fromValue(Object value, Map<String, Object> conversionParams) throws
     if (value == null) {
       return null;
     }
+    Calendar calendar = conversionParams != null ? (Calendar) conversionParams.get("calendar") : null;
     if (value instanceof Time) {
-      return asTime((Time) value);
+      return (Time) value;
     } else if (value instanceof String) {
-      return asTime((String) value);
+      return asTime((String) value, calendar);
     } else if (value instanceof Number) {
-      return this.asTime((Number) value);
+      return asTime((Number) value);
     } else {
       throw objectConversionException(value);
     }
   }
 
-  public Time asTime(Time value) {
-    return localTimetoSqlTime(value.toLocalTime());
-  }
+  public Time asTime(String value, Calendar calendar) {
+    Time time;
+    LocalDateTime localDateTime;
 
-  public Time asTime(String value) throws SQLException {
-    return localTimetoSqlTime(toLocalTime(value));
-  }
+    if (value.length() > 10) {
+      TemporalAccessor temporal = DateTimeFormatter
+              .ofPattern("yyyy-MM-dd HH:mm:ss", Locale.getDefault())
+              .parse(value);
+
+      localDateTime = LocalDateTime.from(temporal);
+      time = Time.valueOf(localDateTime.toLocalTime());
+    } else {
+      time = Time.valueOf(value);
+    }
+
+    if (calendar == null) {
+      return time;
+    }
 
-  private Time localTimetoSqlTime(LocalTime localTime) {
-    return new Time(localTime.getHour(), localTime.getMinute(), localTime.getSecond());
+    localDateTime = time.toLocalTime().atDate(LocalDate.now());
+
+    return localDateTimeToTime(localDateTime, calendar);
   }
 
   public Time asTime(Number value) {
     return new Time(value.longValue());
   }
 
-  private LocalTime toLocalTime(String value) throws SQLException {
-    if (value == null)
-      throw stringConversionException(value, null);
-    return LocalTime.parse(value);
-  }
-
   @Override
   public String getTypeName() {
     return "Time";
   }
+
+  private Time localDateTimeToTime(LocalDateTime ldt, Calendar calendar) {
+    calendar.set(ldt.getYear(), ldt.getMonthValue()-1, ldt.getDayOfMonth(),
+            ldt.getHour(), ldt.getMinute(), ldt.getSecond());
+    calendar.set(Calendar.MILLISECOND, ldt.getNano()/1000000);
+
+    return new Time(new Timestamp(calendar.getTimeInMillis()).getTime());
+  }
 }
diff --git a/src/main/java/org/opensearch/jdbc/types/TypeConverters.java b/src/main/java/org/opensearch/jdbc/types/TypeConverters.java
index 2dce3e8..189e970 100644
--- a/src/main/java/org/opensearch/jdbc/types/TypeConverters.java
+++ b/src/main/java/org/opensearch/jdbc/types/TypeConverters.java
@@ -66,7 +66,7 @@ public static class TimestampTypeConverter extends BaseTypeConverter {
 
         private static final Set<Class> supportedJavaClasses = Collections.unmodifiableSet(
                 new HashSet<>(Arrays.asList(
-                        String.class, Timestamp.class
+                        String.class, Timestamp.class, Time.class
                 )));
 
         private TimestampTypeConverter() {
diff --git a/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java b/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
index 263c1dd..072d0df 100644
--- a/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
+++ b/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
@@ -8,8 +8,13 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import com.google.common.collect.ImmutableMap;
+import org.junit.jupiter.api.Test;
 import org.opensearch.jdbc.test.UTCTimeZoneTestExtension;
 import java.sql.Time;
+import java.time.LocalTime;
+import java.util.Calendar;
+
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.extension.ExtendWith;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -22,11 +27,61 @@ public class TimeTypeTest {
   @CsvSource(value = {
       "00:00:00, 00:00:00",
       "01:01:01, 01:01:01",
-      "23:59:59, 23:59:59"
+      "23:59:59, 23:59:59",
+      "1880-12-22 00:00:00, 00:00:00",
+      "2000-01-10 01:01:01, 01:01:01",
+      "1998-08-17 23:59:59, 23:59:59"
   })
   void testTimeFromString(String inputString, String resultString) {
     Time time = Assertions.assertDoesNotThrow(
         () -> TimeType.INSTANCE.fromValue(inputString, null));
     assertEquals(resultString, time.toString());
   }
+
+  @ParameterizedTest
+  @CsvSource(value = {
+          "00:00:00, 00:00:00",
+          "01:01:01, 01:01:01",
+          "23:59:59, 23:59:59"
+  })
+  void testTimeFromStringWithCalendar(String inputString, String resultString) {
+    Time time = Assertions.assertDoesNotThrow(
+            () -> TimeType.INSTANCE.fromValue(
+                    inputString, ImmutableMap.of("calendar", Calendar.getInstance())));
+    assertEquals(resultString, time.toString());
+  }
+
+  @Test
+  void testTimeFromTime() {
+    Time timeNow = Time.valueOf(LocalTime.now());
+    Time time = Assertions.assertDoesNotThrow(
+            () -> TimeType.INSTANCE.fromValue(timeNow, null));
+    assertEquals(timeNow, time);
+  }
+
+  @Test
+  void testTimeFromTimeWithCalendar() {
+    Time timeNow = Time.valueOf(LocalTime.now());
+    Time time = Assertions.assertDoesNotThrow(
+            () -> TimeType.INSTANCE.fromValue(
+                    timeNow, ImmutableMap.of("calendar", Calendar.getInstance())));
+    assertEquals(timeNow, time);
+  }
+
+  @Test
+  void testTimeFromNumber() {
+    long currentTimestamp = System.currentTimeMillis();
+    Time time = Assertions.assertDoesNotThrow(
+            () -> TimeType.INSTANCE.fromValue(currentTimestamp, null));
+    assertEquals(currentTimestamp, time.getTime());
+  }
+
+  @Test
+  void testTimeFromNumberWithCalendar() {
+    long currentTimestamp = System.currentTimeMillis();
+    Time time = Assertions.assertDoesNotThrow(
+            () -> TimeType.INSTANCE.fromValue(
+                    currentTimestamp, ImmutableMap.of("calendar", Calendar.getInstance())));
+    assertEquals(currentTimestamp, time.getTime());
+  }
 }

From 69420d5acc02aa8c62add7f2011c874c8eaac25b Mon Sep 17 00:00:00 2001
From: Margarit Hakobyan <margarit.hakobyan@improving.com>
Date: Wed, 21 Dec 2022 14:19:49 -0800
Subject: [PATCH 2/5] Add more test cases

Signed-off-by: Margarit Hakobyan <margarit.hakobyan@improving.com>
---
 src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java b/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
index 072d0df..5e3a688 100644
--- a/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
+++ b/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
@@ -42,7 +42,10 @@ void testTimeFromString(String inputString, String resultString) {
   @CsvSource(value = {
           "00:00:00, 00:00:00",
           "01:01:01, 01:01:01",
-          "23:59:59, 23:59:59"
+          "23:59:59, 23:59:59",
+          "1880-12-22 00:00:00, 00:00:00",
+          "2000-01-10 01:01:01, 01:01:01",
+          "1998-08-17 23:59:59, 23:59:59"
   })
   void testTimeFromStringWithCalendar(String inputString, String resultString) {
     Time time = Assertions.assertDoesNotThrow(

From 0a36aced8bf6903165c856dadfcdc212582c0f58 Mon Sep 17 00:00:00 2001
From: Margarit Hakobyan <margarit.hakobyan@improving.com>
Date: Wed, 21 Dec 2022 14:36:11 -0800
Subject: [PATCH 3/5] Fix indentation

Signed-off-by: Margarit Hakobyan <margarit.hakobyan@improving.com>
---
 .../java/org/opensearch/jdbc/types/TimeTypeTest.java | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java b/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
index 5e3a688..bec269e 100644
--- a/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
+++ b/src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java
@@ -40,12 +40,12 @@ void testTimeFromString(String inputString, String resultString) {
 
   @ParameterizedTest
   @CsvSource(value = {
-          "00:00:00, 00:00:00",
-          "01:01:01, 01:01:01",
-          "23:59:59, 23:59:59",
-          "1880-12-22 00:00:00, 00:00:00",
-          "2000-01-10 01:01:01, 01:01:01",
-          "1998-08-17 23:59:59, 23:59:59"
+      "00:00:00, 00:00:00",
+      "01:01:01, 01:01:01",
+      "23:59:59, 23:59:59",
+      "1880-12-22 00:00:00, 00:00:00",
+      "2000-01-10 01:01:01, 01:01:01",
+      "1998-08-17 23:59:59, 23:59:59"
   })
   void testTimeFromStringWithCalendar(String inputString, String resultString) {
     Time time = Assertions.assertDoesNotThrow(

From b1f8e06b72614396cb0f17378d1a50605046fb90 Mon Sep 17 00:00:00 2001
From: Margarit Hakobyan <margarit.hakobyan@improving.com>
Date: Tue, 3 Jan 2023 11:11:21 -0800
Subject: [PATCH 4/5] Address PR review feedback

Signed-off-by: Margarit Hakobyan <margarit.hakobyan@improving.com>
---
 .../org/opensearch/jdbc/ResultSetImpl.java    | 22 ++++++++++++++-----
 .../org/opensearch/jdbc/types/TimeType.java   |  5 +++--
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/src/main/java/org/opensearch/jdbc/ResultSetImpl.java b/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
index e34a42d..cb6205d 100644
--- a/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
+++ b/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
@@ -1078,9 +1078,14 @@ public Time getTime(int columnIndex, Calendar cal) throws SQLException {
         log.debug(() -> logEntry("getTime (%d, %s)", columnIndex,
                 cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
         checkCursorOperationPossible();
-        Time value = getTimeX(columnIndex, cal);
-        log.debug(() -> logExit("getTime", value));
-        return value;
+        log.debug(() -> {
+            try {
+                return logExit("getTime", getTimeX(columnIndex, cal));
+            } catch (SQLException e) {
+                throw new RuntimeException(e);
+            }
+        });
+        return getTimeX(columnIndex, cal);
     }
 
     @Override
@@ -1088,9 +1093,14 @@ public Time getTime(String columnLabel, Calendar cal) throws SQLException {
         log.debug(() -> logEntry("getTime (%s, %s)", columnLabel,
                 cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
         checkCursorOperationPossible();
-        Time value = getTimeX(getColumnIndex(columnLabel), cal);
-        log.debug(() -> logExit("getTime", value));
-        return value;
+        log.debug(() -> {
+            try {
+                return logExit("getTime", getTimeX(getColumnIndex(columnLabel), cal));
+            } catch (SQLException e) {
+                throw new RuntimeException(e);
+            }
+        });
+        return getTimeX(getColumnIndex(columnLabel), cal);
     }
 
     @Override
diff --git a/src/main/java/org/opensearch/jdbc/types/TimeType.java b/src/main/java/org/opensearch/jdbc/types/TimeType.java
index 74cc51c..73593c3 100644
--- a/src/main/java/org/opensearch/jdbc/types/TimeType.java
+++ b/src/main/java/org/opensearch/jdbc/types/TimeType.java
@@ -12,6 +12,7 @@
 import java.time.LocalDate;
 import java.time.LocalDateTime;
 import java.time.format.DateTimeFormatter;
+import java.time.format.DateTimeParseException;
 import java.time.temporal.TemporalAccessor;
 import java.util.Calendar;
 import java.util.Locale;
@@ -46,14 +47,14 @@ public Time asTime(String value, Calendar calendar) {
     Time time;
     LocalDateTime localDateTime;
 
-    if (value.length() > 10) {
+    try {
       TemporalAccessor temporal = DateTimeFormatter
               .ofPattern("yyyy-MM-dd HH:mm:ss", Locale.getDefault())
               .parse(value);
 
       localDateTime = LocalDateTime.from(temporal);
       time = Time.valueOf(localDateTime.toLocalTime());
-    } else {
+    } catch (DateTimeParseException exception) {
       time = Time.valueOf(value);
     }
 

From b1de4b8ddfd214a347d5f9618ad362ac2022d19a Mon Sep 17 00:00:00 2001
From: Margarit Hakobyan <margarit.hakobyan@improving.com>
Date: Tue, 3 Jan 2023 11:31:54 -0800
Subject: [PATCH 5/5] Reverted ResultSetImpl latest changes

Signed-off-by: Margarit Hakobyan <margarit.hakobyan@improving.com>
---
 .../org/opensearch/jdbc/ResultSetImpl.java    | 22 +++++--------------
 1 file changed, 6 insertions(+), 16 deletions(-)

diff --git a/src/main/java/org/opensearch/jdbc/ResultSetImpl.java b/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
index cb6205d..e34a42d 100644
--- a/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
+++ b/src/main/java/org/opensearch/jdbc/ResultSetImpl.java
@@ -1078,14 +1078,9 @@ public Time getTime(int columnIndex, Calendar cal) throws SQLException {
         log.debug(() -> logEntry("getTime (%d, %s)", columnIndex,
                 cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
         checkCursorOperationPossible();
-        log.debug(() -> {
-            try {
-                return logExit("getTime", getTimeX(columnIndex, cal));
-            } catch (SQLException e) {
-                throw new RuntimeException(e);
-            }
-        });
-        return getTimeX(columnIndex, cal);
+        Time value = getTimeX(columnIndex, cal);
+        log.debug(() -> logExit("getTime", value));
+        return value;
     }
 
     @Override
@@ -1093,14 +1088,9 @@ public Time getTime(String columnLabel, Calendar cal) throws SQLException {
         log.debug(() -> logEntry("getTime (%s, %s)", columnLabel,
                 cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
         checkCursorOperationPossible();
-        log.debug(() -> {
-            try {
-                return logExit("getTime", getTimeX(getColumnIndex(columnLabel), cal));
-            } catch (SQLException e) {
-                throw new RuntimeException(e);
-            }
-        });
-        return getTimeX(getColumnIndex(columnLabel), cal);
+        Time value = getTimeX(getColumnIndex(columnLabel), cal);
+        log.debug(() -> logExit("getTime", value));
+        return value;
     }
 
     @Override