From f65ccc8b3654aaec9a6c3f7b4ca3a2aa19ace0c9 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Fri, 5 Mar 2021 16:06:22 -0800 Subject: [PATCH 1/6] supported between operator --- .../sql/analysis/ExpressionAnalyzer.java | 9 +++ .../sql/ast/AbstractNodeVisitor.java | 5 ++ .../sql/ast/dsl/AstDSL.java | 6 ++ .../sql/ast/expression/Between.java | 45 +++++++++++ .../sql/expression/DSL.java | 4 + .../function/BuiltinFunctionName.java | 3 +- .../predicate/BinaryPredicateOperator.java | 22 +++++ .../sql/utils/OperatorUtils.java | 24 ++++++ .../sql/analysis/ExpressionAnalyzerTest.java | 11 +++ .../BinaryPredicateOperatorTest.java | 80 ++++++++++++++++++- .../correctness/expressions/predicates.txt | 9 ++- sql/src/main/antlr/OpenDistroSQLParser.g4 | 1 + .../sql/sql/parser/AstExpressionBuilder.java | 7 ++ .../sql/parser/AstExpressionBuilderTest.java | 9 +++ 14 files changed, 230 insertions(+), 5 deletions(-) create mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java index 6d123b45a8..bd5690f27c 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java @@ -21,6 +21,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.AggregateFunction; import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields; import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Compare; @@ -143,6 +144,14 @@ public Expression visitNot(Not node, AnalysisContext context) { return dsl.not(node.getExpression().accept(this, context)); } + @Override + public Expression visitBetween(Between node, AnalysisContext context) { + Expression expr = node.getExpr().accept(this, context); + Expression min = node.getMin().accept(this, context); + Expression max = node.getMax().accept(this, context); + return dsl.between(expr, min, max); + } + @Override public Expression visitAggregateFunction(AggregateFunction node, AnalysisContext context) { Optional builtinFunctionName = BuiltinFunctionName.of(node.getFuncName()); diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java index 896e70cfa2..918f53c284 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java @@ -21,6 +21,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.AttributeList; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Compare; @@ -143,6 +144,10 @@ public T visitXor(Xor node, C context) { return visitChildren(node, context); } + public T visitBetween(Between node, C context) { + return visitChildren(node, context); + } + public T visitAggregateFunction(AggregateFunction node, C context) { return visitChildren(node, context); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java index a1515a7479..fa2502186c 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java @@ -20,6 +20,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields; import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Compare; @@ -263,6 +264,11 @@ public static UnresolvedExpression in( return new In(field, Arrays.asList(valueList)); } + public static UnresolvedExpression between( + UnresolvedExpression value, UnresolvedExpression min, UnresolvedExpression max) { + return new Between(value, min, max); + } + public static UnresolvedExpression compare( String operator, UnresolvedExpression left, UnresolvedExpression right) { return new Compare(operator, left, right); diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java new file mode 100644 index 0000000000..8775751115 --- /dev/null +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java @@ -0,0 +1,45 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + * + */ + +package com.amazon.opendistroforelasticsearch.sql.ast.expression; + +import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor; +import com.google.common.collect.ImmutableList; +import java.util.List; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.RequiredArgsConstructor; +import lombok.ToString; + +@RequiredArgsConstructor +@Getter +@EqualsAndHashCode(callSuper = false) +@ToString +public class Between extends UnresolvedExpression { + private final UnresolvedExpression expr; + private final UnresolvedExpression min; + private final UnresolvedExpression max; + + @Override + public List getChild() { + return ImmutableList.of(expr, min, max); + } + + @Override + public R accept(AbstractNodeVisitor nodeVisitor, C context) { + return nodeVisitor.visitBetween(this, context); + } +} 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 0d03ddc536..57d607ea3c 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 @@ -461,6 +461,10 @@ public FunctionExpression notLike(Expression... expressions) { return function(BuiltinFunctionName.NOT_LIKE, expressions); } + public FunctionExpression between(Expression... expressions) { + return function(BuiltinFunctionName.BETWEEN, expressions); + } + public Aggregator avg(Expression... expressions) { return aggregate(BuiltinFunctionName.AVG, 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 6b29c68da1..a7252c4b69 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 @@ -92,7 +92,7 @@ public enum BuiltinFunctionName { MODULES(FunctionName.of("%")), /** - * Boolean Operators. + * Comparison Operators. */ AND(FunctionName.of("and")), OR(FunctionName.of("or")), @@ -106,6 +106,7 @@ public enum BuiltinFunctionName { GTE(FunctionName.of(">=")), LIKE(FunctionName.of("like")), NOT_LIKE(FunctionName.of("not like")), + BETWEEN(FunctionName.of("between")), /** * Aggregation Function. 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 d08c3fab8f..2352e2d128 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,8 +20,13 @@ 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.DATE; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATETIME; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DOUBLE; 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.data.type.ExprCoreType.TIME; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIMESTAMP; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; @@ -63,6 +68,7 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(like()); repository.register(notLike()); repository.register(regexp()); + repository.register(between()); } /** @@ -262,6 +268,22 @@ private static FunctionResolver notLike() { STRING)); } + private static FunctionResolver between() { + return FunctionDSL.define(BuiltinFunctionName.BETWEEN.getName(), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), + INTEGER, DOUBLE, DOUBLE, DOUBLE), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), + INTEGER, STRING, STRING, STRING), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), + INTEGER, DATE, DATE, DATE), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), + INTEGER, DATETIME, DATETIME, DATETIME), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), + INTEGER, TIME, TIME, TIME), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), + INTEGER, TIMESTAMP, TIMESTAMP, TIMESTAMP)); + } + private static ExprValue lookupTableFunction(ExprValue arg1, ExprValue arg2, Table table) { if (table.contains(arg1, arg2)) { 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 d887d5c391..4f921957dc 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,9 +15,12 @@ package com.amazon.opendistroforelasticsearch.sql.utils; +import com.amazon.opendistroforelasticsearch.sql.data.model.AbstractExprNumberValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprIntegerValue; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprStringValue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; +import java.sql.Timestamp; import java.util.regex.Pattern; import lombok.experimental.UtilityClass; @@ -99,4 +102,25 @@ private static String patternToRegex(String patternString) { regex.append('$'); return regex.toString(); } + + /** + * Between operator util to judge if min <= expr <= max. + */ + public static ExprIntegerValue between(ExprValue expr, ExprValue min, ExprValue max) { + boolean isBetween; + if (expr instanceof AbstractExprNumberValue) { + isBetween = ((AbstractExprNumberValue) expr).compare(min) >= 0 + && ((AbstractExprNumberValue) expr).compare(max) <= 0; + } else if (expr instanceof ExprStringValue) { + isBetween = ((ExprStringValue) expr).compare(min) >= 0 + && ((ExprStringValue) expr).compare(max) <= 0; + } else { + isBetween = expr.compareTo(min) >= 0 && expr.compareTo(max) <= 0; + } + if (isBetween) { + return new ExprIntegerValue(1); + } else { + return new ExprIntegerValue(0); + } + } } 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 7e6fa063ea..be9303db24 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 @@ -22,6 +22,7 @@ import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.LITERAL_TRUE; import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.integerValue; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; +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.STRUCT; import static java.util.Collections.emptyList; @@ -92,6 +93,16 @@ public void not() { ); } + @Test + public void between() { + assertAnalyzeEqual( + dsl.between(DSL.ref("integer_value", INTEGER), DSL.ref("float_value", FLOAT), + DSL.literal(1)), + AstDSL.between(AstDSL.unresolvedAttr("integer_value"), AstDSL.unresolvedAttr("float_value"), + AstDSL.intLiteral(1)) + ); + } + @Test public void qualified_name() { assertAnalyzeEqual( 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 aa7402142c..0fc5cecad2 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 @@ -27,14 +27,18 @@ 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.model.ExprValueUtils.fromObjectValue; -import static com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils.missingValue; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.BOOLEAN; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATE; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.DATETIME; 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.data.type.ExprCoreType.TIME; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.TIMESTAMP; 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.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprBooleanValue; @@ -49,14 +53,15 @@ 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.exception.ExpressionEvaluationException; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.Expression; import com.amazon.opendistroforelasticsearch.sql.expression.ExpressionTestBase; import com.amazon.opendistroforelasticsearch.sql.expression.FunctionExpression; +import com.amazon.opendistroforelasticsearch.sql.utils.OperatorUtils; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; -import com.sun.org.apache.xpath.internal.Arg; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.ObjectInputStream; @@ -72,7 +77,6 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; -import org.mockito.Mock; class BinaryPredicateOperatorTest extends ExpressionTestBase { @@ -166,6 +170,45 @@ private static Stream testLikeArguments() { return builder.build(); } + private static Stream testBetweenArguments() { + List arguments = Arrays.asList( + Arrays.asList(1, 0, 2), Arrays.asList(1, 2, 0), + Arrays.asList(1L, 1L, 2L), Arrays.asList(2L, 1L, 2L), + Arrays.asList(3F, 1F, 2F), Arrays.asList(0F, 1F, 2F), + Arrays.asList(1D, 1D, 1D), Arrays.asList(1D, 2D, 2D), + Arrays.asList("b", "a", "c"), Arrays.asList("b", "c", "a"), + Arrays.asList("a", "a", "b"), Arrays.asList("b", "a", "b"), + Arrays.asList("c", "a", "b"), Arrays.asList("a", "b", "c"), + Arrays.asList("a", "a", "a"), Arrays.asList("b", "a", "a")); + Stream.Builder builder = Stream.builder(); + for (List argGroup: arguments) { + builder.add(Arguments.of(fromObjectValue(argGroup.get(0)), fromObjectValue(argGroup.get(1)), + fromObjectValue(argGroup.get(2)))); + } + builder + .add(Arguments.of(fromObjectValue("2021-01-02", DATE), + fromObjectValue("2021-01-01", DATE), fromObjectValue("2021-01-03", DATE))) + .add(Arguments.of(fromObjectValue("2021-01-02", DATE), + fromObjectValue("2021-01-03", DATE), fromObjectValue("2021-01-01", DATE))) + .add(Arguments.of(fromObjectValue("01:00:00", TIME), + fromObjectValue("01:00:00", TIME), fromObjectValue("02:00:00", TIME))) + .add(Arguments.of(fromObjectValue("02:00:00", TIME), + fromObjectValue("01:00:00", TIME), fromObjectValue("02:00:00", TIME))) + .add(Arguments.of(fromObjectValue("2021-01-01 03:00:00", DATETIME), + fromObjectValue("2021-01-01 01:00:00", DATETIME), + fromObjectValue("2021-01-01 02:00:00", DATETIME))) + .add(Arguments.of(fromObjectValue("2021-01-01 00:00:00", DATETIME), + fromObjectValue("2021-01-01 01:00:00", DATETIME), + fromObjectValue("2021-01-01 02:00:00", DATETIME))) + .add(Arguments.of(fromObjectValue("2021-01-01 01:00:00", TIMESTAMP), + fromObjectValue("2021-01-01 01:00:00", TIMESTAMP), + fromObjectValue("2021-01-01 01:00:00", TIMESTAMP))) + .add(Arguments.of(fromObjectValue("2021-01-01 00:00:00", TIMESTAMP), + fromObjectValue("2021-01-01 01:00:00", TIMESTAMP), + fromObjectValue("2021-01-01 01:00:00", TIMESTAMP))); + return builder.build(); + } + @ParameterizedTest(name = "and({0}, {1})") @MethodSource("binaryPredicateArguments") public void test_and(Boolean v1, Boolean v2) { @@ -832,4 +875,35 @@ public void compare_int_long() { FunctionExpression equal = dsl.equal(DSL.literal(1), DSL.literal(1L)); assertTrue(equal.valueOf(valueEnv()).booleanValue()); } + + @ParameterizedTest(name = "and({0}, {1})") + @MethodSource("testBetweenArguments") + public void between(ExprValue value, ExprValue minValue, ExprValue maxValue) { + FunctionExpression between = dsl.between( + DSL.literal(value), DSL.literal(minValue), DSL.literal(maxValue)); + assertEquals(INTEGER, between.type()); + assertEquals(OperatorUtils.between(value, minValue, maxValue), between.valueOf(valueEnv())); + } + + @Test + public void between_different_types() { + FunctionExpression between = dsl.between( + DSL.literal(1), DSL.literal(1), DSL.literal("1")); + assertThrows(ExpressionEvaluationException.class, () -> between.valueOf(valueEnv())); + } + + @Test + public void between_null_missing() { + FunctionExpression between = dsl.between( + DSL.literal(1), DSL.literal(0), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER)); + assertTrue(between.valueOf(valueEnv()).isNull()); + + between = dsl.between( + DSL.literal(1), DSL.literal(0), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); + assertTrue(between.valueOf(valueEnv()).isMissing()); + + between = dsl.between(DSL.literal(1), + DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); + assertTrue(between.valueOf(valueEnv()).isMissing()); + } } \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/expressions/predicates.txt b/integ-test/src/test/resources/correctness/expressions/predicates.txt index 9bc00c0be9..63efbd379a 100644 --- a/integ-test/src/test/resources/correctness/expressions/predicates.txt +++ b/integ-test/src/test/resources/correctness/expressions/predicates.txt @@ -4,4 +4,11 @@ false OR false AS bool true or false AS bool NOT true AS bool NOT false AS bool -NOT (true AND false) AS bool \ No newline at end of file +NOT (true AND false) AS bool +1 BETWEEN 0 AND 2 +1 BETWEEN 2 AND 0 +1 BETWEEN 1 AND 1 +1.1 BETWEEN 1.0 AND 1.2 +'1' BETWEEN '0' AND '2' +DATE('2021-01-02') BETWEEN DATE('2021-01-01') AND DATE('2021-01-03') +TIME('00:00:00') BETWEEN TIME('00:00:00') AND TIME('00:00:01') \ No newline at end of file diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index 4f01c657c9..38c8365a9e 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -260,6 +260,7 @@ predicate | predicate IS nullNotnull #isNullPredicate | left=predicate NOT? LIKE right=predicate #likePredicate | left=predicate REGEXP right=predicate #regexpPredicate + | expr=predicate BETWEEN min=predicate AND max=predicate #betweenPredicate ; expressionAtom 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 84e58d9535..04a65e5346 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 @@ -18,6 +18,7 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.qualifiedName; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.stringLiteral; +import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.BETWEEN; 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; @@ -58,6 +59,7 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.AggregateFunction; import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields; import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; +import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function; @@ -233,6 +235,11 @@ public UnresolvedExpression visitRegexpPredicate(RegexpPredicateContext ctx) { Arrays.asList(visit(ctx.left), visit(ctx.right))); } + @Override + public UnresolvedExpression visitBetweenPredicate(OpenDistroSQLParser.BetweenPredicateContext ctx) { + return new Between(visit(ctx.expr), visit(ctx.min), visit(ctx.max)); + } + @Override public UnresolvedExpression visitAndExpression(AndExpressionContext ctx) { return new And(visit(ctx.left), visit(ctx.right)); 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 8ff5c50ce6..d94f131a1e 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 @@ -18,6 +18,7 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.aggregate; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.and; +import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.between; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.booleanLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.caseWhen; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.dateLiteral; @@ -240,6 +241,14 @@ public void canBuildRegexpExpression() { ); } + @Test + public void canBuildBetweenPredicate() { + assertEquals( + between(intLiteral(1), intLiteral(0), intLiteral(2)), + buildExprAst("1 between 0 and 2") + ); + } + @Test public void canBuildLogicalExpression() { assertEquals( From 5a8c6b5879d8b395faaa4f91621a15c7b119567d Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Fri, 5 Mar 2021 17:03:38 -0800 Subject: [PATCH 2/6] supported no between operator --- .../sql/analysis/ExpressionAnalyzer.java | 10 ----- .../sql/ast/AbstractNodeVisitor.java | 5 --- .../sql/ast/dsl/AstDSL.java | 6 --- .../sql/ast/expression/Between.java | 45 ------------------- .../sql/expression/DSL.java | 4 ++ .../function/BuiltinFunctionName.java | 1 + .../predicate/BinaryPredicateOperator.java | 17 +++++++ .../sql/utils/OperatorUtils.java | 37 ++++++++++----- .../sql/analysis/ExpressionAnalyzerTest.java | 10 ----- .../BinaryPredicateOperatorTest.java | 22 ++++++--- .../correctness/expressions/predicates.txt | 4 +- sql/src/main/antlr/OpenDistroSQLParser.g4 | 2 +- .../sql/sql/parser/AstExpressionBuilder.java | 7 ++- 13 files changed, 74 insertions(+), 96 deletions(-) delete mode 100644 core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java index bd5690f27c..9f32331673 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/analysis/ExpressionAnalyzer.java @@ -21,7 +21,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.AggregateFunction; import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields; import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Compare; @@ -53,7 +52,6 @@ import com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionRepository; import com.amazon.opendistroforelasticsearch.sql.expression.function.FunctionName; import com.amazon.opendistroforelasticsearch.sql.expression.window.aggregation.AggregateWindowFunction; -import com.amazon.opendistroforelasticsearch.sql.expression.window.ranking.RankingWindowFunction; import com.google.common.collect.ImmutableSet; import java.util.ArrayList; import java.util.Arrays; @@ -144,14 +142,6 @@ public Expression visitNot(Not node, AnalysisContext context) { return dsl.not(node.getExpression().accept(this, context)); } - @Override - public Expression visitBetween(Between node, AnalysisContext context) { - Expression expr = node.getExpr().accept(this, context); - Expression min = node.getMin().accept(this, context); - Expression max = node.getMax().accept(this, context); - return dsl.between(expr, min, max); - } - @Override public Expression visitAggregateFunction(AggregateFunction node, AnalysisContext context) { Optional builtinFunctionName = BuiltinFunctionName.of(node.getFuncName()); diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java index 918f53c284..896e70cfa2 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/AbstractNodeVisitor.java @@ -21,7 +21,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; import com.amazon.opendistroforelasticsearch.sql.ast.expression.AttributeList; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Compare; @@ -144,10 +143,6 @@ public T visitXor(Xor node, C context) { return visitChildren(node, context); } - public T visitBetween(Between node, C context) { - return visitChildren(node, context); - } - public T visitAggregateFunction(AggregateFunction node, C context) { return visitChildren(node, context); } diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java index fa2502186c..a1515a7479 100644 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java +++ b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/dsl/AstDSL.java @@ -20,7 +20,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields; import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Argument; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Compare; @@ -264,11 +263,6 @@ public static UnresolvedExpression in( return new In(field, Arrays.asList(valueList)); } - public static UnresolvedExpression between( - UnresolvedExpression value, UnresolvedExpression min, UnresolvedExpression max) { - return new Between(value, min, max); - } - public static UnresolvedExpression compare( String operator, UnresolvedExpression left, UnresolvedExpression right) { return new Compare(operator, left, right); diff --git a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java b/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java deleted file mode 100644 index 8775751115..0000000000 --- a/core/src/main/java/com/amazon/opendistroforelasticsearch/sql/ast/expression/Between.java +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - * - */ - -package com.amazon.opendistroforelasticsearch.sql.ast.expression; - -import com.amazon.opendistroforelasticsearch.sql.ast.AbstractNodeVisitor; -import com.google.common.collect.ImmutableList; -import java.util.List; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.RequiredArgsConstructor; -import lombok.ToString; - -@RequiredArgsConstructor -@Getter -@EqualsAndHashCode(callSuper = false) -@ToString -public class Between extends UnresolvedExpression { - private final UnresolvedExpression expr; - private final UnresolvedExpression min; - private final UnresolvedExpression max; - - @Override - public List getChild() { - return ImmutableList.of(expr, min, max); - } - - @Override - public R accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitBetween(this, context); - } -} 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 57d607ea3c..b21bc8f913 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 @@ -465,6 +465,10 @@ public FunctionExpression between(Expression... expressions) { return function(BuiltinFunctionName.BETWEEN, expressions); } + public FunctionExpression not_between(Expression... expressions) { + return function(BuiltinFunctionName.NOT_BETWEEN, expressions); + } + public Aggregator avg(Expression... expressions) { return aggregate(BuiltinFunctionName.AVG, 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 a7252c4b69..88d2ade365 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 @@ -107,6 +107,7 @@ public enum BuiltinFunctionName { LIKE(FunctionName.of("like")), NOT_LIKE(FunctionName.of("not like")), BETWEEN(FunctionName.of("between")), + NOT_BETWEEN(FunctionName.of("not_between")), /** * Aggregation Function. 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 2352e2d128..dae3f23584 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 @@ -69,6 +69,7 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(notLike()); repository.register(regexp()); repository.register(between()); + repository.register(not_between()); } /** @@ -284,6 +285,22 @@ private static FunctionResolver between() { INTEGER, TIMESTAMP, TIMESTAMP, TIMESTAMP)); } + private static FunctionResolver not_between() { + return FunctionDSL.define(BuiltinFunctionName.NOT_BETWEEN.getName(), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), + INTEGER, DOUBLE, DOUBLE, DOUBLE), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), + INTEGER, STRING, STRING, STRING), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), + INTEGER, DATE, DATE, DATE), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), + INTEGER, DATETIME, DATETIME, DATETIME), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), + INTEGER, TIME, TIME, TIME), + FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), + INTEGER, TIMESTAMP, TIMESTAMP, TIMESTAMP)); + } + private static ExprValue lookupTableFunction(ExprValue arg1, ExprValue arg2, Table table) { if (table.contains(arg1, arg2)) { 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 4f921957dc..1ba3482cfa 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 @@ -104,23 +104,40 @@ private static String patternToRegex(String patternString) { } /** - * Between operator util to judge if min <= expr <= max. + * BETWEEN ... AND ... operator util. + * Expression { expr BETWEEN min AND max } is to judge if min <= expr <= max. */ public static ExprIntegerValue between(ExprValue expr, ExprValue min, ExprValue max) { - boolean isBetween; + if (isBetween(expr, min, max)) { + return new ExprIntegerValue(1); + } else { + return new ExprIntegerValue(0); + } + } + + /** + * NOT BETWEEN ... AND ... operator util. + * { expr NOT BETWEEN min AND max } is equivalent to { NOT (expr BETWEEN min AND max) }. + */ + public static ExprIntegerValue not_between(ExprValue expr, ExprValue min, ExprValue max) { + if (isBetween(expr, min, max)) { + return new ExprIntegerValue(0); + } else { + return new ExprIntegerValue(1); + } + } + + private static boolean isBetween(ExprValue expr, ExprValue min, ExprValue max) { if (expr instanceof AbstractExprNumberValue) { - isBetween = ((AbstractExprNumberValue) expr).compare(min) >= 0 + return ((AbstractExprNumberValue) expr).compare(min) >= 0 && ((AbstractExprNumberValue) expr).compare(max) <= 0; } else if (expr instanceof ExprStringValue) { - isBetween = ((ExprStringValue) expr).compare(min) >= 0 + return ((ExprStringValue) expr).compare(min) >= 0 && ((ExprStringValue) expr).compare(max) <= 0; } else { - isBetween = expr.compareTo(min) >= 0 && expr.compareTo(max) <= 0; - } - if (isBetween) { - return new ExprIntegerValue(1); - } else { - return new ExprIntegerValue(0); + return expr.compareTo(min) >= 0 && expr.compareTo(max) <= 0; } } } + + 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 be9303db24..f1739d3e1a 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 @@ -93,16 +93,6 @@ public void not() { ); } - @Test - public void between() { - assertAnalyzeEqual( - dsl.between(DSL.ref("integer_value", INTEGER), DSL.ref("float_value", FLOAT), - DSL.literal(1)), - AstDSL.between(AstDSL.unresolvedAttr("integer_value"), AstDSL.unresolvedAttr("float_value"), - AstDSL.intLiteral(1)) - ); - } - @Test public void qualified_name() { assertAnalyzeEqual( 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 0fc5cecad2..080208044f 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 @@ -876,7 +876,7 @@ public void compare_int_long() { assertTrue(equal.valueOf(valueEnv()).booleanValue()); } - @ParameterizedTest(name = "and({0}, {1})") + @ParameterizedTest(name = "between({0}, {1}, {2})") @MethodSource("testBetweenArguments") public void between(ExprValue value, ExprValue minValue, ExprValue maxValue) { FunctionExpression between = dsl.between( @@ -885,11 +885,21 @@ public void between(ExprValue value, ExprValue minValue, ExprValue maxValue) { assertEquals(OperatorUtils.between(value, minValue, maxValue), between.valueOf(valueEnv())); } + @ParameterizedTest(name = "not between({0}, {1}, {2})") + @MethodSource("testBetweenArguments") + public void not_between(ExprValue value, ExprValue minValue, ExprValue maxValue) { + FunctionExpression notBetween = dsl.not_between( + DSL.literal(value), DSL.literal(minValue), DSL.literal(maxValue)); + assertEquals(INTEGER, notBetween.type()); + assertEquals(new ExprIntegerValue( + Math.abs(1 - OperatorUtils.between(value, minValue, maxValue).integerValue())), + notBetween.valueOf(valueEnv())); + } + @Test public void between_different_types() { - FunctionExpression between = dsl.between( - DSL.literal(1), DSL.literal(1), DSL.literal("1")); - assertThrows(ExpressionEvaluationException.class, () -> between.valueOf(valueEnv())); + assertThrows(ExpressionEvaluationException.class, () -> + dsl.between(DSL.literal(1), DSL.literal(1), DSL.literal("1"))); } @Test @@ -902,8 +912,8 @@ public void between_null_missing() { DSL.literal(1), DSL.literal(0), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertTrue(between.valueOf(valueEnv()).isMissing()); - between = dsl.between(DSL.literal(1), - DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); + between = dsl.between(DSL.literal(1), DSL.ref(INT_TYPE_NULL_VALUE_FIELD, INTEGER), + DSL.ref(INT_TYPE_MISSING_VALUE_FIELD, INTEGER)); assertTrue(between.valueOf(valueEnv()).isMissing()); } } \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/expressions/predicates.txt b/integ-test/src/test/resources/correctness/expressions/predicates.txt index 63efbd379a..305695cea8 100644 --- a/integ-test/src/test/resources/correctness/expressions/predicates.txt +++ b/integ-test/src/test/resources/correctness/expressions/predicates.txt @@ -11,4 +11,6 @@ NOT (true AND false) AS bool 1.1 BETWEEN 1.0 AND 1.2 '1' BETWEEN '0' AND '2' DATE('2021-01-02') BETWEEN DATE('2021-01-01') AND DATE('2021-01-03') -TIME('00:00:00') BETWEEN TIME('00:00:00') AND TIME('00:00:01') \ No newline at end of file +TIME('00:00:00') BETWEEN TIME('00:00:00') AND TIME('00:00:01') +1 NOT BETWEEN 0 AND 2 +1 NOT BETWEEN 2 AND 0 \ No newline at end of file diff --git a/sql/src/main/antlr/OpenDistroSQLParser.g4 b/sql/src/main/antlr/OpenDistroSQLParser.g4 index 38c8365a9e..a169983078 100644 --- a/sql/src/main/antlr/OpenDistroSQLParser.g4 +++ b/sql/src/main/antlr/OpenDistroSQLParser.g4 @@ -260,7 +260,7 @@ predicate | predicate IS nullNotnull #isNullPredicate | left=predicate NOT? LIKE right=predicate #likePredicate | left=predicate REGEXP right=predicate #regexpPredicate - | expr=predicate BETWEEN min=predicate AND max=predicate #betweenPredicate + | expr=predicate NOT? BETWEEN min=predicate AND max=predicate #betweenPredicate ; expressionAtom 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 04a65e5346..abe3f739ae 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 @@ -22,6 +22,7 @@ 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_BETWEEN; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.NOT_LIKE; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.REGEXP; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BinaryComparisonPredicateContext; @@ -59,7 +60,6 @@ import com.amazon.opendistroforelasticsearch.sql.ast.expression.AggregateFunction; import com.amazon.opendistroforelasticsearch.sql.ast.expression.AllFields; import com.amazon.opendistroforelasticsearch.sql.ast.expression.And; -import com.amazon.opendistroforelasticsearch.sql.ast.expression.Between; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Case; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Cast; import com.amazon.opendistroforelasticsearch.sql.ast.expression.Function; @@ -237,7 +237,10 @@ public UnresolvedExpression visitRegexpPredicate(RegexpPredicateContext ctx) { @Override public UnresolvedExpression visitBetweenPredicate(OpenDistroSQLParser.BetweenPredicateContext ctx) { - return new Between(visit(ctx.expr), visit(ctx.min), visit(ctx.max)); + return new Function( + ctx.NOT() == null ? BETWEEN.getName().getFunctionName() : + NOT_BETWEEN.getName().getFunctionName(), + Arrays.asList(visit(ctx.expr), visit(ctx.min), visit(ctx.max))); } @Override From 7148fa902a51608a8b5e4b663f195df64c954a71 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Fri, 5 Mar 2021 17:27:47 -0800 Subject: [PATCH 3/6] Updated user manual --- docs/user/dql/expressions.rst | 17 +++++++++++++++++ .../sql/sql/parser/AstExpressionBuilder.java | 3 ++- 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index 861e233d1f..4bd2381eb8 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -130,6 +130,8 @@ Operators +----------------+----------------------------------------+ | REGEXP | String matches regular expression test | +----------------+----------------------------------------+ +| BETWEEN AND | In the range of two values | ++----------------+----------------------------------------+ Basic Comparison Operator @@ -185,6 +187,21 @@ expr REGEXP pattern. The expr is string value, pattern is supports regular expre | 1 | 0 | +------------------------+------------------+ + +BETWEEN AND +----------- + +expr BETWEEN min AND max. This operator is to judge if expr is in the range from min to max (min <= expr <= max), and returns 1 for true, 0 for false. expr NOT BETWEEN min AND max is the equivalent to NOT expr BETWEEN min AND max. The three expressions expr, min and max should be consistent in their types for value comparisons, or expression evaluation exception would be thrown. The supported types in this operator include number, string, and date and time related types. Implicit casting is not supported yet, so you would need to explicitly specifies the types of compared values. Here follow some examples:: + + od> SELECT 1 BETWEEN 0 AND 2 AS res1, '1' BETWEEN '2' AND '0' AS res2, date('2021-03-05') BETWEEN date('2021-03-05') AND date('2021-03-05') AS res3; + fetched rows / total rows = 1/1 + +--------+--------+--------+ + | res1 | res2 | res3 | + |--------+--------+--------| + | 1 | 0 | 1 | + +--------+--------+--------+ + + Function Call ============= 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 abe3f739ae..e9a8476d98 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 @@ -25,6 +25,7 @@ import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.NOT_BETWEEN; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.NOT_LIKE; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.REGEXP; +import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BetweenPredicateContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BinaryComparisonPredicateContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BooleanContext; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.CaseFuncAlternativeContext; @@ -236,7 +237,7 @@ public UnresolvedExpression visitRegexpPredicate(RegexpPredicateContext ctx) { } @Override - public UnresolvedExpression visitBetweenPredicate(OpenDistroSQLParser.BetweenPredicateContext ctx) { + public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { return new Function( ctx.NOT() == null ? BETWEEN.getName().getFunctionName() : NOT_BETWEEN.getName().getFunctionName(), From a22e7a95b3c5dd112564ca4371406f3b79b86085 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Fri, 5 Mar 2021 17:42:30 -0800 Subject: [PATCH 4/6] update --- .../sql/sql/parser/AstExpressionBuilderTest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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 d94f131a1e..9ec1b5b77f 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 @@ -18,7 +18,6 @@ import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.aggregate; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.and; -import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.between; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.booleanLiteral; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.caseWhen; import static com.amazon.opendistroforelasticsearch.sql.ast.dsl.AstDSL.dateLiteral; @@ -244,11 +243,19 @@ public void canBuildRegexpExpression() { @Test public void canBuildBetweenPredicate() { assertEquals( - between(intLiteral(1), intLiteral(0), intLiteral(2)), + function("between", intLiteral(1), intLiteral(0), intLiteral(2)), buildExprAst("1 between 0 and 2") ); } + @Test + public void canBuildNotBetweenPredicate() { + assertEquals( + function("not_between", intLiteral(1), intLiteral(0), intLiteral(2)), + buildExprAst("1 not between 0 and 2") + ); + } + @Test public void canBuildLogicalExpression() { assertEquals( From 703b2e2190bbc960190307732c19b6800dadad6b Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 29 Mar 2021 17:44:39 -0700 Subject: [PATCH 5/6] change return type to boolean --- .../predicate/BinaryPredicateOperator.java | 24 +++++++++---------- .../sql/utils/OperatorUtils.java | 16 ++++--------- .../BinaryPredicateOperatorTest.java | 9 ++++--- docs/user/dql/expressions.rst | 10 ++++---- 4 files changed, 25 insertions(+), 34 deletions(-) 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 dae3f23584..9312a46217 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 @@ -272,33 +272,33 @@ private static FunctionResolver notLike() { private static FunctionResolver between() { return FunctionDSL.define(BuiltinFunctionName.BETWEEN.getName(), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), - INTEGER, DOUBLE, DOUBLE, DOUBLE), + BOOLEAN, DOUBLE, DOUBLE, DOUBLE), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), - INTEGER, STRING, STRING, STRING), + BOOLEAN, STRING, STRING, STRING), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), - INTEGER, DATE, DATE, DATE), + BOOLEAN, DATE, DATE, DATE), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), - INTEGER, DATETIME, DATETIME, DATETIME), + BOOLEAN, DATETIME, DATETIME, DATETIME), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), - INTEGER, TIME, TIME, TIME), + BOOLEAN, TIME, TIME, TIME), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::between), - INTEGER, TIMESTAMP, TIMESTAMP, TIMESTAMP)); + BOOLEAN, TIMESTAMP, TIMESTAMP, TIMESTAMP)); } private static FunctionResolver not_between() { return FunctionDSL.define(BuiltinFunctionName.NOT_BETWEEN.getName(), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - INTEGER, DOUBLE, DOUBLE, DOUBLE), + BOOLEAN, DOUBLE, DOUBLE, DOUBLE), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - INTEGER, STRING, STRING, STRING), + BOOLEAN, STRING, STRING, STRING), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - INTEGER, DATE, DATE, DATE), + BOOLEAN, DATE, DATE, DATE), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - INTEGER, DATETIME, DATETIME, DATETIME), + BOOLEAN, DATETIME, DATETIME, DATETIME), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - INTEGER, TIME, TIME, TIME), + BOOLEAN, TIME, TIME, TIME), FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - INTEGER, TIMESTAMP, TIMESTAMP, TIMESTAMP)); + BOOLEAN, TIMESTAMP, TIMESTAMP, TIMESTAMP)); } private static ExprValue lookupTableFunction(ExprValue arg1, ExprValue arg2, 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 1ba3482cfa..759a27cf0a 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 @@ -107,24 +107,16 @@ private static String patternToRegex(String patternString) { * BETWEEN ... AND ... operator util. * Expression { expr BETWEEN min AND max } is to judge if min <= expr <= max. */ - public static ExprIntegerValue between(ExprValue expr, ExprValue min, ExprValue max) { - if (isBetween(expr, min, max)) { - return new ExprIntegerValue(1); - } else { - return new ExprIntegerValue(0); - } + public static ExprBooleanValue between(ExprValue expr, ExprValue min, ExprValue max) { + return ExprBooleanValue.of(isBetween(expr, min, max)); } /** * NOT BETWEEN ... AND ... operator util. * { expr NOT BETWEEN min AND max } is equivalent to { NOT (expr BETWEEN min AND max) }. */ - public static ExprIntegerValue not_between(ExprValue expr, ExprValue min, ExprValue max) { - if (isBetween(expr, min, max)) { - return new ExprIntegerValue(0); - } else { - return new ExprIntegerValue(1); - } + public static ExprBooleanValue not_between(ExprValue expr, ExprValue min, ExprValue max) { + return ExprBooleanValue.of(!isBetween(expr, min, max)); } private static boolean isBetween(ExprValue expr, ExprValue min, ExprValue max) { 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 080208044f..5af8dad985 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 @@ -881,7 +881,7 @@ public void compare_int_long() { public void between(ExprValue value, ExprValue minValue, ExprValue maxValue) { FunctionExpression between = dsl.between( DSL.literal(value), DSL.literal(minValue), DSL.literal(maxValue)); - assertEquals(INTEGER, between.type()); + assertEquals(BOOLEAN, between.type()); assertEquals(OperatorUtils.between(value, minValue, maxValue), between.valueOf(valueEnv())); } @@ -890,10 +890,9 @@ public void between(ExprValue value, ExprValue minValue, ExprValue maxValue) { public void not_between(ExprValue value, ExprValue minValue, ExprValue maxValue) { FunctionExpression notBetween = dsl.not_between( DSL.literal(value), DSL.literal(minValue), DSL.literal(maxValue)); - assertEquals(INTEGER, notBetween.type()); - assertEquals(new ExprIntegerValue( - Math.abs(1 - OperatorUtils.between(value, minValue, maxValue).integerValue())), - notBetween.valueOf(valueEnv())); + assertEquals(BOOLEAN, notBetween.type()); + assertEquals( + OperatorUtils.not_between(value, minValue, maxValue), notBetween.valueOf(valueEnv())); } @Test diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index 4bd2381eb8..baaeabaea7 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -195,11 +195,11 @@ expr BETWEEN min AND max. This operator is to judge if expr is in the range from od> SELECT 1 BETWEEN 0 AND 2 AS res1, '1' BETWEEN '2' AND '0' AS res2, date('2021-03-05') BETWEEN date('2021-03-05') AND date('2021-03-05') AS res3; fetched rows / total rows = 1/1 - +--------+--------+--------+ - | res1 | res2 | res3 | - |--------+--------+--------| - | 1 | 0 | 1 | - +--------+--------+--------+ + +-----------+------------+-----------+ + | res1 | res2 | res3 | + |-----------+------------+-----------| + | true | false | true | + +-----------+------------+-----------+ Function Call From 1545188878eeb60551a62f7ec5dcf285ec477ff0 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 29 Mar 2021 19:52:26 -0700 Subject: [PATCH 6/6] push down between operator --- .../sql/expression/DSL.java | 2 +- .../function/BuiltinFunctionName.java | 1 - .../predicate/BinaryPredicateOperator.java | 17 -------- .../sql/utils/OperatorUtils.java | 8 ---- .../BinaryPredicateOperatorTest.java | 4 +- docs/user/dql/expressions.rst | 10 ++--- .../script/filter/FilterQueryBuilder.java | 1 + .../script/filter/lucene/LuceneQuery.java | 19 ++++++-- .../script/filter/lucene/RangeQuery.java | 20 ++++++++- .../script/filter/FilterQueryBuilderTest.java | 43 ++++++++++++++++++- .../script/filter/lucene/LuceneQueryTest.java | 7 ++- .../script/filter/lucene/RangeQueryTest.java | 15 +++++-- .../correctness/expressions/predicates.txt | 11 +---- .../sql/sql/parser/AstExpressionBuilder.java | 8 ++-- .../sql/parser/AstExpressionBuilderTest.java | 2 +- 15 files changed, 110 insertions(+), 58 deletions(-) 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 b21bc8f913..107ffa2533 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 @@ -466,7 +466,7 @@ public FunctionExpression between(Expression... expressions) { } public FunctionExpression not_between(Expression... expressions) { - return function(BuiltinFunctionName.NOT_BETWEEN, expressions); + return not(between(expressions)); } public Aggregator avg(Expression... 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 88d2ade365..a7252c4b69 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 @@ -107,7 +107,6 @@ public enum BuiltinFunctionName { LIKE(FunctionName.of("like")), NOT_LIKE(FunctionName.of("not like")), BETWEEN(FunctionName.of("between")), - NOT_BETWEEN(FunctionName.of("not_between")), /** * Aggregation Function. 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 9312a46217..7fe90bc8c2 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 @@ -69,7 +69,6 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(notLike()); repository.register(regexp()); repository.register(between()); - repository.register(not_between()); } /** @@ -285,22 +284,6 @@ private static FunctionResolver between() { BOOLEAN, TIMESTAMP, TIMESTAMP, TIMESTAMP)); } - private static FunctionResolver not_between() { - return FunctionDSL.define(BuiltinFunctionName.NOT_BETWEEN.getName(), - FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - BOOLEAN, DOUBLE, DOUBLE, DOUBLE), - FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - BOOLEAN, STRING, STRING, STRING), - FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - BOOLEAN, DATE, DATE, DATE), - FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - BOOLEAN, DATETIME, DATETIME, DATETIME), - FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - BOOLEAN, TIME, TIME, TIME), - FunctionDSL.impl(FunctionDSL.nullMissingHandling(OperatorUtils::not_between), - BOOLEAN, TIMESTAMP, TIMESTAMP, TIMESTAMP)); - } - private static ExprValue lookupTableFunction(ExprValue arg1, ExprValue arg2, Table table) { if (table.contains(arg1, arg2)) { 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 759a27cf0a..ecc554442c 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 @@ -111,14 +111,6 @@ public static ExprBooleanValue between(ExprValue expr, ExprValue min, ExprValue return ExprBooleanValue.of(isBetween(expr, min, max)); } - /** - * NOT BETWEEN ... AND ... operator util. - * { expr NOT BETWEEN min AND max } is equivalent to { NOT (expr BETWEEN min AND max) }. - */ - public static ExprBooleanValue not_between(ExprValue expr, ExprValue min, ExprValue max) { - return ExprBooleanValue.of(!isBetween(expr, min, max)); - } - private static boolean isBetween(ExprValue expr, ExprValue min, ExprValue max) { if (expr instanceof AbstractExprNumberValue) { return ((AbstractExprNumberValue) expr).compare(min) >= 0 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 5af8dad985..71da6a59b1 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 @@ -891,8 +891,8 @@ public void not_between(ExprValue value, ExprValue minValue, ExprValue maxValue) FunctionExpression notBetween = dsl.not_between( DSL.literal(value), DSL.literal(minValue), DSL.literal(maxValue)); assertEquals(BOOLEAN, notBetween.type()); - assertEquals( - OperatorUtils.not_between(value, minValue, maxValue), notBetween.valueOf(valueEnv())); + assertEquals(!OperatorUtils.between(value, minValue, maxValue).booleanValue(), + notBetween.valueOf(valueEnv()).booleanValue()); } @Test diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index baaeabaea7..f58f160d67 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -195,11 +195,11 @@ expr BETWEEN min AND max. This operator is to judge if expr is in the range from od> SELECT 1 BETWEEN 0 AND 2 AS res1, '1' BETWEEN '2' AND '0' AS res2, date('2021-03-05') BETWEEN date('2021-03-05') AND date('2021-03-05') AS res3; fetched rows / total rows = 1/1 - +-----------+------------+-----------+ - | res1 | res2 | res3 | - |-----------+------------+-----------| - | true | false | true | - +-----------+------------+-----------+ + +--------+--------+--------+ + | res1 | res2 | res3 | + |--------+--------+--------| + | True | False | True | + +--------+--------+--------+ Function Call diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java index 4cc8be3512..b47f4fdda1 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilder.java @@ -62,6 +62,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor= 2) && (func.getArguments().get(0) instanceof ReferenceExpression) && (func.getArguments().get(1) instanceof LiteralExpression); } @@ -53,8 +55,14 @@ public boolean canSupport(FunctionExpression func) { */ public QueryBuilder build(FunctionExpression func) { ReferenceExpression ref = (ReferenceExpression) func.getArguments().get(0); - LiteralExpression literal = (LiteralExpression) func.getArguments().get(1); - return doBuild(ref.getAttr(), ref.type(), literal.valueOf(null)); + if (func.getArguments().size() > 2) { + List literalList = func.getArguments().stream().skip(1) + .map(v -> v.valueOf(null)).collect(Collectors.toList()); + return doBuild(ref.getAttr(), ref.type(), literalList); + } else { + LiteralExpression literal = (LiteralExpression) func.getArguments().get(1); + return doBuild(ref.getAttr(), ref.type(), literal.valueOf(null)); + } } /** @@ -71,6 +79,11 @@ protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue l "Subclass doesn't implement this and build method either"); } + protected QueryBuilder doBuild(String fieldName, ExprType fieldType, List literals) { + throw new UnsupportedOperationException( + "Subclass doesn't implement this and build method either"); + } + /** * Convert multi-field text field name to its inner keyword field. The limitation and assumption * is that the keyword field name is always "keyword" which is true by default. diff --git a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java index 4d156311e3..b4848738da 100644 --- a/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java +++ b/elasticsearch/src/main/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQuery.java @@ -18,6 +18,7 @@ import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.data.type.ExprType; +import java.util.List; import lombok.RequiredArgsConstructor; import org.elasticsearch.index.query.QueryBuilder; import org.elasticsearch.index.query.QueryBuilders; @@ -53,7 +54,24 @@ protected QueryBuilder doBuild(String fieldName, ExprType fieldType, ExprValue l case GTE: return query.gte(value); default: - throw new IllegalStateException("Comparison is supported by range query: " + comparison); + throw new IllegalStateException( + "Comparison is not supported by range query or improper number of arguments for " + + comparison); + } + } + + @Override + public QueryBuilder doBuild(String fieldName, ExprType fieldType, List literals) { + Object minValue = literals.get(0).value(); + Object maxValue = literals.get(1).value(); + RangeQueryBuilder query = QueryBuilders.rangeQuery(fieldName); + switch (comparison) { + case BETWEEN: + return query.gte(minValue).lte(maxValue); + default: + throw new IllegalStateException( + "Comparison is not supported by range query or improper number of arguments for " + + comparison); } } diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java index 4de1956cdc..9fc818b47c 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/FilterQueryBuilderTest.java @@ -33,6 +33,7 @@ import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; import com.google.common.collect.ImmutableMap; import java.util.Map; +import org.elasticsearch.index.query.QueryBuilder; import org.json.JSONObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.DisplayNameGeneration; @@ -99,6 +100,45 @@ void should_build_range_query_for_comparison_expression() { buildQuery(expr))); } + @Test + void should_build_range_query_for_between_operator() { + assertJsonEquals( + "{\n" + + " \"range\" : {\n" + + " \"age\" : {\n" + + " \"from\" : 20,\n" + + " \"to\" : 30,\n" + + " \"include_lower\" : true,\n" + + " \"include_upper\" : true,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery(dsl.between(ref("age", INTEGER), literal(20), literal(30)))); + + assertJsonEquals( + "{\n" + + " \"bool\" : {\n" + + " \"must_not\" : [\n" + + " {\n" + + " \"range\" : {\n" + + " \"age\" : {\n" + + " \"from\" : 20,\n" + + " \"to\" : 30,\n" + + " \"include_lower\" : true,\n" + + " \"include_upper\" : true,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + " }\n" + + " ],\n" + + " \"adjust_pure_negative\" : true,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}", + buildQuery(dsl.not_between(ref("age", INTEGER), literal(20), literal(30)))); + } + @Test void should_build_wildcard_query_for_like_expression() { assertJsonEquals( @@ -271,7 +311,8 @@ private static void assertJsonEquals(String expected, String actual) { } private String buildQuery(Expression expr) { - return filterQueryBuilder.build(expr).toString(); + QueryBuilder builder = filterQueryBuilder.build(expr); + return builder.toString(); } private void mockToStringSerializer() { diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java index f8d194f76a..84d77d53f8 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/LuceneQueryTest.java @@ -20,8 +20,10 @@ import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValue; import com.amazon.opendistroforelasticsearch.sql.expression.DSL; import com.amazon.opendistroforelasticsearch.sql.expression.config.ExpressionConfig; +import java.util.List; import org.junit.jupiter.api.DisplayNameGeneration; import org.junit.jupiter.api.DisplayNameGenerator; import org.junit.jupiter.api.Test; @@ -38,7 +40,10 @@ void should_not_support_single_argument_by_default() { @Test void should_throw_exception_if_not_implemented() { assertThrows(UnsupportedOperationException.class, () -> - new LuceneQuery(){}.doBuild(null, null, null)); + new LuceneQuery(){}.doBuild(null, null, (ExprValue) null)); + + assertThrows(UnsupportedOperationException.class, () -> + new LuceneQuery(){}.doBuild(null, null, (List) null)); } } \ No newline at end of file diff --git a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java index c1ed7e5393..9450719fa6 100644 --- a/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java +++ b/elasticsearch/src/test/java/com/amazon/opendistroforelasticsearch/sql/elasticsearch/storage/script/filter/lucene/RangeQueryTest.java @@ -16,11 +16,14 @@ package com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene; +import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.INTEGER; import static com.amazon.opendistroforelasticsearch.sql.data.type.ExprCoreType.STRING; import static org.junit.jupiter.api.Assertions.assertThrows; import com.amazon.opendistroforelasticsearch.sql.data.model.ExprValueUtils; import com.amazon.opendistroforelasticsearch.sql.elasticsearch.storage.script.filter.lucene.RangeQuery.Comparison; +import java.util.Arrays; +import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.DisplayNameGeneration; import org.junit.jupiter.api.DisplayNameGenerator; import org.junit.jupiter.api.Test; @@ -29,11 +32,17 @@ class RangeQueryTest { @Test - void should_throw_exception_for_unsupported_comparison() { - // Note that since we do switch check on enum comparison, this should'be impossible + void should_throw_exception_for_unsupported_comparison_or_incorrect_argument_number() { assertThrows(IllegalStateException.class, () -> new RangeQuery(Comparison.BETWEEN) - .doBuild("name", STRING, ExprValueUtils.stringValue("John"))); + .doBuild("name", STRING, ExprValueUtils.stringValue("John")), + "Comparison is not supported by range query or improper number of arguments for BETWEEN"); + + assertThrows(IllegalStateException.class, () -> + new RangeQuery(Comparison.LT) + .doBuild("age", INTEGER, Arrays.asList(ExprValueUtils.integerValue(30), + ExprValueUtils.integerValue(20))), + "Comparison is not supported by range query or improper number of arguments for LT"); } } \ No newline at end of file diff --git a/integ-test/src/test/resources/correctness/expressions/predicates.txt b/integ-test/src/test/resources/correctness/expressions/predicates.txt index 305695cea8..9bc00c0be9 100644 --- a/integ-test/src/test/resources/correctness/expressions/predicates.txt +++ b/integ-test/src/test/resources/correctness/expressions/predicates.txt @@ -4,13 +4,4 @@ false OR false AS bool true or false AS bool NOT true AS bool NOT false AS bool -NOT (true AND false) AS bool -1 BETWEEN 0 AND 2 -1 BETWEEN 2 AND 0 -1 BETWEEN 1 AND 1 -1.1 BETWEEN 1.0 AND 1.2 -'1' BETWEEN '0' AND '2' -DATE('2021-01-02') BETWEEN DATE('2021-01-01') AND DATE('2021-01-03') -TIME('00:00:00') BETWEEN TIME('00:00:00') AND TIME('00:00:01') -1 NOT BETWEEN 0 AND 2 -1 NOT BETWEEN 2 AND 0 \ No newline at end of file +NOT (true AND false) AS bool \ No newline at end of file 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 e9a8476d98..b69ee169e9 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 @@ -22,7 +22,7 @@ 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_BETWEEN; +import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.NOT; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.NOT_LIKE; import static com.amazon.opendistroforelasticsearch.sql.expression.function.BuiltinFunctionName.REGEXP; import static com.amazon.opendistroforelasticsearch.sql.sql.antlr.parser.OpenDistroSQLParser.BetweenPredicateContext; @@ -238,10 +238,10 @@ public UnresolvedExpression visitRegexpPredicate(RegexpPredicateContext ctx) { @Override public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { - return new Function( - ctx.NOT() == null ? BETWEEN.getName().getFunctionName() : - NOT_BETWEEN.getName().getFunctionName(), + Function between = new Function(BETWEEN.getName().getFunctionName(), Arrays.asList(visit(ctx.expr), visit(ctx.min), visit(ctx.max))); + return ctx.NOT() == null ? between : + new Function(NOT.getName().getFunctionName(), Collections.singletonList(between)); } @Override 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 9ec1b5b77f..75a1ba494c 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 @@ -251,7 +251,7 @@ public void canBuildBetweenPredicate() { @Test public void canBuildNotBetweenPredicate() { assertEquals( - function("not_between", intLiteral(1), intLiteral(0), intLiteral(2)), + function("not", function("between", intLiteral(1), intLiteral(0), intLiteral(2))), buildExprAst("1 not between 0 and 2") ); }