Resolve indices using original index pattern #134218

idegtiarenko · 2025-09-05T13:26:39Z

Today we parse original index pattern into EsqlExecutionInfo#clusterInfo and then assemble it back createIndexExpressionFromAvailableClusters.

In CPS with flat expressions resolution logic is more complex and should be delegated to field caps call using original expression.

idegtiarenko · 2025-09-05T13:28:15Z

...ql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueryIT.java

            String q = "FROM nomatch*,cluster-a:nomatch";
-            String expectedError = "Unknown index [cluster-a:nomatch,nomatch*]";
+            String expectedError = "Unknown index [nomatch*,cluster-a:nomatch]";
            expectVerificationExceptionForQuery(q, expectedError, requestIncludeMeta);


Here and in 3 below cases the error message index now matches the original input (notice that the order was different).

Is this ordering stable - i.e. is it going to always match original order, no matter how many patterns and clusters we have?

Correct, error uses indexPattern. We are supplying original user input into it now.

idegtiarenko · 2025-09-05T13:33:49Z

...erTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueriesWithInvalidLicenseIT.java

        assertThat(e.getMessage(), containsString(LICENSE_ERROR_MESSAGE));
    }

+    @AwaitsFix(bugUrl = "es-12487")


It is an interesting issue.
This test verifies index resolution with something like FROM fake-remote:events.

Field caps relies on returnLocalAll = true at

elasticsearch/server/src/main/java/org/elasticsearch/action/fieldcaps/TransportFieldCapabilitiesAction.java

Lines 171 to 172 in 18286be

final Map<String, OriginalIndices> remoteClusterIndices = transportService.getRemoteClusterService()

.groupIndices(request.indicesOptions(), request.indices());

that causes it to return all indices from local clusters when all remotes are not found.

There are existing issues about it: #115262 and #115872

Today this flag is not exposed via FieldCapabilitiesRequest so I am not sure how this could be fixed.

Depends on #134284

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

idegtiarenko · 2025-09-17T13:31:33Z

...erTest/java/org/elasticsearch/xpack/esql/action/CrossClusterQueriesWithInvalidLicenseIT.java

+            VerificationException.class,
+            containsString("Unknown index [xremote*:events]"),
+            () -> runQuery("FROM xremote*:events | STATS count(*)", requestIncludeMeta).close()
+        );


Prior behavior returned empty result for empty index resolution.
Now this is complaining about empty index resolution (consistent with FROM empty-pattern-* case)

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

idegtiarenko · 2025-09-24T11:47:17Z

...server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RequestIndexFilteringTestCase.java

        assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
        assertThat(e.getMessage(), containsString("verification_exception"));
-        assertThat(e.getMessage(), anyOf(containsString("Unknown index [foo]"), containsString("Unknown index [remote_cluster:foo]")));
+        assertThat(e.getMessage(), anyOf(containsString("Unknown index [foo]"), containsString("Unknown index [*:foo]")));


We randomly query foo or *:foo. Prior message was actually resolving * to all known remotes. I do not think this is desired behavior.

Okay, we need to keep old resolved remote_cluster:foo in case old cluster is coordinating the query.
I will check if we can make this check version dependent.

idegtiarenko · 2025-09-24T11:48:04Z

...src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityEsqlIT.java

-            );
-            Set<String> expectedUnknownIndexes = org.elasticsearch.common.Strings.commaDelimitedListToSet(expectedIndexExpressionInError);
-            assertThat(actualUnknownIndexes, equalTo(expectedUnknownIndexes));
+            assertThat(error.getMessage(), containsString("Unknown index [" + index + "]"));


Match original query pattern, not semi resolved pattern

elasticsearchmachine · 2025-09-24T14:10:57Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/PreAnalyzer.java

