From af8a444caaef64516d1a1f08481db853759fdb70 Mon Sep 17 00:00:00 2001 From: Marios Trivyzas Date: Fri, 8 Feb 2019 22:55:51 +0200 Subject: [PATCH] SQL: Replace joda with java time (#38437) Replace remaining usages of joda classes with java time. Fixes: #37703 --- .../xpack/sql/jdbc/TypeConverterTests.java | 17 +++--- .../search/extractor/FieldHitExtractor.java | 5 -- .../xpack/sql/parser/ExpressionBuilder.java | 33 ++++------ .../xpack/sql/type/DataTypeConversion.java | 5 +- .../xpack/sql/util/DateUtils.java | 61 ++++++------------- .../scalar/datetime/DateTimeTestUtils.java | 9 +-- .../sql/parser/EscapedFunctionsTests.java | 8 ++- .../sql/type/DataTypeConversionTests.java | 14 +++-- 8 files changed, 56 insertions(+), 96 deletions(-) diff --git a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/TypeConverterTests.java b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/TypeConverterTests.java index 2e33f4e1307..588e3a6392e 100644 --- a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/TypeConverterTests.java +++ b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/TypeConverterTests.java @@ -10,17 +10,18 @@ import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.json.JsonXContent; import org.elasticsearch.test.ESTestCase; -import org.joda.time.DateTime; -import org.joda.time.ReadableDateTime; import java.sql.Timestamp; +import java.time.Clock; +import java.time.Duration; +import java.time.ZoneId; +import java.time.ZonedDateTime; import static org.hamcrest.Matchers.instanceOf; public class TypeConverterTests extends ESTestCase { - public void testFloatAsNative() throws Exception { assertThat(convertAsNative(42.0f, EsType.FLOAT), instanceOf(Float.class)); assertThat(convertAsNative(42.0, EsType.FLOAT), instanceOf(Float.class)); @@ -40,9 +41,9 @@ public class TypeConverterTests extends ESTestCase { } public void testTimestampAsNative() throws Exception { - DateTime now = DateTime.now(); + ZonedDateTime now = ZonedDateTime.now(Clock.tick(Clock.system(ZoneId.of("Z")), Duration.ofMillis(1))); assertThat(convertAsNative(now, EsType.DATETIME), instanceOf(Timestamp.class)); - assertEquals(now.getMillis(), ((Timestamp) convertAsNative(now, EsType.DATETIME)).getTime()); + assertEquals(now.toInstant().toEpochMilli(), ((Timestamp) convertAsNative(now, EsType.DATETIME)).getTime()); } private Object convertAsNative(Object value, EsType type) throws Exception { @@ -50,11 +51,7 @@ public class TypeConverterTests extends ESTestCase { XContentBuilder builder = JsonXContent.contentBuilder(); builder.startObject(); builder.field("value"); - if (value instanceof ReadableDateTime) { - builder.value(((ReadableDateTime) value).getMillis()); - } else { - builder.value(value); - } + builder.value(value); builder.endObject(); builder.close(); Object copy = XContentHelper.convertToMap(BytesReference.bytes(builder), false, builder.contentType()).v2().get("value"); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java index 589481247ac..23747787b82 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java @@ -14,7 +14,6 @@ import org.elasticsearch.search.SearchHit; import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.util.DateUtils; -import org.joda.time.DateTime; import java.io.IOException; import java.util.ArrayDeque; @@ -132,10 +131,6 @@ public class FieldHitExtractor implements HitExtractor { if (values instanceof String) { return DateUtils.asDateTime(Long.parseLong(values.toString())); } - // returned by nested types... - if (values instanceof DateTime) { - return DateUtils.asDateTime((DateTime) values); - } } if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean) { return values; diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java index fe8f5ac9925..05069ef42a1 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java @@ -111,15 +111,12 @@ import org.elasticsearch.xpack.sql.tree.Source; import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.DataTypeConversion; import org.elasticsearch.xpack.sql.type.DataTypes; -import org.elasticsearch.xpack.sql.util.DateUtils; import org.elasticsearch.xpack.sql.util.StringUtils; -import org.joda.time.DateTime; -import org.joda.time.format.DateTimeFormatter; -import org.joda.time.format.DateTimeFormatterBuilder; -import org.joda.time.format.ISODateTimeFormat; import java.time.Duration; +import java.time.LocalTime; import java.time.Period; +import java.time.format.DateTimeParseException; import java.time.temporal.TemporalAmount; import java.util.EnumSet; import java.util.List; @@ -127,9 +124,12 @@ import java.util.Locale; import java.util.Map; import java.util.StringJoiner; +import static java.time.format.DateTimeFormatter.ISO_LOCAL_TIME; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.elasticsearch.xpack.sql.type.DataTypeConversion.conversionFor; +import static org.elasticsearch.xpack.sql.util.DateUtils.asDateOnly; +import static org.elasticsearch.xpack.sql.util.DateUtils.ofEscapedLiteral; abstract class ExpressionBuilder extends IdentifierBuilder { @@ -791,13 +791,11 @@ abstract class ExpressionBuilder extends IdentifierBuilder { String string = string(ctx.string()); Source source = source(ctx); // parse yyyy-MM-dd - DateTime dt = null; try { - dt = ISODateTimeFormat.date().parseDateTime(string); - } catch(IllegalArgumentException ex) { + return new Literal(source, asDateOnly(string), DataType.DATE); + } catch(DateTimeParseException ex) { throw new ParsingException(source, "Invalid date received; {}", ex.getMessage()); } - return new Literal(source, DateUtils.asDateOnly(dt), DataType.DATE); } @Override @@ -806,10 +804,10 @@ abstract class ExpressionBuilder extends IdentifierBuilder { Source source = source(ctx); // parse HH:mm:ss - DateTime dt = null; + LocalTime lt = null; try { - dt = ISODateTimeFormat.hourMinuteSecond().parseDateTime(string); - } catch (IllegalArgumentException ex) { + lt = LocalTime.parse(string, ISO_LOCAL_TIME); + } catch (DateTimeParseException ex) { throw new ParsingException(source, "Invalid time received; {}", ex.getMessage()); } @@ -822,18 +820,11 @@ abstract class ExpressionBuilder extends IdentifierBuilder { Source source = source(ctx); // parse yyyy-mm-dd hh:mm:ss(.f...) - DateTime dt = null; try { - DateTimeFormatter formatter = new DateTimeFormatterBuilder() - .append(ISODateTimeFormat.date()) - .appendLiteral(" ") - .append(ISODateTimeFormat.hourMinuteSecondFraction()) - .toFormatter(); - dt = formatter.parseDateTime(string); - } catch (IllegalArgumentException ex) { + return new Literal(source, ofEscapedLiteral(string), DataType.DATETIME); + } catch (DateTimeParseException ex) { throw new ParsingException(source, "Invalid timestamp received; {}", ex.getMessage()); } - return new Literal(source, DateUtils.asDateTime(dt), DataType.DATETIME); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java index bc89b0f1e15..9dbb2a3abb6 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java @@ -11,6 +11,7 @@ import org.elasticsearch.xpack.sql.SqlIllegalArgumentException; import org.elasticsearch.xpack.sql.util.DateUtils; import java.time.ZonedDateTime; +import java.time.format.DateTimeParseException; import java.util.Locale; import java.util.function.DoubleFunction; import java.util.function.Function; @@ -546,8 +547,8 @@ public abstract class DataTypeConversion { return converter.apply(value.toString()); } catch (NumberFormatException e) { throw new SqlIllegalArgumentException(e, "cannot cast [{}] to [{}]", value, to); - } catch (IllegalArgumentException e) { - throw new SqlIllegalArgumentException(e, "cannot cast [{}] to [{}]:{}", value, to, e.getMessage()); + } catch (DateTimeParseException | IllegalArgumentException e) { + throw new SqlIllegalArgumentException(e, "cannot cast [{}] to [{}]: {}", value, to, e.getMessage()); } }; } diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java index deb7b9e9703..38db3cbe131 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java @@ -6,29 +6,35 @@ package org.elasticsearch.xpack.sql.util; +import org.elasticsearch.common.time.DateFormatter; +import org.elasticsearch.common.time.DateFormatters; import org.elasticsearch.xpack.sql.proto.StringUtils; -import org.joda.time.DateTime; -import org.joda.time.format.DateTimeFormatter; -import org.joda.time.format.ISODateTimeFormat; import java.time.Instant; -import java.time.LocalDateTime; +import java.time.LocalDate; import java.time.ZoneId; -import java.time.ZoneOffset; import java.time.ZonedDateTime; +import java.time.format.DateTimeFormatter; +import java.time.format.DateTimeFormatterBuilder; import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE; +import static java.time.format.DateTimeFormatter.ISO_LOCAL_TIME; public final class DateUtils { - private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000; - - // TODO: do we have a java.time based parser we can use instead? - private static final DateTimeFormatter UTC_DATE_FORMATTER = ISODateTimeFormat.dateOptionalTimeParser().withZoneUTC(); - public static final ZoneId UTC = ZoneId.of("Z"); public static final String DATE_PARSE_FORMAT = "epoch_millis"; + private static final DateTimeFormatter DATE_TIME_ESCAPED_LITERAL_FORMATTER = new DateTimeFormatterBuilder() + .append(ISO_LOCAL_DATE) + .appendLiteral(" ") + .append(ISO_LOCAL_TIME) + .toFormatter().withZone(UTC); + + private static final DateFormatter UTC_DATE_TIME_FORMATTER = DateFormatter.forPattern("date_optional_time").withZone(UTC); + + private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000L; + private DateUtils() {} /** @@ -56,22 +62,7 @@ public final class DateUtils { * Parses the given string into a Date (SQL DATE type) using UTC as a default timezone. */ public static ZonedDateTime asDateOnly(String dateFormat) { - return asDateOnly(UTC_DATE_FORMATTER.parseDateTime(dateFormat)); - } - - public static ZonedDateTime asDateOnly(DateTime dateTime) { - LocalDateTime ldt = LocalDateTime.of( - dateTime.getYear(), - dateTime.getMonthOfYear(), - dateTime.getDayOfMonth(), - 0, - 0, - 0, - 0); - - return ZonedDateTime.ofStrict(ldt, - ZoneOffset.ofTotalSeconds(dateTime.getZone().getOffset(dateTime) / 1000), - org.elasticsearch.common.time.DateUtils.dateTimeZoneToZoneId(dateTime.getZone())); + return LocalDate.parse(dateFormat, ISO_LOCAL_DATE).atStartOfDay(UTC); } public static ZonedDateTime asDateOnly(ZonedDateTime zdt) { @@ -82,25 +73,13 @@ public final class DateUtils { * Parses the given string into a DateTime using UTC as a default timezone. */ public static ZonedDateTime asDateTime(String dateFormat) { - return asDateTime(UTC_DATE_FORMATTER.parseDateTime(dateFormat)); + return DateFormatters.from(UTC_DATE_TIME_FORMATTER.parse(dateFormat)).withZoneSameInstant(UTC); } - public static ZonedDateTime asDateTime(DateTime dateTime) { - LocalDateTime ldt = LocalDateTime.of( - dateTime.getYear(), - dateTime.getMonthOfYear(), - dateTime.getDayOfMonth(), - dateTime.getHourOfDay(), - dateTime.getMinuteOfHour(), - dateTime.getSecondOfMinute(), - dateTime.getMillisOfSecond() * 1_000_000); - - return ZonedDateTime.ofStrict(ldt, - ZoneOffset.ofTotalSeconds(dateTime.getZone().getOffset(dateTime) / 1000), - org.elasticsearch.common.time.DateUtils.dateTimeZoneToZoneId(dateTime.getZone())); + public static ZonedDateTime ofEscapedLiteral(String dateFormat) { + return ZonedDateTime.parse(dateFormat, DATE_TIME_ESCAPED_LITERAL_FORMATTER.withZone(UTC)); } - public static String toString(ZonedDateTime dateTime) { return StringUtils.toString(dateTime); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeTestUtils.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeTestUtils.java index 2ae6e571ac9..4323cce234c 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeTestUtils.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeTestUtils.java @@ -7,22 +7,15 @@ package org.elasticsearch.xpack.sql.expression.function.scalar.datetime; import org.elasticsearch.xpack.sql.util.DateUtils; -import org.joda.time.DateTime; -import org.joda.time.DateTimeZone; import java.time.ZonedDateTime; -import static org.junit.Assert.assertEquals; - public class DateTimeTestUtils { private DateTimeTestUtils() {} public static ZonedDateTime dateTime(int year, int month, int day, int hour, int minute) { - DateTime dateTime = new DateTime(year, month, day, hour, minute, DateTimeZone.UTC); - ZonedDateTime zdt = ZonedDateTime.of(year, month, day, hour, minute, 0, 0, DateUtils.UTC); - assertEquals(dateTime.getMillis() / 1000, zdt.toEpochSecond()); - return zdt; + return ZonedDateTime.of(year, month, day, hour, minute, 0, 0, DateUtils.UTC); } public static ZonedDateTime dateTime(long millisSinceEpoch) { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java index 8cbb0b528e9..bc3ea049c24 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java @@ -175,7 +175,8 @@ public class EscapedFunctionsTests extends ESTestCase { public void testDateLiteralValidation() { ParsingException ex = expectThrows(ParsingException.class, () -> dateLiteral("2012-13-01")); - assertEquals("line 1:2: Invalid date received; Cannot parse \"2012-13-01\": Value 13 for monthOfYear must be in the range [1,12]", + assertEquals("line 1:2: Invalid date received; Text '2012-13-01' could not be parsed: " + + "Invalid value for MonthOfYear (valid values 1 - 12): 13", ex.getMessage()); } @@ -186,7 +187,8 @@ public class EscapedFunctionsTests extends ESTestCase { public void testTimeLiteralValidation() { ParsingException ex = expectThrows(ParsingException.class, () -> timeLiteral("10:10:65")); - assertEquals("line 1:2: Invalid time received; Cannot parse \"10:10:65\": Value 65 for secondOfMinute must be in the range [0,59]", + assertEquals("line 1:2: Invalid time received; Text '10:10:65' could not be parsed: " + + "Invalid value for SecondOfMinute (valid values 0 - 59): 65", ex.getMessage()); } @@ -198,7 +200,7 @@ public class EscapedFunctionsTests extends ESTestCase { public void testTimestampLiteralValidation() { ParsingException ex = expectThrows(ParsingException.class, () -> timestampLiteral("2012-01-01T10:01:02.3456")); assertEquals( - "line 1:2: Invalid timestamp received; Invalid format: \"2012-01-01T10:01:02.3456\" is malformed at \"T10:01:02.3456\"", + "line 1:2: Invalid timestamp received; Text '2012-01-01T10:01:02.3456' could not be parsed at index 10", ex.getMessage()); } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java index 73b4ea8fa8d..a2f60d4b43a 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java @@ -151,10 +151,10 @@ public class DataTypeConversionTests extends ESTestCase { Conversion conversion = conversionFor(KEYWORD, to); assertNull(conversion.convert(null)); - assertEquals(date(0L), conversion.convert("1970-01-01T00:10:01Z")); - assertEquals(date(1483228800000L), conversion.convert("2017-01-01T00:11:00Z")); - assertEquals(date(-1672531200000L), conversion.convert("1917-01-01T00:11:00Z")); - assertEquals(date(18000000L), conversion.convert("1970-01-01T03:10:20-05:00")); + assertEquals(date(0L), conversion.convert("1970-01-01")); + assertEquals(date(1483228800000L), conversion.convert("2017-01-01")); + assertEquals(date(-1672531200000L), conversion.convert("1917-01-01")); + assertEquals(date(18000000L), conversion.convert("1970-01-01")); // double check back and forth conversion ZonedDateTime zdt = TestUtils.now(); @@ -162,7 +162,7 @@ public class DataTypeConversionTests extends ESTestCase { Conversion back = conversionFor(KEYWORD, DATE); assertEquals(DateUtils.asDateOnly(zdt), back.convert(forward.convert(zdt))); Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert("0xff")); - assertEquals("cannot cast [0xff] to [date]:Invalid format: \"0xff\" is malformed at \"xff\"", e.getMessage()); + assertEquals("cannot cast [0xff] to [date]: Text '0xff' could not be parsed at index 0", e.getMessage()); } } @@ -199,6 +199,7 @@ public class DataTypeConversionTests extends ESTestCase { Conversion conversion = conversionFor(KEYWORD, to); assertNull(conversion.convert(null)); + assertEquals(dateTime(0L), conversion.convert("1970-01-01")); assertEquals(dateTime(1000L), conversion.convert("1970-01-01T00:00:01Z")); assertEquals(dateTime(1483228800000L), conversion.convert("2017-01-01T00:00:00Z")); assertEquals(dateTime(1483228800000L), conversion.convert("2017-01-01T00:00:00Z")); @@ -210,7 +211,8 @@ public class DataTypeConversionTests extends ESTestCase { Conversion back = conversionFor(KEYWORD, DATETIME); assertEquals(dt, back.convert(forward.convert(dt))); Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert("0xff")); - assertEquals("cannot cast [0xff] to [datetime]:Invalid format: \"0xff\" is malformed at \"xff\"", e.getMessage()); + assertEquals("cannot cast [0xff] to [datetime]: failed to parse date field [0xff] with format [date_optional_time]", + e.getMessage()); } }