Skip to content

Commit 1cf9dcd

Browse files
committed
Re-base PPL coverage bundle wiring onto main
Hand-port the analytics-engine compatibility test toggles from the original ahkcs/feature/ppl-coverage-bundle branch (which was based on the now-merged feature/mustang-ppl-integration). Most of the bundle's commits were squash-merged into main via opensearch-project#5430; what remains are the test-infra toggles for force-routing every IT through the analytics-engine path with parquet-backed indices. Changes: - Add CALCITE_ANALYTICS_FORCE_ROUTING cluster setting key in common Settings + register it in OpenSearchSettings. - RestUnifiedQueryAction.isAnalyticsIndex() short-circuits to true when the setting is on (the test-only knob; default false). - PPLIntegTestCase.init() reads -Dtests.analytics.force_routing and flips the cluster setting on at setup. Provides static helpers enable/disableAnalyticsForceRouting(). - TestUtils.createIndexByRestClient honors -Dtests.analytics.parquet_indices by switching every test-created index to a single-shard composite/parquet store. Bulk loads on those indices use refresh=true (workaround for LuceneCommitter.getSafeCommitInfo TODO that hangs refresh=wait_for). Run via: ./gradlew :integ-test:integTestRemote \ -Dtests.rest.cluster=localhost:9200 \ -Dtests.cluster=localhost:9300 \ -Dtests.clustername=runTask \ -Dtests.analytics.force_routing=true \ -Dtests.analytics.parquet_indices=true \ --tests "<class>" Signed-off-by: Kai Huang <huangkaics@gmail.com>
1 parent f1f39ef commit 1cf9dcd

5 files changed

Lines changed: 101 additions & 2 deletions

File tree

common/src/main/java/org/opensearch/sql/common/setting/Settings.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,15 @@ public enum Key {
4545
"plugins.calcite.pushdown.rowcount.estimation.factor"),
4646
CALCITE_SUPPORT_ALL_JOIN_TYPES("plugins.calcite.all_join_types.allowed"),
4747

48+
/**
49+
* Force-route every PPL/SQL query through the analytics engine path, regardless of the index
50+
* name pattern. Test-only knob used by the analytics compatibility report; should remain {@code
51+
* false} in production. When {@code true}, {@link
52+
* org.opensearch.sql.plugin.rest.RestUnifiedQueryAction#isAnalyticsIndex} returns {@code true}
53+
* unconditionally.
54+
*/
55+
CALCITE_ANALYTICS_FORCE_ROUTING("plugins.calcite.analytics.force_routing"),
56+
4857
/** Query Settings. */
4958
FIELD_TYPE_TOLERANCE("plugins.query.field_type_tolerance"),
5059

integ-test/src/test/java/org/opensearch/sql/legacy/TestUtils.java

