For empty mappings use a LocalRelation

[astefan]

Fixes #104809 by converting a plan to a local relation when there is no mapping for the index pattern.

[elasticsearchmachine]

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

[elasticsearchmachine]

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

[astefan]

@elasticsearchmachine run elasticsearch-ci/part-1

[astefan]

@elasticmachine run elasticsearch-ci/part-1

[astefan]

@elasticmachine update branch

[bpintea]

LGTM

[bpintea]

👍

[bpintea]

Why not part of the skip batch? It'd be worth a comment if there's a reason.

[costin]

Because this can be determine upfront, which facilitates the rest of the rules and folding of LocalRelation through-out the plan.
Skip is done at the end for cases where a normal relationship is changed due to filtering.

[bpintea]

It does make the other rules do less work, yes, but since there's a ONCE limiter, it might then have made more sense as a first rule. Anyways, minor.

[bpintea]

With all the similar changes to add a concrete index, wondering if EsIndex(String name, Map<String, EsField> mapping) c'tor, which is only used in tests, shouldn't be updated instead to add a set with the name.

[astefan]

I have mixed feelings about this. I know it's not a big deal, but if something is used only in tests, the method/constructor access modifier should reflect this. But this class is used in many different packages and the only possible access modifier is public. I would rather have the usage be explicit and the user of the constructor be aware of the implications of providing an empty Set or specific values in that Set, rather than use the constructor without being aware of its content.

[dnhatn]

Hello Andrei, I think this PR handles ESQL with an empty mapping rather than fixing #104809, where field-caps were reading from a not up-to-date shard. I have addressed that field-caps issue in #105153.

[costin]

LGTM

[astefan]

@dnhatn

Hello Andrei, I think this PR handles ESQL with an empty mapping rather than fixing #104809, where field-caps were reading from a not up-to-date shard. I have addressed that field-caps issue in #105153.

That's interesting, I wasn't aware of this scenario. Thank you for the awareness ping.
Nevertheless, the CI test caught an interesting edge case with how we deal with mappings coming from the IndexResolver; note that it is ok for the concreteIndices to be empty. This happens in two situations:

• a query has concrete fields in it (from test | where age > 18) but those fields cannot be found in the mappings (a typo maybe). In this case the Analyzer handles this scenario and returns an error message about a missing field
• there are no concrete fields in the query and still the concreteIndices is empty (because of an empty mapping). The Analyzer allows the query to pass , since there are no fields to check for existence. The correct approach is for the planner to skip creating an unnecessary query to Elasticsearch by marking the entire plan as a local plan. Which is correct: no mappings, no data, no need to go down to Lucene.