From 4b402287bf08b3d81278e515ac4998dbb033ee6b Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 4 May 2022 18:00:52 -0700 Subject: [PATCH 01/12] Resolve an ANTLR warning In the parser, use tokens from the lexer instead of string literals. 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 f44db2f834..11a42d8d0b 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -171,7 +171,7 @@ statsFunctionName ; percentileAggFunction - : PERCENTILE '<' value=integerLiteral '>' LT_PRTHS aggField=fieldExpression RT_PRTHS + : PERCENTILE LESS value=integerLiteral GREATER LT_PRTHS aggField=fieldExpression RT_PRTHS ; /** expressions */ From e0feaadc9ccc5ca66f334a9528e748af4dc84d78 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 4 May 2022 23:31:03 -0700 Subject: [PATCH 02/12] Support matchphrase in ExpressionBuilder and FilterQueryBuilder. Signed-off-by: MaxKsyunz --- .../sql/expression/function/BuiltinFunctionName.java | 1 + .../sql/expression/function/OpenSearchFunctions.java | 11 ++++++++--- .../storage/script/filter/FilterQueryBuilder.java | 1 + 3 files changed, 10 insertions(+), 3 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 2d632d4109..a0a6cc9a4d 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,7 +30,8 @@ public class OpenSearchFunctions { public void register(BuiltinFunctionRepository repository) { repository.register(match()); - repository.register(match_phrase()); + repository.register(match_phrase(BuiltinFunctionName.MATCH_PHRASE)); + repository.register(match_phrase(BuiltinFunctionName.MATCHPHRASE)); } private static FunctionResolver match() { @@ -38,8 +39,12 @@ 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 matchphrase() { + FunctionName funcName = BuiltinFunctionName.MATCHPHRASE.getName(); + return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); + } + private static FunctionResolver match_phrase(BuiltinFunctionName matchPhrase) { + FunctionName funcName = matchPhrase.getName(); return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); } 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 Date: Thu, 5 May 2022 14:03:31 -0700 Subject: [PATCH 03/12] Add `MATCHPHRASE` to 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 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 5afbda6ef239ec7e794ff08dd2ee7cea1ba8b9d5 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Thu, 5 May 2022 14:05:20 -0700 Subject: [PATCH 04/12] Add `MATCHPHRASE` to integration tests. Signed-off-by: Yury Fridlyand --- .../org/opensearch/sql/ppl/WhereCommandIT.java | 18 ++++++++++++++++++ .../sql/sql/RelevanceFunctionIT.java | 13 +++++++++++++ 2 files changed, 31 insertions(+) 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 e5f8f35448..1c0e0c8fea 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 @@ -131,4 +131,22 @@ public void testMathPhraseWithSlop() throws IOException { "source=%s | where match_phrase(phrase, 'brown fox', slop = 2) | fields phrase", TEST_INDEX_PHRASE)); verifyDataRows(result, rows("brown fox"), rows("fox brown")); } + + @Test + public void testMatchPhraseNoUnderscoreFunction() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | where matchphrase(firstname, 'Hattie') | fields firstname", TEST_INDEX_BANK)); + verifyDataRows(result, rows("Hattie")); + } + + @Test + public void testMathPhraseNoUnderscoreWithSlop() throws IOException { + JSONObject result = + executeQuery( + String.format( + "source=%s | where matchphrase(firstname, 'Hattie', slop = 2) | fields firstname", TEST_INDEX_BANK)); + verifyDataRows(result, rows("Hattie")); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java index dd8d79fbd7..b2f0c365fc 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java @@ -35,4 +35,17 @@ void match_in_having() throws IOException { verifyDataRows(result, rows("Bates")); } + @Test + void match_phrase_in_where() throws IOException { + JSONObject result = executeQuery("SELECT firstname WHERE match_phrase(lastname, 'Bates')"); + verifySchema(result, schema("firstname", "text")); + verifyDataRows(result, rows("Nanette")); + } + + @Test + void matchphrase_in_where() throws IOException { + JSONObject result = executeQuery("SELECT firstname WHERE matchphrase(lastname, 'Bates')"); + verifySchema(result, schema("firstname", "text")); + verifyDataRows(result, rows("Nanette")); + } } From e19272d7a68440d382e157d829b7962ce9132d07 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Thu, 5 May 2022 18:47:18 -0700 Subject: [PATCH 05/12] Fix code style. Signed-off-by: Yury Fridlyand --- .../opensearch/sql/expression/function/OpenSearchFunctions.java | 2 ++ 1 file changed, 2 insertions(+) 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 a0a6cc9a4d..70de1a9caa 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,6 +30,8 @@ public class OpenSearchFunctions { public void register(BuiltinFunctionRepository repository) { repository.register(match()); + // Register MATCHPHRASE as MATCH_PHRASE as well for backwards + // compatibility. repository.register(match_phrase(BuiltinFunctionName.MATCH_PHRASE)); repository.register(match_phrase(BuiltinFunctionName.MATCHPHRASE)); } From a5f8ea46e889048918de1ce647c3dbbbe1b8cc0c Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Thu, 5 May 2022 18:48:32 -0700 Subject: [PATCH 06/12] Disable PPL integration tests for `MATCH_PHRASE`. Signed-off-by: Yury Fridlyand --- .../test/java/org/opensearch/sql/ppl/WhereCommandIT.java | 6 ++++-- 1 file changed, 4 insertions(+), 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 1c0e0c8fea..95d746ad23 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 @@ -133,7 +133,8 @@ public void testMathPhraseWithSlop() throws IOException { } @Test - public void testMatchPhraseNoUnderscoreFunction() throws IOException { + @Disabled("`MATCHPHRASE` (without '_') is not supported [yet] in PPL.") + public void notestMatchPhraseNoUnderscoreFunction() throws IOException { JSONObject result = executeQuery( String.format( @@ -142,7 +143,8 @@ public void testMatchPhraseNoUnderscoreFunction() throws IOException { } @Test - public void testMathPhraseNoUnderscoreWithSlop() throws IOException { + @Disabled("`MATCHPHRASE` (without '_') is not supported [yet] in PPL.") + public void notestMathPhraseNoUnderscoreWithSlop() throws IOException { JSONObject result = executeQuery( String.format( From 733f9f63a6d65f3677c4b94074c8f0c137356404 Mon Sep 17 00:00:00 2001 From: Yury Fridlyand Date: Thu, 5 May 2022 19:54:15 -0700 Subject: [PATCH 07/12] Add `MATCHPHRASE` to docs. Signed-off-by: Yury Fridlyand --- docs/user/dql/functions.rst | 2 ++ docs/user/ppl/functions/relevance.rst | 2 ++ 2 files changed, 4 insertions(+) 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 From 28ef64ffa2bfc55f04de9aa374412b5befbb6ccc Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 10 May 2022 16:58:14 -0700 Subject: [PATCH 08/12] Fix checkstyle failure. Signed-off-by: MaxKsyunz --- .../sql/expression/function/OpenSearchFunctions.java | 4 ++++ 1 file changed, 4 insertions(+) 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 70de1a9caa..6022f43cbe 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,6 +28,9 @@ 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()); // Register MATCHPHRASE as MATCH_PHRASE as well for backwards @@ -45,6 +48,7 @@ private static FunctionResolver matchphrase() { FunctionName funcName = BuiltinFunctionName.MATCHPHRASE.getName(); return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); } + private static FunctionResolver match_phrase(BuiltinFunctionName matchPhrase) { FunctionName funcName = matchPhrase.getName(); return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); From 0d59a5e9373aa44d20c7e275505875256607c864 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 10 May 2022 16:59:32 -0700 Subject: [PATCH 09/12] Remove unused method. Signed-off-by: MaxKsyunz --- .../sql/expression/function/OpenSearchFunctions.java | 5 ----- 1 file changed, 5 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 6022f43cbe..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 @@ -44,11 +44,6 @@ private static FunctionResolver match() { return getRelevanceFunctionResolver(funcName, MATCH_MAX_NUM_PARAMETERS); } - private static FunctionResolver matchphrase() { - FunctionName funcName = BuiltinFunctionName.MATCHPHRASE.getName(); - return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); - } - private static FunctionResolver match_phrase(BuiltinFunctionName matchPhrase) { FunctionName funcName = matchPhrase.getName(); return getRelevanceFunctionResolver(funcName, MATCH_PHRASE_MAX_NUM_PARAMETERS); From d5b91c308027dd472fa3becbfb1625dfe5cd387f Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Tue, 10 May 2022 18:16:48 -0700 Subject: [PATCH 10/12] Remove disabled tests. We confirmed that PPL does not need to support matchphrase variant. Signed-off-by: MaxKsyunz --- .../opensearch/sql/ppl/WhereCommandIT.java | 20 ------------------- 1 file changed, 20 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 95d746ad23..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 @@ -131,24 +131,4 @@ public void testMathPhraseWithSlop() throws IOException { "source=%s | where match_phrase(phrase, 'brown fox', slop = 2) | fields phrase", TEST_INDEX_PHRASE)); verifyDataRows(result, rows("brown fox"), rows("fox brown")); } - - @Test - @Disabled("`MATCHPHRASE` (without '_') is not supported [yet] in PPL.") - public void notestMatchPhraseNoUnderscoreFunction() throws IOException { - JSONObject result = - executeQuery( - String.format( - "source=%s | where matchphrase(firstname, 'Hattie') | fields firstname", TEST_INDEX_BANK)); - verifyDataRows(result, rows("Hattie")); - } - - @Test - @Disabled("`MATCHPHRASE` (without '_') is not supported [yet] in PPL.") - public void notestMathPhraseNoUnderscoreWithSlop() throws IOException { - JSONObject result = - executeQuery( - String.format( - "source=%s | where matchphrase(firstname, 'Hattie', slop = 2) | fields firstname", TEST_INDEX_BANK)); - verifyDataRows(result, rows("Hattie")); - } } From 2b95ff10655b6109b7bc30dbd733eb43eb15a8b6 Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Wed, 11 May 2022 21:42:08 -0700 Subject: [PATCH 11/12] Remove test that's currently not being run. Signed-off-by: MaxKsyunz --- .../java/org/opensearch/sql/sql/RelevanceFunctionIT.java | 7 ------- 1 file changed, 7 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java index b2f0c365fc..fba633af8b 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java @@ -41,11 +41,4 @@ void match_phrase_in_where() throws IOException { verifySchema(result, schema("firstname", "text")); verifyDataRows(result, rows("Nanette")); } - - @Test - void matchphrase_in_where() throws IOException { - JSONObject result = executeQuery("SELECT firstname WHERE matchphrase(lastname, 'Bates')"); - verifySchema(result, schema("firstname", "text")); - verifyDataRows(result, rows("Nanette")); - } } From 9dd66632eb4616e4294cb927023ce87c07ba592f Mon Sep 17 00:00:00 2001 From: MaxKsyunz Date: Thu, 12 May 2022 01:02:43 -0700 Subject: [PATCH 12/12] Remove test that's currently not being run. part 2. Signed-off-by: MaxKsyunz --- .../java/org/opensearch/sql/sql/RelevanceFunctionIT.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java index fba633af8b..dd8d79fbd7 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/RelevanceFunctionIT.java @@ -35,10 +35,4 @@ void match_in_having() throws IOException { verifyDataRows(result, rows("Bates")); } - @Test - void match_phrase_in_where() throws IOException { - JSONObject result = executeQuery("SELECT firstname WHERE match_phrase(lastname, 'Bates')"); - verifySchema(result, schema("firstname", "text")); - verifyDataRows(result, rows("Nanette")); - } }