Always error out if CCS expression shows up when CCS is not supported by smalyshev · Pull Request #139009 · elastic/elasticsearch

smalyshev · 2025-12-03T20:07:32Z

This patch decouples ignore_unavailable from "CCS not supported" error when trying to resolve index expression. IndexNameExpressionResolver can not resolve remote expressions, and for most code paths, it is the right thing to return an error when such expression is supplied. For some code paths, it may be OK to ignore such expressions, at least for now, but in either case, ignore_unavailable should have nothing to do with it.

This patch tries to preserve the functionality in most places which misuse IndexNameExpressionResolver by trying to sent it remote indices, but we'd need to go back and fix those eventually.

Closes: #138987

elasticsearchmachine · 2025-12-03T20:08:19Z

Hi @smalyshev, I've created a changelog YAML for you.

elasticsearchmachine · 2025-12-09T23:27:18Z

Pinging @elastic/es-search-foundations (Team:Search Foundations)

elasticsearchmachine · 2025-12-09T23:27:18Z

Hi @smalyshev, I've updated the changelog YAML for you.

smalyshev · 2025-12-15T19:53:09Z

Almost every API is using the IndexNameExpressionResolver. So, this has the potential of breaking a lot of requests that in the past would have returned a partial response. Do you understand my concern?

I totally understand your concern. You can use this API in a variety of ways though. The thing is, if you pass a remote index to it, it is supposed to throw. And it will never resolve it properly. The latter has always been the case, so if some API uses it and expects it to resolve remote indices, it is already broken. Fortunately, I didn't and the tests do not reveal any cases where it really happens - except maybe indices_boost case where it's marginal (and it is currently broken with remote indices, unfortunately). So, I think other APIs use it in a way that it was supposed to be used - to resolve only local indices. Except some APIs that I am fixing in this pull, which were kind of sloppy doing this, and relied on just shutting up the errors and not reporting them.

Now, can there be more corner cases where other APIs are relying on the same thing and aren't covered by tests? There could be. But that means they are already broken now, and the only way we can detect it (instead of users just silently getting wrong results in their responses) is to provide an identifiable error where it happens, and then when this error pops up, fix it. Short of reimplementing IndexNameExpressionResolver to actually feature full support for remote indices, which will be a considerable lift, I see no other way of handling it.

gmarouli · 2025-12-16T12:05:28Z

Now, can there be more corner cases where other APIs are relying on the same thing and aren't covered by tests? There could be. But that means they are already broken now, and the only way we can detect it (instead of users just silently getting wrong results in their responses) is to provide an identifiable error where it happens, and then when this error pops up, fix it.

I am not saying that this "ignoring of remote indices" behaviour was tested and documented, but it is how it has been working and in the code it looked like a conscious choice. We can revise that choice (fwiw I also find it cleaner to consider this behaviour a bug).

However, because it is working like this now, and it is possible that users might have remote expressions that have flew under the radar because of ignore_unavailable: true, we should consider the trade-offs before proceeding, that's all.

This reverts commit 2977d97.

quux00 · 2025-12-17T20:20:53Z

I am not saying that this "ignoring of remote indices" behaviour was tested and documented, but it is how it has been working and in the code it looked like a conscious choice. We can revise that choice (fwiw I also find it cleaner to consider this behaviour a bug).

However, because it is working like this now, and it is possible that users might have remote expressions that have flew under the radar because of ignore_unavailable: true, we should consider the trade-offs before proceeding, that's all.

@gmarouli - I created the issue for this as a bug, not a breaking change, after discussing it with Najwa and Jason Tedor. Given the surface area of the change that you and Stas are highlighting here, I spoke with Jason again today about it and our question is - is there a usage from a user that wouldn't be considered incorrect behavior to start with?

For example, are all the cases where users would now start getting an error (with this code change) ones where they are referencing a remote index in an endpoint that is not cross-cluster enabled, such as GET logs,remote1:logs/_mapping? If yes, and they added ignore_unavailable=true just to bypass an error, that is incorrect usage to start with, so we should address it both on the backend and the client side (adjust their queries). Or is there a case you know of where the user is correctly using an API and might now get an error with the changes in this PR?

