Land analytics-engine PPL integration into main

[ahkcs]

Summary

Single squashed delivery of the long-running feature/mustang-ppl-integration branch into main, consolidating 22 feature-branch PRs plus the conflict-resolved merge of current main.

Why squashed (DCO)

Squashed because the feature branch's history includes commits with missing or mismatched Signed-off-by trailers that block DCO at this scope — the equivalent issue documented for the catch-up squashes in #5397. Specifically:

• 4 feature commits have no Signed-off-by (Route analytics queries by index setting, not table-name prefix #5429, Switch sandbox deps to analytics-api (JDK 21 surface) #5426, Bridge PPL_REX_MAX_MATCH_LIMIT into UnifiedQueryContext on the unified query path #5418, Exclude httpcore5-h2, httpcore5-reactive, httpclient5 from SQL bundle to fix jar hell #5400)
• 2 have mismatched sign-off emails (Marc Handalian: handalm@amazon.com author vs marc.handalian@gmail.com signer; bowenlan vs bowenlan-amzn)
• 54 main-side commits absorbed by the merge also have inconsistent sign-offs (the same gap that pushed Catch up feature/mustang-ppl-integration with main: version bump to 3.7 #5397 to squash)

Tried a real two-parent merge commit first; DCO flagged 29 commits. Squash is the only practical fix given the upstream history. Feature-branch commit lineage is retained on feature/mustang-ppl-integration (f006e29cd).

What this delivers

Analytics-engine PPL integration — a new execution path that routes Parquet-backed (non-Lucene) indices through an analytics engine while keeping Lucene-backed indices on the existing v2 / Calcite paths.

Headline pieces:

• Query routing (Add query routing and execution handoff for Parquet-backed indices #5267) — PPL queries against Parquet-backed indices hand off to the analytics-engine execution path; Lucene-backed indices continue through the legacy path
• Explain support (Add explain support for analytics engine path #5275) — EXPLAIN covers the analytics-engine path
• Profiling + UnifiedQueryParser (Enable profiling and migrate to UnifiedQueryParser #5285) — migrates PPL parsing to the unified parser and wires profiling metrics through the analytics path
• extendedPlugins wiring (Wire analytics-engine as extendedPlugins dependency #5302) — analytics-engine attaches as an OpenSearch extension via SPI
• SQL REST endpoint integration (Integrate SQL REST endpoint with analytics engine path #5317) — analytics-route fork applied to the SQL transport; delegateToV2Engine extracted in RestSqlAction
• Async QueryPlanExecutor (Version bump to OpenSearch 3.7 with async QueryPlanExecutor #5396) — async execution for analytics-engine plans + version bump to OpenSearch 3.7
• Optional dependency (Make analytics-engine an optional dependency  #5403) — analytics-engine becomes an optional runtime dep so the SQL bundle is shippable without it
• Index-setting-based routing (Route analytics queries by index setting, not table-name prefix #5429) — replaces the earlier table-name-prefix heuristic with an authoritative index-setting check

Supporting infrastructure:

• Gradle wrapper bump to 9.4.1 (Bump gradle wrapper to 9.4.1 #5406)
• Jar-hell exclusions for arrow-flight-rpc / httpcore5-h2 / httpcore5-reactive / httpclient5 (Exclude httpcore5-h2, httpcore5-reactive, httpclient5 from SQL bundle to fix jar hell #5400, Exclude jsr305 from SQL bundle to fix jar hell with arrow-flight-rpc #5409)
• IT plumbing: CalciteEvalCommandIT / CalciteFieldFormatCommandIT carried through the helper-managed index path (Carry CalciteEvalCommandIT through the helper-managed index path #5407, Carry CalciteFieldFormatCommandIT through the helper-managed index path #5417); CalciteReplaceCommandIT column-order-agnostic (Make CalciteReplaceCommandIT column-order-agnostic for analytics-engine route #5415); @Ignore'd Calcite ITs dropped from CalciteNoPushdownIT (Drop @Ignore'd Calcite ITs from CalciteNoPushdownIT @Suite #5416)
• plugins.calcite.enabled=true defaulted on the unified query path (Default plugins.calcite.enabled=true on the unified query path #5413)
• PPL_REX_MAX_MATCH_LIMIT bridged into UnifiedQueryContext (Bridge PPL_REX_MAX_MATCH_LIMIT into UnifiedQueryContext on the unified query path #5418)
• Calcite tolerance fixes: array() default type (Default empty array() return type to ARRAY<VARCHAR> #5421), containsNestedAggregator flat-leaf schemas ([Calcite] Make containsNestedAggregator tolerate flat-leaf schemas #5423)
• Sandbox deps switched to analytics-api JDK 21 surface (Switch sandbox deps to analytics-api (JDK 21 surface) #5426)

Feature-branch commits squashed (22)

#5429, #5426, #5423, #5421, #5418, #5403, #5417, #5415, #5416, #5413, #5407, #5409, #5406, #5400, #5396, #5317, #5302, #5285, #5275, #5267, #5397, #5286

Main commits absorbed via merge (54)

Brings the branch up to current upstream/main (54 commits since the last catch-up at #5397, divergence point 513e1b220). Highlights: #5419, #5408, #5414, #5399, #5394, #5361, #5360, #5240, #5266, #5278, plus 44 others.

Conflict resolutions (7)

Resolved during the merge of main into the feature branch. Resolution kept the feature branch's analytics-engine-path semantics where main's changes would have regressed them.

File	Resolution
api/.../UnifiedQueryContext.java	Blank-line-only conflict; took main's formatting (no extra blank).
core/.../executor/QueryService.java	Kept feature's CalciteClassLoaderHelper.withCalciteClassLoader(...) wrapping (required for analytics-engine classloader isolation) and the matching import.
integ-test/build.gradle	Kept feature's detailed root-cause comment on the Gradle 9.4.1 TestEventReporterAsListener workaround; kept ASCII ordering of JSONRequestIT/JoinIT and SQLFunctionsIT/ShowIT/SourceFieldIT entries.
integ-test/.../CalciteEvalCommandIT.java	Kept feature's if (!TestUtils.isIndexExist(...)) idempotency guards on test_eval and test_eval_agent setup.
legacy/.../RestSqlAction.java	Kept feature's delegateToV2Engine(...). Both sides added handleException/getRestStatus/getRawErrorCode; removed the duplicate set git produced.
plugin/.../SQLPlugin.java	Took the union of imports: ExecutionEngine + ExecutionEngine.ExplainResponse + QueryType.
plugin/.../transport/TransportPPLQueryAction.java	Combined main's OpenSearchPluginModule(extensionsHolder.engines()) and feature's local pluginSettings/pluginSettingsRef wiring.

EngineExtensionsHolder.java is a new file from main (#5298) preserved as-is.

Compatibility / opt-in

The analytics-engine path is gated by the extendedPlugins extension being installed (#5403 makes the dep optional). Clusters without analytics-engine installed see no behavior change. Clusters with analytics-engine installed route only Parquet-backed indices through the new path (#5429 — by index setting).

Merge mode

Either "Create a merge commit" or "Rebase and merge" works — the PR is one commit so the outcome is equivalent.

Test plan

• :api / :core / :legacy / :opensearch-sql-plugin compileJava pass locally
• :integ-test:compileTestJava passes locally
• DCO sign-off present on the single squashed commit
• CI build passes
• Integration tests on the analytics-engine path still pass
• Compatibility run on tests.analytics.parquet_indices=true shows no regressions

---

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.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit f1bc872)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Unguarded Field Access executionEngineExtensions is initialized as an empty list at line 154 but reassigned in loadExtensions (line 176). If loadExtensions is never called or called after createComponents, the field may be in an inconsistent state. The plugin lifecycle does not guarantee loadExtensions runs before createComponents, so createComponents at line 371 may pass an empty list to OpenSearchPluginModule even when extensions exist. private List<ExecutionEngine> executionEngineExtensions = List.of(); private ClusterService clusterService; /** Settings should be inited when bootstrap the plugin. */ private org.opensearch.sql.common.setting.Settings pluginSettings; private NodeClient client; private DataSourceServiceImpl dataSourceService; private OpenSearchAsyncQueryScheduler asyncQueryScheduler; private Injector injector; public String name() { return "sql"; } public String description() { return "Use sql to query OpenSearch."; } @Override public void loadExtensions(ExtensionLoader loader) { List<ExecutionEngine> loaded = loader.loadExtensions(ExecutionEngine.class); this.executionEngineExtensions = loaded != null ? List.copyOf(loaded) : List.of();

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to f1bc872
Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Filter null extensions from loaded list The loadExtensions method may return a list containing null elements. Filter out null entries before copying to prevent NullPointerException when iterating over executionEngineExtensions later. plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java [175-176] List<ExecutionEngine> loaded = loader.loadExtensions(ExecutionEngine.class); -this.executionEngineExtensions = loaded != null ? List.copyOf(loaded) : List.of(); +this.executionEngineExtensions = loaded != null + ? loaded.stream().filter(Objects::nonNull).toList() + : List.of(); Suggestion importance[1-10]: 7 __ Why: Valid defensive programming suggestion. Filtering null elements prevents potential NullPointerException when iterating over executionEngineExtensions. The concern about loadExtensions returning nulls is reasonable for external plugin loading mechanisms.	Medium
	Filter null engines in record constructor The compact constructor should filter out null elements from the input list to prevent potential NullPointerException when consumers iterate over the engines. This ensures defensive copying with validation. plugin/src/main/java/org/opensearch/sql/plugin/config/EngineExtensionsHolder.java [17-18] public record EngineExtensionsHolder(List<ExecutionEngine> engines) { public EngineExtensionsHolder(List<ExecutionEngine> engines) { - this.engines = engines != null ? List.copyOf(engines) : List.of(); + this.engines = engines != null + ? engines.stream().filter(Objects::nonNull).toList() + : List.of(); } } Suggestion importance[1-10]: 7 __ Why: Valid defensive programming suggestion. Filtering null elements in the constructor prevents potential issues when consumers iterate over the engines list. This is consistent with the filtering suggested for SQLPlugin.loadExtensions.	Medium
General	Make extensions field immutable and final The field executionEngineExtensions should be declared as final and initialized in the constructor or via loadExtensions. Currently, it's mutable and initialized to an empty list, which could lead to race conditions if accessed before loadExtensions is called. plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java [154] -private List<ExecutionEngine> executionEngineExtensions = List.of(); +private final List<ExecutionEngine> executionEngineExtensions; Suggestion importance[1-10]: 3 __ Why: While making the field final would improve immutability, the current initialization to List.of() provides a safe default. The suggestion about race conditions is overstated since loadExtensions is called during plugin initialization before concurrent access occurs.	Low

---

Previous suggestions

Suggestions up to commit 6076c1b

Category	Suggestion	Impact
Possible issue	Filter null elements from loaded extensions The loadExtensions method may return a list containing null elements. Filter out null entries before copying to prevent potential NullPointerException when iterating over executionEngineExtensions later. plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java [175-176] List<ExecutionEngine> loaded = loader.loadExtensions(ExecutionEngine.class); -this.executionEngineExtensions = loaded != null ? List.copyOf(loaded) : List.of(); +this.executionEngineExtensions = loaded != null + ? loaded.stream().filter(Objects::nonNull).toList() + : List.of(); Suggestion importance[1-10]: 7 __ Why: Valid concern about potential null elements in the loaded list. Filtering nulls prevents NullPointerException when iterating over executionEngineExtensions, improving robustness.	Medium
	Ensure thread-safe visibility for field Mark executionEngineExtensions as volatile to ensure thread-safe visibility when accessed from multiple threads, since it's modified in loadExtensions and later read in createComponents. plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java [154] -private List<ExecutionEngine> executionEngineExtensions = List.of(); +private volatile List<ExecutionEngine> executionEngineExtensions = List.of(); Suggestion importance[1-10]: 6 __ Why: The executionEngineExtensions field is written in loadExtensions and read in createComponents. Adding volatile ensures proper visibility across threads in a multi-threaded plugin environment, though the actual risk depends on OpenSearch's plugin lifecycle guarantees.	Low

[github-actions]

Persistent review updated to latest commit f1bc872

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 940798c.

Path	Line	Severity	Description
core/build.gradle	36	high	mavenLocal() added as a dependency repository. This resolves artifacts from the local Maven cache (~/.m2) before mavenCentral, enabling dependency-confusion and poisoned-cache attacks where a locally injected artifact shadows the legitimate one.
core/build.gradle	67	high	New dependency on 'org.opensearch.sandbox:analytics-api:3.7.0-SNAPSHOT' (both compileOnly and testImplementation). SNAPSHOT artifacts are mutable and can be silently replaced between builds. The 'sandbox' group namespace has no verified ownership in the public registry and is susceptible to namespace hijacking.
plugin/build.gradle	57	high	extendedPlugins list modified to include 'analytics-engine;optional=true'. This declares a dependency on an externally-supplied plugin whose classloader has full access to the SQL plugin's classes. Mandatory flag: build plugin / compiler extension change.
plugin/build.gradle	162	high	Added runtime dependency 'org.opensearch.sandbox:analytics-api' resolved at ${opensearch_version}. The 'sandbox' group is unverified in public registries and the version is dynamically interpolated, making artifact pinning impossible without an accompanying hash check.
integ-test/build.gradle	274	high	Integration test cluster downloads binary plugin ZIPs from 'https://ci.opensearch.org/ci/dbc/feature-build-opensearch/feature-datafusion/latest/linux/x64/...' using a floating 'latest' path with no checksum verification. An attacker who controls that CI path could serve a backdoored plugin that is then installed into the test OpenSearch cluster.
core/src/main/java/org/opensearch/sql/calcite/utils/CalciteClassLoaderHelper.java	38	medium	Thread context classloader is swapped to the caller's classloader around every Calcite execution. While documented as a CALCITE-3745 workaround, this pattern permanently widens what classes Janino (a code-generation runtime) can resolve and deserves review to confirm no unintended class-load paths are opened.
plugin/src/main/java/org/opensearch/sql/plugin/rest/AnalyticsExecutorHolder.java	27	medium	Global static volatile field holds a QueryPlanExecutor set by whichever plugin satisfies the Guice binding. If analytics-engine is replaced with a malicious artifact (see supply-chain issues above), this holder silently routes all matching queries through attacker-controlled execution logic without any integrity check.
plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java	160	low	Query routing to the analytics engine is decided by reading mutable index settings (PLUGGABLE_DATAFORMAT_ENABLED + value == 'composite'). If an attacker can write cluster/index settings they could redirect arbitrary SQL/PPL queries to the analytics execution path, bypassing the normal SQL engine's authorization and auditing stack.

The table above displays the top 10 most important findings.

Total: 8 | Critical: 0 | High: 5 | Medium: 2 | Low: 1

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[penghuo]

nit. revert it

[ahkcs]

Updated

[bowenlan-amzn]

When using the analytics engine, sql will use a patched version of Calcite published from opensearch core side.
Leaving this here for broader awareness.
opensearch-project/OpenSearch#21501