Lines changed: 46 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,16 @@ public class TestUtils {
3737

3838
private static final String MAPPING_FILE_PATH = "src/test/resources/indexDefinitions/";
3939

40+
/**
41+
* System property that makes every test-created index parquet-backed (composite primary data
42+
* format = parquet) with a single shard. Set by the analytics-engine compatibility test path so
43+
* {@link org.opensearch.sql.ppl.PPLIntegTestCase}'s force-routing flag (which routes every query
44+
* through the analytics-engine planner) can actually find a backend that supports the underlying
45+
* storage. Without parquet, the analytics planner fails with "No backend can scan all requested
46+
* fields..." regardless of the query.
47+
*/
48+
public static final String ANALYTICS_PARQUET_INDICES_PROP = "tests.analytics.parquet_indices";
49+
4050
/**
4151
* Create test index by REST client.
4252
*
@@ -48,6 +58,9 @@ public static void createIndexByRestClient(RestClient client, String indexName,
4858
Request request = new Request("PUT", "/" + indexName);
4959
JSONObject jsonObject = isNullOrEmpty(mapping) ? new JSONObject() : new JSONObject(mapping);
5060
setZeroReplicas(jsonObject);
61+
if (Boolean.parseBoolean(System.getProperty(ANALYTICS_PARQUET_INDICES_PROP, "false"))) {
62+
makeParquetBacked(jsonObject);
63+
}
5164
request.setJsonEntity(jsonObject.toString());
5265
performRequest(client, request);
5366
}
@@ -69,6 +82,28 @@ private static void setZeroReplicas(JSONObject jsonObject) {
6982
jsonObject.put("settings", settings);
7083
}
7184

85+
/**
86+
* Switches the test index to a parquet-backed composite store with a single shard so the
87+
* analytics-engine path has a backend that can scan it. Mirrors the per-IT setup pattern in
88+
* {@code
89+
* sandbox/plugins/analytics-backend-datafusion/src/internalClusterTest/.../CoordinatorReduceIT.java}
90+
* but applied at the REST-test layer. No-op for tests that go through the legacy / Calcite path
91+
* against Lucene indices.
92+
*/
93+
private static void makeParquetBacked(JSONObject jsonObject) {
94+
JSONObject settings =
95+
jsonObject.has("settings") ? jsonObject.getJSONObject("settings") : new JSONObject();
96+
JSONObject indexSettings =
97+
settings.has("index") ? settings.getJSONObject("index") : new JSONObject();
98+
indexSettings.put("number_of_shards", 1);
99+
indexSettings.put("pluggable.dataformat.enabled", true);
100+
indexSettings.put("pluggable.dataformat", "composite");
101+
indexSettings.put("composite.primary_data_format", "parquet");
102+
indexSettings.put("composite.secondary_data_formats", new org.json.JSONArray());
103+
settings.put("index", indexSettings);
104+
jsonObject.put("settings", settings);
105+
}
106+
72107
/**
73108
* https://github.com/elastic/elasticsearch/pull/49959<br>
74109
* Deprecate creation of dot-prefixed index names except for hidden and system indices. Create
@@ -116,8 +151,17 @@ public static boolean isIndexExist(RestClient client, String indexName) {
116151
public static void loadDataByRestClient(
117152
RestClient client, String indexName, String dataSetFilePath) throws IOException {
118153
Path path = Paths.get(getResourceFilePath(dataSetFilePath));
119-
Request request =
120-
new Request("POST", "/" + indexName + "/_bulk?refresh=wait_for&wait_for_active_shards=all");
154+
// Workaround: parquet-backed indices in the analytics-backend-lucene composite engine
155+
// do not yet implement LuceneCommitter.getSafeCommitInfo (UnsupportedOperationException
156+
// "TODO:: with index deleter"), which hangs refresh=wait_for until the test framework
157+
// request timeout (~60s). Force-refresh sidesteps the safe-commit-info path while still
158+
// making the bulk-loaded docs immediately searchable. Drop this branch once
159+
// LuceneCommitter.getSafeCommitInfo is implemented.
160+
String refreshParam =
161+
Boolean.parseBoolean(System.getProperty(ANALYTICS_PARQUET_INDICES_PROP, "false"))
162+
? "refresh=true"
163+
: "refresh=wait_for&wait_for_active_shards=all";
164+
Request request = new Request("POST", "/" + indexName + "/_bulk?" + refreshParam);
121165
request.setJsonEntity(new String(Files.readAllBytes(path)));
122166
performRequest(client, request);
123167
}

integ-test/src/test/java/org/opensearch/sql/ppl/PPLIntegTestCase.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,38 @@ public abstract class PPLIntegTestCase extends SQLIntegTestCase {
4242
public static final Integer DEFAULT_SUBSEARCH_MAXOUT = 10000;
4343
public static final Integer DEFAULT_JOIN_SUBSEARCH_MAXOUT = 50000;
4444

45+
/**
46+
* System property toggle for the analytics compatibility report. When set, every PPL IT base
47+
* setup forces the analytics routing flag on so all queries traverse the analytics-engine path.
48+
* Off by default — normal CI runs untouched.
49+
*/
50+
public static final String ANALYTICS_FORCE_ROUTING_PROP = "tests.analytics.force_routing";
51+
4552
@Override
4653
protected void init() throws Exception {
4754
super.init();
4855
updatePushdownSettings();
4956
disableCalcite(); // calcite is enabled by default from 3.3.0
57+
if (Boolean.parseBoolean(System.getProperty(ANALYTICS_FORCE_ROUTING_PROP, "false"))) {
58+
enableCalcite();
59+
enableAnalyticsForceRouting();
60+
}
61+
}
62+
63+
/**
64+
* Force every PPL/SQL query to route through the analytics-engine path regardless of the index
65+
* name pattern. Used by the analytics compatibility report.
66+
*/
67+
public static void enableAnalyticsForceRouting() throws IOException {
68+
updateClusterSettings(
69+
new SQLIntegTestCase.ClusterSetting(
70+
"persistent", Settings.Key.CALCITE_ANALYTICS_FORCE_ROUTING.getKeyValue(), "true"));
71+
}
72+
73+
public static void disableAnalyticsForceRouting() throws IOException {
74+
updateClusterSettings(
75+
new SQLIntegTestCase.ClusterSetting(
76+
"persistent", Settings.Key.CALCITE_ANALYTICS_FORCE_ROUTING.getKeyValue(), "false"));
5077
}
5178

5279
protected JSONObject executeQuery(String query) throws IOException {

opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,13 @@ public class OpenSearchSettings extends Settings {
186186
Setting.Property.NodeScope,
187187
Setting.Property.Dynamic);
188188

189+
public static final Setting<Boolean> CALCITE_ANALYTICS_FORCE_ROUTING_SETTING =
190+
Setting.boolSetting(
191+
Key.CALCITE_ANALYTICS_FORCE_ROUTING.getKeyValue(),
192+
false,
193+
Setting.Property.NodeScope,
194+
Setting.Property.Dynamic);
195+
189196
public static final Setting<?> QUERY_MEMORY_LIMIT_SETTING =
190197
Setting.memorySizeSetting(
191198
Key.QUERY_MEMORY_LIMIT.getKeyValue(),
@@ -467,6 +474,12 @@ public OpenSearchSettings(ClusterSettings clusterSettings) {
467474
Key.CALCITE_SUPPORT_ALL_JOIN_TYPES,
468475
CALCITE_SUPPORT_ALL_JOIN_TYPES_SETTING,
469476
new Updater(Key.CALCITE_SUPPORT_ALL_JOIN_TYPES));
477+
register(
478+
settingBuilder,
479+
clusterSettings,
480+
Key.CALCITE_ANALYTICS_FORCE_ROUTING,
481+
CALCITE_ANALYTICS_FORCE_ROUTING_SETTING,
482+
new Updater(Key.CALCITE_ANALYTICS_FORCE_ROUTING));
470483
register(
471484
settingBuilder,
472485
clusterSettings,
@@ -658,6 +671,7 @@ public static List<Setting<?>> pluginSettings() {
658671
.add(CALCITE_PUSHDOWN_ENABLED_SETTING)
659672
.add(CALCITE_PUSHDOWN_ROWCOUNT_ESTIMATION_FACTOR_SETTING)
660673
.add(CALCITE_SUPPORT_ALL_JOIN_TYPES_SETTING)
674+
.add(CALCITE_ANALYTICS_FORCE_ROUTING_SETTING)
661675
.add(DEFAULT_PATTERN_METHOD_SETTING)
662676
.add(DEFAULT_PATTERN_MODE_SETTING)
663677
.add(DEFAULT_PATTERN_MAX_SAMPLE_COUNT_SETTING)

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.opensearch.sql.calcite.CalcitePlanContext;
3838
import org.opensearch.sql.calcite.plan.rel.LogicalSystemLimit;
3939
import org.opensearch.sql.common.response.ResponseListener;
40+
import org.opensearch.sql.common.setting.Settings;
4041
import org.opensearch.sql.executor.ExecutionEngine.QueryResponse;
4142
import org.opensearch.sql.executor.QueryType;
4243
import org.opensearch.sql.executor.analytics.AnalyticsExecutionEngine;
@@ -88,6 +89,10 @@ public boolean isAnalyticsIndex(String query, QueryType queryType) {
8889
if (query == null || query.isEmpty()) {
8990
return false;
9091
}
92+
if (Boolean.TRUE.equals(
93+
pluginSettings.getSettingValue(Settings.Key.CALCITE_ANALYTICS_FORCE_ROUTING))) {
94+
return true;
95+
}
9196
try (UnifiedQueryContext context = buildParsingContext(queryType)) {
9297
return extractIndexName(query, queryType, context)
9398
.map(this::stripSchemaPrefix)

0 commit comments

Comments
 (0)