Skip to content

Commit f6072e5

Browse files
matrivmatriv
committed
SQL: Fix getTime() methods in JDBC (#40484)
Previously, `getTime(colIdx/colLabel)` and `getObject(colIdx/colLabel, java.sql.Time.class)` methods were computing the time from a `ZonedDateTime` by applying day in millis modulo on the epoch millis of the `ZonedDateTime` object. This is wrong as we need to keep the time related fields at the timezone of the `ZonedDateTime` object and just set the date info to the epoch date (01/01/1970). Additionally fixes a testing issue as the original timezone id is converted to an offset string when parsing the response from the server.
1 parent c4848c5 commit f6072e5

File tree

3 files changed

+45
-53
lines changed

3 files changed

+45
-53
lines changed

‎x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDateUtils.java‎

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import java.sql.Date;
1010
import java.sql.Time;
1111
import java.sql.Timestamp;
12+
import java.time.LocalDate;
1213
import java.time.ZonedDateTime;
1314
import java.time.format.DateTimeFormatter;
1415
import java.time.format.DateTimeFormatterBuilder;
@@ -27,10 +28,9 @@
2728
*/
2829
final class JdbcDateUtils {
2930

30-
private JdbcDateUtils() {
31-
}
31+
private JdbcDateUtils() {}
3232

33-
private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000L;
33+
private static final LocalDate EPOCH = LocalDate.of(1970, 1, 1);
3434

3535
static final DateTimeFormatter ISO_WITH_MILLIS = new DateTimeFormatterBuilder()
3636
.parseCaseInsensitive()
@@ -58,20 +58,9 @@ static Date asDate(String date) {
5858
return new Date(zdt.toLocalDate().atStartOfDay(zdt.getZone()).toInstant().toEpochMilli());
5959
}
6060

61-
/**
62-
* In contrast to {@link JdbcDateUtils#asDate(String)} here we just want to eliminate
63-
* the date part and just set it to EPOCH (1970-01-1)
64-
*/
65-
static Time asTime(long millisSinceEpoch) {
66-
return new Time(utcMillisRemoveDate(millisSinceEpoch));
67-
}
68-
69-
/**
70-
* In contrast to {@link JdbcDateUtils#asDate(String)} here we just want to eliminate
71-
* the date part and just set it to EPOCH (1970-01-1)
72-
*/
7361
static Time asTime(String date) {
74-
return asTime(asMillisSinceEpoch(date));
62+
ZonedDateTime zdt = asDateTime(date);
63+
return new Time(zdt.toLocalTime().atDate(EPOCH).atZone(zdt.getZone()).toInstant().toEpochMilli());
7564
}
7665

7766
static Timestamp asTimestamp(long millisSinceEpoch) {
@@ -93,8 +82,4 @@ static <R> R asDateTimeField(Object value, Function<String, R> asDateTimeMethod,
9382
return ctor.apply(((Number) value).longValue());
9483
}
9584
}
96-
97-
private static long utcMillisRemoveDate(long l) {
98-
return l % DAY_IN_MILLIS;
99-
}
10085
}

‎x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java‎

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -10,24 +10,29 @@
1010
import org.elasticsearch.xpack.sql.proto.ColumnInfo;
1111
import org.elasticsearch.xpack.sql.proto.StringUtils;
1212

13+
import java.sql.Date;
1314
import java.sql.ResultSet;
1415
import java.sql.ResultSetMetaData;
1516
import java.sql.SQLException;
17+
import java.sql.Time;
1618
import java.time.Instant;
19+
import java.time.LocalDate;
1720
import java.time.ZoneId;
1821
import java.time.ZonedDateTime;
1922
import java.util.ArrayList;
2023
import java.util.List;
2124

22-
public abstract class JdbcTestUtils {
25+
final class JdbcTestUtils {
2326

24-
public static final String SQL_TRACE = "org.elasticsearch.xpack.sql:TRACE";
27+
private JdbcTestUtils() {}
2528

26-
public static final String JDBC_TIMEZONE = "timezone";
27-
28-
public static ZoneId UTC = ZoneId.of("Z");
29+
private static final int MAX_WIDTH = 20;
30+
31+
static final String SQL_TRACE = "org.elasticsearch.xpack.sql:TRACE";
32+
static final String JDBC_TIMEZONE = "timezone";
33+
static final LocalDate EPOCH = LocalDate.of(1970, 1, 1);
2934

30-
public static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLException {
35+
static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLException {
3136
ResultSetMetaData metaData = rs.getMetaData();
3237
// header
3338
StringBuilder sb = new StringBuilder();
@@ -57,35 +62,24 @@ public static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLE
5762
logger.info(sb.toString());
5863
}
5964

60-
private static final int MAX_WIDTH = 20;
61-
62-
public static void logResultSetData(ResultSet rs, Logger log) throws SQLException {
65+
static void logResultSetData(ResultSet rs, Logger log) throws SQLException {
6366
ResultSetMetaData metaData = rs.getMetaData();
64-
StringBuilder sb = new StringBuilder();
65-
StringBuilder column = new StringBuilder();
6667

6768
int columns = metaData.getColumnCount();
6869

6970
while (rs.next()) {
70-
sb.setLength(0);
71-
for (int i = 1; i <= columns; i++) {
72-
column.setLength(0);
73-
if (i > 1) {
74-
sb.append(" | ");
75-
}
76-
sb.append(trimOrPad(column.append(rs.getString(i))));
77-
}
78-
log.info(sb);
71+
log.info(rowAsString(rs, columns));
7972
}
8073
}
8174

82-
public static String resultSetCurrentData(ResultSet rs) throws SQLException {
75+
static String resultSetCurrentData(ResultSet rs) throws SQLException {
8376
ResultSetMetaData metaData = rs.getMetaData();
84-
StringBuilder column = new StringBuilder();
85-
86-
int columns = metaData.getColumnCount();
77+
return rowAsString(rs, metaData.getColumnCount());
78+
}
8779

80+
private static String rowAsString(ResultSet rs, int columns) throws SQLException {
8881
StringBuilder sb = new StringBuilder();
82+
StringBuilder column = new StringBuilder();
8983
for (int i = 1; i <= columns; i++) {
9084
column.setLength(0);
9185
if (i > 1) {
@@ -135,7 +129,18 @@ public static void logLikeCLI(ResultSet rs, Logger logger) throws SQLException {
135129
logger.info("\n" + formatter.formatWithHeader(cols, data));
136130
}
137131

138-
public static String of(long millis, String zoneId) {
132+
static String of(long millis, String zoneId) {
139133
return StringUtils.toString(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneId.of(zoneId)));
140134
}
135+
136+
static Date asDate(long millis, ZoneId zoneId) {
137+
return new java.sql.Date(
138+
ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), zoneId)
139+
.toLocalDate().atStartOfDay(zoneId).toInstant().toEpochMilli());
140+
}
141+
142+
static Time asTime(long millis, ZoneId zoneId) {
143+
return new Time(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), zoneId)
144+
.toLocalTime().atDate(JdbcTestUtils.EPOCH).atZone(zoneId).toInstant().toEpochMilli());
145+
}
141146
}

‎x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
import java.sql.Types;
3636
import java.time.Instant;
3737
import java.time.ZoneId;
38-
import java.time.ZonedDateTime;
3938
import java.util.Arrays;
4039
import java.util.Calendar;
4140
import java.util.Date;
@@ -62,6 +61,8 @@
6261
import static java.util.Calendar.SECOND;
6362
import static java.util.Calendar.YEAR;
6463
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.JDBC_TIMEZONE;
64+
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.asDate;
65+
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.asTime;
6566
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.of;
6667

6768
public class ResultSetTestCase extends JdbcIntegrationTestCase {
@@ -822,10 +823,7 @@ public void testGettingDateWithoutCalendar() throws Exception {
822823
doWithQuery(SELECT_ALL_FIELDS, (results) -> {
823824
results.next();
824825

825-
ZoneId zoneId = ZoneId.of(timeZoneId);
826-
java.sql.Date expectedDate = new java.sql.Date(
827-
ZonedDateTime.ofInstant(Instant.ofEpochMilli(randomLongDate), zoneId)
828-
.toLocalDate().atStartOfDay(zoneId).toInstant().toEpochMilli());
826+
java.sql.Date expectedDate = asDate(randomLongDate, getZoneFromOffset(randomLongDate));
829827

830828
assertEquals(expectedDate, results.getDate("test_date"));
831829
assertEquals(expectedDate, results.getDate(9));
@@ -881,11 +879,11 @@ public void testGettingTimeWithoutCalendar() throws Exception {
881879
});
882880
Long randomLongDate = randomNonNegativeLong();
883881
indexSimpleDocumentWithTrueValues(randomLongDate);
884-
882+
885883
doWithQuery(SELECT_ALL_FIELDS, (results) -> {
886884
results.next();
887885

888-
java.sql.Time expectedTime = new java.sql.Time(randomLongDate % 86400000L);
886+
java.sql.Time expectedTime = asTime(randomLongDate, getZoneFromOffset(randomLongDate));
889887

890888
assertEquals(expectedTime, results.getTime("test_date"));
891889
assertEquals(expectedTime, results.getTime(9));
@@ -895,7 +893,7 @@ public void testGettingTimeWithoutCalendar() throws Exception {
895893
validateErrorsForTimeTestsWithoutCalendar(results::getTime);
896894
});
897895
}
898-
896+
899897
public void testGettingTimeWithCalendar() throws Exception {
900898
createIndex("test");
901899
updateMappingForNumericValuesTests("test");
@@ -1615,4 +1613,8 @@ private Connection useDataSource(String timeZoneId) throws SQLException {
16151613
private String asDateString(long millis) {
16161614
return of(millis, timeZoneId);
16171615
}
1616+
1617+
private ZoneId getZoneFromOffset(Long randomLongDate) {
1618+
return ZoneId.of(ZoneId.of(timeZoneId).getRules().getOffset(Instant.ofEpochMilli(randomLongDate)).toString());
1619+
}
16181620
}

0 commit comments

Comments
 (0)