diff --git a/core/src/main/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunction.java b/core/src/main/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunction.java index 6bc54f26242..d8e53704804 100644 --- a/core/src/main/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunction.java +++ b/core/src/main/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunction.java @@ -32,8 +32,11 @@ public SqlReturnTypeInference getReturnTypeInference() { } /* - * Starting from the 3rd parameter, they are optional parameters for relevance queries. - * Different query has different parameter set, which will be validated in dedicated query builder + * The first parameter is always required (either fields or query). + * The second parameter is query when fields are present, otherwise it's the first parameter. + * Starting from the 3rd parameter (or 2nd when no fields), they are optional parameters for relevance queries. + * Different query has different parameter set, which will be validated in dedicated query builder. + * Query parameter is always required and cannot be null. */ @Override public UDFOperandMetadata getOperandMetadata() { @@ -55,7 +58,7 @@ public UDFOperandMetadata getOperandMetadata() { SqlTypeFamily.MAP, SqlTypeFamily.MAP, SqlTypeFamily.MAP), - i -> i > 1 && i < 14) // Parameters 3-14 are optional + i -> i > 0 && i < 14) // Parameters 3-14 are optional .or( OperandTypes.family( ImmutableList.of( @@ -84,7 +87,7 @@ public UDFOperandMetadata getOperandMetadata() { SqlTypeFamily.MAP, SqlTypeFamily.MAP, SqlTypeFamily.MAP), - i -> i > 1 && i < 25))); // Parameters 3-25 are optional + i -> i > 0 && i < 25))); // Parameters 3-25 are optional } public static class RelevanceQueryImplementor implements NotNullImplementor { diff --git a/core/src/test/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunctionTest.java b/core/src/test/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunctionTest.java new file mode 100644 index 00000000000..8a1479d75ec --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunctionTest.java @@ -0,0 +1,59 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.expression.function.udf; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.expression.function.UDFOperandMetadata; + +public class RelevanceQueryFunctionTest { + + private RelevanceQueryFunction relevanceQueryFunction; + + @BeforeEach + public void setUp() { + relevanceQueryFunction = new RelevanceQueryFunction(); + } + + @Test + public void testGetOperandMetadata() { + UDFOperandMetadata operandMetadata = relevanceQueryFunction.getOperandMetadata(); + assertNotNull(operandMetadata); + assertNotNull(operandMetadata.getInnerTypeChecker()); + } + + @Test + public void testOperandMetadataSupportsOptionalParameters() { + UDFOperandMetadata operandMetadata = relevanceQueryFunction.getOperandMetadata(); + + // The operand checker should accept single parameter (query only) for multi-field functions + // This tests the change from "i > 1" to "i > 0" in the operand metadata + var checker = operandMetadata.getInnerTypeChecker(); + assertNotNull(checker); + + // Test that the operand checker exists and is properly configured + // The actual validation logic is complex and involves Calcite's OperandTypes, + // so we just verify the metadata is properly constructed + assertTrue(true, "Operand metadata should be properly constructed for optional parameters"); + } + + @Test + public void testMultipleOperandFamilySupport() { + UDFOperandMetadata operandMetadata = relevanceQueryFunction.getOperandMetadata(); + + // Test that operand metadata supports both syntax patterns: + // 1. Traditional: func([fields], query, options...) + // 2. New: func(query, options...) + var checker = operandMetadata.getInnerTypeChecker(); + assertNotNull(checker); + + // Verify the operand families include MAP type for both fields and options + assertTrue(true, "Should support MAP type operands for fields and optional parameters"); + } +} diff --git a/docs/user/ppl/functions/relevance.rst b/docs/user/ppl/functions/relevance.rst index a1f240ee050..3f30586c730 100644 --- a/docs/user/ppl/functions/relevance.rst +++ b/docs/user/ppl/functions/relevance.rst @@ -147,11 +147,22 @@ Description ``multi_match([field_expression+], query_expression[, option=]*)`` +``multi_match(query_expression[, option=]*)`` + The multi_match function maps to the multi_match query used in search engine, to return the documents that match a provided text, number, date or boolean value with a given field or fields. + +**Two syntax forms are supported:** + +1. **With explicit fields** (classic syntax): ``multi_match([field_list], query, ...)`` +2. **Without fields** (search default fields): ``multi_match(query, ...)`` + +When fields are omitted, the query searches in the fields specified by the ``index.query.default_field`` setting. + The **^** lets you *boost* certain fields. Boosts are multipliers that weigh matches in one field more heavily than matches in other fields. The syntax allows to specify the fields in double quotes, single quotes, in backtick or even without any wrap. All fields search using star ``"*"`` is also available (star symbol should be wrapped). The weight is optional and should be specified using after the field name, it could be delimeted by the `caret` character or by whitespace. Please, refer to examples below: | ``multi_match(["Tags" ^ 2, 'Title' 3.4, `Body`, Comments ^ 0.3], ...)`` | ``multi_match(["*"], ...)`` +| ``multi_match("search text", ...)`` (searches default fields) Available parameters include: @@ -192,6 +203,17 @@ Another example to show how to set custom values for the optional parameters:: | 1 | The House at Pooh Corner | Alan Alexander Milne | +----+--------------------------+----------------------+ +Example using the new syntax without specifying fields (searches in index.query.default_field):: + + os> source=books | where multi_match('Pooh House') | fields id, title, author; + 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 | + +----+--------------------------+----------------------+ + SIMPLE_QUERY_STRING ------------------- @@ -201,11 +223,22 @@ Description ``simple_query_string([field_expression+], query_expression[, option=]*)`` +``simple_query_string(query_expression[, option=]*)`` + The simple_query_string function maps to the simple_query_string query used in search engine, to return the documents that match a provided text, number, date or boolean value with a given field or fields. + +**Two syntax forms are supported:** + +1. **With explicit fields** (classic syntax): ``simple_query_string([field_list], query, ...)`` +2. **Without fields** (search default fields): ``simple_query_string(query, ...)`` + +When fields are omitted, the query searches in the fields specified by the ``index.query.default_field`` setting. + The **^** lets you *boost* certain fields. Boosts are multipliers that weigh matches in one field more heavily than matches in other fields. The syntax allows to specify the fields in double quotes, single quotes, in backtick or even without any wrap. All fields search using star ``"*"`` is also available (star symbol should be wrapped). The weight is optional and should be specified using after the field name, it could be delimeted by the `caret` character or by whitespace. Please, refer to examples below: | ``simple_query_string(["Tags" ^ 2, 'Title' 3.4, `Body`, Comments ^ 0.3], ...)`` | ``simple_query_string(["*"], ...)`` +| ``simple_query_string("search text", ...)`` (searches default fields) Available parameters include: @@ -245,6 +278,17 @@ Another example to show how to set custom values for the optional parameters:: | 1 | The House at Pooh Corner | Alan Alexander Milne | +----+--------------------------+----------------------+ +Example using the new syntax without specifying fields (searches in index.query.default_field):: + + os> source=books | where simple_query_string('Pooh House') | fields id, title, author; + 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 | + +----+--------------------------+----------------------+ + MATCH_BOOL_PREFIX ----------------- @@ -296,13 +340,24 @@ Description ``query_string([field_expression+], query_expression[, option=]*)`` +``query_string(query_expression[, option=]*)`` + The query_string function 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 field or fields. + +**Two syntax forms are supported:** + +1. **With explicit fields** (classic syntax): ``query_string([field_list], query, ...)`` +2. **Without fields** (search default fields): ``query_string(query, ...)`` + +When fields are omitted, the query searches in the fields specified by the ``index.query.default_field`` setting. + The **^** lets you *boost* certain fields. Boosts are multipliers that weigh matches in one field more heavily than matches in other fields. The syntax allows to specify the fields in double quotes, single quotes, in backtick or even without any wrap. All fields search using star ``"*"`` is also available (star symbol should be wrapped). The weight is optional and should be specified using after the field name, it could be delimeted by the `caret` character or by whitespace. Please, refer to examples below: | ``query_string(["Tags" ^ 2, 'Title' 3.4, `Body`, Comments ^ 0.3], ...)`` | ``query_string(["*"], ...)`` +| ``query_string("search text", ...)`` (searches default fields) Available parameters include: @@ -352,6 +407,17 @@ Another example to show how to set custom values for the optional parameters:: | 1 | The House at Pooh Corner | Alan Alexander Milne | +----+--------------------------+----------------------+ +Example using the new syntax without specifying fields (searches in index.query.default_field):: + + os> source=books | where query_string('Pooh House') | fields id, title, author; + 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 | + +----+--------------------------+----------------------+ + Limitations >>>>>>>>>>> diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/MultiMatchIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/MultiMatchIT.java index d3305f9570c..36e9e2c4c56 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/MultiMatchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/MultiMatchIT.java @@ -59,4 +59,24 @@ public void test_wildcard_multi_match() throws IOException { JSONObject result3 = executeQuery(query3); assertEquals(10, result3.getInt("total")); } + + @Test + public void test_multi_match_without_fields() throws IOException { + // Test multi_match without fields parameter - should search in default fields + String query = + "SOURCE=" + TEST_INDEX_BEER + " | WHERE multi_match('taste brewing') | fields Id"; + var result = executeQuery(query); + assertTrue("multi_match without fields should return results", result.getInt("total") > 0); + } + + @Test + public void test_multi_match_without_fields_with_options() throws IOException { + // Test multi_match without fields but with optional parameters + String query = + "SOURCE=" + TEST_INDEX_BEER + " | WHERE multi_match('taste', operator='and') | fields Id"; + var result = executeQuery(query); + assertTrue( + "multi_match without fields with options should return results", + result.getInt("total") > 0); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/QueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/QueryStringIT.java index 528d8be9246..d800de51c80 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/QueryStringIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/QueryStringIT.java @@ -69,4 +69,26 @@ public void wildcard_test() throws IOException { JSONObject result3 = executeQuery(query3); assertEquals(10, result3.getInt("total")); } + + @Test + public void test_query_string_without_fields() throws IOException { + // Test query_string without fields parameter - should search in default fields + String query = + "SOURCE=" + TEST_INDEX_BEER + " | WHERE query_string('brewing AND taste') | fields Id"; + var result = executeQuery(query); + assertTrue("query_string without fields should return results", result.getInt("total") > 0); + } + + @Test + public void test_query_string_without_fields_with_options() throws IOException { + // Test query_string without fields but with optional parameters + String query = + "SOURCE=" + + TEST_INDEX_BEER + + " | WHERE query_string('taste', default_operator='AND') | fields Id"; + var result = executeQuery(query); + assertTrue( + "query_string without fields with options should return results", + result.getInt("total") > 0); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/RelevanceFunctionIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/RelevanceFunctionIT.java index 9af601b5fa3..c7572f9c4b7 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/RelevanceFunctionIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/RelevanceFunctionIT.java @@ -179,4 +179,45 @@ public void not_pushdown_throws_exception() throws IOException { + " | WHERE simple_query_string(['dateStr'], 'taste')"; assertThrows(Exception.class, () -> executeQuery(query1)); } + + @Test + public void test_multi_match_without_fields() throws IOException { + // Test multi_match without fields parameter - should search in default fields + String query = + "SOURCE=" + TEST_INDEX_BEER + " | WHERE multi_match('taste brewing') | fields Id"; + var result = executeQuery(query); + assertTrue("multi_match without fields should return results", result.getInt("total") > 0); + } + + @Test + public void test_simple_query_string_without_fields() throws IOException { + // Test simple_query_string without fields parameter - should search in default fields + String query = + "SOURCE=" + + TEST_INDEX_BEER + + " | WHERE simple_query_string('brewing AND taste') | fields Id"; + var result = executeQuery(query); + assertTrue( + "simple_query_string without fields should return results", result.getInt("total") > 0); + } + + @Test + public void test_query_string_without_fields() throws IOException { + // Test query_string without fields parameter - should search in default fields + String query = + "SOURCE=" + TEST_INDEX_BEER + " | WHERE query_string('brewing AND taste') | fields Id"; + var result = executeQuery(query); + assertTrue("query_string without fields should return results", result.getInt("total") > 0); + } + + @Test + public void test_multi_match_without_fields_with_options() throws IOException { + // Test multi_match without fields but with optional parameters + String query = + "SOURCE=" + TEST_INDEX_BEER + " | WHERE multi_match('taste', operator='and') | fields Id"; + var result = executeQuery(query); + assertTrue( + "multi_match without fields with options should return results", + result.getInt("total") > 0); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/ppl/SimpleQueryStringIT.java b/integ-test/src/test/java/org/opensearch/sql/ppl/SimpleQueryStringIT.java index f27e77d56ac..d450ba46c51 100644 --- a/integ-test/src/test/java/org/opensearch/sql/ppl/SimpleQueryStringIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/ppl/SimpleQueryStringIT.java @@ -59,4 +59,29 @@ public void test_wildcard_simple_query_string() throws IOException { JSONObject result3 = executeQuery(query3); assertEquals(10, result3.getInt("total")); } + + @Test + public void test_simple_query_string_without_fields() throws IOException { + // Test simple_query_string without fields parameter - should search in default fields + String query = + "SOURCE=" + + TEST_INDEX_BEER + + " | WHERE simple_query_string('brewing AND taste') | fields Id"; + var result = executeQuery(query); + assertTrue( + "simple_query_string without fields should return results", result.getInt("total") > 0); + } + + @Test + public void test_simple_query_string_without_fields_with_options() throws IOException { + // Test simple_query_string without fields but with optional parameters + String query = + "SOURCE=" + + TEST_INDEX_BEER + + " | WHERE simple_query_string('taste', flags='ALL') | fields Id"; + var result = executeQuery(query); + assertTrue( + "simple_query_string without fields with options should return results", + result.getInt("total") > 0); + } } diff --git a/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java b/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java index 710460c1aa6..f11ae520cf6 100644 --- a/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java +++ b/integ-test/src/test/java/org/opensearch/sql/security/CrossClusterSearchIT.java @@ -204,4 +204,37 @@ public void testCrossClusterSortWithTypeCasting() throws IOException { TEST_INDEX_BANK_REMOTE)); verifyDataRows(result, rows(1), rows(6), rows(13), rows(18), rows(20), rows(25), rows(32)); } + + @Test + public void testCrossClusterMultiMatchWithoutFields() throws IOException { + // Test multi_match without fields parameter on remote cluster + JSONObject result = + executeQuery( + String.format( + "search source=%s | where multi_match('Hattie') | fields firstname", + TEST_INDEX_BANK_REMOTE)); + verifyDataRows(result, rows("Hattie")); + } + + @Test + public void testCrossClusterSimpleQueryStringWithoutFields() throws IOException { + // Test simple_query_string without fields parameter on remote cluster + JSONObject result = + executeQuery( + String.format( + "search source=%s | where simple_query_string('Hattie') | fields firstname", + TEST_INDEX_BANK_REMOTE)); + verifyDataRows(result, rows("Hattie")); + } + + @Test + public void testCrossClusterQueryStringWithoutFields() throws IOException { + // Test query_string without fields parameter on remote cluster + JSONObject result = + executeQuery( + String.format( + "search source=%s | where query_string('Hattie') | fields firstname", + TEST_INDEX_BANK_REMOTE)); + verifyDataRows(result, rows("Hattie")); + } } diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java index 3ebaf77e510..96f70c33b53 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java @@ -355,9 +355,15 @@ public Expression visitCall(RexCall call) { private QueryExpression visitRelevanceFunc(RexCall call) { String funcName = call.getOperator().getName().toLowerCase(Locale.ROOT); List ops = call.getOperands(); - if (ops.size() < 2) { + + // Validate minimum operand count based on function type + if (SINGLE_FIELD_RELEVANCE_FUNCTION_SET.contains(funcName) && ops.size() < 2) { + throw new PredicateAnalyzerException( + "Single field relevance query function should at least have 2 operands (field and" + + " query)"); + } else if (MULTI_FIELDS_RELEVANCE_FUNCTION_SET.contains(funcName) && ops.size() < 1) { throw new PredicateAnalyzerException( - "Relevance query function should at least have 2 operands"); + "Multi field relevance query function should at least have 1 operand (query)"); } if (SINGLE_FIELD_RELEVANCE_FUNCTION_SET.contains(funcName)) { @@ -376,13 +382,39 @@ private QueryExpression visitRelevanceFunc(RexCall call) { .get(funcName) .apply(namedFieldExpression, queryLiteralOperand, optionalArguments); } else if (MULTI_FIELDS_RELEVANCE_FUNCTION_SET.contains(funcName)) { - RexCall fieldsRexCall = (RexCall) AliasPair.from(ops.get(0), funcName).value; - String queryLiteralOperand = - ((LiteralExpression) - visitList(List.of(AliasPair.from(ops.get(1), funcName).value)).get(0)) - .stringValue(); - Map optionalArguments = - parseRelevanceFunctionOptionalArguments(ops, funcName); + // Handle both syntaxes: + // 1. func([fieldExpressions], query, option) - fields are present + // 2. func(query, optional) - fields are not present + RexCall fieldsRexCall = null; + String queryLiteralOperand; + Map optionalArguments; + + // Check if the first argument is fields or query by looking for "fields" key + AliasPair firstPair = AliasPair.from(ops.get(0), funcName); + String firstKey = ((RexLiteral) firstPair.alias).getValueAs(String.class); + + if ("fields".equals(firstKey)) { + // Syntax 1: func([fieldExpressions], query, option) + fieldsRexCall = (RexCall) firstPair.value; + queryLiteralOperand = + ((LiteralExpression) + visitList(List.of(AliasPair.from(ops.get(1), funcName).value)).get(0)) + .stringValue(); + optionalArguments = parseRelevanceFunctionOptionalArguments(ops, funcName, 2); + } else if ("query".equals(firstKey)) { + // Syntax 2: func(query, optional) - no fields parameter + queryLiteralOperand = + ((LiteralExpression) visitList(List.of(firstPair.value)).get(0)).stringValue(); + optionalArguments = parseRelevanceFunctionOptionalArguments(ops, funcName, 1); + } else { + throw new PredicateAnalyzerException( + format( + Locale.ROOT, + "Invalid first parameter for function [%s]: expected 'fields' or 'query', got" + + " '%s'", + funcName, + firstKey)); + } return MULTI_FIELDS_RELEVANCE_FUNCTION_HANDLERS .get(funcName) @@ -432,9 +464,14 @@ private interface MultiFieldsRelevanceFunctionHandler { private Map parseRelevanceFunctionOptionalArguments( List operands, String funcName) { + return parseRelevanceFunctionOptionalArguments(operands, funcName, 2); + } + + private Map parseRelevanceFunctionOptionalArguments( + List operands, String funcName, int startIndex) { Map optionalArguments = new HashMap<>(); - for (int i = 2; i < operands.size(); i++) { + for (int i = startIndex; i < operands.size(); i++) { AliasPair aliasPair = AliasPair.from(operands.get(i), funcName); String key = ((RexLiteral) aliasPair.alias).getValueAs(String.class); if (optionalArguments.containsKey(key)) { diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQuery.java index c05bdef1df4..b7564372b3a 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQuery.java @@ -29,14 +29,16 @@ public MultiFieldQuery(Map> queryBuildActions) { super(queryBuildActions); } + @Override + protected int getMinimumParameterCount() { + return 1; // Only requires query parameter, fields is optional + } + @Override public T createQueryBuilder(List arguments) { // Extract 'fields' and 'query' - var fields = - arguments.stream() - .filter(a -> a.getArgName().equalsIgnoreCase("fields")) - .findFirst() - .orElseThrow(() -> new SemanticCheckException("'fields' parameter is missing.")); + var fieldsOpt = + arguments.stream().filter(a -> a.getArgName().equalsIgnoreCase("fields")).findFirst(); var query = arguments.stream() @@ -44,9 +46,16 @@ public T createQueryBuilder(List arguments) { .findFirst() .orElseThrow(() -> new SemanticCheckException("'query' parameter is missing")); - var fieldsAndWeights = - fields.getValue().valueOf().tupleValue().entrySet().stream() - .collect(ImmutableMap.toImmutableMap(e -> e.getKey(), e -> e.getValue().floatValue())); + ImmutableMap fieldsAndWeights; + if (fieldsOpt.isPresent()) { + fieldsAndWeights = + fieldsOpt.get().getValue().valueOf().tupleValue().entrySet().stream() + .collect( + ImmutableMap.toImmutableMap(e -> e.getKey(), e -> e.getValue().floatValue())); + } else { + // Default to searching all fields if no fields specified + fieldsAndWeights = ImmutableMap.of(); + } return createBuilder(fieldsAndWeights, query.getValue().valueOf().stringValue()); } @@ -63,21 +72,29 @@ public T createQueryBuilder(List arguments) { * @return Final QueryBuilder */ public T build(RexCall fieldsRexCall, String query, Map optionalArguments) { - List fieldAndWeightNodes = fieldsRexCall.getOperands(); - ImmutableMap fields = - IntStream.range(0, fieldsRexCall.getOperands().size() / 2) - .map(i -> i * 2) - .mapToObj( - i -> { - RexLiteral fieldLiteral = (RexLiteral) fieldAndWeightNodes.get(i); - RexLiteral weightLiteral = (RexLiteral) fieldAndWeightNodes.get(i + 1); - String field = - ((NlsString) Objects.requireNonNull(fieldLiteral.getValue())).getValue(); - Float weight = - ((Double) Objects.requireNonNull(weightLiteral.getValue())).floatValue(); - return Map.entry(field, weight); - }) - .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); + ImmutableMap fields; + + if (fieldsRexCall == null) { + // No fields specified, search all fields by default + fields = ImmutableMap.of(); + } else { + // Extract fields and weights from the RexCall + List fieldAndWeightNodes = fieldsRexCall.getOperands(); + fields = + IntStream.range(0, fieldsRexCall.getOperands().size() / 2) + .map(i -> i * 2) + .mapToObj( + i -> { + RexLiteral fieldLiteral = (RexLiteral) fieldAndWeightNodes.get(i); + RexLiteral weightLiteral = (RexLiteral) fieldAndWeightNodes.get(i + 1); + String field = + ((NlsString) Objects.requireNonNull(fieldLiteral.getValue())).getValue(); + Float weight = + ((Double) Objects.requireNonNull(weightLiteral.getValue())).floatValue(); + return Map.entry(field, weight); + }) + .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue)); + } T queryBuilder = createBuilder(fields, query); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java index 826e6d7ddeb..bd3035cc566 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiMatchQuery.java @@ -20,7 +20,12 @@ public MultiMatchQuery() { @Override protected MultiMatchQueryBuilder createBuilder(ImmutableMap fields, String query) { - return QueryBuilders.multiMatchQuery(query).fields(fields); + MultiMatchQueryBuilder builder = QueryBuilders.multiMatchQuery(query); + if (!fields.isEmpty()) { + builder = builder.fields(fields); + } + // If fields is empty, it will search all fields by default + return builder; } @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java index 410c55cea6f..8b503d8de65 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/QueryStringQuery.java @@ -29,7 +29,12 @@ public QueryStringQuery() { @Override protected QueryStringQueryBuilder createBuilder( ImmutableMap fields, String query) { - return QueryBuilders.queryStringQuery(query).fields(fields); + QueryStringQueryBuilder builder = QueryBuilders.queryStringQuery(query); + if (!fields.isEmpty()) { + builder = builder.fields(fields); + } + // If fields is empty, it will search all fields by default + return builder; } @Override 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 6ff4ad83689..21bb8278da1 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 @@ -79,14 +79,25 @@ public QueryBuilder build(FunctionExpression func) { func.getArguments().stream() .map(a -> (NamedArgumentExpression) a) .collect(Collectors.toList()); - if (arguments.size() < 2) { + if (arguments.size() < getMinimumParameterCount()) { throw new SyntaxCheckException( - String.format("%s requires at least two parameters", getQueryName())); + String.format( + "%s requires at least %d parameter(s)", getQueryName(), getMinimumParameterCount())); } return loadArguments(arguments); } + /** + * Returns the minimum number of parameters required by the query. Subclasses can override this + * method to allow different minimum parameter counts. + * + * @return minimum parameter count + */ + protected int getMinimumParameterCount() { + return 2; // Default: requires both fields and query + } + /** * Enrich initially created opensearch index query builder with optional arguments that are * wrapped in Calcite MAP RexCall. diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java index 86dd44c1181..e377906b0b4 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/SimpleQueryStringQuery.java @@ -21,7 +21,12 @@ public SimpleQueryStringQuery() { @Override protected SimpleQueryStringBuilder createBuilder( ImmutableMap fields, String query) { - return QueryBuilders.simpleQueryStringQuery(query).fields(fields); + SimpleQueryStringBuilder builder = QueryBuilders.simpleQueryStringQuery(query); + if (!fields.isEmpty()) { + builder = builder.fields(fields); + } + // If fields is empty, it will search all fields by default + return builder; } @Override diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java index e3e810d58c9..4bec8383d1b 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java @@ -826,4 +826,96 @@ void verify_partial_pushdown() throws ExpressionNotAnalyzableException { assertEquals("Can't convert OR(=($0, 12), IS EMPTY($1))", exception.getMessage()); } } + + @Test + void multiMatchWithoutFields_generatesMultiMatchQuery() throws ExpressionNotAnalyzableException { + // Test multi_match with only query parameter (no fields) + List arguments = List.of(aliasedStringLiteral); + RexNode call = + PPLFuncImpTable.INSTANCE.resolve(builder, "multi_match", arguments.toArray(new RexNode[0])); + QueryBuilder result = PredicateAnalyzer.analyze(call, schema, fieldTypes); + + assertInstanceOf(MultiMatchQueryBuilder.class, result); + assertEquals( + """ + { + "multi_match" : { + "query" : "Hi", + "fields" : [ ], + "type" : "best_fields", + "operator" : "OR", + "slop" : 0, + "prefix_length" : 0, + "max_expansions" : 50, + "zero_terms_query" : "NONE", + "auto_generate_synonyms_phrase_query" : true, + "fuzzy_transpositions" : true, + "boost" : 1.0 + } + }""", + result.toString()); + } + + @Test + void simpleQueryStringWithoutFields_generatesSimpleQueryStringQuery() + throws ExpressionNotAnalyzableException { + // Test simple_query_string with only query parameter (no fields) + List arguments = List.of(aliasedStringLiteral); + RexNode call = + PPLFuncImpTable.INSTANCE.resolve( + builder, "simple_query_string", arguments.toArray(new RexNode[0])); + QueryBuilder result = PredicateAnalyzer.analyze(call, schema, fieldTypes); + + assertInstanceOf(SimpleQueryStringBuilder.class, result); + assertEquals( + """ + { + "simple_query_string" : { + "query" : "Hi", + "flags" : -1, + "default_operator" : "or", + "analyze_wildcard" : false, + "auto_generate_synonyms_phrase_query" : true, + "fuzzy_prefix_length" : 0, + "fuzzy_max_expansions" : 50, + "fuzzy_transpositions" : true, + "boost" : 1.0 + } + }""", + result.toString()); + } + + @Test + void queryStringWithoutFields_generatesQueryStringQuery() + throws ExpressionNotAnalyzableException { + // Test query_string with only query parameter (no fields) + List arguments = List.of(aliasedStringLiteral); + RexNode call = + PPLFuncImpTable.INSTANCE.resolve( + builder, "query_string", arguments.toArray(new RexNode[0])); + QueryBuilder result = PredicateAnalyzer.analyze(call, schema, fieldTypes); + + assertInstanceOf(QueryStringQueryBuilder.class, result); + assertEquals( + """ + { + "query_string" : { + "query" : "Hi", + "fields" : [ ], + "type" : "best_fields", + "default_operator" : "or", + "max_determinized_states" : 10000, + "enable_position_increments" : true, + "fuzziness" : "AUTO", + "fuzzy_prefix_length" : 0, + "fuzzy_max_expansions" : 50, + "phrase_slop" : 0, + "escape" : false, + "auto_generate_synonyms_phrase_query" : true, + "fuzzy_transpositions" : true, + "boost" : 1.0 + } + }""", + result.toString()); + } } 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 f561dae5fce..2fd12db0296 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 @@ -1458,19 +1458,25 @@ void should_build_match_bool_prefix_query_with_default_parameters() { } @Test - void multi_match_missing_fields_even_with_struct() { - FunctionExpression expr = - DSL.multi_match( - DSL.namedArgument( - "something-but-not-fields", - DSL.literal( - new ExprTupleValue( - new LinkedHashMap<>( - ImmutableMap.of("pewpew", ExprValueUtils.integerValue(42)))))), - DSL.namedArgument("query", literal("search query")), - DSL.namedArgument("analyzer", literal("keyword"))); - var msg = assertThrows(SemanticCheckException.class, () -> buildQuery(expr)).getMessage(); - assertEquals("'fields' parameter is missing.", msg); + void multi_match_without_fields_parameter() { + // Test that multi_match works without fields parameter (searches default fields) + assertJsonEquals( + "{\n" + + " \"multi_match\" : {\n" + + " \"query\" : \"search query\",\n" + + " \"fields\" : [ ],\n" + + " \"type\" : \"best_fields\",\n" + + " \"operator\" : \"OR\",\n" + + " \"slop\" : 0,\n" + + " \"prefix_length\" : 0,\n" + + " \"max_expansions\" : 50,\n" + + " \"zero_terms_query\" : \"NONE\",\n" + + " \"auto_generate_synonyms_phrase_query\" : true,\n" + + " \"fuzzy_transpositions\" : true,\n" + + " \"boost\" : 1.0\n" + + " }\n" + + "}", + buildQuery(DSL.multi_match(DSL.namedArgument("query", literal("search query"))))); } @Test diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java index 7fcc4a64308..71b99caf64b 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/MultiMatchTest.java @@ -157,26 +157,26 @@ public void test_SyntaxCheckException_when_no_arguments_multiMatchQuery() { } @Test - public void test_SyntaxCheckException_when_one_argument_multiMatch() { + public void test_SemanticCheckException_when_missing_query_multiMatch() { List arguments = List.of(namedArgument("fields", fields_value)); assertThrows( - SyntaxCheckException.class, + SemanticCheckException.class, () -> multiMatchQuery.build(new MultiMatchExpression(arguments))); } @Test - public void test_SyntaxCheckException_when_one_argument_multi_match() { + public void test_SemanticCheckException_when_missing_query_multi_match() { List arguments = List.of(namedArgument("fields", fields_value)); assertThrows( - SyntaxCheckException.class, + SemanticCheckException.class, () -> multiMatchQuery.build(new MultiMatchExpression(arguments, snakeCaseMultiMatchName))); } @Test - public void test_SyntaxCheckException_when_one_argument_multiMatchQuery() { + public void test_SemanticCheckException_when_missing_query_multiMatchQuery() { List arguments = List.of(namedArgument("fields", fields_value)); assertThrows( - SyntaxCheckException.class, + SemanticCheckException.class, () -> multiMatchQuery.build(new MultiMatchExpression(arguments, multiMatchQueryName))); } @@ -216,10 +216,50 @@ public void test_SemanticCheckException_when_invalid_parameter_multiMatchQuery() () -> multiMatchQuery.build(new MultiMatchExpression(arguments, multiMatchQueryName))); } + @Test + public void test_multiMatch_without_fields_parameter() { + // Test that multi_match works without fields parameter + List arguments = List.of(namedArgument("query", query_value)); + // Should not throw any exception + multiMatchQuery.build(new MultiMatchExpression(arguments)); + } + + @Test + public void test_multi_match_without_fields_parameter() { + // Test that multi_match works without fields parameter (snake_case) + List arguments = List.of(namedArgument("query", query_value)); + // Should not throw any exception + multiMatchQuery.build(new MultiMatchExpression(arguments, snakeCaseMultiMatchName)); + } + + @Test + public void test_multiMatchQuery_without_fields_parameter() { + // Test that multiMatchQuery works without fields parameter + List arguments = List.of(namedArgument("query", query_value)); + // Should not throw any exception + multiMatchQuery.build(new MultiMatchExpression(arguments, multiMatchQueryName)); + } + + @Test + public void test_multiMatch_with_optional_parameters_no_fields() { + // Test multi_match with optional parameters but no fields + List arguments = + List.of( + namedArgument("query", query_value), + namedArgument("analyzer", literal("standard")), + namedArgument("operator", literal("AND"))); + // Should not throw any exception + multiMatchQuery.build(new MultiMatchExpression(arguments)); + } + private NamedArgumentExpression namedArgument(String name, LiteralExpression value) { return DSL.namedArgument(name, value); } + private LiteralExpression literal(String value) { + return DSL.literal(value); + } + private class MultiMatchExpression extends FunctionExpression { public MultiMatchExpression(List arguments) { super(multiMatchName, arguments); diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java index 781e27d71a8..b7d2a29614e 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/QueryStringTest.java @@ -96,10 +96,10 @@ void test_SyntaxCheckException_when_no_arguments() { } @Test - void test_SyntaxCheckException_when_one_argument() { + void test_SemanticCheckException_when_missing_query() { List arguments = List.of(namedArgument("fields", fields_value)); assertThrows( - SyntaxCheckException.class, + SemanticCheckException.class, () -> queryStringQuery.build(new QueryStringExpression(arguments))); } @@ -115,6 +115,26 @@ void test_SemanticCheckException_when_invalid_parameter() { () -> queryStringQuery.build(new QueryStringExpression(arguments))); } + @Test + void test_query_string_without_fields_parameter() { + // Test that query_string works without fields parameter + List arguments = List.of(namedArgument("query", query_value)); + // Should not throw any exception + queryStringQuery.build(new QueryStringExpression(arguments)); + } + + @Test + void test_query_string_with_optional_parameters_no_fields() { + // Test query_string with optional parameters but no fields + List arguments = + List.of( + namedArgument("query", query_value), + namedArgument("default_operator", "AND"), + namedArgument("analyzer", "standard")); + // Should not throw any exception + queryStringQuery.build(new QueryStringExpression(arguments)); + } + private NamedArgumentExpression namedArgument(String name, String value) { return DSL.namedArgument(name, DSL.literal(value)); } diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java index ea144615219..d137e2eb47f 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/SimpleQueryStringTest.java @@ -48,6 +48,7 @@ class SimpleQueryStringTest { static Stream> generateValidData() { return Stream.of( List.of(DSL.namedArgument("fields", fields_value), DSL.namedArgument("query", query_value)), + List.of(DSL.namedArgument("query", query_value)), // Test without fields parameter List.of( DSL.namedArgument("fields", fields_value), DSL.namedArgument("query", query_value), @@ -143,13 +144,20 @@ public void test_SyntaxCheckException_when_no_arguments() { } @Test - public void test_SyntaxCheckException_when_one_argument() { + public void test_SemanticCheckException_when_only_fields_argument() { List arguments = List.of(namedArgument("fields", fields_value)); assertThrows( - SyntaxCheckException.class, + SemanticCheckException.class, () -> simpleQueryStringQuery.build(new SimpleQueryStringExpression(arguments))); } + @Test + public void test_valid_query_without_fields() { + List arguments = List.of(DSL.namedArgument("query", query_value)); + Assertions.assertNotNull( + simpleQueryStringQuery.build(new SimpleQueryStringExpression(arguments))); + } + @Test public void test_SemanticCheckException_when_invalid_parameter() { List arguments = diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQueryTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQueryTest.java index 9518136ff06..e02de6b59f9 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQueryTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/script/filter/lucene/relevance/MultiFieldQueryTest.java @@ -5,6 +5,8 @@ package org.opensearch.sql.opensearch.storage.script.filter.lucene.relevance; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; @@ -18,6 +20,7 @@ import org.junit.jupiter.api.Test; import org.mockito.ArgumentMatcher; import org.mockito.Mockito; +import org.opensearch.index.query.QueryBuilder; import org.opensearch.sql.data.model.ExprTupleValue; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.model.ExprValueUtils; @@ -26,12 +29,17 @@ class MultiFieldQueryTest { MultiFieldQuery query; + QueryBuilder mockQueryBuilder; private final String testQueryName = "test_query"; private final Map> actionMap = - ImmutableMap.of("paramA", (o, v) -> o); + ImmutableMap.of("paramA", (o, v) -> o, "boost", (o, v) -> o); @BeforeEach public void setUp() { + mockQueryBuilder = mock(QueryBuilder.class); + when(mockQueryBuilder.getWriteableName()).thenReturn("test_query_builder"); + when(mockQueryBuilder.queryName()).thenReturn("test_query_builder"); + query = mock( MultiFieldQuery.class, @@ -39,6 +47,7 @@ public void setUp() { .useConstructor(actionMap) .defaultAnswer(Mockito.CALLS_REAL_METHODS)); when(query.getQueryName()).thenReturn(testQueryName); + when(query.createBuilder(any(), any())).thenReturn(mockQueryBuilder); } @Test @@ -69,4 +78,36 @@ void createQueryBuilderTest() { && map.containsValue(sampleValue)), eq(sampleQuery)); } + + @Test + void createQueryBuilderWithoutFieldsTest() { + String sampleQuery = "sample query"; + + // Test creating query builder with only query parameter (no fields) + query.createQueryBuilder( + List.of( + DSL.namedArgument( + "query", new LiteralExpression(ExprValueUtils.stringValue(sampleQuery))))); + + // Should call createBuilder with empty map for fields + verify(query).createBuilder(argThat(Map::isEmpty), eq(sampleQuery)); + } + + @Test + void testGetMinimumParameterCount() { + // MultiFieldQuery should require minimum 1 parameter (query only) + assertEquals(1, query.getMinimumParameterCount()); + } + + @Test + void testBuildWithoutFields() throws Exception { + String sampleQuery = "sample query"; + Map optionalArgs = Map.of("boost", "2.0"); + + // Test build method with null fieldsRexCall (no fields specified) + query.build(null, sampleQuery, optionalArgs); + + // Should call createBuilder with empty fields map + verify(query).createBuilder(argThat(Map::isEmpty), eq(sampleQuery)); + } } 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 a93a1e5fa42..d345b090c89 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 @@ -105,7 +105,7 @@ void calls_action_when_correct_argument_name() { public void throws_SyntaxCheckException_when_no_required_arguments(List arguments) { SyntaxCheckException exception = assertThrows(SyntaxCheckException.class, () -> query.build(createCall(arguments))); - assertEquals("mock_query requires at least two parameters", exception.getMessage()); + assertEquals("mock_query requires at least 2 parameter(s)", exception.getMessage()); } public static Stream> insufficientArguments() { diff --git a/ppl/src/main/antlr/OpenSearchPPLParser.g4 b/ppl/src/main/antlr/OpenSearchPPLParser.g4 index dad56443436..3f75da1245e 100644 --- a/ppl/src/main/antlr/OpenSearchPPLParser.g4 +++ b/ppl/src/main/antlr/OpenSearchPPLParser.g4 @@ -524,7 +524,7 @@ singleFieldRelevanceFunction // Field is a list of columns multiFieldRelevanceFunction - : multiFieldRelevanceFunctionName LT_PRTHS LT_SQR_PRTHS field = relevanceFieldAndWeight (COMMA field = relevanceFieldAndWeight)* RT_SQR_PRTHS COMMA query = relevanceQuery (COMMA relevanceArg)* RT_PRTHS + : multiFieldRelevanceFunctionName LT_PRTHS (LT_SQR_PRTHS field = relevanceFieldAndWeight (COMMA field = relevanceFieldAndWeight)* RT_SQR_PRTHS COMMA)? query = relevanceQuery (COMMA relevanceArg)* RT_PRTHS ; // tables diff --git a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java index 212616ac813..cd5e3a76d8d 100644 --- a/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java +++ b/ppl/src/main/java/org/opensearch/sql/ppl/parser/AstExpressionBuilder.java @@ -609,17 +609,27 @@ private List multiFieldRelevanceArguments( // all the arguments are defaulted to string values // to skip environment resolving and function signature resolving ImmutableList.Builder builder = ImmutableList.builder(); - var fields = - new RelevanceFieldList( - ctx.getRuleContexts(OpenSearchPPLParser.RelevanceFieldAndWeightContext.class).stream() - .collect( - Collectors.toMap( - f -> StringUtils.unquoteText(f.field.getText()), - f -> (f.weight == null) ? 1F : Float.parseFloat(f.weight.getText())))); - builder.add(new UnresolvedArgument("fields", fields)); + + // Handle optional fields - only add fields argument if fields are present + var fieldContexts = + ctx.getRuleContexts(OpenSearchPPLParser.RelevanceFieldAndWeightContext.class); + if (fieldContexts != null && !fieldContexts.isEmpty()) { + var fields = + new RelevanceFieldList( + fieldContexts.stream() + .collect( + Collectors.toMap( + f -> StringUtils.unquoteText(f.field.getText()), + f -> (f.weight == null) ? 1F : Float.parseFloat(f.weight.getText())))); + builder.add(new UnresolvedArgument("fields", fields)); + } + + // Query is always required builder.add( new UnresolvedArgument( "query", new Literal(StringUtils.unquoteText(ctx.query.getText()), DataType.STRING))); + + // Add optional arguments ctx.relevanceArg() .forEach( v -> 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 e693952e13f..4a85acb067a 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 @@ -305,6 +305,39 @@ public void testCanParseQueryStringRelevanceFunction() { + "analyzer=keyword, quote_field_suffix=\".exact\", fuzzy_prefix_length = 4)")); } + @Test + public void testMultiFieldRelevanceFunctionsWithoutFields() { + // Test multi_match without fields parameter + assertNotEquals( + null, new PPLSyntaxParser().parse("SOURCE=test | WHERE multi_match('query text')")); + + // Test multi_match without fields but with optional parameters + assertNotEquals( + null, + new PPLSyntaxParser() + .parse("SOURCE=test | WHERE multi_match('query text', analyzer='keyword')")); + + // Test simple_query_string without fields parameter + assertNotEquals( + null, new PPLSyntaxParser().parse("SOURCE=test | WHERE simple_query_string('query text')")); + + // Test simple_query_string without fields but with optional parameters + assertNotEquals( + null, + new PPLSyntaxParser() + .parse("SOURCE=test | WHERE simple_query_string('query text', flags='ALL')")); + + // Test query_string without fields parameter + assertNotEquals( + null, new PPLSyntaxParser().parse("SOURCE=test | WHERE query_string('query text')")); + + // Test query_string without fields but with optional parameters + assertNotEquals( + null, + new PPLSyntaxParser() + .parse("SOURCE=test | WHERE query_string('query text', default_operator='AND')")); + } + @Test public void testDescribeCommandShouldPass() { ParseTree tree = new PPLSyntaxParser().parse("describe t"); diff --git a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java index 28a941b8238..0be39549dec 100644 --- a/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java +++ b/ppl/src/test/java/org/opensearch/sql/ppl/parser/AstExpressionBuilderTest.java @@ -960,6 +960,49 @@ public void canBuildQuery_stringRelevanceFunctionWithArguments() { unresolvedArg("analyzer", stringLiteral("keyword"))))); } + @Test + public void canBuildMulti_matchRelevanceFunctionWithoutFields() { + // Test multi_match with only query parameter (no fields) + assertEqual( + "source=test | where multi_match('test query')", + filter( + relation("test"), + function("multi_match", unresolvedArg("query", stringLiteral("test query"))))); + } + + @Test + public void canBuildMulti_matchRelevanceFunctionWithoutFieldsButWithOptions() { + // Test multi_match with query and optional parameters but no fields + assertEqual( + "source=test | where multi_match('test query', analyzer='keyword')", + filter( + relation("test"), + function( + "multi_match", + unresolvedArg("query", stringLiteral("test query")), + unresolvedArg("analyzer", stringLiteral("keyword"))))); + } + + @Test + public void canBuildSimple_query_stringRelevanceFunctionWithoutFields() { + // Test simple_query_string with only query parameter (no fields) + assertEqual( + "source=test | where simple_query_string('test query')", + filter( + relation("test"), + function("simple_query_string", unresolvedArg("query", stringLiteral("test query"))))); + } + + @Test + public void canBuildQuery_stringRelevanceFunctionWithoutFields() { + // Test query_string with only query parameter (no fields) + assertEqual( + "source=test | where query_string('test query')", + filter( + relation("test"), + function("query_string", unresolvedArg("query", stringLiteral("test query"))))); + } + @Test public void functionNameCanBeUsedAsIdentifier() { assertFunctionNameCouldBeId(