From c6fb336fb9b76de7a52c402ffc5a0b10b76f5348 Mon Sep 17 00:00:00 2001 From: penghuo Date: Wed, 29 Jul 2020 14:50:15 -0700 Subject: [PATCH 1/4] Add comparsion operator for SQL --- .../data/model/AbstractExprNumberValue.java | 8 + .../sql/data/model/AbstractExprValue.java | 9 ++ .../sql/data/model/ExprMissingValue.java | 21 +++ .../sql/data/model/ExprNullValue.java | 17 ++ .../sql/data/model/ExprValue.java | 9 ++ .../sql/data/type/WideningTypeRule.java | 6 +- .../sql/expression/DSL.java | 12 ++ .../function/BuiltinFunctionName.java | 9 +- .../sql/expression/function/FunctionDSL.java | 23 +++ .../predicate/BinaryPredicateOperator.java | 145 +++++++++--------- .../predicate/UnaryPredicateOperator.java | 80 +++++----- .../sql/utils/OperatorUtils.java | 8 +- .../sql/analysis/ExpressionAnalyzerTest.java | 1 + .../sql/data/model/ExprMissingValueTest.java | 11 +- .../sql/data/model/ExprNullValueTest.java | 11 +- .../sql/data/utils/ExprValueOrderingTest.java | 10 +- .../BinaryPredicateOperatorTest.java | 81 +++++----- .../predicate/UnaryPredicateOperatorTest.java | 39 ++++- docs/user/dql/expressions.rst | 77 ++++++++++ docs/user/index.rst | 2 + .../sql/ppl/OperatorIT.java | 22 +-- sql/src/main/antlr/OpenDistroSQLLexer.g4 | 1 + sql/src/main/antlr/OpenDistroSQLParser.g4 | 14 +- .../sql/sql/parser/AstExpressionBuilder.java | 35 +++++ .../sql/parser/AstExpressionBuilderTest.java | 53 +++++++ 25 files changed, 515 insertions(+), 189 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprNumberValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprNumberValue.java index e5214aa8a5..13a490eda1 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprNumberValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprNumberValue.java @@ -28,6 +28,14 @@ public abstract class AbstractExprNumberValue extends AbstractExprValue { private final Number value; @Override +<<<<<<< HEAD + public boolean isNumber() { + return true; + } + + @Override +======= +>>>>>>> develop public Integer integerValue() { return value.intValue(); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java index 1e6e51a336..cee02da817 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java @@ -33,13 +33,22 @@ public int compareTo(ExprValue other) { } else if (other.isNull() || other.isMissing()) { return -other.compareTo(this); } +<<<<<<< HEAD + if ((this.isNumber() && other.isNumber()) || this.type() == other.type()) { + return compare(other); + } else { +======= if (!this.type().equals(other.type())) { +>>>>>>> develop throw new ExpressionEvaluationException( String.format( "compare expected value have same type, but with [%s, %s]", this.type(), other.type())); } +<<<<<<< HEAD +======= return compare(other); +>>>>>>> develop } /** diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java index 7eb29ab8f0..49a39467f0 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java @@ -15,6 +15,10 @@ package com.amazon.opendistroforelasticsearch.sql.data.model; +<<<<<<< HEAD +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; +======= +>>>>>>> develop import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.exception.ExpressionEvaluationException; import java.util.Objects; @@ -40,7 +44,11 @@ public Object value() { @Override public ExprType type() { +<<<<<<< HEAD + return ExprCoreType.UNKNOWN; +======= throw new ExpressionEvaluationException("invalid to call type operation on missing value"); +>>>>>>> develop } @Override @@ -50,8 +58,13 @@ public boolean isMissing() { /** * When MISSING value compare to other expression value. +<<<<<<< HEAD + * 1) NULL is equal to MISSING. + * 2) NULL is less than all other expression values. +======= * 1) MISSING is equal to MISSING. * 2) MISSING is less than all other expression values. +>>>>>>> develop */ @Override public int compare(ExprValue other) { @@ -70,4 +83,12 @@ public boolean equal(ExprValue other) { public int hashCode() { return Objects.hashCode("MISSING"); } +<<<<<<< HEAD + + @Override + public String toString() { + return "MISSING"; + } +======= +>>>>>>> develop } \ No newline at end of file diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java index 64d035e82f..ab0e1c9db5 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java @@ -15,8 +15,13 @@ package com.amazon.opendistroforelasticsearch.sql.data.model; +<<<<<<< HEAD +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; +======= import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; import com.amazon.opendistroforelasticsearch.sql.exception.ExpressionEvaluationException; +>>>>>>> develop import java.util.Objects; /** @@ -37,6 +42,14 @@ public int hashCode() { return Objects.hashCode("NULL"); } +<<<<<<< HEAD + @Override + public String toString() { + return "NULL"; + } + +======= +>>>>>>> develop public static ExprValue of() { return instance; } @@ -48,7 +61,11 @@ public Object value() { @Override public ExprType type() { +<<<<<<< HEAD + return ExprCoreType.UNKNOWN; +======= throw new ExpressionEvaluationException("invalid to call type operation on null value"); +>>>>>>> develop } @Override diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValue.java index fc005a3301..f902e4d7a8 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValue.java @@ -58,6 +58,15 @@ default boolean isMissing() { return false; } + /** + * Is Number value. + * + * @return true: is number value, otherwise false + */ + default boolean isNumber() { + return false; + } + /** * Get the {@link BindingTuple}. */ diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/type/WideningTypeRule.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/type/WideningTypeRule.java index 26d4950d1f..df4033ec0b 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/type/WideningTypeRule.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/type/WideningTypeRule.java @@ -52,10 +52,10 @@ public static int distance(ExprType type1, ExprType type2) { } private static int distance(ExprType type1, ExprType type2, int distance) { - if (type1 == UNKNOWN) { - return IMPOSSIBLE_WIDENING; - } else if (type1 == type2) { + if (type1 == type2) { return distance; + } else if (type1 == UNKNOWN) { + return IMPOSSIBLE_WIDENING; } else { return type1.getParent().stream() .map(parentOfType1 -> distance(parentOfType1, type2, distance + 1)) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java index 34a648b385..f0d38ae143 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/DSL.java @@ -268,6 +268,10 @@ public FunctionExpression like(Expression... expressions) { return function(BuiltinFunctionName.LIKE, expressions); } + public FunctionExpression notLike(Expression... expressions) { + return function(BuiltinFunctionName.NOT_LIKE, expressions); + } + public Aggregator avg(Expression... expressions) { return aggregate(BuiltinFunctionName.AVG, expressions); } @@ -289,4 +293,12 @@ private Aggregator aggregate(BuiltinFunctionName functionName, Expression... exp return (Aggregator) repository.compile( functionName.getName(), Arrays.asList(expressions)); } + + public FunctionExpression isnull(Expression... expressions) { + return function(BuiltinFunctionName.IS_NULL, expressions); + } + + public FunctionExpression isnotnull(Expression... expressions) { + return function(BuiltinFunctionName.IS_NOT_NULL, expressions); + } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java index a17b5d65d2..f38f3c5848 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/BuiltinFunctionName.java @@ -76,6 +76,7 @@ public enum BuiltinFunctionName { GREATER(FunctionName.of(">")), GTE(FunctionName.of(">=")), LIKE(FunctionName.of("like")), + NOT_LIKE(FunctionName.of("not like")), /** * Date and Time Functions. @@ -87,7 +88,13 @@ public enum BuiltinFunctionName { */ AVG(FunctionName.of("avg")), SUM(FunctionName.of("sum")), - COUNT(FunctionName.of("count")); + COUNT(FunctionName.of("count")), + + /** + * NULL Test. + */ + IS_NULL(FunctionName.of("is null")), + IS_NOT_NULL(FunctionName.of("is not null")); private final FunctionName name; diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/FunctionDSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/FunctionDSL.java index 841027656f..0c5f89a119 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/FunctionDSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/function/FunctionDSL.java @@ -119,7 +119,11 @@ public String toString() { * @param args2Type argument type. * @return Unary Function Implementation. */ +<<<<<<< HEAD + public Function> impl( +======= public SerializableFunction> impl( +>>>>>>> develop SerializableBiFunction function, ExprType returnType, ExprType args1Type, @@ -167,4 +171,23 @@ public SerializableFunction nullMissingHandling( } }; } +<<<<<<< HEAD + + /** + * Wrapper the binary ExprValue function with default NULL and MISSING handling. + */ + public SerializableBiFunction nullMissingHandling( + SerializableBiFunction function) { + return (v1, v2) -> { + if (v1.isMissing() || v2.isMissing()) { + return ExprValueUtils.missingValue(); + } else if (v1.isNull() || v2.isNull()) { + return ExprValueUtils.nullValue(); + } else { + return function.apply(v1, v2); + } + }; + } +======= +>>>>>>> develop } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java index edfd60c153..54585ca0be 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java @@ -20,35 +20,35 @@ import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; -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; -import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.LONG; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; +<<<<<<< HEAD +======= import static com.amazon.opendistroforelasticsearch.sql.expression.operator.OperatorUtils.binaryOperator; import static com.amazon.opendistroforelasticsearch.sql.utils.OperatorUtils.matches; +>>>>>>> develop import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; -import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; -import com.amazon.opendistroforelasticsearch.sql.expression.Expression; -import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; -import com.amazon.opendistroforelasticsearch.sql.expression.env.Environment; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionRepository; +<<<<<<< HEAD +import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL; +======= import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionBuilder; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionName; +>>>>>>> develop import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionResolver; -import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionSignature; -import com.google.common.collect.ImmutableMap; +import com.amazon.opendistroforelasticsearch.sql.utils.OperatorUtils; import com.google.common.collect.ImmutableTable; import com.google.common.collect.Table; +<<<<<<< HEAD +======= import java.util.Arrays; import java.util.Map; import java.util.function.BiFunction; +>>>>>>> develop import java.util.stream.Collectors; import lombok.experimental.UtilityClass; @@ -77,6 +77,7 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(greater()); repository.register(gte()); repository.register(like()); + repository.register(notLike()); } /** @@ -164,33 +165,21 @@ public static void register(BuiltinFunctionRepository repository) { .build(); private static FunctionResolver and() { - FunctionName functionName = BuiltinFunctionName.AND.getName(); - return FunctionResolver.builder() - .functionName(functionName) - .functionBundle(new FunctionSignature(functionName, - Arrays.asList(BOOLEAN, BOOLEAN)), binaryPredicate(functionName, - andTable, BOOLEAN)) - .build(); + return FunctionDSL.define(BuiltinFunctionName.AND.getName(), FunctionDSL + .impl((v1, v2) -> lookupTableFunction(v1, v2, andTable), BOOLEAN, BOOLEAN, + BOOLEAN)); } private static FunctionResolver or() { - FunctionName functionName = BuiltinFunctionName.OR.getName(); - return FunctionResolver.builder() - .functionName(functionName) - .functionBundle(new FunctionSignature(functionName, - Arrays.asList(BOOLEAN, BOOLEAN)), binaryPredicate(functionName, - orTable, BOOLEAN)) - .build(); + return FunctionDSL.define(BuiltinFunctionName.OR.getName(), FunctionDSL + .impl((v1, v2) -> lookupTableFunction(v1, v2, orTable), BOOLEAN, BOOLEAN, + BOOLEAN)); } private static FunctionResolver xor() { - FunctionName functionName = BuiltinFunctionName.XOR.getName(); - return FunctionResolver.builder() - .functionName(functionName) - .functionBundle(new FunctionSignature(functionName, - Arrays.asList(BOOLEAN, BOOLEAN)), binaryPredicate(functionName, - xorTable, BOOLEAN)) - .build(); + return FunctionDSL.define(BuiltinFunctionName.XOR.getName(), FunctionDSL + .impl((v1, v2) -> lookupTableFunction(v1, v2, xorTable), BOOLEAN, BOOLEAN, + BOOLEAN)); } private static FunctionResolver equal() { @@ -212,62 +201,65 @@ private static FunctionResolver notEqual() { } private static FunctionResolver less() { - return new FunctionResolver( - BuiltinFunctionName.LESS.getName(), - predicate( - BuiltinFunctionName.LESS.getName(), - (v1, v2) -> v1 < v2, - (v1, v2) -> v1 < v2, - (v1, v2) -> v1 < v2, - (v1, v2) -> v1 < v2, - (v1, v2) -> v1.compareTo(v2) < 0 - ) - ); + return FunctionDSL + .define(BuiltinFunctionName.LESS.getName(), ExprCoreType.coreTypes().stream() + .map(type -> FunctionDSL + .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) < 0), BOOLEAN, type, type)) + .collect( + Collectors.toList())); } private static FunctionResolver lte() { - return new FunctionResolver( - BuiltinFunctionName.LTE.getName(), - predicate( - BuiltinFunctionName.LTE.getName(), - (v1, v2) -> v1 <= v2, - (v1, v2) -> v1 <= v2, - (v1, v2) -> v1 <= v2, - (v1, v2) -> v1 <= v2, - (v1, v2) -> v1.compareTo(v2) <= 0 - ) - ); + return FunctionDSL + .define(BuiltinFunctionName.LTE.getName(), ExprCoreType.coreTypes().stream() + .map(type -> FunctionDSL + .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) <= 0), BOOLEAN, type, type)) + .collect( + Collectors.toList())); } private static FunctionResolver greater() { - return new FunctionResolver( - BuiltinFunctionName.GREATER.getName(), - predicate( - BuiltinFunctionName.GREATER.getName(), - (v1, v2) -> v1 > v2, - (v1, v2) -> v1 > v2, - (v1, v2) -> v1 > v2, - (v1, v2) -> v1 > v2, - (v1, v2) -> v1.compareTo(v2) > 0 - ) - ); + return FunctionDSL + .define(BuiltinFunctionName.GREATER.getName(), ExprCoreType.coreTypes().stream() + .map(type -> FunctionDSL + .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) > 0), BOOLEAN, type, type)) + .collect( + Collectors.toList())); } private static FunctionResolver gte() { - return new FunctionResolver( - BuiltinFunctionName.GTE.getName(), - predicate( - BuiltinFunctionName.GTE.getName(), - (v1, v2) -> v1 >= v2, - (v1, v2) -> v1 >= v2, - (v1, v2) -> v1 >= v2, - (v1, v2) -> v1 >= v2, - (v1, v2) -> v1.compareTo(v2) >= 0 - ) - ); + return FunctionDSL + .define(BuiltinFunctionName.GTE.getName(), ExprCoreType.coreTypes().stream() + .map(type -> FunctionDSL + .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) >= 0), BOOLEAN, type, type)) + .collect( + Collectors.toList())); } private static FunctionResolver like() { +<<<<<<< HEAD + return FunctionDSL.define(BuiltinFunctionName.LIKE.getName(), FunctionDSL + .impl(FunctionDSL.nullMissingHandling(OperatorUtils::matches), BOOLEAN, STRING, + STRING)); + } + + private static FunctionResolver notLike() { + return FunctionDSL.define(BuiltinFunctionName.NOT_LIKE.getName(), FunctionDSL + .impl(FunctionDSL.nullMissingHandling( + (v1, v2) -> UnaryPredicateOperator.not(OperatorUtils.matches(v1, v2))), + BOOLEAN, + STRING, + STRING)); + } + + private static ExprValue lookupTableFunction(ExprValue arg1, ExprValue arg2, + Table table) { + if (table.contains(arg1, arg2)) { + return table.get(arg1, arg2); + } else { + return table.get(arg2, arg1); + } +======= return new FunctionResolver( BuiltinFunctionName.LIKE.getName(), predicate( @@ -361,5 +353,6 @@ public String toString() { .get(1).toString()); } }; +>>>>>>> develop } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java index f849a72bff..18a621acb3 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperator.java @@ -15,26 +15,17 @@ package com.amazon.opendistroforelasticsearch.sql.expression.operator.predicate; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; -import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; -import com.amazon.opendistroforelasticsearch.sql.expression.Expression; -import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; -import com.amazon.opendistroforelasticsearch.sql.expression.env.Environment; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionRepository; -import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionBuilder; -import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionName; +import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionDSL; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionResolver; -import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionSignature; -import com.google.common.collect.ImmutableMap; import java.util.Arrays; -import java.util.Map; +import java.util.stream.Collectors; import lombok.experimental.UtilityClass; /** @@ -43,8 +34,18 @@ */ @UtilityClass public class UnaryPredicateOperator { + /** + * Register Unary Predicate Function. + */ public static void register(BuiltinFunctionRepository repository) { repository.register(not()); + repository.register(isNull()); + repository.register(isNotNull()); + } + + private static FunctionResolver not() { + return FunctionDSL.define(BuiltinFunctionName.NOT.getName(), FunctionDSL + .impl(UnaryPredicateOperator::not, BOOLEAN, BOOLEAN)); } /** @@ -55,41 +56,30 @@ public static void register(BuiltinFunctionRepository repository) { * NULL NULL * MISSING MISSING */ - private static Map notMap = - new ImmutableMap.Builder() - .put(LITERAL_TRUE, LITERAL_FALSE) - .put(LITERAL_FALSE, LITERAL_TRUE) - .put(LITERAL_NULL, LITERAL_NULL) - .put(LITERAL_MISSING, LITERAL_MISSING) - .build(); - - private static FunctionResolver not() { - FunctionName functionName = BuiltinFunctionName.NOT.getName(); - return FunctionResolver.builder() - .functionName(functionName) - .functionBundle(new FunctionSignature(functionName, - Arrays.asList(BOOLEAN)), predicateFunction(functionName, BOOLEAN)) - .build(); + public ExprValue not(ExprValue v) { + if (v.isMissing() || v.isNull()) { + return v; + } else { + return ExprBooleanValue.of(!v.booleanValue()); + } } - private static FunctionBuilder predicateFunction( - FunctionName functionName, - ExprType returnType) { - return arguments -> new FunctionExpression(functionName, arguments) { - @Override - public ExprValue valueOf(Environment env) { - return notMap.get(arguments.get(0).valueOf(env)); - } + private static FunctionResolver isNull() { - @Override - public ExprType type() { - return returnType; - } + return FunctionDSL + .define(BuiltinFunctionName.IS_NULL.getName(), Arrays.stream(ExprCoreType.values()) + .map(type -> FunctionDSL + .impl((v) -> ExprBooleanValue.of(v.isNull()), BOOLEAN, type)) + .collect( + Collectors.toList())); + } - @Override - public String toString() { - return String.format("%s %s", functionName, arguments.get(0).toString()); - } - }; + private static FunctionResolver isNotNull() { + return FunctionDSL + .define(BuiltinFunctionName.IS_NOT_NULL.getName(), Arrays.stream(ExprCoreType.values()) + .map(type -> FunctionDSL + .impl((v) -> ExprBooleanValue.of(!v.isNull()), BOOLEAN, type)) + .collect( + Collectors.toList())); } } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/OperatorUtils.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/OperatorUtils.java index 2b5479ebf9..d9b3352c69 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/OperatorUtils.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/OperatorUtils.java @@ -15,6 +15,8 @@ package com.amazon.opendistroforelasticsearch.sql.utils; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import java.util.regex.Pattern; import lombok.experimental.UtilityClass; @@ -27,8 +29,10 @@ public class OperatorUtils { * @param pattern string pattern to match. * @return if text matches pattern returns true; else return false. */ - public static boolean matches(String pattern, String text) { - return Pattern.compile(patternToRegex(pattern)).matcher(text).matches(); + public static ExprBooleanValue matches(ExprValue text, ExprValue pattern) { + return ExprBooleanValue + .of(Pattern.compile(patternToRegex(pattern.stringValue())).matcher(text.stringValue()) + .matches()); } private static final char DEFAULT_ESCAPE = '\\'; diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzerTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzerTest.java index b9ff216110..843007edac 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzerTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzerTest.java @@ -25,6 +25,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL; import com.amazon.opendistroforelasticsearch.sql.ast.expression.UnresolvedExpression; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprNullValue; import com.amazon.opendistroforelasticsearch.sql.exception.SemanticCheckException; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java index 13cdd5f24f..4d671e1d6b 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java @@ -16,11 +16,13 @@ package com.amazon.opendistroforelasticsearch.sql.data.model; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static com.amazon.opendistroforelasticsearch.sql.utils.ComparisonUtil.compare; 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 com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; import com.amazon.opendistroforelasticsearch.sql.exception.ExpressionEvaluationException; import org.junit.jupiter.api.Test; @@ -41,9 +43,12 @@ public void getValue() { @Test public void getType() { - ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class, - () -> LITERAL_MISSING.type()); - assertEquals("invalid to call type operation on missing value", exception.getMessage()); + assertEquals(ExprCoreType.UNKNOWN, LITERAL_MISSING.type()); + } + + @Test + public void toStringTest() { + assertEquals("MISSING", LITERAL_MISSING.toString()); } @Test diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java index 8b9127fe56..facfc7d851 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java @@ -15,7 +15,6 @@ package com.amazon.opendistroforelasticsearch.sql.data.model; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static com.amazon.opendistroforelasticsearch.sql.utils.ComparisonUtil.compare; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -23,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; +import com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType; import com.amazon.opendistroforelasticsearch.sql.exception.ExpressionEvaluationException; import org.junit.jupiter.api.Test; @@ -40,9 +40,12 @@ public void getValue() { @Test public void getType() { - ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class, - () -> LITERAL_NULL.type()); - assertEquals("invalid to call type operation on null value", exception.getMessage()); + assertEquals(ExprCoreType.UNKNOWN, LITERAL_NULL.type()); + } + + @Test + public void toStringTest() { + assertEquals("NULL", LITERAL_NULL.toString()); } @Test diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/utils/ExprValueOrderingTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/utils/ExprValueOrderingTest.java index 755501f86c..e80668ee57 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/utils/ExprValueOrderingTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/utils/ExprValueOrderingTest.java @@ -194,15 +194,21 @@ public void natural_order_tuple_value() { tupleValue(ImmutableMap.of("v1", 3)), tupleValue(ImmutableMap.of("v1", 1, "v2", 2)))); } + @Test + public void order_compare_value_with_compatible_number_type() { + ExprValueOrdering ordering = ExprValueOrdering.natural(); + assertEquals(1, ordering.compare(integerValue(2), doubleValue(1d))); + } + @Test public void order_compare_value_with_different_type() { ExprValueOrdering ordering = ExprValueOrdering.natural(); ExpressionEvaluationException exception = assertThrows( ExpressionEvaluationException.class, - () -> ordering.compare(integerValue(1), doubleValue(2d))); + () -> ordering.compare(integerValue(1), stringValue("2"))); assertEquals( - "compare expected value have same type, but with [INTEGER, DOUBLE]", + "compare expected value have same type, but with [INTEGER, STRING]", exception.getMessage()); } } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index 5194d70278..d3f0ede7cf 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -30,10 +30,10 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; - import static com.amazon.opendistroforelasticsearch.sql.utils.ComparisonUtil.compare; import static com.amazon.opendistroforelasticsearch.sql.utils.OperatorUtils.matches; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertTrue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; @@ -458,16 +458,16 @@ public void test_less_null() { FunctionExpression less = dsl.less(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_NULL, less.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_NULL, less.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_NULL, less.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); } @Test @@ -475,16 +475,16 @@ public void test_less_missing() { FunctionExpression less = dsl.less(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); } @Test @@ -492,12 +492,12 @@ public void test_null_less_missing() { FunctionExpression less = dsl.less(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, less.valueOf(valueEnv())); } @ParameterizedTest(name = "lte({0}, {1})") @@ -515,16 +515,16 @@ public void test_lte_null() { FunctionExpression lte = dsl.lte(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_NULL, lte.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_NULL, lte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_NULL, lte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); } @Test @@ -532,16 +532,16 @@ public void test_lte_missing() { FunctionExpression lte = dsl.lte(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); } @Test @@ -549,12 +549,12 @@ public void test_null_lte_missing() { FunctionExpression lte = dsl.lte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); } @ParameterizedTest(name = "greater({0}, {1})") @@ -572,16 +572,16 @@ public void test_greater_null() { FunctionExpression greater = dsl.greater(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_NULL, greater.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_NULL, greater.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_NULL, greater.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); } @Test @@ -589,16 +589,16 @@ public void test_greater_missing() { FunctionExpression greater = dsl.greater(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); } @Test @@ -606,12 +606,12 @@ public void test_null_greater_missing() { FunctionExpression greater = dsl.greater(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); } @ParameterizedTest(name = "gte({0}, {1})") @@ -629,16 +629,16 @@ public void test_gte_null() { FunctionExpression gte = dsl.gte(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_NULL, gte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_NULL, gte.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_NULL, gte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); } @Test @@ -646,16 +646,16 @@ public void test_gte_missing() { FunctionExpression gte = dsl.gte(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); } @Test @@ -663,12 +663,12 @@ public void test_null_gte_missing() { FunctionExpression gte = dsl.gte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); + assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); + assertEquals(LITERAL_FALSE, gte.valueOf(valueEnv())); } @ParameterizedTest(name = "like({0}, {1})") @@ -676,11 +676,22 @@ public void test_null_gte_missing() { public void test_like(ExprValue v1, ExprValue v2) { FunctionExpression like = dsl.like(DSL.literal(v1), DSL.literal(v2)); assertEquals(BOOLEAN, like.type()); - assertEquals(matches(((String) v2.value()), (String) v1.value()), - ExprValueUtils.getBooleanValue(like.valueOf(valueEnv()))); + assertEquals(matches(v1, v2), like.valueOf(valueEnv())); assertEquals(String.format("%s like %s", v1.toString(), v2.toString()), like.toString()); } + @Test + public void test_not_like() { + FunctionExpression notLike = dsl.notLike(DSL.literal("bob"), DSL.literal("tom")); + assertEquals(BOOLEAN, notLike.type()); + assertTrue(notLike.valueOf(valueEnv()).booleanValue()); + assertEquals(String.format("\"%s\" not like \"%s\"", "bob", "tom"), notLike.toString()); + + notLike = dsl.notLike(DSL.literal("bob"), DSL.literal("bo%")); + assertFalse(notLike.valueOf(valueEnv()).booleanValue()); + assertEquals(String.format("\"%s\" not like \"%s\"", "bob", "bo%"), notLike.toString()); + } + @Test public void test_like_null() { FunctionExpression like = diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java index 374f8e0fc8..bfdc097080 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/UnaryPredicateOperatorTest.java @@ -17,12 +17,15 @@ import static com.amazon.opendistroforelasticsearch.sql.config.TestConfig.BOOL_TYPE_MISSING_VALUE_FIELD; import static com.amazon.opendistroforelasticsearch.sql.config.TestConfig.BOOL_TYPE_NULL_VALUE_FIELD; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.booleanValue; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static org.junit.jupiter.api.Assertions.assertEquals; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprNullValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionTestBase; @@ -38,20 +41,42 @@ public void test_not(Boolean v) { FunctionExpression not = dsl.not(DSL.literal(booleanValue(v))); assertEquals(BOOLEAN, not.type()); assertEquals(!v, ExprValueUtils.getBooleanValue(not.valueOf(valueEnv()))); - assertEquals(String.format("not %s", v.toString()), not.toString()); + assertEquals(String.format("not(%s)", v.toString()), not.toString()); } @Test public void test_not_null() { - FunctionExpression and = dsl.not(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); - assertEquals(BOOLEAN, and.type()); - assertEquals(LITERAL_NULL, and.valueOf(valueEnv())); + FunctionExpression expression = dsl.not(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_NULL, expression.valueOf(valueEnv())); } @Test public void test_not_missing() { - FunctionExpression and = dsl.not(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); - assertEquals(BOOLEAN, and.type()); - assertEquals(LITERAL_MISSING, and.valueOf(valueEnv())); + FunctionExpression expression = dsl.not(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_MISSING, expression.valueOf(valueEnv())); + } + + @Test + public void is_null_predicate() { + FunctionExpression expression = dsl.isnull(DSL.literal(1)); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); + + expression = dsl.isnull(DSL.literal(ExprNullValue.of())); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); + } + + @Test + public void is_not_null_predicate() { + FunctionExpression expression = dsl.isnotnull(DSL.literal(1)); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_TRUE, expression.valueOf(valueEnv())); + + expression = dsl.isnotnull(DSL.literal(ExprNullValue.of())); + assertEquals(BOOLEAN, expression.type()); + assertEquals(LITERAL_FALSE, expression.valueOf(valueEnv())); } } \ No newline at end of file diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index dca1c6463b..6c6c821585 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -92,6 +92,83 @@ Here is an example for different type of arithmetic expressions:: | 3 | 2 | 2 | +---------+-------------+-------------+ +Comparison Operators +================================== + +Description +----------- + +Comparison operators are used to compare values. The MISSING and NULL value comparison has following the rule. MISSING value only equal to MISSING value and less than all the other values. NULL value equals to NULL value, large than MISSING value, but less than all the other values. + +Operators +````````` + ++----------------+--------------------------------+ +| name | description | ++----------------+--------------------------------+ +| > | Greater than operator | ++----------------+--------------------------------+ +| >= | Greater than or equal operator | ++----------------+--------------------------------+ +| < | Less than operator | ++----------------+--------------------------------+ +| != | Not equal operator | ++----------------+--------------------------------+ +| <= | Less than or equal operator | ++----------------+--------------------------------+ +| = | Equal operator | ++----------------+--------------------------------+ +| LIKE | Simple pattern matching | ++----------------+--------------------------------+ +| IS NULL | NULL value test | ++----------------+--------------------------------+ +| IS NOT NULL | NOT NULL value test | ++----------------+--------------------------------+ +| IS MISSING | MISSING value test | ++----------------+--------------------------------+ +| IS NOT MISSING | NOT MISSING value test | ++----------------+--------------------------------+ + + +Basic Comparison Operator +------------------------- + +Here is an example for different type of comparison operators:::: + + od> SELECT 2 > 1, 2 >= 1, 2 < 1, 2 != 1, 2 <= 1, 2 = 1; + fetched rows / total rows = 1/1 + +---------+----------+---------+----------+----------+---------+ + | 2 > 1 | 2 >= 1 | 2 < 1 | 2 != 1 | 2 <= 1 | 2 = 1 | + |---------+----------+---------+----------+----------+---------| + | True | True | False | True | False | False | + +---------+----------+---------+----------+----------+---------+ + +LIKE +---- + +expr LIKE pattern. The expr is string value, pattern is supports literal text, a percent ( % ) character for a wildcard, and an underscore ( _ ) character for a single character match:::: + + od> SELECT 'axyzb' LIKE 'a%b', 'acb' LIKE 'a_b', 'axyzb' NOT LIKE 'a%b', 'acb' NOT LIKE 'a_b'; + fetched rows / total rows = 1/1 + +----------------------+--------------------+--------------------------+------------------------+ + | "axyzb" like "a%b" | "acb" like "a_b" | "axyzb" not like "a%b" | "acb" not like "a_b" | + |----------------------+--------------------+--------------------------+------------------------| + | True | True | False | False | + +----------------------+--------------------+--------------------------+------------------------+ + +NULL value test +--------------- + +Here is an example for null value test:::: + + od> SELECT 0 IS NULL, 0 IS NOT NULL, NULL IS NULL, NULL IS NOT NULL; + fetched rows / total rows = 1/1 + +--------------+------------------+-----------------+---------------------+ + | is null(0) | is not null(0) | is null(NULL) | is not null(NULL) | + |--------------+------------------+-----------------+---------------------| + | False | True | True | False | + +--------------+------------------+-----------------+---------------------+ + Function Call ============= diff --git a/docs/user/index.rst b/docs/user/index.rst index d5f668e9f3..7688522954 100644 --- a/docs/user/index.rst +++ b/docs/user/index.rst @@ -22,6 +22,8 @@ Open Distro for Elasticsearch SQL enables you to extract insights out of Elastic - `Identifiers `_ - `Data Types `_ + - `Data Types `_ + * **Data Query Language** - `Expressions `_ diff --git a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/OperatorIT.java b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/OperatorIT.java index 5e632ce3a0..2dec2f8311 100644 --- a/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/OperatorIT.java +++ b/integ-test/src/test/java/com/amazon/opendistroforelasticsearch/sql/ppl/OperatorIT.java @@ -273,19 +273,21 @@ public void testLikeOperator() throws IOException { } @Test - public void testBinaryPredicateWithNullValue() { - queryExecutionShouldThrowExceptionDueToNullOrMissingValue( - String.format("source=%s | where age < 32", TEST_INDEX_BANK_WITH_NULL_VALUES), - "invalid to call type operation on null value" - ); + public void testBinaryPredicateWithNullValue() throws IOException { + JSONObject result = + executeQuery( + String.format("source=%s | where age >= 36 | fields age", + TEST_INDEX_BANK_WITH_NULL_VALUES)); + verifyDataRows(result, rows(36), rows(36)); } @Test - public void testBinaryPredicateWithMissingValue() { - queryExecutionShouldThrowExceptionDueToNullOrMissingValue( - String.format("source=%s | where balance > 3000", TEST_INDEX_BANK_WITH_NULL_VALUES), - "invalid to call type operation on missing value" - ); + public void testBinaryPredicateWithMissingValue() throws IOException { + JSONObject result = + executeQuery( + String.format("source=%s | where balance > 40000 | fields balance", + TEST_INDEX_BANK_WITH_NULL_VALUES)); + verifyDataRows(result, rows(48086)); } private void queryExecutionShouldThrowExceptionDueToNullOrMissingValue( diff --git a/sql/src/main/antlr/OpenDistroSQLLexer.g4 b/sql/src/main/antlr/OpenDistroSQLLexer.g4 index 66e3d1bade..bf56a09786 100644 --- a/sql/src/main/antlr/OpenDistroSQLLexer.g4 +++ b/sql/src/main/antlr/OpenDistroSQLLexer.g4 @@ -75,6 +75,7 @@ LIMIT: 'LIMIT'; LONG: 'LONG'; MATCH: 'MATCH'; NATURAL: 'NATURAL'; +MISSING_LITERAL: 'MISSING'; NOT: 'NOT'; NULL_LITERAL: 'NULL'; ON: 'ON'; diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index 49422c061c..72bc4e90df 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -88,8 +88,8 @@ constant | sign? realLiteral #signedReal | booleanLiteral #boolean | datetimeLiteral #datetime + | nullLiteral #null // Doesn't support the following types for now - //| nullLiteral #null //| BIT_STRING //| NOT? nullLiteral=(NULL_LITERAL | NULL_SPEC_LITERAL) //| LEFT_BRACE dateType=(D | T | TS | DATE | TIME | TIMESTAMP) stringLiteral RIGHT_BRACE @@ -147,6 +147,9 @@ expression predicate : expressionAtom #expressionAtomPredicate + | left=predicate comparisonOperator right=predicate #binaryComparisonPredicate + | predicate IS nullNotnull #isNullPredicate + | left=predicate NOT? LIKE right=predicate #likePredicate ; expressionAtom @@ -160,6 +163,15 @@ mathOperator : PLUS | MINUS | STAR | DIVIDE | MODULE ; +comparisonOperator + : '=' | '>' | '<' | '<' '=' | '>' '=' + | '<' '>' | '!' '=' + ; + +nullNotnull + : NOT? NULL_LITERAL + ; + functionCall : scalarFunctionName LR_BRACKET functionArgs? RR_BRACKET #scalarFunctionCall ; diff --git a/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java b/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java index b3fab91c47..2daf1c0822 100644 --- a/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java +++ b/sql/src/main/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilder.java @@ -17,6 +17,10 @@ package com.amazon.opendistroforelasticsearch.sql.sql.parser; import static com.amazon.opendistroforelasticsearch.sql.common.utils.StringUtils.unquoteIdentifier; +import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.IS_NOT_NULL; +import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.IS_NULL; +import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.LIKE; +import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.NOT_LIKE; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BooleanContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.MathExpressionAtomContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.ScalarFunctionCallContext; @@ -88,6 +92,22 @@ public UnresolvedExpression visitScalarFunctionCall(ScalarFunctionCallContext ct ); } + @Override + public UnresolvedExpression visitIsNullPredicate(OpenDistroSQLParser.IsNullPredicateContext ctx) { + return new Function( + ctx.nullNotnull().NOT() == null ? IS_NULL.getName().getFunctionName() : + IS_NOT_NULL.getName().getFunctionName(), + Arrays.asList(visit(ctx.predicate()))); + } + + @Override + public UnresolvedExpression visitLikePredicate(OpenDistroSQLParser.LikePredicateContext ctx) { + return new Function( + ctx.NOT() == null ? LIKE.getName().getFunctionName() : + NOT_LIKE.getName().getFunctionName(), + Arrays.asList(visit(ctx.left), visit(ctx.right))); + } + @Override public UnresolvedExpression visitString(StringContext ctx) { return AstDSL.stringLiteral(unquoteIdentifier(ctx.getText())); @@ -108,6 +128,11 @@ public UnresolvedExpression visitBoolean(BooleanContext ctx) { return AstDSL.booleanLiteral(Boolean.valueOf(ctx.getText())); } + @Override + public UnresolvedExpression visitNullLiteral(OpenDistroSQLParser.NullLiteralContext ctx) { + return AstDSL.nullLiteral(); + } + @Override public UnresolvedExpression visitDateLiteral(OpenDistroSQLParser.DateLiteralContext ctx) { return AstDSL.dateLiteral(unquoteIdentifier(ctx.date.getText())); @@ -124,6 +149,16 @@ public UnresolvedExpression visitTimestampLiteral( return AstDSL.timestampLiteral(unquoteIdentifier(ctx.timestamp.getText())); } + @Override + public UnresolvedExpression visitBinaryComparisonPredicate( + OpenDistroSQLParser.BinaryComparisonPredicateContext ctx) { + String functionName = ctx.comparisonOperator().getText(); + return new Function( + functionName.equals("<>") ? "!=" : functionName, + Arrays.asList(visit(ctx.left), visit(ctx.right)) + ); + } + private String visitQualifiedNameText(RuleNode node) { return unquoteIdentifier(node.getText()); } diff --git a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java index 5dece1f09f..cde9340313 100644 --- a/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java +++ b/sql/src/test/java/com/amazon/opendistroforelasticsearch/sql/sql/parser/AstExpressionBuilderTest.java @@ -21,6 +21,7 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.doubleLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.function; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.intLiteral; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.nullLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.stringLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.timeLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.timestampLiteral; @@ -142,6 +143,58 @@ public void canBuildDateAndTimeFunctionCall() { ); } + @Test + public void canBuildComparisonExpression() { + assertEquals( + function("!=", intLiteral(1), intLiteral(2)), + buildExprAst("1 != 2") + ); + + assertEquals( + function("!=", intLiteral(1), intLiteral(2)), + buildExprAst("1 <> 2") + ); + } + + @Test + public void canBuildNullTestExpression() { + assertEquals( + function("is null", intLiteral(1)), + buildExprAst("1 is NULL") + ); + + assertEquals( + function("is not null", intLiteral(1)), + buildExprAst("1 IS NOT null") + ); + } + + @Test + public void canBuildNullTestExpressionWithNULLLiteral() { + assertEquals( + function("is null", nullLiteral()), + buildExprAst("NULL is NULL") + ); + + assertEquals( + function("is not null", nullLiteral()), + buildExprAst("NULL IS NOT null") + ); + } + + @Test + public void canBuildLikeExpression() { + assertEquals( + function("like", stringLiteral("str"), stringLiteral("st%")), + buildExprAst("'str' like 'st%'") + ); + + assertEquals( + function("not like", stringLiteral("str"), stringLiteral("st%")), + buildExprAst("'str' not like 'st%'") + ); + } + private Node buildExprAst(String expr) { OpenDistroSQLLexer lexer = new OpenDistroSQLLexer(new CaseInsensitiveCharStream(expr)); OpenDistroSQLParser parser = new OpenDistroSQLParser(new CommonTokenStream(lexer)); From a1c9d031f914f402583dde9dcda69e8dc91de58b Mon Sep 17 00:00:00 2001 From: penghuo Date: Thu, 30 Jul 2020 14:00:02 -0700 Subject: [PATCH 2/4] address comments --- docs/user/dql/expressions.rst | 6 +++--- docs/user/index.rst | 1 - 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index 6c6c821585..7c1af0853d 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -133,7 +133,7 @@ Operators Basic Comparison Operator ------------------------- -Here is an example for different type of comparison operators:::: +Here is an example for different type of comparison operators:: od> SELECT 2 > 1, 2 >= 1, 2 < 1, 2 != 1, 2 <= 1, 2 = 1; fetched rows / total rows = 1/1 @@ -146,7 +146,7 @@ Here is an example for different type of comparison operators:::: LIKE ---- -expr LIKE pattern. The expr is string value, pattern is supports literal text, a percent ( % ) character for a wildcard, and an underscore ( _ ) character for a single character match:::: +expr LIKE pattern. The expr is string value, pattern is supports literal text, a percent ( % ) character for a wildcard, and an underscore ( _ ) character for a single character match:: od> SELECT 'axyzb' LIKE 'a%b', 'acb' LIKE 'a_b', 'axyzb' NOT LIKE 'a%b', 'acb' NOT LIKE 'a_b'; fetched rows / total rows = 1/1 @@ -159,7 +159,7 @@ expr LIKE pattern. The expr is string value, pattern is supports literal text, a NULL value test --------------- -Here is an example for null value test:::: +Here is an example for null value test:: od> SELECT 0 IS NULL, 0 IS NOT NULL, NULL IS NULL, NULL IS NOT NULL; fetched rows / total rows = 1/1 diff --git a/docs/user/index.rst b/docs/user/index.rst index 7688522954..f2c6eb899a 100644 --- a/docs/user/index.rst +++ b/docs/user/index.rst @@ -20,7 +20,6 @@ Open Distro for Elasticsearch SQL enables you to extract insights out of Elastic * **Language Structure** - `Identifiers `_ - - `Data Types `_ - `Data Types `_ From 2f3615bb3e2f88fb1c4ec794035ff327c4cf43b9 Mon Sep 17 00:00:00 2001 From: penghuo Date: Thu, 30 Jul 2020 16:29:08 -0700 Subject: [PATCH 3/4] address comments --- .../sql/data/model/AbstractExprValue.java | 7 +- .../sql/data/model/ExprMissingValue.java | 14 +- .../sql/data/model/ExprNullValue.java | 19 +-- .../predicate/BinaryPredicateOperator.java | 32 ++++- .../sql/data/model/ExprMissingValueTest.java | 10 +- .../sql/data/model/ExprNullValueTest.java | 9 ++ .../sql/data/model/ExprValueCompareTest.java | 51 +++++--- .../BinaryPredicateOperatorTest.java | 121 +++++++++--------- 8 files changed, 148 insertions(+), 115 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java index 59cf245374..5774c314b4 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/AbstractExprValue.java @@ -28,10 +28,9 @@ public abstract class AbstractExprValue implements ExprValue { */ @Override public int compareTo(ExprValue other) { - if (this.isNull() || this.isMissing()) { - return this.compare(other); - } else if (other.isNull() || other.isMissing()) { - return -other.compareTo(this); + if (this.isNull() || this.isMissing() || other.isNull() || other.isMissing()) { + throw new IllegalStateException( + String.format("[BUG] Unreachable, Comparing with NULL or MISSING is undefined")); } if ((this.isNumber() && other.isNumber()) || this.type() == other.type()) { return compare(other); diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java index 71fe8d63c8..2d822c47f1 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValue.java @@ -22,15 +22,14 @@ /** * Expression Missing Value. - * Missing value only equal to missing value, and is smaller than any other value. */ public class ExprMissingValue extends AbstractExprValue { - private static final ExprValue instance = new ExprMissingValue(); + private static final ExprMissingValue instance = new ExprMissingValue(); private ExprMissingValue() { } - public static ExprValue of() { + public static ExprMissingValue of() { return instance; } @@ -49,18 +48,15 @@ public boolean isMissing() { return true; } - /** - * When MISSING value compare to other expression value. - * 1) MISSING is equal to MISSING. - * 2) MISSING is less than all other expression values. - */ @Override public int compare(ExprValue other) { - return other.isMissing() ? 0 : -1; + throw new IllegalStateException(String.format("[BUG] Unreachable, Comparing with MISSING is " + + "undefined")); } /** * Missing value is equal to Missing value. + * Notes, this function should only used for Java Object Compare. */ @Override public boolean equal(ExprValue other) { diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java index ad8046e0ad..2638e62537 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValue.java @@ -21,13 +21,9 @@ /** * Expression Null Value. - * Null value - *
  • equal to null value. - *
  • large than missing value. - *
  • less than any other value. */ public class ExprNullValue extends AbstractExprValue { - private static final ExprValue instance = new ExprNullValue(); + private static final ExprNullValue instance = new ExprNullValue(); private ExprNullValue() { } @@ -42,7 +38,7 @@ public String toString() { return "NULL"; } - public static ExprValue of() { + public static ExprNullValue of() { return instance; } @@ -61,23 +57,18 @@ public boolean isNull() { return true; } - /** - * When NULL value compare to other expression value. - * 1) NULL is equal to NULL. - * 2) NULL is large than MISSING. - * 3) NULL is less than all other expression values. - */ @Override public int compare(ExprValue other) { - return other.isNull() ? 0 : other.isMissing() ? 1 : -1; + throw new IllegalStateException( + String.format("[BUG] Unreachable, Comparing with NULL is undefined")); } /** * NULL value is equal to NULL value. + * Notes, this function should only used for Java Object Compare. */ @Override public boolean equal(ExprValue other) { return other.isNull(); } - } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java index 69dfb46679..a45699546a 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperator.java @@ -168,7 +168,8 @@ private static FunctionResolver xor() { private static FunctionResolver equal() { return FunctionDSL.define(BuiltinFunctionName.EQUAL.getName(), ExprCoreType.coreTypes().stream() - .map(type -> FunctionDSL.impl((v1, v2) -> ExprBooleanValue.of(v1.equals(v2)), + .map(type -> FunctionDSL.impl( + FunctionDSL.nullMissingHandling((v1, v2) -> ExprBooleanValue.of(v1.equals(v2))), BOOLEAN, type, type)) .collect( Collectors.toList())); @@ -178,7 +179,12 @@ private static FunctionResolver notEqual() { return FunctionDSL .define(BuiltinFunctionName.NOTEQUAL.getName(), ExprCoreType.coreTypes().stream() .map(type -> FunctionDSL - .impl((v1, v2) -> ExprBooleanValue.of(!v1.equals(v2)), BOOLEAN, type, type)) + .impl( + FunctionDSL + .nullMissingHandling((v1, v2) -> ExprBooleanValue.of(!v1.equals(v2))), + BOOLEAN, + type, + type)) .collect( Collectors.toList())); } @@ -187,7 +193,10 @@ private static FunctionResolver less() { return FunctionDSL .define(BuiltinFunctionName.LESS.getName(), ExprCoreType.coreTypes().stream() .map(type -> FunctionDSL - .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) < 0), BOOLEAN, type, type)) + .impl(FunctionDSL + .nullMissingHandling((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) < 0)), + BOOLEAN, + type, type)) .collect( Collectors.toList())); } @@ -196,7 +205,12 @@ private static FunctionResolver lte() { return FunctionDSL .define(BuiltinFunctionName.LTE.getName(), ExprCoreType.coreTypes().stream() .map(type -> FunctionDSL - .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) <= 0), BOOLEAN, type, type)) + .impl( + FunctionDSL + .nullMissingHandling( + (v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) <= 0)), + BOOLEAN, + type, type)) .collect( Collectors.toList())); } @@ -205,7 +219,9 @@ private static FunctionResolver greater() { return FunctionDSL .define(BuiltinFunctionName.GREATER.getName(), ExprCoreType.coreTypes().stream() .map(type -> FunctionDSL - .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) > 0), BOOLEAN, type, type)) + .impl(FunctionDSL + .nullMissingHandling((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) > 0)), + BOOLEAN, type, type)) .collect( Collectors.toList())); } @@ -214,7 +230,11 @@ private static FunctionResolver gte() { return FunctionDSL .define(BuiltinFunctionName.GTE.getName(), ExprCoreType.coreTypes().stream() .map(type -> FunctionDSL - .impl((v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) >= 0), BOOLEAN, type, type)) + .impl( + FunctionDSL.nullMissingHandling( + (v1, v2) -> ExprBooleanValue.of(v1.compareTo(v2) >= 0)), + BOOLEAN, + type, type)) .collect( Collectors.toList())); } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java index 4d671e1d6b..337eef847b 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprMissingValueTest.java @@ -15,10 +15,11 @@ package com.amazon.opendistroforelasticsearch.sql.data.model; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static com.amazon.opendistroforelasticsearch.sql.utils.ComparisonUtil.compare; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -51,6 +52,13 @@ public void toStringTest() { assertEquals("MISSING", LITERAL_MISSING.toString()); } + @Test + public void equal() { + assertTrue(LITERAL_MISSING.equals(LITERAL_MISSING)); + assertFalse(LITERAL_FALSE.equals(LITERAL_MISSING)); + assertFalse(LITERAL_MISSING.equals(LITERAL_FALSE)); + } + @Test public void comparabilityTest() { ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class, diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java index facfc7d851..ca4c85cb6e 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprNullValueTest.java @@ -15,9 +15,11 @@ package com.amazon.opendistroforelasticsearch.sql.data.model; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static com.amazon.opendistroforelasticsearch.sql.utils.ComparisonUtil.compare; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -48,6 +50,13 @@ public void toStringTest() { assertEquals("NULL", LITERAL_NULL.toString()); } + @Test + public void equal() { + assertTrue(LITERAL_NULL.equals(LITERAL_NULL)); + assertFalse(LITERAL_FALSE.equals(LITERAL_NULL)); + assertFalse(LITERAL_NULL.equals(LITERAL_FALSE)); + } + @Test public void comparabilityTest() { ExpressionEvaluationException exception = assertThrows(ExpressionEvaluationException.class, diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValueCompareTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValueCompareTest.java index e5d6d877b3..1ef39a79bd 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValueCompareTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/data/model/ExprValueCompareTest.java @@ -17,7 +17,11 @@ package com.amazon.opendistroforelasticsearch.sql.data.model; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_FALSE; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import org.junit.jupiter.api.Test; @@ -51,33 +55,38 @@ public void timestampValueCompare() { } @Test - public void nullValueEqualToNullValue() { - assertEquals(0, ExprNullValue.of().compareTo(ExprNullValue.of())); - } + public void missingCompareToMethodShouldNotBeenCalledDirectly() { + IllegalStateException exception = assertThrows(IllegalStateException.class, + () -> LITERAL_MISSING.compareTo(LITERAL_FALSE)); + assertEquals("[BUG] Unreachable, Comparing with NULL or MISSING is undefined", + exception.getMessage()); - @Test - public void nullValueLessThanNotNullValue() { - assertEquals(-1, ExprNullValue.of().compareTo(ExprBooleanValue.of(true))); - assertEquals(1, ExprBooleanValue.of(true).compareTo(ExprNullValue.of())); - } + exception = assertThrows(IllegalStateException.class, + () -> LITERAL_FALSE.compareTo(LITERAL_MISSING)); + assertEquals("[BUG] Unreachable, Comparing with NULL or MISSING is undefined", + exception.getMessage()); - @Test - public void missingValueEqualToMissingValue() { - assertEquals(0, ExprMissingValue.of().compareTo(ExprMissingValue.of())); + exception = assertThrows(IllegalStateException.class, + () -> ExprMissingValue.of().compare(LITERAL_MISSING)); + assertEquals("[BUG] Unreachable, Comparing with MISSING is undefined", + exception.getMessage()); } @Test - public void missingValueLessThanNotMissingValue() { - assertEquals(-1, ExprMissingValue.of().compareTo(ExprBooleanValue.of(true))); - assertEquals(1, ExprBooleanValue.of(true).compareTo(ExprMissingValue.of())); + public void nullCompareToMethodShouldNotBeenCalledDirectly() { + IllegalStateException exception = assertThrows(IllegalStateException.class, + () -> LITERAL_NULL.compareTo(LITERAL_FALSE)); + assertEquals("[BUG] Unreachable, Comparing with NULL or MISSING is undefined", + exception.getMessage()); - assertEquals(-1, ExprMissingValue.of().compareTo(ExprNullValue.of())); - assertEquals(1, ExprNullValue.of().compareTo(ExprMissingValue.of())); - } + exception = assertThrows(IllegalStateException.class, + () -> LITERAL_FALSE.compareTo(LITERAL_NULL)); + assertEquals("[BUG] Unreachable, Comparing with NULL or MISSING is undefined", + exception.getMessage()); - @Test - public void missingValueLessThanNullValue() { - assertEquals(-1, ExprMissingValue.of().compareTo(ExprNullValue.of())); - assertEquals(1, ExprNullValue.of().compareTo(ExprMissingValue.of())); + exception = assertThrows(IllegalStateException.class, + () -> ExprNullValue.of().compare(LITERAL_MISSING)); + assertEquals("[BUG] Unreachable, Comparing with NULL is undefined", + exception.getMessage()); } } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index d3f0ede7cf..84b756643a 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -30,6 +30,7 @@ import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; + import static com.amazon.opendistroforelasticsearch.sql.utils.ComparisonUtil.compare; import static com.amazon.opendistroforelasticsearch.sql.utils.OperatorUtils.matches; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -356,38 +357,38 @@ public void test_null_equal_missing() { FunctionExpression equal = dsl.equal(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_TRUE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, equal.valueOf(valueEnv())); equal = dsl.equal(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_TRUE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, equal.valueOf(valueEnv())); equal = dsl.equal(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_FALSE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, equal.valueOf(valueEnv())); equal = dsl.equal(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_FALSE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, equal.valueOf(valueEnv())); equal = dsl.equal(DSL.literal(LITERAL_TRUE), DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_FALSE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, equal.valueOf(valueEnv())); equal = dsl.equal(DSL.literal(LITERAL_TRUE), DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_FALSE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, equal.valueOf(valueEnv())); equal = dsl.equal(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN), DSL.literal(LITERAL_TRUE)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_FALSE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, equal.valueOf(valueEnv())); equal = dsl.equal(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN), DSL.literal(LITERAL_TRUE)); assertEquals(BOOLEAN, equal.type()); - assertEquals(LITERAL_FALSE, equal.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, equal.valueOf(valueEnv())); } @ParameterizedTest(name = "equal({0}, {1})") @@ -405,42 +406,42 @@ public void test_null_notequal_missing() { FunctionExpression notequal = dsl.notequal(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_FALSE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, notequal.valueOf(valueEnv())); notequal = dsl.notequal(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_FALSE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, notequal.valueOf(valueEnv())); notequal = dsl.notequal(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_TRUE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, notequal.valueOf(valueEnv())); notequal = dsl.notequal(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN), DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_TRUE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, notequal.valueOf(valueEnv())); notequal = dsl.notequal(DSL.literal(LITERAL_TRUE), DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_TRUE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, notequal.valueOf(valueEnv())); notequal = dsl.notequal(DSL.literal(LITERAL_TRUE), DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_TRUE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, notequal.valueOf(valueEnv())); notequal = dsl.notequal(DSL.ref(BOOL_TYPE_MISSING_VALUE_FIELD, BOOLEAN), DSL.literal(LITERAL_TRUE)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_TRUE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, notequal.valueOf(valueEnv())); notequal = dsl.notequal(DSL.ref(BOOL_TYPE_NULL_VALUE_FIELD, BOOLEAN), DSL.literal(LITERAL_TRUE)); assertEquals(BOOLEAN, notequal.type()); - assertEquals(LITERAL_TRUE, notequal.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, notequal.valueOf(valueEnv())); } @ParameterizedTest(name = "less({0}, {1})") @@ -458,16 +459,16 @@ public void test_less_null() { FunctionExpression less = dsl.less(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_TRUE, less.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, less.valueOf(valueEnv())); } @Test @@ -475,16 +476,16 @@ public void test_less_missing() { FunctionExpression less = dsl.less(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_TRUE, less.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); } @Test @@ -492,12 +493,12 @@ public void test_null_less_missing() { FunctionExpression less = dsl.less(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_FALSE, less.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); less = dsl.less(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, less.type()); - assertEquals(LITERAL_TRUE, less.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, less.valueOf(valueEnv())); } @ParameterizedTest(name = "lte({0}, {1})") @@ -515,16 +516,16 @@ public void test_lte_null() { FunctionExpression lte = dsl.lte(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_FALSE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, lte.valueOf(valueEnv())); } @Test @@ -532,16 +533,16 @@ public void test_lte_missing() { FunctionExpression lte = dsl.lte(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_FALSE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); } @Test @@ -549,12 +550,12 @@ public void test_null_lte_missing() { FunctionExpression lte = dsl.lte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_FALSE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); lte = dsl.lte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, lte.type()); - assertEquals(LITERAL_TRUE, lte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, lte.valueOf(valueEnv())); } @ParameterizedTest(name = "greater({0}, {1})") @@ -572,16 +573,16 @@ public void test_greater_null() { FunctionExpression greater = dsl.greater(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_TRUE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, greater.valueOf(valueEnv())); } @Test @@ -589,16 +590,16 @@ public void test_greater_missing() { FunctionExpression greater = dsl.greater(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_TRUE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); } @Test @@ -606,12 +607,12 @@ public void test_null_greater_missing() { FunctionExpression greater = dsl.greater(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_TRUE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); greater = dsl.greater(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, greater.type()); - assertEquals(LITERAL_FALSE, greater.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, greater.valueOf(valueEnv())); } @ParameterizedTest(name = "gte({0}, {1})") @@ -629,16 +630,16 @@ public void test_gte_null() { FunctionExpression gte = dsl.gte(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_FALSE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_NULL, gte.valueOf(valueEnv())); } @Test @@ -646,16 +647,16 @@ public void test_gte_missing() { FunctionExpression gte = dsl.gte(DSL.literal(1), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.literal(1)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_FALSE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); } @Test @@ -663,12 +664,12 @@ public void test_null_gte_missing() { FunctionExpression gte = dsl.gte(DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_TRUE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); gte = dsl.gte(DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); assertEquals(BOOLEAN, gte.type()); - assertEquals(LITERAL_FALSE, gte.valueOf(valueEnv())); + assertEquals(LITERAL_MISSING, gte.valueOf(valueEnv())); } @ParameterizedTest(name = "like({0}, {1})") @@ -680,18 +681,6 @@ public void test_like(ExprValue v1, ExprValue v2) { assertEquals(String.format("%s like %s", v1.toString(), v2.toString()), like.toString()); } - @Test - public void test_not_like() { - FunctionExpression notLike = dsl.notLike(DSL.literal("bob"), DSL.literal("tom")); - assertEquals(BOOLEAN, notLike.type()); - assertTrue(notLike.valueOf(valueEnv()).booleanValue()); - assertEquals(String.format("\"%s\" not like \"%s\"", "bob", "tom"), notLike.toString()); - - notLike = dsl.notLike(DSL.literal("bob"), DSL.literal("bo%")); - assertFalse(notLike.valueOf(valueEnv()).booleanValue()); - assertEquals(String.format("\"%s\" not like \"%s\"", "bob", "bo%"), notLike.toString()); - } - @Test public void test_like_null() { FunctionExpression like = @@ -739,6 +728,18 @@ public void test_null_like_missing() { assertEquals(LITERAL_MISSING, like.valueOf(valueEnv())); } + @Test + public void test_not_like() { + FunctionExpression notLike = dsl.notLike(DSL.literal("bob"), DSL.literal("tom")); + assertEquals(BOOLEAN, notLike.type()); + assertTrue(notLike.valueOf(valueEnv()).booleanValue()); + assertEquals(String.format("\"%s\" not like \"%s\"", "bob", "tom"), notLike.toString()); + + notLike = dsl.notLike(DSL.literal("bob"), DSL.literal("bo%")); + assertFalse(notLike.valueOf(valueEnv()).booleanValue()); + assertEquals(String.format("\"%s\" not like \"%s\"", "bob", "bo%"), notLike.toString()); + } + /** * Todo. remove this test cases after script serilization implemented. */ From f17208387be564868c998e1f48010024de29ca46 Mon Sep 17 00:00:00 2001 From: penghuo Date: Thu, 30 Jul 2020 17:48:53 -0700 Subject: [PATCH 4/4] update --- .../sql/planner/physical/FilterOperator.java | 3 +- .../planner/physical/FilterOperatorTest.java | 38 +++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperator.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperator.java index be6d48be39..7dec9266b7 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperator.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperator.java @@ -1,7 +1,6 @@ package com.amazon.opendistroforelasticsearch.sql.planner.physical; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; -import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.operator.predicate.BinaryPredicateOperator; import com.amazon.opendistroforelasticsearch.sql.storage.bindingtuple.BindingTuple; @@ -42,7 +41,7 @@ public boolean hasNext() { while (input.hasNext()) { ExprValue inputValue = input.next(); ExprValue exprValue = conditions.valueOf(inputValue.bindingTuples()); - if (ExprValueUtils.getBooleanValue(exprValue)) { + if (!(exprValue.isNull() || exprValue.isMissing()) && (exprValue.booleanValue())) { next = inputValue; return true; } diff --git a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperatorTest.java b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperatorTest.java index b95e039917..b831d3ba53 100644 --- a/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperatorTest.java +++ b/core/src/test/java/com/amazon/opendistroforelasticsearch/sql/planner/physical/FilterOperatorTest.java @@ -15,19 +15,31 @@ package com.amazon.opendistroforelasticsearch.sql.planner.physical; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_MISSING; +import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_NULL; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.when; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprTupleValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.google.common.collect.ImmutableMap; +import java.util.LinkedHashMap; import java.util.List; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +@ExtendWith(MockitoExtension.class) class FilterOperatorTest extends PhysicalPlanTestBase { + @Mock + private PhysicalPlan inputPlan; @Test public void filterTest() { @@ -40,4 +52,30 @@ public void filterTest() { .of("ip", "209.160.24.63", "action", "GET", "response", 404, "referer", "www.amazon.com")))); } + + @Test + public void nullValueShouldBeenIgnored() { + LinkedHashMap value = new LinkedHashMap<>(); + value.put("response", LITERAL_NULL); + when(inputPlan.hasNext()).thenReturn(true, false); + when(inputPlan.next()).thenReturn(new ExprTupleValue(value)); + + FilterOperator plan = new FilterOperator(inputPlan, + dsl.equal(DSL.ref("response", INTEGER), DSL.literal(404))); + List result = execute(plan); + assertEquals(0, result.size()); + } + + @Test + public void missingValueShouldBeenIgnored() { + LinkedHashMap value = new LinkedHashMap<>(); + value.put("response", LITERAL_MISSING); + when(inputPlan.hasNext()).thenReturn(true, false); + when(inputPlan.next()).thenReturn(new ExprTupleValue(value)); + + FilterOperator plan = new FilterOperator(inputPlan, + dsl.equal(DSL.ref("response", INTEGER), DSL.literal(404))); + List result = execute(plan); + assertEquals(0, result.size()); + } } \ No newline at end of file