Skip to content

Add Support for Relevance functions - #21562

Merged
bharath-techie merged 9 commits into
opensearch-project:mainfrom
nssuresh2007:relevance_functions
May 14, 2026
Merged

Add Support for Relevance functions#21562
bharath-techie merged 9 commits into
opensearch-project:mainfrom
nssuresh2007:relevance_functions

Conversation

@nssuresh2007

@nssuresh2007 nssuresh2007 commented May 8, 2026

Copy link
Copy Markdown
Contributor

Description

Adds support for the remaining OpenSearch relevance (full-text search) functions in the analytics engine's Lucene backend serialization layer:

  • Single-field functions: match_phrase, match_bool_prefix, match_phrase_prefix
  • Multi-field functions: multi_match, query_string, simple_query_string
Changes
  • ScalarFunction — Registers the 5 new full-text function enum entries
  • QuerySerializerRegistry — Adds serializers for all 6 new functions (converts Calcite RexCall → OpenSearch QueryBuilder → serialized bytes)
  • ConversionUtils — New extractFieldsFromRelevanceMap() method to extract multiple field names from nested MAP operands (supports both literal field names with boosts
    and RexInputRef fallback)
  • OpenSearchFilterRule — Moves function resolution before the fieldIndices.isEmpty() check so multi-field functions (which encode fields as string literals in MAPs
    rather than RexInputRef) can resolve backend viability correctly
  • LuceneAnalyticsBackendPlugin — Registers the new functions in the FULL_TEXT_OPS capability set
Tests
  • ConversionUtilsTests — Unit tests for multi-field extraction (single field, multiple fields, RexInputRef fallback, error case)
  • MultiFieldExtractionOrderPropertyTests — Property-based tests verifying field extraction preserves insertion order (100 iterations, 1-10 random fields)
  • SingleFieldSerializationPropertyTests — Round-trip serialization tests for match_phrase, match_bool_prefix, match_phrase_prefix
  • MultiFieldSerializationPropertyTests — Round-trip serialization tests for multi_match, query_string, simple_query_string
  • QuerySerializerRegistryTests — Registry completeness validation (7 entries, correct keys, non-null values)

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

Testing

1. match() on text field
1a. Single term match on description
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"search\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 5 docs (Alice, Charlie, Eve, Henry, Jack) — all contain "search" in description
Actual: 5 docs ✅

1b. Multi-term match (OR semantics)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"search optimization\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[31,"Seattle","Backend developer focused on high performance computing and optimization","Frank",77.8],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 6 docs — those with "search" OR "optimization"
Actual: 6 docs (Alice, Charlie, Eve, Frank, Henry, Jack) ✅

