Skip to content

Commit

Permalink
SQL: protocol returns ISO 8601 String formatted dates instead of Long…
Browse files Browse the repository at this point in the history
… 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.

(cherry picked from commit d31eaf7)
  • Loading branch information
astefan committed Dec 19, 2018
1 parent a99e9ca commit f233452
Show file tree
Hide file tree
Showing 7 changed files with 118 additions and 39 deletions.
Original file line number Diff line number Diff line change
@@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.function.Function;

import static java.lang.String.format;

Expand Down Expand Up @@ -248,7 +249,10 @@ private Long dateTime(int columnIndex) throws SQLException {
// 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -216,7 +214,7 @@ static Object convert(Object v, EsType columnType, String typeString) throws SQL
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:
Expand Down Expand Up @@ -470,21 +468,21 @@ private static Double asDouble(Object val, EsType columnType, String typeString)

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);
}
Expand Down Expand Up @@ -513,14 +511,6 @@ private static OffsetDateTime asOffsetDateTime(Object val, EsType columnType, St
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)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
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;

Expand All @@ -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();
Expand Down Expand Up @@ -128,4 +133,8 @@ public static void logLikeCLI(ResultSet rs, Logger logger) throws SQLException {
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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
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 {

Expand Down Expand Up @@ -200,10 +201,10 @@ public void testGettingInvalidByte() throws Exception {
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());
});
}
Expand Down Expand Up @@ -323,10 +324,10 @@ public void testGettingInvalidShort() throws Exception {
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());
});
}
Expand Down Expand Up @@ -438,10 +439,10 @@ public void testGettingInvalidInteger() throws Exception {
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());
});
}
Expand Down Expand Up @@ -540,10 +541,10 @@ public void testGettingInvalidLong() throws Exception {
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());
});
}
Expand Down Expand Up @@ -623,10 +624,10 @@ public void testGettingInvalidDouble() throws Exception {
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());
});
}
Expand Down Expand Up @@ -706,10 +707,10 @@ public void testGettingInvalidFloat() throws Exception {
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());
});
}
Expand Down Expand Up @@ -767,7 +768,7 @@ public void testGettingBooleanValues() throws Exception {
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();
Expand All @@ -777,11 +778,11 @@ public void testGettingBooleanValues() throws Exception {
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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -171,15 +171,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
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);
}
Expand Down

0 comments on commit f233452

Please sign in to comment.