Skip to content

Commit 981dfb1

Browse files
committed
Fail fast when relevance functions cannot be pushed down
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>
1 parent a54acb4 commit 981dfb1

5 files changed

Lines changed: 283 additions & 48 deletions

File tree

‎core/src/main/java/org/opensearch/sql/calcite/utils/UserDefinedFunctionUtils.java‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import java.util.ArrayList;
1818
import java.util.Collections;
1919
import java.util.List;
20+
import java.util.Locale;
2021
import java.util.Objects;
2122
import java.util.Set;
2223
import javax.annotation.Nullable;
@@ -29,6 +30,7 @@
2930
import org.apache.calcite.rel.type.RelDataType;
3031
import org.apache.calcite.rex.RexCall;
3132
import org.apache.calcite.rex.RexNode;
33+
import org.apache.calcite.rex.RexVisitorImpl;
3234
import org.apache.calcite.schema.impl.AggregateFunctionImpl;
3335
import org.apache.calcite.sql.SqlAggFunction;
3436
import org.apache.calcite.sql.SqlIdentifier;
@@ -81,6 +83,76 @@ public class UserDefinedFunctionUtils {
8183
ImmutableSet.of("simple_query_string", "query_string", "multi_match");
8284
public static String IP_FUNCTION_NAME = "IP";
8385

86+
/** Returns true if the given operator name is a relevance search query function. */
87+
public static boolean isRelevanceFunction(String operatorName) {
88+
String lowerCased = operatorName.toLowerCase(Locale.ROOT);
89+
return SINGLE_FIELD_RELEVANCE_FUNCTION_SET.contains(lowerCased)
90+
|| MULTI_FIELDS_RELEVANCE_FUNCTION_SET.contains(lowerCased);
91+
}
92+
93+
/**
94+
* Checks whether a {@link RexNode} tree contains a relevance search query function such as {@code
95+
* match} or {@code query_string}.
96+
*
97+
* <p>Relevance functions have no executable implementation: they exist only as markers to be
98+
* rewritten into an OpenSearch query during push down (see {@code RelevanceQueryFunction}).
99+
* Callers must use this to avoid handing such a node to any execution path that would try to
100+
* generate code for it -- notably script push down, where the failure would surface as a shard
101+
* side "Failed to compile inline script" error instead of a local planning error.
102+
*
103+
* @param node the node to inspect
104+
* @return true if the tree contains a relevance function
105+
*/
106+
public static boolean containsRelevanceFunction(RexNode node) {
107+
RelevanceFunctionFinder finder = new RelevanceFunctionFinder();
108+
node.accept(finder);
109+
return finder.found;
110+
}
111+
112+
/** Visitor that detects relevance functions anywhere in a {@link RexNode} tree. */
113+
private static class RelevanceFunctionFinder extends RexVisitorImpl<Void> {
114+
private boolean found = false;
115+
116+
RelevanceFunctionFinder() {
117+
super(true);
118+
}
119+
120+
@Override
121+
public Void visitCall(RexCall call) {
122+
if (isRelevanceFunction(call.getOperator().getName())) {
123+
found = true;
124+
return null; // stop descending once found
125+
}
126+
return super.visitCall(call);
127+
}
128+
}
129+
130+
/**
131+
* Returns the name of the first relevance function found in the tree, or null if there is none.
132+
*/
133+
public static String findRelevanceFunctionName(RexNode node) {
134+
RelevanceFunctionNameFinder finder = new RelevanceFunctionNameFinder();
135+
node.accept(finder);
136+
return finder.name;
137+
}
138+
139+
private static class RelevanceFunctionNameFinder extends RexVisitorImpl<Void> {
140+
private String name = null;
141+
142+
RelevanceFunctionNameFinder() {
143+
super(true);
144+
}
145+
146+
@Override
147+
public Void visitCall(RexCall call) {
148+
if (name == null && isRelevanceFunction(call.getOperator().getName())) {
149+
name = call.getOperator().getName().toLowerCase(Locale.ROOT);
150+
return null;
151+
}
152+
return super.visitCall(call);
153+
}
154+
}
155+
84156
/**
85157
* Creates a SqlUserDefinedAggFunction that wraps a Java class implementing an aggregate function.
86158
*

‎core/src/main/java/org/opensearch/sql/expression/function/udf/RelevanceQueryFunction.java‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import com.google.common.collect.ImmutableList;
99
import java.util.List;
10+
import java.util.Locale;
1011
import org.apache.calcite.adapter.enumerable.NotNullImplementor;
1112
import org.apache.calcite.adapter.enumerable.NullPolicy;
1213
import org.apache.calcite.adapter.enumerable.RexToLixTranslator;
@@ -94,8 +95,17 @@ public static class RelevanceQueryImplementor implements NotNullImplementor {
9495
@Override
9596
public Expression implement(
9697
RexToLixTranslator translator, RexCall call, List<Expression> translatedOperands) {
98+
// Reaching code generation means the call was not rewritten into an OpenSearch query. There
99+
// is no row-by-row implementation to fall back to, so report why rather than just that.
97100
throw new UnsupportedOperationException(
98-
"Relevance search query functions are only supported when they are pushed down");
101+
String.format(
102+
Locale.ROOT,
103+
"Relevance search function [%s] could not be pushed down to OpenSearch, and it has no"
104+
+ " other execution path. Apply it directly to an indexed field before any"
105+
+ " command that transforms rows (eval, parse, rex, stats, top, rare, sort, head,"
106+
+ " lookup), and not to a column computed by the query itself. Expression: %s",
107+
call.getOperator().getName().toLowerCase(Locale.ROOT),
108+
call));
99109
}
100110
}
101111
}
Lines changed: 150 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,150 @@
1+
/*
2+
* Copyright OpenSearch Contributors
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
6+
package org.opensearch.sql.calcite.remote;
7+
8+
import static org.opensearch.sql.util.MatcherUtils.rows;
9+
import static org.opensearch.sql.util.MatcherUtils.verifyDataRows;
10+
import static org.opensearch.sql.util.TestUtils.createIndexByRestClient;
11+
import static org.opensearch.sql.util.TestUtils.getResponseBody;
12+
import static org.opensearch.sql.util.TestUtils.isIndexExist;
13+
import static org.opensearch.sql.util.TestUtils.performRequest;
14+
15+
import java.io.IOException;
16+
import java.util.Locale;
17+
import org.json.JSONObject;
18+
import org.junit.Test;
19+
import org.opensearch.client.Request;
20+
import org.opensearch.client.ResponseException;
21+
import org.opensearch.sql.ppl.PPLIntegTestCase;
22+
23+
/**
24+
* Tests that relevance search functions which cannot be pushed down fail fast on the coordinator
25+
* with an actionable message, instead of being serialized into a script that only fails when
26+
* compiled on the shards.
27+
*
28+
* <p>Relevance functions have no row-by-row implementation -- they exist only to be rewritten into
29+
* an OpenSearch query during push down. Before this fix, a query the analyzer could not handle was
30+
* wrapped in a script and shipped to every data node, surfacing as {@code QueryShardException:
31+
* Failed to compile inline script} with all shards failing.
32+
*/
33+
public class CalciteRelevanceFunctionPushdownFailureIT extends PPLIntegTestCase {
34+
35+
private static final String TEST_INDEX = "relevance_pushdown_failure";
36+
37+
@Override
38+
public void init() throws Exception {
39+
super.init();
40+
enableCalcite();
41+
createTestIndex();
42+
}
43+
44+
private void createTestIndex() throws IOException {
45+
if (isIndexExist(client(), TEST_INDEX)) {
46+
return;
47+
}
48+
createIndexByRestClient(
49+
client(),
50+
TEST_INDEX,
51+
"{\"mappings\":{\"properties\":{\"body\":{\"type\":\"text\"},"
52+
+ "\"idx\":{\"type\":\"integer\"}}}}");
53+
Request bulk = new Request("POST", "/" + TEST_INDEX + "/_bulk?refresh=true");
54+
bulk.setJsonEntity(
55+
"{\"index\":{\"_id\":\"1\"}}\n"
56+
+ "{\"body\":\"ERROR something bad happened\",\"idx\":1}\n"
57+
+ "{\"index\":{\"_id\":\"2\"}}\n"
58+
+ "{\"body\":\"INFO all good\",\"idx\":2}\n");
59+
performRequest(client(), bulk);
60+
}
61+
62+
private String errorOf(String query) throws IOException {
63+
ResponseException e =
64+
assertThrows(ResponseException.class, () -> executeQuery(query));
65+
return getResponseBody(e.getResponse());
66+
}
67+
68+
private void assertFailsCleanly(String query, String expectedFunctionName)
69+
throws IOException {
70+
String message = errorOf(query);
71+
assertFalse(
72+
"Relevance function must not be pushed down as a script; it cannot compile on the shard."
73+
+ " Error was: "
74+
+ message,
75+
message.contains("Failed to compile inline script"));
76+
assertFalse(
77+
"Query must fail on the coordinator, not as a shard failure. Error was: " + message,
78+
message.contains("all shards failed"));
79+
assertTrue(
80+
String.format(
81+
Locale.ROOT, "Error should name the function [%s]. Error was: %s", "match", message),
82+
message.toLowerCase(Locale.ROOT).contains(expectedFunctionName));
83+
}
84+
85+
/** A relevance function over a column computed by the query cannot be pushed down. */
86+
@Test
87+
public void relevanceOnEvalDerivedColumnFailsCleanly() throws IOException {
88+
assertFailsCleanly(
89+
String.format(
90+
Locale.ROOT,
91+
"source=%s | eval b2 = upper(body) | where match(b2, 'ERROR') | fields idx",
92+
TEST_INDEX),
93+
"match");
94+
}
95+
96+
/** Same for a column produced by parse. */
97+
@Test
98+
public void relevanceOnParseDerivedColumnFailsCleanly() throws IOException {
99+
assertFailsCleanly(
100+
String.format(
101+
Locale.ROOT,
102+
"source=%s | parse body '(?<lvl>\\\\w+)' | where match(lvl, 'ERROR') | fields idx",
103+
TEST_INDEX),
104+
"match");
105+
}
106+
107+
/** A relevance filter above an aggregation has no scan to be pushed onto. */
108+
@Test
109+
public void relevanceAboveAggregationFailsCleanly() throws IOException {
110+
assertFailsCleanly(
111+
String.format(Locale.ROOT, "source=%s | top 1 body | where match(body, 'ERROR')", TEST_INDEX),
112+
"match");
113+
}
114+
115+
/** A relevance function in a projection is never rewritten into a query. */
116+
@Test
117+
public void relevanceInProjectionFailsCleanly() throws IOException {
118+
assertFailsCleanly(
119+
String.format(
120+
Locale.ROOT, "source=%s | eval m = match(body, 'ERROR') | fields idx, m", TEST_INDEX),
121+
"match");
122+
}
123+
124+
/**
125+
* Regression guard: the pattern from the customer report -- a relevance filter combined with a
126+
* LIKE filter on a pure text field -- must keep working. The LIKE goes down as a script while the
127+
* relevance function goes down as a native match query.
128+
*/
129+
@Test
130+
public void relevanceCombinedWithLikeStillWorks() throws IOException {
131+
JSONObject result =
132+
executeQuery(
133+
String.format(
134+
Locale.ROOT,
135+
"source=%s | where match(body, 'ERROR') | where like(body, '%%error%%') | fields"
136+
+ " idx",
137+
TEST_INDEX));
138+
verifyDataRows(result, rows(1));
139+
}
140+
141+
/** Regression guard: a plain relevance filter on an indexed field is unaffected. */
142+
@Test
143+
public void relevanceOnIndexedFieldStillWorks() throws IOException {
144+
JSONObject result =
145+
executeQuery(
146+
String.format(
147+
Locale.ROOT, "source=%s | where match(body, 'ERROR') | fields idx", TEST_INDEX));
148+
verifyDataRows(result, rows(1));
149+
}
150+
}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/RelevanceFunctionPushdownRule.java‎

Lines changed: 1 addition & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,13 @@
55

66
package org.opensearch.sql.opensearch.planner.rules;
77

8-
import static org.opensearch.sql.calcite.utils.UserDefinedFunctionUtils.MULTI_FIELDS_RELEVANCE_FUNCTION_SET;
9-
import static org.opensearch.sql.calcite.utils.UserDefinedFunctionUtils.SINGLE_FIELD_RELEVANCE_FUNCTION_SET;
8+
import static org.opensearch.sql.calcite.utils.UserDefinedFunctionUtils.containsRelevanceFunction;
109

1110
import org.apache.calcite.plan.RelOptRuleCall;
1211
import org.apache.calcite.rel.AbstractRelNode;
1312
import org.apache.calcite.rel.core.Filter;
1413
import org.apache.calcite.rel.logical.LogicalFilter;
1514
import org.apache.calcite.rel.rules.SubstitutionRule;
16-
import org.apache.calcite.rex.RexCall;
17-
import org.apache.calcite.rex.RexNode;
18-
import org.apache.calcite.rex.RexVisitorImpl;
19-
import org.apache.calcite.sql.SqlOperator;
2015
import org.immutables.value.Value;
2116
import org.opensearch.sql.calcite.plan.rule.OpenSearchRuleConfig;
2217
import org.opensearch.sql.calcite.utils.PlanUtils;
@@ -61,47 +56,6 @@ protected void apply(RelOptRuleCall call, Filter filter, CalciteLogicalIndexScan
6156
}
6257
}
6358

