From 290112b693ab0afb878862ac0a7806903785a95c Mon Sep 17 00:00:00 2001 From: GabeFernandez310 Date: Tue, 13 Dec 2022 08:44:36 -0800 Subject: [PATCH] Add Alternate Syntax For Match_Query And Other Functions Added Tests And Implementation For Match_Query, Match_Phrase, and Multi_Match Functions Signed-off-by: GabeFernandez310 --- docs/user/dql/functions.rst | 55 +++++++++++++ .../opensearch/sql/legacy/MethodQueryIT.java | 2 +- .../java/org/opensearch/sql/sql/MatchIT.java | 33 +++++++- .../org/opensearch/sql/sql/MatchPhraseIT.java | 28 ++++++- .../org/opensearch/sql/sql/MultiMatchIT.java | 31 +++++++ sql/src/main/antlr/OpenSearchSQLParser.g4 | 27 ++++++- .../sql/sql/parser/AstExpressionBuilder.java | 43 ++++++++++ .../sql/sql/antlr/SQLSyntaxParserTest.java | 24 ++++++ .../sql/parser/AstExpressionBuilderTest.java | 81 +++++++++++++++++++ 9 files changed, 320 insertions(+), 4 deletions(-) diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 7be50ccffb..2f360ffe4b 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -2888,6 +2888,16 @@ Another example to show how to set custom values for the optional parameters:: | Bond | +------------+ + The matchquery function also supports an alternative syntax:: + + os> SELECT firstname FROM accounts WHERE firstname = matchquery('Hattie'); + fetched rows / total rows = 1/1 + +-------------+ + | firstname | + |-------------| + | Hattie | + +-------------+ + MATCH_QUERY ----- @@ -2917,6 +2927,16 @@ Another example to show how to set custom values for the optional parameters:: | Bond | +------------+ +The match_query function also supports an alternative syntax:: + + os> SELECT firstname FROM accounts WHERE firstname = match_query('Hattie'); + fetched rows / total rows = 1/1 + +-------------+ + | firstname | + |-------------| + | Hattie | + +-------------+ + MATCH_PHRASE ------------ @@ -2956,6 +2976,23 @@ Another example to show how to set custom values for the optional parameters:: | Alan Alexander Milne | Winnie-the-Pooh | +----------------------+--------------------------+ +The match_phrase function also supports an alternative syntax:: + + os> SELECT firstname FROM accounts WHERE firstname = match_phrase('Hattie'); + fetched rows / total rows = 1/1 + +-------------+ + | firstname | + |-------------| + | Hattie | + +-------------+ + + os> SELECT firstname FROM accounts WHERE firstname = matchphrase('Hattie'); + fetched rows / total rows = 1/1 + +-------------+ + | firstname | + |-------------| + | Hattie | + +-------------+ MATCH_BOOL_PREFIX ----- @@ -3099,6 +3136,24 @@ Another example to show how to set custom values for the optional parameters:: | 1 | The House at Pooh Corner | Alan Alexander Milne | +------+--------------------------+----------------------+ +The multi_match function also supports an alternative syntax:: + + os> SELECT firstname FROM accounts WHERE firstname = multi_match('Hattie'); + fetched rows / total rows = 1/1 + +-------------+ + | firstname | + |-------------| + | Hattie | + +-------------+ + + os> SELECT firstname FROM accounts WHERE firstname = multimatch('Hattie'); + fetched rows / total rows = 1/1 + +-------------+ + | firstname | + |-------------| + | Hattie | + +-------------+ + SIMPLE_QUERY_STRING ------------------- diff --git a/integ-test/src/test/java/org/opensearch/sql/legacy/MethodQueryIT.java b/integ-test/src/test/java/org/opensearch/sql/legacy/MethodQueryIT.java index b1b6b24ada..fdbbb0f6ba 100644 --- a/integ-test/src/test/java/org/opensearch/sql/legacy/MethodQueryIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/legacy/MethodQueryIT.java @@ -54,7 +54,7 @@ public void matchQueryTest() throws IOException { "select address from %s where address= matchQuery('880 Holmes Lane') limit 3", TestsConstants.TEST_INDEX_ACCOUNT)); Assert.assertThat(result, - containsString("{\"match\":{\"address\":{\"query\":\"880 Holmes Lane\"")); + containsString("{\\\"match\\\":{\\\"address\\\":{\\\"query\\\":\\\"880 Holmes Lane\\\"")); } /** diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java index 09e3504e4a..404a0476fd 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/MatchIT.java @@ -66,7 +66,7 @@ public void match_query_in_having() throws IOException { } @Test - public void alternate_syntaxes_return_the_same_results() throws IOException { + public void match_aliases_return_the_same_results() throws IOException { String query1 = "SELECT lastname FROM " + TEST_INDEX_ACCOUNT + " HAVING match(firstname, 'Nanette')"; JSONObject result1 = executeJdbcRequest(query1); @@ -79,4 +79,35 @@ public void alternate_syntaxes_return_the_same_results() throws IOException { assertEquals(result1.getInt("total"), result2.getInt("total")); assertEquals(result1.getInt("total"), result3.getInt("total")); } + + @Test + public void match_query_alternate_syntax() throws IOException { + JSONObject result = executeJdbcRequest( + "SELECT lastname FROM " + TEST_INDEX_ACCOUNT + " WHERE lastname = match_query('Bates')"); + verifySchema(result, schema("lastname", "text")); + verifyDataRows(result, rows("Bates")); + } + + @Test + public void matchquery_alternate_syntax() throws IOException { + JSONObject result = executeJdbcRequest( + "SELECT lastname FROM " + TEST_INDEX_ACCOUNT + " WHERE lastname = matchquery('Bates')"); + verifySchema(result, schema("lastname", "text")); + verifyDataRows(result, rows("Bates")); + } + + @Test + public void match_alternate_syntaxes_return_the_same_results() throws IOException { + String query1 = "SELECT * FROM " + + TEST_INDEX_ACCOUNT + " WHERE match(firstname, 'Nanette')"; + JSONObject result1 = executeJdbcRequest(query1); + String query2 = "SELECT * FROM " + + TEST_INDEX_ACCOUNT + " WHERE firstname = match_query('Nanette')"; + JSONObject result2 = executeJdbcRequest(query2); + String query3 = "SELECT * FROM " + + TEST_INDEX_ACCOUNT + " WHERE firstname = matchquery('Nanette')"; + JSONObject result3 = executeJdbcRequest(query3); + assertEquals(result1.getInt("total"), result2.getInt("total")); + assertEquals(result1.getInt("total"), result3.getInt("total")); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/MatchPhraseIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/MatchPhraseIT.java index b870a60604..3b7e65dcc6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/MatchPhraseIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/MatchPhraseIT.java @@ -51,7 +51,7 @@ public void test_match_phrase_with_slop() throws IOException { } @Test - public void test_alternate_syntax_for_match_phrase_returns_same_result() throws IOException { + public void test_aliases_for_match_phrase_returns_same_result() throws IOException { String query1 = "SELECT phrase FROM %s WHERE matchphrase(phrase, 'quick fox')"; String query2 = "SELECT phrase FROM %s WHERE match_phrase(phrase, 'quick fox')"; String query3 = "SELECT phrase FROM %s WHERE matchphrasequery(phrase, 'quick fox')"; @@ -61,4 +61,30 @@ public void test_alternate_syntax_for_match_phrase_returns_same_result() throws assertTrue(result1.similar(result2)); assertTrue(result1.similar(result3)); } + + @Test + public void match_phrase_alternate_syntax() throws IOException { + String query = "SELECT phrase FROM %s WHERE phrase = match_phrase('quick fox')"; + JSONObject result = executeJdbcRequest(String.format(query, TEST_INDEX_PHRASE)); + verifyDataRows(result, rows("quick fox"), rows("quick fox here")); + } + + @Test + public void matchphrase_alternate_syntax() throws IOException { + String query = "SELECT phrase FROM %s WHERE phrase = matchphrase('quick fox')"; + JSONObject result = executeJdbcRequest(String.format(query, TEST_INDEX_PHRASE)); + verifyDataRows(result, rows("quick fox"), rows("quick fox here")); + } + + @Test + public void match_phrase_alternate_syntaxes_return_the_same_results() throws IOException { + String query1 = "SELECT phrase FROM %s WHERE matchphrase(phrase, 'quick fox')"; + String query2 = "SELECT phrase FROM %s WHERE phrase = matchphrase('quick fox')"; + String query3 = "SELECT phrase FROM %s WHERE phrase = match_phrase('quick fox')"; + JSONObject result1 = executeJdbcRequest(String.format(query1, TEST_INDEX_PHRASE)); + JSONObject result2 = executeJdbcRequest(String.format(query2, TEST_INDEX_PHRASE)); + JSONObject result3 = executeJdbcRequest(String.format(query3, TEST_INDEX_PHRASE)); + assertTrue(result1.similar(result2)); + assertTrue(result1.similar(result3)); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/MultiMatchIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/MultiMatchIT.java index 24ce45fd20..07c89b4cdf 100644 --- a/integ-test/src/test/java/org/opensearch/sql/sql/MultiMatchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/sql/MultiMatchIT.java @@ -124,4 +124,35 @@ public void test_all_params_multimatchquery_alternate_parameter_syntax() { JSONObject result = executeJdbcRequest(query); assertEquals(2, result.getInt("total")); } + + @Test + public void multi_match_alternate_syntax() throws IOException { + String query = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE CreationDate = multi_match('2014-01-22');"; + var result = new JSONObject(executeQuery(query, "jdbc")); + assertEquals(8, result.getInt("total")); + } + + @Test + public void multimatch_alternate_syntax() throws IOException { + String query = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE CreationDate = multimatch('2014-01-22');"; + var result = new JSONObject(executeQuery(query, "jdbc")); + assertEquals(8, result.getInt("total")); + } + + @Test + public void multi_match_alternate_syntaxes_return_the_same_results() throws IOException { + String query1 = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE multi_match(['CreationDate'], '2014-01-22');"; + String query2 = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE CreationDate = multi_match('2014-01-22');"; + String query3 = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE CreationDate = multimatch('2014-01-22');"; + var result1 = new JSONObject(executeQuery(query1, "jdbc")); + var result2 = new JSONObject(executeQuery(query2, "jdbc")); + var result3 = new JSONObject(executeQuery(query3, "jdbc")); + assertEquals(result1.getInt("total"), result2.getInt("total")); + assertEquals(result1.getInt("total"), result3.getInt("total")); + } } diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index 6e8e0e08fe..6d6c808f60 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -328,6 +328,10 @@ positionFunction : POSITION LR_BRACKET functionArg IN functionArg RR_BRACKET ; +matchQueryAltSyntaxFunction + : field=relevanceField EQUAL_SYMBOL MATCH_QUERY LR_BRACKET query=relevanceQuery RR_BRACKET + ; + scalarFunctionName : mathematicalFunctionName | dateTimeFunctionName @@ -345,7 +349,8 @@ specificFunction ; relevanceFunction - : noFieldRelevanceFunction | singleFieldRelevanceFunction | multiFieldRelevanceFunction + : noFieldRelevanceFunction | singleFieldRelevanceFunction | multiFieldRelevanceFunction | altSingleFieldRelevanceFunction | altMultiFieldRelevanceFunction + ; noFieldRelevanceFunction @@ -367,6 +372,14 @@ multiFieldRelevanceFunction alternateMultiMatchQuery COMMA alternateMultiMatchField (COMMA relevanceArg)* RR_BRACKET ; +altSingleFieldRelevanceFunction + : field=relevanceField EQUAL_SYMBOL altSyntaxFunctionName=altSingleFieldRelevanceFunctionName LR_BRACKET query=relevanceQuery (COMMA relevanceArg)* RR_BRACKET + ; + +altMultiFieldRelevanceFunction + : field=relevanceField EQUAL_SYMBOL altSyntaxFunctionName=altMultiFieldRelevanceFunctionName LR_BRACKET query=relevanceQuery (COMMA relevanceArg)* RR_BRACKET + ; + convertedDataType : typeName=DATE | typeName=TIME @@ -484,6 +497,18 @@ multiFieldRelevanceFunctionName | QUERY_STRING ; +altSingleFieldRelevanceFunctionName + : MATCH_QUERY + | MATCHQUERY + | MATCH_PHRASE + | MATCHPHRASE + ; + +altMultiFieldRelevanceFunctionName + : MULTI_MATCH + | MULTIMATCH + ; + functionArgs : (functionArg (COMMA functionArg)*)? ; diff --git a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java index bae22595ca..b46d8f081d 100644 --- a/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java +++ b/sql/src/main/java/org/opensearch/sql/sql/parser/AstExpressionBuilder.java @@ -406,6 +406,14 @@ public UnresolvedExpression visitSingleFieldRelevanceFunction( singleFieldRelevanceArguments(ctx)); } + @Override + public UnresolvedExpression visitAltSingleFieldRelevanceFunction( + OpenSearchSQLParser.AltSingleFieldRelevanceFunctionContext ctx) { + return new Function( + ctx.altSyntaxFunctionName.getText().toLowerCase(), + altSingleFieldRelevanceFunctionArguments(ctx)); + } + @Override public UnresolvedExpression visitMultiFieldRelevanceFunction( MultiFieldRelevanceFunctionContext ctx) { @@ -427,6 +435,14 @@ public UnresolvedExpression visitMultiFieldRelevanceFunction( } } + @Override + public UnresolvedExpression visitAltMultiFieldRelevanceFunction( + OpenSearchSQLParser.AltMultiFieldRelevanceFunctionContext ctx) { + return new Function( + ctx.altSyntaxFunctionName.getText().toLowerCase(), + altMultiFieldRelevanceFunctionArguments(ctx)); + } + private Function buildFunction(String functionName, List arg) { return new Function( @@ -483,6 +499,18 @@ private List singleFieldRelevanceArguments( } + private List altSingleFieldRelevanceFunctionArguments( + OpenSearchSQLParser.AltSingleFieldRelevanceFunctionContext ctx) { + // all the arguments are defaulted to string values + // to skip environment resolving and function signature resolving + ImmutableList.Builder builder = ImmutableList.builder(); + builder.add(new UnresolvedArgument("field", + new Literal(StringUtils.unquoteText(ctx.field.getText()), DataType.STRING))); + builder.add(new UnresolvedArgument("query", + new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); + fillRelevanceArgs(ctx.relevanceArg(), builder); + return builder.build(); + } private List multiFieldRelevanceArguments( OpenSearchSQLParser.MultiFieldRelevanceFunctionContext ctx) { @@ -538,4 +566,19 @@ private List alternateMultiMatchArguments( return builder.build(); } + + private List altMultiFieldRelevanceFunctionArguments( + OpenSearchSQLParser.AltMultiFieldRelevanceFunctionContext ctx) { + // all the arguments are defaulted to string values + // to skip environment resolving and function signature resolving + var map = new HashMap(); + map.put(ctx.field.getText(), 1F); + ImmutableList.Builder builder = ImmutableList.builder(); + var fields = new RelevanceFieldList(map); + builder.add(new UnresolvedArgument("fields", fields)); + builder.add(new UnresolvedArgument("query", + new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); + fillRelevanceArgs(ctx.relevanceArg(), builder); + return builder.build(); + } } 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 bfd0f93ec9..fbdbe0a3df 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 @@ -498,6 +498,30 @@ private static Stream matchPhraseComplexQueries() { ); } + @Test + public void canParseMatchQueryAlternateSyntax() { + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = matchquery('query')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = matchquery(\"query\")")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = match_query('query')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = match_query(\"query\")")); + } + + @Test + public void canParseMatchPhraseAlternateSyntax() { + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = match_phrase('query')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = match_phrase(\"query\")")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = matchphrase('query')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = matchphrase(\"query\")")); + } + + @Test + public void canParseMultiMatchAlternateSyntax() { + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = multi_match('query')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = multi_match(\"query\")")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = multimatch('query')")); + assertNotNull(parser.parse("SELECT * FROM test WHERE Field = multimatch(\"query\")")); + } + private static Stream matchPhraseQueryComplexQueries() { return Stream.of( "SELECT * FROM t WHERE matchphrasequery(c, 3)", diff --git a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java index 9af4119fdf..07b9ff1f19 100644 --- a/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java +++ b/sql/src/test/java/org/opensearch/sql/sql/parser/AstExpressionBuilderTest.java @@ -541,6 +541,87 @@ public void relevanceMatch_Query() { buildExprAst("match_query('message', 'search query', analyzer='keyword', operator='AND')")); } + @Test + public void relevanceMatchQueryAltSyntax() { + assertEquals(AstDSL.function("match_query", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = match_query('search query')") + ); + + assertEquals(AstDSL.function("match_query", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = match_query(\"search query\")") + ); + + assertEquals(AstDSL.function("matchquery", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = matchquery('search query')") + ); + + assertEquals(AstDSL.function("matchquery", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = matchquery(\"search query\")") + ); + } + + @Test + public void relevanceMatchPhraseAltSyntax() { + assertEquals(AstDSL.function("match_phrase", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = match_phrase('search query')") + ); + + assertEquals(AstDSL.function("match_phrase", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = match_phrase(\"search query\")") + ); + + assertEquals(AstDSL.function("matchphrase", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = matchphrase('search query')") + ); + + assertEquals(AstDSL.function("matchphrase", + unresolvedArg("field", stringLiteral("message")), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("message = matchphrase(\"search query\")") + ); + } + + @Test + public void relevanceMultiMatchAltSyntax() { + assertEquals(AstDSL.function("multi_match", + unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of("field1", 1.F))), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("field1 = multi_match('search query')") + ); + + assertEquals(AstDSL.function("multi_match", + unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of("field1", 1.F))), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("field1 = multi_match(\"search query\")") + ); + + assertEquals(AstDSL.function("multimatch", + unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of("field1", 1.F))), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("field1 = multimatch('search query')") + ); + + assertEquals(AstDSL.function("multimatch", + unresolvedArg("fields", new RelevanceFieldList(ImmutableMap.of("field1", 1.F))), + unresolvedArg("query", stringLiteral("search query"))), + buildExprAst("field1 = multimatch(\"search query\")") + ); + } + @Test public void relevanceMulti_match() { assertEquals(AstDSL.function("multi_match",