smalyshev · 2025-09-30T18:20:29Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

        if (preAnalysis.indexPattern() != null) {
-            String indexExpressionToResolve = EsqlCCSUtils.createIndexExpressionFromAvailableClusters(executionInfo);
-            if (indexExpressionToResolve.isEmpty()) {
+            if (executionInfo.clusterAliases().isEmpty()) {


I think we're back to the same question of whether this is right or not when we call it the second time. Is this intentional to have it here, or it's just sneaked in from merging the previous patch?

Related to #135353 (comment)

OK, I see! But we are calling preAnalyzeMainIndices twice - once at the beginning (now) and once if the filtered analysis fails. Is that also true for the second invocation? In any case, I'd add a comment to it - with this change, the existing comment is no longer true, and would be just confusing.

There are several aspects to this question:

today this behaves different from local only resolution: local does not allow empty resolution. We would like to change it to alight with DSL behavior. Once that is done this check should actually go away.

with respect for retrying analysis without filter: this would retry the same expression (even if some clusters should be skipped resulting in duplicated connection errors). I feel like this falls into ES-12978 issue category. With flat expression it is very tricky to manipulate input to exclude those clusters. Since we are going to be restarting analysis from scratch I would rather clear errors and start analysis fresh with no filter.

All considered, IMHO this seems reasonable.

I feel like this falls into ES-12978

ES-12978 is for CPS, but I guess we have the same problem with CCS now.
My assumption is that we'll address this problem before 9.3 FF.

Since we are going to be restarting analysis from scratch I would rather clear errors and start analysis fresh

++

luigidellaquila

I think this is good, as long as we agree on the point raised by Stas (ie. ES-12978)

luigidellaquila · 2025-10-03T09:43:13Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

        if (preAnalysis.indexPattern() != null) {
-            String indexExpressionToResolve = EsqlCCSUtils.createIndexExpressionFromAvailableClusters(executionInfo);
-            if (indexExpressionToResolve.isEmpty()) {
+            if (executionInfo.clusterAliases().isEmpty()) {


All considered, IMHO this seems reasonable.

I feel like this falls into ES-12978

ES-12978 is for CPS, but I guess we have the same problem with CCS now.
My assumption is that we'll address this problem before 9.3 FF.

Since we are going to be restarting analysis from scratch I would rather clear errors and start analysis fresh

++

idegtiarenko · 2025-10-07T06:41:03Z

Replaced with #136009

resolve original index pattern

e0f5989

idegtiarenko added >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.2.0 labels Sep 5, 2025

idegtiarenko commented Sep 5, 2025

View reviewed changes

idegtiarenko added 5 commits September 8, 2025 09:53

Merge branch 'main' into es-12487

9e56546

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

fix csv tests

e172c13

Merge branch 'main' into es-12487

62cb8e9

Merge branch 'main' into es-12487

d653020

returnLocalAll=false

21ccd63

idegtiarenko commented Sep 17, 2025

View reviewed changes

idegtiarenko added 4 commits September 17, 2025 16:13

move execution info updates

24b7a4c

Merge branch 'main' into es-12487

b1e75f4

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

resolve using original index pattern

3029bfb

assert original index expression in error

58c8230

idegtiarenko commented Sep 24, 2025

View reviewed changes

idegtiarenko requested review from luigidellaquila and smalyshev September 24, 2025 11:48

elasticsearchmachine and others added 3 commits September 24, 2025 11:52

[CI] Update transport version definitions

ee777a4

upd

06bfcfa

Merge branch 'main' into es-12487

3c9a40b

idegtiarenko marked this pull request as ready for review September 24, 2025 14:10

idegtiarenko added 4 commits September 24, 2025 16:47

fix flakiness

cf31115

Merge branch 'main' into es-12487

e9a0e29

Merge branch 'main' into es-12487

880f169

Merge branch 'main' into es-12487

444217d

# Conflicts: # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/PreAnalyzer.java

smalyshev reviewed Sep 30, 2025

View reviewed changes

idegtiarenko requested a review from smalyshev October 1, 2025 13:31

idegtiarenko added 2 commits October 1, 2025 15:36

update comment

e5254ec

Merge branch 'main' into es-12487

685830f

elasticsearchmachine added v9.3.0 and removed v9.2.0 labels Oct 2, 2025

luigidellaquila approved these changes Oct 3, 2025

View reviewed changes

Merge branch 'main' into es-12487

adaa593

idegtiarenko mentioned this pull request Oct 6, 2025

Resolve indices using original index pattern and retry the entire resolution #136009

Merged

idegtiarenko closed this Oct 7, 2025

idegtiarenko deleted the es-12487 branch October 7, 2025 06:41

	final Map<String, OriginalIndices> remoteClusterIndices = transportService.getRemoteClusterService()
	.groupIndices(request.indicesOptions(), request.indices());

Resolve indices using original index pattern #134218

Resolve indices using original index pattern #134218

Conversation

idegtiarenko commented Sep 5, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Sep 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luigidellaquila left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

idegtiarenko commented Oct 7, 2025

Uh oh!

Uh oh!