Skip to content

Commit e4d41d3

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 400d420 commit e4d41d3

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,26 +10,31 @@
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

2225
import static org.elasticsearch.xpack.sql.action.BasicFormatter.FormatOption.CLI;
2326

24-
public abstract class JdbcTestUtils {
27+
final class JdbcTestUtils {
2528

26-
public static final String SQL_TRACE = "org.elasticsearch.xpack.sql:TRACE";
29+
private JdbcTestUtils() {}
2730

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

32-
public static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLException {
37+
static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLException {
3338
ResultSetMetaData metaData = rs.getMetaData();
3439
// header
3540
StringBuilder sb = new StringBuilder();
@@ -59,35 +64,24 @@ public static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLE
5964
logger.info(sb.toString());
6065
}
6166

62-
private static final int MAX_WIDTH = 20;
63-
64-
public static void logResultSetData(ResultSet rs, Logger log) throws SQLException {
67+
static void logResultSetData(ResultSet rs, Logger log) throws SQLException {
6568
ResultSetMetaData metaData = rs.getMetaData();
66-
StringBuilder sb = new StringBuilder();
67-
StringBuilder column = new StringBuilder();
6869

6970
int columns = metaData.getColumnCount();
7071

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

84-
public static String resultSetCurrentData(ResultSet rs) throws SQLException {
77+
static String resultSetCurrentData(ResultSet rs) throws SQLException {
8578
ResultSetMetaData metaData = rs.getMetaData();
86-
StringBuilder column = new StringBuilder();
87-
88-
int columns = metaData.getColumnCount();
79+
return rowAsString(rs, metaData.getColumnCount());
80+
}
8981

82+
private static String rowAsString(ResultSet rs, int columns) throws SQLException {
9083
StringBuilder sb = new StringBuilder();
84+
StringBuilder column = new StringBuilder();
9185
for (int i = 1; i <= columns; i++) {
9286
column.setLength(0);
9387
if (i > 1) {
@@ -137,7 +131,18 @@ public static void logLikeCLI(ResultSet rs, Logger logger) throws SQLException {
137131
logger.info("\n" + formatter.formatWithHeader(cols, data));
138132
}
139133

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

‎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
@@ -34,7 +34,6 @@
3434
import java.sql.Types;
3535
import java.time.Instant;
3636
import java.time.ZoneId;
37-
import java.time.ZonedDateTime;
3837
import java.util.Arrays;
3938
import java.util.Calendar;
4039
import java.util.Date;
@@ -61,6 +60,8 @@
6160
import static java.util.Calendar.SECOND;
6261
import static java.util.Calendar.YEAR;
6362
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.JDBC_TIMEZONE;
63+
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.asDate;
64+
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.asTime;
6465
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.of;
6566

6667
public class ResultSetTestCase extends JdbcIntegrationTestCase {
@@ -880,10 +881,7 @@ public void testGettingDateWithoutCalendar() throws Exception {
880881
doWithQuery(SELECT_ALL_FIELDS, (results) -> {
881882
results.next();
882883

883-
ZoneId zoneId = ZoneId.of(timeZoneId);
884-
java.sql.Date expectedDate = new java.sql.Date(
885-
ZonedDateTime.ofInstant(Instant.ofEpochMilli(randomLongDate), zoneId)
886-
.toLocalDate().atStartOfDay(zoneId).toInstant().toEpochMilli());
884+
java.sql.Date expectedDate = asDate(randomLongDate, getZoneFromOffset(randomLongDate));
887885

888886
assertEquals(expectedDate, results.getDate("test_date"));
889887
assertEquals(expectedDate, results.getDate(9));
@@ -939,11 +937,11 @@ public void testGettingTimeWithoutCalendar() throws Exception {
939937
});
940938
Long randomLongDate = randomNonNegativeLong();
941939
indexSimpleDocumentWithTrueValues(randomLongDate);
942-
940+
943941
doWithQuery(SELECT_ALL_FIELDS, (results) -> {
944942
results.next();
945943

946-
java.sql.Time expectedTime = new java.sql.Time(randomLongDate % 86400000L);
944+
java.sql.Time expectedTime = asTime(randomLongDate, getZoneFromOffset(randomLongDate));
947945

948946
assertEquals(expectedTime, results.getTime("test_date"));
949947
assertEquals(expectedTime, results.getTime(9));
@@ -953,7 +951,7 @@ public void testGettingTimeWithoutCalendar() throws Exception {
953951
validateErrorsForTimeTestsWithoutCalendar(results::getTime);
954952
});
955953
}
956-
954+
957955
public void testGettingTimeWithCalendar() throws Exception {
958956
createIndex("test");
959957
updateMappingForNumericValuesTests("test");
@@ -1752,4 +1750,8 @@ private Connection esWithLeniency(boolean multiValueLeniency) throws SQLExceptio
17521750
private String asDateString(long millis) {
17531751
return of(millis, timeZoneId);
17541752
}
1753+
1754+
private ZoneId getZoneFromOffset(Long randomLongDate) {
1755+
return ZoneId.of(ZoneId.of(timeZoneId).getRules().getOffset(Instant.ofEpochMilli(randomLongDate)).toString());
1756+
}
17551757
}

0 commit comments

Comments
 (0)