QL: Preserve subfields for invalid types

[costin]

In certain scenarios, a field can be mapped both as a primitive and
object, causing it to be marked as unsupported, losing any potential
subfields that might have been discovered before.
This commit preserve them to avoid subfields from being incorrectly
reported as missing.

Fix #100869

[elasticsearchmachine]

Pinging @elastic/es-ql (Team:QL)

[elasticsearchmachine]

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

[elasticsearchmachine]

Pinging @elastic/elasticsearch-esql (:Query Languages/ES|QL)

[luigidellaquila]

LGTM!

[astefan]

This changes a fundamental behavior in IndexResolver, more specifically: #50823. We decided back then, according to PR's description, that all sub-fields of an unsupported root field to be unsupported as well.

I don't remember exactly why we did this, the code changes there start from the new (back then) "flattened" data type, but the hierarchy of an unsupported root field being marked as unsupported as well, was discovered/discussed/decided in the middle of the PR development.

Please, note one breaking (?) change this PR introduces in SQL, I haven't looked as EQL but likely it's the same:

• before this PR, query select file.name from "test*" would have complained about file.name not existing in the merged mappings
• with this PR, the query succeeds and returns the values it finds.

Digging deeper into this issue, I strongly believe the fact that in one case we report the field as missing (from test* | where file.name is not null) but not in an apparently almost identical query from the verification pov (from test* | where file.name is not null | stats count(*)) is the result of #98987. With that PR in place we don't have visibility in the upper hierarchy of file.name unless file is specifically asked in the query. Because of this we are required to re-build the hierarchy from bits and pieces. Without that PR, we get back in field_caps response the full list of file fields and we can decide what's the real (aka useful) hierarchy for file.name.

There are likely many implications/unknowns/concerns here; naming two that I clearly see:

• I don't think this solution should cover SQL, only EQL and ESQL. The reason is that QL: EQL and ESQL to use only the necessary fields in the internal field_caps calls #98987 does affect only these two projects. And the workaround in this PR is a direct consequence of QL: EQL and ESQL to use only the necessary fields in the internal field_caps calls #98987.
• I don't know (yet) what was the original reasoning behind marking the entire hierarchy as unsupported for an unsupported root field, but this is definitely not a bug. The fact that we consider it as such is strictly related to ESQL's very particular demands when it comes to "what to display if fields are unsupported"

[bpintea]

The change to expose the subfields LGTM, but wondering about the considerations when choosing this solution. In the case of #100869, there are three options available:

• discard everything on any conflict (existing solution, with issues).
• keep "top" field, if this isn't an object (i.e. file field) and drop subfields (file.name).
• drop top field (even though not an object) and keep the non-conflicting subfields.

Not sure what the expectation would be in this case. So we can prescribe a solution here, but we're making a choice which might not be always desirable.

Generally, I believe the 1st option would be the most consistent, but practically, the 3rd might be the most practical (even if it seems a bit arbitrary in the case of #100869).

[costin]

I have updated the PR taking @astefan suggestion - for EQL and SQL the previous behavior is in place, ESQL however changes it and keeps the old properties.

A clarification - this change affects invalid fields not unsupported ones; I used the incorrect terminology in my PR which might have caused confusion. Hierarchies of Unsupported fields are still supported but marked as unsupported so fields do get reported but don't disappear (as is the current case).

@bpintea the current solution keeps all the fields and will allow queries that work on non-conflicting/invalid fields to work while those that touch on invalid ones to report an error (don't use field x, is invalid).
I expect this issue to be a focus for us moving forward in providing means to solve such conflicts at runtime through dedicated functions/commands the user can opt into.

[elasticsearchmachine]

💚 Backport successful

Status	Branch	Result
✅	8.11