Skip to content

Commit 982d366

Browse files
opensearch-trigger-bot[bot]Yury-Fridlyand
opensearch-trigger-bot[bot]
and
Yury-Fridlyand
authored
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]>
1 parent db9be18 commit 982d366

File tree

19 files changed

+1796
-104
lines changed

19 files changed

+1796
-104
lines changed

‎core/src/main/java/org/opensearch/sql/data/model/AbstractExprValue.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,11 @@ public abstract class AbstractExprValue implements ExprValue {
1919
public int compareTo(ExprValue other) {
2020
if (this.isNull() || this.isMissing() || other.isNull() || other.isMissing()) {
2121
throw new IllegalStateException(
22-
String.format("[BUG] Unreachable, Comparing with NULL or MISSING is undefined"));
22+
"[BUG] Unreachable, Comparing with NULL or MISSING is undefined");
2323
}
24-
if ((this.isNumber() && other.isNumber()) || this.type() == other.type()) {
24+
if ((this.isNumber() && other.isNumber())
25+
|| (this.isDateTime() && other.isDateTime())
26+
|| this.type() == other.type()) {
2527
return compare(other);
2628
} else {
2729
throw new ExpressionEvaluationException(

‎core/src/main/java/org/opensearch/sql/data/model/ExprDateValue.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ public Instant timestampValue() {
7171
return ZonedDateTime.of(date, timeValue(), ExprTimestampValue.ZONE).toInstant();
7272
}
7373

74+
@Override
75+
public boolean isDateTime() {
76+
return true;
77+
}
78+
7479
@Override
7580
public String toString() {
7681
return String.format("DATE '%s'", value());

‎core/src/main/java/org/opensearch/sql/data/model/ExprDatetimeValue.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,11 @@ public Instant timestampValue() {
7373
return ZonedDateTime.of(datetime, ExprTimestampValue.ZONE).toInstant();
7474
}
7575

76+
@Override
77+
public boolean isDateTime() {
78+
return true;
79+
}
80+
7681
@Override
7782
public int compare(ExprValue other) {
7883
return datetime.compareTo(other.datetimeValue());

‎core/src/main/java/org/opensearch/sql/data/model/ExprTimeValue.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ public Instant timestampValue(FunctionProperties functionProperties) {
7070
.toInstant();
7171
}
7272

73+
@Override
74+
public boolean isDateTime() {
75+
return true;
76+
}
77+
7378
@Override
7479
public String toString() {
7580
return String.format("TIME '%s'", value());

‎core/src/main/java/org/opensearch/sql/data/model/ExprTimestampValue.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,11 @@ public LocalDateTime datetimeValue() {
8181
return timestamp.atZone(ZONE).toLocalDateTime();
8282
}
8383

84+
@Override
85+
public boolean isDateTime() {
86+
return true;
87+
}
88+
8489
@Override
8590
public String toString() {
8691
return String.format("TIMESTAMP '%s'", value());

‎core/src/main/java/org/opensearch/sql/data/model/ExprValue.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,15 @@ default boolean isNumber() {
6060
return false;
6161
}
6262

63+
/**
64+
* Is Datetime value.
65+
*
66+
* @return true: is a datetime value, otherwise false
67+
*/
68+
default boolean isDateTime() {
69+
return false;
70+
}
71+
6372
/**
6473
* Get the {@link BindingTuple}.
6574
*/

‎core/src/main/java/org/opensearch/sql/data/type/ExprCoreType.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,11 @@ public enum ExprCoreType implements ExprType {
5353

5454
/**
5555
* Date.
56-
* Todo. compatible relationship.
5756
*/
58-
TIMESTAMP(STRING),
5957
DATE(STRING),
6058
TIME(STRING),
61-
DATETIME(STRING),
59+
DATETIME(STRING, DATE, TIME),
60+
TIMESTAMP(STRING, DATETIME),
6261
INTERVAL(UNDEFINED),
6362

6463
/**

‎core/src/main/java/org/opensearch/sql/expression/DSL.java

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -540,28 +540,52 @@ public static FunctionExpression not(Expression... expressions) {
540540
return compile(FunctionProperties.None, BuiltinFunctionName.NOT, expressions);
541541
}
542542

543+
public static FunctionExpression equal(FunctionProperties fp, Expression... expressions) {
544+
return compile(fp, BuiltinFunctionName.EQUAL, expressions);
545+
}
546+
543547
public static FunctionExpression equal(Expression... expressions) {
544-
return compile(FunctionProperties.None, BuiltinFunctionName.EQUAL, expressions);
548+
return equal(FunctionProperties.None, expressions);
549+
}
550+
551+
public static FunctionExpression notequal(FunctionProperties fp, Expression... expressions) {
552+
return compile(fp, BuiltinFunctionName.NOTEQUAL, expressions);
545553
}
546554

547555
public static FunctionExpression notequal(Expression... expressions) {
548-
return compile(FunctionProperties.None, BuiltinFunctionName.NOTEQUAL, expressions);
556+
return notequal(FunctionProperties.None, expressions);
557+
}
558+
559+
public static FunctionExpression less(FunctionProperties fp, Expression... expressions) {
560+
return compile(fp, BuiltinFunctionName.LESS, expressions);
549561
}
550562

551563
public static FunctionExpression less(Expression... expressions) {
552-
return compile(FunctionProperties.None, BuiltinFunctionName.LESS, expressions);
564+
return less(FunctionProperties.None, expressions);
565+
}
566+
567+
public static FunctionExpression lte(FunctionProperties fp, Expression... expressions) {
568+
return compile(fp, BuiltinFunctionName.LTE, expressions);
553569
}
554570

555571
public static FunctionExpression lte(Expression... expressions) {
556-
return compile(FunctionProperties.None, BuiltinFunctionName.LTE, expressions);
572+
return lte(FunctionProperties.None, expressions);
573+
}
574+
575+
public static FunctionExpression greater(FunctionProperties fp, Expression... expressions) {
576+
return compile(fp, BuiltinFunctionName.GREATER, expressions);
557577
}
558578

559579
public static FunctionExpression greater(Expression... expressions) {
560-
return compile(FunctionProperties.None, BuiltinFunctionName.GREATER, expressions);
580+
return greater(FunctionProperties.None, expressions);
581+
}
582+
583+
public static FunctionExpression gte(FunctionProperties fp, Expression... expressions) {
584+
return compile(fp, BuiltinFunctionName.GTE, expressions);
561585
}
562586

563587
public static FunctionExpression gte(Expression... expressions) {
564-
return compile(FunctionProperties.None, BuiltinFunctionName.GTE, expressions);
588+
return gte(FunctionProperties.None, expressions);
565589
}
566590

567591
public static FunctionExpression like(Expression... expressions) {

‎core/src/main/java/org/opensearch/sql/expression/operator/convert/TypeCastOperator.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@
1919
import static org.opensearch.sql.data.type.ExprCoreType.TIME;
2020
import static org.opensearch.sql.data.type.ExprCoreType.TIMESTAMP;
2121
import static org.opensearch.sql.expression.function.FunctionDSL.impl;
22+
import static org.opensearch.sql.expression.function.FunctionDSL.implWithProperties;
2223
import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandling;
24+
import static org.opensearch.sql.expression.function.FunctionDSL.nullMissingHandlingWithProperties;
2325

2426
import java.util.Arrays;
2527
import java.util.stream.Collectors;
@@ -176,12 +178,18 @@ private static DefaultFunctionResolver castToTime() {
176178
);
177179
}
178180

181+
// `DATE`/`TIME`/`DATETIME` -> `DATETIME`/TIMESTAMP` cast tested in BinaryPredicateOperatorTest
179182
private static DefaultFunctionResolver castToTimestamp() {
180183
return FunctionDSL.define(BuiltinFunctionName.CAST_TO_TIMESTAMP.getName(),
181184
impl(nullMissingHandling(
182185
(v) -> new ExprTimestampValue(v.stringValue())), TIMESTAMP, STRING),
183186
impl(nullMissingHandling(
184187
(v) -> new ExprTimestampValue(v.timestampValue())), TIMESTAMP, DATETIME),
188+
impl(nullMissingHandling(
189+
(v) -> new ExprTimestampValue(v.timestampValue())), TIMESTAMP, DATE),
190+
implWithProperties(nullMissingHandlingWithProperties(
191+
(fp, v) -> new ExprTimestampValue(((ExprTimeValue)v).timestampValue(fp))),
192+
TIMESTAMP, TIME),
185193
impl(nullMissingHandling((v) -> v), TIMESTAMP, TIMESTAMP)
186194
);
187195
}
@@ -193,7 +201,11 @@ private static DefaultFunctionResolver castToDatetime() {
193201
impl(nullMissingHandling(
194202
(v) -> new ExprDatetimeValue(v.datetimeValue())), DATETIME, TIMESTAMP),
195203
impl(nullMissingHandling(
196-
(v) -> new ExprDatetimeValue(v.datetimeValue())), DATETIME, DATE)
204+
(v) -> new ExprDatetimeValue(v.datetimeValue())), DATETIME, DATE),
205+
implWithProperties(nullMissingHandlingWithProperties(
206+
(fp, v) -> new ExprDatetimeValue(((ExprTimeValue)v).datetimeValue(fp))),
207+
DATETIME, TIME),
208+
impl(nullMissingHandling((v) -> v), DATETIME, DATETIME)
197209
);
198210
}
199211
}

‎core/src/test/java/org/opensearch/sql/data/model/ExprBooleanValueTest.java

Lines changed: 43 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,55 @@
88

99
import static org.junit.jupiter.api.Assertions.assertEquals;
1010
import static org.junit.jupiter.api.Assertions.assertThrows;
11-
import static org.opensearch.sql.utils.ComparisonUtil.compare;
11+
import static org.junit.jupiter.api.Assertions.assertTrue;
12+
import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN;
1213

1314
import org.junit.jupiter.api.Test;
1415
import org.opensearch.sql.exception.ExpressionEvaluationException;
1516

1617
public class ExprBooleanValueTest {
1718

1819
@Test
19-
public void comparabilityTest() {
20-
ExprValue booleanValue = ExprValueUtils.booleanValue(false);
21-
ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class,
22-
() -> compare(booleanValue, booleanValue));
23-
assertEquals("ExprBooleanValue instances are not comparable", exception.getMessage());
20+
public void equals_to_self() {
21+
ExprValue value = ExprValueUtils.booleanValue(false);
22+
assertEquals(value.booleanValue(), false);
23+
assertTrue(value.equals(value));
24+
}
25+
26+
@Test
27+
public void equal() {
28+
ExprValue v1 = ExprBooleanValue.of(true);
29+
ExprValue v2 = ExprBooleanValue.of(true);
30+
assertTrue(v1.equals(v2));
31+
assertTrue(v2.equals(v1));
32+
assertEquals(0, ((ExprBooleanValue)v1).compare((ExprBooleanValue)v2));
33+
assertEquals(0, ((ExprBooleanValue)v2).compare((ExprBooleanValue)v1));
34+
}
35+
36+
@Test
37+
public void compare() {
38+
var v1 = ExprBooleanValue.of(true);
39+
var v2 = ExprBooleanValue.of(false);
40+
assertEquals(1, v1.compare(v2));
41+
assertEquals(-1, v2.compare(v1));
42+
}
43+
44+
@Test
45+
public void invalid_get_value() {
46+
ExprDateValue value = new ExprDateValue("2020-08-20");
47+
assertThrows(ExpressionEvaluationException.class, value::booleanValue,
48+
String.format("invalid to get booleanValue from value of type %s", value.type()));
49+
}
50+
51+
@Test
52+
public void value() {
53+
ExprValue value = ExprBooleanValue.of(true);
54+
assertEquals(true, value.value());
55+
}
56+
57+
@Test
58+
public void type() {
59+
ExprValue value = ExprBooleanValue.of(false);
60+
assertEquals(BOOLEAN, value.type());
2461
}
2562
}

‎core/src/test/java/org/opensearch/sql/data/model/ExprStringValueTest.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.data.model;
7+
8+
import static org.junit.jupiter.api.Assertions.assertEquals;
9+
import static org.junit.jupiter.api.Assertions.assertThrows;
10+
import static org.junit.jupiter.api.Assertions.assertTrue;
11+
import static org.opensearch.sql.data.type.ExprCoreType.STRING;
12+
13+
import org.junit.jupiter.api.Test;
14+
import org.opensearch.sql.exception.ExpressionEvaluationException;
15+
16+
public class ExprStringValueTest {
17+
@Test
18+
public void equals_to_self() {
19+
ExprValue string = ExprValueUtils.stringValue("str");
20+
assertEquals(string.stringValue(), "str");
21+
assertTrue(string.equals(string));
22+
}
23+
24+
@Test
25+
public void equal() {
26+
ExprValue v1 = new ExprStringValue("str");
27+
ExprValue v2 = ExprValueUtils.stringValue("str");
28+
assertTrue(v1.equals(v2));
29+
assertTrue(v2.equals(v1));
30+
assertEquals(0, ((ExprStringValue)v1).compare((ExprStringValue)v2));
31+
assertEquals(0, ((ExprStringValue)v2).compare((ExprStringValue)v1));
32+
}
33+
34+
@Test
35+
public void compare() {
36+
ExprStringValue v1 = new ExprStringValue("str1");
37+
ExprStringValue v2 = new ExprStringValue("str2");
38+
assertEquals(-1, v1.compare(v2));
39+
assertEquals(1, v2.compare(v1));
40+
}
41+
42+
@Test
43+
public void invalid_get_value() {
44+
ExprDateValue value = new ExprDateValue("2020-08-20");
45+
assertThrows(ExpressionEvaluationException.class, value::stringValue,
46+
String.format("invalid to get intervalValue from value of type %s", value.type()));
47+
}
48+
49+
@Test
50+
public void value() {
51+
ExprValue value = new ExprStringValue("string");
52+
assertEquals("string", value.value());
53+
}
54+
55+
@Test
56+
public void type() {
57+
ExprValue value = new ExprStringValue("string");
58+
assertEquals(STRING, value.type());
59+
}
60+
}

0 commit comments

Comments
 (0)