From 1c68307db300b7b0edb8561278050a72f95d3fab Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Mon, 12 Dec 2022 16:48:54 -0800 Subject: [PATCH 1/8] Add between grammar and in-memory impl Signed-off-by: Chen Dai --- .../org/opensearch/sql/expression/DSL.java | 4 ++ .../function/BuiltinFunctionName.java | 1 + .../predicate/BinaryPredicateOperator.java | 14 +++++ .../BinaryPredicateOperatorTest.java | 27 ++++++++++ sql/src/main/antlr/OpenSearchSQLParser.g4 | 1 + .../sql/sql/parser/AstExpressionBuilder.java | 54 +++++++++++++------ .../sql/parser/AstExpressionBuilderTest.java | 8 +++ 7 files changed, 92 insertions(+), 17 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java index fc425c6c20..c1564b3f98 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -137,6 +137,10 @@ public static RegexExpression regex(Expression sourceField, Expression pattern, return new RegexExpression(sourceField, pattern, identifier); } + public static FunctionExpression between(Expression... expressions) { + return compile(FunctionProperties.None, BuiltinFunctionName.BETWEEN, expressions); + } + public static PatternsExpression patterns(Expression sourceField, Expression pattern, Expression identifier) { return new PatternsExpression(sourceField, pattern, identifier); diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java index b23c7613d6..34f58cc4e1 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java @@ -133,6 +133,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/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java index 99399249c2..263d338e45 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java @@ -54,6 +54,7 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(like()); repository.register(notLike()); repository.register(regexp()); + repository.register(between()); } /** @@ -253,6 +254,19 @@ private static DefaultFunctionResolver notLike() { STRING)); } + private static DefaultFunctionResolver between() { + return FunctionDSL.define(BuiltinFunctionName.BETWEEN.getName(), + ExprCoreType.coreTypes().stream().map( + type -> FunctionDSL.impl( + FunctionDSL.nullMissingHandling((v1, v2, v3) -> + ExprBooleanValue.of(v1.compareTo(v2) >= 0 && v1.compareTo(v3) <= 0)), + BOOLEAN, + type, + type, + type)) + .collect(Collectors.toList())); + } + private static ExprValue lookupTableFunction(ExprValue arg1, ExprValue arg2, Table table) { if (table.contains(arg1, arg2)) { diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index 7354b52e24..657c4bdba2 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -786,6 +786,29 @@ void testRegexpString(StringPatternPair stringPatternPair) { .valueOf(valueEnv()).integerValue()); } + @Test + public void test_between() { + Object[][] testData = { + {false, 5, 10, 30}, + {true, 10, 10, 30}, + {true, 20, 10, 30}, + {true, 30, 10, 30}, + {false, 45, 10, 30}, + {false, "a", "b", "e"}, + {true, "c", "b", "e"}, + }; + + for (Object[] data : testData) { + assertEquals( + data[0], + eval(DSL.between( + DSL.literal(fromObjectValue(data[1])), + DSL.literal(fromObjectValue(data[2])), + DSL.literal(fromObjectValue(data[3])))), + String.format("Failed on test data: %s", Arrays.toString(data))); + } + } + /** * Todo. remove this test cases after script serilization implemented. */ @@ -819,4 +842,8 @@ public void compare_int_long() { FunctionExpression equal = DSL.equal(DSL.literal(1), DSL.literal(1L)); assertTrue(equal.valueOf(valueEnv()).booleanValue()); } + + private boolean eval(Expression expr) { + return expr.valueOf().booleanValue(); + } } diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index 58d4be1813..cd78efab0c 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 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..cc9027caa4 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 @@ -8,12 +8,16 @@ 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.BETWEEN; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NOT_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.LIKE; 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 +28,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 +95,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 +149,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 +163,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 +196,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 +253,7 @@ public UnresolvedExpression visitCountStarFunctionCall(CountStarFunctionCallCont } @Override - public UnresolvedExpression visitFilterClause(OpenSearchSQLParser.FilterClauseContext ctx) { + public UnresolvedExpression visitFilterClause(FilterClauseContext ctx) { return visit(ctx.expression()); } @@ -253,6 +265,14 @@ public UnresolvedExpression visitIsNullPredicate(IsNullPredicateContext ctx) { Arrays.asList(visit(ctx.predicate()))); } + @Override + public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { + return new Function(BETWEEN.getName().getFunctionName(), + ctx.predicate().stream() + .map(this::visit) + .collect(Collectors.toList())); + } + @Override public UnresolvedExpression visitLikePredicate(LikePredicateContext ctx) { return new Function( @@ -268,7 +288,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 +412,7 @@ public UnresolvedExpression visitConvertedDataType( @Override public UnresolvedExpression visitNoFieldRelevanceFunction( - OpenSearchSQLParser.NoFieldRelevanceFunctionContext ctx) { + NoFieldRelevanceFunctionContext ctx) { return new Function( ctx.noFieldRelevanceFunctionName().getText().toLowerCase(), noFieldRelevanceArguments(ctx)); @@ -415,7 +435,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 +448,7 @@ public UnresolvedExpression visitMultiFieldRelevanceFunction( } private Function buildFunction(String functionName, - List arg) { + List arg) { return new Function( functionName, arg @@ -447,7 +467,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 +479,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 +490,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 +505,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 +529,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 +547,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..e50acf4fa3 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 @@ -261,6 +261,14 @@ public void canBuildRegexpExpression() { ); } + @Test + public void canBuildBetweenExpression() { + assertEquals( + function("between", qualifiedName("age"), intLiteral(10), intLiteral(30)), + buildExprAst("age BETWEEN 10 AND 30") + ); + } + @Test public void canBuildLogicalExpression() { assertEquals( From 110b29abd687fde26c213a09c1bcaf05ddff9582 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 13 Dec 2022 09:41:39 -0800 Subject: [PATCH 2/8] Add comparison test for between Signed-off-by: Chen Dai --- .../resources/correctness/expressions/predicates.txt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/integ-test/src/test/resources/correctness/expressions/predicates.txt b/integ-test/src/test/resources/correctness/expressions/predicates.txt index 9bc00c0be9..e2a07b6821 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 +-3 BETWEEN 1 AND 5 AS bool +1 BETWEEN 1 AND 5 AS bool +3 BETWEEN 1 AND 5 AS bool +5 BETWEEN 1 AND 5 AS bool +6 BETWEEN 1 AND 5 AS bool +'a' BETWEEN 'b' AND 'e' AS bool +'c' BETWEEN 'b' AND 'e' AS bool \ No newline at end of file From c440f718fdbb105d8fdd3f823c5af8cb6e1ab0bd Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 13 Dec 2022 10:09:44 -0800 Subject: [PATCH 3/8] Add doctest for between Signed-off-by: Chen Dai --- docs/user/dql/expressions.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index a167ce29b5..192e9479e2 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -132,6 +132,8 @@ Operators +----------------+----------------------------------------+ | NOT IN | NOT IN value list test | +----------------+----------------------------------------+ +| BETWEEN | Between a range (endpoint inclusive) | ++----------------+----------------------------------------+ Basic Comparison Operator ------------------------- @@ -199,6 +201,20 @@ 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; + fetched rows / total rows = 1/1 + +---------------------+---------------------+ + | 1 BETWEEN 1 AND 3 | 4 BETWEEN 1 AND 3 | + |---------------------+---------------------| + | True | False | + +---------------------+---------------------+ + + Function Call ============= From 9c1a11fe0e5a4cfbed02e9451e38c128b8119776 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 13 Dec 2022 14:05:56 -0800 Subject: [PATCH 4/8] Add not between support Signed-off-by: Chen Dai --- .../predicate/BinaryPredicateOperatorTest.java | 16 ++++++++-------- docs/user/dql/expressions.rst | 17 +++++++++++------ sql/src/main/antlr/OpenSearchSQLParser.g4 | 2 +- .../sql/sql/parser/AstExpressionBuilder.java | 8 +++++++- .../sql/parser/AstExpressionBuilderTest.java | 10 ++++++++++ 5 files changed, 37 insertions(+), 16 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index 657c4bdba2..1492bc23a2 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -787,7 +787,7 @@ void testRegexpString(StringPatternPair stringPatternPair) { } @Test - public void test_between() { + public void test_between_and_not_between() { Object[][] testData = { {false, 5, 10, 30}, {true, 10, 10, 30}, @@ -799,13 +799,13 @@ public void test_between() { }; for (Object[] data : testData) { - assertEquals( - data[0], - eval(DSL.between( - DSL.literal(fromObjectValue(data[1])), - DSL.literal(fromObjectValue(data[2])), - DSL.literal(fromObjectValue(data[3])))), - String.format("Failed on test data: %s", Arrays.toString(data))); + Expression[] args = { + DSL.literal(fromObjectValue(data[1])), + DSL.literal(fromObjectValue(data[2])), + DSL.literal(fromObjectValue(data[3])) + }; + assertEquals(data[0], eval(DSL.between(args)), + String.format("Failed with test data: %s", Arrays.toString(data))); } } diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index 192e9479e2..9e04279648 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -134,6 +134,8 @@ Operators +----------------+----------------------------------------+ | BETWEEN | Between a range (endpoint inclusive) | +----------------+----------------------------------------+ +| NOT BETWEEN | Not between a range (between negation) | ++----------------+----------------------------------------+ Basic Comparison Operator ------------------------- @@ -206,13 +208,16 @@ 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; + 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 | - |---------------------+---------------------| - | True | False | - +---------------------+---------------------+ + +---------------------+---------------------+-------------------------| + | 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 cd78efab0c..e63d9054d9 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -278,7 +278,7 @@ predicate : expressionAtom #expressionAtomPredicate | left=predicate comparisonOperator right=predicate #binaryComparisonPredicate | predicate IS nullNotnull #isNullPredicate - | predicate BETWEEN predicate AND predicate #betweenPredicate + | 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 cc9027caa4..0594b4f6b7 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 @@ -12,6 +12,7 @@ import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NOT_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.LIKE; +import static org.opensearch.sql.expression.function.BuiltinFunctionName.NOT; 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; @@ -267,10 +268,15 @@ public UnresolvedExpression visitIsNullPredicate(IsNullPredicateContext ctx) { @Override public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { - return new Function(BETWEEN.getName().getFunctionName(), + Function func = new Function(BETWEEN.getName().getFunctionName(), ctx.predicate().stream() .map(this::visit) .collect(Collectors.toList())); + + if (ctx.NOT() != null) { + func = new Function(NOT.getName().getFunctionName(), Collections.singletonList(func)); + } + return func; } @Override 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 e50acf4fa3..6935a499a9 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 @@ -269,6 +269,16 @@ public void canBuildBetweenExpression() { ); } + @Test + public void canBuildNotBetweenExpression() { + assertEquals( + function("not", + function("between", + qualifiedName("age"), intLiteral(10), intLiteral(30))), + buildExprAst("age NOT BETWEEN 10 AND 30") + ); + } + @Test public void canBuildLogicalExpression() { assertEquals( From 9cb57305eabc6c507376b498d49732c136fafa20 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 13 Dec 2022 14:43:54 -0800 Subject: [PATCH 5/8] Fix doctest failure Signed-off-by: Chen Dai --- docs/user/dql/expressions.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index 9e04279648..1a2905e781 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -213,11 +213,11 @@ Here is an example for range test by BETWEEN expression:: ... 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 From 13d641701096532357b70d6294b91b7b88a1365c Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 13 Dec 2022 15:34:56 -0800 Subject: [PATCH 6/8] Refactor to rewrite to basic comparison expression 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 +++++++++++++++++++ .../org/opensearch/sql/expression/DSL.java | 4 -- .../function/BuiltinFunctionName.java | 1 - .../predicate/BinaryPredicateOperator.java | 14 ------- .../sql/analysis/ExpressionAnalyzerTest.java | 16 ++++++++ .../BinaryPredicateOperatorTest.java | 23 ----------- .../sql/sql/parser/AstExpressionBuilder.java | 11 ++--- .../sql/parser/AstExpressionBuilderTest.java | 6 +-- 11 files changed, 92 insertions(+), 50 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/main/java/org/opensearch/sql/expression/DSL.java b/core/src/main/java/org/opensearch/sql/expression/DSL.java index c1564b3f98..fc425c6c20 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -137,10 +137,6 @@ public static RegexExpression regex(Expression sourceField, Expression pattern, return new RegexExpression(sourceField, pattern, identifier); } - public static FunctionExpression between(Expression... expressions) { - return compile(FunctionProperties.None, BuiltinFunctionName.BETWEEN, expressions); - } - public static PatternsExpression patterns(Expression sourceField, Expression pattern, Expression identifier) { return new PatternsExpression(sourceField, pattern, identifier); diff --git a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java index 34f58cc4e1..b23c7613d6 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/BuiltinFunctionName.java @@ -133,7 +133,6 @@ 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/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java index 263d338e45..99399249c2 100644 --- a/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java +++ b/core/src/main/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperator.java @@ -54,7 +54,6 @@ public static void register(BuiltinFunctionRepository repository) { repository.register(like()); repository.register(notLike()); repository.register(regexp()); - repository.register(between()); } /** @@ -254,19 +253,6 @@ private static DefaultFunctionResolver notLike() { STRING)); } - private static DefaultFunctionResolver between() { - return FunctionDSL.define(BuiltinFunctionName.BETWEEN.getName(), - ExprCoreType.coreTypes().stream().map( - type -> FunctionDSL.impl( - FunctionDSL.nullMissingHandling((v1, v2, v3) -> - ExprBooleanValue.of(v1.compareTo(v2) >= 0 && v1.compareTo(v3) <= 0)), - BOOLEAN, - type, - type, - type)) - .collect(Collectors.toList())); - } - private static ExprValue lookupTableFunction(ExprValue arg1, ExprValue arg2, Table table) { if (table.contains(arg1, arg2)) { 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/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index 1492bc23a2..5e3ff93d8b 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -786,29 +786,6 @@ void testRegexpString(StringPatternPair stringPatternPair) { .valueOf(valueEnv()).integerValue()); } - @Test - public void test_between_and_not_between() { - Object[][] testData = { - {false, 5, 10, 30}, - {true, 10, 10, 30}, - {true, 20, 10, 30}, - {true, 30, 10, 30}, - {false, 45, 10, 30}, - {false, "a", "b", "e"}, - {true, "c", "b", "e"}, - }; - - for (Object[] data : testData) { - Expression[] args = { - DSL.literal(fromObjectValue(data[1])), - DSL.literal(fromObjectValue(data[2])), - DSL.literal(fromObjectValue(data[3])) - }; - assertEquals(data[0], eval(DSL.between(args)), - String.format("Failed with test data: %s", Arrays.toString(data))); - } - } - /** * Todo. remove this test cases after script serilization implemented. */ 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 0594b4f6b7..3044401843 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 @@ -8,7 +8,6 @@ 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.BETWEEN; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NOT_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.LIKE; @@ -77,6 +76,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.DataType; @@ -268,10 +268,11 @@ public UnresolvedExpression visitIsNullPredicate(IsNullPredicateContext ctx) { @Override public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { - Function func = new Function(BETWEEN.getName().getFunctionName(), - ctx.predicate().stream() - .map(this::visit) - .collect(Collectors.toList())); + UnresolvedExpression func = + new Between( + visit(ctx.predicate(0)), + visit(ctx.predicate(1)), + visit(ctx.predicate(2))); if (ctx.NOT() != null) { func = new Function(NOT.getName().getFunctionName(), Collections.singletonList(func)); 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 6935a499a9..a5ff6175db 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; @@ -264,7 +265,7 @@ public void canBuildRegexpExpression() { @Test public void canBuildBetweenExpression() { assertEquals( - function("between", qualifiedName("age"), intLiteral(10), intLiteral(30)), + between(qualifiedName("age"), intLiteral(10), intLiteral(30)), buildExprAst("age BETWEEN 10 AND 30") ); } @@ -273,8 +274,7 @@ public void canBuildBetweenExpression() { public void canBuildNotBetweenExpression() { assertEquals( function("not", - function("between", - qualifiedName("age"), intLiteral(10), intLiteral(30))), + between(qualifiedName("age"), intLiteral(10), intLiteral(30))), buildExprAst("age NOT BETWEEN 10 AND 30") ); } From f31fd698d6870ac742a0f77bb7c92c398ea82eed Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 13 Dec 2022 15:53:57 -0800 Subject: [PATCH 7/8] Clean up unused code Signed-off-by: Chen Dai --- .../operator/predicate/BinaryPredicateOperatorTest.java | 4 ---- docs/user/dql/expressions.rst | 2 +- .../resources/correctness/expressions/predicates.txt | 9 +-------- .../opensearch/sql/sql/parser/AstExpressionBuilder.java | 2 +- .../sql/sql/parser/AstExpressionBuilderTest.java | 8 +++++--- 5 files changed, 8 insertions(+), 17 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java index 5e3ff93d8b..7354b52e24 100644 --- a/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/operator/predicate/BinaryPredicateOperatorTest.java @@ -819,8 +819,4 @@ public void compare_int_long() { FunctionExpression equal = DSL.equal(DSL.literal(1), DSL.literal(1L)); assertTrue(equal.valueOf(valueEnv()).booleanValue()); } - - private boolean eval(Expression expr) { - return expr.valueOf().booleanValue(); - } } diff --git a/docs/user/dql/expressions.rst b/docs/user/dql/expressions.rst index 1a2905e781..d28c949405 100644 --- a/docs/user/dql/expressions.rst +++ b/docs/user/dql/expressions.rst @@ -134,7 +134,7 @@ Operators +----------------+----------------------------------------+ | BETWEEN | Between a range (endpoint inclusive) | +----------------+----------------------------------------+ -| NOT BETWEEN | Not between a range (between negation) | +| NOT BETWEEN | Not between a range (BETWEEN negation) | +----------------+----------------------------------------+ Basic Comparison Operator diff --git a/integ-test/src/test/resources/correctness/expressions/predicates.txt b/integ-test/src/test/resources/correctness/expressions/predicates.txt index e2a07b6821..9bc00c0be9 100644 --- a/integ-test/src/test/resources/correctness/expressions/predicates.txt +++ b/integ-test/src/test/resources/correctness/expressions/predicates.txt @@ -4,11 +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 --3 BETWEEN 1 AND 5 AS bool -1 BETWEEN 1 AND 5 AS bool -3 BETWEEN 1 AND 5 AS bool -5 BETWEEN 1 AND 5 AS bool -6 BETWEEN 1 AND 5 AS bool -'a' BETWEEN 'b' AND 'e' AS bool -'c' BETWEEN 'b' AND 'e' AS bool \ 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/org/opensearch/sql/sql/parser/AstExpressionBuilder.java b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java index 3044401843..a2c90cc7c9 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 @@ -275,7 +275,7 @@ public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { visit(ctx.predicate(2))); if (ctx.NOT() != null) { - func = new Function(NOT.getName().getFunctionName(), Collections.singletonList(func)); + func = AstDSL.not(func); } return func; } 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 a5ff6175db..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 @@ -265,7 +265,8 @@ public void canBuildRegexpExpression() { @Test public void canBuildBetweenExpression() { assertEquals( - between(qualifiedName("age"), intLiteral(10), intLiteral(30)), + between( + qualifiedName("age"), intLiteral(10), intLiteral(30)), buildExprAst("age BETWEEN 10 AND 30") ); } @@ -273,8 +274,9 @@ public void canBuildBetweenExpression() { @Test public void canBuildNotBetweenExpression() { assertEquals( - function("not", - between(qualifiedName("age"), intLiteral(10), intLiteral(30))), + not( + between( + qualifiedName("age"), intLiteral(10), intLiteral(30))), buildExprAst("age NOT BETWEEN 10 AND 30") ); } From 7743e59c073727aa0faf279b47dddd5571186399 Mon Sep 17 00:00:00 2001 From: Chen Dai Date: Tue, 13 Dec 2022 15:58:42 -0800 Subject: [PATCH 8/8] Prepare to publish PR Signed-off-by: Chen Dai --- .../opensearch/sql/sql/parser/AstExpressionBuilder.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 a2c90cc7c9..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,12 +6,13 @@ 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; import static org.opensearch.sql.expression.function.BuiltinFunctionName.IS_NULL; import static org.opensearch.sql.expression.function.BuiltinFunctionName.LIKE; -import static org.opensearch.sql.expression.function.BuiltinFunctionName.NOT; 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; @@ -76,7 +77,6 @@ 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.DataType; @@ -269,13 +269,13 @@ public UnresolvedExpression visitIsNullPredicate(IsNullPredicateContext ctx) { @Override public UnresolvedExpression visitBetweenPredicate(BetweenPredicateContext ctx) { UnresolvedExpression func = - new Between( + between( visit(ctx.predicate(0)), visit(ctx.predicate(1)), visit(ctx.predicate(2))); if (ctx.NOT() != null) { - func = AstDSL.not(func); + func = not(func); } return func; }