Skip to content

Commit cc5b0f7

Browse files
opensearch-trigger-bot[bot]margarit-h
opensearch-trigger-bot[bot]
and
margarit-h
authored
Fixes "ResultSet.getTime() returns null" bug (#1) (#41) (#51)
* Fixes ResultSet.getTime returns null bug Signed-off-by: Margarit Hakobyan <[email protected]> Signed-off-by: Margarit Hakobyan <[email protected]> (cherry picked from commit 2e4aec2) Co-authored-by: Margarit Hakobyan <[email protected]>
1 parent 877a70d commit cc5b0f7

File tree

4 files changed

+121
-29
lines changed

4 files changed

+121
-29
lines changed

‎src/main/java/org/opensearch/jdbc/ResultSetImpl.java

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -347,14 +347,18 @@ private Date getDateX(int columnIndex, Calendar calendar) throws SQLException {
347347
public Time getTime(int columnIndex) throws SQLException {
348348
log.debug(() -> logEntry("getTime (%d)", columnIndex));
349349
checkCursorOperationPossible();
350-
Time value = getTimeX(columnIndex);
350+
Time value = getTimeX(columnIndex, null);
351351
log.debug(() -> logExit("getTime", value));
352352
return value;
353353
}
354354

355-
private Time getTimeX(int columnIndex) throws SQLException {
356-
// TODO - add/check support
357-
return getObjectX(columnIndex, Time.class);
355+
private Time getTimeX(int columnIndex, Calendar calendar) throws SQLException {
356+
Map<String, Object> conversionParams = null;
357+
if (calendar != null) {
358+
conversionParams = new HashMap<>();
359+
conversionParams.put("calendar", calendar);
360+
}
361+
return getObjectX(columnIndex, Time.class, conversionParams);
358362
}
359363

360364
@Override
@@ -494,7 +498,7 @@ public Date getDate(String columnLabel) throws SQLException {
494498
public Time getTime(String columnLabel) throws SQLException {
495499
log.debug(() -> logEntry("getTime (%s)", columnLabel));
496500
checkCursorOperationPossible();
497-
Time value = getTimeX(getColumnIndex(columnLabel));
501+
Time value = getTimeX(getColumnIndex(columnLabel), null);
498502
log.debug(() -> logExit("getTime", value));
499503
return value;
500504
}
@@ -1071,14 +1075,22 @@ public Date getDate(String columnLabel, Calendar cal) throws SQLException {
10711075

10721076
@Override
10731077
public Time getTime(int columnIndex, Calendar cal) throws SQLException {
1074-
// TODO - implement?
1075-
return null;
1078+
log.debug(() -> logEntry("getTime (%d, %s)", columnIndex,
1079+
cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
1080+
checkCursorOperationPossible();
1081+
Time value = getTimeX(columnIndex, cal);
1082+
log.debug(() -> logExit("getTime", value));
1083+
return value;
10761084
}
10771085

10781086
@Override
10791087
public Time getTime(String columnLabel, Calendar cal) throws SQLException {
1080-
// TODO - implement?
1081-
return null;
1088+
log.debug(() -> logEntry("getTime (%s, %s)", columnLabel,
1089+
cal == null ? "null" : "Calendar TZ= " + cal.getTimeZone()));
1090+
checkCursorOperationPossible();
1091+
Time value = getTimeX(getColumnIndex(columnLabel), cal);
1092+
log.debug(() -> logExit("getTime", value));
1093+
return value;
10821094
}
10831095

10841096
@Override

‎src/main/java/org/opensearch/jdbc/types/TimeType.java

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,14 @@
88

99
import java.sql.SQLException;
1010
import java.sql.Time;
11-
import java.time.LocalTime;
11+
import java.sql.Timestamp;
12+
import java.time.LocalDate;
13+
import java.time.LocalDateTime;
14+
import java.time.format.DateTimeFormatter;
15+
import java.time.format.DateTimeParseException;
16+
import java.time.temporal.TemporalAccessor;
17+
import java.util.Calendar;
18+
import java.util.Locale;
1219
import java.util.Map;
1320

1421
public class TimeType implements TypeHelper<Time>{
@@ -24,41 +31,56 @@ public Time fromValue(Object value, Map<String, Object> conversionParams) throws
2431
if (value == null) {
2532
return null;
2633
}
34+
Calendar calendar = conversionParams != null ? (Calendar) conversionParams.get("calendar") : null;
2735
if (value instanceof Time) {
28-
return asTime((Time) value);
36+
return (Time) value;
2937
} else if (value instanceof String) {
30-
return asTime((String) value);
38+
return asTime((String) value, calendar);
3139
} else if (value instanceof Number) {
32-
return this.asTime((Number) value);
40+
return asTime((Number) value);
3341
} else {
3442
throw objectConversionException(value);
3543
}
3644
}
3745

38-
public Time asTime(Time value) {
39-
return localTimetoSqlTime(value.toLocalTime());
40-
}
46+
public Time asTime(String value, Calendar calendar) {
47+
Time time;
48+
LocalDateTime localDateTime;
4149

42-
public Time asTime(String value) throws SQLException {
43-
return localTimetoSqlTime(toLocalTime(value));
44-
}
50+
try {
51+
TemporalAccessor temporal = DateTimeFormatter
52+
.ofPattern("yyyy-MM-dd HH:mm:ss", Locale.getDefault())
53+
.parse(value);
54+
55+
localDateTime = LocalDateTime.from(temporal);
56+
time = Time.valueOf(localDateTime.toLocalTime());
57+
} catch (DateTimeParseException exception) {
58+
time = Time.valueOf(value);
59+
}
60+
61+
if (calendar == null) {
62+
return time;
63+
}
4564

46-
private Time localTimetoSqlTime(LocalTime localTime) {
47-
return new Time(localTime.getHour(), localTime.getMinute(), localTime.getSecond());
65+
localDateTime = time.toLocalTime().atDate(LocalDate.now());
66+
67+
return localDateTimeToTime(localDateTime, calendar);
4868
}
4969

5070
public Time asTime(Number value) {
5171
return new Time(value.longValue());
5272
}
5373

54-
private LocalTime toLocalTime(String value) throws SQLException {
55-
if (value == null)
56-
throw stringConversionException(value, null);
57-
return LocalTime.parse(value);
58-
}
59-
6074
@Override
6175
public String getTypeName() {
6276
return "Time";
6377
}
78+
79+
private Time localDateTimeToTime(LocalDateTime ldt, Calendar calendar) {
80+
calendar.set(ldt.getYear(), ldt.getMonthValue()-1, ldt.getDayOfMonth(),
81+
ldt.getHour(), ldt.getMinute(), ldt.getSecond());
82+
calendar.set(Calendar.MILLISECOND, ldt.getNano()/1000000);
83+
84+
return new Time(new Timestamp(calendar.getTimeInMillis()).getTime());
85+
}
6486
}

‎src/main/java/org/opensearch/jdbc/types/TypeConverters.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public static class TimestampTypeConverter extends BaseTypeConverter {
6666

6767
private static final Set<Class> supportedJavaClasses = Collections.unmodifiableSet(
6868
new HashSet<>(Arrays.asList(
69-
String.class, Timestamp.class
69+
String.class, Timestamp.class, Time.class
7070
)));
7171

7272
private TimestampTypeConverter() {

‎src/test/java/org/opensearch/jdbc/types/TimeTypeTest.java

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,13 @@
88

99
import static org.junit.jupiter.api.Assertions.assertEquals;
1010

11+
import com.google.common.collect.ImmutableMap;
12+
import org.junit.jupiter.api.Test;
1113
import org.opensearch.jdbc.test.UTCTimeZoneTestExtension;
1214
import java.sql.Time;
15+
import java.time.LocalTime;
16+
import java.util.Calendar;
17+
1318
import org.junit.jupiter.api.Assertions;
1419
import org.junit.jupiter.api.extension.ExtendWith;
1520
import org.junit.jupiter.params.ParameterizedTest;
@@ -22,11 +27,64 @@ public class TimeTypeTest {
2227
@CsvSource(value = {
2328
"00:00:00, 00:00:00",
2429
"01:01:01, 01:01:01",
25-
"23:59:59, 23:59:59"
30+
"23:59:59, 23:59:59",
31+
"1880-12-22 00:00:00, 00:00:00",
32+
"2000-01-10 01:01:01, 01:01:01",
33+
"1998-08-17 23:59:59, 23:59:59"
2634
})
2735
void testTimeFromString(String inputString, String resultString) {
2836
Time time = Assertions.assertDoesNotThrow(
2937
() -> TimeType.INSTANCE.fromValue(inputString, null));
3038
assertEquals(resultString, time.toString());
3139
}
40+
41+
@ParameterizedTest
42+
@CsvSource(value = {
43+
"00:00:00, 00:00:00",
44+
"01:01:01, 01:01:01",
45+
"23:59:59, 23:59:59",
46+
"1880-12-22 00:00:00, 00:00:00",
47+
"2000-01-10 01:01:01, 01:01:01",
48+
"1998-08-17 23:59:59, 23:59:59"
49+
})
50+
void testTimeFromStringWithCalendar(String inputString, String resultString) {
51+
Time time = Assertions.assertDoesNotThrow(
52+
() -> TimeType.INSTANCE.fromValue(
53+
inputString, ImmutableMap.of("calendar", Calendar.getInstance())));
54+
assertEquals(resultString, time.toString());
55+
}
56+
57+
@Test
58+
void testTimeFromTime() {
59+
Time timeNow = Time.valueOf(LocalTime.now());
60+
Time time = Assertions.assertDoesNotThrow(
61+
() -> TimeType.INSTANCE.fromValue(timeNow, null));
62+
assertEquals(timeNow, time);
63+
}
64+
65+
@Test
66+
void testTimeFromTimeWithCalendar() {
67+
Time timeNow = Time.valueOf(LocalTime.now());
68+
Time time = Assertions.assertDoesNotThrow(
69+
() -> TimeType.INSTANCE.fromValue(
70+
timeNow, ImmutableMap.of("calendar", Calendar.getInstance())));
71+
assertEquals(timeNow, time);
72+
}
73+
74+
@Test
75+
void testTimeFromNumber() {
76+
long currentTimestamp = System.currentTimeMillis();
77+
Time time = Assertions.assertDoesNotThrow(
78+
() -> TimeType.INSTANCE.fromValue(currentTimestamp, null));
79+
assertEquals(currentTimestamp, time.getTime());
80+
}
81+
82+
@Test
83+
void testTimeFromNumberWithCalendar() {
84+
long currentTimestamp = System.currentTimeMillis();
85+
Time time = Assertions.assertDoesNotThrow(
86+
() -> TimeType.INSTANCE.fromValue(
87+
currentTimestamp, ImmutableMap.of("calendar", Calendar.getInstance())));
88+
assertEquals(currentTimestamp, time.getTime());
89+
}
3290
}

0 commit comments

Comments
 (0)