Extend comparison methods to accept different datetime types. (#129) (#…

…1196) (#1294) * Extend comparison methods to accept different datetime types. (#129) * Extend comparison methods to accept different datetime types. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> * Typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Rework fix according to @dai-chen's comment. #1196 (review) * Revert `BinaryPredicateOperator`. * Add automatic cast rules `DATE`/`TIME`/`DATETIME` -> `DATETIME`/TIMESTAMP`. * Update unit tests. Signed-off-by: Yury-Fridlyand <[email protected]> * Add doctest sample. Signed-off-by: Yury-Fridlyand <[email protected]> * Docs typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Minor comments update. Signed-off-by: Yury-Fridlyand <[email protected]> * Doctest typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Doctest typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Doctest typo fix. Signed-off-by: Yury-Fridlyand <[email protected]> * Modify `ExprCoreType` dependencies. Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> Signed-off-by: Yury-Fridlyand <[email protected]> (cherry picked from commit a4f8066) Co-authored-by: Yury-Fridlyand <[email protected]>
opensearch-project · Jan 26, 2023 · 982d366 · 982d366
1 parent db9be18
commit 982d366
Show file tree

Hide file tree

Showing 19 changed files with 1,796 additions and 104 deletions.
diff --git a/core/src/main/java/org/opensearch/sql/data/model/AbstractExprValue.java b/core/src/main/java/org/opensearch/sql/data/model/AbstractExprValue.java
@@ -19,9 +19,11 @@ public abstract class AbstractExprValue implements ExprValue {
   public int compareTo(ExprValue other) {
     if (this.isNull() || this.isMissing() || other.isNull() || other.isMissing()) {
       throw new IllegalStateException(
-           String.format("[BUG] Unreachable, Comparing with NULL or MISSING is undefined"));
+           "[BUG] Unreachable, Comparing with NULL or MISSING is undefined");
     }
-    if ((this.isNumber() && other.isNumber()) || this.type() == other.type()) {
+    if ((this.isNumber() && other.isNumber())
+        || (this.isDateTime() && other.isDateTime())
+        || this.type() == other.type()) {
       return compare(other);
     } else {
       throw new ExpressionEvaluationException(

diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprDateValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprDateValue.java
@@ -71,6 +71,11 @@ public Instant timestampValue() {
     return ZonedDateTime.of(date, timeValue(), ExprTimestampValue.ZONE).toInstant();
   }
 
+  @Override
+  public boolean isDateTime() {
+    return true;
+  }
+
   @Override
   public String toString() {
     return String.format("DATE '%s'", value());

diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprDatetimeValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprDatetimeValue.java
@@ -73,6 +73,11 @@ public Instant timestampValue() {
     return ZonedDateTime.of(datetime, ExprTimestampValue.ZONE).toInstant();
   }
 
+  @Override
+  public boolean isDateTime() {
+    return true;
+  }
+
   @Override
   public int compare(ExprValue other) {
     return datetime.compareTo(other.datetimeValue());

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
@@ -70,6 +70,11 @@ public Instant timestampValue(FunctionProperties functionProperties) {
         .toInstant();
   }
 
+  @Override
+  public boolean isDateTime() {
+    return true;
+  }
+
   @Override
   public String toString() {
     return String.format("TIME '%s'", value());

diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java
@@ -81,6 +81,11 @@ public LocalDateTime datetimeValue() {
     return timestamp.atZone(ZONE).toLocalDateTime();
   }
 
+  @Override
+  public boolean isDateTime() {
+    return true;
+  }
+
   @Override
   public String toString() {
     return String.format("TIMESTAMP '%s'", value());

diff --git a/core/src/main/java/org/opensearch/sql/data/model/ExprValue.java b/core/src/main/java/org/opensearch/sql/data/model/ExprValue.java
@@ -60,6 +60,15 @@ default boolean isNumber() {
     return false;
   }
 
+  /**
+   * Is Datetime value.
+   *
+   * @return true: is a datetime value, otherwise false
+   */
+  default boolean isDateTime() {
+    return false;
+  }
+
   /**
    * Get the {@link BindingTuple}.
    */

diff --git a/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java b/core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java
@@ -53,12 +53,11 @@ public enum ExprCoreType implements ExprType {
 
   /**
    * Date.
-   * Todo. compatible relationship.
    */
-  TIMESTAMP(STRING),
   DATE(STRING),
   TIME(STRING),
-  DATETIME(STRING),
+  DATETIME(STRING, DATE, TIME),
+  TIMESTAMP(STRING, DATETIME),
   INTERVAL(UNDEFINED),
 
   /**

diff --git a/core/src/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java
@@ -540,28 +540,52 @@ public static FunctionExpression not(Expression... expressions) {
     return compile(FunctionProperties.None, BuiltinFunctionName.NOT, expressions);
   }
 
+  public static FunctionExpression equal(FunctionProperties fp, Expression... expressions) {
+    return compile(fp, BuiltinFunctionName.EQUAL, expressions);
+  }
+
   public static FunctionExpression equal(Expression... expressions) {
-    return compile(FunctionProperties.None, BuiltinFunctionName.EQUAL, expressions);
+    return equal(FunctionProperties.None, expressions);
+  }
+
+  public static FunctionExpression notequal(FunctionProperties fp, Expression... expressions) {
+    return compile(fp, BuiltinFunctionName.NOTEQUAL, expressions);
   }
 
   public static FunctionExpression notequal(Expression... expressions) {
-    return compile(FunctionProperties.None, BuiltinFunctionName.NOTEQUAL, expressions);
+    return notequal(FunctionProperties.None, expressions);
+  }
+
+  public static FunctionExpression less(FunctionProperties fp, Expression... expressions) {
+    return compile(fp, BuiltinFunctionName.LESS, expressions);
   }
 
   public static FunctionExpression less(Expression... expressions) {
-    return compile(FunctionProperties.None, BuiltinFunctionName.LESS, expressions);
+    return less(FunctionProperties.None, expressions);
+  }
+
+  public static FunctionExpression lte(FunctionProperties fp, Expression... expressions) {
+    return compile(fp, BuiltinFunctionName.LTE, expressions);
   }
 
   public static FunctionExpression lte(Expression... expressions) {
-    return compile(FunctionProperties.None, BuiltinFunctionName.LTE, expressions);
+    return lte(FunctionProperties.None, expressions);
+  }
+
+  public static FunctionExpression greater(FunctionProperties fp, Expression... expressions) {
+    return compile(fp, BuiltinFunctionName.GREATER, expressions);
   }
 
   public static FunctionExpression greater(Expression... expressions) {
-    return compile(FunctionProperties.None, BuiltinFunctionName.GREATER, expressions);
+    return greater(FunctionProperties.None, expressions);
+  }
+
+  public static FunctionExpression gte(FunctionProperties fp, Expression... expressions) {
+    return compile(fp, BuiltinFunctionName.GTE, expressions);
   }
 
   public static FunctionExpression gte(Expression... expressions) {
-    return compile(FunctionProperties.None, BuiltinFunctionName.GTE, expressions);
+    return gte(FunctionProperties.None, expressions);
   }
 
   public static FunctionExpression like(Expression... expressions) {

diff --git a/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperator.java
@@ -19,7 +19,9 @@
 import static org.opensearch.sql.data.type.ExprCoreType.TIME;
 import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP;
 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 java.util.Arrays;
 import java.util.stream.Collectors;
@@ -176,12 +178,18 @@ private static DefaultFunctionResolver castToTime() {
     );
   }
 
+  // `DATE`/`TIME`/`DATETIME` -> `DATETIME`/TIMESTAMP` cast tested in BinaryPredicateOperatorTest
   private static DefaultFunctionResolver castToTimestamp() {
     return FunctionDSL.define(BuiltinFunctionName.CAST_TO_TIMESTAMP.getName(),
         impl(nullMissingHandling(
             (v) -> new ExprTimestampValue(v.stringValue())), TIMESTAMP, STRING),
         impl(nullMissingHandling(
             (v) -> new ExprTimestampValue(v.timestampValue())), TIMESTAMP, DATETIME),
+        impl(nullMissingHandling(
+            (v) -> new ExprTimestampValue(v.timestampValue())), TIMESTAMP, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(
+            (fp, v) -> new ExprTimestampValue(((ExprTimeValue)v).timestampValue(fp))),
+            TIMESTAMP, TIME),
         impl(nullMissingHandling((v) -> v), TIMESTAMP, TIMESTAMP)
     );
   }
@@ -193,7 +201,11 @@ private static DefaultFunctionResolver castToDatetime() {
         impl(nullMissingHandling(
             (v) -> new ExprDatetimeValue(v.datetimeValue())), DATETIME, TIMESTAMP),
         impl(nullMissingHandling(
-            (v) -> new ExprDatetimeValue(v.datetimeValue())), DATETIME, DATE)
+            (v) -> new ExprDatetimeValue(v.datetimeValue())), DATETIME, DATE),
+        implWithProperties(nullMissingHandlingWithProperties(
+            (fp, v) -> new ExprDatetimeValue(((ExprTimeValue)v).datetimeValue(fp))),
+            DATETIME, TIME),
+        impl(nullMissingHandling((v) -> v), DATETIME, DATETIME)
     );
   }
 }
diff --git a/core/src/test/java/org/opensearch/sql/data/model/ExprBooleanValueTest.java b/core/src/test/java/org/opensearch/sql/data/model/ExprBooleanValueTest.java
@@ -8,18 +8,55 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.opensearch.sql.utils.ComparisonUtil.compare;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN;
 
 import org.junit.jupiter.api.Test;
 import org.opensearch.sql.exception.ExpressionEvaluationException;
 
 public class ExprBooleanValueTest {
 
   @Test
-  public void comparabilityTest() {
-    ExprValue booleanValue = ExprValueUtils.booleanValue(false);
-    ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class,
-        () -> compare(booleanValue, booleanValue));
-    assertEquals("ExprBooleanValue instances are not comparable", exception.getMessage());
+  public void equals_to_self() {
+    ExprValue value = ExprValueUtils.booleanValue(false);
+    assertEquals(value.booleanValue(), false);
+    assertTrue(value.equals(value));
+  }
+
+  @Test
+  public void equal() {
+    ExprValue v1 = ExprBooleanValue.of(true);
+    ExprValue v2 = ExprBooleanValue.of(true);
+    assertTrue(v1.equals(v2));
+    assertTrue(v2.equals(v1));
+    assertEquals(0, ((ExprBooleanValue)v1).compare((ExprBooleanValue)v2));
+    assertEquals(0, ((ExprBooleanValue)v2).compare((ExprBooleanValue)v1));
+  }
+
+  @Test
+  public void compare() {
+    var v1 = ExprBooleanValue.of(true);
+    var v2 = ExprBooleanValue.of(false);
+    assertEquals(1, v1.compare(v2));
+    assertEquals(-1, v2.compare(v1));
+  }
+
+  @Test
+  public void invalid_get_value() {
+    ExprDateValue value = new ExprDateValue("2020-08-20");
+    assertThrows(ExpressionEvaluationException.class, value::booleanValue,
+        String.format("invalid to get booleanValue from value of type %s", value.type()));
+  }
+
+  @Test
+  public void value() {
+    ExprValue value = ExprBooleanValue.of(true);
+    assertEquals(true, value.value());
+  }
+
+  @Test
+  public void type() {
+    ExprValue value = ExprBooleanValue.of(false);
+    assertEquals(BOOLEAN, value.type());
   }
 }
diff --git a/core/src/test/java/org/opensearch/sql/data/model/ExprStringValueTest.java b/core/src/test/java/org/opensearch/sql/data/model/ExprStringValueTest.java
@@ -0,0 +1,60 @@
+/*
+ * Copyright OpenSearch Contributors
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+package org.opensearch.sql.data.model;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.opensearch.sql.data.type.ExprCoreType.STRING;
+
+import org.junit.jupiter.api.Test;
+import org.opensearch.sql.exception.ExpressionEvaluationException;
+
+public class ExprStringValueTest {
+  @Test
+  public void equals_to_self() {
+    ExprValue string = ExprValueUtils.stringValue("str");
+    assertEquals(string.stringValue(), "str");
+    assertTrue(string.equals(string));
+  }
+
+  @Test
+  public void equal() {
+    ExprValue v1 = new ExprStringValue("str");
+    ExprValue v2 = ExprValueUtils.stringValue("str");
+    assertTrue(v1.equals(v2));
+    assertTrue(v2.equals(v1));
+    assertEquals(0, ((ExprStringValue)v1).compare((ExprStringValue)v2));
+    assertEquals(0, ((ExprStringValue)v2).compare((ExprStringValue)v1));
+  }
+
+  @Test
+  public void compare() {
+    ExprStringValue v1 = new ExprStringValue("str1");
+    ExprStringValue v2 = new ExprStringValue("str2");
+    assertEquals(-1, v1.compare(v2));
+    assertEquals(1, v2.compare(v1));
+  }
+
+  @Test
+  public void invalid_get_value() {
+    ExprDateValue value = new ExprDateValue("2020-08-20");
+    assertThrows(ExpressionEvaluationException.class, value::stringValue,
+        String.format("invalid to get intervalValue from value of type %s", value.type()));
+  }
+
+  @Test
+  public void value() {
+    ExprValue value = new ExprStringValue("string");
+    assertEquals("string", value.value());
+  }
+
+  @Test
+  public void type() {
+    ExprValue value = new ExprStringValue("string");
+    assertEquals(STRING, value.type());
+  }
+}