diff --git a/libs/analytics-engine-3.7.0-SNAPSHOT.zip b/libs/analytics-engine-3.7.0-SNAPSHOT.zip index 601225ea102..fedfaa441a0 100644 Binary files a/libs/analytics-engine-3.7.0-SNAPSHOT.zip and b/libs/analytics-engine-3.7.0-SNAPSHOT.zip differ diff --git a/libs/analytics-framework-3.7.0-SNAPSHOT.jar b/libs/analytics-framework-3.7.0-SNAPSHOT.jar index a5cfcc32946..a28474484f7 100644 Binary files a/libs/analytics-framework-3.7.0-SNAPSHOT.jar and b/libs/analytics-framework-3.7.0-SNAPSHOT.jar differ diff --git a/plugin/build.gradle b/plugin/build.gradle index 4306235b13d..103c87ff398 100644 --- a/plugin/build.gradle +++ b/plugin/build.gradle @@ -55,53 +55,21 @@ opensearchplugin { name 'opensearch-sql' description 'OpenSearch SQL' classname 'org.opensearch.sql.plugin.SQLPlugin' - extendedPlugins = ['opensearch-job-scheduler', 'analytics-engine'] + extendedPlugins = ['opensearch-job-scheduler', 'analytics-engine;optional=true'] licenseFile rootProject.file("LICENSE.txt") noticeFile rootProject.file("NOTICE") } -// Exclude jars provided by the analytics-engine plugin (shared via extendedPlugins classloader). -// MUST match what's actually in the analytics-engine ZIP to avoid both jar hell (when analytics-engine -// ships it) and ClassNotFoundException at runtime (when it doesn't). Last verified against -// analytics-engine-3.7.0-SNAPSHOT — keep this list aligned when bumping versions. -bundlePlugin { - exclude 'calcite-core-*.jar' - exclude 'calcite-linq4j-*.jar' - exclude 'avatica-core-*.jar' - exclude 'guava-*.jar' - exclude 'failureaccess-*.jar' - exclude 'slf4j-api-*.jar' - exclude 'commons-codec-*.jar' - exclude 'commons-compiler-*.jar' - exclude 'commons-io-*.jar' - exclude 'commons-lang3-*.jar' - exclude 'janino-*.jar' - exclude 'joou-java-6-*.jar' - exclude 'json-path-*.jar' - exclude 'jackson-annotations-*.jar' - exclude 'jackson-databind-*.jar' - exclude 'httpcore5-5*.jar' - // TODO: Remove the three httpcore5/httpclient5 exclusions below — and ideally this entire - // bundlePlugin exclusion block — once analytics-engine becomes an optional dependency via the - // AnalyticsFrontEndExtension SPI (opensearch-project/OpenSearch#21449). The shared-classloader - // jar deduplication that requires this hand-maintained list goes away with the SPI. - exclude 'httpcore5-h2-*.jar' - exclude 'httpcore5-reactive-*.jar' - exclude 'httpclient5-*.jar' - exclude 'commons-text-*.jar' - // Calcite transitive deps now bundled in analytics-engine 3.7 — exclude from sql to avoid jar hell. - exclude 'commons-math3-*.jar' - exclude 'commons-dbcp2-*.jar' - exclude 'commons-pool2-*.jar' - exclude 'uzaygezen-core-*.jar' - exclude 'sketches-core-*.jar' - exclude 'memory-0*.jar' - exclude 'jts-io-common-*.jar' - exclude 'proj4j-*.jar' - exclude 'json-smart-*.jar' - exclude 'accessors-smart-*.jar' - exclude 'asm-9*.jar' -} +// SQL bundles its own copies of all transitive deps (Calcite, Guava, etc.) so the plugin can +// boot and serve queries when analytics-engine is not installed. With ';optional=true', +// PluginsService.checkBundleJarHell skips both URL-overlap and class-level checks against +// analytics-engine (server/.../PluginsService.java:763-765), so duplicate bundling is safe. +// +// At runtime: when analytics-engine IS installed, parent-first classloader delegation gives +// SQL analytics-engine's patched Calcite (CALCITE-3745) and shared deps; SQL's own bundled +// copies are shadowed. When analytics-engine is absent, SQL falls back to its own bundled +// copies — Janino's classloader story works because everything lives in SQL's classloader, +// no parent visibility gap to bridge. publishing { publications { diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLAnalyticsFrontEndExtension.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLAnalyticsFrontEndExtension.java new file mode 100644 index 00000000000..1f016f06aa0 --- /dev/null +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLAnalyticsFrontEndExtension.java @@ -0,0 +1,28 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.plugin; + +import org.opensearch.analytics.spi.AnalyticsFrontEndExtension; +import org.opensearch.analytics.spi.AnalyticsServices; +import org.opensearch.sql.plugin.rest.AnalyticsExecutorHolder; + +/** + * SPI consumer that publishes the analytics-engine services into {@link AnalyticsExecutorHolder} + * when analytics-engine is installed. + * + *