1c. match on keyword field (city)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(city, \"Seattle\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[31,"Seattle","Backend developer focused on high performance computing and optimization","Frank",77.8],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 4 docs in Seattle
Actual: 4 docs (Alice, Charlie, Frank, Jack) ✅


2. match_phrase()
2a. Phrase that exists (adjacent terms)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match_phrase(description, \"search engines\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5]]}

Expected: 1 doc (Alice) — only one has "search engines" as adjacent terms
Actual: 1 doc ✅

2b. Phrase that does NOT exist (non-adjacent terms)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match_phrase(description, \"search distributed\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[]}

Expected: 0 docs — "search" and "distributed" are not adjacent in any doc
Actual: 0 docs ✅


3. multi_match()
3a. multi_match across text fields
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where multi_match([description, name], \"search\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 5 docs matching "search" in description (name is keyword, won't match analyzed "search")
Actual: 5 docs ✅

3b. multi_match across text + keyword fields
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where multi_match([description, city], \"Seattle\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"seattle","This is seattle city","alice",95.5],[35,"seattle","This is seattle city","carol",92.3],[32,"seattle","This is seattle city","eve",91.0],[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[31,"Seattle","Backend developer focused on high performance computing and optimization","Frank",77.8],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: Docs matching "Seattle" in city (keyword exact) or description (text analyzed)
Actual: 7 docs (includes pre-existing lowercase docs + our 4 Seattle docs) ✅


4. query_string()
4a. query_string with AND operator
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where query_string([description], \"search AND optimization\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0]]}

Expected: 1 doc (Charlie) — only one has both "search" AND "optimization"
Actual: 1 doc ✅

4b. query_string with OR operator
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where query_string([description], \"search OR machine\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[25,"Portland","Data scientist specializing in machine learning and natural language processing","Bob",72.3],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[27,"Portland","Machine learning engineer working on natural language understanding","Grace",88.1],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 7 docs — those with "search" OR "machine"
Actual: 7 docs ✅

4c. query_string with wildcard
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where query_string([description], \"search*\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 5 docs with words starting with "search"
Actual: 5 docs ✅


5. simple_query_string()
5a. simple_query_string with + (required) operator
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where simple_query_string([description], \"search +optimization\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0]]}

Expected: 1 doc — must have "optimization", optionally "search"
Actual: 1 doc (Charlie) ✅


6. match_phrase_prefix()
6a. Prefix completion of phrase
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match_phrase_prefix(description, \"search eng\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 2 docs — "search eng*" matches "search engines" (Alice) and "Search engineer" (Jack)
Actual: 2 docs ✅


7. match_bool_prefix()
7a. Bool prefix matching
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match_bool_prefix(description, \"search plat\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: Docs matching "search" (as full term) AND/OR "plat*" (as prefix) — match_bool_prefix creates term queries for all but last term, prefix query for last
Actual: 5 docs — all have "search"; "plat" prefix matches "platform" in Eve and Henry ✅


8. Term/Terms on keyword fields
8a. Exact term match on keyword (name)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where name = \"Alice\""}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5]]}

Expected: 1 doc (Alice)
Actual: 1 doc ✅

8b. IN clause (terms equivalent) on keyword (city)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where city IN (\"Seattle\", \"Portland\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[25,"Portland","Data scientist specializing in machine learning and natural language processing","Bob",72.3],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[31,"Seattle","Backend developer focused on high performance computing and optimization","Frank",77.8],[27,"Portland","Machine learning engineer working on natural language understanding","Grace",88.1],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 6 docs (4 Seattle + 2 Portland)
Actual: 6 docs ✅


9. Range queries on numeric fields
9a. Range on integer (age > 35)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where age > 35"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4]]}

Expected: 2 docs (Eve age=42, Henry age=45)
Actual: 2 docs ✅

9b. Range on double (score between 85 and 95)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where score >= 85.0 AND score <= 95.0"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[27,"Portland","Machine learning engineer working on natural language understanding","Grace",88.1],[25,"portland","This is portland","bob",88.0],[35,"seattle","This is seattle city","carol",92.3],[32,"seattle","This is seattle city","eve",91.0]]}

Expected: Docs with score in [85.0, 95.0] — includes pre-existing docs
Actual: 6 docs (3 from our set + 3 pre-existing) ✅


10. Bool compound queries
10a. match OR match (two FTS functions combined)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"search\") OR match(description, \"machine\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[25,"Portland","Data scientist specializing in machine learning and natural language processing","Bob",72.3],[35,"Seattle","Senior developer building search infrastructure and query optimization","Charlie",91.0],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2],[27,"Portland","Machine learning engineer working on natural language understanding","Grace",88.1],[45,"New York","Project manager overseeing search and analytics platform teams","Henry",62.4],[29,"Seattle","Search engineer improving relevance scoring and query processing","Jack",83.6]]}

Expected: 7 docs — union of "search" (5) and "machine" (2+)
Actual: 7 docs ✅

10b. match AND match (two FTS functions intersected)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"search\") AND match(description, \"distributed\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5],[42,"New York","Principal engineer leading distributed search platform development","Eve",95.2]]}

Expected: 2 docs — those with both "search" AND "distributed" (Alice, Eve)
Actual: 2 docs ✅

10c. match AND range filter (FTS + numeric)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"search\") AND age > 30"}'

Result: ❌ FAIL

{"error":{"root_cause":[{"type":"runtime_exception","reason":"Stage 0 failed"}],"type":"runtime_exception","reason":"Stage 0 failed","caused_by":{"type":"stream_exception","reason":"java.lang.RuntimeException: Execution error: Execution error: Panic: primitive array","content-type":"application/grpc","raw-header":"RVMAAAA2AAAAAAAAABMBCC7YkwAAACUBHF9zeXN0ZW1faW5kZXhfYWNjZXNzX2FsbG93ZWQFZmFsc2UAAAAAAA==","correlation-id":"1864987403856379924"}},"status":500}

Expected: 3 docs (Charlie age=35, Eve age=42, Henry age=45 — all have "search" and age>30)
Actual: Runtime panic
[Clarified that this issue is not related to this PR and will be handled separately]

10d. match AND keyword filter (FTS + keyword equality)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"search\") AND city = \"Seattle\""}'

Result: ❌ FAIL

{"error":{"root_cause":[{"type":"runtime_exception","reason":"Stage 0 failed"}],"type":"runtime_exception","reason":"Stage 0 failed","caused_by":{"type":"stream_exception","reason":"java.lang.RuntimeException: Execution error: Execution error: Panic: primitive array","content-type":"application/grpc","raw-header":"RVMAAAA2AAAAAAAAABQBCC7YkwAAACUBHF9zeXN0ZW1faW5kZXhfYWNjZXNzX2FsbG93ZWQFZmFsc2UAAAAAAA==","correlation-id":"1864987403856379925"}},"status":500}

Expected: 3 docs (Alice, Charlie, Jack — have "search" and city=Seattle)
Actual: Runtime panic
[Clarified that this issue is not related to this PR and will be handled separately]


11. Negative scenarios
11a. match on non-existent field
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(nonexistent_field, \"search\")"}'

Result: ✅ PASS (proper error)

{"error":{"root_cause":[{"type":"illegal_state_exception","reason":"Failed to plan query"}],"type":"illegal_state_exception","reason":"Failed to plan query","caused_by":{"type":"error_report","reason":"Field [nonexistent_field] not found.","caused_by":{"type":"illegal_argument_exception","reason":"Field [nonexistent_field] not found."}}},"status":500}

Expected: Error indicating field not found
Actual: Clear error message ✅

11b. match on integer field (unsupported type for FTS)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(age, \"30\")"}'

Result: ✅ PASS (proper error)

{"error":{"root_cause":[{"type":"illegal_state_exception","reason":"No backend can evaluate filter predicate [OTHER_FUNCTION] on fields [age:integer]"}],"type":"illegal_state_exception","reason":"No backend can evaluate filter predicate [OTHER_FUNCTION] on fields [age:integer]"},"status":500}

Expected: Error — match not supported on integer fields
Actual: Clear error about no backend for integer field ✅

11c. match on double field (unsupported type for FTS)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(score, \"85.5\")"}'

Result: ✅ PASS (proper error)

{"error":{"root_cause":[{"type":"illegal_state_exception","reason":"No backend can evaluate filter predicate [OTHER_FUNCTION] on fields [score:double]"}],"type":"illegal_state_exception","reason":"No backend can evaluate filter predicate [OTHER_FUNCTION] on fields [score:double]"},"status":500}

Expected: Error — match not supported on double fields
Actual: Clear error ✅

11d. match_phrase on keyword field (should work — keyword supports phrase)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match_phrase(name, \"Alice\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[[30,"Seattle","Software engineer working on search engines and distributed systems","Alice",85.5]]}

Expected: 1 doc — keyword field supports exact phrase match
Actual: 1 doc ✅

11e. match with empty string
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[]}

Expected: 0 docs — empty query matches nothing
Actual: 0 docs ✅

11f. match with non-matching term
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where match(description, \"xyznonexistent\")"}'

Result: ✅ PASS

{"columns":["age","city","description","name","score"],"rows":[]}

Expected: 0 docs — term doesn't exist in any document
Actual: 0 docs ✅

11g. query_string with invalid syntax (all operators, no terms)
curl -s -X POST "http://localhost:9200/_analytics/ppl" -H 'Content-Type: application/json' -d '{"query": "source=parquet_test | where query_string([description], \"AND OR NOT\")"}'

Result: ✅ PASS (error returned)

{"error":{"root_cause":[{"type":"runtime_exception","reason":"Stage 0 failed"}],"type":"runtime_exception","reason":"Stage 0 failed","caused_by":{"type":"stream_exception","reason":"Failed to start streaming fragment on [parquet_test][0]"}},"status":500}

Expected: Error — invalid query syntax
Actual: Error returned ✅

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Suresh N S <nssuresh@amazon.com>
@nssuresh2007
nssuresh2007 requested a review from a team as a code owner May 8, 2026 11:33
@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

PR Reviewer Guide 🔍

(Review updated until commit 6ab7900)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
📝 TODO sections

🔀 No multiple PR themes
⚡ Recommended focus areas for review

Possible Issue

In extractFieldsFromRelevanceMap, the nested MAP extraction loop increments by 2 (i += 2) assuming strict alternating field/boost pairs. If the nested MAP has an odd number of operands, the loop will skip the last operand without validation. This could silently ignore a field name if the MAP structure is malformed (e.g., missing a boost value for the last field).

for (int i = 0; i < nestedOperands.size(); i += 2) {
    RexNode fieldNode = nestedOperands.get(i);
    if (fieldNode instanceof RexLiteral fieldLiteral) {
        fields.add(fieldLiteral.getValueAs(String.class));
    }
}
if (fields.isEmpty() == false) {
    return fields;
}
Possible Issue

In extractFieldsFromRelevanceMap RexInputRef fallback path, the loop increments by 2 (i += 2) starting from index 1, assuming alternating key/value pairs. If mapOperands has an even number of elements, the loop will attempt to access an out-of-bounds index on the last iteration (e.g., if size is 4, loop tries i=1, i=3, then i=5 which is out of bounds). This will throw IndexOutOfBoundsException.

for (int i = 1; i < mapOperands.size(); i += 2) {
    RexNode val = mapOperands.get(i);
    if (val instanceof RexInputRef inputRef) {
        fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
    }
}
Possible Issue

In serializeMultiMatch, if operands.fields() is null, the code constructs a MultiMatchQueryBuilder with no fields specified. OpenSearch's MultiMatchQueryBuilder may reject queries with no fields at query execution time, causing runtime failures. The code should either require at least one field or handle the no-fields case explicitly.

MultiMatchQueryBuilder queryBuilder = fields != null
    ? new MultiMatchQueryBuilder(operands.query(), fields.toArray(String[]::new))
    : new MultiMatchQueryBuilder(operands.query());

@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

PR Code Suggestions ✨

Latest suggestions up to 6ab7900

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Validate MAP key literals in fallback

The fallback path for RexInputRef-based multi-field extraction assumes all
odd-indexed operands are RexInputRef, but doesn't validate that even-indexed
operands are 'field' key literals. This could silently skip non-conforming entries
or extract fields from malformed structures. Add validation to ensure even indices
contain the expected 'field' key literal before processing odd-indexed RexInputRef
values.

sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/ConversionUtils.java [189-196]

 static List<String> extractFieldsFromRelevanceMap(RexCall call, int operandIndex, List<FieldStorageInfo> fieldStorage) {
     RexNode operand = call.getOperands().get(operandIndex);
     List<String> fields = new ArrayList<>();
     if (operand instanceof RexCall outerMapCall) {
         ...
         // Fallback: RexInputRef-based structure MAP('field', $ref1, 'field', $ref2, ...)
         List<RexNode> mapOperands = outerMapCall.getOperands();
-        for (int i = 1; i < mapOperands.size(); i += 2) {
-            RexNode val = mapOperands.get(i);
-            if (val instanceof RexInputRef inputRef) {
-                fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
+        for (int i = 0; i < mapOperands.size() - 1; i += 2) {
+            RexNode key = mapOperands.get(i);
+            if (key instanceof RexLiteral keyLiteral && KEY_FIELD.equals(keyLiteral.getValueAs(String.class))) {
+                RexNode val = mapOperands.get(i + 1);
+                if (val instanceof RexInputRef inputRef) {
+                    fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
+                }
             }
         }
     } else if (operand instanceof RexInputRef inputRef) {
         fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
     }
     if (fields.isEmpty()) {
         throw new IllegalArgumentException("Cannot extract field names from operand " + operandIndex + ": " + operand);
     }
     return fields;
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the fallback path doesn't validate the 'field' key literals at even indices before extracting RexInputRef values at odd indices. Adding this validation would prevent silent failures and ensure the MAP structure conforms to expectations, improving robustness.

Medium
Validate operand types before fallback extraction

The positional fallback unconditionally attempts extraction when all three values
are null, but doesn't verify that operands 0 and 1 are actually the expected types
(RexInputRef/RexCall for field, RexLiteral for query). If the operands are malformed
or in a different structure, the extraction methods will throw exceptions. Add type
checks before invoking the extraction methods to fail fast with a clearer error
message.

sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/ConversionUtils.java [138-141]

 static RelevanceOperands extractRelevanceOperands(RexCall call, List<FieldStorageInfo> fieldStorage) {
     String fieldName = null;
     List<String> fields = null;
     String query = null;
 
     for (int i = 0; i < call.getOperands().size(); i++) {
         String key = extractMapKey(call, i);
         if (KEY_FIELD.equals(key)) {
             fieldName = extractFieldFromRelevanceMap(call, i, fieldStorage);
         } else if (KEY_FIELDS.equals(key)) {
             fields = extractFieldsFromRelevanceMap(call, i, fieldStorage);
         } else if (KEY_QUERY.equals(key)) {
             query = extractStringFromRelevanceMap(call, i);
         }
     }
 
     // Fallback: positional extraction for non-MAP operand structures (e.g. MATCH($ref, literal))
     if (fieldName == null && fields == null && query == null && call.getOperands().size() >= 2) {
-        fieldName = extractFieldFromRelevanceMap(call, 0, fieldStorage);
-        query = extractStringFromRelevanceMap(call, 1);
+        RexNode operand0 = call.getOperands().get(0);
+        RexNode operand1 = call.getOperands().get(1);
+        if ((operand0 instanceof RexInputRef || operand0 instanceof RexCall) && (operand1 instanceof RexLiteral || operand1 instanceof RexCall)) {
+            fieldName = extractFieldFromRelevanceMap(call, 0, fieldStorage);
+            query = extractStringFromRelevanceMap(call, 1);
+        }
     }
 
     return new RelevanceOperands(fieldName, fields, query);
 }
Suggestion importance[1-10]: 6

__

Why: The suggestion identifies that the positional fallback doesn't validate operand types before calling extraction methods, which could lead to unclear exceptions. Adding type checks would provide better error messages and fail-fast behavior, though the existing extraction methods likely already handle invalid types with exceptions.

Low

Previous suggestions

Suggestions up to commit e384326
CategorySuggestion                                                                                                                                    Impact
General
Ensure mutually exclusive extraction paths

The single RexInputRef fallback path is executed after the RexCall paths, which
means if the operand is a RexInputRef, fields will be added but the previous RexCall
logic may have already populated the list. This could lead to unexpected behavior
where both paths contribute to the result. Consider using else-if to ensure mutual
exclusivity.

sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/ConversionUtils.java [134-136]

-if (operand instanceof RexInputRef inputRef) {
+} else if (operand instanceof RexInputRef inputRef) {
     fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
 }
 if (fields.isEmpty()) {
     throw new IllegalArgumentException("Cannot extract field names from operand " + operandIndex + ": " + operand);
 }
Suggestion importance[1-10]: 8

__

Why: This is a valid bug catch. The current code structure allows both the RexCall path (lines 105-132) and the single RexInputRef path (lines 134-136) to execute for the same operand, potentially adding fields twice. Using else if ensures mutual exclusivity and prevents incorrect behavior where fields could be duplicated in the result list.

Medium
Validate multi-field function assumption explicitly

The logic assumes all full-text functions with empty fieldIndices are multi-field
functions, but single-field functions like match_phrase could also have empty
fieldIndices if field extraction fails. This could mask extraction bugs by returning
viable backends instead of failing. Add validation to ensure only multi-field
functions take this path.

sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchFilterRule.java [154-165]

 if (fieldIndices.isEmpty()) {
     // Multi-field full-text functions (multi_match, query_string, simple_query_string)
     // encode field names as string literals in nested MAPs rather than RexInputRef.
-    // Resolve viability against any backend that supports the function on text fields.
-    if (function.getCategory() == ScalarFunction.Category.FULL_TEXT) {
+    if (function.getCategory() == ScalarFunction.Category.FULL_TEXT 
+        && (function == ScalarFunction.MULTI_MATCH 
+            || function == ScalarFunction.QUERY_STRING 
+            || function == ScalarFunction.SIMPLE_QUERY_STRING)) {
         return new ArrayList<>(registry.filterBackendsAnyFormat(function, FieldType.TEXT));
     }
     throw new UnsupportedOperationException(
         "Constant predicate with no field references reached the filter rule: ["
             + predicate
             + "]. ReduceExpressionsRule in PlannerImpl should have eliminated it."
     );
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the current logic assumes all full-text functions with empty fieldIndices are multi-field functions, which could mask bugs in field extraction for single-field functions. Adding explicit validation for MULTI_MATCH, QUERY_STRING, and SIMPLE_QUERY_STRING improves correctness and makes the code more defensive against extraction failures.

Medium
Prevent duplicate field names in extraction

The fallback path for RexInputRef extraction may produce duplicate field names when
the same field appears multiple times in the MAP structure. This could lead to
incorrect query behavior in multi-field functions. Add deduplication logic or
validate that each field appears only once.

sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/ConversionUtils.java [126-132]

-static List<String> extractFieldsFromRelevanceMap(RexCall call, int operandIndex, List<FieldStorageInfo> fieldStorage) {
-    RexNode operand = call.getOperands().get(operandIndex);
-    List<String> fields = new ArrayList<>();
-    if (operand instanceof RexCall outerMapCall) {
-        // Check if the value (index 1) is a nested MAP containing field name/boost pairs
-        if (outerMapCall.getOperands().size() >= 2) {
-            RexNode value = outerMapCall.getOperands().get(1);
-            if (value instanceof RexCall nestedMapCall) {
-                ...
-                if (fields.isEmpty() == false) {
-                    return fields;
-                }
-            }
-        }
-        // Fallback: RexInputRef-based structure MAP('field', $ref1, 'field', $ref2, ...)
-        List<RexNode> mapOperands = outerMapCall.getOperands();
-        for (int i = 1; i < mapOperands.size(); i += 2) {
-            RexNode val = mapOperands.get(i);
-            if (val instanceof RexInputRef inputRef) {
-                fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
-            }
+// Fallback: RexInputRef-based structure MAP('field', $ref1, 'field', $ref2, ...)
+List<RexNode> mapOperands = outerMapCall.getOperands();
+Set<String> seenFields = new HashSet<>();
+for (int i = 1; i < mapOperands.size(); i += 2) {
+    RexNode val = mapOperands.get(i);
+    if (val instanceof RexInputRef inputRef) {
+        String fieldName = FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName();
+        if (seenFields.add(fieldName)) {
+            fields.add(fieldName);
         }
     }
+}
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly identifies that the fallback path could produce duplicate field names when the same field appears multiple times. However, the PR code already handles this by using a List which preserves order, and duplicates may be intentional for boost handling. The suggestion adds defensive deduplication which improves robustness, but the impact is moderate since the current implementation may be working as intended.

Low
Suggestions up to commit c7590ae
CategorySuggestion                                                                                                                                    Impact
General
Verify multi-field function before routing

The logic assumes all full-text functions with empty fieldIndices are multi-field
functions, but single-field functions like match or match_phrase could also have
empty indices if malformed. Verify that function is actually a multi-field function
before returning backends, or the planner may incorrectly route single-field
queries.

sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchFilterRule.java [154-166]

 if (fieldIndices.isEmpty()) {
     // Multi-field full-text functions (multi_match, query_string, simple_query_string)
     // encode field names as string literals in nested MAPs rather than RexInputRef.
     // Resolve viability against any backend that supports the function on text fields.
     if (function.getCategory() == ScalarFunction.Category.FULL_TEXT) {
-        return new ArrayList<>(registry.filterBackendsAnyFormat(function, FieldType.TEXT));
+        if (function == ScalarFunction.MULTI_MATCH || function == ScalarFunction.QUERY_STRING || function == ScalarFunction.SIMPLE_QUERY_STRING) {
+            return new ArrayList<>(registry.filterBackendsAnyFormat(function, FieldType.TEXT));
+        }
     }
     throw new UnsupportedOperationException(
         "Constant predicate with no field references reached the filter rule: ["
             + predicate
             + "]. ReduceExpressionsRule in PlannerImpl should have eliminated it."
     );
 }
Suggestion importance[1-10]: 9

__

Why: The suggestion identifies a critical logic flaw: the code assumes all full-text functions with empty fieldIndices are multi-field functions, but single-field functions like MATCH or MATCH_PHRASE could also have empty indices if malformed. The improved code adds an explicit check for MULTI_MATCH, QUERY_STRING, and SIMPLE_QUERY_STRING before returning backends, preventing incorrect routing of single-field queries. This is a high-impact correctness fix.

High
Add early return after successful extraction

The fallback path for RexInputRef-based structure continues execution even after
successfully extracting fields from the nested MAP path. Add an early return after
the nested MAP extraction succeeds to prevent unnecessary fallback processing and
potential duplicate field entries.

sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/ConversionUtils.java [105-141]

 static List<String> extractFieldsFromRelevanceMap(RexCall call, int operandIndex, List<FieldStorageInfo> fieldStorage) {
     RexNode operand = call.getOperands().get(operandIndex);
     List<String> fields = new ArrayList<>();
     if (operand instanceof RexCall outerMapCall) {
         // Check if the value (index 1) is a nested MAP containing field name/boost pairs
         if (outerMapCall.getOperands().size() >= 2) {
             RexNode value = outerMapCall.getOperands().get(1);
             if (value instanceof RexCall nestedMapCall) {
                 ...
+                if (fields.isEmpty() == false) {
+                    return fields;
+                }
             }
         }
         // Fallback: RexInputRef-based structure MAP('field', $ref1, 'field', $ref2, ...)
         List<RexNode> mapOperands = outerMapCall.getOperands();
         for (int i = 1; i < mapOperands.size(); i += 2) {
             RexNode val = mapOperands.get(i);
             if (val instanceof RexInputRef inputRef) {
                 fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
             }
+        }
+        if (fields.isEmpty() == false) {
+            return fields;
         }
     }
     if (operand instanceof RexInputRef inputRef) {
         fields.add(FieldStorageInfo.resolve(fieldStorage, inputRef.getIndex()).getFieldName());
     }
     if (fields.isEmpty()) {
         throw new IllegalArgumentException("Cannot extract field names from operand " + operandIndex + ": " + operand);
     }
     return fields;
 }
Suggestion importance[1-10]: 8

__

Why: The code already has an early return at line 121 (if (fields.isEmpty() == false) return fields;), but the suggestion correctly identifies that the fallback RexInputRef path (lines 126-132) will still execute even after the nested MAP path succeeds. The improved code adds an additional early return after the fallback path succeeds (line 132), preventing the final RexInputRef check (lines 134-136) from executing unnecessarily. This improves efficiency and prevents potential duplicate field entries.

Medium
Validate even operand count for pairs

The loop assumes strict alternating key-value pairs but doesn't validate that the
list size is even. If nestedOperands.size() is odd, the last iteration will access
index i but skip the boost at i+1, which could mask malformed input. Add a size
validation check before the loop.

sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/ConversionUtils.java [113-119]

+if (nestedOperands.size() % 2 != 0) {
+    throw new IllegalArgumentException("Nested MAP operands must have even size (field-boost pairs), got: " + nestedOperands.size());
+}
 for (int i = 0; i < nestedOperands.size(); i += 2) {
     RexNode fieldNode = nestedOperands.get(i);
     if (fieldNode instanceof RexLiteral fieldLiteral) {
         fields.add(fieldLiteral.getValueAs(String.class));
     }
 }
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies that the loop at lines 114-119 assumes strict alternating key-value pairs but doesn't validate that nestedOperands.size() is even. Adding a validation check before the loop would catch malformed input early and provide a clearer error message. However, the impact is moderate since malformed input would likely fail elsewhere in the pipeline.

Medium
Suggestions up to commit 1f28400
CategorySuggestion                                                                                                                                    Impact
Security
Avoid logging full operand object

The error message includes the entire operand object which may contain sensitive
data or produce extremely verbose output. Consider logging only the operand's class
type or a truncated representation to prevent potential information leakage or log
flooding.

sandbox/plugins/analytics-backend-lucene/src/main/java/org/opensearch/be/lucene/ConversionUtils.java [132-134]

 if (fields.isEmpty()) {
-    throw new IllegalArgumentException("Cannot extract field names from operand " + operandIndex + ": " + operand);
+    throw new IllegalArgumentException("Cannot extract field names from operand " + operandIndex + " of type " + operand.getClass().getSimpleName());
 }
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that logging the full operand object could produce verbose output. However, the security concern about "sensitive data" is overstated since this is internal query processing. The improvement to log only the class type is reasonable for cleaner logs and debugging.

Low
General
Avoid logging full predicate object

The exception message includes the entire predicate object which may produce verbose
output or expose internal query structure. Consider using a more concise
representation such as the predicate's operator name and kind to avoid log
pollution.

sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/planner/rules/OpenSearchFilterRule.java [161-165]

 if (fieldIndices.isEmpty()) {
     // Multi-field full-text functions (multi_match, query_string, simple_query_string)
     // encode field names as string literals in nested MAPs rather than RexInputRef.
     // Resolve viability against any backend that supports the function on text fields.
     if (function.getCategory() == ScalarFunction.Category.FULL_TEXT) {
         return new ArrayList<>(registry.filterBackendsAnyFormat(function, FieldType.TEXT));
     }
     throw new UnsupportedOperationException(
         "Constant predicate with no field references reached the filter rule: ["
-            + predicate
+            + predicate.getOperator().getName() + " / " + predicate.getKind()
             + "]. ReduceExpressionsRule in PlannerImpl should have eliminated it."
     );
 }
Suggestion importance[1-10]: 4

__

Why: The suggestion addresses log verbosity by proposing to log only predicate.getOperator().getName() and predicate.getKind() instead of the full predicate object. However, the improved_code already includes this pattern in the error message construction, making the suggestion's impact minimal. The concern is valid but the improvement is marginal.

Low

@github-actions

github-actions Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 1f28400: SUCCESS

@codecov

codecov Bot commented May 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.40%. Comparing base (36809cc) to head (6ab7900).
⚠️ Report is 24 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #21562      +/-   ##
============================================
- Coverage     73.50%   73.40%   -0.10%     
+ Complexity    74644    74559      -85     
============================================
  Files          5980     5980              
  Lines        338777   338825      +48     
  Branches      48848    48857       +9     
============================================
- Hits         249011   248728     -283     
- Misses        69946    70328     +382     
+ Partials      19820    19769      -51     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@expani expani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for getting this started @nssuresh2007

We can also look at running the Integ tests in SQL/PPL plugin to see the improvement coverage and identify any gaps early.

  ./gradlew :integ-test:integTestRemote \
    -Dtests.rest.cluster=localhost:9200 \
    -Dtests.cluster=localhost:9300 \
    -Dtests.clustername=runTask \
    --tests "org.opensearch.sql.calcite.remote.CalcitePPLAggregationIT"

Signed-off-by: Suresh N S <nssuresh@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit c7590ae

Signed-off-by: Suresh N S <nssuresh@amazon.com>
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit e384326

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for e384326: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

nssuresh2007 and others added 5 commits May 12, 2026 15:49
Signed-off-by: Suresh N S <nssuresh@amazon.com>
Signed-off-by: Suresh N S <nssuresh@amazon.com>
Signed-off-by: Suresh N S <nssuresh@amazon.com>
Signed-off-by: Suresh N S <nssuresh@amazon.com>
Updated code to remove the assumption on the ordering of the elements
within the MAP structure

Signed-off-by: Suresh N S <nssuresh@amazon.com>
@nssuresh2007
nssuresh2007 requested a review from expani May 14, 2026 03:40
@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 6ab7900

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 6ab7900: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@expani expani left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sticking to good design patterns in the age of LLM coding :)

LGTM 🚀

@github-actions

Copy link
Copy Markdown
Contributor

Persistent review updated to latest commit 6ab7900

@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 6ab7900: SUCCESS

@bharath-techie bharath-techie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bharath-techie
bharath-techie merged commit 2733336 into opensearch-project:main May 14, 2026
28 of 30 checks passed
ahkcs added a commit to ahkcs/OpenSearch that referenced this pull request May 14, 2026
PR opensearch-project#21562 ("Add Support for Relevance functions") introduced a
reference to `FieldType.TEXT` at OpenSearchFilterRule.java:158 inside
the new FULL_TEXT branch of `resolveViableBackends`, but didn't add
the corresponding `import org.opensearch.analytics.spi.FieldType;`.
That breaks `:sandbox:plugins:analytics-engine:compileJava` on
upstream/main, blocking any downstream branch (including this one)
from compiling cleanly.

Fix: add the import. Single-line change with no functional impact.

Included in this PR purely to keep the branch buildable; should be
landed in a standalone follow-up PR if a maintainer prefers to keep
this PR scoped to SPAN only.

Signed-off-by: Kai Huang <huangkaics@gmail.com>
Signed-off-by: Kai Huang <ahkcs@amazon.com>
rishabhmaurya pushed a commit to rishabhmaurya/OpenSearch that referenced this pull request May 27, 2026
* Adding support for Relevance Functions

Signed-off-by: Suresh N S <nssuresh@amazon.com>

* Addressing comments

Signed-off-by: Suresh N S <nssuresh@amazon.com>

* Removing the unintentional checkin of md file

Signed-off-by: Suresh N S <nssuresh@amazon.com>

* Adding support for Relevance Functions

Signed-off-by: Suresh N S <nssuresh@amazon.com>

* Addressing comments

Signed-off-by: Suresh N S <nssuresh@amazon.com>

* Removing the unintentional checkin of md file

Signed-off-by: Suresh N S <nssuresh@amazon.com>

* Addressing comments from 2nd iteration

Signed-off-by: Suresh N S <nssuresh@amazon.com>

* Fixing a bug where MAP ordering can be random

Updated code to remove the assumption on the ordering of the elements
within the MAP structure

Signed-off-by: Suresh N S <nssuresh@amazon.com>

---------

Signed-off-by: Suresh N S <nssuresh@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants