Skip to content

Conversation

@idegtiarenko
Copy link
Contributor

@idegtiarenko idegtiarenko commented Oct 6, 2025

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 (for example the list of remotes is no longer known beforehand). It should be delegated to field caps call using original expression. This also changes how resolution is retried. It is no longer trivial to exclude clusters with failures. Additionally flat expression could resolve to different set of remotes without filters. As a result we need to retry resolving dependencies such as enrich and lookup joins as well as we need to ensure they are present on a wider set of remotes.

Resolves: ES-12487, ES-12978

@idegtiarenko idegtiarenko added >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.3.0 labels Oct 6, 2025
syncEsqlQueryRequest().query("FROM data,*:data | LOOKUP JOIN lookup ON key | WHERE f1 == 1")
.filter(new TermQueryBuilder("f2", 2))
)
);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test would fail if we only retry previously resolved subset of clusters opposed to ones resolved to all clusters without the filter.

@idegtiarenko idegtiarenko marked this pull request as ready for review October 6, 2025 13:34
@elasticsearchmachine
Copy link
Collaborator

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

@idegtiarenko idegtiarenko changed the title Retry the entire resolution Resolve indices using original index pattern and retry the entire resolution Oct 7, 2025
Copy link
Contributor

@luigidellaquila luigidellaquila left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as #134218, IMHO this is good

Thanks!

}
}));
// retrying the index resolution without index filtering.
executionInfo.clusterInfo.clear();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@smalyshev smalyshev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine. I am not 100% happy with the fact that we do other resolutions twice in the case of filters, but I hope it'll go away when we fix field caps, so for now we can live with it I think.

VerificationException.class,
containsString("lookup index [lookup] is not available in remote cluster [remote-b]"),
() -> runQuery(
syncEsqlQueryRequest().query("FROM data,*:data | LOOKUP JOIN lookup ON key | WHERE f1 == 1")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reminder: please add the other use case without join we've discussed.

@idegtiarenko idegtiarenko merged commit 0b3d23a into elastic:main Oct 8, 2025
34 checks passed
@idegtiarenko idegtiarenko deleted the retry_complete_resolution branch October 8, 2025 08:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants