Rework to use FunctionProperties.

Signed-off-by: Yury-Fridlyand <[email protected]>
Bit-Quill · Dec 10, 2022 · ee5eb3e · ee5eb3e
1 parent 00c3a84
commit ee5eb3e
Show file tree

Hide file tree

Showing 9 changed files with 259 additions and 64 deletions.
diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java
@@ -20,6 +20,7 @@
 import org.opensearch.sql.data.type.ExprCoreType;
 import org.opensearch.sql.data.type.ExprType;
 import org.opensearch.sql.exception.SemanticCheckException;
+import org.opensearch.sql.expression.function.FunctionProperties;
 
 /**
  * Expression Time Value.
@@ -56,19 +57,17 @@ public LocalTime timeValue() {
     return time;
   }
 
-  @Override
-  public LocalDate dateValue() {
-    return LocalDate.now();
+  public LocalDate dateValue(FunctionProperties functionProperties) {
+    return LocalDate.now(functionProperties.getQueryStartClock());
   }
 
-  @Override
-  public LocalDateTime datetimeValue() {
-    return LocalDateTime.of(dateValue(), timeValue());
+  public LocalDateTime datetimeValue(FunctionProperties functionProperties) {
+    return LocalDateTime.of(dateValue(functionProperties), timeValue());
   }
 
-  @Override
-  public Instant timestampValue() {
-    return ZonedDateTime.of(dateValue(), timeValue(), ExprTimestampValue.ZONE).toInstant();
+  public Instant timestampValue(FunctionProperties functionProperties) {
+    return ZonedDateTime.of(dateValue(functionProperties), timeValue(), ExprTimestampValue.ZONE)
+        .toInstant();
   }
 
   @Override

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
@@ -20,11 +20,13 @@
 import static org.opensearch.sql.expression.function.FunctionDSL.impl;
 import static org.opensearch.sql.expression.function.FunctionDSL.implWithProperties;
 import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling;
+import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandlingWithProperties;
 import static org.opensearch.sql.utils.DateTimeFormatters.DATE_FORMATTER_LONG_YEAR;
 import static org.opensearch.sql.utils.DateTimeFormatters.DATE_FORMATTER_SHORT_YEAR;
 import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_LONG_YEAR;
 import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_SHORT_YEAR;
 import static org.opensearch.sql.utils.DateTimeFormatters.DATE_TIME_FORMATTER_STRICT_WITH_TZ;
+import static org.opensearch.sql.utils.DateTimeUtils.extractDateTime;
 
 import java.math.BigDecimal;
 import java.math.RoundingMode;
@@ -64,6 +66,7 @@
 import org.opensearch.sql.expression.function.DefaultFunctionResolver;
 import org.opensearch.sql.expression.function.FunctionDSL;
 import org.opensearch.sql.expression.function.FunctionName;
+import org.opensearch.sql.expression.function.FunctionProperties;
 import org.opensearch.sql.expression.function.FunctionResolver;
 import org.opensearch.sql.utils.DateTimeUtils;
 
@@ -247,22 +250,38 @@ private DefaultFunctionResolver adddate() {
    */
   private DefaultFunctionResolver addtime() {
     return define(BuiltinFunctionName.ADDTIME.getName(),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), TIME, TIME, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), TIME, TIME, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), TIME, TIME, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), TIME, TIME, TIMESTAMP),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATETIME, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATETIME, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATETIME, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATETIME, TIMESTAMP),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATE, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATE, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATE, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, DATE, TIMESTAMP),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, TIMESTAMP, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, TIMESTAMP, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, TIMESTAMP, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprAddTime), DATETIME, TIMESTAMP, TIMESTAMP)
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            TIME, TIME, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            TIME, TIME, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            TIME, TIME, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            TIME, TIME, TIMESTAMP),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATETIME, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATETIME, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATETIME, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATETIME, TIMESTAMP),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATE, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATE, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATE, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, DATE, TIMESTAMP),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, TIMESTAMP, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, TIMESTAMP, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, TIMESTAMP, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprAddTime),
+            DATETIME, TIMESTAMP, TIMESTAMP)
     );
   }
 
@@ -549,22 +568,38 @@ private DefaultFunctionResolver subdate() {
    */
   private DefaultFunctionResolver subtime() {
     return define(BuiltinFunctionName.SUBTIME.getName(),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), TIME, TIME, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), TIME, TIME, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), TIME, TIME, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), TIME, TIME, TIMESTAMP),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATETIME, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATETIME, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATETIME, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATETIME, TIMESTAMP),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATE, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATE, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATE, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, DATE, TIMESTAMP),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, TIMESTAMP, TIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, TIMESTAMP, DATE),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, TIMESTAMP, DATETIME),
-        impl(nullMissingHandling(DateTimeFunction::exprSubTime), DATETIME, TIMESTAMP, TIMESTAMP)
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            TIME, TIME, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            TIME, TIME, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            TIME, TIME, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            TIME, TIME, TIMESTAMP),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATETIME, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATETIME, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATETIME, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATETIME, TIMESTAMP),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATE, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATE, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATE, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, DATE, TIMESTAMP),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, TIMESTAMP, TIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, TIMESTAMP, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, TIMESTAMP, DATETIME),
+        implWithProperties(nullMissingHandlingWithProperties(DateTimeFunction::exprSubTime),
+            DATETIME, TIMESTAMP, TIMESTAMP)
     );
   }
 
