From 57ae38bb2d4764e50585681761f9a9e6af087371 Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Wed, 30 Nov 2022 10:47:46 -0800 Subject: [PATCH 1/7] Add position() to V2 engine (#177) Signed-off-by: Margarit Hakobyan Rebased --- .../sql/analysis/ExpressionAnalyzer.java | 8 ++ .../sql/ast/AbstractNodeVisitor.java | 5 + .../org/opensearch/sql/ast/dsl/AstDSL.java | 6 + .../sql/ast/expression/PositionFunction.java | 39 ++++++ .../org/opensearch/sql/expression/DSL.java | 4 + .../function/BuiltinFunctionName.java | 25 ++-- .../sql/expression/text/TextFunction.java | 39 ++++-- .../analysis/NamedExpressionAnalyzerTest.java | 11 ++ .../sql/expression/text/TextFunctionTest.java | 20 +++ docs/user/dql/functions.rst | 25 ++++ .../sql/sql/PositionFunctionIT.java | 129 ++++++++++++++++++ sql/src/main/antlr/OpenSearchSQLLexer.g4 | 1 + sql/src/main/antlr/OpenSearchSQLParser.g4 | 5 + .../sql/sql/parser/AstExpressionBuilder.java | 8 ++ .../sql/parser/AstExpressionBuilderTest.java | 17 +++ 15 files changed, 319 insertions(+), 23 deletions(-) create mode 100644 core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java create mode 100644 integ-test/src/test/java/org/opensearch/sql/sql/PositionFunctionIT.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..7ce4871634 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java @@ -34,6 +34,7 @@ import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; +import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.Span; @@ -201,6 +202,13 @@ public Expression visitHighlightFunction(HighlightFunction node, AnalysisContext return new HighlightExpression(expr); } + @Override + public Expression visitPositionFunction(PositionFunction node, AnalysisContext context) { + Expression stringPatternExpr = node.getStringPatternExpr().accept(this, context); + Expression searchStringExpr = node.getSearchStringExpr().accept(this, context); + return DSL.position(stringPatternExpr, searchStringExpr); + } + @Override public Expression visitIn(In node, AnalysisContext context) { return visitIn(node.getField(), node.getValueList(), context); 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..e806510074 100644 --- a/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java @@ -26,6 +26,7 @@ import org.opensearch.sql.ast.expression.Map; import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; +import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.Span; @@ -273,6 +274,10 @@ public T visitHighlightFunction(HighlightFunction node, C context) { return visitChildren(node, context); } + public T visitPositionFunction(PositionFunction node, C context) { + return visitChildren(node, context); + } + public T visitStatement(Statement node, C context) { return visit(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..a64a1fc246 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 @@ -32,6 +32,7 @@ import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; import org.opensearch.sql.ast.expression.ParseMethod; +import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; @@ -283,6 +284,11 @@ public UnresolvedExpression highlight(UnresolvedExpression fieldName, return new HighlightFunction(fieldName, arguments); } + public UnresolvedExpression position(UnresolvedExpression stringPatternExpr, + UnresolvedExpression searchStringExpr) { + return new PositionFunction(stringPatternExpr, searchStringExpr); + } + public UnresolvedExpression window(UnresolvedExpression function, List partitionByList, List> sortList) { diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java b/core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java new file mode 100644 index 0000000000..988237ebd0 --- /dev/null +++ b/core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java @@ -0,0 +1,39 @@ +/* + * 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.AllArgsConstructor; +import lombok.EqualsAndHashCode; +import lombok.Getter; +import lombok.ToString; +import org.opensearch.sql.ast.AbstractNodeVisitor; + + +/** + * Expression node of Position function. + */ +@AllArgsConstructor +@EqualsAndHashCode(callSuper = false) +@Getter +@ToString +public class PositionFunction extends UnresolvedExpression { + @Getter + private UnresolvedExpression stringPatternExpr; + @Getter + private UnresolvedExpression searchStringExpr; + + @Override + public List getChild() { + return Arrays.asList(stringPatternExpr, searchStringExpr); + } + + @Override + public R accept(AbstractNodeVisitor nodeVisitor, C context) { + return nodeVisitor.visitPositionFunction(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 19cd8fd326..c08851c635 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -230,6 +230,10 @@ public static FunctionExpression cbrt(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.CBRT, expressions); } + public static FunctionExpression position(Expression... expressions) { + return compile(BuiltinFunctionName.POSITION, expressions); + } + public static FunctionExpression truncate(Expression... expressions) { return compile(FunctionProperties.None, BuiltinFunctionName.TRUNCATE, expressions); } 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 9dcd73a427..49370c8da8 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 @@ -153,23 +153,24 @@ public enum BuiltinFunctionName { /** * Text Functions. */ - SUBSTR(FunctionName.of("substr")), - SUBSTRING(FunctionName.of("substring")), - RTRIM(FunctionName.of("rtrim")), - LTRIM(FunctionName.of("ltrim")), - TRIM(FunctionName.of("trim")), - UPPER(FunctionName.of("upper")), - LOWER(FunctionName.of("lower")), - REGEXP(FunctionName.of("regexp")), + ASCII(FunctionName.of("ascii")), CONCAT(FunctionName.of("concat")), CONCAT_WS(FunctionName.of("concat_ws")), - LENGTH(FunctionName.of("length")), - STRCMP(FunctionName.of("strcmp")), - RIGHT(FunctionName.of("right")), LEFT(FunctionName.of("left")), - ASCII(FunctionName.of("ascii")), + LENGTH(FunctionName.of("length")), LOCATE(FunctionName.of("locate")), + LOWER(FunctionName.of("lower")), + LTRIM(FunctionName.of("ltrim")), + POSITION(FunctionName.of("position")), + REGEXP(FunctionName.of("regexp")), REPLACE(FunctionName.of("replace")), + RIGHT(FunctionName.of("right")), + RTRIM(FunctionName.of("rtrim")), + STRCMP(FunctionName.of("strcmp")), + SUBSTR(FunctionName.of("substr")), + SUBSTRING(FunctionName.of("substring")), + TRIM(FunctionName.of("trim")), + UPPER(FunctionName.of("upper")), /** * NULL Test. diff --git a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java b/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java index 8035728d19..b51d6a2716 100644 --- a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java @@ -39,22 +39,23 @@ public class TextFunction { * @param repository {@link BuiltinFunctionRepository}. */ public void register(BuiltinFunctionRepository repository) { - repository.register(substr()); - repository.register(substring()); - repository.register(ltrim()); - repository.register(rtrim()); - repository.register(trim()); - repository.register(lower()); - repository.register(upper()); + repository.register(ascii()); repository.register(concat()); repository.register(concat_ws()); - repository.register(length()); - repository.register(strcmp()); - repository.register(right()); repository.register(left()); - repository.register(ascii()); + repository.register(length()); repository.register(locate()); + repository.register(lower()); + repository.register(ltrim()); + repository.register(position()); repository.register(replace()); + repository.register(right()); + repository.register(rtrim()); + repository.register(strcmp()); + repository.register(substr()); + repository.register(substring()); + repository.register(trim()); + repository.register(upper()); } /** @@ -241,6 +242,18 @@ private DefaultFunctionResolver locate() { TextFunction::exprLocate), INTEGER, STRING, STRING, INTEGER)); } + /** + * Returns the position of the first occurrence of a substring in a string starting from 1. + * Returns 0 if substring is not in string. + * Returns NULL if any argument is NULL. + * Supports following signature: + * (STRING IN STRING) -> INTEGER + */ + private DefaultFunctionResolver position() { + return define(BuiltinFunctionName.POSITION.getName(), + impl(nullMissingHandling(TextFunction::exprPosition), INTEGER, STRING, STRING)); + } + /** * REPLACE(str, from_str, to_str) returns the string str with all occurrences of * the string from_str replaced by the string to_str. @@ -313,6 +326,10 @@ private static ExprValue exprLocate(ExprValue subStr, ExprValue str, ExprValue p str.stringValue().indexOf(subStr.stringValue(), pos.integerValue() - 1) + 1); } + private static ExprValue exprPosition(ExprValue subStr, ExprValue str) { + return exprLocate(subStr, str); + } + private static ExprValue exprReplace(ExprValue str, ExprValue from, ExprValue to) { return new ExprStringValue(str.stringValue().replaceAll(from.stringValue(), to.stringValue())); } diff --git a/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java index 913593add3..2f97c6480e 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java @@ -16,6 +16,7 @@ import org.opensearch.sql.ast.expression.Alias; import org.opensearch.sql.ast.expression.HighlightFunction; import org.opensearch.sql.ast.expression.Literal; +import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.expression.NamedExpression; import org.springframework.context.annotation.Configuration; import org.springframework.test.context.ContextConfiguration; @@ -48,4 +49,14 @@ void visit_highlight() { NamedExpression analyze = analyzer.analyze(alias, analysisContext); assertEquals("highlight(fieldA)", analyze.getNameOrAlias()); } + + @Test + void visit_position() { + Alias alias = AstDSL.alias("position(fieldA IN fieldB)", + new PositionFunction(AstDSL.stringLiteral("fieldA"), AstDSL.stringLiteral("fieldB"))); + NamedExpressionAnalyzer analyzer = new NamedExpressionAnalyzer(expressionAnalyzer); + + NamedExpression analyze = analyzer.analyze(alias, analysisContext); + assertEquals("position(fieldA IN fieldB)", analyze.getNameOrAlias()); + } } diff --git a/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java index 502fe70ec8..5e32678b94 100644 --- a/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/text/TextFunctionTest.java @@ -377,6 +377,26 @@ void locate() { DSL.locate(missingRef, DSL.literal("hello"), DSL.literal(1)))); } + @Test + void position() { + FunctionExpression expression = DSL.position( + DSL.literal("world"), + DSL.literal("helloworldworld")); + assertEquals(INTEGER, expression.type()); + assertEquals(6, eval(expression).integerValue()); + + expression = DSL.position( + DSL.literal("abc"), + DSL.literal("hello world")); + assertEquals(INTEGER, expression.type()); + assertEquals(0, eval(expression).integerValue()); + + when(nullRef.type()).thenReturn(STRING); + assertEquals(nullValue(), eval(DSL.position(nullRef, DSL.literal("hello")))); + when(missingRef.type()).thenReturn(STRING); + assertEquals(missingValue(), eval(DSL.position(missingRef, DSL.literal("hello")))); + } + @Test void replace() { FunctionExpression expression = DSL.replace( diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 0644b970f5..4c53f84b6b 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2330,6 +2330,31 @@ Example:: +---------------------+---------------------+ +POSITION +------ + +Description +>>>>>>>>>>> + +Usage: The syntax POSITION(substr IN str) returns the position of the first occurrence of substring substr in string str. Returns 0 if substr is not in str. Returns NULL if any argument is NULL. + +Argument type: STRING, STRING, INTEGER + +Return type integer: + +(STRING IN STRING) -> INTEGER + +Example:: + + os> SELECT POSITION('world' IN 'helloworld') + fetched rows / total rows = 1/1 + +-------------------------------------+---------------------------------------+ + | POSITION('world' IN 'helloworld') | POSITION('invalid' IN 'helloworld') | + |-------------------------------------+---------------------------------------| + | 6 | 0 | + +-------------------------------------+---------------------------------------+ + + REPLACE ------- diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/PositionFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/PositionFunctionIT.java new file mode 100644 index 0000000000..f51a3a0977 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/sql/PositionFunctionIT.java @@ -0,0 +1,129 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.sql; + +import org.json.JSONObject; +import org.junit.Test; +import org.opensearch.sql.legacy.SQLIntegTestCase; +import org.opensearch.sql.legacy.TestsConstants; + +import static org.opensearch.sql.util.MatcherUtils.rows; +import static org.opensearch.sql.util.MatcherUtils.schema; +import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; +import static org.opensearch.sql.util.MatcherUtils.verifySchema; + +public class PositionFunctionIT extends SQLIntegTestCase { + + @Override + protected void init() throws Exception { + loadIndex(Index.PEOPLE2); + loadIndex(Index.CALCS); + } + + @Test + public void position_function_test() { + String query = "SELECT firstname, position('a' IN firstname) FROM %s"; + JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_PEOPLE2)); + + verifySchema(response, schema("firstname", null, "keyword"), + schema("position('a' IN firstname)", null, "integer")); + assertEquals(12, response.getInt("total")); + + verifyDataRows(response, + rows("Daenerys", 2), rows("Hattie", 2), + rows("Nanette", 2), rows("Dale", 2), + rows("Elinor", 0), rows("Virginia", 8), + rows("Dillard", 5), rows("Mcgee", 0), + rows("Aurelia", 7), rows("Fulton", 0), + rows("Burton", 0), rows("Josie", 0)); + } + + @Test + public void position_function_with_nulls_test() { + String query = "SELECT str2, position('ee' IN str2) FROM %s"; + JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS)); + + verifySchema(response, schema("str2", null, "keyword"), + schema("position('ee' IN str2)", null, "integer")); + assertEquals(17, response.getInt("total")); + + verifyDataRows(response, + rows("one", 0), rows("two", 0), + rows("three", 4), rows(null, null), + rows("five", 0), rows("six", 0), + rows(null, null), rows("eight", 0), + rows("nine", 0), rows("ten", 0), + rows("eleven", 0), rows("twelve", 0), + rows(null, null), rows("fourteen", 6), + rows("fifteen", 5), rows("sixteen", 5), + rows(null, null)); + } + + @Test + public void position_function_with_string_literals_test() { + String query = "SELECT position('world' IN 'hello world')"; + JSONObject response = executeJdbcRequest(query); + + verifySchema(response, schema("position('world' IN 'hello world')", null, "integer")); + assertEquals(1, response.getInt("total")); + + verifyDataRows(response, rows(7)); + } + + @Test + public void position_function_with_only_fields_as_args_test() { + String query = "SELECT position(str3 IN str2) FROM %s WHERE str2 IN ('one', 'two', 'three')"; + JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS)); + + verifySchema(response, schema("position(str3 IN str2)", null, "integer")); + assertEquals(3, response.getInt("total")); + + verifyDataRows(response, rows(3), rows(0), rows(4)); + } + + @Test + public void position_function_with_function_as_arg_test() { + String query = "SELECT position(upper(str3) IN str1) FROM %s WHERE str1 LIKE 'BINDING SUPPLIES'"; + JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS)); + + verifySchema(response, schema("position(upper(str3) IN str1)", null, "integer")); + assertEquals(1, response.getInt("total")); + + verifyDataRows(response, rows(15)); + } + + @Test + public void position_function_in_where_clause_test() { + String query = "SELECT str2 FROM %s WHERE position(str3 IN str2)=1"; + JSONObject response = executeJdbcRequest(String.format(query, TestsConstants.TEST_INDEX_CALCS)); + + verifySchema(response, schema("str2", null, "keyword")); + assertEquals(2, response.getInt("total")); + + verifyDataRows(response, rows("eight"), rows("eleven")); + } + + @Test + public void position_function_with_null_args_test() { + String query1 = "SELECT str2, position(null IN str2) FROM %s WHERE str2 IN ('one')"; + String query2 = "SELECT str2, position(str2 IN null) FROM %s WHERE str2 IN ('one')"; + JSONObject response1 = executeJdbcRequest(String.format(query1, TestsConstants.TEST_INDEX_CALCS)); + JSONObject response2 = executeJdbcRequest(String.format(query2, TestsConstants.TEST_INDEX_CALCS)); + + verifySchema(response1, + schema("str2", null, "keyword"), + schema("position(null IN str2)", null, "integer")); + assertEquals(1, response1.getInt("total")); + + verifySchema(response2, + schema("str2", null, "keyword"), + schema("position(str2 IN null)", null, "integer")); + assertEquals(1, response2.getInt("total")); + + verifyDataRows(response1, rows("one", null)); + verifyDataRows(response2, rows("one", null)); + } +} diff --git a/sql/src/main/antlr/OpenSearchSQLLexer.g4 b/sql/src/main/antlr/OpenSearchSQLLexer.g4 index df104d2a2a..422d23794b 100644 --- a/sql/src/main/antlr/OpenSearchSQLLexer.g4 +++ b/sql/src/main/antlr/OpenSearchSQLLexer.g4 @@ -234,6 +234,7 @@ NULLIF: 'NULLIF'; PERIOD_ADD: 'PERIOD_ADD'; PERIOD_DIFF: 'PERIOD_DIFF'; PI: 'PI'; +POSITION: 'POSITION'; POW: 'POW'; POWER: 'POWER'; RADIANS: 'RADIANS'; diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index b17c25261a..af8536d06b 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -313,6 +313,7 @@ functionCall | aggregateFunction (orderByClause)? filterClause #filteredAggregationFunctionCall | relevanceFunction #relevanceFunctionCall | highlightFunction #highlightFunctionCall + | positionFunction #positionFunctionCall ; @@ -320,6 +321,10 @@ highlightFunction : HIGHLIGHT LR_BRACKET relevanceField (COMMA highlightArg)* RR_BRACKET ; + positionFunction + : POSITION LR_BRACKET functionArg IN functionArg RR_BRACKET + ; + scalarFunctionName : mathematicalFunctionName | dateTimeFunctionName 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 18efef039f..1b4b3a2a54 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 @@ -70,6 +70,7 @@ import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; +import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.UnresolvedArgument; @@ -145,6 +146,13 @@ public UnresolvedExpression visitHighlightFunctionCall( builder.build()); } + @Override + public UnresolvedExpression visitPositionFunction( + OpenSearchSQLParser.PositionFunctionContext ctx) { + return new PositionFunction(visitFunctionArg(ctx.functionArg(0)), + visitFunctionArg(ctx.functionArg(1))); + } + @Override public UnresolvedExpression visitTableFilter(TableFilterContext ctx) { return new Function( 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 72358986e6..9170331e72 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 @@ -21,6 +21,7 @@ import static org.opensearch.sql.ast.dsl.AstDSL.not; import static org.opensearch.sql.ast.dsl.AstDSL.nullLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.or; +import static org.opensearch.sql.ast.dsl.AstDSL.position; import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.timeLiteral; @@ -328,6 +329,22 @@ public void canBuildQualifiedNameHighlightFunction() { ); } + @Test + public void canBuildPositionFunction() { + assertEquals( + position(AstDSL.qualifiedName("fieldA"), AstDSL.qualifiedName("fieldB")), + buildExprAst("position(fieldA IN fieldB)") + ); + } + + @Test + public void canBuildStringLiteralPositionFunction() { + assertEquals( + position(AstDSL.stringLiteral("fieldA"), AstDSL.stringLiteral("fieldB")), + buildExprAst("position(\"fieldA\" IN \"fieldB\")") + ); + } + @Test public void canBuildWindowFunctionWithoutOrderBy() { assertEquals( From b6d9a396b6ab6c3cdd9ad84b4e39521dd0ded6fd Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Wed, 30 Nov 2022 11:54:54 -0800 Subject: [PATCH 2/7] Address doctest failure Signed-off-by: Margarit Hakobyan --- docs/user/dql/functions.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 4c53f84b6b..8c7c37a3ef 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2346,7 +2346,7 @@ Return type integer: Example:: - os> SELECT POSITION('world' IN 'helloworld') + opensearchsql> SELECT POSITION('world' IN 'helloworld'), POSITION('invalid' IN 'helloworld'); fetched rows / total rows = 1/1 +-------------------------------------+---------------------------------------+ | POSITION('world' IN 'helloworld') | POSITION('invalid' IN 'helloworld') | From 52714ae835a230459854e4431e6b7e5b822543ea Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Wed, 30 Nov 2022 13:17:44 -0800 Subject: [PATCH 3/7] Address PR review feedback Signed-off-by: Margarit Hakobyan --- docs/user/dql/functions.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 8c7c37a3ef..64ab1629a5 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2346,7 +2346,7 @@ Return type integer: Example:: - opensearchsql> SELECT POSITION('world' IN 'helloworld'), POSITION('invalid' IN 'helloworld'); + os> SELECT POSITION('world' IN 'helloworld'), POSITION('invalid' IN 'helloworld'); fetched rows / total rows = 1/1 +-------------------------------------+---------------------------------------+ | POSITION('world' IN 'helloworld') | POSITION('invalid' IN 'helloworld') | From ee8cb7de72b2782c18cc5aceefe9ea6119d7ab3e Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Mon, 5 Dec 2022 12:35:34 -0800 Subject: [PATCH 4/7] Fixes after rebase Signed-off-by: Margarit Hakobyan --- core/src/main/java/org/opensearch/sql/expression/DSL.java | 2 +- docs/user/dql/functions.rst | 2 +- 2 files changed, 2 insertions(+), 2 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 c08851c635..fce37f913c 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -231,7 +231,7 @@ public static FunctionExpression cbrt(Expression... expressions) { } public static FunctionExpression position(Expression... expressions) { - return compile(BuiltinFunctionName.POSITION, expressions); + return compile(FunctionProperties.None, BuiltinFunctionName.POSITION, expressions); } public static FunctionExpression truncate(Expression... expressions) { diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 64ab1629a5..6512ab1c2f 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2338,7 +2338,7 @@ Description Usage: The syntax POSITION(substr IN str) returns the position of the first occurrence of substring substr in string str. Returns 0 if substr is not in str. Returns NULL if any argument is NULL. -Argument type: STRING, STRING, INTEGER +Argument type: STRING, STRING Return type integer: From 3786f04ae04c91df1d5befa728ea9f7b2eea6a48 Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Mon, 5 Dec 2022 14:33:44 -0800 Subject: [PATCH 5/7] Addressed PR review comments Signed-off-by: Margarit Hakobyan --- .../sql/analysis/ExpressionAnalyzer.java | 8 ---- .../sql/ast/AbstractNodeVisitor.java | 5 --- .../org/opensearch/sql/ast/dsl/AstDSL.java | 6 --- .../sql/ast/expression/PositionFunction.java | 39 ------------------- .../analysis/NamedExpressionAnalyzerTest.java | 11 ------ .../sql/sql/parser/AstExpressionBuilder.java | 8 ++-- .../sql/parser/AstExpressionBuilderTest.java | 13 +------ 7 files changed, 7 insertions(+), 83 deletions(-) delete mode 100644 core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.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 7ce4871634..719c3adbce 100644 --- a/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java +++ b/core/src/main/java/org/opensearch/sql/analysis/ExpressionAnalyzer.java @@ -34,7 +34,6 @@ import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; -import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.Span; @@ -202,13 +201,6 @@ public Expression visitHighlightFunction(HighlightFunction node, AnalysisContext return new HighlightExpression(expr); } - @Override - public Expression visitPositionFunction(PositionFunction node, AnalysisContext context) { - Expression stringPatternExpr = node.getStringPatternExpr().accept(this, context); - Expression searchStringExpr = node.getSearchStringExpr().accept(this, context); - return DSL.position(stringPatternExpr, searchStringExpr); - } - @Override public Expression visitIn(In node, AnalysisContext context) { return visitIn(node.getField(), node.getValueList(), context); 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 e806510074..fe993c899e 100644 --- a/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java +++ b/core/src/main/java/org/opensearch/sql/ast/AbstractNodeVisitor.java @@ -26,7 +26,6 @@ import org.opensearch.sql.ast.expression.Map; import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; -import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.Span; @@ -274,10 +273,6 @@ public T visitHighlightFunction(HighlightFunction node, C context) { return visitChildren(node, context); } - public T visitPositionFunction(PositionFunction node, C context) { - return visitChildren(node, context); - } - public T visitStatement(Statement node, C context) { return visit(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 a64a1fc246..2959cae4a1 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 @@ -32,7 +32,6 @@ import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; import org.opensearch.sql.ast.expression.ParseMethod; -import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.Span; import org.opensearch.sql.ast.expression.SpanUnit; @@ -284,11 +283,6 @@ public UnresolvedExpression highlight(UnresolvedExpression fieldName, return new HighlightFunction(fieldName, arguments); } - public UnresolvedExpression position(UnresolvedExpression stringPatternExpr, - UnresolvedExpression searchStringExpr) { - return new PositionFunction(stringPatternExpr, searchStringExpr); - } - public UnresolvedExpression window(UnresolvedExpression function, List partitionByList, List> sortList) { diff --git a/core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java b/core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java deleted file mode 100644 index 988237ebd0..0000000000 --- a/core/src/main/java/org/opensearch/sql/ast/expression/PositionFunction.java +++ /dev/null @@ -1,39 +0,0 @@ -/* - * 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.AllArgsConstructor; -import lombok.EqualsAndHashCode; -import lombok.Getter; -import lombok.ToString; -import org.opensearch.sql.ast.AbstractNodeVisitor; - - -/** - * Expression node of Position function. - */ -@AllArgsConstructor -@EqualsAndHashCode(callSuper = false) -@Getter -@ToString -public class PositionFunction extends UnresolvedExpression { - @Getter - private UnresolvedExpression stringPatternExpr; - @Getter - private UnresolvedExpression searchStringExpr; - - @Override - public List getChild() { - return Arrays.asList(stringPatternExpr, searchStringExpr); - } - - @Override - public R accept(AbstractNodeVisitor nodeVisitor, C context) { - return nodeVisitor.visitPositionFunction(this, context); - } -} diff --git a/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java index 2f97c6480e..913593add3 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/NamedExpressionAnalyzerTest.java @@ -16,7 +16,6 @@ import org.opensearch.sql.ast.expression.Alias; import org.opensearch.sql.ast.expression.HighlightFunction; import org.opensearch.sql.ast.expression.Literal; -import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.expression.NamedExpression; import org.springframework.context.annotation.Configuration; import org.springframework.test.context.ContextConfiguration; @@ -49,14 +48,4 @@ void visit_highlight() { NamedExpression analyze = analyzer.analyze(alias, analysisContext); assertEquals("highlight(fieldA)", analyze.getNameOrAlias()); } - - @Test - void visit_position() { - Alias alias = AstDSL.alias("position(fieldA IN fieldB)", - new PositionFunction(AstDSL.stringLiteral("fieldA"), AstDSL.stringLiteral("fieldB"))); - NamedExpressionAnalyzer analyzer = new NamedExpressionAnalyzer(expressionAnalyzer); - - NamedExpression analyze = analyzer.analyze(alias, analysisContext); - assertEquals("position(fieldA IN fieldB)", analyze.getNameOrAlias()); - } } 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 1b4b3a2a54..814b872c02 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_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.BinaryComparisonPredicateContext; import static org.opensearch.sql.sql.antlr.parser.OpenSearchSQLParser.BooleanContext; @@ -70,7 +71,6 @@ import org.opensearch.sql.ast.expression.Literal; import org.opensearch.sql.ast.expression.Not; import org.opensearch.sql.ast.expression.Or; -import org.opensearch.sql.ast.expression.PositionFunction; import org.opensearch.sql.ast.expression.QualifiedName; import org.opensearch.sql.ast.expression.RelevanceFieldList; import org.opensearch.sql.ast.expression.UnresolvedArgument; @@ -149,8 +149,10 @@ public UnresolvedExpression visitHighlightFunctionCall( @Override public UnresolvedExpression visitPositionFunction( OpenSearchSQLParser.PositionFunctionContext ctx) { - return new PositionFunction(visitFunctionArg(ctx.functionArg(0)), - visitFunctionArg(ctx.functionArg(1))); + return new Function( + POSITION.getName().getFunctionName(), + Arrays.asList(visitFunctionArg(ctx.functionArg(0)), + visitFunctionArg(ctx.functionArg(1)))); } @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 9170331e72..365de6e1b2 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 @@ -21,7 +21,6 @@ import static org.opensearch.sql.ast.dsl.AstDSL.not; import static org.opensearch.sql.ast.dsl.AstDSL.nullLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.or; -import static org.opensearch.sql.ast.dsl.AstDSL.position; import static org.opensearch.sql.ast.dsl.AstDSL.qualifiedName; import static org.opensearch.sql.ast.dsl.AstDSL.stringLiteral; import static org.opensearch.sql.ast.dsl.AstDSL.timeLiteral; @@ -329,19 +328,11 @@ public void canBuildQualifiedNameHighlightFunction() { ); } - @Test - public void canBuildPositionFunction() { - assertEquals( - position(AstDSL.qualifiedName("fieldA"), AstDSL.qualifiedName("fieldB")), - buildExprAst("position(fieldA IN fieldB)") - ); - } - @Test public void canBuildStringLiteralPositionFunction() { assertEquals( - position(AstDSL.stringLiteral("fieldA"), AstDSL.stringLiteral("fieldB")), - buildExprAst("position(\"fieldA\" IN \"fieldB\")") + function("position", stringLiteral("substr"), stringLiteral("str")), + buildExprAst("position(\"substr\" IN \"str\")") ); } From f4ee6d067e9e601c728fdf0d45dc8e0ed8d72080 Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Mon, 5 Dec 2022 15:06:36 -0800 Subject: [PATCH 6/7] minor fix Signed-off-by: Margarit Hakobyan --- sql/src/main/antlr/OpenSearchSQLParser.g4 | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index af8536d06b..73426bafa9 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -321,9 +321,9 @@ highlightFunction : HIGHLIGHT LR_BRACKET relevanceField (COMMA highlightArg)* RR_BRACKET ; - positionFunction - : POSITION LR_BRACKET functionArg IN functionArg RR_BRACKET - ; +positionFunction + : POSITION LR_BRACKET functionArg IN functionArg RR_BRACKET + ; scalarFunctionName : mathematicalFunctionName From 17bd91d4bf5ac4f6a3d7f95a949dd137a458c3df Mon Sep 17 00:00:00 2001 From: Margarit Hakobyan Date: Mon, 5 Dec 2022 19:56:48 -0800 Subject: [PATCH 7/7] Add more PR review feedback Signed-off-by: Margarit Hakobyan --- .../org/opensearch/sql/expression/text/TextFunction.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java b/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java index b51d6a2716..5915700bf1 100644 --- a/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/text/TextFunction.java @@ -251,7 +251,9 @@ private DefaultFunctionResolver locate() { */ private DefaultFunctionResolver position() { return define(BuiltinFunctionName.POSITION.getName(), - impl(nullMissingHandling(TextFunction::exprPosition), INTEGER, STRING, STRING)); + impl(nullMissingHandling( + (SerializableBiFunction) + TextFunction::exprLocate), INTEGER, STRING, STRING)); } /** @@ -326,10 +328,6 @@ private static ExprValue exprLocate(ExprValue subStr, ExprValue str, ExprValue p str.stringValue().indexOf(subStr.stringValue(), pos.integerValue() - 1) + 1); } - private static ExprValue exprPosition(ExprValue subStr, ExprValue str) { - return exprLocate(subStr, str); - } - private static ExprValue exprReplace(ExprValue str, ExprValue from, ExprValue to) { return new ExprStringValue(str.stringValue().replaceAll(from.stringValue(), to.stringValue())); }