Add multi-index support for DSL search queries

[ask-kamal-nayan]

Description

Adds multi-index support to the DSL DataFusion path, for both hits and aggregations. A _search against N indices (comma list, wildcard, alias, or data stream) now resolves, plans, executes, and renders correctly. When all indices agree on a referenced field, results match single-index behaviour; when they genuinely disagree, the request fails with HTTP 400 instead of returning wrong matches or garbled buckets. All new logic is a no-op at N==1 (single-index path byte-identical, no feature flag).

Why: resolution previously hard-required exactly one concrete index. Relaxing that is unsafe on its own — the schema builder unions indices first-wins with no conflict detection and collapses distinct OpenSearch types onto one Calcite type, so divergent fields surface as wrong query literals and wrong bucket keys, neither raising an error today.

What changes:

• MultiIndexResolutionStrategy — sibling to SingleIndexResolutionStrategy; resolves to an ordered List<IndexMetadata> instead of throwing when count != 1. Selected unconditionally by TransportExecuteAction / TransportValidateAction.
• RequestScopedMapperService — widened to an ordered index list with first-non-null field lookup and lazy per-index mapper construction (handles split fields with no reject rule).
• SchemaEquivalenceGate — two-concern gate, post-conversion / pre-execution, scoped to fields the resolved plan references (RexInputRef), compared at MappedFieldType granularity:
  • Conversion — all defining indices must agree on the Calcite type used for literals (catches keyword/long, ip/keyword). Governs hits and predicates, not just aggs.
  • Render — aggregated bucket fields must agree on TermsResponseStrategy.forType(typeName) + docValueFormat + a per-type discriminator (scaled_float factor, keyword normalizer, date/date_nanos, float precision). Catches same-Calcite-type splits the conversion gate can't see. Metrics RAW-exempt. Format equality via .equals() with a RAW fast-path.
• Filtered-alias guard — 400 on filtering aliases the engine can't honour; notably rejects the expand_wildcards=open,hidden case where vanilla silently drops the filter (a confirmed tenant-leak). Documented divergence from vanilla.
• resolveExpressions(ClusterState, IndicesOptions, String...) — additive IndexNameExpressionResolver overload so the guard resolves under the request's own options. No existing signature changes.
• Phantom-column fix — backing indices filtered to State.OPEN before their properties union into the schema.

Testing: unit dsl-query-executor 568/0, analytics-engine 1070/0, server resolver 77/0. Live E2E on a running cluster covering expression forms, IndicesOptions, both gates (incl. the discriminator holes), phantom-column exclusion, filtered/hidden-alias guards, split-field resolution, hits, and aggregation types — verified to reject genuine divergence and not over-reject equivalents (keyword≡text, identical factor/normalizer); empty resolution returns a clean empty 200.

Known limitations: (1) a pre-existing broad whole-mapping check in shared IndexResolution (unmodified here) fires before the scoped gate and over-rejects on divergence in an unreferenced field. (2) scaled_float with divergent factors used as a metric returns a wrong sum — metrics are RAW-exempt and this is a separate pre-existing single-index engine bug, not introduced or fixed here.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

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 090665f)

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

📝 TODO sections sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/TransportExecuteAction.java [117]: cache per (indexUUID, mappingVersion) to avoid rebuilding analyzers.

🔀 Multiple PR themes Sub-PR theme: Add IndicesOptions-aware resolveExpressions overload Relevant files: server/src/main/java/org/opensearch/cluster/metadata/IndexNameExpressionResolver.java server/src/test/java/org/opensearch/cluster/metadata/IndexNameExpressionResolverTests.java Sub-PR theme: Thread IndicesOptions through analytics schema build Relevant files: sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/EngineContextProvider.java sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java sandbox/plugins/analytics-engine/src/main/java/org/opensearch/analytics/AnalyticsPlugin.java sandbox/plugins/analytics-engine/src/test/java/org/opensearch/analytics/engine/OpenSearchSchemaBuilderTests.java Sub-PR theme: Introduce FieldTypeLookup abstraction (Supplier -> FieldTypeLookup) Relevant files: sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/aggregation/FieldTypeLookup.java sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/aggregation/AggregationRegistryFactory.java sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/aggregation/bucket/TermsBucketTranslator.java sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/converter/SearchSourceConverter.java sandbox/plugins/dsl-query-executor/src/test/java/org/opensearch/dsl/aggregation/bucket/TermsBucketTranslatorTests.java sandbox/plugins/dsl-query-executor/src/test/java/org/opensearch/dsl/golden/TestMapperServices.java sandbox/plugins/dsl-query-executor/src/test/java/org/opensearch/dsl/result/SearchResponseBuilderTests.java

