Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SQL: Replace joda with java time #38437

Merged
merged 9 commits into from
Feb 8, 2019
Merged

SQL: Replace joda with java time #38437

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
af51ec6
SQL: Replace joda with java time
matriv Feb 5, 2019
bc77167
Address comments
matriv Feb 6, 2019
88ef084
Use the date_optional_time formatter instead of a custom one
matriv Feb 6, 2019
459c8db
fix checkstyle
matriv Feb 6, 2019
904c0c8
Merge remote-tracking branch 'upstream/master' into mt/fix-37703
matriv Feb 6, 2019
8e06556
Merge remote-tracking branch 'upstream/master' into mt/fix-37703
matriv Feb 6, 2019
cf9dc6b
Merge remote-tracking branch 'upstream/master' into mt/fix-37703
matriv Feb 6, 2019
e37ddc5
Merge remote-tracking branch 'upstream/master' into mt/fix-37703
matriv Feb 6, 2019
153fa09
Merge remote-tracking branch 'upstream/master' into mt/fix-37703
matriv Feb 8, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions ...ck/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/TypeConverterTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,18 @@
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));
Expand All @@ -40,21 +41,17 @@ public void testDoubleAsNative() throws Exception {
}

public void testTimestampAsNative() throws Exception {
DateTime now = DateTime.now();
ZonedDateTime now = ZonedDateTime.now(Clock.tick(Clock.system(ZoneId.of("Z")), Duration.ofMillis(1)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why using Clock here?
maybe we could just use
ZonedDateTime.now(ZoneOffset.UTC)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I recall correctly, it's because the clock approach had unpredictable behavior between jdk 8 and 11.
@astefan do you remember?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - you guys made an excellent fix here #38437 (comment)
I had similar problems this morning with watcher.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@costin yes. We had this issue in the past for the NOW() function - #36877. @matriv's approach is spot on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same approach is used here maybe you can re-use that method.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, ZoneId.of("Z") seems to have a constant defined in DateUtils. Can you use that one or is not visible in this project?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jdbc module doesn't have access to DateUtils or TestUtils so both are not possible.

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 {
// Simulate sending over XContent
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");
Expand Down
5 changes: 0 additions & 5 deletions ...c/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
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;
Expand Down Expand Up @@ -132,10 +131,6 @@ private Object unwrapMultiValue(Object values) {
if (values instanceof String) {
return DateUtils.asDateTime(Long.parseLong(values.toString()));
}
// returned by nested types...
Copy link
Contributor Author

@matriv matriv Feb 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@costin I'm not sure about this, but doesn't seem to break something. Should we check more in depth and add tests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be removed now with Joda out. Likely this will eliminate asDateTime as well.

if (values instanceof DateTime) {
return DateUtils.asDateTime((DateTime) values);
}
}
if (values instanceof Long || values instanceof Double || values instanceof String || values instanceof Boolean) {
return values;
Expand Down
38 changes: 20 additions & 18 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/ExpressionBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,25 +111,29 @@
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.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAmount;
import java.util.EnumSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.StringJoiner;

import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE;
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.UTC;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably matter of aesthetics preference and aesthetics but wouldn't direct calls for DateUtils.UTC or DateUtils.asDateOnly be more obvious here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 - in other places we call DateUtils explicit.

import static org.elasticsearch.xpack.sql.util.DateUtils.asDateOnly;

abstract class ExpressionBuilder extends IdentifierBuilder {

Expand Down Expand Up @@ -789,13 +793,11 @@ public Literal visitDateEscapedLiteral(DateEscapedLiteralContext ctx) {
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
Expand All @@ -804,10 +806,10 @@ public Literal visitTimeEscapedLiteral(TimeEscapedLiteralContext ctx) {
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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see ISO_LOCAL_TIME deals, also, with format as HH:mm (not only HH:mm:ss) and the previous joda implementation seems to have looked at HH:mm:ss only... not sure if this is an issue or not, meaning if there are 0 seconds, does it return 12:13 only or 12:13:00?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will dive into that when we implement the TIME data type, since now this method always throws an exception throw new SqlIllegalArgumentException("Time (only) literals are not supported; a date component is required as well");

} catch (DateTimeParseException ex) {
throw new ParsingException(source, "Invalid time received; {}", ex.getMessage());
}

Expand All @@ -820,18 +822,18 @@ public Literal visitTimestampEscapedLiteral(TimestampEscapedLiteralContext ctx)

Source source = source(ctx);
// parse yyyy-mm-dd hh:mm:ss(.f...)
DateTime dt = null;
ZonedDateTime zdt = null;
try {
DateTimeFormatter formatter = new DateTimeFormatterBuilder()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a Formatter in DateFormatters that we can use here?
if not, maybe we could make this static final field

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on making this a static field in DateUtils. I'm not aware of any pattern that matches this (it's basically ISO with ' ' instead of 'T'.

.append(ISODateTimeFormat.date())
.append(ISO_LOCAL_DATE)
.appendLiteral(" ")
.append(ISODateTimeFormat.hourMinuteSecondFraction())
.append(ISO_LOCAL_TIME)
.toFormatter();
dt = formatter.parseDateTime(string);
} catch (IllegalArgumentException ex) {
zdt = ZonedDateTime.parse(string, formatter.withZone(UTC));
return new Literal(source, zdt, DataType.DATETIME);
} catch (DateTimeParseException ex) {
throw new ParsingException(source, "Invalid timestamp received; {}", ex.getMessage());
}
return new Literal(source, DateUtils.asDateTime(dt), DataType.DATETIME);
}

@Override
Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
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;
Expand Down Expand Up @@ -546,8 +547,8 @@ private static Function<Object, Object> fromString(Function<String, Object> conv
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());
}
};
}
Expand Down
78 changes: 52 additions & 26 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/util/DateUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,69 @@

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.LocalDate;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.format.SignStyle;
import java.time.temporal.ChronoField;
import java.util.Locale;

import static java.time.format.DateTimeFormatter.ISO_LOCAL_DATE;
import static java.time.temporal.ChronoField.DAY_OF_MONTH;
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
import static java.time.temporal.ChronoField.MINUTE_OF_HOUR;
import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
import static java.time.temporal.ChronoField.NANO_OF_SECOND;
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;

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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it be the same if we use DateFormatters.forPattern("date_optional_time") ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might work (see the TODO above).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I use the date_optional_time and try to parse a date only String like 2019-02-06 I get:

java.time.DateTimeException: Unable to obtain Instant from TemporalAccessor: {},ISO resolved to 1970-01-01 of type java.time.format.Parsed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I need the same formatter but with:

.parseDefaulting(HOUR_OF_DAY, 0)
.parseDefaulting(MINUTE_OF_HOUR, 0)
.parseDefaulting(SECOND_OF_MINUTE, 0)


public static final ZoneId UTC = ZoneId.of("Z");
public static final String DATE_PARSE_FORMAT = "epoch_millis";

private static final DateTimeFormatter UTC_DATE_FORMATTER = new DateTimeFormatterBuilder()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure we need this (looks like date_optional_time to me) but if so, maybe we could have it in DateFormatters?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it looks to be date_optional_time

Copy link
Contributor Author

@matriv matriv Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my response here: #38437 (comment)

Is there an elegant way to overcome this when using the date_optional_time ?

Copy link
Contributor Author

@matriv matriv Feb 6, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch that, I didn't check again after using DateFormatters.from.
This method has the logic to handle the date only part.

.appendValue(ChronoField.YEAR, 4, 10, SignStyle.EXCEEDS_PAD)
.appendLiteral("-")
.appendValue(MONTH_OF_YEAR, 2, 2, SignStyle.NOT_NEGATIVE)
.appendLiteral('-')
.appendValue(DAY_OF_MONTH, 2, 2, SignStyle.NOT_NEGATIVE)
.optionalStart()
.appendLiteral('T')
.optionalStart()
.appendValue(HOUR_OF_DAY, 2, 2, SignStyle.NOT_NEGATIVE)
.optionalStart()
.appendLiteral(':')
.appendValue(MINUTE_OF_HOUR, 2, 2, SignStyle.NOT_NEGATIVE)
.optionalStart()
.appendLiteral(':')
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
.optionalStart()
.appendFraction(NANO_OF_SECOND, 3, 9, true)
.optionalEnd()
.optionalEnd()
.optionalStart()
.appendZoneOrOffsetId()
.optionalEnd()
.optionalStart()
.appendOffset("+HHmm", "Z")
.optionalEnd()
.optionalEnd()
.optionalEnd()
.optionalEnd()
.parseDefaulting(HOUR_OF_DAY, 0)
.parseDefaulting(MINUTE_OF_HOUR, 0)
.parseDefaulting(SECOND_OF_MINUTE, 0)
.toFormatter(Locale.ROOT)
.withZone(UTC);

private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000;

private DateUtils() {}

/**
Expand Down Expand Up @@ -56,22 +98,7 @@ public static ZonedDateTime asDateTime(long millis, ZoneId id) {
* 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) {
Expand All @@ -82,7 +109,7 @@ public static ZonedDateTime asDateOnly(ZonedDateTime zdt) {
* 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 asDateTime(Instant.from(UTC_DATE_FORMATTER.parse(dateFormat)).toEpochMilli());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about something like?

return DateFormatters.from(DATE_TIME_FORMATTER.parse(dateFormat)))

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I need return DateFormatters.from(UTC_DATE_FORMATTER.parse(dateFormat)).withZoneSameInstant(UTC);

}

public static ZonedDateTime asDateTime(DateTime dateTime) {
Expand All @@ -94,13 +121,12 @@ public static ZonedDateTime asDateTime(DateTime dateTime) {
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 String toString(ZonedDateTime dateTime) {
return StringUtils.toString(dateTime);
}
Expand Down
9 changes: 1 addition & 8 deletions ...va/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/DateTimeTestUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
8 changes: 5 additions & 3 deletions ...ck/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/EscapedFunctionsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,8 @@ public void testDateLiteral() {

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());
}

Expand All @@ -186,7 +187,8 @@ public void testTimeLiteralUnsupported() {

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());
}

Expand All @@ -198,7 +200,7 @@ public void testTimestampLiteral() {
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());
}

Expand Down
13 changes: 7 additions & 6 deletions ...ck/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,18 +151,18 @@ public void testConversionToDate() {
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();
Conversion forward = conversionFor(DATE, KEYWORD);
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());
}
}

Expand Down Expand Up @@ -199,6 +199,7 @@ public void testConversionToDateTime() {
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"));
Expand All @@ -210,7 +211,7 @@ public void testConversionToDateTime() {
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]: Text '0xff' could not be parsed at index 0", e.getMessage());
}
}

Expand Down