From 51d82cf3b2a834ab60fb2fc672a0ff2f25724fdc Mon Sep 17 00:00:00 2001 From: Yury-Fridlyand Date: Wed, 25 Jan 2023 19:06:18 -0800 Subject: [PATCH] Simplify `TIMESTAMP` function implementation by using automatic datetime types cast (PR #1196). Signed-off-by: Yury-Fridlyand --- .../expression/datetime/DateTimeFunction.java | 93 ++----------------- .../opensearch/sql/utils/DateTimeUtils.java | 11 --- .../datetime/DateTimeFunctionTest.java | 22 +++-- .../expression/datetime/TimestampTest.java | 3 + 4 files changed, 22 insertions(+), 107 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java index 5ecc15ddc0..bb07322ed2 100644 --- a/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java @@ -737,68 +737,15 @@ private DefaultFunctionResolver time_to_sec() { * Input strings may contain a timestamp only in format 'yyyy-MM-dd HH:mm:ss[.SSSSSSSSS]' * STRING/DATE/TIME/DATETIME/TIMESTAMP -> TIMESTAMP * STRING/DATE/TIME/DATETIME/TIMESTAMP, STRING/DATE/TIME/DATETIME/TIMESTAMP -> TIMESTAMP + * All types are converted to TIMESTAMP actually before the function call - it is responsibility + * of the automatic cast mechanism defined in `ExprCoreType` and performed by `TypeCastOperator`. */ private DefaultFunctionResolver timestamp() { return define(BuiltinFunctionName.TIMESTAMP.getName(), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestamp), - TIMESTAMP, STRING), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestamp), - TIMESTAMP, DATE), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestamp), - TIMESTAMP, TIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestamp), - TIMESTAMP, DATETIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestamp), - TIMESTAMP, TIMESTAMP), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, STRING, STRING), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, STRING, DATE), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, STRING, TIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, STRING, DATETIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, STRING, TIMESTAMP), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATE, STRING), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATE, DATE), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATE, TIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATE, DATETIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATE, TIMESTAMP), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIME, STRING), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIME, DATE), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIME, TIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIME, DATETIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIME, TIMESTAMP), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATETIME, STRING), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATETIME, DATE), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATETIME, TIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATETIME, DATETIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, DATETIME, TIMESTAMP), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIMESTAMP, STRING), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIMESTAMP, DATE), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIMESTAMP, TIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), - TIMESTAMP, TIMESTAMP, DATETIME), - implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprTimestampEx), + impl(nullMissingHandling(v -> v), TIMESTAMP, TIMESTAMP), + // We can use FunctionProperties.None, because it is not used. It is required to convert + // TIME to other datetime types, but arguments there are already converted. + impl(nullMissingHandling((v1, v2) -> exprAddTime(FunctionProperties.None, v1, v2)), TIMESTAMP, TIMESTAMP, TIMESTAMP)); } @@ -1444,34 +1391,6 @@ private ExprValue exprTimeDiff(ExprValue first, ExprValue second) { Duration.between(second.timeValue(), first.timeValue()))); } - /** - * Timestamp implementation for ExprValue. - * - * @param exprValue ExprValue of Timestamp. - * @return ExprValue. - */ - private ExprValue exprTimestamp(FunctionProperties functionProperties, ExprValue exprValue) { - if (exprValue instanceof ExprStringValue) { - return new ExprTimestampValue(exprValue.stringValue()); - } - return new ExprTimestampValue(DateTimeUtils.extractTimestamp(exprValue, functionProperties)); - } - - /** - * Timestamp implementation for two arguments. It extracts time expression from exprValue2 - * and adds it to the expression exprValue1 and returns the result as a timestamp value. - * - * @param exprValue1 ExprValue of Timestamp type or String type. - * @param exprValue2 ExprValue of Timestamp type or String type. - * @return ExprValue. - */ - private ExprValue exprTimestampEx(FunctionProperties functionProperties, - ExprValue exprValue1, ExprValue exprValue2) { - return exprAddTime(functionProperties, - exprTimestamp(functionProperties, exprValue1), - exprTimestamp(functionProperties, exprValue2)); - } - /** * Time To Sec implementation for ExprValue. * diff --git a/core/src/main/java/org/opensearch/sql/utils/DateTimeUtils.java b/core/src/main/java/org/opensearch/sql/utils/DateTimeUtils.java index 1b89930ced..06d40ea6aa 100644 --- a/core/src/main/java/org/opensearch/sql/utils/DateTimeUtils.java +++ b/core/src/main/java/org/opensearch/sql/utils/DateTimeUtils.java @@ -151,15 +151,4 @@ public static LocalDate extractDate(ExprValue value, ? ((ExprTimeValue) value).dateValue(functionProperties) : value.dateValue(); } - - /** - * Extracts Instant from a datetime ExprValue. - * Uses `FunctionProperties` for `ExprTimeValue`. - */ - public static Instant extractTimestamp(ExprValue value, - FunctionProperties functionProperties) { - return value instanceof ExprTimeValue - ? ((ExprTimeValue) value).timestampValue(functionProperties) - : value.timestampValue(); - } } diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java index ff9f154f75..0fcedea419 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/DateTimeFunctionTest.java @@ -176,7 +176,8 @@ public void adddate() { expr = DSL.adddate(DSL.timestamp(DSL.literal("2020-08-26 12:05:00")), DSL.literal(7)); assertEquals(DATETIME, expr.type()); assertEquals(new ExprDatetimeValue("2020-09-02 12:05:00"), expr.valueOf(env)); - assertEquals("adddate(timestamp(\"2020-08-26 12:05:00\"), 7)", expr.toString()); + assertEquals("adddate(timestamp(cast_to_timestamp(\"2020-08-26 12:05:00\")), 7)", + expr.toString()); expr = DSL.adddate( DSL.date(DSL.literal("2020-08-26")), DSL.interval(DSL.literal(1), DSL.literal("hour"))); @@ -278,7 +279,8 @@ public void date_add() { expr = DSL.date_add(DSL.timestamp(DSL.literal("2020-08-26 12:05:00")), DSL.literal(7)); assertEquals(DATETIME, expr.type()); assertEquals(new ExprDatetimeValue("2020-09-02 12:05:00"), expr.valueOf(env)); - assertEquals("date_add(timestamp(\"2020-08-26 12:05:00\"), 7)", expr.toString()); + assertEquals("date_add(timestamp(cast_to_timestamp(\"2020-08-26 12:05:00\")), 7)", + expr.toString()); expr = DSL.date_add( DSL.date(DSL.literal("2020-08-26")), DSL.interval(DSL.literal(1), DSL.literal("hour"))); @@ -334,7 +336,8 @@ public void date_sub() { expr = DSL.date_sub(DSL.timestamp(DSL.literal("2020-08-26 12:05:00")), DSL.literal(7)); assertEquals(DATETIME, expr.type()); assertEquals(new ExprDatetimeValue("2020-08-19 12:05:00"), expr.valueOf(env)); - assertEquals("date_sub(timestamp(\"2020-08-26 12:05:00\"), 7)", expr.toString()); + assertEquals("date_sub(timestamp(cast_to_timestamp(\"2020-08-26 12:05:00\")), 7)", + expr.toString()); expr = DSL.date_sub(DSL.literal("2020-08-26 12:05:00"), DSL.literal(7)); assertEquals(DATETIME, expr.type()); @@ -345,8 +348,8 @@ public void date_sub() { DSL.interval(DSL.literal(1), DSL.literal("hour"))); assertEquals(DATETIME, expr.type()); assertEquals(new ExprDatetimeValue("2020-08-26 11:05:00"), expr.valueOf(env)); - assertEquals("date_sub(timestamp(\"2020-08-26 12:05:00\"), interval(1, \"hour\"))", - expr.toString()); + assertEquals("date_sub(timestamp(cast_to_timestamp(\"2020-08-26 12:05:00\")), " + + "interval(1, \"hour\"))", expr.toString()); expr = DSL.date_sub(DSL.literal("2020-08-26 12:05:00"), DSL.interval(DSL.literal(1), DSL.literal("year"))); @@ -1479,7 +1482,8 @@ public void subdate() { expr = DSL.subdate(DSL.timestamp(DSL.literal("2020-08-26 12:05:00")), DSL.literal(7)); assertEquals(DATETIME, expr.type()); assertEquals(new ExprDatetimeValue("2020-08-19 12:05:00"), expr.valueOf(env)); - assertEquals("subdate(timestamp(\"2020-08-26 12:05:00\"), 7)", expr.toString()); + assertEquals("subdate(timestamp(cast_to_timestamp(\"2020-08-26 12:05:00\")), 7)", + expr.toString()); expr = DSL.subdate(DSL.literal("2020-08-26 12:05:00"), DSL.literal(7)); assertEquals(DATETIME, expr.type()); @@ -1490,8 +1494,8 @@ public void subdate() { DSL.interval(DSL.literal(1), DSL.literal("hour"))); assertEquals(DATETIME, expr.type()); assertEquals(new ExprDatetimeValue("2020-08-26 11:05:00"), expr.valueOf(env)); - assertEquals("subdate(timestamp(\"2020-08-26 12:05:00\"), interval(1, \"hour\"))", - expr.toString()); + assertEquals("subdate(timestamp(cast_to_timestamp(\"2020-08-26 12:05:00\")), " + + "interval(1, \"hour\"))", expr.toString()); expr = DSL.subdate(DSL.literal("2020-08-26 12:05:00"), DSL.interval(DSL.literal(1), DSL.literal("hour"))); @@ -1604,7 +1608,7 @@ public void timestamp() { FunctionExpression expr = DSL.timestamp(DSL.literal("2020-08-17 01:01:01")); assertEquals(TIMESTAMP, expr.type()); assertEquals(new ExprTimestampValue("2020-08-17 01:01:01"), expr.valueOf(env)); - assertEquals("timestamp(\"2020-08-17 01:01:01\")", expr.toString()); + assertEquals("timestamp(cast_to_timestamp(\"2020-08-17 01:01:01\"))", expr.toString()); expr = DSL.timestamp(DSL.literal(new ExprTimestampValue("2020-08-17 01:01:01"))); assertEquals(TIMESTAMP, expr.type()); diff --git a/core/src/test/java/org/opensearch/sql/expression/datetime/TimestampTest.java b/core/src/test/java/org/opensearch/sql/expression/datetime/TimestampTest.java index 98c4332b17..6e939cd6be 100644 --- a/core/src/test/java/org/opensearch/sql/expression/datetime/TimestampTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/datetime/TimestampTest.java @@ -14,6 +14,8 @@ import java.time.LocalDateTime; import java.time.LocalTime; import java.util.stream.Stream; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; @@ -25,6 +27,7 @@ import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.ExpressionTestBase; +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) public class TimestampTest extends ExpressionTestBase { @Test