Skip to content

Commit d481e0b

Browse files
authored
Bridge PPL_REX_MAX_MATCH_LIMIT into UnifiedQueryContext on the unified query path (#5418)
1 parent fde168f commit d481e0b

6 files changed

Lines changed: 68 additions & 14 deletions

File tree

api/src/main/java/org/opensearch/sql/api/UnifiedQueryContext.java

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

88
import static org.opensearch.sql.common.setting.Settings.Key.CALCITE_ENGINE_ENABLED;
99
import static org.opensearch.sql.common.setting.Settings.Key.PPL_JOIN_SUBSEARCH_MAXOUT;
10+
import static org.opensearch.sql.common.setting.Settings.Key.PPL_REX_MAX_MATCH_LIMIT;
1011
import static org.opensearch.sql.common.setting.Settings.Key.PPL_SUBSEARCH_MAXOUT;
1112
import static org.opensearch.sql.common.setting.Settings.Key.QUERY_SIZE_LIMIT;
1213

@@ -127,14 +128,24 @@ public static class Builder {
127128
* org.opensearch.sql.api.parser.PPLQueryParser} reuses the v2 {@code AstBuilder}, which gates
128129
* Calcite-only commands (e.g. {@code visitTableCommand}) on this setting; without the default,
129130
* those commands fail at parse time even when the cluster setting is true.
131+
*
132+
* <p>{@link Settings.Key#PPL_REX_MAX_MATCH_LIMIT} defaults to {@code 10} here because {@code
133+
* AstBuilder.visitRexCommand} reads it unconditionally and unboxes to {@code int} — a {@code
134+
* null} return from {@code getSettingValue} NPEs the planner before any operator-level
135+
* capability check runs. The value mirrors the cluster-side default of {@code 10} registered by
136+
* {@code OpenSearchSettings.PPL_REX_MAX_MATCH_LIMIT_SETTING}. Cluster-side overrides reach this
137+
* map via {@link #setting(String, Object)} — the REST handler reads the live value from {@code
138+
* OpenSearchSettings} and routes it through that existing API, keeping {@link
139+
* UnifiedQueryContext} decoupled from any specific {@link Settings} implementation.
130140
*/
131141
private final Map<Settings.Key, Object> settings =
132142
new HashMap<Settings.Key, Object>(
133143
Map.of(
134144
QUERY_SIZE_LIMIT, SysLimit.DEFAULT.querySizeLimit(),
135145
PPL_SUBSEARCH_MAXOUT, SysLimit.DEFAULT.subsearchLimit(),
136146
PPL_JOIN_SUBSEARCH_MAXOUT, SysLimit.DEFAULT.joinSubsearchLimit(),
137-
CALCITE_ENGINE_ENABLED, true));
147+
CALCITE_ENGINE_ENABLED, true,
148+
PPL_REX_MAX_MATCH_LIMIT, 10));
138149

