[Enhancement] Fail fast when relevance functions cannot be pushed down by RyanL1997 · Pull Request #5662 · opensearch-project/sql

RyanL1997 · 2026-07-29T18:40:23Z

Description

Relevance search functions (match, match_phrase, query_string, ...) are Calcite marker UDFs with no executable implementation — RelevanceQueryFunction.RelevanceQueryImplementor.implement unconditionally throws. They only work when the planner rewrites them into an OpenSearch query during push down.

Nothing guaranteed that rewrite happened. The two script fallbacks in PredicateAnalyzer would serialize whatever they could not analyze into a Calcite script — including a relevance call. That script was then shipped to every data node, where code generation hit the throwing implementor. The user got a 500 with all shards failing, for a query the coordinator could have rejected instantly.

Before

POST _plugins/_ppl
{"query": "source=logs | eval b2 = upper(body) | where match(b2, 'ERROR') | fields idx"}

The planner emits a script containing the relevance call:

PushDownContext=[[PROJECT->[idx, body],
                  SCRIPT->match(MAP('field', UPPER($1)), MAP('query', 'ERROR')), ...]]

and the shards reject it:

Failed to execute phase [query], all shards failed;
  shardFailures {[...][logs][0]:
    RemoteTransportException[...[indices:data/read/search[phase/query]]];
    nested: QueryShardException[failed to create query:
      Failed to compile inline script [{"langType":"calcite","script":"rO0ABXQF63sK…}]]

→ HTTP 500

Every data node does work that cannot possibly succeed, and the message says nothing about the actual problem.

A second shape failed at planning time, but with a message that names no field and offers no remedy:

source=logs | top 1 body | where match(body, 'ERROR')
source=logs | eval m = match(body, 'ERROR') | fields idx, m

→ "Relevance search query functions are only supported when they are pushed down"

After

Both shapes now fail on the coordinator, before any shard is contacted, with a message that names the offending column (real output from the new IT, for eval b2 = upper(body) | where match(b2, 'ERROR')):

{
  "error": {
    "details": "Relevance search function [match] cannot be applied to column [b2]. It must run against a field indexed by OpenSearch, and cannot be evaluated on a value the query computes (eval, parse, rex), on aggregated output (stats, top, rare), or after a row limit (head). Move the match filter so it directly follows `source=` and targets an indexed field, or use a non-relevance predicate such as `like` which can be evaluated per row.",
    "context": {
      "relevance_function": "match",
      "relevance_column": "b2"
    },
    "suggestion": "apply match directly to an indexed field before any command that transforms rows",
    "code": "UNSUPPORTED_OPERATION"
  },
  "status": 500
}

No Failed to compile inline script, no all shards failed, and the column is named as the user wrote it — b2 for eval, lvl for parse, body for top and for a relevance call in a projection.

Queries that legitimately push down are unaffected — including a relevance filter combined with a LIKE filter on a pure text field, where the LIKE still goes down as a script alongside a native match query:

source=logs | where match(body, 'ERROR') | where like(body, '%error%')

{"query":{"bool":{"must":[
  {"match":{"body":{"query":"ERROR", ...}}},
  {"script":{"script":{ ... ILIKE ... }}}
]}}}

Related Issues

Relates to #5491 — implements @penghuo's proposal 1 from that issue

Reject early, not at runtime. When the planner sees match / match_phrase against a non-pushdownable column (Calcite-derived, no underlying field), fail fast at planning time with a clear error message naming the column and suggesting alternatives. Don't generate a SCRIPT[match] clause that's guaranteed to fail server-side.

Document the keyword-field case in PPL relevance-function docs: match / match_phrase on a keyword field perform whole-value term matching, not substring/phrase. Common alternatives: use a text analyzer, query the .keyword subfield only with =/LIKE %X%, or restructure the data.

One deviation from proposal 2, found while verifying each case against a running cluster: a keyword subfield cannot be referenced directly in PPL. match(body.keyword, 'ERROR') fails with Field [body.keyword] not found, so "query the .keyword subfield" is not an available workaround. For a text field with a keyword subfield the engine already picks the right form automatically — match uses the analyzed text form, like uses the keyword subfield. The documented alternatives are therefore = for exact whole-value match, like for substring, and a text mapping for word or phrase matching.

Check List

New functionality includes testing.
New functionality has been documented.
New functionality has javadoc added.
Commits are signed per the DCO using --signoff or -s.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Relevance search functions (match, match_phrase, query_string, ...) are Calcite marker UDFs with no executable implementation -- they exist only to be rewritten into an OpenSearch query during push down. Nothing guaranteed that rewrite happened, and the two script fallbacks in PredicateAnalyzer would serialize whatever they could not analyze into a Calcite script, including a relevance call. That script was then shipped to every data node, where code generation hit RelevanceQueryImplementor and failed, surfacing to the user as: QueryShardException[failed to create query: Failed to compile inline script [...]] -> 500, all shards failed Guard both fallbacks so a node containing a relevance function is never serialized into a script, and report it on the coordinator instead. Also: - Reject a non-indexed field or non-literal query operand in visitRelevanceFunc as PredicateAnalyzerException rather than letting an unchecked ClassCastException escape to the top-level Throwable catch, which was one route into the script fallback. - Give the last-resort UnsupportedOperationException an actionable message naming the function and the constraint, instead of only "only supported when they are pushed down". - Hoist the relevance-function detection out of RelevanceFunctionPushdownRule into UserDefinedFunctionUtils so the analyzer and the rule share one definition. Queries that legitimately push down are unaffected; the reported combination of match() with a like() filter keeps working, with the LIKE going down as a script and the relevance call as a native match query. Signed-off-by: Jialiang Liang <ryanleeang@gmail.com>

github-actions · 2026-07-29T18:41:44Z

PR Reviewer Guide 🔍

(Review updated until commit `d91ad18`)

Here are some key observations to aid the review process:

🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ Recommended focus areas for review Possible Issue In `RelevanceUsageFinder.resolveColumnName`, if `relevanceCall.getOperands().isEmpty()` is true, the method returns null. However, the caller `enrichRelevanceNotPushedDown` then uses this null in a ternary that produces "the column it is applied to" as a fallback. This fallback message is vague when the actual problem is that the relevance call has no operands at all, which is a malformed call. A relevance function with zero operands should either be rejected earlier or reported with a message that says "malformed call" rather than implying the column is unknown. private String resolveColumnName(RexCall relevanceCall) { if (relevanceCall.getOperands().isEmpty()) { return null; } Possible Issue In `visitRelevanceFunc`, the code checks `!(fieldQueryOperands.get(0) instanceof NamedFieldExpression)` and throws a `PredicateAnalyzerException`. However, `visitList` is called on `AliasPair.from(ops.get(0), funcName).value` before this check. If `ops.get(0)` is null or `visitList` throws an exception, the check never runs. The code should validate that `ops` has at least two elements and that `ops.get(0)` and `ops.get(1)` are non-null before calling `visitList`, or handle exceptions from `visitList` that might obscure the real issue. List<Expression> fieldQueryOperands = visitList( List.of( AliasPair.from(ops.get(0), funcName).value, AliasPair.from(ops.get(1), funcName).value)); // The field operand must resolve to a real indexed field. A computed column (eval, parse, // rex, ...) resolves to something else -- reject it as a PredicateAnalyzerException so the // caller reports it at planning time instead of failing later on the shards. if (!(fieldQueryOperands.get(0) instanceof NamedFieldExpression)) { throw new PredicateAnalyzerException( format( Locale.ROOT, "Relevance search function [%s] requires an indexed field as its first argument," + " but got [%s]. Computed columns cannot be searched.", funcName, ops.get(0))); }

github-actions · 2026-07-29T18:42:04Z

PR Code Suggestions ✨

Latest suggestions up to d91ad18

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Prevent stack overflow in recursion The recursive traversal of `findRelevanceUsage` lacks depth protection, which could cause stack overflow on deeply nested plans. Add a maximum depth parameter to prevent unbounded recursion. core/src/main/java/org/opensearch/sql/calcite/utils/UserDefinedFunctionUtils.java [150-159] public static RelevanceUsage findRelevanceUsage(RelNode root) { - if (root == null) { + return findRelevanceUsage(root, 0, 100); +} + +private static RelevanceUsage findRelevanceUsage(RelNode root, int depth, int maxDepth) { + if (root == null \|\| depth > maxDepth) { return null; } for (RelNode input : root.getInputs()) { - RelevanceUsage fromInput = findRelevanceUsage(input); + RelevanceUsage fromInput = findRelevanceUsage(input, depth + 1, maxDepth); if (fromInput != null) { return fromInput; } } Suggestion importance[1-10]: 5 __ Why: The suggestion identifies a potential stack overflow risk in the recursive `findRelevanceUsage` method. While this is a valid concern for deeply nested plans, such plans are uncommon in practice, and this is error handling code. Adding depth protection would improve robustness but is not critical.	Low
General	Cache or limit plan traversal depth The `findRelevanceUsage` method traverses the entire plan tree recursively, which could be expensive for large plans. Consider caching the result or limiting the traversal depth to avoid performance degradation during error reporting. core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java [507-514] +RelevanceUsage usage = findRelevanceUsage(rel); +if (usage == null) { + return; +} +String target = + usage.columnName() == null + ? "the column it is applied to" + : String.format(Locale.ROOT, "column [%s]", usage.columnName()); - Suggestion importance[1-10]: 4 __ Why: The suggestion correctly identifies that `findRelevanceUsage` traverses the plan tree recursively, which could be expensive. However, this is error reporting code that only runs when a query fails, so performance is less critical. The suggestion is valid but has moderate impact.	Low

Previous suggestions

Suggestions up to commit 3bb6701

Category	Suggestion	Impact
General	Use case-insensitive string matching The method checks if `rootCause` contains "Relevance search function" but the actual exception message from `RelevanceQueryImplementor` uses lowercase "function" while the check is case-sensitive. This could cause the enrichment to be skipped if the message format changes slightly. Use case-insensitive matching or verify the exact message format. core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java [503-504] private static void enrichRelevanceNotPushedDown( ErrorReport.Builder report, SQLException e, RelNode rel) { String rootCause = rootCauseMessage(e); - if (rootCause == null \|\| !rootCause.contains("Relevance search function")) { + if (rootCause == null \|\| !rootCause.toLowerCase(Locale.ROOT).contains("relevance search function")) { return; } RelevanceUsage usage = findRelevanceUsage(rel); if (usage == null) { return; } String target = usage.columnName() == null ? "the column it is applied to" : String.format(Locale.ROOT, "column [%s]", usage.columnName()); report .details(...) .code(ErrorCode.UNSUPPORTED_OPERATION) .context("relevance_function", usage.functionName()) .context("relevance_column", String.valueOf(usage.columnName())) .suggestion(...); } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential fragility in the string matching logic. Making the check case-insensitive improves robustness against message format variations. However, since the current code works with the existing exception message format, this is a defensive improvement rather than a bug fix.	Medium
	Add negative index bounds check The method accesses `inputFieldNames.get(collector.index)` without verifying that `collector.index` is non-negative. If `collector.index` is negative, this will throw an `IndexOutOfBoundsException`. Add a bounds check to ensure the index is valid before accessing the list. core/src/main/java/org/opensearch/sql/calcite/utils/UserDefinedFunctionUtils.java [193-202] private String resolveColumnName(RexCall relevanceCall) { if (relevanceCall.getOperands().isEmpty()) { return null; } ColumnRefCollector collector = new ColumnRefCollector(); relevanceCall.getOperands().get(0).accept(collector); - if (collector.index == null) { + if (collector.index == null \|\| collector.index < 0) { return null; } return collector.index < inputFieldNames.size() ? inputFieldNames.get(collector.index) : null; } Suggestion importance[1-10]: 6 __ Why: The suggestion identifies a potential `IndexOutOfBoundsException` if `collector.index` is negative. While `RexInputRef.getIndex()` typically returns non-negative values, adding this defensive check improves code safety and prevents unexpected runtime errors.	Low
	Use transformed operand in error message The error message references `ops.get(0)` which is the original operand, but the validation is performed on `fieldQueryOperands.get(0)` which is the visited/transformed operand. If the transformation changes the representation, the error message may show misleading information. Consider using the transformed operand in the error message for consistency. opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java [454-462] if (!(fieldQueryOperands.get(0) instanceof NamedFieldExpression)) { throw new PredicateAnalyzerException( format( Locale.ROOT, "Relevance search function [%s] requires an indexed field as its first argument," + " but got [%s]. Computed columns cannot be searched.", funcName, - ops.get(0))); + fieldQueryOperands.get(0))); } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly notes an inconsistency between the validated operand (`fieldQueryOperands.get(0)`) and the one shown in the error message (`ops.get(0)`). Using the transformed operand would provide more accurate error reporting, though both representations are likely similar in most cases.	Low

Suggestions up to commit b55674c

Category	Suggestion	Impact
General	Use case-insensitive string matching The method checks for the string "Relevance search function" in the root cause message, but the actual exception message in `RelevanceQueryImplementor` uses a different format. This string matching may fail to detect relevance function errors, causing the enrichment logic to be skipped. core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java [503-504] private static void enrichRelevanceNotPushedDown( ErrorReport.Builder report, SQLException e, RelNode rel) { String rootCause = rootCauseMessage(e); - if (rootCause == null \|\| !rootCause.contains("Relevance search function")) { + if (rootCause == null \|\| !rootCause.toLowerCase(Locale.ROOT).contains("relevance search function")) { return; } RelevanceUsage usage = findRelevanceUsage(rel); if (usage == null) { return; } Suggestion importance[1-10]: 7 __ Why: The suggestion correctly identifies a potential issue where the string matching for "Relevance search function" should be case-insensitive to ensure the enrichment logic is triggered. The improved code uses `toLowerCase(Locale.ROOT)` which is consistent with the codebase style and makes the check more robust.	Medium
	Search all operands for column reference The method assumes the first operand contains the column reference, but relevance functions wrap their field argument in a MAP construct. This may fail to extract the actual column name, returning null when a valid column exists deeper in the expression tree. core/src/main/java/org/opensearch/sql/calcite/utils/UserDefinedFunctionUtils.java [193-203] private String resolveColumnName(RexCall relevanceCall) { if (relevanceCall.getOperands().isEmpty()) { return null; } ColumnRefCollector collector = new ColumnRefCollector(); - relevanceCall.getOperands().get(0).accept(collector); + for (RexNode operand : relevanceCall.getOperands()) { + operand.accept(collector); + if (collector.index != null) { + break; + } + } if (collector.index == null) { return null; } return collector.index < inputFieldNames.size() ? inputFieldNames.get(collector.index) : null; } Suggestion importance[1-10]: 6 __ Why: The suggestion addresses a potential issue where the column reference might not be in the first operand. However, the comment in the code states that relevance calls are shaped as `match(MAP('field', <expr>), ...)`, suggesting the first operand should contain the field. The suggestion adds robustness by searching all operands, which is a reasonable improvement.	Low
	Use consistent operand in error message The error message references `ops.get(0)` which is the raw RexNode, but the validation is performed on `fieldQueryOperands.get(0)` which is the visited/transformed expression. This inconsistency may produce confusing error messages showing different representations of the same operand. opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java [454-462] if (!(fieldQueryOperands.get(0) instanceof NamedFieldExpression)) { throw new PredicateAnalyzerException( format( Locale.ROOT, "Relevance search function [%s] requires an indexed field as its first argument," + " but got [%s]. Computed columns cannot be searched.", funcName, - ops.get(0))); + fieldQueryOperands.get(0))); } Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies an inconsistency where the error message uses `ops.get(0)` (raw RexNode) while the validation uses `fieldQueryOperands.get(0)` (transformed expression). Using `fieldQueryOperands.get(0)` in the error message would provide more consistent output, though the impact is relatively minor as both represent the same operand.	Low

Suggestions up to commit 981dfb1

Category	Suggestion	Impact
General	Use checked expression in error message The error message uses `ops.get(0)` which is the original RexNode, but the check is on `fieldQueryOperands.get(0)` which is the visited/transformed expression. For consistency and clarity, the error message should reference what was actually checked. opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java [454-461] if (!(fieldQueryOperands.get(0) instanceof NamedFieldExpression)) { throw new PredicateAnalyzerException( format( Locale.ROOT, "Relevance search function [%s] requires an indexed field as its first argument," + " but got [%s]. Computed columns cannot be searched.", funcName, - ops.get(0))); + fieldQueryOperands.get(0))); } Suggestion importance[1-10]: 6 __ Why: The suggestion correctly identifies an inconsistency where `ops.get(0)` is used in the error message while `fieldQueryOperands.get(0)` is checked. Using `fieldQueryOperands.get(0)` in the error message would be more consistent with what was actually validated, improving clarity for debugging.	Low

Follows the "reject early, not at runtime" proposal in opensearch-project#5491, which asks for a planning-time failure whose message names the column. The previous commit stopped the shard-side crash, but the message the user actually saw still came from RelevanceQueryImplementor and carried only positional references: Expression: match($t4, $t7) The message built in PredicateAnalyzer never reached the user at all, because CalciteLogicalIndexScan.pushDownFilter swallows the exception and returns null, leaving the filter in the plan for code generation to reject. Resolve the column against the pre-compilation plan in the planning error path instead, where the row type is still available, so the report names the column as the user wrote it: Relevance search function [match] cannot be applied to column [b2]. It must run against a field indexed by OpenSearch, and cannot be evaluated on a value the query computes (eval, parse, rex), on aggregated output (stats, top, rare), or after a row limit (head). Move the match filter so it directly follows `source=` and targets an indexed field, or use a non-relevance predicate such as `like` which can be evaluated per row. The error is also now coded UNSUPPORTED_OPERATION with the function and column exposed as context, and carries a suggestion. Extends the IT to assert the column name is present for each shape: b2 (eval), lvl (parse), body (top, and relevance in a projection). Signed-off-by: Jialiang Liang <ryanleeang@gmail.com>

github-actions · 2026-07-29T19:06:44Z

Persistent review updated to latest commit b55674c

Adds a "Usage constraints" section to the relevance function docs explaining that these functions are executed by the OpenSearch search engine against the inverted index built at write time, so they must be applied to an indexed field before any command that transforms rows. Each unsupported pattern is listed with the reason it cannot work, and the planning-time error the user will see. Every claim was verified against a running cluster rather than inferred: - stats on the grouping key followed by a relevance filter DOES work (the filter transposes below the aggregation), so it is documented as supported rather than listed as a limitation. - sort + head followed by a relevance filter fails at planning time. - top / rare followed by a relevance filter fails at planning time. Also documents the known issue where a relevance filter placed directly after `head` silently returns rows drawn from the whole index instead of from the intended sample, since that reordering is not yet guarded. Signed-off-by: Jialiang Liang <ryanleeang@gmail.com>

github-actions · 2026-07-29T19:09:44Z

Persistent review updated to latest commit 3bb6701

Implements proposal 2 from opensearch-project#5491: explain that match / match_phrase on a keyword field perform whole-value term matching rather than substring or phrase matching, which is the most common reason a relevance query returns zero results without an error. Every case in the new section was verified against a running cluster. That check corrected the alternatives suggested in the original proposal: "query the .keyword subfield only with =/LIKE %X%" does not work in PPL, because a keyword subfield cannot be referenced directly -- match(body.keyword, 'ERROR') fails with "Field [body.keyword] not found". For a text field with a keyword subfield the engine already selects the right form automatically (match uses the analyzed text form, like uses the keyword subfield), so the documented alternatives are = for exact whole-value match, like for substring, and a text mapping for word or phrase matching. Signed-off-by: Jialiang Liang <ryanleeang@gmail.com>

github-actions · 2026-07-29T19:21:04Z

Persistent review updated to latest commit d91ad18

RyanL1997 changed the title ~~Fail fast when relevance functions cannot be pushed down~~ [Enhancement] Fail fast when relevance functions cannot be pushed down Jul 29, 2026

RyanL1997 added enhancement New feature or request PPL Piped processing language labels Jul 29, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Enhancement] Fail fast when relevance functions cannot be pushed down - #5662

[Enhancement] Fail fast when relevance functions cannot be pushed down#5662
RyanL1997 wants to merge 4 commits into
opensearch-project:mainfrom
RyanL1997:fix/relevance-function-pushdown-failfast

RyanL1997 commented Jul 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 29, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 29, 2026

Uh oh!

github-actions Bot commented Jul 29, 2026

Uh oh!

github-actions Bot commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

RyanL1997 commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Before

After

Related Issues

Check List

Uh oh!

github-actions Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Reviewer Guide 🔍

(Review updated until commit d91ad18)

Uh oh!

github-actions Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Code Suggestions ✨

Previous suggestions

Uh oh!

github-actions Bot commented Jul 29, 2026

Uh oh!

github-actions Bot commented Jul 29, 2026

Uh oh!

github-actions Bot commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

RyanL1997 commented Jul 29, 2026 •

edited

Loading

github-actions Bot commented Jul 29, 2026 •

edited

Loading

(Review updated until commit `d91ad18`)

github-actions Bot commented Jul 29, 2026 •

edited

Loading