Kept separate from {@link SQLPlugin} so that SQLPlugin's bytecode does not reference any + * analytics-framework class. When analytics-engine is absent, OpenSearch never invokes {@code + * ServiceLoader.load(AnalyticsFrontEndExtension.class)} (because no plugin defining the SPI is + * present), so this class is never resolved and SQL boots without needing analytics-framework on + * its runtime classpath. + */ +public class SQLAnalyticsFrontEndExtension implements AnalyticsFrontEndExtension { + + @Override + public void setAnalyticsServices(AnalyticsServices services) { + AnalyticsExecutorHolder.set(services.queryPlanExecutor(), services.schemaProvider()); + } +} diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index 41a6d8c486e..33973292fa1 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -204,11 +204,14 @@ private BiFunction createSqlAnalyticsRout java.util.function.Supplier handlerSupplier = () -> { if (cached[0] == null) { - var executor = AnalyticsExecutorHolder.get(); - if (executor == null) { + Object executor = AnalyticsExecutorHolder.getQueryPlanExecutor(); + Object schemaProvider = AnalyticsExecutorHolder.getSchemaProvider(); + if (executor == null || schemaProvider == null) { return null; } - cached[0] = new RestUnifiedQueryAction(client, clusterService, executor); + cached[0] = + RestUnifiedQueryAction.fromUnknownExecutor( + client, clusterService, executor, schemaProvider); } return cached[0]; }; diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/AnalyticsExecutorHolder.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/AnalyticsExecutorHolder.java index fa3e7d1d1fa..a08f4588bee 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/AnalyticsExecutorHolder.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/AnalyticsExecutorHolder.java @@ -5,32 +5,48 @@ package org.opensearch.sql.plugin.rest; -import org.apache.calcite.rel.RelNode; -import org.opensearch.analytics.exec.QueryPlanExecutor; - /** - * Bridge for sharing the analytics-engine {@link QueryPlanExecutor} between the PPL transport - * action (where Guice resolves the binding via {@code @Inject}) and the REST-only SQL router (where - * Guice cannot, because {@code SQLPlugin#getRestHandlers} runs before the Node-level injector - * satisfies {@code @Inject} parameters). + * Static bridge that lets {@code SQLAnalyticsFrontEndExtension} (the SPI consumer) hand off the + * analytics-engine services to the SQL request paths that need them. * - *

Why a static holder: cross-plugin Guice injection needs a class registered in the Node - * injector, and {@link org.opensearch.sql.plugin.SQLPlugin}'s SQL routing path is built in {@code - * getRestHandlers} — outside any Guice-managed lifecycle. Persisting the executor in this holder - * once {@link org.opensearch.sql.plugin.transport.TransportPPLQueryAction} is constructed lets the - * SQL router read the same instance without going back through the injector. + *