elasticsearchmachine · 2025-12-17T23:20:20Z

Hi @smalyshev, I've updated the changelog YAML for you.

gmarouli · 2025-12-18T08:28:23Z

I spoke with Jason again today about it and our question is - is there a usage from a user that wouldn't be considered incorrect behaviour to start with?

I do not know why this was implemented like this and there were no documented use cases in the code as far as I know. I do not know what we have communicated to the users about this either. So, my answer is I do not know, and I do not have someone in mind to redirect you to.

is there a case you know of where the user is correctly using an API and might now get an error with the changes in this PR

I doubt this, but I cannot say it with certainty. In the past when fixing a bug that would change a silent fail to an error, we considered the element of surprise to some users, and that could influence the choice of fixing the bug or letting it be. This is what I am asking, to double check that the benefit of fixing this behaviour outweighs the possibility of surprising some users.

gmarouli

Apologies for the late code review, I thought I had submitted it 😶‍🌫️

server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java

gmarouli · 2025-12-16T11:38:08Z

server/src/main/java/org/elasticsearch/search/SearchFeatures.java

    public static final NodeFeature SEARCH_WITH_NO_DIMENSIONS_BUGFIX = new NodeFeature("search.vectors.no_dimensions_bugfix");
    public static final NodeFeature SEARCH_RESCORE_SCRIPT = new NodeFeature("search.rescore.script");
    public static final NodeFeature NEGATIVE_FUNCTION_SCORE_BAD_REQUEST = new NodeFeature("search.negative.function.score.bad.request");
+    public static final NodeFeature INDICES_BOOST_REMOTE_INDEX_FIX = new NodeFeature("search.indices_boost_remote_index_fix");


All the APIs that use index resolution? Or can use index boosts?

I was referring to the APIs that use index resolution but their behaviour changes with this.

That could be a lot of them, and I am not sure how to practically do that.

Considering that this is only relevant for testing, so I would add it to the APIs you are testing, in this case the mapping, the index boosts and any other you choose to test.

Unless I am mistaken API capabilities were the recommended way to capture API (and behaviour) changes. Do you think that's not the case?

rest-api-spec/build.gradle

quux00 · 2026-01-05T20:17:42Z

x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/index/IndexResolver.java


+    /**
+     * Filter out remote index expressions.
+     * TODO: SQL Metadata commands currently do not support remote indices.


Do we need to handle this in this PR? If not, how urgent is the follow-up?

The tests in x-pack/plugin/sql/qa/server/src/main/resources/multi-cluster-with-security/multi-cluster-command-sys.csv-spec do not support remote aliases, so I don't think we need to fix it here. As for fixing it in general, I think we need to ask the Analytics team, but given as it has been there for a while and nobody complained, it's probably not very high priority.

quux00

LGTM. Let's go ahead with merging.

* upstream/main: (191 commits) Overall Decision for Deciders prioritizes THROTTLE (elastic#140237) Apply group by all logic not only to top-level aggregates (elastic#140248) [ES|QL] Refactor MV_UNION and MV_INTERSECTION to use shared set operation helper (elastic#139982) Avoid reading entire bloom filter file on reader open (elastic#139374) Mark bloom filter files for random access (elastic#139375) Ensure that the buffer used for ES93BloomFilterStoredFieldsFormat is zeroed (elastic#139034) Add busy assertion to avoid race condition for testStalledShardMigrationProperlyDetected (elastic#140230) Remove line number check for testTransitiveFindsDeepCallChain (elastic#140228) Allow a slight difference in rescored docs (elastic#139931) Mute org.elasticsearch.xpack.inference.integration.AuthorizationTaskExecutorIT testCreatesEisChatCompletion_DoesNotRemoveEndpointWhenNoLongerAuthorized elastic#138480 Start exchange sink fetchers concurrently (elastic#140196) Allow allocation to replacement target node on vacate completion (elastic#140150) Ignore JNA cleaner threads in SecureHdfsRepositoryAnalysisRestIT (elastic#139925) DeterministicQueue refactor and enhancement (elastic#140151) Always error out if CCS expression shows up when CCS is not supported (elastic#139009) Use IllegalArgumentException over RepositoryException for readonly-repository checks (elastic#140200) Guard promql capabilities in AnalyzerTests (elastic#140232) [Inference API] Fix flaky AuthorizationTaskExecutorIT tests (elastic#139978) Cleaning up exitable vector value impls (elastic#140190) [Inference API] Fix auth exception listener not called bug (elastic#139966) ...

…elastic#139009) * Always error out if CCS expression shows up when CCS is not supported

Always error out if CCS expression shows up when CCS is not supported

1cfd2ca

smalyshev added >bug :Search Foundations/CCS labels Dec 3, 2025

elasticsearchmachine added the v9.3.0 label Dec 3, 2025

Update docs/changelog/139009.yaml

0cc995c

smalyshev and others added 16 commits December 3, 2025 15:17

Better fix with context option

3c38e78

bug ref

c1944c2

Reduce ctor explosion

5664342

improve comment

ea87ec0

fix tests

266048a

Merge branch 'main' into ccs-not-supported-fix

f1f63dc

Fix semantic query tests

e177894

more tests

734f118

fix test

9217de3

disable bwc

711f73f

temp fix for SQL

9709893

More test fixes

0bbf320

Fix test

a1bfc7a

fix test

a7e0a17

Merge branch 'main' into ccs-not-supported-fix

4ee6cbe

Update comments

e83781e

smalyshev marked this pull request as ready for review December 9, 2025 23:26

elasticsearchmachine added the Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch label Dec 9, 2025

Update docs/changelog/139009.yaml

2977d97

smalyshev requested a review from quux00 December 9, 2025 23:37

smalyshev and others added 2 commits December 9, 2025 16:52

add test

aba5bea

Merge branch 'main' into ccs-not-supported-fix

d92f41b

smalyshev added 2 commits December 16, 2025 10:40

Revert "Update docs/changelog/139009.yaml"

225e8a9

This reverts commit 2977d97.

Merge branch 'main' into ccs-not-supported-fix

9b7867e

smalyshev requested a review from gmarouli December 17, 2025 20:31

elasticsearchmachine added v9.4.0 and removed v9.3.0 labels Dec 17, 2025

Update docs/changelog/139009.yaml

30dea66

gmarouli reviewed Dec 18, 2025

View reviewed changes

smalyshev and others added 4 commits December 23, 2025 15:30

Merge branch 'main' into ccs-not-supported-fix

fe336ed

Maybe this is not needed

589d807

Merge branch 'main' into ccs-not-supported-fix

b661188

Update 139009.yaml

83d2c82

smalyshev requested review from gmarouli and quux00 January 5, 2026 16:48

smalyshev and others added 4 commits January 5, 2026 09:48

Merge branch 'main' into ccs-not-supported-fix

e8a6a0e

rename method to emphasize it ignores remote expressions

f9965a8

fix comments

92396d7

ws

a577e26

quux00 reviewed Jan 5, 2026

View reviewed changes

quux00 approved these changes Jan 6, 2026

View reviewed changes

Merge branch 'main' into ccs-not-supported-fix

2c9bc9b

smalyshev enabled auto-merge (squash) January 6, 2026 22:30

smalyshev merged commit 628906c into elastic:main Jan 6, 2026
35 checks passed

smalyshev deleted the ccs-not-supported-fix branch January 6, 2026 23:47

sidosera pushed a commit to sidosera/elasticsearch that referenced this pull request Jan 7, 2026

Always error out if CCS expression shows up when CCS is not supported (…

c341d66

…elastic#139009) * Always error out if CCS expression shows up when CCS is not supported

Comments

Conversation

smalyshev commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Dec 3, 2025

Uh oh!

elasticsearchmachine commented Dec 9, 2025

Uh oh!

elasticsearchmachine commented Dec 9, 2025

Uh oh!

smalyshev commented Dec 15, 2025

Uh oh!

gmarouli commented Dec 16, 2025

Uh oh!

quux00 commented Dec 17, 2025

Uh oh!

elasticsearchmachine commented Dec 17, 2025

Uh oh!

gmarouli commented Dec 18, 2025

Uh oh!

gmarouli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gmarouli Dec 16, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

quux00 Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

smalyshev Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

quux00 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

smalyshev commented Dec 3, 2025 •

edited

Loading