SQL: protocol returns ISO 8601 String formatted dates instead of Long for JDBC/ODBC requests (#36800)

* Change the way the protocol returns date fields from Long values in case
of JDBC/ODBC, to ISO 8601 with millis String.
This commit is contained in:
Andrei Stefan 2018-12-19 16:36:16 +02:00 committed by GitHub
parent a34a3532ce
commit d31eaf7313
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 118 additions and 39 deletions

View File

@ -0,0 +1,81 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
package org.elasticsearch.xpack.sql.jdbc;
import java.sql.Date;
import java.sql.Time;
import java.sql.Timestamp;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.util.Locale;
import java.util.function.Function;
import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE;
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
import static java.time.temporal.ChronoField.MILLI_OF_SECOND;
import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
/**
* JDBC specific datetime specific utility methods. Because of lack of visibility, this class borrows code
* from {@code org.elasticsearch.xpack.sql.util.DateUtils} and {@code org.elasticsearch.xpack.sql.proto.StringUtils}.
*/
final class JdbcDateUtils {
private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000;
static final DateTimeFormatter ISO_WITH_MILLIS = new DateTimeFormatterBuilder()
.parseCaseInsensitive()
.append(ISO_LOCAL_DATE)
.appendLiteral('T')
.appendValue(HOUR_OF_DAY, 2)
.appendLiteral(':')
.appendValue(MINUTE_OF_HOUR, 2)
.appendLiteral(':')
.appendValue(SECOND_OF_MINUTE, 2)
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
.appendOffsetId()
.toFormatter(Locale.ROOT);
static long asMillisSinceEpoch(String date) {
ZonedDateTime zdt = ISO_WITH_MILLIS.parse(date, ZonedDateTime::from);
return zdt.toInstant().toEpochMilli();
}
static Date asDate(String date) {
return new Date(utcMillisRemoveTime(asMillisSinceEpoch(date)));
}
static Time asTime(String date) {
return new Time(utcMillisRemoveDate(asMillisSinceEpoch(date)));
}
static Timestamp asTimestamp(String date) {
return new Timestamp(asMillisSinceEpoch(date));
}
/*
* Handles the value received as parameter, as either String (a ZonedDateTime formatted in ISO 8601 standard with millis) -
* date fields being returned formatted like this. Or a Long value, in case of Histograms.
*/
static <R> R asDateTimeField(Object value, Function<String, R> asDateTimeMethod, Function<Long, R> ctor) {
if (value instanceof String) {
return asDateTimeMethod.apply((String) value);
} else {
return ctor.apply(((Number) value).longValue());
}
}
private static long utcMillisRemoveTime(long l) {
return l - (l % DAY_IN_MILLIS);
}
private static long utcMillisRemoveDate(long l) {
return l % DAY_IN_MILLIS;
}
}

View File

@ -30,6 +30,7 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.Function;
import static java.lang.String.format;
@ -248,7 +249,10 @@ class JdbcResultSet implements ResultSet, JdbcWrapper {
// the cursor can return an Integer if the date-since-epoch is small enough, XContentParser (Jackson) will
// return the "smallest" data type for numbers when parsing
// TODO: this should probably be handled server side
return val == null ? null : ((Number) val).longValue();
if (val == null) {
return null;
}
return JdbcDateUtils.asDateTimeField(val, JdbcDateUtils::asMillisSinceEpoch, Function.identity());
};
return val == null ? null : (Long) val;
} catch (ClassCastException cce) {

View File

@ -48,8 +48,6 @@ final class TypeConverter {
private TypeConverter() {}
private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000;
/**
* Converts millisecond after epoc to date
*/
@ -216,7 +214,7 @@ final class TypeConverter {
case FLOAT:
return floatValue(v); // Float might be represented as string for infinity and NaN values
case DATE:
return new Timestamp(((Number) v).longValue());
return JdbcDateUtils.asDateTimeField(v, JdbcDateUtils::asTimestamp, Timestamp::new);
case INTERVAL_YEAR:
case INTERVAL_MONTH:
case INTERVAL_YEAR_TO_MONTH:
@ -470,21 +468,21 @@ final class TypeConverter {
private static Date asDate(Object val, EsType columnType, String typeString) throws SQLException {
if (columnType == EsType.DATE) {
return new Date(utcMillisRemoveTime(((Number) val).longValue()));
return JdbcDateUtils.asDateTimeField(val, JdbcDateUtils::asDate, Date::new);
}
return failConversion(val, columnType, typeString, Date.class);
}
private static Time asTime(Object val, EsType columnType, String typeString) throws SQLException {
if (columnType == EsType.DATE) {
return new Time(utcMillisRemoveDate(((Number) val).longValue()));
return JdbcDateUtils.asDateTimeField(val, JdbcDateUtils::asTime, Time::new);
}
return failConversion(val, columnType, typeString, Time.class);
}
private static Timestamp asTimestamp(Object val, EsType columnType, String typeString) throws SQLException {
if (columnType == EsType.DATE) {
return new Timestamp(((Number) val).longValue());
return JdbcDateUtils.asDateTimeField(val, JdbcDateUtils::asTimestamp, Timestamp::new);
}
return failConversion(val, columnType, typeString, Timestamp.class);
}
@ -513,14 +511,6 @@ final class TypeConverter {
throw new SQLFeatureNotSupportedException();
}
private static long utcMillisRemoveTime(long l) {
return l - (l % DAY_IN_MILLIS);
}
private static long utcMillisRemoveDate(long l) {
return l % DAY_IN_MILLIS;
}
private static byte safeToByte(long x) throws SQLException {
if (x > Byte.MAX_VALUE || x < Byte.MIN_VALUE) {
throw new SQLException(format(Locale.ROOT, "Numeric %s out of range", Long.toString(x)));

View File

@ -31,6 +31,7 @@ import java.util.Set;
import static org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase.assertNoSearchContexts;
public abstract class JdbcIntegrationTestCase extends ESRestTestCase {
@After
public void checkSearchContent() throws Exception {
// Some context might linger due to fire and forget nature of scroll cleanup

View File

@ -12,6 +12,9 @@ import org.elasticsearch.xpack.sql.proto.ColumnInfo;
import java.sql.ResultSet;
import java.sql.ResultSetMetaData;
import java.sql.SQLException;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.List;
@ -20,6 +23,8 @@ public abstract class JdbcTestUtils {
public static final String SQL_TRACE = "org.elasticsearch.xpack.sql:TRACE";
public static final String JDBC_TIMEZONE = "timezone";
public static ZoneId UTC = ZoneId.of("Z");
public static void logResultSetMetadata(ResultSet rs, Logger logger) throws SQLException {
ResultSetMetaData metaData = rs.getMetaData();
@ -128,4 +133,8 @@ public abstract class JdbcTestUtils {
CliFormatter formatter = new CliFormatter(cols, data);
logger.info("\n" + formatter.formatWithHeader(cols, data));
}
public static ZonedDateTime of(long millis) {
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), UTC);
}
}

View File

@ -58,6 +58,7 @@ import static java.util.Calendar.MONTH;
import static java.util.Calendar.SECOND;
import static java.util.Calendar.YEAR;
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.JDBC_TIMEZONE;
import static org.elasticsearch.xpack.sql.qa.jdbc.JdbcTestUtils.of;
public class ResultSetTestCase extends JdbcIntegrationTestCase {
@ -200,10 +201,10 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getByte("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Byte]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Byte]", of(randomDate)),
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getObject("test_date", Byte.class));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Byte]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Byte]", of(randomDate)),
sqle.getMessage());
});
}
@ -323,10 +324,10 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getShort("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Short]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Short]", of(randomDate)),
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getObject("test_date", Short.class));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Short]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Short]", of(randomDate)),
sqle.getMessage());
});
}
@ -438,10 +439,10 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getInt("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Integer]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Integer]", of(randomDate)),
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getObject("test_date", Integer.class));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Integer]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Integer]", of(randomDate)),
sqle.getMessage());
});
}
@ -540,10 +541,10 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getLong("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Long]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Long]", of(randomDate)),
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getObject("test_date", Long.class));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Long]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Long]", of(randomDate)),
sqle.getMessage());
});
}
@ -623,10 +624,10 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getDouble("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Double]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Double]", of(randomDate)),
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getObject("test_date", Double.class));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Double]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Double]", of(randomDate)),
sqle.getMessage());
});
}
@ -706,10 +707,10 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getFloat("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Float]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Float]", of(randomDate)),
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getObject("test_date", Float.class));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Float]", randomDate),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Float]", of(randomDate)),
sqle.getMessage());
});
}
@ -767,7 +768,7 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
assertEquals("Expected: <true> but was: <false> for field " + fld, true, results.getObject(fld, Boolean.class));
}
SQLException sqle = expectThrows(SQLException.class, () -> results.getBoolean("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Boolean]", randomDate1),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Boolean]", of(randomDate1)),
sqle.getMessage());
results.next();
@ -777,11 +778,11 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase {
assertEquals("Expected: <false> but was: <true> for field " + fld, false, results.getObject(fld, Boolean.class));
}
sqle = expectThrows(SQLException.class, () -> results.getBoolean("test_date"));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Boolean]", randomDate2),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Boolean]", of(randomDate2)),
sqle.getMessage());
sqle = expectThrows(SQLException.class, () -> results.getObject("test_date", Boolean.class));
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Boolean]", randomDate2),
assertEquals(format(Locale.ROOT, "Unable to convert value [%.128s] of type [DATE] to [Boolean]", of(randomDate2)),
sqle.getMessage());
results.next();

View File

@ -171,15 +171,8 @@ public class SqlQueryResponse extends ActionResponse implements ToXContentObject
public static XContentBuilder value(XContentBuilder builder, Mode mode, Object value) throws IOException {
if (value instanceof ZonedDateTime) {
ZonedDateTime zdt = (ZonedDateTime) value;
if (Mode.isDriver(mode)) {
// JDBC cannot parse dates in string format and ODBC can have issues with it
// so instead, use the millis since epoch (in UTC)
builder.value(zdt.toInstant().toEpochMilli());
}
// otherwise use the ISO format
else {
builder.value(StringUtils.toString(zdt));
}
// use the ISO format
builder.value(StringUtils.toString(zdt));
} else {
builder.value(value);
}