@@ -707,16 +742,18 @@ private ExprValue exprAddDateDays(ExprValue date, ExprValue days) {
   /**
    * Adds or subtracts time to/from date and returns the result.
    *
+   * @param functionProperties A FunctionProperties object.
    * @param temporal A Date/Time/Datetime/Timestamp value to change.
    * @param temporalDelta A Date/Time/Datetime/Timestamp object to add/subtract time from.
    * @param isAdd A flag: true to add, false to subtract.
    * @return A value calculated.
    */
-  private ExprValue exprApplyTime(ExprValue temporal, ExprValue temporalDelta, Boolean isAdd) {
+  private ExprValue exprApplyTime(FunctionProperties functionProperties,
+                                  ExprValue temporal, ExprValue temporalDelta, Boolean isAdd) {
     var interval = Duration.between(LocalTime.MIN, temporalDelta.timeValue());
     var result = isAdd
-        ? temporal.datetimeValue().plus(interval)
-        : temporal.datetimeValue().minus(interval);
+        ? extractDateTime(temporal, functionProperties).plus(interval)
+        : extractDateTime(temporal, functionProperties).minus(interval);
     return temporal.type() == TIME
         ? new ExprTimeValue(result.toLocalTime())
         : new ExprDatetimeValue(result);
@@ -725,12 +762,14 @@ private ExprValue exprApplyTime(ExprValue temporal, ExprValue temporalDelta, Boo
   /**
    * Adds time to date and returns the result.
    *
+   * @param functionProperties A FunctionProperties object.
    * @param temporal A Date/Time/Datetime/Timestamp value to change.
    * @param temporalDelta A Date/Time/Datetime/Timestamp object to add time from.
    * @return A value calculated.
    */
-  private ExprValue exprAddTime(ExprValue temporal, ExprValue temporalDelta) {
-    return exprApplyTime(temporal, temporalDelta, true);
+  private ExprValue exprAddTime(FunctionProperties functionProperties,
+                                ExprValue temporal, ExprValue temporalDelta) {
+    return exprApplyTime(functionProperties, temporal, temporalDelta, true);
   }
 
   /**
@@ -1124,8 +1163,9 @@ private ExprValue exprSubDateInterval(ExprValue date, ExprValue expr) {
    * @param temporalDelta A Date/Time/Datetime/Timestamp to subtract time from.
    * @return A value calculated.
    */
-  private ExprValue exprSubTime(ExprValue temporal, ExprValue temporalDelta) {
-    return exprApplyTime(temporal, temporalDelta, false);
+  private ExprValue exprSubTime(FunctionProperties functionProperties,
+                                ExprValue temporal, ExprValue temporalDelta) {
+    return exprApplyTime(functionProperties, temporal, temporalDelta, false);
   }
 
   /**

diff --git a/core/src/main/java/org/opensearch/sql/expression/function/FunctionDSL.java b/core/src/main/java/org/opensearch/sql/expression/function/FunctionDSL.java
@@ -137,6 +137,52 @@ public String toString() {
     };
   }
 
+  /**
+   * Implementation of a function that takes two arguments, returns a value, and
+   * requires FunctionProperties to complete.
+   *
+   * @param function   {@link ExprValue} based unary function.
+   * @param returnType return type.
+   * @param args1Type first argument type.
+   * @param args2Type second argument type.
+   * @return Unary Function Implementation.
+   */
+  public static SerializableFunction<FunctionName, Pair<FunctionSignature, FunctionBuilder>>
+      implWithProperties(
+      SerializableTriFunction<FunctionProperties, ExprValue, ExprValue, ExprValue> function,
+      ExprType returnType,
+      ExprType args1Type,
+      ExprType args2Type) {
+
+    return functionName -> {
+      FunctionSignature functionSignature =
+          new FunctionSignature(functionName, Arrays.asList(args1Type, args2Type));
+      FunctionBuilder functionBuilder =
+          (functionProperties, arguments) -> new FunctionExpression(functionName, arguments) {
+            @Override
+            public ExprValue valueOf(Environment<Expression, ExprValue> valueEnv) {
+              ExprValue arg1 = arguments.get(0).valueOf(valueEnv);
+              ExprValue arg2 = arguments.get(1).valueOf(valueEnv);
+              return function.apply(functionProperties, arg1, arg2);
+            }
+
+            @Override
+            public ExprType type() {
+              return returnType;
+            }
+
+            @Override
+            public String toString() {
+              return String.format("%s(%s)", functionName,
+                  arguments.stream()
+                      .map(Object::toString)
+                      .collect(Collectors.joining(", ")));
+            }
+          };
+      return Pair.of(functionSignature, functionBuilder);
+    };
+  }
+
   /**
    * No Arg Function Implementation.
    *
@@ -317,4 +363,22 @@ public SerializableTriFunction<ExprValue, ExprValue, ExprValue, ExprValue> nullM
       }
     };
   }
+
+  /**
+   * Wrapper for the ExprValue function that takes 2 arguments and is aware of FunctionProperties,
+   * with default NULL and MISSING handling.
+   */
+  public static SerializableTriFunction<FunctionProperties, ExprValue, ExprValue, ExprValue>
+        nullMissingHandlingWithProperties(
+      SerializableTriFunction<FunctionProperties, ExprValue, ExprValue,ExprValue> implementation) {
+    return (functionProperties, v1, v2) -> {
+      if (v1.isMissing() || v2.isMissing()) {
+        return ExprValueUtils.missingValue();
+      } else if (v1.isNull() || v2.isNull()) {
+        return ExprValueUtils.nullValue();
+      } else {
+        return implementation.apply(functionProperties, v1, v2);
+      }
+    };
+  }
 }
diff --git a/core/src/main/java/org/opensearch/sql/utils/DateTimeUtils.java b/core/src/main/java/org/opensearch/sql/utils/DateTimeUtils.java
@@ -10,6 +10,9 @@
 import java.time.ZoneId;
 import java.time.ZonedDateTime;
 import lombok.experimental.UtilityClass;
+import org.opensearch.sql.data.model.ExprTimeValue;
+import org.opensearch.sql.data.model.ExprValue;
+import org.opensearch.sql.expression.function.FunctionProperties;
 
 @UtilityClass
 public class DateTimeUtils {
@@ -125,4 +128,15 @@ public Boolean isValidMySqlTimeZoneId(ZoneId zone) {
         && (passedTzValidator.isAfter(minTzValidator)
         || passedTzValidator.isEqual(minTzValidator));
   }
+
+  /**
+   * Extracts LocalDateTime from a datetime ExprValue.
+   * Uses `FunctionProperties` for `ExprTimeValue`.
+   */
+  public static LocalDateTime extractDateTime(ExprValue value,
+                                              FunctionProperties functionProperties) {
+    return value instanceof ExprTimeValue
+        ? ((ExprTimeValue) value).datetimeValue(functionProperties)
+        : value.datetimeValue();
+  }
 }
diff --git a/core/src/test/java/org/opensearch/sql/data/model/DateTimeValueTest.java b/core/src/test/java/org/opensearch/sql/data/model/DateTimeValueTest.java
@@ -19,25 +19,41 @@
 import org.junit.jupiter.api.Test;
 import org.opensearch.sql.exception.ExpressionEvaluationException;
 import org.opensearch.sql.exception.SemanticCheckException;
+import org.opensearch.sql.expression.function.FunctionProperties;
 
 public class DateTimeValueTest {
 
   private static final int NANOS_PRECISION_MAX = 9;
 
   @Test
   public void timeValueInterfaceTest() {
-    ExprValue timeValue = new ExprTimeValue("01:01:01");
+    ExprTimeValue timeValue = new ExprTimeValue("01:01:01");
 
     assertEquals(TIME, timeValue.type());
+
     assertEquals(LocalTime.parse("01:01:01"), timeValue.timeValue());
-    assertEquals(LocalDate.now(), timeValue.dateValue());
-    assertEquals(LocalDate.now().atTime(1, 1, 1), timeValue.datetimeValue());
-    assertEquals(ZonedDateTime.of(LocalTime.parse("01:01:01").atDate(LocalDate.now()),
-        ExprTimestampValue.ZONE).toInstant(), timeValue.timestampValue());
+    // It is prohibited to acquire values which include date part from `ExprTimeValue`
+    // without a FunctionProperties object
+    var exception = assertThrows(ExpressionEvaluationException.class, timeValue::dateValue);
+    assertEquals("invalid to get dateValue from value of type TIME", exception.getMessage());
+    exception = assertThrows(ExpressionEvaluationException.class, timeValue::datetimeValue);
+    assertEquals("invalid to get datetimeValue from value of type TIME", exception.getMessage());
+    exception = assertThrows(ExpressionEvaluationException.class, timeValue::timestampValue);
+    assertEquals("invalid to get timestampValue from value of type TIME", exception.getMessage());
+
+    var functionProperties = new FunctionProperties();
+    var today = LocalDate.now(functionProperties.getQueryStartClock());
+
+    assertEquals(today, timeValue.dateValue(functionProperties));
+    assertEquals(today.atTime(1, 1, 1), timeValue.datetimeValue(functionProperties));
+    assertEquals(ZonedDateTime.of(LocalTime.parse("01:01:01").atDate(today),
+        ExprTimestampValue.ZONE).toInstant(), timeValue.timestampValue(functionProperties));
+
     assertEquals("01:01:01", timeValue.value());
     assertEquals("TIME '01:01:01'", timeValue.toString());
-    assertThrows(ExpressionEvaluationException.class, () -> integerValue(1).timeValue(),
-        "invalid to get timeValue from value of type INTEGER");
+    exception = assertThrows(ExpressionEvaluationException.class,
+        () -> integerValue(1).timeValue());
+    assertEquals("invalid to get timeValue from value of type INTEGER", exception.getMessage());
   }
 
   @Test