⚡ Recommended focus areas for review Empty resolution leaks mapper holder When resolvedIndices.isEmpty(), the code returns early via listener.onResponse(...) without constructing or closing a RequestScopedMapperService. That is fine here because none was built, but FilteringAliasGuard.check is invoked before the empty check with a non-null request.indices() — for allow_no_indices=true matching nothing, this is a no-op, but consider whether the ordering is intended. Additionally, SearchResponseBuilder.empty declares throws ConversionException; if it ever throws, the exception escapes the outer try/catch (which was already exited) and is not routed to the listener, leaving the request without a response. Wrap the empty short-circuit in a try/catch that routes failures to listener.onFailure. if (resolvedIndices.isEmpty()) { long tookInMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos); listener.onResponse(SearchResponseBuilder.empty(request, tookInMillis)); return; } Reflection fail-closed identity leaks between requests renderDiscriminator for scaled_float when the accessor cannot be found returns "scaled_float:unresolved:" + System.identityHashCode(fieldType). Two indices whose MappedFieldType instances happen to share an identity hash code (collisions are possible since identityHashCode is 32-bit and not unique) would compare equal and silently pass the gate — the exact silent-pass hole this branch tries to close. Consider using the field-type object identity via a per-check IdentityHashMap/counter, or reject unconditionally when the factor cannot be resolved, so equality is deterministic and cannot alias. private static String renderDiscriminator(MappedFieldType fieldType) { String typeName = fieldType.typeName(); switch (typeName) { case "scaled_float": Double factor = scaledFloatScalingFactor(fieldType); if (factor == null) { // Fail CLOSED: the scaling_factor could not be read (accessor renamed/removed), // so we cannot prove two scaled_float fields share it. Returning a bare // "scaled_float" would collapse every unresolved field to one value, let // Objects.equals pass, and silently reopen the grouping-corruption hole this // gate closes. Keying on the field type's identity guarantees two unresolved // fields never compare equal, forcing the gate's normal incompatible-parameters // rejection rather than a silent pass. return "scaled_float:unresolved:" + System.identityHashCode(fieldType); } Terms sub-agg field references not collected referencedFields walks the RelNode plans and aggregatedBucketFields walks the SearchSource aggregations. However, a request may reference a field only through a nested terms aggregation (aggregatedBucketFields includes it) that is not present in the plan's TableScan projection. If such a field is not returned by referencedFields, SchemaEquivalenceGate.checkConversion will not run on it — only checkRender. That is likely fine because render already validates conversion via the strategy, but conversion-type divergences that would only show up in query literals for a non-aggregated referenced field must still be caught. Verify that terms sub-agg bucket fields flow into referencedFields (e.g., via the aggregation-derived RelNode plan) so the conversion concern is enforced. static Set<String> referencedFields(QueryPlans plans) { Set<String> baseNames = new HashSet<>(); RelVisitor scanCollector = new RelVisitor() { @Override public void visit(RelNode node, int ordinal, RelNode parent) { if (node instanceof TableScan) { baseNames.addAll(node.getRowType().getFieldNames()); } super.visit(node, ordinal, parent); } }; for (QueryPlans.QueryPlan plan : plans.getAll()) { scanCollector.go(plan.relNode()); } Set<String> referenced = new HashSet<>(); RelVisitor refCollector = new RelVisitor() { @Override public void visit(RelNode node, int ordinal, RelNode parent) { collectNodeReferences(node, baseNames, referenced); super.visit(node, ordinal, parent); } }; for (QueryPlans.QueryPlan plan : plans.getAll()) { refCollector.go(plan.relNode()); } return referenced; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 090665f
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Check empty resolution before alias guard The empty-resolution short-circuit runs AFTER FilteringAliasGuard.check, but the guard's loop over concreteIndices is a no-op when the list is empty, so this ordering is fine. However, invoking FilteringAliasGuard.check before checking resolvedIndices.isEmpty() still executes resolveExpressions unnecessarily; move the empty check first to avoid the wasted resolver call and to make the intent explicit. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/TransportExecuteAction.java [101-113] try { resolvedIndices = indexResolutionStrategy.resolve(indexNameExpressionResolver, state, request); - // Reject filtering aliases the engine cannot honor (including hidden aliases reached via - // expand_wildcards=open,hidden) before conversion/execution — an intentional 400 divergence. - FilteringAliasGuard.check(indexNameExpressionResolver, state, request.indices(), request.indicesOptions(), resolvedIndices); // Legitimate empty resolution (allow_no_indices=true matching nothing) answers an // empty 200 like vanilla _search, before building a mapper for zero indices. - // allow_no_indices=false with no match already threw IndexNotFoundException above. if (resolvedIndices.isEmpty()) { long tookInMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos); listener.onResponse(SearchResponseBuilder.empty(request, tookInMillis)); return; } + // Reject filtering aliases the engine cannot honor (including hidden aliases reached via + // expand_wildcards=open,hidden) before conversion/execution — an intentional 400 divergence. + FilteringAliasGuard.check(indexNameExpressionResolver, state, request.indices(), request.indicesOptions(), resolvedIndices); Suggestion importance[1-10]: 3 __ Why: Minor optimization: avoids a resolver call on empty resolution. The current ordering is functionally correct (the guard is a no-op on empty concrete lists), so the impact is small.	Low
	Fail loudly on unknown index lookup Comparing IndexMetadata by getIndex() equality is correct, but silently returning null when the passed index is not part of this request's indices masks caller bugs (e.g. the schema-equivalence gate passing a wrong IndexMetadata). Consider throwing IllegalArgumentException in that case to fail loudly, since the gate contract states callers pass "one of the request's resolved indices". sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/RequestScopedMapperService.java [109-118] synchronized MappedFieldType fieldType(IndexMetadata index, String field) { for (int i = 0; i < indices.size(); i++) { IndexMetadata candidate = indices.get(i); if (candidate == index || candidate.getIndex().equals(index.getIndex())) { MapperService mapperService = mapperFor(i); return mapperService == null ? null : mapperService.fieldType(field); } } - return null; + throw new IllegalArgumentException("index [" + index.getIndex().getName() + "] is not part of this request's resolved indices"); } Suggestion importance[1-10]: 3 __ Why: Reasonable defensive-programming suggestion, but the javadoc already documents the null return contract ("null when the index is not part of this request"). Changing to throw would break the documented API.	Low
	Normalize case for schema table key Concrete index names can contain characters that collide with the schema table lookup (e.g. commas are impossible in index names but the joined expression is used verbatim as a table key in the Calcite schema). Since OpenSearchSchemaBuilder's lazy tableMap.get lowercases the key, an index name with mixed case will only resolve because of the lowercase key normalization — but if any index name changes case between resolution and schema lookup, the union table may not be found. Consider lowercasing here to match the schema's key normalization explicitly. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/TransportExecuteAction.java [176-178] /** Joins the resolved concrete index names into one schema expression resolving to their union. */ private static String concreteIndexExpression(List<IndexMetadata> resolvedIndices) { - return resolvedIndices.stream().map(index -> index.getIndex().getName()).collect(Collectors.joining(",")); + return resolvedIndices.stream() + .map(index -> index.getIndex().getName().toLowerCase(java.util.Locale.ROOT)) + .collect(Collectors.joining(",")); } Suggestion importance[1-10]: 2 __ Why: OpenSearch index names must be lowercase, so the concern about case mismatch is largely theoretical. The suggestion offers marginal defensive value.	Low
	Align alias closed-index handling with options The alias/data-stream short-circuit unconditionally filters to OPEN backings regardless of indicesOptions, while the wildcard/expression path (else branch) honors caller options and may include closed indices. This creates an inconsistency: a caller passing forbidClosedIndices=false options gets closed backings via wildcards but not via literal alias names. Consider honoring indicesOptions.forbidClosedIndices() (or expandWildcardsClosed()) here for consistency, or document this asymmetry. sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [152-164] if (abstraction != null && (abstraction.getType() == IndexAbstraction.Type.ALIAS || abstraction.getType() == IndexAbstraction.Type.DATA_STREAM)) { - // Filter alias / data-stream backings to State.OPEN unconditionally — never gated on - // indicesOptions. The execution-side IndexResolution.resolveAlias/resolveDataStream - // drop closed backings unconditionally (and vanilla _search forbids closed indices), - // so the schema must mirror that: a closed backing's columns would otherwise become - // phantom columns that pass Calcite validation but never receive rows at scan time. + boolean includeClosed = indicesOptions.forbidClosedIndices() == false; backing = new ArrayList<>(abstraction.getIndices().size()); for (IndexMetadata index : abstraction.getIndices()) { - if (index.getState() == IndexMetadata.State.OPEN) { + if (index.getState() == IndexMetadata.State.OPEN || includeClosed) { backing.add(index); } } } else { Suggestion importance[1-10]: 2 __ Why: The suggestion contradicts the explicit intent documented in the code comment ("never gated on indicesOptions") to mirror execution-side IndexResolution behavior that drops closed backings unconditionally. Applying this change would reintroduce the phantom-column issue the PR closes.	Low

---

Previous suggestions

Suggestions up to commit 4ea4ba2

Category	Suggestion	Impact
General	Fail loudly on unknown index lookup The fieldType(IndexMetadata, String) method silently returns null when the passed index is not among the request's resolved indices, which cannot be distinguished from the index simply not defining the field. Since this is used by the schema-equivalence gate — a security-relevant path — a mismatched index should fail loudly (e.g., throw IllegalArgumentException) to catch caller bugs that would otherwise skip equivalence checks and reopen the grouping-corruption hole this gate closes. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/RequestScopedMapperService.java [113-122] synchronized MappedFieldType fieldType(IndexMetadata index, String field) { for (int i = 0; i < indices.size(); i++) { IndexMetadata candidate = indices.get(i); if (candidate == index || candidate.getIndex().equals(index.getIndex())) { MapperService mapperService = mapperFor(i); return mapperService == null ? null : mapperService.fieldType(field); } } - return null; + throw new IllegalArgumentException("index [" + index.getIndex().getName() + "] is not part of this request's resolved indices"); } Suggestion importance[1-10]: 4 __ Why: Reasonable defensive programming suggestion, but changing to throw could break callers that intentionally probe; the current null-return contract is documented in the javadoc.	Low
	Short-circuit empty resolution before alias guard Reorder to check the empty-resolution short-circuit before invoking FilteringAliasGuard.check. Currently, an empty resolvedIndices still goes through the alias guard, which is wasted work but more importantly, the intent stated in the comment ("before building a mapper... this short-circuit only covers the legitimately-empty case") is clearer when the empty check runs first. Additionally, SearchResponseBuilder.empty is declared to throw ConversionException, but the surrounding catch only catches Exception — verify the call to listener.onResponse inside the try block does not swallow a builder failure via the outer catch and route it as onFailure with a misleading context. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/TransportExecuteAction.java [101-115] try { resolvedIndices = indexResolutionStrategy.resolve(indexNameExpressionResolver, state, request); - // Reject filtering aliases the engine cannot honor (including hidden aliases reached via - // expand_wildcards=open,hidden) before conversion/execution — an intentional 400 divergence. - FilteringAliasGuard.check(indexNameExpressionResolver, state, request.indices(), request.indicesOptions(), resolvedIndices); - // Legitimate empty resolution (allow_no_indices=true with a wildcard/none matching - // nothing) answers an empty 200 like vanilla _search, before building a mapper for - // zero indices (the RequestScopedMapperService ctor rejects an empty index set). - // allow_no_indices=false with no match throws IndexNotFoundException in resolve() - // above, so this short-circuit only covers the legitimately-empty case. if (resolvedIndices.isEmpty()) { long tookInMillis = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - startNanos); listener.onResponse(SearchResponseBuilder.empty(request, tookInMillis)); return; } + // Reject filtering aliases the engine cannot honor (including hidden aliases reached via + // expand_wildcards=open,hidden) before conversion/execution — an intentional 400 divergence. + FilteringAliasGuard.check(indexNameExpressionResolver, state, request.indices(), request.indicesOptions(), resolvedIndices); Suggestion importance[1-10]: 3 __ Why: Minor reordering optimization; the alias guard on an empty list is cheap and correctness is unaffected. The ConversionException concern is speculative.	Low
	Stabilize multi-index expression ordering Building a comma-joined index expression as a schema table name is brittle: the schema resolves this via Strings.splitStringByCommaToArray in resolveTable, but if any concrete index name ever contains a comma (unlikely but possible in edge cases), or if resolution ordering differs between requests hitting the same set of indices, cache/table lookups behave inconsistently. Consider sorting the concrete names before joining so equivalent index sets produce identical expressions, and document/assert that index names contain no commas. sandbox/plugins/dsl-query-executor/src/main/java/org/opensearch/dsl/action/TransportExecuteAction.java [178-180] /** Joins the resolved concrete index names into one schema expression resolving to their union. */ private static String concreteIndexExpression(List<IndexMetadata> resolvedIndices) { - return resolvedIndices.stream().map(index -> index.getIndex().getName()).collect(Collectors.joining(",")); + return resolvedIndices.stream() + .map(index -> index.getIndex().getName()) + .sorted() + .collect(Collectors.joining(",")); } Suggestion importance[1-10]: 3 __ Why: Sorting would change resolution ordering semantics which the resolver deliberately preserves; the concern about commas in index names is not realistic since OpenSearch disallows them.	Low
	Align alias filter with indices options The alias/data-stream short-circuit filters to OPEN backings unconditionally, but the wildcard path honors indicesOptions and can include CLOSED indices when the caller opts in. This creates an inconsistency: logs resolving via wildcard can include a closed index, but my_alias pointing to the same closed index cannot. If the closed-index inclusion is a legitimate caller option, this short-circuit silently diverges from the coordinator's resolution the comment claims to mirror. Consider aligning the alias path with indicesOptions.forbidClosedIndices()/expandClosed semantics rather than an unconditional OPEN filter.* sandbox/libs/analytics-api/src/main/java/org/opensearch/analytics/schema/OpenSearchSchemaBuilder.java [152-164] if (abstraction != null && (abstraction.getType() == IndexAbstraction.Type.ALIAS || abstraction.getType() == IndexAbstraction.Type.DATA_STREAM)) { - // Filter alias / data-stream backings to State.OPEN unconditionally — never gated on - // indicesOptions. The execution-side IndexResolution.resolveAlias/resolveDataStream - // drop closed backings unconditionally (and vanilla _search forbids closed indices), - // so the schema must mirror that: a closed backing's columns would otherwise become - // phantom columns that pass Calcite validation but never receive rows at scan time. backing = new ArrayList<>(abstraction.getIndices().size()); for (IndexMetadata index : abstraction.getIndices()) { - if (index.getState() == IndexMetadata.State.OPEN) { + if (index.getState() == IndexMetadata.State.OPEN + || (indicesOptions.expandWildcardsClosed() && indicesOptions.forbidClosedIndices() == false)) { backing.add(index); } } } Suggestion importance[1-10]: 2 __ Why: The suggestion contradicts the PR's explicit intent (documented in the comment) to mirror execution-side IndexResolution.resolveAlias/resolveDataStream which drops closed backings unconditionally to prevent phantom columns.	Low

[github-actions]

❌ Gradle check result for 4ea4ba2: QUEUE_TIMEOUT

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[github-actions]

Persistent review updated to latest commit 090665f

[github-actions]

❌ Gradle check result for 090665f: QUEUE_TIMEOUT

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?