From 6c0af83978ed0d8a6e3a4f7a4800932141578c3f Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Fri, 16 Dec 2022 09:35:22 -0800 Subject: [PATCH] Add BETWEEN expression in v2 engine (#1163) * Add between grammar and in-memory impl Signed-off-by: Chen Dai * Add comparison test for between Signed-off-by: Chen Dai * Add doctest for between Signed-off-by: Chen Dai * Add not between support Signed-off-by: Chen Dai * Fix doctest failure Signed-off-by: Chen Dai * Refactor to rewrite to basic comparison expression Signed-off-by: Chen Dai * Clean up unused code Signed-off-by: Chen Dai * Prepare to publish PR Signed-off-by: Chen Dai Signed-off-by: Chen Dai --- .../sql/analysis/ExpressionAnalyzer.java | 14 +++++ .../sql/ast/AbstractNodeVisitor.java | 5 ++ .../org/opensearch/sql/ast/dsl/AstDSL.java | 8 +++ .../sql/ast/expression/Between.java | 40 ++++++++++++ .../sql/analysis/ExpressionAnalyzerTest.java | 16 +++++ docs/user/dql/expressions.rst | 21 +++++++ sql/src/main/antlr/OpenSearchSQLParser.g4 | 1 + .../sql/sql/parser/AstExpressionBuilder.java | 61 +++++++++++++------ .../sql/parser/AstExpressionBuilderTest.java | 20 ++++++ 9 files changed, 169 insertions(+), 17 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/ast/expression/Between.java diff --git a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java index 719c3adbce..ff3c01d5b8 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java @@ -6,6 +6,11 @@ package org.opensearch.sql.analysis; +import static org.opensearch.sql.ast.dsl.AstDSL.and; +import static org.opensearch.sql.ast.dsl.AstDSL.compare; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.GTE; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.LTE; + import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; @@ -22,6 +27,7 @@ import org.opensearch.sql.ast.expression.AggregateFunction; import org.opensearch.sql.ast.expression.AllFields; import org.opensearch.sql.ast.expression.And; +import org.opensearch.sql.ast.expression.Between; import org.opensearch.sql.ast.expression.Case; import org.opensearch.sql.ast.expression.Cast; import org.opensearch.sql.ast.expression.Compare; @@ -229,6 +235,14 @@ public Expression visitCompare(Compare node, AnalysisContext context) { functionName, Arrays.asList(left, right)); } + @Override + public Expression visitBetween(Between node, AnalysisContext context) { + return and( + compare(">=", node.getValue(), node.getLowerBound()), + compare("<=", node.getValue(), node.getUpperBound()) + ).accept(this, context); + } + @Override public Expression visitCase(Case node, AnalysisContext context) { List whens = new ArrayList<>(); diff --git a/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java b/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java index fe993c899e..393de05164 100644 --- a/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java @@ -12,6 +12,7 @@ import org.opensearch.sql.ast.expression.And; import org.opensearch.sql.ast.expression.Argument; import org.opensearch.sql.ast.expression.AttributeList; +import org.opensearch.sql.ast.expression.Between; import org.opensearch.sql.ast.expression.Case; import org.opensearch.sql.ast.expression.Cast; import org.opensearch.sql.ast.expression.Compare; @@ -173,6 +174,10 @@ public T visitCompare(Compare node, C context) { return visitChildren(node, context); } + public T visitBetween(Between node, C context) { + return visitChildren(node, context); + } + public T visitArgument(Argument node, C context) { return visitChildren(node, context); } diff --git a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java index 2959cae4a1..039b6380f7 100644 --- a/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java +++ b/core/src/main/java/org/opensearch/sql/ast/dsl/AstDSL.java @@ -16,6 +16,7 @@ import org.opensearch.sql.ast.expression.AllFields; import org.opensearch.sql.ast.expression.And; import org.opensearch.sql.ast.expression.Argument; +import org.opensearch.sql.ast.expression.Between; import org.opensearch.sql.ast.expression.Case; import org.opensearch.sql.ast.expression.Cast; import org.opensearch.sql.ast.expression.Compare; @@ -59,6 +60,7 @@ import org.opensearch.sql.ast.tree.TableFunction; import org.opensearch.sql.ast.tree.UnresolvedPlan; import org.opensearch.sql.ast.tree.Values; +import org.opensearch.sql.expression.function.BuiltinFunctionName; /** * Class of static methods to create specific node instances. @@ -320,6 +322,12 @@ public static UnresolvedExpression compare( return new Compare(operator, left, right); } + public static UnresolvedExpression between(UnresolvedExpression value, + UnresolvedExpression lowerBound, + UnresolvedExpression upperBound) { + return new Between(value, lowerBound, upperBound); + } + public static Argument argument(String argName, Literal argValue) { return new Argument(argName, argValue); } diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/Between.java b/core/src/main/java/org/opensearch/sql/ast/expression/Between.java new file mode 100644 index 0000000000..886c9a9282 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/ast/expression/Between.java @@ -0,0 +1,40 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.ast.expression; + +import java.util.Arrays; +import java.util.List; +import lombok.Data; +import lombok.EqualsAndHashCode; +import org.opensearch.sql.ast.AbstractNodeVisitor; +import org.opensearch.sql.ast.Node; + +/** + * Unresolved expression for BETWEEN. + */ +@Data +@EqualsAndHashCode(callSuper = false) +public class Between extends UnresolvedExpression { + + /** Value for range check. */ + private final UnresolvedExpression value; + + /** Lower bound of the range (inclusive). */ + private final UnresolvedExpression lowerBound; + + /** Upper bound of the range (inclusive). */ + private final UnresolvedExpression upperBound; + + @Override + public List getChild() { + return Arrays.asList(value, lowerBound, upperBound); + } + + @Override + public T accept(AbstractNodeVisitor nodeVisitor, C context) { + return nodeVisitor.visitBetween(this, context); + } +} diff --git a/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java index 7114b220ab..a69b27fc61 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java @@ -107,6 +107,22 @@ public void qualified_name() { ); } + @Test + public void between() { + assertAnalyzeEqual( + DSL.and( + DSL.gte( + DSL.ref("integer_value", INTEGER), + DSL.literal(20)), + DSL.lte( + DSL.ref("integer_value", INTEGER), + DSL.literal(30))), + AstDSL.between( + qualifiedName("integer_value"), + AstDSL.intLiteral(20), + AstDSL.intLiteral(30))); + } + @Test public void case_value() { assertAnalyzeEqual( diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index 89fcd7f0af..09818c902d 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -132,6 +132,10 @@ Operators +----------------+----------------------------------------+ | NOT IN | NOT IN value list test | +----------------+----------------------------------------+ +| BETWEEN | Between a range (endpoint inclusive) | ++----------------+----------------------------------------+ +| NOT BETWEEN | Not between a range (BETWEEN negation) | ++----------------+----------------------------------------+ Basic Comparison Operator ------------------------- @@ -199,6 +203,23 @@ Here is an example for IN value test:: | True | True | +---------------+-------------------+ +BETWEEN range test +------------------ + +Here is an example for range test by BETWEEN expression:: + + os> SELECT + ... 1 BETWEEN 1 AND 3, + ... 4 BETWEEN 1 AND 3, + ... 4 NOT BETWEEN 1 AND 3; + fetched rows / total rows = 1/1 + +---------------------+---------------------+-------------------------+ + | 1 BETWEEN 1 AND 3 | 4 BETWEEN 1 AND 3 | 4 NOT BETWEEN 1 AND 3 | + |---------------------+---------------------+-------------------------| + | True | False | True | + +---------------------+---------------------+-------------------------+ + + Function Call ============= diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index 58d4be1813..e63d9054d9 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -278,6 +278,7 @@ predicate : expressionAtom #expressionAtomPredicate | left=predicate comparisonOperator right=predicate #binaryComparisonPredicate | predicate IS nullNotnull #isNullPredicate + | predicate NOT? BETWEEN predicate AND predicate #betweenPredicate | left=predicate NOT? LIKE right=predicate #likePredicate | left=predicate REGEXP right=predicate #regexpPredicate | predicate NOT? IN '(' expressions ')' #inPredicate diff --git a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java index bae22595ca..c5300869ef 100644 --- a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java +++ b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java @@ -6,6 +6,8 @@ package org.opensearch.sql.sql.parser; +import static org.opensearch.sql.ast.dsl.AstDSL.between; +import static org.opensearch.sql.ast.dsl.AstDSL.not; import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NOT_NULL; @@ -14,6 +16,9 @@ import static org.opensearch.sql.expression.function.BuiltinFunctionName.NOT_LIKE; import static org.opensearch.sql.expression.function.BuiltinFunctionName.POSITION; import static org.opensearch.sql.expression.function.BuiltinFunctionName.REGEXP; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.AlternateMultiMatchFieldContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.AlternateMultiMatchQueryContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.BetweenPredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.BinaryComparisonPredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.BooleanContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.CaseFuncAlternativeContext; @@ -24,16 +29,25 @@ import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.DataTypeFunctionCallContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.DateLiteralContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.DistinctCountFunctionCallContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.FilterClauseContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.FilteredAggregationFunctionCallContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.FunctionArgContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.HighlightFunctionCallContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.InPredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.IsNullPredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.LikePredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.MathExpressionAtomContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.MultiFieldRelevanceFunctionContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.NoFieldRelevanceFunctionContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.NotExpressionContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.NullLiteralContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.OverClauseContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.PositionFunctionContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.QualifiedNameContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.RegexpPredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.RegularAggregateFunctionCallContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.RelevanceArgContext; +import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.RelevanceFieldAndWeightContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.ScalarFunctionCallContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.ScalarWindowFunctionContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.ShowDescribePatternContext; @@ -82,7 +96,6 @@ import org.opensearch.sql.ast.tree.Sort.SortOption; import org.opensearch.sql.common.utils.StringUtils; import org.opensearch.sql.expression.function.BuiltinFunctionName; -import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser; import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.AndExpressionContext; import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.ColumnNameContext; import org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.IdentContext; @@ -137,7 +150,7 @@ public UnresolvedExpression visitScalarFunctionCall(ScalarFunctionCallContext ct @Override public UnresolvedExpression visitHighlightFunctionCall( - OpenSearchSQLParser.HighlightFunctionCallContext ctx) { + HighlightFunctionCallContext ctx) { ImmutableMap.Builder builder = ImmutableMap.builder(); ctx.highlightFunction().highlightArg().forEach(v -> builder.put( v.highlightArgName().getText().toLowerCase(), @@ -151,7 +164,7 @@ public UnresolvedExpression visitHighlightFunctionCall( @Override public UnresolvedExpression visitPositionFunction( - OpenSearchSQLParser.PositionFunctionContext ctx) { + PositionFunctionContext ctx) { return new Function( POSITION.getName().getFunctionName(), Arrays.asList(visitFunctionArg(ctx.functionArg(0)), @@ -184,7 +197,7 @@ public UnresolvedExpression visitShowDescribePattern( @Override public UnresolvedExpression visitFilteredAggregationFunctionCall( - OpenSearchSQLParser.FilteredAggregationFunctionCallContext ctx) { + FilteredAggregationFunctionCallContext ctx) { AggregateFunction agg = (AggregateFunction) visit(ctx.aggregateFunction()); return agg.condition(visit(ctx.filterClause())); } @@ -241,7 +254,7 @@ public UnresolvedExpression visitCountStarFunctionCall(CountStarFunctionCallCont } @Override - public UnresolvedExpression visitFilterClause(OpenSearchSQLParser.FilterClauseContext ctx) { + public UnresolvedExpression visitFilterClause(FilterClauseContext ctx) { return visit(ctx.expression()); } @@ -253,6 +266,20 @@ public UnresolvedExpression visitIsNullPredicate(IsNullPredicateContext ctx) { Arrays.asList(visit(ctx.predicate()))); } + @Override + public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { + UnresolvedExpression func = + between( + visit(ctx.predicate(0)), + visit(ctx.predicate(1)), + visit(ctx.predicate(2))); + + if (ctx.NOT() != null) { + func = not(func); + } + return func; + } + @Override public UnresolvedExpression visitLikePredicate(LikePredicateContext ctx) { return new Function( @@ -268,7 +295,7 @@ public UnresolvedExpression visitRegexpPredicate(RegexpPredicateContext ctx) { } @Override - public UnresolvedExpression visitInPredicate(OpenSearchSQLParser.InPredicateContext ctx) { + public UnresolvedExpression visitInPredicate(InPredicateContext ctx) { UnresolvedExpression field = visit(ctx.predicate()); List inLists = ctx .expressions() @@ -392,7 +419,7 @@ public UnresolvedExpression visitConvertedDataType( @Override public UnresolvedExpression visitNoFieldRelevanceFunction( - OpenSearchSQLParser.NoFieldRelevanceFunctionContext ctx) { + NoFieldRelevanceFunctionContext ctx) { return new Function( ctx.noFieldRelevanceFunctionName().getText().toLowerCase(), noFieldRelevanceArguments(ctx)); @@ -415,7 +442,7 @@ public UnresolvedExpression visitMultiFieldRelevanceFunction( if ((funcName.equalsIgnoreCase(BuiltinFunctionName.MULTI_MATCH.toString()) || funcName.equalsIgnoreCase(BuiltinFunctionName.MULTIMATCH.toString()) || funcName.equalsIgnoreCase(BuiltinFunctionName.MULTIMATCHQUERY.toString())) - && ! ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchQueryContext.class) + && ! ctx.getRuleContexts(AlternateMultiMatchQueryContext.class) .isEmpty()) { return new Function( ctx.multiFieldRelevanceFunctionName().getText().toLowerCase(), @@ -428,7 +455,7 @@ public UnresolvedExpression visitMultiFieldRelevanceFunction( } private Function buildFunction(String functionName, - List arg) { + List arg) { return new Function( functionName, arg @@ -447,7 +474,7 @@ private QualifiedName visitIdentifiers(List identifiers) { ); } - private void fillRelevanceArgs(List args, + private void fillRelevanceArgs(List args, ImmutableList.Builder builder) { // To support old syntax we must support argument keys as quoted strings. args.forEach(v -> builder.add(v.argName == null @@ -459,7 +486,7 @@ private void fillRelevanceArgs(List arg } private List noFieldRelevanceArguments( - OpenSearchSQLParser.NoFieldRelevanceFunctionContext ctx) { + NoFieldRelevanceFunctionContext ctx) { // all the arguments are defaulted to string values // to skip environment resolving and function signature resolving ImmutableList.Builder builder = ImmutableList.builder(); @@ -470,7 +497,7 @@ private List noFieldRelevanceArguments( } private List singleFieldRelevanceArguments( - OpenSearchSQLParser.SingleFieldRelevanceFunctionContext ctx) { + SingleFieldRelevanceFunctionContext ctx) { // all the arguments are defaulted to string values // to skip environment resolving and function signature resolving ImmutableList.Builder builder = ImmutableList.builder(); @@ -485,12 +512,12 @@ private List singleFieldRelevanceArguments( private List multiFieldRelevanceArguments( - OpenSearchSQLParser.MultiFieldRelevanceFunctionContext ctx) { + MultiFieldRelevanceFunctionContext ctx) { // all the arguments are defaulted to string values // to skip environment resolving and function signature resolving ImmutableList.Builder builder = ImmutableList.builder(); var fields = new RelevanceFieldList(ctx - .getRuleContexts(OpenSearchSQLParser.RelevanceFieldAndWeightContext.class) + .getRuleContexts(RelevanceFieldAndWeightContext.class) .stream() .collect(Collectors.toMap( f -> StringUtils.unquoteText(f.field.getText()), @@ -509,14 +536,14 @@ private List multiFieldRelevanceArguments( * @return : Returns list of all arguments for relevance function. */ private List alternateMultiMatchArguments( - OpenSearchSQLParser.MultiFieldRelevanceFunctionContext ctx) { + MultiFieldRelevanceFunctionContext ctx) { // all the arguments are defaulted to string values // to skip environment resolving and function signature resolving ImmutableList.Builder builder = ImmutableList.builder(); Map fieldAndWeightMap = new HashMap<>(); String[] fieldAndWeights = StringUtils.unquoteText( - ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchFieldContext.class) + ctx.getRuleContexts(AlternateMultiMatchFieldContext.class) .stream().findFirst().get().argVal.getText()).split(","); for (var fieldAndWeight : fieldAndWeights) { @@ -527,7 +554,7 @@ private List alternateMultiMatchArguments( builder.add(new UnresolvedArgument("fields", new RelevanceFieldList(fieldAndWeightMap))); - ctx.getRuleContexts(OpenSearchSQLParser.AlternateMultiMatchQueryContext.class) + ctx.getRuleContexts(AlternateMultiMatchQueryContext.class) .stream().findFirst().ifPresent( arg -> builder.add(new UnresolvedArgument("query", diff --git a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java index 9af4119fdf..79ae64c796 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.opensearch.sql.ast.dsl.AstDSL.aggregate; import static org.opensearch.sql.ast.dsl.AstDSL.and; +import static org.opensearch.sql.ast.dsl.AstDSL.between; import static org.opensearch.sql.ast.dsl.AstDSL.booleanLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.caseWhen; import static org.opensearch.sql.ast.dsl.AstDSL.dateLiteral; @@ -261,6 +262,25 @@ public void canBuildRegexpExpression() { ); } + @Test + public void canBuildBetweenExpression() { + assertEquals( + between( + qualifiedName("age"), intLiteral(10), intLiteral(30)), + buildExprAst("age BETWEEN 10 AND 30") + ); + } + + @Test + public void canBuildNotBetweenExpression() { + assertEquals( + not( + between( + qualifiedName("age"), intLiteral(10), intLiteral(30))), + buildExprAst("age NOT BETWEEN 10 AND 30") + ); + } + @Test public void canBuildLogicalExpression() { assertEquals(