From 72806d3249edc43d850c4ef50495d86c867938bd Mon Sep 17 00:00:00 2001 From: Rupal <56703525+rupal-bq@users.noreply.github.com> Date: Tue, 8 Dec 2020 13:13:45 -0800 Subject: [PATCH] Fix for ExprValueFactory construct issue (#898) * add date & time * add datetime * try comparison test * add manual IT * add integration test for issue #867 Co-authored-by: Rupal Mahajan <> --- .../value/ElasticsearchExprValueFactory.java | 22 ++++++++-- .../ElasticsearchExprValueFactoryTest.java | 40 ++++++++++++++----- .../sql/sql/DateTimeFunctionIT.java | 32 +++++++++++++++ 3 files changed, 79 insertions(+), 15 deletions(-) diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactory.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactory.java index d22c55637a..e5bef1bdcf 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactory.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactory.java @@ -21,6 +21,8 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.ARRAY; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BYTE; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATE; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATETIME; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DOUBLE; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.FLOAT; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; @@ -28,6 +30,7 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.SHORT; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRUCT; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIME; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIMESTAMP; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_BINARY; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_GEO_POINT; @@ -41,12 +44,15 @@ import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprByteValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprCollectionValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprDateValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprDatetimeValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprDoubleValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprFloatValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprIntegerValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprLongValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprShortValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprStringValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTimeValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTimestampValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; @@ -177,14 +183,22 @@ public ExprValue construct(String field, Object value) { return constructString((String) value); } else if (type.equals(BOOLEAN)) { return constructBoolean((Boolean) value); - } else if (type.equals(TIMESTAMP)) { + } else if (type.equals(TIMESTAMP) || type.equals(DATE) || type.equals(TIME) + || type.equals(DATETIME)) { + ExprValue exprValue; if (value instanceof Number) { - return constructTimestamp(((Number) value).longValue()); + exprValue = constructTimestamp(((Number) value).longValue()); } else if (value instanceof Instant) { - return constructTimestamp((Instant) value); + exprValue = constructTimestamp((Instant) value); } else { - return constructTimestamp(String.valueOf(value)); + exprValue = constructTimestamp(String.valueOf(value)); + } + if (type.equals(DATE)) { + return new ExprDateValue(exprValue.dateValue().toString()); + } else if (type.equals(TIME)) { + return new ExprTimeValue(exprValue.timeValue().toString()); } + return exprValue; } else if (type.equals(ES_TEXT)) { return new ElasticsearchExprTextValue((String) value); } else if (type.equals(ES_TEXT_KEYWORD)) { diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java index 0ed476fcc2..b3ad51f91f 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/data/value/ElasticsearchExprValueFactoryTest.java @@ -29,6 +29,8 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.ARRAY; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BYTE; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATE; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATETIME; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DOUBLE; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.FLOAT; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; @@ -36,6 +38,7 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.SHORT; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRUCT; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIME; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIMESTAMP; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_BINARY; import static com.amazon.opendistroforelasticsearch.sql.elasticsearch.data.type.ElasticsearchDataType.ES_GEO_POINT; @@ -46,6 +49,8 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprCollectionValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprDateValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprDatetimeValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTimeValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTimestampValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; @@ -54,6 +59,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import java.time.Instant; +import java.time.LocalDateTime; import java.util.LinkedHashMap; import java.util.Map; import lombok.EqualsAndHashCode; @@ -71,7 +77,10 @@ class ElasticsearchExprValueFactoryTest { .put("floatV", FLOAT) .put("doubleV", DOUBLE) .put("stringV", STRING) - .put("dateV", TIMESTAMP) + .put("dateV", DATE) + .put("datetimeV", DATETIME) + .put("timeV", TIME) + .put("timestampV", TIMESTAMP) .put("boolV", BOOLEAN) .put("structV", STRUCT) .put("structV.id", INTEGER) @@ -162,39 +171,48 @@ public void constructText() { public void constructDate() { assertEquals( new ExprTimestampValue("2015-01-01 00:00:00"), - tupleValue("{\"dateV\":\"2015-01-01\"}").get("dateV")); + tupleValue("{\"timestampV\":\"2015-01-01\"}").get("timestampV")); assertEquals( new ExprTimestampValue("2015-01-01 12:10:30"), - tupleValue("{\"dateV\":\"2015-01-01T12:10:30Z\"}").get("dateV")); + tupleValue("{\"timestampV\":\"2015-01-01T12:10:30Z\"}").get("timestampV")); assertEquals( new ExprTimestampValue("2015-01-01 12:10:30"), - tupleValue("{\"dateV\":\"2015-01-01T12:10:30\"}").get("dateV")); + tupleValue("{\"timestampV\":\"2015-01-01T12:10:30\"}").get("timestampV")); assertEquals( new ExprTimestampValue("2015-01-01 12:10:30"), - tupleValue("{\"dateV\":\"2015-01-01 12:10:30\"}").get("dateV")); + tupleValue("{\"timestampV\":\"2015-01-01 12:10:30\"}").get("timestampV")); assertEquals( new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)), - tupleValue("{\"dateV\":1420070400001}").get("dateV")); + tupleValue("{\"timestampV\":1420070400001}").get("timestampV")); assertEquals( new ExprTimeValue("19:36:22"), - tupleValue("{\"dateV\":\"19:36:22\"}").get("dateV")); + tupleValue("{\"timestampV\":\"19:36:22\"}").get("timestampV")); assertEquals( new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)), - constructFromObject("dateV", 1420070400001L)); + constructFromObject("timestampV", 1420070400001L)); assertEquals( new ExprTimestampValue(Instant.ofEpochMilli(1420070400001L)), - constructFromObject("dateV", Instant.ofEpochMilli(1420070400001L))); + constructFromObject("timestampV", Instant.ofEpochMilli(1420070400001L))); assertEquals( new ExprTimestampValue("2015-01-01 12:10:30"), - constructFromObject("dateV", "2015-01-01 12:10:30")); + constructFromObject("timestampV", "2015-01-01 12:10:30")); + assertEquals( + new ExprDateValue("2015-01-01"), + constructFromObject("dateV","2015-01-01")); + assertEquals( + new ExprTimeValue("12:10:30"), + constructFromObject("timeV","12:10:30")); + assertEquals( + new ExprDatetimeValue("2015-01-01 12:10:30"), + constructFromObject("datetimeV", "2015-01-01 12:10:30")); } @Test public void constructDateFromUnsupportedFormatThrowException() { IllegalStateException exception = assertThrows( - IllegalStateException.class, () -> tupleValue("{\"dateV\":\"2015-01-01 12:10\"}")); + IllegalStateException.class, () -> tupleValue("{\"timestampV\":\"2015-01-01 12:10\"}")); assertEquals( "Construct ExprTimestampValue from \"2015-01-01 12:10\" failed, " + "unsupported date format.", diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java index 3e603963c6..677621d2bc 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/DateTimeFunctionIT.java @@ -20,8 +20,10 @@ import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.schema; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifyDataRows; import static com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils.verifySchema; +import static com.amazon.opendistroforelasticsearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; import static com.amazon.opendistroforelasticsearch.sql.util.TestUtils.getResponseBody; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.In; import com.amazon.opendistroforelasticsearch.sql.common.utils.StringUtils; import com.amazon.opendistroforelasticsearch.sql.legacy.SQLIntegTestCase; import com.amazon.opendistroforelasticsearch.sql.util.TestUtils; @@ -39,6 +41,23 @@ public class DateTimeFunctionIT extends SQLIntegTestCase { public void init() throws Exception { super.init(); TestUtils.enableNewQueryEngine(client()); + loadIndex(Index.BANK); + } + + @Test + public void testDateInGroupBy() throws IOException{ + JSONObject result = + executeQuery(String.format("SELECT DATE(birthdate) FROM %s GROUP BY DATE(birthdate)",TEST_INDEX_BANK) ); + verifySchema(result, + schema("DATE(birthdate)", null, "date")); + verifyDataRows(result, + rows("2017-10-23"), + rows("2017-11-20"), + rows("2018-06-23"), + rows("2018-11-13"), + rows("2018-06-27"), + rows("2018-08-19"), + rows("2018-08-11")); } @Test @@ -93,6 +112,19 @@ public void testDateAdd() throws IOException { verifySchema(result, schema("date_add('2020-09-16', interval 1 day)", null, "datetime")); verifyDataRows(result, rows("2020-09-17")); + + result = + executeQuery(String.format("SELECT DATE_ADD(birthdate, INTERVAL 1 YEAR) FROM %s GROUP BY 1",TEST_INDEX_BANK) ); + verifySchema(result, + schema("DATE_ADD(birthdate, INTERVAL 1 YEAR)", null, "datetime")); + verifyDataRows(result, + rows("2018-10-23 00:00:00"), + rows("2018-11-20 00:00:00"), + rows("2019-06-23 00:00:00"), + rows("2019-11-13 23:33:20"), + rows("2019-06-27 00:00:00"), + rows("2019-08-19 00:00:00"), + rows("2019-08-11 00:00:00")); } @Test