139150
/**
140151
* Sets the query language frontend to be used.
@@ -223,6 +234,7 @@ public UnifiedQueryContext build() {
223234
case SQL -> UnifiedSqlSpec.extended();
224235
case PPL -> UnifiedPplSpec.create();
225236
};
237+
226238
Settings settings = buildSettings();
227239
CalcitePlanContext planContext =
228240
CalcitePlanContext.create(

api/src/test/java/org/opensearch/sql/api/UnifiedQueryContextTest.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ public void testContextCreationWithDefaults() {
3333
"Settings should have default system limits",
3434
SysLimit.DEFAULT,
3535
SysLimit.fromSettings(context.getSettings()));
36+
assertEquals(
37+
"PPL_REX_MAX_MATCH_LIMIT default should be 10",
38+
Integer.valueOf(10),
39+
context.getSettings().getSettingValue(PPL_REX_MAX_MATCH_LIMIT));
3640
}
3741

3842
@Test
@@ -43,10 +47,15 @@ public void testContextCreationWithCustomConfig() {
4347
.catalog("opensearch", testSchema)
4448
.cacheMetadata(true)
4549
.setting("plugins.query.size_limit", 200)
50+
.setting("plugins.ppl.rex.max_match.limit", 5)
4651
.build();
4752

4853
Integer querySizeLimit = context.getSettings().getSettingValue(QUERY_SIZE_LIMIT);
4954
assertEquals("Custom setting should be applied", Integer.valueOf(200), querySizeLimit);
55+
assertEquals(
56+
"Cluster-side override for PPL_REX_MAX_MATCH_LIMIT should reach the unified path",
57+
Integer.valueOf(5),
58+
context.getSettings().getSettingValue(PPL_REX_MAX_MATCH_LIMIT));
5059
}
5160

5261
@Test(expected = IllegalArgumentException.class)

plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,8 @@ private BiFunction<SQLQueryRequest, RestChannel, Boolean> createSqlAnalyticsRout
208208
if (executor == null) {
209209
return null;
210210
}
211-
cached[0] = new RestUnifiedQueryAction(client, clusterService, executor);
211+
cached[0] =
212+
new RestUnifiedQueryAction(client, clusterService, executor, pluginSettings);
212213
}
213214
return cached[0];
214215
};

plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,14 +59,17 @@ public class RestUnifiedQueryAction {
5959
private final AnalyticsExecutionEngine analyticsEngine;
6060
private final NodeClient client;
6161
private final ClusterService clusterService;
62+
private final org.opensearch.sql.common.setting.Settings pluginSettings;
6263

6364
public RestUnifiedQueryAction(
6465
NodeClient client,
6566
ClusterService clusterService,
66-
QueryPlanExecutor<RelNode, Iterable<Object[]>> planExecutor) {
67+
QueryPlanExecutor<RelNode, Iterable<Object[]>> planExecutor,
68+
org.opensearch.sql.common.setting.Settings pluginSettings) {
6769
this.client = client;
6870
this.clusterService = clusterService;
6971
this.analyticsEngine = new AnalyticsExecutionEngine(planExecutor);
72+
this.pluginSettings = pluginSettings;
7073
}
7174

7275
/**
@@ -154,19 +157,42 @@ public void explain(
154157
* Build a lightweight context for parsing only (index name extraction). Does not require cluster
155158
* state or catalog schema.
156159
*/
157-
private static UnifiedQueryContext buildParsingContext(QueryType queryType) {
158-
return UnifiedQueryContext.builder().language(queryType).build();
160+
private UnifiedQueryContext buildParsingContext(QueryType queryType) {
161+
return applyClusterOverrides(UnifiedQueryContext.builder().language(queryType)).build();
159162
}
160163

161164
private UnifiedQueryContext buildContext(QueryType queryType, boolean profiling) {
162-
return UnifiedQueryContext.builder()
163-
.language(queryType)
164-
.catalog(SCHEMA_NAME, OpenSearchSchemaBuilder.buildSchema(clusterService.state()))
165-
.defaultNamespace(SCHEMA_NAME)
166-
.profiling(profiling)
165+
return applyClusterOverrides(
166+
UnifiedQueryContext.builder()
167+
.language(queryType)
168+
.catalog(SCHEMA_NAME, OpenSearchSchemaBuilder.buildSchema(clusterService.state()))
169+
.defaultNamespace(SCHEMA_NAME)
170+
.profiling(profiling))
167171
.build();
168172
}
169173

174+
/**
175+
* Routes operator-configured cluster overrides into the builder via the existing {@code
176+
* setting(String, Object)} API, keeping {@link UnifiedQueryContext} decoupled from any specific
177+
* {@link org.opensearch.sql.common.setting.Settings} implementation.
178+
*
179+
* <p>Currently scoped to {@code plugins.ppl.rex.max_match.limit} — required so the unified path
180+
* honors {@code _cluster/settings} updates for {@code rex max_match} (CalciteRexCommandIT's
181+
* testRexMaxMatchConfigurableLimit). Add keys here if a future PR / IT depends on cluster-side
182+
* fidelity for one of the other planning settings.
183+
*/
184+
private UnifiedQueryContext.Builder applyClusterOverrides(UnifiedQueryContext.Builder builder) {
185+
Object rexLimit =
186+
pluginSettings.getSettingValue(
187+
org.opensearch.sql.common.setting.Settings.Key.PPL_REX_MAX_MATCH_LIMIT);
188+
if (rexLimit != null) {
189+
builder.setting(
190+
org.opensearch.sql.common.setting.Settings.Key.PPL_REX_MAX_MATCH_LIMIT.getKeyValue(),
191+
rexLimit);
192+
}
193+
return builder;
194+
}
195+
170196
/**
171197
* Extract the source index name by parsing the query and visiting the AST to find the Relation
172198
* node. Uses the context's parser which supports both PPL and SQL.

plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ public class TransportPPLQueryAction
6767

6868
private final NodeClient clientRef;
6969
private final ClusterService clusterServiceRef;
70+
private final org.opensearch.sql.common.setting.Settings pluginSettingsRef;
7071

7172
@Inject
7273
public TransportPPLQueryAction(
@@ -82,11 +83,13 @@ public TransportPPLQueryAction(
8283

8384
ModulesBuilder modules = new ModulesBuilder();
8485
modules.add(new OpenSearchPluginModule());
86+
org.opensearch.sql.common.setting.Settings pluginSettings =
87+
new OpenSearchSettings(clusterService.getClusterSettings());
88+
this.pluginSettingsRef = pluginSettings;
8589
modules.add(
8690
b -> {
8791
b.bind(NodeClient.class).toInstance(client);
88-
b.bind(org.opensearch.sql.common.setting.Settings.class)
89-
.toInstance(new OpenSearchSettings(clusterService.getClusterSettings()));
92+
b.bind(org.opensearch.sql.common.setting.Settings.class).toInstance(pluginSettings);
9093
b.bind(DataSourceService.class).toInstance(dataSourceService);
9194
});
9295
this.injector = Guice.createInjector(modules);
@@ -105,7 +108,8 @@ public void setQueryPlanExecutor(
105108
QueryPlanExecutor<RelNode, Iterable<Object[]>> queryPlanExecutor) {
106109
AnalyticsExecutorHolder.set(queryPlanExecutor);
107110
this.unifiedQueryHandler =
108-
new RestUnifiedQueryAction(clientRef, clusterServiceRef, queryPlanExecutor);
111+
new RestUnifiedQueryAction(
112+
clientRef, clusterServiceRef, queryPlanExecutor, pluginSettingsRef);
109113
}
110114

111115
/**

plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.junit.Test;
1515
import org.opensearch.analytics.exec.QueryPlanExecutor;
1616
import org.opensearch.cluster.service.ClusterService;
17+
import org.opensearch.sql.common.setting.Settings;
1718
import org.opensearch.sql.executor.QueryType;
1819
import org.opensearch.transport.client.node.NodeClient;
1920

@@ -30,7 +31,8 @@ public void setUp() {
3031
@SuppressWarnings("unchecked")
3132
QueryPlanExecutor<RelNode, Iterable<Object[]>> executor = mock(QueryPlanExecutor.class);
3233
action =
33-
new RestUnifiedQueryAction(mock(NodeClient.class), mock(ClusterService.class), executor);
34+
new RestUnifiedQueryAction(
35+
mock(NodeClient.class), mock(ClusterService.class), executor, mock(Settings.class));
3436
}
3537

3638
@Test

0 commit comments

Comments
 (0)