honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

SQL: Fix issue timezone issues with JDBC getDate/getTime (#40360) 

Previously, `getDate(int columnIdx)/getDate(String columnLabel)` and
were using legacy`java.util.Calendar` instead of the the `java.time.*`
classes to reset to the start of day. This resulted in different results 
for certain timestamps and timezones when calling
`getDate(col)` vs`getObject(col, java.sql.Date)`

Now only the methods (that must be implemented due to the JDBC spec)
`getDate(int columnIdx, Calendar cal)/getDate(String columnLabel, Calendar cal)`
are still using the `java.util.Calendar` for those conversion.

The same change was applied to
`getTime(int columnIdx)/getTime(String columnLabel)`
and
`getTimestamp(int columnIdx)/getTimestamp(String columnLabel)`

Fixes: #40289

(cherry picked from commit 44560671f18397e0c58e3647732880fcb73a5034)
This commit is contained in:
Marios Trivyzas 2019-03-23 16:59:19 +01:00
parent 10bbb082a4
commit 143db10980
No known key found for this signature in database
GPG Key ID: 8817B46B0CF36A3F
3 changed files with 104 additions and 35 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcDateUtils.java
View File

@ -58,12 +58,24 @@ final class JdbcDateUtils {
return new Date(zdt.toLocalDate().atStartOfDay(zdt.getZone()).toInstant().toEpochMilli());
}
/**
* In contrast to {@link JdbcDateUtils#asDate(String)} here we just want to eliminate
* the date part and just set it to EPOCH (1970-01-1)
*/
static Time asTime(long millisSinceEpoch) {
return new Time(utcMillisRemoveDate(millisSinceEpoch));
}
/**
* In contrast to {@link JdbcDateUtils#asDate(String)} here we just want to eliminate
* the date part and just set it to EPOCH (1970-01-1)
*/
static Time asTime(String date) {
return new Time(utcMillisRemoveDate(asMillisSinceEpoch(date)));
return asTime(asMillisSinceEpoch(date));
}
static Timestamp asTimestamp(long millisSinceEpoch) {
return new Timestamp(millisSinceEpoch);
}
static Timestamp asTimestamp(String date) {

69
x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java
View File

@ -178,17 +178,17 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
@Override
public Date getDate(int columnIndex) throws SQLException {
return getDate(columnIndex, null);
return asDate(columnIndex);
}
@Override
public Time getTime(int columnIndex) throws SQLException {
return getTime(columnIndex, null);
return asTime(columnIndex);
}
@Override
public Timestamp getTimestamp(int columnIndex) throws SQLException {
return getTimestamp(columnIndex, null);
return asTimeStamp(columnIndex);
}
@Override
@ -244,7 +244,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
return getDate(column(columnLabel));
}
private Long dateTime(int columnIndex) throws SQLException {
private Long dateTimeAsMillis(int columnIndex) throws SQLException {
Object val = column(columnIndex);
EsType type = columnType(columnIndex);
try {
@ -270,13 +270,68 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
}
}
private Date asDate(int columnIndex) throws SQLException {
Object val = column(columnIndex);
if (val == null) {
return null;
}
try {
return JdbcDateUtils.asDate(val.toString());
} catch (Exception e) {
EsType type = columnType(columnIndex);
throw new SQLException(
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Date", val, type.getName()), e);
}
}
private Time asTime(int columnIndex) throws SQLException {
Object val = column(columnIndex);
if (val == null) {
return null;
}
EsType type = columnType(columnIndex);
if (type == EsType.DATE) {
return new Time(0L);
}
try {
return JdbcDateUtils.asTime(val.toString());
} catch (Exception e) {
throw new SQLException(
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Time", val, type.getName()), e);
}
}
private Timestamp asTimeStamp(int columnIndex) throws SQLException {
Object val = column(columnIndex);
if (val == null) {
return null;
}
try {
if (val instanceof Number) {
return JdbcDateUtils.asTimestamp(((Number) val).longValue());
}
return JdbcDateUtils.asTimestamp(val.toString());
} catch (Exception e) {
EsType type = columnType(columnIndex);
throw new SQLException(
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Timestamp", val, type.getName()), e);
}
}
private Calendar safeCalendar(Calendar calendar) {
return calendar == null ? defaultCalendar : calendar;
}
@Override
public Date getDate(int columnIndex, Calendar cal) throws SQLException {
return TypeConverter.convertDate(dateTime(columnIndex), safeCalendar(cal));
return TypeConverter.convertDate(dateTimeAsMillis(columnIndex), safeCalendar(cal));
}
@Override
@ -290,7 +345,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
if (type == EsType.DATE) {
return new Time(0L);
}
return TypeConverter.convertTime(dateTime(columnIndex), safeCalendar(cal));
return TypeConverter.convertTime(dateTimeAsMillis(columnIndex), safeCalendar(cal));
}
@Override
@ -300,7 +355,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
@Override
public Timestamp getTimestamp(int columnIndex, Calendar cal) throws SQLException {
return TypeConverter.convertTimestamp(dateTime(columnIndex), safeCalendar(cal));
return TypeConverter.convertTimestamp(dateTimeAsMillis(columnIndex), safeCalendar(cal));
}
@Override

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

@ -32,6 +32,9 @@ import java.sql.SQLFeatureNotSupportedException;
import java.sql.SQLType;
import java.sql.Timestamp;
import java.sql.Types;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Date;
@ -874,17 +877,13 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
Long randomLongDate = randomNonNegativeLong();
indexSimpleDocumentWithTrueValues(randomLongDate);
Calendar connCalendar = Calendar.getInstance(TimeZone.getTimeZone(timeZoneId), Locale.ROOT);
doWithQuery(SELECT_ALL_FIELDS, (results) -> {
results.next();
connCalendar.setTimeInMillis(randomLongDate);
connCalendar.set(HOUR_OF_DAY, 0);
connCalendar.set(MINUTE, 0);
connCalendar.set(SECOND, 0);
connCalendar.set(MILLISECOND, 0);
java.sql.Date expectedDate = new java.sql.Date(connCalendar.getTimeInMillis());
ZoneId zoneId = ZoneId.of(timeZoneId);
java.sql.Date expectedDate = new java.sql.Date(
ZonedDateTime.ofInstant(Instant.ofEpochMilli(randomLongDate), zoneId)
.toLocalDate().atStartOfDay(zoneId).toInstant().toEpochMilli());
assertEquals(expectedDate, results.getDate("test_date"));
assertEquals(expectedDate, results.getDate(9));
@ -892,7 +891,7 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
assertEquals(expectedDate, results.getObject(9, java.sql.Date.class));
// bulk validation for all fields which are not of type date
validateErrorsForDateTimeTestsWithoutCalendar(results::getDate);
validateErrorsForDateTestsWithoutCalendar(results::getDate);
});
}
@ -941,24 +940,17 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
Long randomLongDate = randomNonNegativeLong();
indexSimpleDocumentWithTrueValues(randomLongDate);
Calendar c = Calendar.getInstance(TimeZone.getTimeZone(timeZoneId), Locale.ROOT);
doWithQuery(SELECT_ALL_FIELDS, (results) -> {
results.next();
c.setTimeInMillis(randomLongDate);
c.set(ERA, GregorianCalendar.AD);
c.set(YEAR, 1970);
c.set(MONTH, 0);
c.set(DAY_OF_MONTH, 1);
assertEquals(results.getTime("test_date"), new java.sql.Time(c.getTimeInMillis()));
assertEquals(results.getTime(9), new java.sql.Time(c.getTimeInMillis()));
assertEquals(results.getObject("test_date", java.sql.Time.class),
new java.sql.Time(randomLongDate % 86400000L));
assertEquals(results.getObject(9, java.sql.Time.class),
new java.sql.Time(randomLongDate % 86400000L));
validateErrorsForDateTimeTestsWithoutCalendar(results::getTime);
java.sql.Time expectedTime = new java.sql.Time(randomLongDate % 86400000L);
assertEquals(expectedTime, results.getTime("test_date"));
assertEquals(expectedTime, results.getTime(9));
assertEquals(expectedTime, results.getObject("test_date", java.sql.Time.class));
assertEquals(expectedTime, results.getObject(9, java.sql.Time.class));
validateErrorsForTimeTestsWithoutCalendar(results::getTime);
});
}
@ -1689,15 +1681,25 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
assertThrowsUnsupportedAndExpectErrorMessage(r, "Writes not supported");
}
private void validateErrorsForDateTimeTestsWithoutCalendar(CheckedFunction<String,Object,SQLException> method) {
private void validateErrorsForDateTestsWithoutCalendar(CheckedFunction<String,Object,SQLException> method) {
SQLException sqle;
for (Entry<Tuple<String, Object>, SQLType> field : dateTimeTestingFields.entrySet()) {
sqle = expectThrows(SQLException.class, () -> method.apply(field.getKey().v1()));
assertEquals(
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Long",
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Date",
field.getKey().v2(), field.getValue()), sqle.getMessage());
}
}
private void validateErrorsForTimeTestsWithoutCalendar(CheckedFunction<String,Object,SQLException> method) {
SQLException sqle;
for (Entry<Tuple<String, Object>, SQLType> field : dateTimeTestingFields.entrySet()) {
sqle = expectThrows(SQLException.class, () -> method.apply(field.getKey().v1()));
assertEquals(
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Time",
field.getKey().v2(), field.getValue()), sqle.getMessage());
}
}
private void validateErrorsForDateTimeTestsWithCalendar(Calendar c, CheckedBiFunction<String,Calendar,Object,SQLException> method) {
SQLException sqle;