64-
/**
65-
* Checks if a RexNode contains any relevance functions.
66-
*
67-
* @param node The RexNode to check
68-
* @return true if the node contains relevance functions, false otherwise
69-
*/
70-
private boolean containsRelevanceFunction(RexNode node) {
71-
RelevanceFunctionVisitor visitor = new RelevanceFunctionVisitor();
72-
node.accept(visitor);
73-
return visitor.hasRelevanceFunction();
74-
}
75-
76-
/** Visitor to detect relevance functions in a RexNode tree. */
77-
private static class RelevanceFunctionVisitor extends RexVisitorImpl<Void> {
78-
private boolean foundRelevanceFunction = false;
79-
80-
RelevanceFunctionVisitor() {
81-
super(true);
82-
}
83-
84-
@Override
85-
public Void visitCall(RexCall call) {
86-
SqlOperator operator = call.getOperator();
87-
String operatorName = operator.getName().toLowerCase();
88-
89-
// Check if this is a relevance function
90-
if (SINGLE_FIELD_RELEVANCE_FUNCTION_SET.contains(operatorName)
91-
|| MULTI_FIELDS_RELEVANCE_FUNCTION_SET.contains(operatorName)) {
92-
foundRelevanceFunction = true;
93-
return null; // Stop traversing once we find a relevance function
94-
}
95-
96-
// Continue traversing the tree
97-
return super.visitCall(call);
98-
}
99-
100-
boolean hasRelevanceFunction() {
101-
return foundRelevanceFunction;
102-
}
103-
}
104-
10559
/** Rule configuration. */
10660
@Value.Immutable
10761
public interface Config extends OpenSearchRuleConfig {

0 commit comments

Comments
 (0)