resolve remotes with field caps call

[idegtiarenko]

This change updates enriches to resolve remotes using dedicated field caps rather than executionInfo.getClusters().keySet().
This is required in order to be able to resolve flat index expressions.

Related to: ES-12612

[elasticsearchmachine]

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

[luigidellaquila]

Thanks @idegtiarenko, it looks good in general.

I left just a couple of comments regarding some corner cases (maybe a bit paranoid)

[luigidellaquila]

If the index pattern also contains a cluster/project prefix, will this still be correct?

eg.

FROM remote1:idx1,remote2:idx2
| STATS max(a) by b
| ENRICH policy on a, b

The enrich can only be calculated on the coordinator (after the STATS reduction), but I guess the enrich index on the coordinator won't be resolved. I'm not sure it works at all today tbh, maybe worth checking.

[idegtiarenko]

If the index pattern also contains a cluster/project prefix, will this still be correct?

Yes, this is a valid situation, where all supplied indices are correct specific indices (not aliases nor patterns) and exist.
In such cases we would resolve to exact same 2 identifiers and would derive 2 remotes from them: remote1 and remote2

[luigidellaquila]

Before merging, I'd just prefer to have a test for this query and make sure we are not introducing a regression.
Apart from that, I think we can proceed.

[luigidellaquila]

For LOOKUP JOINs we have a specific check: we just don't allow it.

I'm checking ENRICH now, but I couldn't find similar validation for it in the codebase, so I guess it's being treated differently...

[idegtiarenko]

I believe it is behaving the same way. It looks like we have a similar test for enrich:

elasticsearch/x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/CrossClusterEnrichIT.java

Lines 398 to 421 in 37b28e4

	public void testAggThenEnrichRemote() {
	String query = String.format(Locale.ROOT, """
	FROM *:events,events
	| eval ip= TO_STR(host)
	| %s
	| stats c = COUNT(*) by os
	| %s
	| sort vendor
	""", enrichHosts(Enrich.Mode.ANY), enrichVendors(Enrich.Mode.REMOTE));
	var error = expectThrows(VerificationException.class, () -> runQuery(query, randomBoolean()).close());
	assertThat(error.getMessage(), containsString("ENRICH with remote policy can't be executed after [stats c = COUNT(*) by os]@4:3"));
	}
	public void testEnrichCoordinatorThenEnrichRemote() {
	String query = String.format(Locale.ROOT, """
	FROM *:events,events
	| eval ip= TO_STR(host)
	| %s
	| %s
	| sort vendor
	""", enrichHosts(Enrich.Mode.COORDINATOR), enrichVendors(Enrich.Mode.REMOTE));
	var error = expectThrows(VerificationException.class, () -> runQuery(query, randomBoolean()).close());
	assertThat(error.getMessage(), containsString("ENRICH with remote policy can't be executed after [ENRICH _COORDINATOR"));
	}

[luigidellaquila]

@idegtiarenko I added a test in main for this case, see #134199, and it seems to work fine.
I think it makes sense to run it on this PR and see if the results are the same.

[luigidellaquila]

This is OK for now, but it will change when we start supporting subqueries and proper JOINs.

I'm not sure how it will actually work, maybe the enriches in a subquery will have to consider the remotes from the main indices in the subquery itself.

Anyway, I'd add a // TODO here, so that we don't forget.

[luigidellaquila]

Nit: the error message is a bit misleading, it took me a couple of seconds to realize that we were referring to multiple FROM, not to multiple indices. I know it comes from a similar check in EsqlSession, probably we should change both.

[idegtiarenko]

I copied it from

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

Line 627 in 938aa13

listener.onFailure(new MappingException("Queries with multiple indices are not supported"));

but that is a good call.

This is OK for now, but it will change when we start supporting subqueries and proper JOINs.

I suspect we would still need a distinction between main/top-level from and all the nested ones.
I will think about a way to improve it (across all usages)

[luigidellaquila]

If I understand it correctly, latest CI run (that is green) includes my tests.
All good then 🎉

[smalyshev]

lgtm, the only request is maybe add some comments on what's going on there especially in enrich resolver. I worked on that code and it still took me some time to realize what's going on, it may be tough on somebody who has not as familiar with the code.

[smalyshev]

I wonder though what happens here if one of the clusters fails. Will it fail this listener and thus the whole request? In other places, we have to do some handling to figure out what failed and skip failed clusters and so on, but we don't do it here. I'm not sure what happens here if one of the clusters fails?

[smalyshev]

What I am afraid of is if EsqlResolveFieldsAction returns partial response with say cluster A failing and thus having no indices but other clusters having indices, then A would be excluded from the list but not marked as skipped, and then there could be problems with it downstream.

[idegtiarenko]

This is going to be replaced with #134290