Stored as {@link Object} on purpose. Callers cast at use sites that are already gated on a + * non-null value, ensuring no analytics-framework class is referenced from any signature loaded at + * SQL plugin startup. When analytics-engine is not installed, the SPI never fires, the holder stays + * null, and {@code TransportPPLQueryAction} / {@code SQLPlugin#createSqlAnalyticsRouter} fall + * through to the legacy paths without ever touching analytics-framework types. */ public final class AnalyticsExecutorHolder { - private static volatile QueryPlanExecutor> executor; + private static volatile Object queryPlanExecutor; + private static volatile Object schemaProvider; private AnalyticsExecutorHolder() {} - public static void set(QueryPlanExecutor> instance) { - executor = instance; + /** + * Set both services in one call. Invoked by {@code SQLAnalyticsFrontEndExtension} from the SPI + * push lifecycle. Either argument may be {@code null} (not expected today, but tolerated). + */ + public static void set(Object queryPlanExecutor, Object schemaProvider) { + AnalyticsExecutorHolder.queryPlanExecutor = queryPlanExecutor; + AnalyticsExecutorHolder.schemaProvider = schemaProvider; + } + + /** + * Returns the analytics-engine query plan executor as {@link Object}. Returns {@code null} when + * analytics-engine is not installed (SPI never fired). Callers cast to {@code + * QueryPlanExecutor>} only inside code paths that are gated on a + * non-null return value — see {@code RestUnifiedQueryAction#fromUnknownExecutor}. + */ + public static Object getQueryPlanExecutor() { + return queryPlanExecutor; } - public static QueryPlanExecutor> get() { - return executor; + /** + * Returns the analytics-engine schema provider as {@link Object}. Returns {@code null} when + * analytics-engine is not installed. Callers cast to {@code SchemaProvider} only inside code + * paths that are gated on a non-null return value. + */ + public static Object getSchemaProvider() { + return schemaProvider; } } diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java index acd50ac8b1f..01356b1bd60 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryAction.java @@ -23,7 +23,7 @@ import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.ThreadContext; import org.opensearch.analytics.exec.QueryPlanExecutor; -import org.opensearch.analytics.schema.OpenSearchSchemaBuilder; +import org.opensearch.analytics.schema.SchemaProvider; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.unit.TimeValue; import org.opensearch.core.action.ActionListener; @@ -59,14 +59,33 @@ public class RestUnifiedQueryAction { private final AnalyticsExecutionEngine analyticsEngine; private final NodeClient client; private final ClusterService clusterService; + private final SchemaProvider schemaProvider; public RestUnifiedQueryAction( NodeClient client, ClusterService clusterService, - QueryPlanExecutor> planExecutor) { + QueryPlanExecutor> planExecutor, + SchemaProvider schemaProvider) { this.client = client; this.clusterService = clusterService; this.analyticsEngine = new AnalyticsExecutionEngine(planExecutor); + this.schemaProvider = schemaProvider; + } + + /** + * Construct from holder-stashed services typed as {@link Object} so callers don't take a + * compile-time reference on analytics-framework. The cast is confined to this method — invoking + * it loads the analytics-framework types for the first time, and the caller must gate on a + * non-null executor (i.e. analytics-engine is installed). + */ + @SuppressWarnings("unchecked") + public static RestUnifiedQueryAction fromUnknownExecutor( + NodeClient client, ClusterService clusterService, Object executor, Object schemaProvider) { + return new RestUnifiedQueryAction( + client, + clusterService, + (QueryPlanExecutor>) executor, + (SchemaProvider) schemaProvider); } /** @@ -161,7 +180,7 @@ private static UnifiedQueryContext buildParsingContext(QueryType queryType) { private UnifiedQueryContext buildContext(QueryType queryType, boolean profiling) { return UnifiedQueryContext.builder() .language(queryType) - .catalog(SCHEMA_NAME, OpenSearchSchemaBuilder.buildSchema(clusterService.state())) + .catalog(SCHEMA_NAME, schemaProvider.buildSchema(clusterService.state())) .defaultNamespace(SCHEMA_NAME) .profiling(profiling) .build(); diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java index f83a1277753..00c0eb6f97f 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/transport/TransportPPLQueryAction.java @@ -13,11 +13,9 @@ import java.util.Locale; import java.util.Optional; import java.util.function.Supplier; -import org.apache.calcite.rel.RelNode; import org.opensearch.action.ActionRequest; import org.opensearch.action.support.ActionFilters; import org.opensearch.action.support.HandledTransportAction; -import org.opensearch.analytics.exec.QueryPlanExecutor; import org.opensearch.cluster.service.ClusterService; import org.opensearch.common.inject.Guice; import org.opensearch.common.inject.Inject; @@ -62,7 +60,16 @@ public class TransportPPLQueryAction private final Supplier pplEnabled; - private final RestUnifiedQueryAction unifiedQueryHandler; + private final NodeClient client; + + private final ClusterService clusterService; + + /** + * Lazily-resolved analytics handler. {@code null} until the first analytics-routed request, and + * remains {@code null} forever if analytics-engine is not installed (the SPI never fires, so + * {@link AnalyticsExecutorHolder#getQueryPlanExecutor()} stays null). + */ + private volatile RestUnifiedQueryAction unifiedQueryHandler; /** Constructor of TransportPPLQueryAction. */ @Inject @@ -72,10 +79,12 @@ public TransportPPLQueryAction( NodeClient client, ClusterService clusterService, DataSourceServiceImpl dataSourceService, - org.opensearch.common.settings.Settings clusterSettings, - QueryPlanExecutor> queryPlanExecutor) { + org.opensearch.common.settings.Settings clusterSettings) { super(PPLQueryAction.NAME, transportService, actionFilters, TransportPPLQueryRequest::new); + this.client = client; + this.clusterService = clusterService; + ModulesBuilder modules = new ModulesBuilder(); modules.add(new OpenSearchPluginModule()); modules.add( @@ -86,9 +95,6 @@ public TransportPPLQueryAction( b.bind(DataSourceService.class).toInstance(dataSourceService); }); this.injector = Guice.createInjector(modules); - AnalyticsExecutorHolder.set(queryPlanExecutor); - this.unifiedQueryHandler = - new RestUnifiedQueryAction(client, clusterService, queryPlanExecutor); this.pplEnabled = () -> MULTI_ALLOW_EXPLICIT_INDEX.get(clusterSettings) @@ -98,6 +104,31 @@ public TransportPPLQueryAction( .getSettingValue(Settings.Key.PPL_ENABLED); } + /** + * Resolves the analytics-engine-backed handler lazily. Returns {@code null} when analytics-engine + * is not installed; callers fall through to the legacy PPL pipeline. Synchronized double-checked + * cache so we only build the handler once on the first analytics request. + */ + private RestUnifiedQueryAction analyticsHandler() { + RestUnifiedQueryAction cached = unifiedQueryHandler; + if (cached != null) { + return cached; + } + Object executor = AnalyticsExecutorHolder.getQueryPlanExecutor(); + Object schemaProvider = AnalyticsExecutorHolder.getSchemaProvider(); + if (executor == null || schemaProvider == null) { + return null; + } + synchronized (this) { + if (unifiedQueryHandler == null) { + unifiedQueryHandler = + RestUnifiedQueryAction.fromUnknownExecutor( + client, clusterService, executor, schemaProvider); + } + return unifiedQueryHandler; + } + } + /** * {@inheritDoc} Transform the request and call super.doExecute() to support call from other * plugins. @@ -134,16 +165,20 @@ protected void doExecute( QueryContext.setProfile(transformedRequest.profile()); ActionListener clearingListener = wrapWithProfilingClear(listener); - // Route to analytics engine for non-Lucene (e.g., Parquet-backed) indices - if (unifiedQueryHandler.isAnalyticsIndex(transformedRequest.getRequest(), QueryType.PPL)) { + // Route to analytics engine for non-Lucene (e.g., Parquet-backed) indices. + // analyticsHandler() returns null when analytics-engine isn't installed — we fall through + // to the regular Lucene PPL path so non-analytics queries still work. + RestUnifiedQueryAction analytics = analyticsHandler(); + if (analytics != null + && analytics.isAnalyticsIndex(transformedRequest.getRequest(), QueryType.PPL)) { if (transformedRequest.isExplainRequest()) { - unifiedQueryHandler.explain( + analytics.explain( transformedRequest.getRequest(), QueryType.PPL, transformedRequest.mode(), createExplainResponseListener(transformedRequest, clearingListener)); } else { - unifiedQueryHandler.execute( + analytics.execute( transformedRequest.getRequest(), QueryType.PPL, transformedRequest.profile(), diff --git a/plugin/src/main/resources/META-INF/services/org.opensearch.analytics.spi.AnalyticsFrontEndExtension b/plugin/src/main/resources/META-INF/services/org.opensearch.analytics.spi.AnalyticsFrontEndExtension new file mode 100644 index 00000000000..5b9b892ec1f --- /dev/null +++ b/plugin/src/main/resources/META-INF/services/org.opensearch.analytics.spi.AnalyticsFrontEndExtension @@ -0,0 +1,6 @@ +# +# Copyright OpenSearch Contributors +# SPDX-License-Identifier: Apache-2.0 +# + +org.opensearch.sql.plugin.SQLAnalyticsFrontEndExtension diff --git a/plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java b/plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java index c05a34128d6..9451b1f141a 100644 --- a/plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java +++ b/plugin/src/test/java/org/opensearch/sql/plugin/rest/RestUnifiedQueryActionTest.java @@ -13,6 +13,7 @@ import org.junit.Before; import org.junit.Test; import org.opensearch.analytics.exec.QueryPlanExecutor; +import org.opensearch.analytics.schema.SchemaProvider; import org.opensearch.cluster.service.ClusterService; import org.opensearch.sql.executor.QueryType; import org.opensearch.transport.client.node.NodeClient; @@ -30,7 +31,11 @@ public void setUp() { @SuppressWarnings("unchecked") QueryPlanExecutor> executor = mock(QueryPlanExecutor.class); action = - new RestUnifiedQueryAction(mock(NodeClient.class), mock(ClusterService.class), executor); + new RestUnifiedQueryAction( + mock(NodeClient.class), + mock(ClusterService.class), + executor, + mock(SchemaProvider.class)); } @Test