From e7c45f56d9474e6286ab4aba40539f2fc9c172ad Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Fri, 22 Apr 2022 04:42:24 -0700 Subject: [PATCH 01/56] Support match_phrase in AST expressions Signed-off-by: MaxKsyunz --- .../org/opensearch/sql/expression/DSL.java | 8 +++ .../function/BuiltinFunctionName.java | 1 + .../function/OpenSearchFunctions.java | 49 +++++++++++++++++++ .../function/OpenSearchFunctionsTest.java | 34 +++++++++++-- 4 files changed, 88 insertions(+), 4 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 39aa1b8553..feb0e960af 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -654,4 +654,12 @@ public FunctionExpression match(Expression... args) { return (FunctionExpression) repository .compile(BuiltinFunctionName.MATCH.getName(), Arrays.asList(args.clone())); } + + public FunctionExpression match_phrase(NamedArgumentExpression ... args) { + return compile(BuiltinFunctionName.MATCH_PHRASE, args); + } + + private FunctionExpression compile(BuiltinFunctionName bfn, NamedArgumentExpression ... args) { + return (FunctionExpression) repository.compile(bfn.getName(), Arrays.asList(args.clone())); + } } 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 c52ff150cd..06a29d748e 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 @@ -187,6 +187,7 @@ public enum BuiltinFunctionName { * Relevance Function. */ MATCH(FunctionName.of("match")), + MATCH_PHRASE(FunctionName.of("match_phrase")), /** * Legacy Relevance Function. diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 9b7325bb59..3959d4f157 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -25,6 +25,7 @@ public class OpenSearchFunctions { public void register(BuiltinFunctionRepository repository) { repository.register(match()); + repository.register(match_phrase()); } private static FunctionResolver match() { @@ -75,6 +76,54 @@ private static FunctionResolver match() { .build()); } + private static FunctionResolver match_phrase() { + FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); + return new FunctionResolver(funcName, + ImmutableMap.builder() + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .build()); + } + private static class OpenSearchFunction extends FunctionExpression { private final FunctionName functionName; private final List arguments; diff --git a/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java index c425be704b..d9846e7e07 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java @@ -40,10 +40,14 @@ public class OpenSearchFunctionsTest extends ExpressionTestBase { "operator", DSL.literal("OR")); private final NamedArgumentExpression minimumShouldMatch = new NamedArgumentExpression( "minimum_should_match", DSL.literal("1")); - private final NamedArgumentExpression zeroTermsQuery = new NamedArgumentExpression( - "zero_terms_query", DSL.literal("ALL")); + private final NamedArgumentExpression zeroTermsQueryAll = new NamedArgumentExpression( + "zero_terms_query", DSL.literal("ALL")); + private final NamedArgumentExpression zeroTermsQueryNone = new NamedArgumentExpression( + "zero_terms_query", DSL.literal("None")); private final NamedArgumentExpression boost = new NamedArgumentExpression( "boost", DSL.literal("2.0")); + private final NamedArgumentExpression slop = new NamedArgumentExpression( + "slop", DSL.literal("3")); @Test void match() { @@ -98,16 +102,38 @@ void match() { expr = dsl.match( field, query, analyzer, autoGenerateSynonymsPhrase, fuzziness, maxExpansions, prefixLength, - fuzzyTranspositions, fuzzyRewrite, lenient, operator, minimumShouldMatch, zeroTermsQuery); + fuzzyTranspositions, fuzzyRewrite, lenient, operator, minimumShouldMatch, zeroTermsQueryAll); assertEquals(BOOLEAN, expr.type()); expr = dsl.match( field, query, analyzer, autoGenerateSynonymsPhrase, fuzziness, maxExpansions, prefixLength, - fuzzyTranspositions, fuzzyRewrite, lenient, operator, minimumShouldMatch, zeroTermsQuery, + fuzzyTranspositions, fuzzyRewrite, lenient, operator, minimumShouldMatch, zeroTermsQueryAll, boost); assertEquals(BOOLEAN, expr.type()); } + @Test + void match_phrase() { + { + FunctionExpression expr = dsl.match_phrase(field, query); + assertEquals(BOOLEAN, expr.type()); + } + { + FunctionExpression expr = dsl.match_phrase(field, query, zeroTermsQueryAll); + assertEquals(BOOLEAN, expr.type()); + } + + { + FunctionExpression expr = dsl.match_phrase(field, query, zeroTermsQueryNone); + assertEquals(BOOLEAN, expr.type()); + } + + { + FunctionExpression expr = dsl.match_phrase(field, query, slop); + assertEquals(BOOLEAN, expr.type()); + } + } + @Test void match_in_memory() { FunctionExpression expr = dsl.match(field, query); From a39f42318abe846a4eb4b558739af3ecdd70d807 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 25 Apr 2022 18:23:55 -0700 Subject: [PATCH 02/56] Support match_phrase in PPL syntax Signed-off-by: MaxKsyunz --- ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 1 + ppl/src/main/antlr/OpenSearchPPLParser.g4 | 1 + .../org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java | 7 +++++++ 3 files changed, 9 insertions(+) diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index aee51d0a10..b768b5535b 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -264,6 +264,7 @@ IF: 'IF'; // RELEVANCE FUNCTIONS AND PARAMETERS MATCH: 'MATCH'; +MATCH_PHRASE: 'MATCH_PHRASE'; ANALYZER: 'ANALYZER'; FUZZINESS: 'FUZZINESS'; AUTO_GENERATE_SYNONYMS_PHRASE_QUERY:'AUTO_GENERATE_SYNONYMS_PHRASE_QUERY'; diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 100547ae7a..967f4d13b3 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -351,6 +351,7 @@ binaryOperator relevanceFunctionName : MATCH + | MATCH_PHRASE ; /** literals and values*/ diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index 3d5f8d453c..064de5087f 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -79,4 +79,11 @@ public void testTopCommandWithoutNAndGroupByShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top a by b"); assertNotEquals(null, tree); } + + @Test + public void testMatchPhraseNoOptionsShouldPass() { + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a= 1 | where match_phrase(a, 'hello world')"); + assertNotEquals(null, tree); + } + } From 8d0cd91aa37bf6fb3eec19855ecdb5de1a6a0913 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Mon, 25 Apr 2022 20:09:58 -0700 Subject: [PATCH 03/56] Added MATCH_QUERY to the list of supported functions in SQL parser. Signed-off-by: Yury Fridlyand --- sql/src/main/antlr/OpenSearchSQLParser.g4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index 0b8f3c5250..c8058bba2a 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -383,7 +383,7 @@ flowControlFunctionName ; relevanceFunctionName - : MATCH + : MATCH | MATCH_QUERY ; legacyRelevanceFunctionName From 7d451e53f609c45c928ac595a6ff97f74567ff0b Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 25 Apr 2022 22:44:05 -0700 Subject: [PATCH 04/56] Support slop parameter for relevancy functions Signed-off-by: MaxKsyunz --- ppl/src/main/antlr/OpenSearchPPLLexer.g4 | 2 +- .../org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 index b768b5535b..be4ad272d1 100644 --- a/ppl/src/main/antlr/OpenSearchPPLLexer.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLLexer.g4 @@ -277,7 +277,7 @@ OPERATOR: 'OPERATOR'; MINIMUM_SHOULD_MATCH: 'MINIMUM_SHOULD_MATCH'; ZERO_TERMS_QUERY: 'ZERO_TERMS_QUERY'; BOOST: 'BOOST'; - +SLOP: 'SLOP'; // SPAN KEYWORDS SPAN: 'SPAN'; MS: 'MS'; diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index 064de5087f..60b16d6b79 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -86,4 +86,9 @@ public void testMatchPhraseNoOptionsShouldPass() { assertNotEquals(null, tree); } + @Test + public void testMatchPhraseWithSlopShouldPass() { + ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a = 1 | where match_phrase(a, 'hello world', slop = 3"); + assertNotEquals(null, tree); + } } From 7001b87b9628b997d0edd1bd1821e04ffab91c3b Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Tue, 26 Apr 2022 11:42:37 -0700 Subject: [PATCH 05/56] Typo fix for previous commit. Expanded list of MATCH_* function arguments. Signed-off-by: Yury Fridlyand --- sql/src/main/antlr/OpenSearchSQLLexer.g4 | 26 +++++++++++++++++------ sql/src/main/antlr/OpenSearchSQLParser.g4 | 10 +++++---- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/sql/src/main/antlr/OpenSearchSQLLexer.g4 b/sql/src/main/antlr/OpenSearchSQLLexer.g4 index ed2d7e8160..430ca3a045 100644 --- a/sql/src/main/antlr/OpenSearchSQLLexer.g4 +++ b/sql/src/main/antlr/OpenSearchSQLLexer.g4 @@ -310,18 +310,32 @@ STRCMP: 'STRCMP'; ADDDATE: 'ADDDATE'; // RELEVANCE FUNCTIONS AND PARAMETERS +ALLOW_LEADING_WILDCARD: 'ALLOW_LEADING_WILDCARD'; +ANALYZE_WILDCARD: 'ANALYZE_WILDCARD'; ANALYZER: 'ANALYZER'; -FUZZINESS: 'FUZZINESS'; AUTO_GENERATE_SYNONYMS_PHRASE_QUERY:'AUTO_GENERATE_SYNONYMS_PHRASE_QUERY'; -MAX_EXPANSIONS: 'MAX_EXPANSIONS'; -PREFIX_LENGTH: 'PREFIX_LENGTH'; +BOOST: 'BOOST'; +CUTOFF_FREQUENCY: 'CUTOFF_FREQUENCY'; +ENABLE_POSITION_INCREMENTS: 'ENABLE_POSITION_INCREMENTS'; +FIELDS: 'FIELDS'; +FLAGS: 'FLAGS'; +FUZZINESS: 'FUZZINESS'; FUZZY_TRANSPOSITIONS: 'FUZZY_TRANSPOSITIONS'; -FUZZY_REWRITE: 'FUZZY_REWRITE'; LENIENT: 'LENIENT'; -OPERATOR: 'OPERATOR'; +LOW_FREQ_OPERATOR: 'LOW_FREQ_OPERATOR'; +MAX_DETERMINIZED_STATES: 'MAX_DETERMINIZED_STATES'; +MAX_EXPANSIONS: 'MAX_EXPANSIONS'; MINIMUM_SHOULD_MATCH: 'MINIMUM_SHOULD_MATCH'; +OPERATOR: 'OPERATOR'; +PHRASE_SLOP: 'PHRASE_SLOP'; +PREFIX_LENGTH: 'PREFIX_LENGTH'; +QUOTE_FIELD_SUFFIX: 'QUOTE_FIELD_SUFFIX'; +REWRITE: 'REWRITE'; +SLOP: 'SLOP'; +TIE_BREAKER: 'TIE_BREAKER'; +TIME_ZONE: 'TIME_ZONE'; +TYPE: 'TYPE'; ZERO_TERMS_QUERY: 'ZERO_TERMS_QUERY'; -BOOST: 'BOOST'; // Operators diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index c8058bba2a..f1fef47f45 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -383,7 +383,7 @@ flowControlFunctionName ; relevanceFunctionName - : MATCH | MATCH_QUERY + : MATCH | MATCH_PHRASE ; legacyRelevanceFunctionName @@ -403,9 +403,11 @@ relevanceArg ; relevanceArgName - : ANALYZER | FUZZINESS | AUTO_GENERATE_SYNONYMS_PHRASE_QUERY | MAX_EXPANSIONS | PREFIX_LENGTH - | FUZZY_TRANSPOSITIONS | FUZZY_REWRITE | LENIENT | OPERATOR | MINIMUM_SHOULD_MATCH | ZERO_TERMS_QUERY - | BOOST + : ALLOW_LEADING_WILDCARD | ANALYZE_WILDCARD | ANALYZER | AUTO_GENERATE_SYNONYMS_PHRASE_QUERY | BOOST + | CUTOFF_FREQUENCY | ENABLE_POSITION_INCREMENTS | FIELDS | FLAGS | FUZZINESS | FUZZY_TRANSPOSITIONS + | LENIENT | LOW_FREQ_OPERATOR | MAX_DETERMINIZED_STATES | MAX_EXPANSIONS | MINIMUM_SHOULD_MATCH + | OPERATOR | PHRASE_SLOP | PREFIX_LENGTH | QUOTE_FIELD_SUFFIX | REWRITE | SLOP | TIE_BREAKER | TIME_ZONE + | TYPE | ZERO_TERMS_QUERY ; relevanceArgValue From f0ff81c59469ca1a2df8eec959397fb98d181859 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 26 Apr 2022 11:54:08 -0700 Subject: [PATCH 06/56] Integration tests for match_test in PPL Added a couple test for match_test in PPL. The tests are currently @Ignore'd until more of the issue is complete. Signed-off-by: MaxKsyunz --- .../opensearch/sql/ppl/WhereCommandIT.java | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java index 5415b6e286..deeb19f870 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java @@ -6,14 +6,15 @@ package org.opensearch.sql.ppl; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; -import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES; +import static org.opensearch.sql.legacy.TestsConstants.*; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; import java.io.IOException; import org.json.JSONObject; +import org.junit.Ignore; +import org.junit.jupiter.api.Disabled; +import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; public class WhereCommandIT extends PPLIntegTestCase { @@ -23,6 +24,7 @@ public void init() throws IOException { loadIndex(Index.ACCOUNT); loadIndex(Index.BANK_WITH_NULL_VALUES); loadIndex(Index.BANK); + loadIndex(Index.GAME_OF_THRONES); } @Test @@ -110,4 +112,24 @@ public void testRelevanceFunction() throws IOException { TEST_INDEX_BANK)); verifyDataRows(result, rows("Hattie")); } + + @Test + @Ignore("Not supported yet") + public void testMatchPhraseFunction() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | where match_phrase(firstname, 'Hattie') | fields firstname", TEST_INDEX_BANK)); + verifyDataRows(result, rows("Hattie")); + } + + @Test + @Ignore("Not supported yet") + public void testMathPhraseWithSlop() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | where match_phrase(firstname, 'Hattie', slop = 2) | fields firstname", TEST_INDEX_BANK)); + verifyDataRows(result, rows("Hattie")); + } } From b883254f9d62b4118c8f457d234f014f978b2410 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 26 Apr 2022 12:08:22 -0700 Subject: [PATCH 07/56] Support slop option Signed-off-by: MaxKsyunz --- ppl/src/main/antlr/OpenSearchPPLParser.g4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index 967f4d13b3..f44db2f834 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -305,7 +305,7 @@ relevanceArg relevanceArgName : ANALYZER | FUZZINESS | AUTO_GENERATE_SYNONYMS_PHRASE_QUERY | MAX_EXPANSIONS | PREFIX_LENGTH | FUZZY_TRANSPOSITIONS | FUZZY_REWRITE | LENIENT | OPERATOR | MINIMUM_SHOULD_MATCH | ZERO_TERMS_QUERY - | BOOST + | BOOST | SLOP ; relevanceArgValue From e1c071a46c32419b7e30bb88f8eb348b0c99f65a Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 26 Apr 2022 12:08:58 -0700 Subject: [PATCH 08/56] Tests to verify that PPL parser supports match_phrase Signed-off-by: MaxKsyunz --- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index 60b16d6b79..b93604a508 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -87,8 +87,16 @@ public void testMatchPhraseNoOptionsShouldPass() { } @Test - public void testMatchPhraseWithSlopShouldPass() { - ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a = 1 | where match_phrase(a, 'hello world', slop = 3"); - assertNotEquals(null, tree); + public void testMatchPhraseVariantsShouldPass() { + String[] validQueries = { + "source=t a = 1 | where match_phrase(a, 'hello world', slop = 3)", + "source=t a = 1 | where match_phrase(a, 'hello world', analyzer = 'standard', zero_terms_query = 'none', slop = 3)", + "source=t a = 1 | where match_phrase(a, 'hello world', zero_terms_query = all)" + }; + + for (String query : validQueries) { + ParseTree tree = new PPLSyntaxParser().analyzeSyntax(query); + assertNotEquals(null, tree); + } } } From 9d4f37896112e39897c49f3bd946c35ad672a985 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 26 Apr 2022 12:25:29 -0700 Subject: [PATCH 09/56] Refactor and style fixes Signed-off-by: MaxKsyunz --- core/src/main/java/org/opensearch/sql/expression/DSL.java | 7 +++---- 1 file changed, 3 insertions(+), 4 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 feb0e960af..a7348aaa3a 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -651,15 +651,14 @@ public FunctionExpression castDatetime(Expression value) { } public FunctionExpression match(Expression... args) { - return (FunctionExpression) repository - .compile(BuiltinFunctionName.MATCH.getName(), Arrays.asList(args.clone())); + return compile(BuiltinFunctionName.MATCH, args); } - public FunctionExpression match_phrase(NamedArgumentExpression ... args) { + public FunctionExpression match_phrase(Expression... args) { return compile(BuiltinFunctionName.MATCH_PHRASE, args); } - private FunctionExpression compile(BuiltinFunctionName bfn, NamedArgumentExpression ... args) { + private FunctionExpression compile(BuiltinFunctionName bfn, Expression... args) { return (FunctionExpression) repository.compile(bfn.getName(), Arrays.asList(args.clone())); } } From 3c8bd627317cf2965e4403050d3ac43a0f918838 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 26 Apr 2022 12:25:57 -0700 Subject: [PATCH 10/56] Refactor and fix style in OpenSearchFunctions.java Signed-off-by: MaxKsyunz --- .../function/OpenSearchFunctions.java | 135 +++++++----------- 1 file changed, 48 insertions(+), 87 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 3959d4f157..2fbac96967 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -30,98 +30,59 @@ public void register(BuiltinFunctionRepository repository) { private static FunctionResolver match() { FunctionName funcName = BuiltinFunctionName.MATCH.getName(); - return new FunctionResolver(funcName, - ImmutableMap.builder() - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .build()); + return getFunctionResolver(funcName); } private static FunctionResolver match_phrase() { FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); + return getFunctionResolver(funcName); + } + + private static FunctionResolver getFunctionResolver(FunctionName funcName) { return new FunctionResolver(funcName, - ImmutableMap.builder() - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .build()); + ImmutableMap.builder() + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, + STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .build()); } private static class OpenSearchFunction extends FunctionExpression { From 4a0fe8b2243afe7cdf06b5d2a1499b80bd4220be Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 26 Apr 2022 12:31:20 -0700 Subject: [PATCH 11/56] Style fixes for OpenSearchFunctionsTest.java Signed-off-by: MaxKsyunz --- .../sql/expression/function/OpenSearchFunctionsTest.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java index d9846e7e07..646128a0f4 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java @@ -47,7 +47,7 @@ public class OpenSearchFunctionsTest extends ExpressionTestBase { private final NamedArgumentExpression boost = new NamedArgumentExpression( "boost", DSL.literal("2.0")); private final NamedArgumentExpression slop = new NamedArgumentExpression( - "slop", DSL.literal("3")); + "slop", DSL.literal("3")); @Test void match() { @@ -102,7 +102,8 @@ void match() { expr = dsl.match( field, query, analyzer, autoGenerateSynonymsPhrase, fuzziness, maxExpansions, prefixLength, - fuzzyTranspositions, fuzzyRewrite, lenient, operator, minimumShouldMatch, zeroTermsQueryAll); + fuzzyTranspositions, fuzzyRewrite, lenient, operator, minimumShouldMatch, + zeroTermsQueryAll); assertEquals(BOOLEAN, expr.type()); expr = dsl.match( From 659c4d70bdfe5aac643f52700a360558495d5025 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Tue, 26 Apr 2022 13:06:21 -0700 Subject: [PATCH 12/56] Add tests for `MATCH` and `MATCH_PHRASE` functions. Signed-off-by: Yury Fridlyand --- .../sql/sql/antlr/SQLSyntaxParserTest.java | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index e7cb22e8a2..7189f07cb6 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -9,9 +9,14 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.opensearch.sql.common.antlr.SyntaxCheckException; +import java.util.*; +import java.util.stream.Collectors; +import java.util.stream.Stream; + class SQLSyntaxParserTest { private final SQLSyntaxParser parser = new SQLSyntaxParser(); @@ -144,4 +149,79 @@ public void canNotParseShowStatementWithoutFilterClause() { assertThrows(SyntaxCheckException.class, () -> parser.parse("SHOW TABLES")); } + @Test + public void canParseRelevanceFunctions() { + assertNotNull(parser.parse("SELECT * FROM test WHERE match(column, \"this is a test\")")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match(column, 'this is a test')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match(`column`, \"this is a test\")")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match(`column`, 'this is a test')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match(column, 100500)")); + + assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(column, \"this is a test\")")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(column, 'this is a test')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(`column`, \"this is a test\")")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(`column`, 'this is a test')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(column, 100500)")); + } + + private void generateAndTestQuery(String function, HashMap functionArgs) { + var rand = new Random(); + + for (int i = 0; i < 100; i++) + { + StringBuilder query = new StringBuilder(); + query.append(String.format("SELECT * FROM test WHERE %s(%s, %s", function, + RandomStringUtils.random(10, true, false), + RandomStringUtils.random(10, true, false))); + var args = new ArrayList(); + for (var pair : functionArgs.entrySet()) + { + if (rand.nextBoolean()) + { + var arg = new StringBuilder(); + arg.append(rand.nextBoolean() ? "," : ", "); + arg.append(rand.nextBoolean() ? pair.getKey().toLowerCase() : pair.getKey().toUpperCase()); + arg.append(rand.nextBoolean() ? "=" : " = "); + var quoteSymbol = rand.nextBoolean() ? '\'' : '"'; + arg.append(quoteSymbol); + arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); + arg.append(quoteSymbol); + args.add(arg.toString()); + } + } + Collections.shuffle(args); + for (var arg : args) + query.append(arg); + query.append(rand.nextBoolean() ? ")" : ");"); + //System.out.printf("%d, %s%n", i, query.toString()); + assertNotNull(parser.parse(query.toString())); + } + } + + // TODO run all tests and collect exceptions and raise them in the end + @Test + public void canParseRelevanceFunctionsComplexRandomArgs() { + var matchArgs = new HashMap(); + matchArgs.put("fuzziness", new String[]{ "AUTO", "AUTO:1,5", "1" }); + matchArgs.put("fuzzy_transpositions", new Boolean[]{ true, false }); + matchArgs.put("operator", new String[]{ "and", "or" }); + matchArgs.put("minimum_should_match", new String[]{ "3", "-2", "75%", "-25%", "3<90%", "2<-25% 9<-3" }); + matchArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); + matchArgs.put("zero_terms_query", new String[]{ "none", "all" }); + matchArgs.put("lenient", new Boolean[]{ true, false }); + // deprecated + matchArgs.put("cutoff_frequency", new Double[]{ .0, 0.001, 1., 42. }); + matchArgs.put("prefix_length", new Integer[]{ 0, 2, 5 }); + matchArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); + matchArgs.put("boost", new Double[]{ .5, 1., 2.3 }); + + generateAndTestQuery("match", matchArgs); + + var matchPhraseArgs = new HashMap(); + matchPhraseArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); + matchPhraseArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); + matchPhraseArgs.put("slop", new Integer[]{ 0, 1, 2 }); + + generateAndTestQuery("match_phase", matchPhraseArgs); + } } From 017fbb701a212867f4b4fc9bda160d4a714b4114 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Tue, 26 Apr 2022 13:13:22 -0700 Subject: [PATCH 13/56] Typo fix. Signed-off-by: Yury Fridlyand --- .../java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index 7189f07cb6..2f63d62160 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -222,6 +222,6 @@ public void canParseRelevanceFunctionsComplexRandomArgs() { matchPhraseArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); matchPhraseArgs.put("slop", new Integer[]{ 0, 1, 2 }); - generateAndTestQuery("match_phase", matchPhraseArgs); + generateAndTestQuery("match_phrase", matchPhraseArgs); } } From e93aa2bd82c09eab79583b1f0177d6c32c03f0c3 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 26 Apr 2022 13:15:50 -0700 Subject: [PATCH 14/56] Fix style and minor refactor in PPLSyntaxParserTest.java Signed-off-by: MaxKsyunz --- .../sql/ppl/antlr/PPLSyntaxParserTest.java | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index b93604a508..f8a6ac4d2d 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -80,18 +80,14 @@ public void testTopCommandWithoutNAndGroupByShouldPass() { assertNotEquals(null, tree); } - @Test - public void testMatchPhraseNoOptionsShouldPass() { - ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a= 1 | where match_phrase(a, 'hello world')"); - assertNotEquals(null, tree); - } - @Test public void testMatchPhraseVariantsShouldPass() { String[] validQueries = { - "source=t a = 1 | where match_phrase(a, 'hello world', slop = 3)", - "source=t a = 1 | where match_phrase(a, 'hello world', analyzer = 'standard', zero_terms_query = 'none', slop = 3)", - "source=t a = 1 | where match_phrase(a, 'hello world', zero_terms_query = all)" + "source=t a= 1 | where match_phrase(a, 'hello world')", + "source=t a = 1 | where match_phrase(a, 'hello world', slop = 3)", + "source=t a = 1 | where match_phrase(a, 'hello world', analyzer = 'standard'," + + "zero_terms_query = 'none', slop = 3)", + "source=t a = 1 | where match_phrase(a, 'hello world', zero_terms_query = all)" }; for (String query : validQueries) { From f67b1ac4fa85f1a26ccea0f951ab76213b6fa4f7 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Tue, 26 Apr 2022 14:49:49 -0700 Subject: [PATCH 15/56] Minor test improvements Signed-off-by: Yury Fridlyand --- .../sql/sql/antlr/SQLSyntaxParserTest.java | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index 2f63d62160..0d4bc03d2f 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -182,10 +182,14 @@ private void generateAndTestQuery(String function, HashMap fun arg.append(rand.nextBoolean() ? "," : ", "); arg.append(rand.nextBoolean() ? pair.getKey().toLowerCase() : pair.getKey().toUpperCase()); arg.append(rand.nextBoolean() ? "=" : " = "); - var quoteSymbol = rand.nextBoolean() ? '\'' : '"'; - arg.append(quoteSymbol); - arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); - arg.append(quoteSymbol); + if (pair.getValue() instanceof String[] || rand.nextBoolean()) { + var quoteSymbol = rand.nextBoolean() ? '\'' : '"'; + arg.append(quoteSymbol); + arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); + arg.append(quoteSymbol); + } + else + arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); args.add(arg.toString()); } } From 0e489925b3831b7c1ef2fbc65d4c83ca23548ff3 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 27 Apr 2022 18:08:53 -0700 Subject: [PATCH 16/56] Parameterized test for match_phrase parsing Create a parametrized test for PPLSyntax parser. Signed-off-by: MaxKsyunz --- ...PPLSyntaxParserMatchPhraseSamplesTest.java | 40 +++++++++++++++++++ .../sql/ppl/antlr/PPLSyntaxParserTest.java | 17 +------- 2 files changed, 41 insertions(+), 16 deletions(-) create mode 100644 ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java new file mode 100644 index 0000000000..ee410690b3 --- /dev/null +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java @@ -0,0 +1,40 @@ +package org.opensearch.sql.ppl.antlr; + +import static org.junit.Assert.assertNotEquals; + +import java.util.List; +import org.antlr.v4.runtime.tree.ParseTree; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + + +@RunWith(Parameterized.class) +public class PPLSyntaxParserMatchPhraseSamplesTest { + + + /** Returns sample queries that the PPLSyntaxParser is expected to parse successfully. + * @return an Iterable of sample queries. + */ + @Parameterized.Parameters(name = "{0}") + public static Iterable sampleQueries() { + return List.of( + "source=t a= 1 | where match_phrase(a, 'hello world')", + "source=t a = 1 | where match_phrase(a, 'hello world', slop = 3)", + "source=t a = 1 | where match_phrase(a, 'hello world', analyzer = 'standard'," + + "zero_terms_query = 'none', slop = 3)", + "source=t a = 1 | where match_phrase(a, 'hello world', zero_terms_query = all)"); + } + + private final String query; + + public PPLSyntaxParserMatchPhraseSamplesTest(String query) { + this.query = query; + } + + @Test + public void test() { + ParseTree tree = new PPLSyntaxParser().analyzeSyntax(query); + assertNotEquals(null, tree); + } +} diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java index f8a6ac4d2d..89f608ebe5 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserTest.java @@ -79,20 +79,5 @@ public void testTopCommandWithoutNAndGroupByShouldPass() { ParseTree tree = new PPLSyntaxParser().analyzeSyntax("source=t a=1 | top a by b"); assertNotEquals(null, tree); } - - @Test - public void testMatchPhraseVariantsShouldPass() { - String[] validQueries = { - "source=t a= 1 | where match_phrase(a, 'hello world')", - "source=t a = 1 | where match_phrase(a, 'hello world', slop = 3)", - "source=t a = 1 | where match_phrase(a, 'hello world', analyzer = 'standard'," - + "zero_terms_query = 'none', slop = 3)", - "source=t a = 1 | where match_phrase(a, 'hello world', zero_terms_query = all)" - }; - - for (String query : validQueries) { - ParseTree tree = new PPLSyntaxParser().analyzeSyntax(query); - assertNotEquals(null, tree); - } - } } + From cc2100b8462a2d4937bf9fc66028dcc4b39351d8 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Wed, 27 Apr 2022 13:02:57 -0700 Subject: [PATCH 17/56] Add `MATCH_PHRASE` function to the function repository. Signed-off-by: Yury Fridlyand --- .../java/org/opensearch/sql/expression/DSL.java | 5 +++++ .../expression/function/OpenSearchFunctions.java | 16 ++++++++++++++++ .../script/filter/FilterQueryBuilder.java | 1 + 3 files changed, 22 insertions(+) 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 a7348aaa3a..224dafe7aa 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -661,4 +661,9 @@ public FunctionExpression match_phrase(Expression... args) { private FunctionExpression compile(BuiltinFunctionName bfn, Expression... args) { return (FunctionExpression) repository.compile(bfn.getName(), Arrays.asList(args.clone())); } + + public FunctionExpression match_phrase(Expression... args) { + return (FunctionExpression) repository + .compile(BuiltinFunctionName.MATCH_PHRASE.getName(), Arrays.asList(args.clone())); + } } diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 2fbac96967..1c0bbbf4e8 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -85,6 +85,22 @@ private static FunctionResolver getFunctionResolver(FunctionName funcName) { .build()); } + private static FunctionResolver match_phrase() { + FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); + return new FunctionResolver(funcName, + ImmutableMap.builder() + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .put(new FunctionSignature(funcName, ImmutableList + .of(STRING, STRING, STRING, STRING, STRING)), + args -> new OpenSearchFunction(funcName, args)) + .build()); + } + private static class OpenSearchFunction extends FunctionExpression { private final FunctionName functionName; private final List arguments; diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java index 94179c3369..07f68f1998 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java @@ -52,6 +52,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor Date: Thu, 28 Apr 2022 12:06:09 -0700 Subject: [PATCH 18/56] Finally added `MATCH_PHASE` query support to `FilterQueryBuilder`. Signed-off-by: Yury Fridlyand --- .../script/filter/FilterQueryBuilder.java | 3 +- .../lucene/relevance/MatchPhraseQuery.java | 55 +++++++++++++++++++ 2 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java index 07f68f1998..9aa8e1000b 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java @@ -29,6 +29,7 @@ import org.opensearch.sql.opensearch.storage.script.filter.lucene.RangeQuery.Comparison; import org.opensearch.sql.opensearch.storage.script.filter.lucene.TermQuery; import org.opensearch.sql.opensearch.storage.script.filter.lucene.WildcardQuery; +import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchPhraseQuery; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchQuery; import org.opensearch.sql.opensearch.storage.serialization.ExpressionSerializer; @@ -52,7 +53,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor analyzer = + (b, v) -> b.analyzer(v.stringValue()); + private final BiFunction slop = + (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue())); + private final BiFunction zeroTermsQuery = + (b, v) -> b.zeroTermsQuery( + org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); + + ImmutableMap argAction = ImmutableMap.builder() + .put("analyzer", analyzer) + .put("slop", slop) + .put("zero_terms_query", zeroTermsQuery) + .build(); + + @Override + public QueryBuilder build(FunctionExpression func) { + Iterator iterator = func.getArguments().iterator(); + NamedArgumentExpression field = (NamedArgumentExpression) iterator.next(); + NamedArgumentExpression query = (NamedArgumentExpression) iterator.next(); + MatchPhraseQueryBuilder queryBuilder = QueryBuilders.matchPhraseQuery( + field.getValue().valueOf(null).stringValue(), + query.getValue().valueOf(null).stringValue()); + while (iterator.hasNext()) { + NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); + if (!argAction.containsKey(arg.getArgName())) { + throw new SemanticCheckException(String + .format("Parameter %s is invalid for match function.", arg.getArgName())); + } + ((BiFunction) argAction + .get(arg.getArgName())) + .apply(queryBuilder, arg.getValue().valueOf(null)); + } + return queryBuilder; + } +} From 775cb20ee56f4fa6cac2b3d031403288228f434e Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Thu, 28 Apr 2022 13:07:27 -0700 Subject: [PATCH 19/56] Typo fix + EOL fix Signed-off-by: Yury Fridlyand --- .../lucene/relevance/MatchPhraseQuery.java | 110 +++++++++--------- 1 file changed, 55 insertions(+), 55 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java index fc8256330e..6dbaa315ac 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java @@ -1,55 +1,55 @@ -/* - * Copyright OpenSearch Contributors - * SPDX-License-Identifier: Apache-2.0 - */ - -package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; - -import com.google.common.collect.ImmutableMap; -import java.util.Iterator; -import java.util.function.BiFunction; - -import org.opensearch.index.query.*; -import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.exception.SemanticCheckException; -import org.opensearch.sql.expression.Expression; -import org.opensearch.sql.expression.FunctionExpression; -import org.opensearch.sql.expression.NamedArgumentExpression; -import org.opensearch.sql.opensearch.storage.script.filter.lucene.LuceneQuery; - -public class MatchPhraseQuery extends LuceneQuery { - private final BiFunction analyzer = - (b, v) -> b.analyzer(v.stringValue()); - private final BiFunction slop = - (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue())); - private final BiFunction zeroTermsQuery = - (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); - - ImmutableMap argAction = ImmutableMap.builder() - .put("analyzer", analyzer) - .put("slop", slop) - .put("zero_terms_query", zeroTermsQuery) - .build(); - - @Override - public QueryBuilder build(FunctionExpression func) { - Iterator iterator = func.getArguments().iterator(); - NamedArgumentExpression field = (NamedArgumentExpression) iterator.next(); - NamedArgumentExpression query = (NamedArgumentExpression) iterator.next(); - MatchPhraseQueryBuilder queryBuilder = QueryBuilders.matchPhraseQuery( - field.getValue().valueOf(null).stringValue(), - query.getValue().valueOf(null).stringValue()); - while (iterator.hasNext()) { - NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); - if (!argAction.containsKey(arg.getArgName())) { - throw new SemanticCheckException(String - .format("Parameter %s is invalid for match function.", arg.getArgName())); - } - ((BiFunction) argAction - .get(arg.getArgName())) - .apply(queryBuilder, arg.getValue().valueOf(null)); - } - return queryBuilder; - } -} +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; + +import com.google.common.collect.ImmutableMap; +import java.util.Iterator; +import java.util.function.BiFunction; + +import org.opensearch.index.query.*; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.NamedArgumentExpression; +import org.opensearch.sql.opensearch.storage.script.filter.lucene.LuceneQuery; + +public class MatchPhraseQuery extends LuceneQuery { + private final BiFunction analyzer = + (b, v) -> b.analyzer(v.stringValue()); + private final BiFunction slop = + (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue())); + private final BiFunction zeroTermsQuery = + (b, v) -> b.zeroTermsQuery( + org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); + + ImmutableMap argAction = ImmutableMap.builder() + .put("analyzer", analyzer) + .put("slop", slop) + .put("zero_terms_query", zeroTermsQuery) + .build(); + + @Override + public QueryBuilder build(FunctionExpression func) { + Iterator iterator = func.getArguments().iterator(); + NamedArgumentExpression field = (NamedArgumentExpression) iterator.next(); + NamedArgumentExpression query = (NamedArgumentExpression) iterator.next(); + MatchPhraseQueryBuilder queryBuilder = QueryBuilders.matchPhraseQuery( + field.getValue().valueOf(null).stringValue(), + query.getValue().valueOf(null).stringValue()); + while (iterator.hasNext()) { + NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); + if (!argAction.containsKey(arg.getArgName())) { + throw new SemanticCheckException(String + .format("Parameter %s is invalid for match_phrase function.", arg.getArgName())); + } + ((BiFunction) argAction + .get(arg.getArgName())) + .apply(queryBuilder, arg.getValue().valueOf(null)); + } + return queryBuilder; + } +} From 2aaf04715163797e6d14fe9be492d973111f2eed Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Thu, 28 Apr 2022 17:03:17 -0700 Subject: [PATCH 20/56] Typo fix Signed-off-by: Yury Fridlyand --- .../script/filter/lucene/relevance/MatchPhraseQuery.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java index 6dbaa315ac..46a6379fb5 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java @@ -20,8 +20,8 @@ public class MatchPhraseQuery extends LuceneQuery { private final BiFunction analyzer = (b, v) -> b.analyzer(v.stringValue()); - private final BiFunction slop = - (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue())); + private final BiFunction slop = + (b, v) -> b.slop(Integer.parseInt(v.stringValue())); private final BiFunction zeroTermsQuery = (b, v) -> b.zeroTermsQuery( org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); From 2316173f402b85014219b34a651cb83d38f9907e Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Thu, 28 Apr 2022 17:04:21 -0700 Subject: [PATCH 21/56] Adding tests for `FilterQueryBuilder` for `MATCH_PHRASE`. Some tests for `MATCH` are also updated. Signed-off-by: Yury Fridlyand --- .../script/filter/FilterQueryBuilderTest.java | 114 +++++++++++++++++- 1 file changed, 110 insertions(+), 4 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java index 15b4ad2bad..dd00e13dde 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java @@ -6,8 +6,7 @@ package org.opensearch.sql.opensearch.storage.script.filter; -import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.*; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; @@ -45,6 +44,7 @@ import org.opensearch.sql.data.model.ExprTimeValue; import org.opensearch.sql.data.model.ExprTimestampValue; import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; @@ -361,8 +361,114 @@ void match_invalid_parameter() { dsl.namedArgument("field", literal("message")), dsl.namedArgument("query", literal("search query")), dsl.namedArgument("invalid_parameter", literal("invalid_value"))); - assertThrows(SemanticCheckException.class, () -> buildQuery(expr), - "Parameter invalid_parameter is invalid for match function."); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Parameter invalid_parameter is invalid for match function.", msg); + } + + @Test + void should_build_match_phrase_query_with_default_parameters() { + assertJsonEquals( + "{\n" + + " \"match_phrase\" : {\n" + + " \"message\" : {\n" + + " \"query\" : \"search query\",\n" + + " \"slop\" : 0,\n" + + " \"zero_terms_query\" : \"NONE\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery( + dsl.match_phrase( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query"))))); + } + + @Test + void should_build_match_phrase_query_with_custom_parameters() { + assertJsonEquals( + "{\n" + + " \"match_phrase\" : {\n" + + " \"message\" : {\n" + + " \"query\" : \"search query\",\n" + + " \"analyzer\" : \"keyword\"," + + " \"slop\" : 2,\n" + + " \"zero_terms_query\" : \"ALL\",\n" + + " \"boost\" : 1.0\n" + + " }\n" + + " }\n" + + "}", + buildQuery( + dsl.match_phrase( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("analyzer", literal("keyword")), + dsl.namedArgument("slop", literal("2")), + dsl.namedArgument("zero_terms_query", literal("ALL"))))); + } + + @Test + void match_phrase_invalid_parameter() { + FunctionExpression expr = dsl.match_phrase( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("invalid_parameter", literal("invalid_value"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("Parameter invalid_parameter is invalid for match_phrase function.", msg); + } + + @Test + void match_phrase_invalid_value_slop() { + FunctionExpression expr = dsl.match_phrase( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("slop", literal("1.5"))); + var msg = assertThrows(NumberFormatException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("For input string: \"1.5\"", msg); + } + + @Test + void match_phrase_invalid_value_ztq() { + FunctionExpression expr = dsl.match_phrase( + dsl.namedArgument("field", literal("message")), + dsl.namedArgument("query", literal("search query")), + dsl.namedArgument("zero_terms_query", literal("meow"))); + var msg = assertThrows(IllegalArgumentException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("No enum constant org.opensearch.index.search.MatchQuery.ZeroTermsQuery.meow", msg); + } + + @Test + void match_phrase_missing_field() { + var msg = assertThrows(ExpressionEvaluationException.class, () -> + dsl.match_phrase( + dsl.namedArgument("query", literal("search query")))).getMessage(); + assertEquals("match_phrase function expected {[STRING,STRING],[STRING,STRING,STRING]," + + "[STRING,STRING,STRING,STRING],[STRING,STRING,STRING,STRING,STRING]}, but get [STRING]", msg); + } + + @Test + void match_phrase_missing_query() { + var msg = assertThrows(ExpressionEvaluationException.class, () -> + dsl.match_phrase( + dsl.namedArgument("field", literal("message")))).getMessage(); + assertEquals("match_phrase function expected {[STRING,STRING],[STRING,STRING,STRING]," + + "[STRING,STRING,STRING,STRING],[STRING,STRING,STRING,STRING,STRING]}, but get [STRING]", msg); + } + + @Test + void match_phrase_too_many_args() { + var msg = assertThrows(ExpressionEvaluationException.class, () -> + dsl.match_phrase( + dsl.namedArgument("one", literal("1")), + dsl.namedArgument("two", literal("2")), + dsl.namedArgument("three", literal("3")), + dsl.namedArgument("four", literal("4")), + dsl.namedArgument("fix", literal("5")), + dsl.namedArgument("six", literal("6")) + )).getMessage(); + assertEquals("match_phrase function expected {[STRING,STRING],[STRING,STRING,STRING]," + + "[STRING,STRING,STRING,STRING],[STRING,STRING,STRING,STRING,STRING]}, but get " + + "[STRING,STRING,STRING,STRING,STRING,STRING]", msg); } @Test From e22879ada716b43f5005f6e9bef7e3b923ea5c55 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Mon, 2 May 2022 12:53:59 -0700 Subject: [PATCH 22/56] Address PR feedback. Signed-off-by: Yury Fridlyand --- .../script/filter/lucene/relevance/MatchPhraseQuery.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java index 46a6379fb5..2a8c022ed4 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java @@ -8,8 +8,9 @@ import com.google.common.collect.ImmutableMap; import java.util.Iterator; import java.util.function.BiFunction; - -import org.opensearch.index.query.*; +import org.opensearch.index.query.MatchPhraseQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.QueryBuilders; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.Expression; From a46d6ac52c3098d3ba33abbd129bdaf157421bcf Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Mon, 2 May 2022 15:39:21 -0700 Subject: [PATCH 23/56] Add documentation for `MATCH_PHRASE` function. Signed-off-by: Yury Fridlyand --- docs/user/dql/functions.rst | 38 +++++++++++++++++++++++++++ docs/user/ppl/functions/relevance.rst | 38 +++++++++++++++++++++++++++ 2 files changed, 76 insertions(+) diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 188c326f6d..63b452d2e1 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2195,3 +2195,41 @@ Another example to show how to set custom values for the optional parameters:: | Bond | +------------+ +MATCH_PHRASE +----- + +Description +>>>>>>>>>>> + +``match_phrase(field_expression, query_expression[, option=]*)`` + +The match_phrase function maps to the match_phrase query used in search engine, to return the documents that match a provided text with a given field. Available parameters include: + +- analyzer +- slop +- zero_terms_query + +Example with only ``field`` and ``query`` expressions, and all other parameters are set default values:: + + os> SELECT lastname, address FROM accounts WHERE match_phrase(address, 'Madison Street'); + fetched rows / total rows = 2/2 + +------------+--------------------+ + | lastname | address | + |------------+--------------------| + | James | 671 Madison Street | + | Bond | 789 Madison Street | + +------------+--------------------+ + + + +Another example to show how to set custom values for the optional parameters:: + + os> SELECT title FROM accounts WHERE match_phrase(title, 'Winnie Pooh', slop = 2); + fetched rows / total rows = 1/1 + +-----------------+ + | title | + |-----------------| + | Winnie Pooh | + | Winnie the Pooh | + +-----------------+ + diff --git a/docs/user/ppl/functions/relevance.rst b/docs/user/ppl/functions/relevance.rst index 0b00f382ec..56d804b446 100644 --- a/docs/user/ppl/functions/relevance.rst +++ b/docs/user/ppl/functions/relevance.rst @@ -56,6 +56,44 @@ Another example to show how to set custom values for the optional parameters:: | Bond | +------------+ +MATCH_PHRASE +----- + +Description +>>>>>>>>>>> + +``match_phrase(field_expression, query_expression[, option=]*)`` + +The match_phrase function maps to the match_phrase query used in search engine, to return the documents that match a provided text with a given field. Available parameters include: + +- analyzer +- slop +- zero_terms_query + +Example with only ``field`` and ``query`` expressions, and all other parameters are set default values:: + + os> SELECT lastname, address FROM accounts WHERE match_phrase(address, 'Madison Street'); + fetched rows / total rows = 2/2 + +------------+--------------------+ + | lastname | address | + |------------+--------------------| + | James | 671 Madison Street | + | Bond | 789 Madison Street | + +------------+--------------------+ + + + +Another example to show how to set custom values for the optional parameters:: + + os> SELECT title FROM accounts WHERE match_phrase(title, 'Winnie Pooh', slop = 2); + fetched rows / total rows = 1/1 + +-----------------+ + | title | + |-----------------| + | Winnie Pooh | + | Winnie the Pooh | + +-----------------+ + Limitations >>>>>>>>>>> From 30ef3bacb498b93707b060a2da14a841d45a8644 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 3 May 2022 00:57:51 -0700 Subject: [PATCH 24/56] Remove duplicate definition of match_phrase in DSL.java Signed-off-by: MaxKsyunz --- core/src/main/java/org/opensearch/sql/expression/DSL.java | 5 +---- 1 file changed, 1 insertion(+), 4 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 224dafe7aa..c559276688 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -662,8 +662,5 @@ private FunctionExpression compile(BuiltinFunctionName bfn, Expression... args) return (FunctionExpression) repository.compile(bfn.getName(), Arrays.asList(args.clone())); } - public FunctionExpression match_phrase(Expression... args) { - return (FunctionExpression) repository - .compile(BuiltinFunctionName.MATCH_PHRASE.getName(), Arrays.asList(args.clone())); - } + } From 39ac1ddcfca5335152d861d7cc578a602da49ccd Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 3 May 2022 00:59:46 -0700 Subject: [PATCH 25/56] Generate supported signatures for match and match_phrases Signed-off-by: MaxKsyunz --- .../function/OpenSearchFunctions.java | 82 +++++-------------- 1 file changed, 20 insertions(+), 62 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 1c0bbbf4e8..7b4523f2fd 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -7,11 +7,11 @@ import static org.opensearch.sql.data.type.ExprCoreType.STRING; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.stream.Collectors; -import lombok.ToString; import lombok.experimental.UtilityClass; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprCoreType; @@ -30,75 +30,33 @@ public void register(BuiltinFunctionRepository repository) { private static FunctionResolver match() { FunctionName funcName = BuiltinFunctionName.MATCH.getName(); - return getFunctionResolver(funcName); + // At least field and query parameters + // At most field, query, and all optional parameters + return getFunctionResolver(funcName, 2, 14); } private static FunctionResolver match_phrase() { FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); - return getFunctionResolver(funcName); + // At least field and query parameters + // At most field, query, and all optional parameters + return getFunctionResolver(funcName, 2,6); } - private static FunctionResolver getFunctionResolver(FunctionName funcName) { + private static FunctionResolver getFunctionResolver( + FunctionName funcName, int minNumParameters, int maxNumParameters) { return new FunctionResolver(funcName, - ImmutableMap.builder() - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, STRING, - STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .build()); + getSignatureMap(funcName, minNumParameters, maxNumParameters)); } - private static FunctionResolver match_phrase() { - FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); - return new FunctionResolver(funcName, - ImmutableMap.builder() - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList.of(STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .put(new FunctionSignature(funcName, ImmutableList - .of(STRING, STRING, STRING, STRING, STRING)), - args -> new OpenSearchFunction(funcName, args)) - .build()); + private static Map getSignatureMap( + FunctionName funcName, int minNumParameters, int maxNumParameters) { + FunctionBuilder buildFunction = args -> new OpenSearchFunction(funcName, args); + var signatureMapBuilder = ImmutableMap.builder(); + for (int numParameters = minNumParameters; numParameters <= maxNumParameters; numParameters++) { + List args = Collections.nCopies(numParameters, STRING); + signatureMapBuilder.put(new FunctionSignature(funcName, args), buildFunction); + } + return signatureMapBuilder.build(); } private static class OpenSearchFunction extends FunctionExpression { From 0ec0aec1629cdb021e4508a3a1d380087ee9ee08 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 3 May 2022 01:02:52 -0700 Subject: [PATCH 26/56] Simplify match_phrase test. Signed-off-by: MaxKsyunz --- .../function/OpenSearchFunctionsTest.java | 26 +++++++++---------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java b/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java index 646128a0f4..02dbc40545 100644 --- a/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java +++ b/core/src/test/java/org/opensearch/sql/expression/function/OpenSearchFunctionsTest.java @@ -9,12 +9,15 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; +import java.util.List; import org.junit.jupiter.api.Test; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.ExpressionTestBase; import org.opensearch.sql.expression.FunctionExpression; import org.opensearch.sql.expression.NamedArgumentExpression; + + public class OpenSearchFunctionsTest extends ExpressionTestBase { private final NamedArgumentExpression field = new NamedArgumentExpression( "field", DSL.literal("message")); @@ -115,24 +118,19 @@ void match() { @Test void match_phrase() { - { - FunctionExpression expr = dsl.match_phrase(field, query); - assertEquals(BOOLEAN, expr.type()); - } - { - FunctionExpression expr = dsl.match_phrase(field, query, zeroTermsQueryAll); + for (FunctionExpression expr : match_phrase_dsl_expressions()) { assertEquals(BOOLEAN, expr.type()); } + } - { - FunctionExpression expr = dsl.match_phrase(field, query, zeroTermsQueryNone); - assertEquals(BOOLEAN, expr.type()); - } - { - FunctionExpression expr = dsl.match_phrase(field, query, slop); - assertEquals(BOOLEAN, expr.type()); - } + List match_phrase_dsl_expressions() { + return List.of( + dsl.match_phrase(field, query), + dsl.match_phrase(field, query, analyzer), + dsl.match_phrase(field, query, analyzer, zeroTermsQueryAll), + dsl.match_phrase(field, query, analyzer, zeroTermsQueryNone, slop) + ); } @Test From a987aa389413d592d92c9d436dc69945bc591b9b Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 3 May 2022 02:12:56 -0700 Subject: [PATCH 27/56] Some sample queries with match_phrase for parser test Signed-off-by: MaxKsyunz --- .../sql/sql/antlr/SQLSyntaxParserTest.java | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index 0d4bc03d2f..4ee5f78040 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -11,6 +11,8 @@ import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; import org.opensearch.sql.common.antlr.SyntaxCheckException; import java.util.*; @@ -164,6 +166,28 @@ public void canParseRelevanceFunctions() { assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(column, 100500)")); } + @ParameterizedTest + @MethodSource("matchPhraseComplexQueries") + public void canParseComplexMatchPhraseArgsTest(String query) { + assertNotNull(parser.parse(query)); + } + + private static Stream matchPhraseComplexQueries() { + return Stream.of( + "SELECT * FROM t WHERE match_phrase(c, 3)", + "SELECT * FROM t WHERE match_phrase(c, 3, fuzziness=AUTO)", + "SELECT * FROM t WHERE match_phrase(c, 3, zero_terms_query=\"all\")", + "SELECT * FROM t WHERE match_phrase(c, 3, lenient=true)", + "SELECT * FROM t WHERE match_phrase(c, 3, lenient='true')", + "SELECT * FROM t WHERE match_phrase(c, 3, operator=xor)", + "SELECT * FROM t WHERE match_phrase(c, 3, cutoff_frequency=0.04)", + "SELECT * FROM t WHERE match_phrase(c, 3, cutoff_frequency=0.04, analyzer = english, prefix_length=34, fuzziness='auto', minimum_should_match='2<-25% 9<-3')", + "SELECT * FROM t WHERE match_phrase(c, 3, minimum_should_match='2<-25% 9<-3')", + "SELECT * FROM t WHERE match_phrase(c, 3, operator='AUTO')" + + ); + } + private void generateAndTestQuery(String function, HashMap functionArgs) { var rand = new Random(); From 14cb1a5eb46b87387c5cb2483761b5da4f4ae03d Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 3 May 2022 02:13:47 -0700 Subject: [PATCH 28/56] Update PPL documentation. 1. Use PPL instead of SQL as samples. 2. Use data that doctest runs with. Signed-off-by: MaxKsyunz --- docs/user/ppl/functions/relevance.rst | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/docs/user/ppl/functions/relevance.rst b/docs/user/ppl/functions/relevance.rst index 56d804b446..957f3d318e 100644 --- a/docs/user/ppl/functions/relevance.rst +++ b/docs/user/ppl/functions/relevance.rst @@ -72,27 +72,26 @@ The match_phrase function maps to the match_phrase query used in search engine, Example with only ``field`` and ``query`` expressions, and all other parameters are set default values:: - os> SELECT lastname, address FROM accounts WHERE match_phrase(address, 'Madison Street'); + os> source=accounts | where match(address, 'Street') | fields lastname, address fetched rows / total rows = 2/2 +------------+--------------------+ | lastname | address | |------------+--------------------| - | James | 671 Madison Street | - | Bond | 789 Madison Street | + | Bond | 671 Bristol Street | + | Bates | 789 Madison Street | +------------+--------------------+ Another example to show how to set custom values for the optional parameters:: - os> SELECT title FROM accounts WHERE match_phrase(title, 'Winnie Pooh', slop = 2); + os> source=accounts | where match_phrase(address, 'Street Madison', slop = 2) | fields address fetched rows / total rows = 1/1 - +-----------------+ - | title | - |-----------------| - | Winnie Pooh | - | Winnie the Pooh | - +-----------------+ + +--------------------+ + | address | + |--------------------| + | 789 Madison Street | + +--------------------+ Limitations >>>>>>>>>>> From b4f5ba98b6daebd7b127f4b4c8a5860bc05879dc Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Tue, 3 May 2022 22:08:16 -0700 Subject: [PATCH 29/56] Update query samples in docs. Add a new table to doctest to match the samples. This fixes doctest. Signed-off-by: Yury Fridlyand --- docs/user/dql/functions.rst | 30 +++++++++++++-------------- docs/user/ppl/functions/relevance.rst | 29 +++++++++++++------------- doctest/test_docs.py | 4 +++- 3 files changed, 33 insertions(+), 30 deletions(-) diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 63b452d2e1..6fe206a471 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2211,25 +2211,25 @@ The match_phrase function maps to the match_phrase query used in search engine, Example with only ``field`` and ``query`` expressions, and all other parameters are set default values:: - os> SELECT lastname, address FROM accounts WHERE match_phrase(address, 'Madison Street'); + os> SELECT author, title FROM books WHERE match_phrase(author, 'Alexander Milne'); fetched rows / total rows = 2/2 - +------------+--------------------+ - | lastname | address | - |------------+--------------------| - | James | 671 Madison Street | - | Bond | 789 Madison Street | - +------------+--------------------+ + +----------------------+--------------------------+ + | author | title | + |----------------------+--------------------------| + | Alan Alexander Milne | The House at Pooh Corner | + | Alan Alexander Milne | Winnie-the-Pooh | + +----------------------+--------------------------+ Another example to show how to set custom values for the optional parameters:: - os> SELECT title FROM accounts WHERE match_phrase(title, 'Winnie Pooh', slop = 2); - fetched rows / total rows = 1/1 - +-----------------+ - | title | - |-----------------| - | Winnie Pooh | - | Winnie the Pooh | - +-----------------+ + os> SELECT author, title FROM books WHERE match_phrase(author, 'Alan Milne', slop = 2); + fetched rows / total rows = 2/2 + +----------------------+--------------------------+ + | author | title | + |----------------------+--------------------------| + | Alan Alexander Milne | The House at Pooh Corner | + | Alan Alexander Milne | Winnie-the-Pooh | + +----------------------+--------------------------+ diff --git a/docs/user/ppl/functions/relevance.rst b/docs/user/ppl/functions/relevance.rst index 957f3d318e..cdbadde2b1 100644 --- a/docs/user/ppl/functions/relevance.rst +++ b/docs/user/ppl/functions/relevance.rst @@ -72,26 +72,27 @@ The match_phrase function maps to the match_phrase query used in search engine, Example with only ``field`` and ``query`` expressions, and all other parameters are set default values:: - os> source=accounts | where match(address, 'Street') | fields lastname, address + os> source=books | where match_phrase(author, 'Alexander Milne') | fields author, title fetched rows / total rows = 2/2 - +------------+--------------------+ - | lastname | address | - |------------+--------------------| - | Bond | 671 Bristol Street | - | Bates | 789 Madison Street | - +------------+--------------------+ + +----------------------+--------------------------+ + | author | title | + |----------------------+--------------------------| + | Alan Alexander Milne | The House at Pooh Corner | + | Alan Alexander Milne | Winnie-the-Pooh | + +----------------------+--------------------------+ Another example to show how to set custom values for the optional parameters:: - os> source=accounts | where match_phrase(address, 'Street Madison', slop = 2) | fields address - fetched rows / total rows = 1/1 - +--------------------+ - | address | - |--------------------| - | 789 Madison Street | - +--------------------+ + os> source=books | where match_phrase(author, 'Alan Milne', slop = 2) | fields author, title + fetched rows / total rows = 2/2 + +----------------------+--------------------------+ + | author | title | + |----------------------+--------------------------| + | Alan Alexander Milne | The House at Pooh Corner | + | Alan Alexander Milne | Winnie-the-Pooh | + +----------------------+--------------------------+ Limitations >>>>>>>>>>> diff --git a/doctest/test_docs.py b/doctest/test_docs.py index c6c07d1d0d..76af1fa9e7 100644 --- a/doctest/test_docs.py +++ b/doctest/test_docs.py @@ -24,6 +24,7 @@ PEOPLE = "people" ACCOUNT2 = "account2" NYC_TAXI = "nyc_taxi" +BOOKS = "books" class DocTestConnection(OpenSearchConnection): @@ -88,6 +89,7 @@ def set_up_test_indices(test): load_file("people.json", index_name=PEOPLE) load_file("account2.json", index_name=ACCOUNT2) load_file("nyc_taxi.json", index_name=NYC_TAXI) + load_file("books.json", index_name=BOOKS) def load_file(filename, index_name): @@ -116,7 +118,7 @@ def set_up(test): def tear_down(test): # drop leftover tables after each test - test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI], ignore_unavailable=True) + test_data_client.indices.delete(index=[ACCOUNTS, EMPLOYEES, PEOPLE, ACCOUNT2, NYC_TAXI, BOOKS], ignore_unavailable=True) docsuite = partial(doctest.DocFileSuite, From e4d155b465d1e0222cb5607afed24dd822843bd3 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Tue, 3 May 2022 22:14:15 -0700 Subject: [PATCH 30/56] Recover `FUZZY_REWRITE` - was deleted by mistake. Signed-off-by: Yury Fridlyand --- sql/src/main/antlr/OpenSearchSQLLexer.g4 | 1 + sql/src/main/antlr/OpenSearchSQLParser.g4 | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/src/main/antlr/OpenSearchSQLLexer.g4 b/sql/src/main/antlr/OpenSearchSQLLexer.g4 index 430ca3a045..902a8ed190 100644 --- a/sql/src/main/antlr/OpenSearchSQLLexer.g4 +++ b/sql/src/main/antlr/OpenSearchSQLLexer.g4 @@ -321,6 +321,7 @@ FIELDS: 'FIELDS'; FLAGS: 'FLAGS'; FUZZINESS: 'FUZZINESS'; FUZZY_TRANSPOSITIONS: 'FUZZY_TRANSPOSITIONS'; +FUZZY_REWRITE: 'FUZZY_REWRITE'; LENIENT: 'LENIENT'; LOW_FREQ_OPERATOR: 'LOW_FREQ_OPERATOR'; MAX_DETERMINIZED_STATES: 'MAX_DETERMINIZED_STATES'; diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index f1fef47f45..3a90c0f55a 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -405,7 +405,7 @@ relevanceArg relevanceArgName : ALLOW_LEADING_WILDCARD | ANALYZE_WILDCARD | ANALYZER | AUTO_GENERATE_SYNONYMS_PHRASE_QUERY | BOOST | CUTOFF_FREQUENCY | ENABLE_POSITION_INCREMENTS | FIELDS | FLAGS | FUZZINESS | FUZZY_TRANSPOSITIONS - | LENIENT | LOW_FREQ_OPERATOR | MAX_DETERMINIZED_STATES | MAX_EXPANSIONS | MINIMUM_SHOULD_MATCH + | FUZZY_REWRITE | LENIENT | LOW_FREQ_OPERATOR | MAX_DETERMINIZED_STATES | MAX_EXPANSIONS | MINIMUM_SHOULD_MATCH | OPERATOR | PHRASE_SLOP | PREFIX_LENGTH | QUOTE_FIELD_SUFFIX | REWRITE | SLOP | TIE_BREAKER | TIME_ZONE | TYPE | ZERO_TERMS_QUERY ; From c30338140e1b9bedd4d766102ac187d4f3507325 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Wed, 4 May 2022 09:56:00 -0700 Subject: [PATCH 31/56] Add missing file for `63009a05`. Signed-off-by: Yury Fridlyand --- doctest/test_data/books.json | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 doctest/test_data/books.json diff --git a/doctest/test_data/books.json b/doctest/test_data/books.json new file mode 100644 index 0000000000..36b0fb0307 --- /dev/null +++ b/doctest/test_data/books.json @@ -0,0 +1,2 @@ +{"id": 1, "author": "Alan Alexander Milne", "title": "The House at Pooh Corner"} +{"id": 2, "author": "Alan Alexander Milne", "title": "Winnie-the-Pooh"} \ No newline at end of file From 5020d7f9ff4bc05ae0d704f363af3609c15745ab Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 4 May 2022 10:47:07 -0700 Subject: [PATCH 32/56] Use constant seed to produce repeatable samples Signed-off-by: MaxKsyunz --- .../java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index 4ee5f78040..96be57c088 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -189,7 +189,7 @@ private static Stream matchPhraseComplexQueries() { } private void generateAndTestQuery(String function, HashMap functionArgs) { - var rand = new Random(); + var rand = new Random(0); for (int i = 0; i < 100; i++) { From 82c6ab89a32b1fc3637a413dee398c9dcb6ecd9e Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 4 May 2022 10:47:54 -0700 Subject: [PATCH 33/56] Enable integration tests for match_phrase in PPL Signed-off-by: MaxKsyunz --- .../src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java index deeb19f870..6f5d6a78bd 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java @@ -114,7 +114,6 @@ public void testRelevanceFunction() throws IOException { } @Test - @Ignore("Not supported yet") public void testMatchPhraseFunction() throws IOException { JSONObject result = executeQuery( @@ -124,7 +123,6 @@ public void testMatchPhraseFunction() throws IOException { } @Test - @Ignore("Not supported yet") public void testMathPhraseWithSlop() throws IOException { JSONObject result = executeQuery( From a0d56a8967b48e8f9e39e2cecf2a36c6a2774867 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Wed, 4 May 2022 14:54:30 -0700 Subject: [PATCH 34/56] Fix code style. Signed-off-by: Yury Fridlyand --- .../lucene/relevance/MatchPhraseQuery.java | 10 +++++----- .../script/filter/FilterQueryBuilderTest.java | 17 +++++++++++------ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java index 2a8c022ed4..e1c58bee02 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java @@ -20,12 +20,12 @@ public class MatchPhraseQuery extends LuceneQuery { private final BiFunction analyzer = - (b, v) -> b.analyzer(v.stringValue()); + (b, v) -> b.analyzer(v.stringValue()); private final BiFunction slop = - (b, v) -> b.slop(Integer.parseInt(v.stringValue())); - private final BiFunction zeroTermsQuery = - (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); + (b, v) -> b.slop(Integer.parseInt(v.stringValue())); + private final BiFunction + zeroTermsQuery = (b, v) -> b.zeroTermsQuery( + org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); ImmutableMap argAction = ImmutableMap.builder() .put("analyzer", analyzer) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java index dd00e13dde..9bc6ed5076 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilderTest.java @@ -6,7 +6,9 @@ package org.opensearch.sql.opensearch.storage.script.filter; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doAnswer; import static org.opensearch.sql.data.type.ExprCoreType.BOOLEAN; @@ -368,7 +370,7 @@ void match_invalid_parameter() { @Test void should_build_match_phrase_query_with_default_parameters() { assertJsonEquals( - "{\n" + "{\n" + " \"match_phrase\" : {\n" + " \"message\" : {\n" + " \"query\" : \"search query\",\n" @@ -387,7 +389,7 @@ void should_build_match_phrase_query_with_default_parameters() { @Test void should_build_match_phrase_query_with_custom_parameters() { assertJsonEquals( - "{\n" + "{\n" + " \"match_phrase\" : {\n" + " \"message\" : {\n" + " \"query\" : \"search query\",\n" @@ -434,7 +436,8 @@ void match_phrase_invalid_value_ztq() { dsl.namedArgument("query", literal("search query")), dsl.namedArgument("zero_terms_query", literal("meow"))); var msg = assertThrows(IllegalArgumentException.class, () -> buildQuery(expr)).getMessage(); - assertEquals("No enum constant org.opensearch.index.search.MatchQuery.ZeroTermsQuery.meow", msg); + assertEquals("No enum constant org.opensearch.index.search.MatchQuery.ZeroTermsQuery.meow", + msg); } @Test @@ -443,7 +446,8 @@ void match_phrase_missing_field() { dsl.match_phrase( dsl.namedArgument("query", literal("search query")))).getMessage(); assertEquals("match_phrase function expected {[STRING,STRING],[STRING,STRING,STRING]," - + "[STRING,STRING,STRING,STRING],[STRING,STRING,STRING,STRING,STRING]}, but get [STRING]", msg); + + "[STRING,STRING,STRING,STRING],[STRING,STRING,STRING,STRING,STRING]}, but get [STRING]", + msg); } @Test @@ -452,7 +456,8 @@ void match_phrase_missing_query() { dsl.match_phrase( dsl.namedArgument("field", literal("message")))).getMessage(); assertEquals("match_phrase function expected {[STRING,STRING],[STRING,STRING,STRING]," - + "[STRING,STRING,STRING,STRING],[STRING,STRING,STRING,STRING,STRING]}, but get [STRING]", msg); + + "[STRING,STRING,STRING,STRING],[STRING,STRING,STRING,STRING,STRING]}, but get [STRING]", + msg); } @Test From becdeacd363d7077c4bf0524ec0b29682f48da69 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 4 May 2022 11:38:17 -0700 Subject: [PATCH 35/56] Make generateAndTestQueries() product consistent results. Signed-off-by: MaxKsyunz --- .../sql/sql/antlr/SQLSyntaxParserTest.java | 91 +++++++++++++++---- 1 file changed, 75 insertions(+), 16 deletions(-) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index 96be57c088..b602d03755 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -9,6 +9,7 @@ import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import com.google.common.collect.Streams; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -16,6 +17,7 @@ import org.opensearch.sql.common.antlr.SyntaxCheckException; import java.util.*; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -167,7 +169,7 @@ public void canParseRelevanceFunctions() { } @ParameterizedTest - @MethodSource("matchPhraseComplexQueries") + @MethodSource({"matchPhraseComplexQueries", "matchPhraseGeneratedQueries"}) public void canParseComplexMatchPhraseArgsTest(String query) { assertNotNull(parser.parse(query)); } @@ -188,6 +190,77 @@ private static Stream matchPhraseComplexQueries() { ); } + private static Stream matchPhraseGeneratedQueries() { + var matchArgs = new HashMap(); + matchArgs.put("fuzziness", new String[]{ "AUTO", "AUTO:1,5", "1" }); + matchArgs.put("fuzzy_transpositions", new Boolean[]{ true, false }); + matchArgs.put("operator", new String[]{ "and", "or" }); + matchArgs.put("minimum_should_match", new String[]{ "3", "-2", "75%", "-25%", "3<90%", "2<-25% 9<-3" }); + matchArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); + matchArgs.put("zero_terms_query", new String[]{ "none", "all" }); + matchArgs.put("lenient", new Boolean[]{ true, false }); + // deprecated + matchArgs.put("cutoff_frequency", new Double[]{ .0, 0.001, 1., 42. }); + matchArgs.put("prefix_length", new Integer[]{ 0, 2, 5 }); + matchArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); + matchArgs.put("boost", new Double[]{ .5, 1., 2.3 }); + + return generateQueries("match", matchArgs); + } + + private static Stream generateQueries(String function, HashMap functionArgs) { + var rand = new Random(0); + + class QueryGenerator implements Iterator { + + private final Random rng = new Random(0); + private final int numQueries = 100; + private int currentQuery = 0; + @Override + public boolean hasNext() { + return currentQuery < numQueries; + } + + @Override + public String next() { + currentQuery += 1; + + StringBuilder query = new StringBuilder(); + query.append(String.format("SELECT * FROM test WHERE %s(%s, %s", function, + RandomStringUtils.random(10, true, false), + RandomStringUtils.random(10, true, false))); + var args = new ArrayList(); + for (var pair : functionArgs.entrySet()) + { + if (rand.nextBoolean()) + { + var arg = new StringBuilder(); + arg.append(rand.nextBoolean() ? "," : ", "); + arg.append(rand.nextBoolean() ? pair.getKey().toLowerCase() : pair.getKey().toUpperCase()); + arg.append(rand.nextBoolean() ? "=" : " = "); + if (pair.getValue() instanceof String[] || rand.nextBoolean()) { + var quoteSymbol = rand.nextBoolean() ? '\'' : '"'; + arg.append(quoteSymbol); + arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); + arg.append(quoteSymbol); + } + else + arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); + args.add(arg.toString()); + } + } + Collections.shuffle(args, rand); + for (var arg : args) + query.append(arg); + query.append(rand.nextBoolean() ? ")" : ");"); + return query.toString(); + } + } + + var it = new QueryGenerator(); + return Streams.stream(it); + } + private void generateAndTestQuery(String function, HashMap functionArgs) { var rand = new Random(0); @@ -217,7 +290,7 @@ private void generateAndTestQuery(String function, HashMap fun args.add(arg.toString()); } } - Collections.shuffle(args); + Collections.shuffle(args, rand); for (var arg : args) query.append(arg); query.append(rand.nextBoolean() ? ")" : ");"); @@ -229,21 +302,7 @@ private void generateAndTestQuery(String function, HashMap fun // TODO run all tests and collect exceptions and raise them in the end @Test public void canParseRelevanceFunctionsComplexRandomArgs() { - var matchArgs = new HashMap(); - matchArgs.put("fuzziness", new String[]{ "AUTO", "AUTO:1,5", "1" }); - matchArgs.put("fuzzy_transpositions", new Boolean[]{ true, false }); - matchArgs.put("operator", new String[]{ "and", "or" }); - matchArgs.put("minimum_should_match", new String[]{ "3", "-2", "75%", "-25%", "3<90%", "2<-25% 9<-3" }); - matchArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); - matchArgs.put("zero_terms_query", new String[]{ "none", "all" }); - matchArgs.put("lenient", new Boolean[]{ true, false }); - // deprecated - matchArgs.put("cutoff_frequency", new Double[]{ .0, 0.001, 1., 42. }); - matchArgs.put("prefix_length", new Integer[]{ 0, 2, 5 }); - matchArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); - matchArgs.put("boost", new Double[]{ .5, 1., 2.3 }); - generateAndTestQuery("match", matchArgs); var matchPhraseArgs = new HashMap(); matchPhraseArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); From b30db7e3a9206195f17831569b7547f92bea1c54 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 4 May 2022 12:15:23 -0700 Subject: [PATCH 36/56] use generateQueries to generate samples for match_phrase Signed-off-by: MaxKsyunz --- .../sql/sql/antlr/SQLSyntaxParserTest.java | 72 +++++-------------- 1 file changed, 18 insertions(+), 54 deletions(-) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index b602d03755..1c57b8dcc1 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -169,7 +169,7 @@ public void canParseRelevanceFunctions() { } @ParameterizedTest - @MethodSource({"matchPhraseComplexQueries", "matchPhraseGeneratedQueries"}) + @MethodSource({"matchPhraseComplexQueries", "matchPhraseGeneratedQueries", "generateMatchPhraseQueries"}) public void canParseComplexMatchPhraseArgsTest(String query) { assertNotNull(parser.parse(query)); } @@ -208,6 +208,17 @@ private static Stream matchPhraseGeneratedQueries() { return generateQueries("match", matchArgs); } + private static Stream generateMatchPhraseQueries() { + + + var matchPhraseArgs = new HashMap(); + matchPhraseArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); + matchPhraseArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); + matchPhraseArgs.put("slop", new Integer[]{ 0, 1, 2 }); + + return generateQueries("match_phrase", matchPhraseArgs); + } + private static Stream generateQueries(String function, HashMap functionArgs) { var rand = new Random(0); @@ -216,6 +227,10 @@ class QueryGenerator implements Iterator { private final Random rng = new Random(0); private final int numQueries = 100; private int currentQuery = 0; + private String randomIdentifier() { + return RandomStringUtils.random(10, 0, 0,true, false, null, rand); + } + @Override public boolean hasNext() { return currentQuery < numQueries; @@ -227,8 +242,8 @@ public String next() { StringBuilder query = new StringBuilder(); query.append(String.format("SELECT * FROM test WHERE %s(%s, %s", function, - RandomStringUtils.random(10, true, false), - RandomStringUtils.random(10, true, false))); + randomIdentifier(), + randomIdentifier())); var args = new ArrayList(); for (var pair : functionArgs.entrySet()) { @@ -260,55 +275,4 @@ public String next() { var it = new QueryGenerator(); return Streams.stream(it); } - - private void generateAndTestQuery(String function, HashMap functionArgs) { - var rand = new Random(0); - - for (int i = 0; i < 100; i++) - { - StringBuilder query = new StringBuilder(); - query.append(String.format("SELECT * FROM test WHERE %s(%s, %s", function, - RandomStringUtils.random(10, true, false), - RandomStringUtils.random(10, true, false))); - var args = new ArrayList(); - for (var pair : functionArgs.entrySet()) - { - if (rand.nextBoolean()) - { - var arg = new StringBuilder(); - arg.append(rand.nextBoolean() ? "," : ", "); - arg.append(rand.nextBoolean() ? pair.getKey().toLowerCase() : pair.getKey().toUpperCase()); - arg.append(rand.nextBoolean() ? "=" : " = "); - if (pair.getValue() instanceof String[] || rand.nextBoolean()) { - var quoteSymbol = rand.nextBoolean() ? '\'' : '"'; - arg.append(quoteSymbol); - arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); - arg.append(quoteSymbol); - } - else - arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); - args.add(arg.toString()); - } - } - Collections.shuffle(args, rand); - for (var arg : args) - query.append(arg); - query.append(rand.nextBoolean() ? ")" : ");"); - //System.out.printf("%d, %s%n", i, query.toString()); - assertNotNull(parser.parse(query.toString())); - } - } - - // TODO run all tests and collect exceptions and raise them in the end - @Test - public void canParseRelevanceFunctionsComplexRandomArgs() { - - - var matchPhraseArgs = new HashMap(); - matchPhraseArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); - matchPhraseArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); - matchPhraseArgs.put("slop", new Integer[]{ 0, 1, 2 }); - - generateAndTestQuery("match_phrase", matchPhraseArgs); - } } From 0e8d6da2d89e287a669f67c7b21c1e16a8115968 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Wed, 4 May 2022 16:21:18 -0700 Subject: [PATCH 37/56] Fix finction signature list. Typo fix for `5bf470c7`. Signed-off-by: Yury Fridlyand --- .../opensearch/sql/expression/function/OpenSearchFunctions.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 7b4523f2fd..f0fbe13c8d 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -39,7 +39,7 @@ private static FunctionResolver match_phrase() { FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); // At least field and query parameters // At most field, query, and all optional parameters - return getFunctionResolver(funcName, 2,6); + return getFunctionResolver(funcName, 2, 5); } private static FunctionResolver getFunctionResolver( From 4b92454ef28af24609c7796d9023f48153187d6d Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Wed, 4 May 2022 16:42:52 -0700 Subject: [PATCH 38/56] Fix code styling. Signed-off-by: Yury Fridlyand --- .../sql/sql/antlr/SQLSyntaxParserTest.java | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index 1c57b8dcc1..f9a26f9658 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -10,17 +10,18 @@ import static org.junit.jupiter.api.Assertions.assertThrows; import com.google.common.collect.Streams; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Random; +import java.util.stream.Stream; import org.apache.commons.lang3.RandomStringUtils; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import org.opensearch.sql.common.antlr.SyntaxCheckException; -import java.util.*; -import java.util.function.Supplier; -import java.util.stream.Collectors; -import java.util.stream.Stream; - class SQLSyntaxParserTest { private final SQLSyntaxParser parser = new SQLSyntaxParser(); @@ -161,15 +162,19 @@ public void canParseRelevanceFunctions() { assertNotNull(parser.parse("SELECT * FROM test WHERE match(`column`, 'this is a test')")); assertNotNull(parser.parse("SELECT * FROM test WHERE match(column, 100500)")); - assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(column, \"this is a test\")")); + assertNotNull( + parser.parse("SELECT * FROM test WHERE match_phrase(column, \"this is a test\")")); assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(column, 'this is a test')")); - assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(`column`, \"this is a test\")")); - assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(`column`, 'this is a test')")); + assertNotNull( + parser.parse("SELECT * FROM test WHERE match_phrase(`column`, \"this is a test\")")); + assertNotNull( + parser.parse("SELECT * FROM test WHERE match_phrase(`column`, 'this is a test')")); assertNotNull(parser.parse("SELECT * FROM test WHERE match_phrase(column, 100500)")); } @ParameterizedTest - @MethodSource({"matchPhraseComplexQueries", "matchPhraseGeneratedQueries", "generateMatchPhraseQueries"}) + @MethodSource({"matchPhraseComplexQueries", + "matchPhraseGeneratedQueries", "generateMatchPhraseQueries"}) public void canParseComplexMatchPhraseArgsTest(String query) { assertNotNull(parser.parse(query)); } @@ -183,10 +188,10 @@ private static Stream matchPhraseComplexQueries() { "SELECT * FROM t WHERE match_phrase(c, 3, lenient='true')", "SELECT * FROM t WHERE match_phrase(c, 3, operator=xor)", "SELECT * FROM t WHERE match_phrase(c, 3, cutoff_frequency=0.04)", - "SELECT * FROM t WHERE match_phrase(c, 3, cutoff_frequency=0.04, analyzer = english, prefix_length=34, fuzziness='auto', minimum_should_match='2<-25% 9<-3')", + "SELECT * FROM t WHERE match_phrase(c, 3, cutoff_frequency=0.04, analyzer = english, " + + "prefix_length=34, fuzziness='auto', minimum_should_match='2<-25% 9<-3')", "SELECT * FROM t WHERE match_phrase(c, 3, minimum_should_match='2<-25% 9<-3')", "SELECT * FROM t WHERE match_phrase(c, 3, operator='AUTO')" - ); } @@ -195,7 +200,8 @@ private static Stream matchPhraseGeneratedQueries() { matchArgs.put("fuzziness", new String[]{ "AUTO", "AUTO:1,5", "1" }); matchArgs.put("fuzzy_transpositions", new Boolean[]{ true, false }); matchArgs.put("operator", new String[]{ "and", "or" }); - matchArgs.put("minimum_should_match", new String[]{ "3", "-2", "75%", "-25%", "3<90%", "2<-25% 9<-3" }); + matchArgs.put("minimum_should_match", + new String[]{ "3", "-2", "75%", "-25%", "3<90%", "2<-25% 9<-3" }); matchArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); matchArgs.put("zero_terms_query", new String[]{ "none", "all" }); matchArgs.put("lenient", new Boolean[]{ true, false }); @@ -219,7 +225,8 @@ private static Stream generateMatchPhraseQueries() { return generateQueries("match_phrase", matchPhraseArgs); } - private static Stream generateQueries(String function, HashMap functionArgs) { + private static Stream generateQueries( + String function, HashMap functionArgs) { var rand = new Random(0); class QueryGenerator implements Iterator { @@ -227,6 +234,7 @@ class QueryGenerator implements Iterator { private final Random rng = new Random(0); private final int numQueries = 100; private int currentQuery = 0; + private String randomIdentifier() { return RandomStringUtils.random(10, 0, 0,true, false, null, rand); } @@ -242,31 +250,31 @@ public String next() { StringBuilder query = new StringBuilder(); query.append(String.format("SELECT * FROM test WHERE %s(%s, %s", function, - randomIdentifier(), - randomIdentifier())); + randomIdentifier(), + randomIdentifier())); var args = new ArrayList(); - for (var pair : functionArgs.entrySet()) - { - if (rand.nextBoolean()) - { + for (var pair : functionArgs.entrySet()) { + if (rand.nextBoolean()) { var arg = new StringBuilder(); arg.append(rand.nextBoolean() ? "," : ", "); - arg.append(rand.nextBoolean() ? pair.getKey().toLowerCase() : pair.getKey().toUpperCase()); + arg.append(rand.nextBoolean() ? pair.getKey().toLowerCase() + : pair.getKey().toUpperCase()); arg.append(rand.nextBoolean() ? "=" : " = "); if (pair.getValue() instanceof String[] || rand.nextBoolean()) { var quoteSymbol = rand.nextBoolean() ? '\'' : '"'; arg.append(quoteSymbol); arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); arg.append(quoteSymbol); - } - else + } else { arg.append(pair.getValue()[rand.nextInt(pair.getValue().length)]); + } args.add(arg.toString()); } } Collections.shuffle(args, rand); - for (var arg : args) + for (var arg : args) { query.append(arg); + } query.append(rand.nextBoolean() ? ")" : ");"); return query.toString(); } From 5334c65ae008f0657ad944c29599b7a1165cb295 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Thu, 5 May 2022 23:12:54 -0700 Subject: [PATCH 39/56] Minor style changes. Signed-off-by: MaxKsyunz --- .../opensearch/sql/sql/antlr/SQLSyntaxParserTest.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java index f9a26f9658..48107b1744 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/antlr/SQLSyntaxParserTest.java @@ -215,8 +215,6 @@ private static Stream matchPhraseGeneratedQueries() { } private static Stream generateMatchPhraseQueries() { - - var matchPhraseArgs = new HashMap(); matchPhraseArgs.put("analyzer", new String[]{ "standard", "stop", "english" }); matchPhraseArgs.put("max_expansions", new Integer[]{ 0, 5, 20 }); @@ -225,14 +223,12 @@ private static Stream generateMatchPhraseQueries() { return generateQueries("match_phrase", matchPhraseArgs); } - private static Stream generateQueries( - String function, HashMap functionArgs) { + private static Stream generateQueries(String function, + HashMap functionArgs) { var rand = new Random(0); class QueryGenerator implements Iterator { - private final Random rng = new Random(0); - private final int numQueries = 100; private int currentQuery = 0; private String randomIdentifier() { @@ -241,6 +237,7 @@ private String randomIdentifier() { @Override public boolean hasNext() { + int numQueries = 100; return currentQuery < numQueries; } From 05c6acbcbceba0c6aed664a16645ae0502611dc6 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Thu, 5 May 2022 23:13:55 -0700 Subject: [PATCH 40/56] Add newline at the end of file. Signed-off-by: MaxKsyunz --- doctest/test_data/books.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doctest/test_data/books.json b/doctest/test_data/books.json index 36b0fb0307..f0bb81e9e3 100644 --- a/doctest/test_data/books.json +++ b/doctest/test_data/books.json @@ -1,2 +1,2 @@ {"id": 1, "author": "Alan Alexander Milne", "title": "The House at Pooh Corner"} -{"id": 2, "author": "Alan Alexander Milne", "title": "Winnie-the-Pooh"} \ No newline at end of file +{"id": 2, "author": "Alan Alexander Milne", "title": "Winnie-the-Pooh"} From 80e3da0e4b2925f2f9d8ad2096447c6d5e77491c Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 12:29:55 -0700 Subject: [PATCH 41/56] Simplify MatchPhraseQuery and add unit tests. Signed-off-by: MaxKsyunz --- .../lucene/relevance/MatchPhraseQuery.java | 43 +++++++--- .../filter/lucene/MatchPhraseQueryTest.java | 81 +++++++++++++++++++ 2 files changed, 111 insertions(+), 13 deletions(-) create mode 100644 opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java index e1c58bee02..51bc79fab8 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java @@ -7,6 +7,8 @@ import com.google.common.collect.ImmutableMap; import java.util.Iterator; +import java.util.List; +import java.util.Objects; import java.util.function.BiFunction; import org.opensearch.index.query.MatchPhraseQueryBuilder; import org.opensearch.index.query.QueryBuilder; @@ -18,16 +20,25 @@ import org.opensearch.sql.expression.NamedArgumentExpression; import org.opensearch.sql.opensearch.storage.script.filter.lucene.LuceneQuery; +/** + * Lucene query that builds a match_phrase query. + */ public class MatchPhraseQuery extends LuceneQuery { - private final BiFunction analyzer = + private final FluentAction analyzer = (b, v) -> b.analyzer(v.stringValue()); - private final BiFunction slop = + private final FluentAction slop = (b, v) -> b.slop(Integer.parseInt(v.stringValue())); - private final BiFunction + private final FluentAction zeroTermsQuery = (b, v) -> b.zeroTermsQuery( org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); - ImmutableMap argAction = ImmutableMap.builder() + interface FluentAction + extends BiFunction { + } + + ImmutableMap + argAction = + ImmutableMap.builder() .put("analyzer", analyzer) .put("slop", slop) .put("zero_terms_query", zeroTermsQuery) @@ -35,21 +46,27 @@ public class MatchPhraseQuery extends LuceneQuery { @Override public QueryBuilder build(FunctionExpression func) { - Iterator iterator = func.getArguments().iterator(); - NamedArgumentExpression field = (NamedArgumentExpression) iterator.next(); - NamedArgumentExpression query = (NamedArgumentExpression) iterator.next(); + List arguments = func.getArguments(); + if (arguments.size() < 2) { + throw new SemanticCheckException("match_phrase requires at least two parameters"); + } + NamedArgumentExpression field = (NamedArgumentExpression) arguments.get(0); + NamedArgumentExpression query = (NamedArgumentExpression) arguments.get(1); MatchPhraseQueryBuilder queryBuilder = QueryBuilders.matchPhraseQuery( - field.getValue().valueOf(null).stringValue(), - query.getValue().valueOf(null).stringValue()); + field.getValue().valueOf(null).stringValue(), + query.getValue().valueOf(null).stringValue()); + + Iterator iterator = arguments.listIterator(2); while (iterator.hasNext()) { NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); if (!argAction.containsKey(arg.getArgName())) { throw new SemanticCheckException(String - .format("Parameter %s is invalid for match_phrase function.", arg.getArgName())); + .format("Parameter %s is invalid for match_phrase function.", arg.getArgName())); } - ((BiFunction) argAction - .get(arg.getArgName())) - .apply(queryBuilder, arg.getValue().valueOf(null)); + (Objects.requireNonNull( + argAction + .get(arg.getArgName()))) + .apply(queryBuilder, arg.getValue().valueOf(null)); } return queryBuilder; } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java new file mode 100644 index 0000000000..f4e7ba5629 --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java @@ -0,0 +1,81 @@ +package org.opensearch.sql.opensearch.storage.script.filter.lucene; + + +import static org.junit.Assert.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.config.ExpressionConfig; +import org.opensearch.sql.expression.env.Environment; +import org.opensearch.sql.expression.function.FunctionName; +import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.MatchPhraseQuery; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +public class MatchPhraseQueryTest { + + private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository()); + private final MatchPhraseQuery matchPhraseQuery = new MatchPhraseQuery(); + private final FunctionName matchPhrase = FunctionName.of("match_phrase"); + + @Test + public void test_SemanticCheckException_when_no_arguments() { + + List arguments = List.of(); + FunctionExpression fe = new MatchPhraseExpression(arguments); + assertThrows(SemanticCheckException.class, () -> matchPhraseQuery.build(fe)); + } + + @Test + public void test_SemanticCheckException_when_one_argument() { + List arguments = List.of(dsl.namedArgument("field", + DSL.literal("test"))); + FunctionExpression fe = new MatchPhraseExpression(arguments); + assertThrows(SemanticCheckException.class, () -> matchPhraseQuery.build(fe)); + } + + @Test + public void test_SemanticCheckException_when_invalid_parameter() { + List arguments = List.of( + dsl.namedArgument("field", DSL.literal("test")), + dsl.namedArgument("query", DSL.literal("test2")), + dsl.namedArgument("unsupported", DSL.literal(3))); + FunctionExpression fe = new MatchPhraseExpression(arguments); + Assertions.assertThrows(SemanticCheckException.class, () -> matchPhraseQuery.build(fe)); + } + + @Test + public void build_succeeds_with_two_arguments() { + + List arguments = List.of( + dsl.namedArgument("field", DSL.literal("test")), + dsl.namedArgument("query", DSL.literal("test2"))); + FunctionExpression fe = new MatchPhraseExpression(arguments); + Assertions.assertNotNull(matchPhraseQuery.build(fe)); + } + + private class MatchPhraseExpression extends FunctionExpression { + public MatchPhraseExpression(List arguments) { + super(MatchPhraseQueryTest.this.matchPhrase, arguments); + } + + @Override + public ExprValue valueOf(Environment valueEnv) { + return null; + } + + @Override + public ExprType type() { + return null; + } + } +} From 8198323e1e4cbf9274cb1525a861b91239d0acaa Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 13:53:03 -0700 Subject: [PATCH 42/56] Remove unused imports. Signed-off-by: MaxKsyunz --- .../src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java | 3 --- 1 file changed, 3 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java index 6f5d6a78bd..fe9d6b4224 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java @@ -12,9 +12,6 @@ import java.io.IOException; import org.json.JSONObject; -import org.junit.Ignore; -import org.junit.jupiter.api.Disabled; -import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; public class WhereCommandIT extends PPLIntegTestCase { From 5c6e07f4bc106f9d3b84742c70f3766cd9e4a698 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 14:20:17 -0700 Subject: [PATCH 43/56] Change data used by WhereCommandIT Changed testMatchPhraseFunction and testMathPhraseWithSlop to use data source that exercises match_phrase. Signed-off-by: MaxKsyunz --- .../test/java/org/opensearch/sql/ppl/WhereCommandIT.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java index fe9d6b4224..376936afc2 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java @@ -22,6 +22,7 @@ public void init() throws IOException { loadIndex(Index.BANK_WITH_NULL_VALUES); loadIndex(Index.BANK); loadIndex(Index.GAME_OF_THRONES); + loadIndex(Index.PHRASE); } @Test @@ -115,8 +116,8 @@ public void testMatchPhraseFunction() throws IOException { JSONObject result = executeQuery( String.format( - "source=%s | where match_phrase(firstname, 'Hattie') | fields firstname", TEST_INDEX_BANK)); - verifyDataRows(result, rows("Hattie")); + "source=%s | where match_phrase(phrase, 'quick fox') | fields phrase", TEST_INDEX_PHRASE)); + verifyDataRows(result, rows("quick fox"), rows("quick fox here")); } @Test @@ -124,7 +125,7 @@ public void testMathPhraseWithSlop() throws IOException { JSONObject result = executeQuery( String.format( - "source=%s | where match_phrase(firstname, 'Hattie', slop = 2) | fields firstname", TEST_INDEX_BANK)); - verifyDataRows(result, rows("Hattie")); + "source=%s | where match_phrase(phrase, 'brown fox', slop = 2) | fields phrase", TEST_INDEX_PHRASE)); + verifyDataRows(result, rows("brown fox"), rows("fox brown")); } } From 0f7ea73a391f23ca03968fb92592fec9b9e5b74f Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 15:16:21 -0700 Subject: [PATCH 44/56] Style update for OpenSearchFunctions 1. final variables for all constants. 2. Better name for getFunctionResolver -> getRelevanceFunctionResolver Signed-off-by: MaxKsyunz --- .../function/OpenSearchFunctions.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index f0fbe13c8d..1409d16666 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -30,26 +30,27 @@ public void register(BuiltinFunctionRepository repository) { private static FunctionResolver match() { FunctionName funcName = BuiltinFunctionName.MATCH.getName(); - // At least field and query parameters // At most field, query, and all optional parameters - return getFunctionResolver(funcName, 2, 14); + final int matchMaxNumParameters = 14; + return getRelevanceFunctionResolver(funcName, matchMaxNumParameters); } private static FunctionResolver match_phrase() { FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); - // At least field and query parameters // At most field, query, and all optional parameters - return getFunctionResolver(funcName, 2, 5); + final int matchPhraseMaxNumParameters = 5; + return getRelevanceFunctionResolver(funcName, matchPhraseMaxNumParameters); } - private static FunctionResolver getFunctionResolver( - FunctionName funcName, int minNumParameters, int maxNumParameters) { + private static FunctionResolver getRelevanceFunctionResolver( + FunctionName funcName, int maxNumParameters) { return new FunctionResolver(funcName, - getSignatureMap(funcName, minNumParameters, maxNumParameters)); + getRelevanceFunctionSignatureMap(funcName, maxNumParameters)); } - private static Map getSignatureMap( - FunctionName funcName, int minNumParameters, int maxNumParameters) { + private static Map getRelevanceFunctionSignatureMap( + FunctionName funcName, int maxNumParameters) { + final int minNumParameters = 2; FunctionBuilder buildFunction = args -> new OpenSearchFunction(funcName, args); var signatureMapBuilder = ImmutableMap.builder(); for (int numParameters = minNumParameters; numParameters <= maxNumParameters; numParameters++) { From 026ac4c71eef40eb2630be05c71e08e364cf002e Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 15:38:50 -0700 Subject: [PATCH 45/56] 100% test coverage for MatchPhraseQuery Signed-off-by: MaxKsyunz --- .../filter/lucene/MatchPhraseQueryTest.java | 63 ++++++++++++++----- 1 file changed, 47 insertions(+), 16 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java index f4e7ba5629..85777f6efd 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java @@ -1,7 +1,6 @@ package org.opensearch.sql.opensearch.storage.script.filter.lucene; -import static org.junit.Assert.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; import java.util.List; @@ -15,6 +14,7 @@ import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.NamedArgumentExpression; import org.opensearch.sql.expression.config.ExpressionConfig; import org.opensearch.sql.expression.env.Environment; import org.opensearch.sql.expression.function.FunctionName; @@ -27,40 +27,71 @@ public class MatchPhraseQueryTest { private final MatchPhraseQuery matchPhraseQuery = new MatchPhraseQuery(); private final FunctionName matchPhrase = FunctionName.of("match_phrase"); + private NamedArgumentExpression namedArgument(String name, String value) { + return dsl.namedArgument(name, DSL.literal(value)); + } + @Test public void test_SemanticCheckException_when_no_arguments() { List arguments = List.of(); - FunctionExpression fe = new MatchPhraseExpression(arguments); - assertThrows(SemanticCheckException.class, () -> matchPhraseQuery.build(fe)); + assertThrows(SemanticCheckException.class, + () -> matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } @Test public void test_SemanticCheckException_when_one_argument() { - List arguments = List.of(dsl.namedArgument("field", - DSL.literal("test"))); - FunctionExpression fe = new MatchPhraseExpression(arguments); - assertThrows(SemanticCheckException.class, () -> matchPhraseQuery.build(fe)); + List arguments = List.of(namedArgument("field", "test")); + assertThrows(SemanticCheckException.class, + () -> matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } @Test public void test_SemanticCheckException_when_invalid_parameter() { List arguments = List.of( - dsl.namedArgument("field", DSL.literal("test")), - dsl.namedArgument("query", DSL.literal("test2")), - dsl.namedArgument("unsupported", DSL.literal(3))); - FunctionExpression fe = new MatchPhraseExpression(arguments); - Assertions.assertThrows(SemanticCheckException.class, () -> matchPhraseQuery.build(fe)); + namedArgument("field", "test"), + namedArgument("query", "test2"), + namedArgument("unsupported", "3")); + Assertions.assertThrows(SemanticCheckException.class, + () -> matchPhraseQuery.build(new MatchPhraseExpression(arguments))); + } + + @Test + public void test_analyzer_parameter() { + List arguments = List.of( + namedArgument("field", "t1"), + namedArgument("query", "t2"), + namedArgument("analyzer", "standard") + ); + Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } @Test public void build_succeeds_with_two_arguments() { + List arguments = List.of( + namedArgument("field", "test"), + namedArgument("query", "test2")); + Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments))); + } + + @Test + public void test_slop_parameter() { + List arguments = List.of( + namedArgument("field", "t1"), + namedArgument("query", "t2"), + namedArgument("slop", "2") + ); + Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments))); + } + @Test + public void test_zero_terms_query_parameter() { List arguments = List.of( - dsl.namedArgument("field", DSL.literal("test")), - dsl.namedArgument("query", DSL.literal("test2"))); - FunctionExpression fe = new MatchPhraseExpression(arguments); - Assertions.assertNotNull(matchPhraseQuery.build(fe)); + namedArgument("field", "t1"), + namedArgument("query", "t2"), + namedArgument("zero_terms_query", "ALL") + ); + Assertions.assertNotNull(matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } private class MatchPhraseExpression extends FunctionExpression { From e68ba624e041e16348865c9be5d16604b1badf41 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 17:10:33 -0700 Subject: [PATCH 46/56] Fix imports in WhereCommandIT Signed-off-by: MaxKsyunz --- .../src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java index 376936afc2..e5f8f35448 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/WhereCommandIT.java @@ -6,7 +6,10 @@ package org.opensearch.sql.ppl; -import static org.opensearch.sql.legacy.TestsConstants.*; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_ACCOUNT; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BANK_WITH_NULL_VALUES; +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_PHRASE; import static org.opensearch.sql.util.MatcherUtils.rows; import static org.opensearch.sql.util.MatcherUtils.verifyDataRows; From bcc0a8234bd18b882cf38bf16ecdf6ff5379baa0 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 19:01:01 -0700 Subject: [PATCH 47/56] Move final variables to class constants in OpenSearchFunctions Signed-off-by: MaxKsyunz --- .../function/OpenSearchFunctions.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 1409d16666..2b148811c4 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -23,6 +23,11 @@ @UtilityClass public class OpenSearchFunctions { + + public static final int MATCH_MAX_NUM_PARAMETERS = 12; + public static final int MATCH_PHRASE_MAX_NUM_PARAMETERS = 3; + public static final int MIN_NUM_PARAMETERS = 2; + public void register(BuiltinFunctionRepository repository) { repository.register(match()); repository.register(match_phrase()); @@ -30,16 +35,12 @@ public void register(BuiltinFunctionRepository repository) { private static FunctionResolver match() { FunctionName funcName = BuiltinFunctionName.MATCH.getName(); - // At most field, query, and all optional parameters - final int matchMaxNumParameters = 14; - return getRelevanceFunctionResolver(funcName, matchMaxNumParameters); + return getRelevanceFunctionResolver(funcName, MATCH_MAX_NUM_PARAMETERS); } private static FunctionResolver match_phrase() { FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); - // At most field, query, and all optional parameters - final int matchPhraseMaxNumParameters = 5; - return getRelevanceFunctionResolver(funcName, matchPhraseMaxNumParameters); + return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); } private static FunctionResolver getRelevanceFunctionResolver( @@ -49,11 +50,10 @@ private static FunctionResolver getRelevanceFunctionResolver( } private static Map getRelevanceFunctionSignatureMap( - FunctionName funcName, int maxNumParameters) { - final int minNumParameters = 2; + FunctionName funcName, int numOptionalParameters) { FunctionBuilder buildFunction = args -> new OpenSearchFunction(funcName, args); var signatureMapBuilder = ImmutableMap.builder(); - for (int numParameters = minNumParameters; numParameters <= maxNumParameters; numParameters++) { + for (int numParameters = MIN_NUM_PARAMETERS; numParameters <= MIN_NUM_PARAMETERS + numOptionalParameters; numParameters++) { List args = Collections.nCopies(numParameters, STRING); signatureMapBuilder.put(new FunctionSignature(funcName, args), buildFunction); } From 0127ee612d981bb9ec886e1b87ad0e8e0ad65afa Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 9 May 2022 19:07:04 -0700 Subject: [PATCH 48/56] Fix checkstyle failure. Signed-off-by: MaxKsyunz --- .../sql/expression/function/OpenSearchFunctions.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 2b148811c4..2d632d4109 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -53,7 +53,9 @@ private static Map getRelevanceFunctionSigna FunctionName funcName, int numOptionalParameters) { FunctionBuilder buildFunction = args -> new OpenSearchFunction(funcName, args); var signatureMapBuilder = ImmutableMap.builder(); - for (int numParameters = MIN_NUM_PARAMETERS; numParameters <= MIN_NUM_PARAMETERS + numOptionalParameters; numParameters++) { + for (int numParameters = MIN_NUM_PARAMETERS; + numParameters <= MIN_NUM_PARAMETERS + numOptionalParameters; + numParameters++) { List args = Collections.nCopies(numParameters, STRING); signatureMapBuilder.put(new FunctionSignature(funcName, args), buildFunction); } From d6ea633f52f52ee8caf77f455d55f4bf5f18784e Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Mon, 16 May 2022 13:33:58 -0700 Subject: [PATCH 49/56] Support legacy syntax for match_phrase in the new SQL engine. Add support for matchphrase as an alternative spelling of match_phrase. Mention in both SQL and PPL documentation that matchphrase is accepted as well as match_phrase. Signed-off-by: MaxKsyunz --- .../expression/function/BuiltinFunctionName.java | 1 + .../expression/function/OpenSearchFunctions.java | 16 ++++++++++++---- docs/user/dql/functions.rst | 2 ++ docs/user/ppl/functions/relevance.rst | 2 ++ .../script/filter/FilterQueryBuilder.java | 1 + ppl/src/main/antlr/OpenSearchPPLParser.g4 | 2 +- sql/src/main/antlr/OpenSearchSQLParser.g4 | 2 +- 7 files changed, 20 insertions(+), 6 deletions(-) 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 06a29d748e..a36f289024 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 @@ -188,6 +188,7 @@ public enum BuiltinFunctionName { */ MATCH(FunctionName.of("match")), MATCH_PHRASE(FunctionName.of("match_phrase")), + MATCHPHRASE(FunctionName.of("matchphrase")), /** * Legacy Relevance Function. diff --git a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java index 2b148811c4..4b9aefd8e5 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/OpenSearchFunctions.java @@ -28,9 +28,15 @@ public class OpenSearchFunctions { public static final int MATCH_PHRASE_MAX_NUM_PARAMETERS = 3; public static final int MIN_NUM_PARAMETERS = 2; + /** + * Add functions specific to OpenSearch to repository. + */ public void register(BuiltinFunctionRepository repository) { repository.register(match()); - repository.register(match_phrase()); + // Register MATCHPHRASE as MATCH_PHRASE as well for backwards + // compatibility. + repository.register(match_phrase(BuiltinFunctionName.MATCH_PHRASE)); + repository.register(match_phrase(BuiltinFunctionName.MATCHPHRASE)); } private static FunctionResolver match() { @@ -38,8 +44,8 @@ private static FunctionResolver match() { return getRelevanceFunctionResolver(funcName, MATCH_MAX_NUM_PARAMETERS); } - private static FunctionResolver match_phrase() { - FunctionName funcName = BuiltinFunctionName.MATCH_PHRASE.getName(); + private static FunctionResolver match_phrase(BuiltinFunctionName matchPhrase) { + FunctionName funcName = matchPhrase.getName(); return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); } @@ -53,7 +59,9 @@ private static Map getRelevanceFunctionSigna FunctionName funcName, int numOptionalParameters) { FunctionBuilder buildFunction = args -> new OpenSearchFunction(funcName, args); var signatureMapBuilder = ImmutableMap.builder(); - for (int numParameters = MIN_NUM_PARAMETERS; numParameters <= MIN_NUM_PARAMETERS + numOptionalParameters; numParameters++) { + for (int numParameters = MIN_NUM_PARAMETERS; + numParameters <= MIN_NUM_PARAMETERS + numOptionalParameters; + numParameters++) { List args = Collections.nCopies(numParameters, STRING); signatureMapBuilder.put(new FunctionSignature(funcName, args), buildFunction); } diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 6fe206a471..cde48ae25d 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2209,6 +2209,8 @@ The match_phrase function maps to the match_phrase query used in search engine, - slop - zero_terms_query +For backward compatibility, matchphrase is also supported and mapped to match_phrase query as well. + Example with only ``field`` and ``query`` expressions, and all other parameters are set default values:: os> SELECT author, title FROM books WHERE match_phrase(author, 'Alexander Milne'); diff --git a/docs/user/ppl/functions/relevance.rst b/docs/user/ppl/functions/relevance.rst index cdbadde2b1..204e942e70 100644 --- a/docs/user/ppl/functions/relevance.rst +++ b/docs/user/ppl/functions/relevance.rst @@ -70,6 +70,8 @@ The match_phrase function maps to the match_phrase query used in search engine, - slop - zero_terms_query +For backward compatibility, matchphrase is also supported and mapped to match_phrase query as well. + Example with only ``field`` and ``query`` expressions, and all other parameters are set default values:: os> source=books | where match_phrase(author, 'Alexander Milne') | fields author, title diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java index 9aa8e1000b..ec54e8854e 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/FilterQueryBuilder.java @@ -54,6 +54,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor' LT_PRTHS aggField=fieldExpression RT_PRTHS + : PERCENTILE LESS value=integerLiteral GREATER LT_PRTHS aggField=fieldExpression RT_PRTHS ; /** expressions */ diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index 3a90c0f55a..85ebe2703b 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -383,7 +383,7 @@ flowControlFunctionName ; relevanceFunctionName - : MATCH | MATCH_PHRASE + : MATCH | MATCH_PHRASE | MATCHPHRASE ; legacyRelevanceFunctionName From 1fae66581743a25004f2d79e9cfd63b487093d23 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Thu, 19 May 2022 23:26:30 -0700 Subject: [PATCH 50/56] Added missed license headers. Signed-off-by: MaxKsyunz --- .../storage/script/filter/lucene/MatchPhraseQueryTest.java | 5 +++++ .../sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java index 85777f6efd..599834f89c 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.opensearch.storage.script.filter.lucene; diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java index ee410690b3..a4fbee44e3 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/antlr/PPLSyntaxParserMatchPhraseSamplesTest.java @@ -1,3 +1,8 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + package org.opensearch.sql.ppl.antlr; import static org.junit.Assert.assertNotEquals; From d3667cebc145f2a96e610df695198964a8ec5c19 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Fri, 20 May 2022 00:51:45 -0700 Subject: [PATCH 51/56] Add RelevanceQuery -- a base class for MatchQuery and MatchPhraseQuery. Signed-off-by: MaxKsyunz --- .../lucene/relevance/MatchPhraseQuery.java | 57 +++------ .../filter/lucene/relevance/MatchQuery.java | 98 +++++---------- .../lucene/relevance/RelevanceQuery.java | 73 +++++++++++ .../relevance/RelevanceQueryBuildTest.java | 114 ++++++++++++++++++ 4 files changed, 231 insertions(+), 111 deletions(-) create mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java create mode 100644 opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java index 51bc79fab8..1ded3f4708 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchPhraseQuery.java @@ -23,51 +23,22 @@ /** * Lucene query that builds a match_phrase query. */ -public class MatchPhraseQuery extends LuceneQuery { - private final FluentAction analyzer = - (b, v) -> b.analyzer(v.stringValue()); - private final FluentAction slop = - (b, v) -> b.slop(Integer.parseInt(v.stringValue())); - private final FluentAction - zeroTermsQuery = (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); - - interface FluentAction - extends BiFunction { +public class MatchPhraseQuery extends RelevanceQuery { + /** + * Default constructor for MatchPhraseQuery configures how RelevanceQuery.build() handles + * named arguments. + */ + public MatchPhraseQuery() { + super(ImmutableMap.>builder() + .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) + .put("slop", (b, v) -> b.slop(Integer.parseInt(v.stringValue()))) + .put("zero_terms_query", (b, v) -> b.zeroTermsQuery( + org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue()))) + .build()); } - ImmutableMap - argAction = - ImmutableMap.builder() - .put("analyzer", analyzer) - .put("slop", slop) - .put("zero_terms_query", zeroTermsQuery) - .build(); - @Override - public QueryBuilder build(FunctionExpression func) { - List arguments = func.getArguments(); - if (arguments.size() < 2) { - throw new SemanticCheckException("match_phrase requires at least two parameters"); - } - NamedArgumentExpression field = (NamedArgumentExpression) arguments.get(0); - NamedArgumentExpression query = (NamedArgumentExpression) arguments.get(1); - MatchPhraseQueryBuilder queryBuilder = QueryBuilders.matchPhraseQuery( - field.getValue().valueOf(null).stringValue(), - query.getValue().valueOf(null).stringValue()); - - Iterator iterator = arguments.listIterator(2); - while (iterator.hasNext()) { - NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); - if (!argAction.containsKey(arg.getArgName())) { - throw new SemanticCheckException(String - .format("Parameter %s is invalid for match_phrase function.", arg.getArgName())); - } - (Objects.requireNonNull( - argAction - .get(arg.getArgName()))) - .apply(queryBuilder, arg.getValue().valueOf(null)); - } - return queryBuilder; + protected MatchPhraseQueryBuilder createQueryBuilder(String field, String query) { + return QueryBuilders.matchPhraseQuery(field, query); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java index bcdcc9f296..c69b43cbcb 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MatchQuery.java @@ -6,79 +6,41 @@ package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; import com.google.common.collect.ImmutableMap; -import java.util.Iterator; -import java.util.function.BiFunction; +import java.util.Map; import org.opensearch.index.query.MatchQueryBuilder; import org.opensearch.index.query.Operator; -import org.opensearch.index.query.QueryBuilder; import org.opensearch.index.query.QueryBuilders; -import org.opensearch.sql.data.model.ExprValue; -import org.opensearch.sql.exception.SemanticCheckException; -import org.opensearch.sql.expression.Expression; -import org.opensearch.sql.expression.FunctionExpression; -import org.opensearch.sql.expression.NamedArgumentExpression; -import org.opensearch.sql.opensearch.storage.script.filter.lucene.LuceneQuery; -public class MatchQuery extends LuceneQuery { - private final BiFunction analyzer = - (b, v) -> b.analyzer(v.stringValue()); - private final BiFunction synonymsPhrase = - (b, v) -> b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue())); - private final BiFunction fuzziness = - (b, v) -> b.fuzziness(v.stringValue()); - private final BiFunction maxExpansions = - (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue())); - private final BiFunction prefixLength = - (b, v) -> b.prefixLength(Integer.parseInt(v.stringValue())); - private final BiFunction fuzzyTranspositions = - (b, v) -> b.fuzzyTranspositions(Boolean.parseBoolean(v.stringValue())); - private final BiFunction fuzzyRewrite = - (b, v) -> b.fuzzyRewrite(v.stringValue()); - private final BiFunction lenient = - (b, v) -> b.lenient(Boolean.parseBoolean(v.stringValue())); - private final BiFunction operator = - (b, v) -> b.operator(Operator.fromString(v.stringValue())); - private final BiFunction minimumShouldMatch = - (b, v) -> b.minimumShouldMatch(v.stringValue()); - private final BiFunction zeroTermsQuery = - (b, v) -> b.zeroTermsQuery( - org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue())); - private final BiFunction boost = - (b, v) -> b.boost(Float.parseFloat(v.stringValue())); - - ImmutableMap argAction = ImmutableMap.builder() - .put("analyzer", analyzer) - .put("auto_generate_synonyms_phrase_query", synonymsPhrase) - .put("fuzziness", fuzziness) - .put("max_expansions", maxExpansions) - .put("prefix_length", prefixLength) - .put("fuzzy_transpositions", fuzzyTranspositions) - .put("fuzzy_rewrite", fuzzyRewrite) - .put("lenient", lenient) - .put("operator", operator) - .put("minimum_should_match", minimumShouldMatch) - .put("zero_terms_query", zeroTermsQuery) - .put("boost", boost) - .build(); +/** + * Initializes MatchQueryBuilder from a FunctionExpression. + */ +public class MatchQuery extends RelevanceQuery { + /** + * Default constructor for MatchQuery configures how RelevanceQuery.build() handles + * named arguments. + */ + public MatchQuery() { + super(ImmutableMap.>builder() + .put("analyzer", (b, v) -> b.analyzer(v.stringValue())) + .put("auto_generate_synonyms_phrase_query", + (b, v) -> b.autoGenerateSynonymsPhraseQuery(Boolean.parseBoolean(v.stringValue()))) + .put("fuzziness", (b, v) -> b.fuzziness(v.stringValue())) + .put("max_expansions", (b, v) -> b.maxExpansions(Integer.parseInt(v.stringValue()))) + .put("prefix_length", (b, v) -> b.prefixLength(Integer.parseInt(v.stringValue()))) + .put("fuzzy_transpositions", + (b, v) -> b.fuzzyTranspositions(Boolean.parseBoolean(v.stringValue()))) + .put("fuzzy_rewrite", (b, v) -> b.fuzzyRewrite(v.stringValue())) + .put("lenient", (b, v) -> b.lenient(Boolean.parseBoolean(v.stringValue()))) + .put("operator", (b, v) -> b.operator(Operator.fromString(v.stringValue()))) + .put("minimum_should_match", (b, v) -> b.minimumShouldMatch(v.stringValue())) + .put("zero_terms_query", (b, v) -> b.zeroTermsQuery( + org.opensearch.index.search.MatchQuery.ZeroTermsQuery.valueOf(v.stringValue()))) + .put("boost", (b, v) -> b.boost(Float.parseFloat(v.stringValue()))) + .build()); + } @Override - public QueryBuilder build(FunctionExpression func) { - Iterator iterator = func.getArguments().iterator(); - NamedArgumentExpression field = (NamedArgumentExpression) iterator.next(); - NamedArgumentExpression query = (NamedArgumentExpression) iterator.next(); - MatchQueryBuilder queryBuilder = QueryBuilders.matchQuery( - field.getValue().valueOf(null).stringValue(), - query.getValue().valueOf(null).stringValue()); - while (iterator.hasNext()) { - NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); - if (!argAction.containsKey(arg.getArgName())) { - throw new SemanticCheckException(String - .format("Parameter %s is invalid for match function.", arg.getArgName())); - } - ((BiFunction) argAction - .get(arg.getArgName())) - .apply(queryBuilder, arg.getValue().valueOf(null)); - } - return queryBuilder; + protected MatchQueryBuilder createQueryBuilder(String field, String query) { + return QueryBuilders.matchQuery(field, query); } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java new file mode 100644 index 0000000000..d6883144ec --- /dev/null +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java @@ -0,0 +1,73 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; + +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.function.BiFunction; +import org.opensearch.index.query.MatchPhraseQueryBuilder; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.NamedArgumentExpression; +import org.opensearch.sql.opensearch.storage.script.filter.lucene.LuceneQuery; + +/** + * Base class for query abstraction that builds a relevance query from function expression. + */ +public abstract class RelevanceQuery extends LuceneQuery { + protected Map> queryBuildActions; + + protected RelevanceQuery(Map> actionMap) { + queryBuildActions = actionMap; + } + + @Override + public QueryBuilder build(FunctionExpression func) { + List arguments = func.getArguments(); + if (arguments.size() < 2) { + String queryName = createQueryBuilder("", "").getWriteableName(); + throw new SemanticCheckException( + String.format("%s requires at least two parameters", queryName)); + } + NamedArgumentExpression field = (NamedArgumentExpression) arguments.get(0); + NamedArgumentExpression query = (NamedArgumentExpression) arguments.get(1); + T queryBuilder = createQueryBuilder( + field.getValue().valueOf(null).stringValue(), + query.getValue().valueOf(null).stringValue()); + + Iterator iterator = arguments.listIterator(2); + while (iterator.hasNext()) { + NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); + if (!queryBuildActions.containsKey(arg.getArgName())) { + throw new SemanticCheckException(String + .format("Parameter %s is invalid for %s function.", arg.getArgName(), queryBuilder.getWriteableName())); + } + (Objects.requireNonNull( + queryBuildActions + .get(arg.getArgName()))) + .apply(queryBuilder, arg.getValue().valueOf(null)); + } + return queryBuilder; + } + + protected abstract T createQueryBuilder(String field, String query); + + /** + * Convenience interface for a function that updates a QueryBuilder + * based on ExprValue. + * @param Concrete query builder + */ + public interface QueryBuilderStep extends + BiFunction { + + } +} diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java new file mode 100644 index 0000000000..97e956affd --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java @@ -0,0 +1,114 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; +import static org.mockito.Mockito.withSettings; + +import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.stream.Stream; +import org.apache.commons.lang3.NotImplementedException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import org.mockito.Mockito; +import org.opensearch.index.query.QueryBuilder; +import org.opensearch.sql.data.model.ExprStringValue; +import org.opensearch.sql.data.model.ExprValue; +import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.exception.SemanticCheckException; +import org.opensearch.sql.expression.Expression; +import org.opensearch.sql.expression.FunctionExpression; +import org.opensearch.sql.expression.LiteralExpression; +import org.opensearch.sql.expression.NamedArgumentExpression; +import org.opensearch.sql.expression.env.Environment; +import org.opensearch.sql.expression.function.FunctionName; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +class RelevanceQueryBuildTest { + + public static final NamedArgumentExpression FIELD_ARG = namedArgument("field", "field_A"); + public static final NamedArgumentExpression QUERY_ARG = namedArgument("query", "find me"); + private RelevanceQuery query; + private QueryBuilder queryBuilder; + + @BeforeEach + public void setUp() { + query = mock(RelevanceQuery.class, withSettings().useConstructor( + ImmutableMap.>builder() + .put("boost", (k, v) -> k.boost(Float.parseFloat(v.stringValue()))).build()) + .defaultAnswer(Mockito.CALLS_REAL_METHODS)); + queryBuilder = mock(QueryBuilder.class); + when(query.createQueryBuilder(any(), any())).thenReturn(queryBuilder); + when(queryBuilder.queryName()).thenReturn("mocked_query"); + } + + @Test + void first_arg_field_second_arg_query_test() { + query.build(createCall(List.of(FIELD_ARG, QUERY_ARG))); + verify(query, times(1)).createQueryBuilder("field_A", "find me"); + } + + @Test + void throws_SemanticCheckException_when_wrong_argument_name() { + FunctionExpression expr = + createCall(List.of(FIELD_ARG, QUERY_ARG, namedArgument("wrongArg", "value"))); + + assertThrows(SemanticCheckException.class, () -> query.build(expr)); + } + + @Test + void calls_action_when_correct_argument_name() { + FunctionExpression expr = + createCall(List.of(FIELD_ARG, QUERY_ARG, namedArgument("boost", "2.3"))); + query.build(expr); + + verify(queryBuilder, times(1)).boost(2.3f); + } + + @ParameterizedTest + @MethodSource("insufficientArguments") + public void throws_SemanticCheckException_when_no_required_arguments(List arguments) { + assertThrows(SemanticCheckException.class, () -> query.build(createCall(arguments))); + } + + public static Stream> insufficientArguments() { + return Stream.of(List.of(), + List.of(namedArgument("field", "field_A"))); + } + + private static NamedArgumentExpression namedArgument(String field, String fieldValue) { + return new NamedArgumentExpression(field, createLiteral(fieldValue)); + } + + @Test + private static Expression createLiteral(String value) { + return new LiteralExpression(new ExprStringValue(value)); + } + + private static FunctionExpression createCall(List arguments) { + return new FunctionExpression(new FunctionName("mock_function"), arguments) { + @Override + public ExprValue valueOf(Environment valueEnv) { + throw new NotImplementedException("FunctionExpression.valueOf"); + } + + @Override + public ExprType type() { + throw new NotImplementedException("FunctionExpression.type"); + } + }; + } +} \ No newline at end of file From 7101d072dca42ce4c620210dda7ab8544ec32e4e Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Fri, 20 May 2022 02:20:09 -0700 Subject: [PATCH 52/56] Use SyntaxCheckException from RelevanceQuery.build Also, use getWriteableName() to get the name of the query in the error message. Signed-off-by: MaxKsyunz --- .../filter/lucene/relevance/RelevanceQuery.java | 10 +++++----- .../script/filter/lucene/MatchPhraseQueryTest.java | 12 ++++++------ .../lucene/relevance/RelevanceQueryBuildTest.java | 5 +++-- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java index d6883144ec..0a90370f62 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java @@ -10,9 +10,8 @@ import java.util.Map; import java.util.Objects; import java.util.function.BiFunction; -import org.opensearch.index.query.MatchPhraseQueryBuilder; import org.opensearch.index.query.QueryBuilder; -import org.opensearch.index.query.QueryBuilders; +import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.Expression; @@ -35,7 +34,7 @@ public QueryBuilder build(FunctionExpression func) { List arguments = func.getArguments(); if (arguments.size() < 2) { String queryName = createQueryBuilder("", "").getWriteableName(); - throw new SemanticCheckException( + throw new SyntaxCheckException( String.format("%s requires at least two parameters", queryName)); } NamedArgumentExpression field = (NamedArgumentExpression) arguments.get(0); @@ -48,8 +47,9 @@ public QueryBuilder build(FunctionExpression func) { while (iterator.hasNext()) { NamedArgumentExpression arg = (NamedArgumentExpression) iterator.next(); if (!queryBuildActions.containsKey(arg.getArgName())) { - throw new SemanticCheckException(String - .format("Parameter %s is invalid for %s function.", arg.getArgName(), queryBuilder.getWriteableName())); + throw new SemanticCheckException( + String.format("Parameter %s is invalid for %s function.", + arg.getArgName(), queryBuilder.getWriteableName())); } (Objects.requireNonNull( queryBuildActions diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java index 599834f89c..fef3d64f95 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchPhraseQueryTest.java @@ -13,6 +13,7 @@ import org.junit.jupiter.api.DisplayNameGeneration; import org.junit.jupiter.api.DisplayNameGenerator; import org.junit.jupiter.api.Test; +import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.SemanticCheckException; @@ -37,22 +38,21 @@ private NamedArgumentExpression namedArgument(String name, String value) { } @Test - public void test_SemanticCheckException_when_no_arguments() { - + public void test_SyntaxCheckException_when_no_arguments() { List arguments = List.of(); - assertThrows(SemanticCheckException.class, + assertThrows(SyntaxCheckException.class, () -> matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } @Test - public void test_SemanticCheckException_when_one_argument() { + public void test_SyntaxCheckException_when_one_argument() { List arguments = List.of(namedArgument("field", "test")); - assertThrows(SemanticCheckException.class, + assertThrows(SyntaxCheckException.class, () -> matchPhraseQuery.build(new MatchPhraseExpression(arguments))); } @Test - public void test_SemanticCheckException_when_invalid_parameter() { + public void test_SyntaxCheckException_when_invalid_parameter() { List arguments = List.of( namedArgument("field", "test"), namedArgument("query", "test2"), diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java index 97e956affd..8d0cc9fa7d 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java @@ -25,6 +25,7 @@ import org.junit.jupiter.params.provider.MethodSource; import org.mockito.Mockito; import org.opensearch.index.query.QueryBuilder; +import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.data.model.ExprStringValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; @@ -80,8 +81,8 @@ void calls_action_when_correct_argument_name() { @ParameterizedTest @MethodSource("insufficientArguments") - public void throws_SemanticCheckException_when_no_required_arguments(List arguments) { - assertThrows(SemanticCheckException.class, () -> query.build(createCall(arguments))); + public void throws_SyntaxCheckException_when_no_required_arguments(List arguments) { + assertThrows(SyntaxCheckException.class, () -> query.build(createCall(arguments))); } public static Stream> insufficientArguments() { From b5bf44e4eb4e8bdd20124f1d076ee1a83e5b14bf Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Fri, 20 May 2022 16:39:21 -0700 Subject: [PATCH 53/56] MatchPhraseQueryBuilder constructor requires a non-empty field name. This dummy instance of MatchPhraseQueryBuilder is used in RelevanceQuery when creating an exception to get the name of the query. Signed-off-by: MaxKsyunz --- .../storage/script/filter/lucene/relevance/RelevanceQuery.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java index 0a90370f62..fb0852c18b 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQuery.java @@ -33,7 +33,7 @@ protected RelevanceQuery(Map> actionMap) { public QueryBuilder build(FunctionExpression func) { List arguments = func.getArguments(); if (arguments.size() < 2) { - String queryName = createQueryBuilder("", "").getWriteableName(); + String queryName = createQueryBuilder("dummy_field", "").getWriteableName(); throw new SyntaxCheckException( String.format("%s requires at least two parameters", queryName)); } From 17f0ded04cf80f9461a9b4deac9fd907bfb6bf5d Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Fri, 20 May 2022 17:47:03 -0700 Subject: [PATCH 54/56] Add missing newline at the end of the file. Signed-off-by: MaxKsyunz --- .../script/filter/lucene/relevance/RelevanceQueryBuildTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java index 8d0cc9fa7d..cfa876dafe 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java @@ -112,4 +112,4 @@ public ExprType type() { } }; } -} \ No newline at end of file +} From 15775e8a77ad3705aebd651371a61704ecb8e232 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Fri, 20 May 2022 18:23:33 -0700 Subject: [PATCH 55/56] Update tests from upstream/main to verify new behavior. This branch changed RelevanceQuery.build to throw SyntaxCheckException if it is called with less than two arguments. Previously, build would through SemanticCheckException. Signed-off-by: MaxKsyunz --- .../storage/script/filter/lucene/MatchQueryTest.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java index 7e3e5e0862..99cf132a3e 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MatchQueryTest.java @@ -15,6 +15,7 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import org.opensearch.sql.common.antlr.SyntaxCheckException; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.exception.SemanticCheckException; @@ -110,16 +111,16 @@ public void test_valid_parameters(List validArgs) { } @Test - public void test_SemanticCheckException_when_no_arguments() { + public void test_SyntaxCheckException_when_no_arguments() { List arguments = List.of(); - assertThrows(SemanticCheckException.class, + assertThrows(SyntaxCheckException.class, () -> matchQuery.build(new MatchExpression(arguments))); } @Test - public void test_SemanticCheckException_when_one_argument() { + public void test_SyntaxCheckException_when_one_argument() { List arguments = List.of(namedArgument("field", "field_value")); - assertThrows(SemanticCheckException.class, + assertThrows(SyntaxCheckException.class, () -> matchQuery.build(new MatchExpression(arguments))); } From ee4e31969f47a6e05a0f831b35d4c00168a032a0 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 25 May 2022 14:10:21 -0700 Subject: [PATCH 56/56] Verify exception messages in RelevanceQueryBuildTest Signed-off-by: MaxKsyunz --- .../lucene/relevance/RelevanceQueryBuildTest.java | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java index cfa876dafe..1186031f5f 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/RelevanceQueryBuildTest.java @@ -5,6 +5,7 @@ package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; +import static org.junit.Assert.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -29,6 +30,7 @@ import org.opensearch.sql.data.model.ExprStringValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; +import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.expression.FunctionExpression; @@ -54,6 +56,7 @@ public void setUp() { queryBuilder = mock(QueryBuilder.class); when(query.createQueryBuilder(any(), any())).thenReturn(queryBuilder); when(queryBuilder.queryName()).thenReturn("mocked_query"); + when(queryBuilder.getWriteableName()).thenReturn("mock_query"); } @Test @@ -67,7 +70,9 @@ void throws_SemanticCheckException_when_wrong_argument_name() { FunctionExpression expr = createCall(List.of(FIELD_ARG, QUERY_ARG, namedArgument("wrongArg", "value"))); - assertThrows(SemanticCheckException.class, () -> query.build(expr)); + SemanticCheckException exception = + assertThrows(SemanticCheckException.class, () -> query.build(expr)); + assertEquals("Parameter wrongArg is invalid for mock_query function.", exception.getMessage()); } @Test @@ -82,7 +87,9 @@ void calls_action_when_correct_argument_name() { @ParameterizedTest @MethodSource("insufficientArguments") public void throws_SyntaxCheckException_when_no_required_arguments(List arguments) { - assertThrows(SyntaxCheckException.class, () -> query.build(createCall(arguments))); + SyntaxCheckException exception = assertThrows(SyntaxCheckException.class, + () -> query.build(createCall(arguments))); + assertEquals("mock_query requires at least two parameters", exception.getMessage()); } public static Stream> insufficientArguments() {