From 52ea1e556f7829200c8ade9c574d883d7bebdfb3 Mon Sep 17 00:00:00 2001 From: GabeFernandez310 Date: Wed, 2 Nov 2022 09:15:06 -0700 Subject: [PATCH 1/2] Added Query Function As Alternate Syntax For Query String Function Signed-off-by: GabeFernandez310 --- .../org/opensearch/sql/expression/DSL.java | 4 + .../function/OpenSearchFunctions.java | 6 + .../sql/analysis/ExpressionAnalyzerTest.java | 9 ++ .../function/OpenSearchFunctionsTest.java | 7 + docs/user/dql/functions.rst | 61 +++++++ .../opensearch/sql/legacy/MethodQueryIT.java | 2 +- .../java/org/opensearch/sql/sql/QueryIT.java | 84 ++++++++++ .../script/filter/FilterQueryBuilder.java | 3 +- .../filter/lucene/relevance/NoFieldQuery.java | 72 +++++++++ .../filter/lucene/relevance/QueryQuery.java | 40 +++++ .../lucene/relevance/RelevanceQuery.java | 51 ++++-- .../script/filter/FilterQueryBuilderTest.java | 109 ++++++++++++- .../script/filter/lucene/QueryTest.java | 150 ++++++++++++++++++ .../lucene/relevance/NoFieldQueryTest.java | 49 ++++++ sql/src/main/antlr/OpenSearchSQLParser.g4 | 10 +- .../sql/sql/parser/AstExpressionBuilder.java | 36 ++++- .../sql/sql/antlr/SQLSyntaxParserTest.java | 43 +++++ .../sql/parser/AstExpressionBuilderTest.java | 16 ++ 18 files changed, 727 insertions(+), 25 deletions(-) create mode 100644 integ-test/src/test/java/org/opensearch/sql/sql/QueryIT.java create mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQuery.java create mode 100644 opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java create mode 100644 opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryTest.java create mode 100644 opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQueryTest.java 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 961608d104..e65dbd6fcb 100644 --- a/core/src/main/java/org/opensearch/sql/expression/DSL.java +++ b/core/src/main/java/org/opensearch/sql/expression/DSL.java @@ -707,6 +707,10 @@ public FunctionExpression simple_query_string(Expression... args) { return compile(BuiltinFunctionName.SIMPLE_QUERY_STRING, args); } + public FunctionExpression query(Expression... args) { + return compile(BuiltinFunctionName.QUERY, args); + } + public FunctionExpression query_string(Expression... args) { return compile(BuiltinFunctionName.QUERY_STRING, args); } 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 43a722b838..97afe3675e 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,7 @@ public void register(BuiltinFunctionRepository repository) { repository.register(match()); repository.register(multi_match()); repository.register(simple_query_string()); + repository.register(query()); repository.register(query_string()); // Register MATCHPHRASE as MATCH_PHRASE as well for backwards // compatibility. @@ -68,6 +69,11 @@ private static FunctionResolver simple_query_string() { return new RelevanceFunctionResolver(funcName, STRUCT); } + private static FunctionResolver query() { + FunctionName funcName = BuiltinFunctionName.QUERY.getName(); + return new RelevanceFunctionResolver(funcName, STRING); + } + private static FunctionResolver query_string() { FunctionName funcName = BuiltinFunctionName.QUERY_STRING.getName(); return new RelevanceFunctionResolver(funcName, STRUCT); diff --git a/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java b/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java index d1cef1edb3..98176f0002 100644 --- a/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java +++ b/core/src/test/java/org/opensearch/sql/analysis/ExpressionAnalyzerTest.java @@ -487,6 +487,15 @@ void simple_query_string_expression_two_fields() { AstDSL.unresolvedArg("query", stringLiteral("sample query")))); } + @Test + void query_expression() { + assertAnalyzeEqual( + dsl.query( + dsl.namedArgument("query", DSL.literal("field:query"))), + AstDSL.function("query", + AstDSL.unresolvedArg("query", stringLiteral("field:query")))); + } + @Test void query_string_expression() { assertAnalyzeEqual( 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 741caa5f91..620bdbf1b4 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 @@ -183,6 +183,13 @@ void simple_query_string() { expr.toString()); } + @Test + void query() { + FunctionExpression expr = dsl.query(query); + assertEquals(String.format("query(query=%s)", query.getValue()), + expr.toString()); + } + @Test void query_string() { FunctionExpression expr = dsl.query_string(fields, query); diff --git a/docs/user/dql/functions.rst b/docs/user/dql/functions.rst index 15f8a768bb..1589fd11e0 100644 --- a/docs/user/dql/functions.rst +++ b/docs/user/dql/functions.rst @@ -3017,6 +3017,67 @@ Another example to show how to set custom values for the optional parameters:: +------+--------------------------+----------------------+ +QUERY +----- + +Description +>>>>>>>>>>> + +``query("query_expression" [, option=]*)`` + +The `query` function is an alternative syntax to the `query_string`_ function. It maps to the query_string query used in search engine, to return the documents that match a provided text, number, date or boolean value with a given query expression. +``query_expression`` must be a string provided in Lucene query string syntax. Please refer to examples below: + +| ``query('Tags:taste OR Body:taste', ...)`` +| ``query("Tags:taste AND Body:taste", ...)`` + +Available parameters include: + +- analyzer +- escape +- allow_leading_wildcard +- analyze_wildcard +- auto_generate_synonyms_phrase_query +- boost +- default_operator +- enable_position_increments +- fuzziness +- fuzzy_max_expansions +- fuzzy_prefix_length +- fuzzy_transpositions +- fuzzy_rewrite +- tie_breaker +- lenient +- type +- max_determinized_states +- minimum_should_match +- quote_analyzer +- phrase_slop +- quote_field_suffix +- rewrite +- time_zone + +Example with only ``query_expressions``, and all other parameters are set default values:: + + os> select * from books where query('title:Pooh House'); + fetched rows / total rows = 2/2 + +------+--------------------------+----------------------+ + | id | title | author | + |------+--------------------------+----------------------| + | 1 | The House at Pooh Corner | Alan Alexander Milne | + | 2 | Winnie-the-Pooh | Alan Alexander Milne | + +------+--------------------------+----------------------+ + +Another example to show how to set custom values for the optional parameters:: + + os> select * from books where query('title:Pooh House', default_operator='AND'); + fetched rows / total rows = 1/1 + +------+--------------------------+----------------------+ + | id | title | author | + |------+--------------------------+----------------------| + | 1 | The House at Pooh Corner | Alan Alexander Milne | + +------+--------------------------+----------------------+ + HIGHLIGHT ------------ 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 0859a03784..b1b6b24ada 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 @@ -38,7 +38,7 @@ public void queryTest() throws IOException { "select address from %s where query('address:880 Holmes Lane') limit 3", TestsConstants.TEST_INDEX_ACCOUNT)); Assert.assertThat(result, - containsString("query_string\":{\"query\":\"address:880 Holmes Lane")); + containsString("query_string\\\":{\\\"query\\\":\\\"address:880 Holmes Lane")); } diff --git a/integ-test/src/test/java/org/opensearch/sql/sql/QueryIT.java b/integ-test/src/test/java/org/opensearch/sql/sql/QueryIT.java new file mode 100644 index 0000000000..e61593eb21 --- /dev/null +++ b/integ-test/src/test/java/org/opensearch/sql/sql/QueryIT.java @@ -0,0 +1,84 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.sql; + +import static org.opensearch.sql.legacy.TestsConstants.TEST_INDEX_BEER; + +import java.io.IOException; +import org.json.JSONObject; +import org.junit.Test; +import org.opensearch.sql.legacy.SQLIntegTestCase; + +public class QueryIT extends SQLIntegTestCase { + @Override + public void init() throws IOException { + loadIndex(Index.BEER); + } + + @Test + public void all_fields_test() throws IOException { + String query = "SELECT * FROM " + + TEST_INDEX_BEER + " WHERE query('*:taste')"; + JSONObject result = executeJdbcRequest(query); + assertEquals(16, result.getInt("total")); + } + + @Test + public void mandatory_params_test() throws IOException { + String query = "SELECT Id FROM " + + TEST_INDEX_BEER + " WHERE query('Tags:taste OR Body:taste')"; + JSONObject result = executeJdbcRequest(query); + assertEquals(16, result.getInt("total")); + } + + @Test + public void all_params_test() throws IOException { + String query = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE query('Tags:taste', escape=false," + + "allow_leading_wildcard=true, enable_position_increments=true," + + "fuzziness= 1, fuzzy_rewrite='constant_score', max_determinized_states = 10000," + + "analyzer='standard', analyze_wildcard = false, quote_field_suffix = '.exact'," + + "auto_generate_synonyms_phrase_query=true, boost = 0.77," + + "quote_analyzer='standard', phrase_slop=0, rewrite='constant_score', type='best_fields'," + + "tie_breaker=0.3, time_zone='Canada/Pacific', default_operator='or'," + + "fuzzy_transpositions = false, lenient = true, fuzzy_max_expansions = 25," + + "minimum_should_match = '2<-25% 9<-3', fuzzy_prefix_length = 7);"; + JSONObject result = executeJdbcRequest(query); + assertEquals(8, result.getInt("total")); + } + + @Test + public void wildcard_test() throws IOException { + String query1 = "SELECT Id FROM " + + TEST_INDEX_BEER + " WHERE query('Tags:taste')"; + JSONObject result1 = executeJdbcRequest(query1); + String query2 = "SELECT Id FROM " + + TEST_INDEX_BEER + " WHERE query('*:taste')"; + JSONObject result2 = executeJdbcRequest(query2); + assertNotEquals(result2.getInt("total"), result1.getInt("total")); + + String query3 = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE query('Tags:tas*');"; + JSONObject result3 = executeJdbcRequest(query3); + assertEquals(8, result3.getInt("total")); + + String query4 = "SELECT Id FROM " + TEST_INDEX_BEER + + " WHERE query('Tags:tas?e');"; + JSONObject result4 = executeJdbcRequest(query3); + assertEquals(8, result4.getInt("total")); + } + + @Test + public void query_string_and_query_return_the_same_results_test() throws IOException { + String query1 = "SELECT Id FROM " + + TEST_INDEX_BEER + " WHERE query('Tags:taste')"; + JSONObject result1 = executeJdbcRequest(query1); + String query2 = "SELECT Id FROM " + + TEST_INDEX_BEER + " WHERE query_string(['Tags'],'taste')"; + JSONObject result2 = executeJdbcRequest(query2); + assertEquals(result2.getInt("total"), result1.getInt("total")); + } +} 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 68f8ec8c66..ab8fb562da 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 @@ -34,6 +34,7 @@ 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.script.filter.lucene.relevance.MultiMatchQuery; +import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.QueryQuery; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.QueryStringQuery; import org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance.SimpleQueryStringQuery; import org.opensearch.sql.opensearch.storage.serialization.ExpressionSerializer; @@ -60,7 +61,7 @@ public class FilterQueryBuilder extends ExpressionNodeVisitor The builder class for the OpenSearch query. + */ +abstract class NoFieldQuery extends RelevanceQuery { + public NoFieldQuery(Map> queryBuildActions) { + super(queryBuildActions); + } + + @Override + protected void ignoreArguments(List arguments) { + arguments.removeIf(a -> a.getArgName().equalsIgnoreCase("query")); + } + + @Override + protected void checkValidArguments(String argNormalized, T queryBuilder) { + if (!getQueryBuildActions().containsKey(argNormalized)) { + throw new SemanticCheckException( + String.format("Parameter %s is invalid for %s function.", + argNormalized, getQueryName())); + } + } + /** + * Override build function because RelevanceQuery requires 2 fields, + * but NoFieldQuery must have no fields. + * + * @param func : Contains function name and passed in arguments. + * @return : QueryBuilder object + */ + + @Override + public QueryBuilder build(FunctionExpression func) { + var arguments = func.getArguments().stream().map( + a -> (NamedArgumentExpression) a).collect(Collectors.toList()); + if (arguments.size() < 1) { + throw new SyntaxCheckException(String.format( + "%s requires at least one parameter", func.getFunctionName())); + } + + return loadArguments(arguments); + } + + + @Override + public T createQueryBuilder(List arguments) { + // Extract 'query' + var query = arguments.stream().filter(a -> a.getArgName().equalsIgnoreCase("query")).findFirst() + .orElseThrow(() -> new SemanticCheckException("'query' parameter is missing")); + + return createBuilder(query.getValue().valueOf(null).stringValue()); + } + + protected abstract T createBuilder(String query); +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java new file mode 100644 index 0000000000..6933f8fb5b --- /dev/null +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java @@ -0,0 +1,40 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; + +import org.opensearch.index.query.QueryBuilders; +import org.opensearch.index.query.QueryStringQueryBuilder; + +/** + * Class for Lucene query that builds the query_string query. + */ +public class QueryQuery extends NoFieldQuery { + + final String queryQueryName = "query"; + + /** + * Default constructor for QueryQuery configures how RelevanceQuery.build() handles + * named arguments by calling the constructor of QueryStringQuery. + */ + public QueryQuery() { + super(FunctionParameterRepository.QueryStringQueryBuildActions); + } + + /** + * Builds QueryBuilder with query value and other default parameter values set. + * + * @param query : Query value for query_string query + * @return : Builder for query query + */ + protected QueryStringQueryBuilder createBuilder(String query) { + return QueryBuilders.queryStringQuery(query); + } + + @Override + public String getQueryName() { + return queryQueryName; + } +} 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 579f77d2cd..1512214615 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,6 +10,7 @@ import java.util.Objects; import java.util.function.BiFunction; import java.util.stream.Collectors; +import lombok.Getter; import lombok.RequiredArgsConstructor; import org.opensearch.index.query.QueryBuilder; import org.opensearch.sql.common.antlr.SyntaxCheckException; @@ -24,17 +25,24 @@ */ @RequiredArgsConstructor public abstract class RelevanceQuery extends LuceneQuery { + @Getter private final Map> queryBuildActions; - @Override - public QueryBuilder build(FunctionExpression func) { - var arguments = func.getArguments().stream() - .map(a -> (NamedArgumentExpression)a).collect(Collectors.toList()); - if (arguments.size() < 2) { - throw new SyntaxCheckException( - String.format("%s requires at least two parameters", getQueryName())); + protected void ignoreArguments(List arguments) { + arguments.removeIf(a -> a.getArgName().equalsIgnoreCase("field") + || a.getArgName().equalsIgnoreCase("fields") + || a.getArgName().equalsIgnoreCase("query")); + } + + protected void checkValidArguments(String argNormalized, T queryBuilder) { + if (!queryBuildActions.containsKey(argNormalized)) { + throw new SemanticCheckException( + String.format("Parameter %s is invalid for %s function.", + argNormalized, queryBuilder.getWriteableName())); } + } + protected T loadArguments(List arguments) throws SemanticCheckException { // Aggregate parameters by name, so getting a Map arguments.stream().collect(Collectors.groupingBy(a -> a.getArgName().toLowerCase())) .forEach((k, v) -> { @@ -46,32 +54,45 @@ public QueryBuilder build(FunctionExpression func) { T queryBuilder = createQueryBuilder(arguments); - arguments.removeIf(a -> a.getArgName().equalsIgnoreCase("field") - || a.getArgName().equalsIgnoreCase("fields") - || a.getArgName().equalsIgnoreCase("query")); + ignoreArguments(arguments); var iterator = arguments.listIterator(); while (iterator.hasNext()) { NamedArgumentExpression arg = iterator.next(); String argNormalized = arg.getArgName().toLowerCase(); - if (!queryBuildActions.containsKey(argNormalized)) { - throw new SemanticCheckException( - String.format("Parameter %s is invalid for %s function.", - argNormalized, queryBuilder.getWriteableName())); - } + checkValidArguments(argNormalized, queryBuilder); + (Objects.requireNonNull( queryBuildActions .get(argNormalized))) .apply(queryBuilder, arg.getValue().valueOf(null)); } + return queryBuilder; } + @Override + public QueryBuilder build(FunctionExpression func) { + var arguments = func.getArguments().stream() + .map(a -> (NamedArgumentExpression)a).collect(Collectors.toList()); + if (arguments.size() < 2) { + throw new SyntaxCheckException( + String.format("%s requires at least two parameters", getQueryName())); + } + + return loadArguments(arguments); + + } + protected abstract T createQueryBuilder(List arguments); protected abstract String getQueryName(); + public Map> getQueryBuildActions() { + return queryBuildActions; + } + /** * Convenience interface for a function that updates a QueryBuilder * based on ExprValue. 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 ff80f3bcc0..58f2ea9506 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 @@ -48,7 +48,6 @@ import org.opensearch.sql.data.model.ExprTimestampValue; import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValueUtils; -import org.opensearch.sql.exception.ExpressionEvaluationException; import org.opensearch.sql.exception.SemanticCheckException; import org.opensearch.sql.expression.DSL; import org.opensearch.sql.expression.Expression; @@ -411,6 +410,15 @@ void match_missing_field() { assertEquals("'field' parameter is missing.", msg); } + @Test + void match_missing_query() { + FunctionExpression expr = dsl.match( + dsl.namedArgument("field", literal("field1")), + dsl.namedArgument("analyzer", literal("keyword"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("'query' parameter is missing", msg); + } + @Test void should_build_match_phrase_query_with_default_parameters() { assertJsonEquals( @@ -626,6 +634,93 @@ void should_build_match_phrase_query_with_custom_parameters() { dsl.namedArgument("zero_terms_query", literal("ALL"))))); } + @Test + void query_invalid_parameter() { + FunctionExpression expr = dsl.query( + dsl.namedArgument("invalid_parameter", literal("invalid_value"))); + assertThrows(SemanticCheckException.class, () -> buildQuery(expr), + "Parameter invalid_parameter is invalid for query function."); + } + + @Test + void query_invalid_fields_parameter_exception_message() { + FunctionExpression expr = dsl.query( + dsl.namedArgument("fields", literal("field1")), + dsl.namedArgument("query", literal("search query"))); + + var exception = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)); + assertEquals("Parameter fields is invalid for query function.", exception.getMessage()); + } + + @Test + void should_build_query_query_with_default_parameters() { + var expected = "{\n" + + " \"query_string\" : {\n" + + " \"query\" : \"field1:query_value\",\n" + + " \"fields\" : [],\n" + + " \"type\" : \"best_fields\",\n" + + " \"default_operator\" : \"or\",\n" + + " \"max_determinized_states\" : 10000,\n" + + " \"enable_position_increments\" : true,\n" + + " \"fuzziness\" : \"AUTO\",\n" + + " \"fuzzy_prefix_length\" : 0,\n" + + " \"fuzzy_max_expansions\" : 50,\n" + + " \"phrase_slop\" : 0,\n" + + " \"escape\" : false,\n" + + " \"auto_generate_synonyms_phrase_query\" : true,\n" + + " \"fuzzy_transpositions\" : true,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}"; + + assertJsonEquals(expected, buildQuery(dsl.query( + dsl.namedArgument("query", literal("field1:query_value"))))); + } + + @Test + void should_build_query_query_with_custom_parameters() { + var expected = "{\n" + + " \"query_string\" : {\n" + + " \"query\" : \"field1:query_value\",\n" + + " \"fields\" : [],\n" + + " \"type\" : \"cross_fields\",\n" + + " \"tie_breaker\" : 1.3,\n" + + " \"default_operator\" : \"and\",\n" + + " \"analyzer\" : \"keyword\",\n" + + " \"max_determinized_states\" : 10000,\n" + + " \"enable_position_increments\" : true,\n" + + " \"fuzziness\" : \"AUTO\",\n" + + " \"fuzzy_prefix_length\" : 2,\n" + + " \"fuzzy_max_expansions\" : 10,\n" + + " \"phrase_slop\" : 0,\n" + + " \"analyze_wildcard\" : true,\n" + + " \"minimum_should_match\" : \"3\",\n" + + " \"lenient\" : false,\n" + + " \"escape\" : false,\n" + + " \"auto_generate_synonyms_phrase_query\" : false,\n" + + " \"fuzzy_transpositions\" : false,\n" + + " \"boost\" : 2.0,\n" + + " }\n" + + "}"; + var actual = buildQuery( + dsl.query( + dsl.namedArgument("query", literal("field1:query_value")), + dsl.namedArgument("analyze_wildcard", literal("true")), + dsl.namedArgument("analyzer", literal("keyword")), + dsl.namedArgument("auto_generate_synonyms_phrase_query", literal("false")), + dsl.namedArgument("default_operator", literal("AND")), + dsl.namedArgument("fuzzy_max_expansions", literal("10")), + dsl.namedArgument("fuzzy_prefix_length", literal("2")), + dsl.namedArgument("fuzzy_transpositions", literal("false")), + dsl.namedArgument("lenient", literal("false")), + dsl.namedArgument("minimum_should_match", literal("3")), + dsl.namedArgument("tie_breaker", literal("1.3")), + dsl.namedArgument("type", literal("cross_fields")), + dsl.namedArgument("boost", literal("2.0")))); + + assertJsonEquals(expected, actual); + } + @Test void query_string_invalid_parameter() { FunctionExpression expr = dsl.query_string( @@ -981,6 +1076,18 @@ void multi_match_missing_fields_even_with_struct() { assertEquals("'fields' parameter is missing.", msg); } + @Test + void multi_match_missing_query_even_with_struct() { + FunctionExpression expr = dsl.multi_match( + dsl.namedArgument("fields", DSL.literal( + new ExprTupleValue(new LinkedHashMap<>(ImmutableMap.of( + "field1", ExprValueUtils.floatValue(1.F), + "field2", ExprValueUtils.floatValue(.3F)))))), + dsl.namedArgument("analyzer", literal("keyword"))); + var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); + assertEquals("'query' parameter is missing", msg); + } + @Test void should_build_match_phrase_prefix_query_with_default_parameters() { assertJsonEquals( diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryTest.java new file mode 100644 index 0000000000..e0681bceac --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryTest.java @@ -0,0 +1,150 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.storage.script.filter.lucene; + +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.util.List; +import java.util.stream.Stream; +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.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; +import org.opensearch.sql.expression.DSL; +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.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.QueryQuery; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +class QueryTest { + private static final DSL dsl = new ExpressionConfig() + .dsl(new ExpressionConfig().functionRepository()); + private final QueryQuery queryQuery = new QueryQuery(); + private final FunctionName queryFunc = FunctionName.of("query"); + private static final LiteralExpression query_value = DSL.literal("title:query_value"); + + static Stream> generateValidData() { + Expression query = dsl.namedArgument("query", query_value); + return List.of( + dsl.namedArgument("analyzer", DSL.literal("standard")), + dsl.namedArgument("analyze_wildcard", DSL.literal("true")), + dsl.namedArgument("allow_leading_wildcard", DSL.literal("true")), + dsl.namedArgument("auto_generate_synonyms_phrase_query", DSL.literal("true")), + dsl.namedArgument("boost", DSL.literal("1")), + dsl.namedArgument("default_operator", DSL.literal("AND")), + dsl.namedArgument("default_operator", DSL.literal("and")), + dsl.namedArgument("enable_position_increments", DSL.literal("true")), + dsl.namedArgument("escape", DSL.literal("false")), + dsl.namedArgument("fuzziness", DSL.literal("1")), + dsl.namedArgument("fuzzy_rewrite", DSL.literal("constant_score")), + dsl.namedArgument("fuzzy_max_expansions", DSL.literal("42")), + dsl.namedArgument("fuzzy_prefix_length", DSL.literal("42")), + dsl.namedArgument("fuzzy_transpositions", DSL.literal("true")), + dsl.namedArgument("lenient", DSL.literal("true")), + dsl.namedArgument("max_determinized_states", DSL.literal("10000")), + dsl.namedArgument("minimum_should_match", DSL.literal("4")), + dsl.namedArgument("quote_analyzer", DSL.literal("standard")), + dsl.namedArgument("phrase_slop", DSL.literal("0")), + dsl.namedArgument("quote_field_suffix", DSL.literal(".exact")), + dsl.namedArgument("rewrite", DSL.literal("constant_score")), + dsl.namedArgument("type", DSL.literal("best_fields")), + dsl.namedArgument("tie_breaker", DSL.literal("0.3")), + dsl.namedArgument("time_zone", DSL.literal("Canada/Pacific")), + dsl.namedArgument("ANALYZER", DSL.literal("standard")), + dsl.namedArgument("ANALYZE_wildcard", DSL.literal("true")), + dsl.namedArgument("Allow_Leading_wildcard", DSL.literal("true")), + dsl.namedArgument("Auto_Generate_Synonyms_Phrase_Query", DSL.literal("true")), + dsl.namedArgument("Boost", DSL.literal("1")) + ).stream().map(arg -> List.of(query, arg)); + } + + @ParameterizedTest + @MethodSource("generateValidData") + public void test_valid_parameters(List validArgs) { + Assertions.assertNotNull(queryQuery.build( + new QueryExpression(validArgs))); + } + + @Test + public void test_SyntaxCheckException_when_no_arguments() { + List arguments = List.of(); + assertThrows(SyntaxCheckException.class, + () -> queryQuery.build(new QueryExpression(arguments))); + } + + @Test + public void test_SyntaxCheckException_when_field_argument() { + List arguments = List.of( + namedArgument("fields", "invalid argument"), + namedArgument("query", query_value)); + assertThrows(SemanticCheckException.class, + () -> queryQuery.build(new QueryExpression(arguments))); + } + + @Test + public void test_SemanticCheckException_when_invalid_parameter() { + List arguments = List.of( + namedArgument("query", query_value), + namedArgument("unsupported", "unsupported_value")); + Assertions.assertThrows(SemanticCheckException.class, + () -> queryQuery.build(new QueryExpression(arguments))); + } + + @Test + public void test_SemanticCheckException_when_sending_parameter_multiple_times() { + List arguments = List.of( + namedArgument("query", query_value), + namedArgument("allow_leading_wildcard", DSL.literal("true")), + namedArgument("allow_leading_wildcard", DSL.literal("true"))); + Assertions.assertThrows(SemanticCheckException.class, + () -> queryQuery.build(new QueryExpression(arguments))); + } + + private NamedArgumentExpression namedArgument(String name, String value) { + return dsl.namedArgument(name, DSL.literal(value)); + } + + private NamedArgumentExpression namedArgument(String name, LiteralExpression value) { + return dsl.namedArgument(name, value); + } + + private class QueryExpression extends FunctionExpression { + public QueryExpression(List arguments) { + super(QueryTest.this.queryFunc, arguments); + } + + @Override + public ExprValue valueOf(Environment valueEnv) { + throw new UnsupportedOperationException("Invalid function call, " + + "valueOf function need implementation only to support Expression interface"); + } + + @Override + public ExprType type() { + throw new UnsupportedOperationException("Invalid function call, " + + "type function need implementation only to support Expression interface"); + } + } + + @Test + public void test_can_get_query_name() { + List arguments = List.of(namedArgument("query", query_value)); + queryQuery.build(new QueryExpression(arguments)); + Assertions.assertEquals("query", + queryQuery.getQueryName()); + } +} diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQueryTest.java new file mode 100644 index 0000000000..e06dd7d32a --- /dev/null +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQueryTest.java @@ -0,0 +1,49 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; + +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.google.common.collect.ImmutableMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.opensearch.sql.data.model.ExprValueUtils; +import org.opensearch.sql.expression.DSL; +import org.opensearch.sql.expression.LiteralExpression; +import org.opensearch.sql.expression.config.ExpressionConfig; + +class NoFieldQueryTest { + NoFieldQuery query; + private final DSL dsl = new ExpressionConfig().dsl(new ExpressionConfig().functionRepository()); + private final String testQueryName = "test_query"; + private final Map actionMap + = ImmutableMap.of("paramA", (o, v) -> o); + + @BeforeEach + void setUp() { + query = mock(NoFieldQuery.class, + Mockito.withSettings().useConstructor(actionMap) + .defaultAnswer(Mockito.CALLS_REAL_METHODS)); + when(query.getQueryName()).thenReturn(testQueryName); + } + + @Test + void createQueryBuilderTest() { + String sampleQuery = "field:query"; + + query.createQueryBuilder(List.of( + dsl.namedArgument("query", + new LiteralExpression(ExprValueUtils.stringValue(sampleQuery))))); + + verify(query).createBuilder(eq(sampleQuery)); + } +} diff --git a/sql/src/main/antlr/OpenSearchSQLParser.g4 b/sql/src/main/antlr/OpenSearchSQLParser.g4 index 6470c9f546..22b8753825 100644 --- a/sql/src/main/antlr/OpenSearchSQLParser.g4 +++ b/sql/src/main/antlr/OpenSearchSQLParser.g4 @@ -335,7 +335,11 @@ specificFunction ; relevanceFunction - : singleFieldRelevanceFunction | multiFieldRelevanceFunction + : noFieldRelevanceFunction | singleFieldRelevanceFunction | multiFieldRelevanceFunction + ; + +noFieldRelevanceFunction + : noFieldRelevanceFunctionName LR_BRACKET query=relevanceQuery (COMMA relevanceArg)* RR_BRACKET ; // Field is a single column @@ -418,6 +422,10 @@ flowControlFunctionName : IF | IFNULL | NULLIF | ISNULL ; +noFieldRelevanceFunctionName + : QUERY + ; + systemFunctionName : TYPEOF ; 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 ebfafeec23..f350f16e73 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 @@ -381,6 +381,14 @@ public UnresolvedExpression visitConvertedDataType( return AstDSL.stringLiteral(ctx.getText()); } + @Override + public UnresolvedExpression visitNoFieldRelevanceFunction( + OpenSearchSQLParser.NoFieldRelevanceFunctionContext ctx) { + return new Function( + ctx.noFieldRelevanceFunctionName().getText().toLowerCase(), + noFieldRelevanceArguments(ctx)); + } + @Override public UnresolvedExpression visitSingleFieldRelevanceFunction( SingleFieldRelevanceFunctionContext ctx) { @@ -438,6 +446,24 @@ private QualifiedName visitIdentifiers(List identifiers) { ); } + private void fillRelevanceArgs(List args, + ImmutableList.Builder builder) { + args.forEach(v -> builder.add(new UnresolvedArgument( + v.relevanceArgName().getText().toLowerCase(), new Literal(StringUtils.unquoteText( + v.relevanceArgValue().getText()), DataType.STRING)))); + } + + private List noFieldRelevanceArguments( + OpenSearchSQLParser.NoFieldRelevanceFunctionContext 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("query", + new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); + fillRelevanceArgs(ctx.relevanceArg(), builder); + return builder.build(); + } + private List singleFieldRelevanceArguments( OpenSearchSQLParser.SingleFieldRelevanceFunctionContext ctx) { // all the arguments are defaulted to string values @@ -447,12 +473,12 @@ private List singleFieldRelevanceArguments( new Literal(StringUtils.unquoteText(ctx.field.getText()), DataType.STRING))); builder.add(new UnresolvedArgument("query", new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); - ctx.relevanceArg().forEach(v -> builder.add(new UnresolvedArgument( - v.relevanceArgName().getText().toLowerCase(), new Literal(StringUtils.unquoteText( - v.relevanceArgValue().getText()), DataType.STRING)))); + fillRelevanceArgs(ctx.relevanceArg(), builder); return builder.build(); } + + private List multiFieldRelevanceArguments( OpenSearchSQLParser.MultiFieldRelevanceFunctionContext ctx) { // all the arguments are defaulted to string values @@ -467,9 +493,7 @@ private List multiFieldRelevanceArguments( builder.add(new UnresolvedArgument("fields", fields)); builder.add(new UnresolvedArgument("query", new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); - ctx.relevanceArg().forEach(v -> builder.add(new UnresolvedArgument( - v.relevanceArgName().getText().toLowerCase(), new Literal(StringUtils.unquoteText( - v.relevanceArgValue().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 af428bdc52..6b78376d45 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 @@ -325,6 +325,49 @@ public void can_parse_query_string_relevance_function() { + "operator='AND', tie_breaker=0.3, type = \"most_fields\", fuzziness = 4)")); } + + @Test + public void can_parse_query_relevance_function() { + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query('address:query')")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query('address:query OR notes:query')")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(\"address:query\")")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(\"address:query OR notes:query\")")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(`address:query`)")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(`address:query OR notes:query`)")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query('*:query')")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(\"*:query\")")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(`*:query`)")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query('address:*uery OR notes:?uery')")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(\"address:*uery OR notes:?uery\")")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(`address:*uery OR notes:?uery`)")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query('address:qu*ry OR notes:qu?ry')")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(\"address:qu*ry OR notes:qu?ry\")")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(`address:qu*ry OR notes:qu?ry`)")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query('address:query notes:query')")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE query(\"address:query notes:query\")")); + assertNotNull(parser.parse( + "SELECT id FROM test WHERE " + + "query(\"Body:\'taste beer\' Tags:\'taste beer\' Title:\'taste beer\'\")")); + } + + @Test public void can_parse_match_relevance_function() { assertNotNull(parser.parse("SELECT * FROM test WHERE match(column, \"this is a test\")")); 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 ec0a0dd0d3..cb00ea2f18 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 @@ -543,6 +543,22 @@ public void relevanceQuery_string() { + "analyzer='keyword', time_zone='Canada/Pacific', tie_breaker='1.3')")); } + @Test + public void relevanceQuery() { + assertEquals(AstDSL.function("query", + unresolvedArg("query", stringLiteral("field1:query OR field2:query"))), + buildExprAst("query('field1:query OR field2:query')") + ); + + assertEquals(AstDSL.function("query", + unresolvedArg("query", stringLiteral("search query")), + unresolvedArg("analyzer", stringLiteral("keyword")), + unresolvedArg("time_zone", stringLiteral("Canada/Pacific")), + unresolvedArg("tie_breaker", stringLiteral("1.3"))), + buildExprAst("query('search query'," + + "analyzer='keyword', time_zone='Canada/Pacific', tie_breaker='1.3')")); + } + @Test public void canBuildInClause() { assertEquals( From e73cf7dd5f21f51f250857fa71fc29eb3d1894f0 Mon Sep 17 00:00:00 2001 From: GabeFernandez310 Date: Wed, 2 Nov 2022 14:14:37 -0700 Subject: [PATCH 2/2] Corrected Text For Some Comments And Changed Constant To Be Private Signed-off-by: GabeFernandez310 --- .../storage/script/filter/lucene/relevance/NoFieldQuery.java | 2 +- .../storage/script/filter/lucene/relevance/QueryQuery.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQuery.java index 1214f9f12e..b27cd2e926 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/NoFieldQuery.java @@ -16,7 +16,7 @@ import org.opensearch.sql.expression.NamedArgumentExpression; /** - * Base class to represent relevance queries that search multiple fields. + * Base class to represent relevance queries that have no 'fields' array as an argument. * * @param The builder class for the OpenSearch query. */ diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java index 6933f8fb5b..35d5a43a41 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryQuery.java @@ -9,11 +9,11 @@ import org.opensearch.index.query.QueryStringQueryBuilder; /** - * Class for Lucene query that builds the query_string query. + * Class for Lucene query that builds the 'query' query. */ public class QueryQuery extends NoFieldQuery { - final String queryQueryName = "query"; + private final String queryQueryName = "query"; /** * Default constructor for QueryQuery configures how RelevanceQuery.build() handles