Skip to content

Commit

Permalink
SQL: Fix casting from date to numeric type to use millis (#37869)
Browse files Browse the repository at this point in the history 
Previously casting from a DATE[TIME] type to a numeric (DOUBLE, LONG,
INT, etc. used seconds instead of the epoch millis.

Fixes: #37655
  • Loading branch information
@matriv
matriv committed Jan 25, 2019
1 parent 7b33cc1 commit 5813f6c
Show file tree
Hide file tree
Showing 4 changed files with 152 additions and 68 deletions.
42 changes: 21 additions & 21 deletions x-pack/plugin/sql/qa/src/main/resources/datetime.csv-spec
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,32 +131,32 @@ SELECT CONVERT(birth_date, DOUBLE) AS date FROM test_emp GROUP BY date ORDER BY
date:d
---------------
null
-5.631552E8
-5.586624E8
-5.56416E8
-5.539104E8
-5.517504E8
-5.492448E8
-5.406912E8
-5.371488E8
-5.359392E8
-5.631552E11
-5.586624E11
-5.56416E11
-5.539104E11
-5.517504E11
-5.492448E11
-5.406912E11
-5.371488E11
-5.359392E11
;

castedDateTimeWithGroupBy2
SELECT CAST(hire_date AS INTEGER) AS date FROM test_emp GROUP BY date ORDER BY date LIMIT 10;
SELECT CAST(hire_date AS LONG) AS date FROM test_emp GROUP BY date ORDER BY date LIMIT 10;

date:i
date:l
---------------
477532800
478051200
484790400
489715200
495763200
498096000
498614400
501206400
501292800
501379200
477532800000
478051200000
484790400000
489715200000
495763200000
498096000000
498614400000
501206400000
501292800000
501379200000
;

dateTimeAggByIsoDayOfWeekWithFilter
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/type/DataTypeConversion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -470,9 +470,9 @@ private static Function<Object, Object> fromString(Function<String, Object> conv
private static Function<Object, Object> fromBool(Function<Boolean, Object> converter) {
return (Object l) -> converter.apply(((Boolean) l));
}

private static Function<Object, Object> fromDate(Function<Long, Object> converter) {
return l -> ((ZonedDateTime) l).toEpochSecond();
return l -> converter.apply(((ZonedDateTime) l).toInstant().toEpochMilli());
}

private static Function<Object, Object> toDate(Conversion conversion) {
Expand Down
17 changes: 17 additions & 0 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/TestUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,28 @@
import org.elasticsearch.xpack.sql.session.Configuration;
import org.elasticsearch.xpack.sql.util.DateUtils;

import java.time.Clock;
import java.time.Duration;
import java.time.ZonedDateTime;

public class TestUtils {

private TestUtils() {}

public static final Configuration TEST_CFG = new Configuration(DateUtils.UTC, Protocol.FETCH_SIZE,
Protocol.REQUEST_TIMEOUT, Protocol.PAGE_TIMEOUT, null, Mode.PLAIN, null, null);

/**
* Returns the current UTC date-time with milliseconds precision.
* In Java 9+ (as opposed to Java 8) the {@code Clock} implementation uses system's best clock implementation (which could mean
* that the precision of the clock can be milliseconds, microseconds or nanoseconds), whereas in Java 8
* {@code System.currentTimeMillis()} is always used. To account for these differences, this method defines a new {@code Clock}
* which will offer a value for {@code ZonedDateTime.now()} set to always have milliseconds precision.
*
* @return {@link ZonedDateTime} instance for the current date-time with milliseconds precision in UTC
*/
public static final ZonedDateTime now() {
return ZonedDateTime.now(Clock.tick(Clock.system(DateUtils.UTC), Duration.ofMillis(1)));
}

}
157 changes: 112 additions & 45 deletions ...ck/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/type/DataTypeConversionTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@

import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.SqlIllegalArgumentException;
import org.elasticsearch.xpack.sql.TestUtils;
import org.elasticsearch.xpack.sql.expression.Literal;
import org.elasticsearch.xpack.sql.type.DataTypeConversion.Conversion;
import org.elasticsearch.xpack.sql.util.DateUtils;

import java.time.ZonedDateTime;

Expand All @@ -33,16 +33,21 @@
import static org.elasticsearch.xpack.sql.type.DataTypeConversion.commonType;
import static org.elasticsearch.xpack.sql.type.DataTypeConversion.conversionFor;


public class DataTypeConversionTests extends ESTestCase {
public void testConversionToString() {
Conversion conversion = conversionFor(DOUBLE, KEYWORD);
assertNull(conversion.convert(null));
assertEquals("10.0", conversion.convert(10.0));

conversion = conversionFor(DATE, KEYWORD);
assertNull(conversion.convert(null));
assertEquals("1970-01-01T00:00:00.000Z", conversion.convert(dateTime(0)));
public void testConversionToString() {
DataType to = KEYWORD;
{
Conversion conversion = conversionFor(DOUBLE, to);
assertNull(conversion.convert(null));
assertEquals("10.0", conversion.convert(10.0));
}
{
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals("1973-11-29T21:33:09.101Z", conversion.convert(dateTime(123456789101L)));
assertEquals("1966-02-02T02:26:50.899Z", conversion.convert(dateTime(-123456789101L)));
}
}

/**
Expand Down Expand Up @@ -71,12 +76,20 @@ public void testConversionToLong() {
assertEquals(1L, conversion.convert(true));
assertEquals(0L, conversion.convert(false));
}
Conversion conversion = conversionFor(KEYWORD, to);
assertNull(conversion.convert(null));
assertEquals(1L, conversion.convert("1"));
assertEquals(0L, conversion.convert("-0"));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert("0xff"));
assertEquals("cannot cast [0xff] to [Long]", e.getMessage());
{
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals(123456789101L, conversion.convert(dateTime(123456789101L)));
assertEquals(-123456789101L, conversion.convert(dateTime(-123456789101L)));
}
{
Conversion conversion = conversionFor(KEYWORD, to);
assertNull(conversion.convert(null));
assertEquals(1L, conversion.convert("1"));
assertEquals(0L, conversion.convert("-0"));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert("0xff"));
assertEquals("cannot cast [0xff] to [Long]", e.getMessage());
}
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/35683")
Expand All @@ -89,7 +102,7 @@ public void testConversionToDate() {
assertEquals(dateTime(10L), conversion.convert(10.1));
assertEquals(dateTime(11L), conversion.convert(10.6));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert(Double.MAX_VALUE));
assertEquals("[" + Double.MAX_VALUE + "] out of [Long] range", e.getMessage());
assertEquals("[" + Double.MAX_VALUE + "] out of [long] range", e.getMessage());
}
{
Conversion conversion = conversionFor(INTEGER, to);
Expand All @@ -103,44 +116,60 @@ public void testConversionToDate() {
assertEquals(dateTime(1), conversion.convert(true));
assertEquals(dateTime(0), conversion.convert(false));
}
Conversion conversion = conversionFor(KEYWORD, to);
assertNull(conversion.convert(null));
{
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals(dateTime(123379200000L), conversion.convert(dateTime(123456789101L)));
assertEquals(dateTime(-123465600000L), conversion.convert(dateTime(-123456789101L)));
}
{
Conversion conversion = conversionFor(KEYWORD, to);
assertNull(conversion.convert(null));

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"));
assertEquals(dateTime(18000000L), conversion.convert("1970-01-01T00:00:00-05:00"));

assertEquals(dateTime(1000L), conversion.convert("1970-01-01T00:00:01Z"));
assertEquals(dateTime(1483228800000L), conversion.convert("2017-01-01T00:00:00Z"));
assertEquals(dateTime(18000000L), conversion.convert("1970-01-01T00:00:00-05:00"));

// double check back and forth conversion
ZonedDateTime dt = ZonedDateTime.now(DateUtils.UTC);
Conversion forward = conversionFor(DATE, KEYWORD);
Conversion back = conversionFor(KEYWORD, DATE);
assertEquals(dt, back.convert(forward.convert(dt)));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert("0xff"));
assertEquals("cannot cast [0xff] to [Date]:Invalid format: \"0xff\" is malformed at \"xff\"", e.getMessage());
// double check back and forth conversion
ZonedDateTime dt = TestUtils.now();
Conversion forward = conversionFor(DATE, KEYWORD);
Conversion back = conversionFor(KEYWORD, DATE);
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());
}
}

public void testConversionToDouble() {
DataType to = DOUBLE;
{
Conversion conversion = conversionFor(FLOAT, DOUBLE);
Conversion conversion = conversionFor(FLOAT, to);
assertNull(conversion.convert(null));
assertEquals(10.0, (double) conversion.convert(10.0f), 0.00001);
assertEquals(10.1, (double) conversion.convert(10.1f), 0.00001);
assertEquals(10.6, (double) conversion.convert(10.6f), 0.00001);
}
{
Conversion conversion = conversionFor(INTEGER, DOUBLE);
Conversion conversion = conversionFor(INTEGER, to);
assertNull(conversion.convert(null));
assertEquals(10.0, (double) conversion.convert(10), 0.00001);
assertEquals(-134.0, (double) conversion.convert(-134), 0.00001);
}
{
Conversion conversion = conversionFor(BOOLEAN, DOUBLE);
Conversion conversion = conversionFor(BOOLEAN, to);
assertNull(conversion.convert(null));
assertEquals(1.0, (double) conversion.convert(true), 0);
assertEquals(0.0, (double) conversion.convert(false), 0);
}
{
Conversion conversion = conversionFor(KEYWORD, DOUBLE);
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals(1.23456789101E11, (double) conversion.convert(dateTime(123456789101L)), 0);
assertEquals(-1.23456789101E11, (double) conversion.convert(dateTime(-123456789101L)), 0);
}
{
Conversion conversion = conversionFor(KEYWORD, to);
assertNull(conversion.convert(null));
assertEquals(1.0, (double) conversion.convert("1"), 0);
assertEquals(0.0, (double) conversion.convert("-0"), 0);
Expand All @@ -151,36 +180,44 @@ public void testConversionToDouble() {
}

public void testConversionToBoolean() {
DataType to = BOOLEAN;
{
Conversion conversion = conversionFor(FLOAT, BOOLEAN);
Conversion conversion = conversionFor(FLOAT, to);
assertNull(conversion.convert(null));
assertEquals(true, conversion.convert(10.0f));
assertEquals(true, conversion.convert(-10.0f));
assertEquals(false, conversion.convert(0.0f));
}
{
Conversion conversion = conversionFor(INTEGER, BOOLEAN);
Conversion conversion = conversionFor(INTEGER, to);
assertNull(conversion.convert(null));
assertEquals(true, conversion.convert(10));
assertEquals(true, conversion.convert(-10));
assertEquals(false, conversion.convert(0));
}
{
Conversion conversion = conversionFor(LONG, BOOLEAN);
Conversion conversion = conversionFor(LONG, to);
assertNull(conversion.convert(null));
assertEquals(true, conversion.convert(10L));
assertEquals(true, conversion.convert(-10L));
assertEquals(false, conversion.convert(0L));
}
{
Conversion conversion = conversionFor(DOUBLE, BOOLEAN);
Conversion conversion = conversionFor(DOUBLE, to);
assertNull(conversion.convert(null));
assertEquals(true, conversion.convert(10.0d));
assertEquals(true, conversion.convert(-10.0d));
assertEquals(false, conversion.convert(0.0d));
}
{
Conversion conversion = conversionFor(KEYWORD, BOOLEAN);
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals(true, conversion.convert(dateTime(123456789101L)));
assertEquals(true, conversion.convert(dateTime(-123456789101L)));
assertEquals(false, conversion.convert(dateTime(0L)));
}
{
Conversion conversion = conversionFor(KEYWORD, to);
assertNull(conversion.convert(null));
// We only handled upper and lower case true and false
assertEquals(true, conversion.convert("true"));
Expand All @@ -204,39 +241,69 @@ public void testConversionToBoolean() {
}

public void testConversionToInt() {
DataType to = INTEGER;
{
Conversion conversion = conversionFor(DOUBLE, INTEGER);
Conversion conversion = conversionFor(DOUBLE, to);
assertNull(conversion.convert(null));
assertEquals(10, conversion.convert(10.0));
assertEquals(10, conversion.convert(10.1));
assertEquals(11, conversion.convert(10.6));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert(Long.MAX_VALUE));
assertEquals("[" + Long.MAX_VALUE + "] out of [Int] range", e.getMessage());
}
{
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals(12345678, conversion.convert(dateTime(12345678L)));
assertEquals(223456789, conversion.convert(dateTime(223456789L)));
assertEquals(-123456789, conversion.convert(dateTime(-123456789L)));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert(dateTime(Long.MAX_VALUE)));
assertEquals("[" + Long.MAX_VALUE + "] out of [Int] range", e.getMessage());
}
}

public void testConversionToShort() {
DataType to = SHORT;
{
Conversion conversion = conversionFor(DOUBLE, SHORT);
Conversion conversion = conversionFor(DOUBLE, to);
assertNull(conversion.convert(null));
assertEquals((short) 10, conversion.convert(10.0));
assertEquals((short) 10, conversion.convert(10.1));
assertEquals((short) 11, conversion.convert(10.6));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert(Integer.MAX_VALUE));
assertEquals("[" + Integer.MAX_VALUE + "] out of [Short] range", e.getMessage());
}
{
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals((short) 12345, conversion.convert(dateTime(12345L)));
assertEquals((short) -12345, conversion.convert(dateTime(-12345L)));
Exception e = expectThrows(SqlIllegalArgumentException.class,
() -> conversion.convert(dateTime(Integer.MAX_VALUE)));
assertEquals("[" + Integer.MAX_VALUE + "] out of [Short] range", e.getMessage());
}
}

public void testConversionToByte() {
DataType to = BYTE;
{
Conversion conversion = conversionFor(DOUBLE, BYTE);
Conversion conversion = conversionFor(DOUBLE, to);
assertNull(conversion.convert(null));
assertEquals((byte) 10, conversion.convert(10.0));
assertEquals((byte) 10, conversion.convert(10.1));
assertEquals((byte) 11, conversion.convert(10.6));
Exception e = expectThrows(SqlIllegalArgumentException.class, () -> conversion.convert(Short.MAX_VALUE));
assertEquals("[" + Short.MAX_VALUE + "] out of [Byte] range", e.getMessage());
}
{
Conversion conversion = conversionFor(DATE, to);
assertNull(conversion.convert(null));
assertEquals((byte) 123, conversion.convert(dateTime(123L)));
assertEquals((byte) -123, conversion.convert(dateTime(-123L)));
Exception e = expectThrows(SqlIllegalArgumentException.class,
() -> conversion.convert(dateTime(Integer.MAX_VALUE)));
assertEquals("[" + Integer.MAX_VALUE + "] out of [Byte] range", e.getMessage());
}
}

public void testConversionToNull() {
Expand Down Expand Up @@ -264,7 +331,7 @@ public void testCommonType() {
assertEquals(NULL, commonType(NULL, NULL));
assertEquals(INTEGER, commonType(INTEGER, KEYWORD));
assertEquals(LONG, commonType(TEXT, LONG));
assertEquals(null, commonType(TEXT, KEYWORD));
assertNull(commonType(TEXT, KEYWORD));
assertEquals(SHORT, commonType(SHORT, BYTE));
assertEquals(FLOAT, commonType(BYTE, FLOAT));
assertEquals(FLOAT, commonType(FLOAT, INTEGER));
Expand All @@ -278,9 +345,9 @@ public void testEsDataTypes() {
}

public void testConversionToUnsupported() {
Exception e = expectThrows(SqlIllegalArgumentException.class,
() -> conversionFor(INTEGER, UNSUPPORTED));
assertEquals("cannot convert from [INTEGER] to [UNSUPPORTED]", e.getMessage());
Exception e = expectThrows(SqlIllegalArgumentException.class,
() -> conversionFor(INTEGER, UNSUPPORTED));
assertEquals("cannot convert from [INTEGER] to [UNSUPPORTED]", e.getMessage());
}

public void testStringToIp() {
Expand Down

0 comments on commit 5813f6c

Please sign in to comment.