ESQL: Fix enrich and lookup join resolution based on min transport version #137431
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Collaborator
|
Hi @alex-spies, I've created a changelog YAML for you. |
alex-spies
added
auto-backport
…ution-min-transport-version
elasticsearchmachine
added
the
Team:Analytics
Collaborator
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
idegtiarenko
reviewed
Nov 3, 2025
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java
Outdated
Show resolved
Hide resolved
idegtiarenko
reviewed
Nov 3, 2025
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java
Outdated
Show resolved
Hide resolved
idegtiarenko
reviewed
Nov 3, 2025
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java
Show resolved
Hide resolved
idegtiarenko
reviewed
Nov 3, 2025
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java
Outdated
Show resolved
Hide resolved
idegtiarenko
approved these changes
Nov 28, 2025
Contributor
Author
|
Thanks for the review and your very good suggestion for simplification by using the cluster state, @idegtiarenko ! |
And fix expectations for the determined min cluster version in non-CCS tests.
…ution-min-transport-version
Contributor
Author
|
Ok, CI is green except for 2 unrelated release tests that are failing. These are
This is safe to merge. |
Collaborator
💔 Backport failed
You can use sqren/backport to manually backport by running |
Contributor
Author
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
alex-spies
added a commit
to alex-spies/elasticsearch
that referenced
this pull request
Dec 2, 2025
…rsion (elastic#137431) When deciding which types are supported, we did not use the correct minimum transport version during the enrich resolution in case of CCS and ROW queries. What's more, the EnrichPolicyResolver did not account for the fact that the node requesting resolution might be on a version that doesn't support the types in the resolved mapping, which led to serialization bugs surfacing when trying to enable the DATE_RANGE type. - Initialize the minimum transport version with the minimum version from the cluster state before any resolution steps. That makes ROW queries correct. - Send the determined minimum transport version along the enrich resolution request so that remote clusters don't send un-deserializable data types back. - Add the determined minimum transport version to the profile. - Add a bunch of tests. (cherry picked from commit 4a14f83) # Conflicts: # server/src/main/resources/transport/upper_bounds/9.3.csv # x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/PushExpressionToLoadIT.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichPolicyResolver.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java
alex-spies
added a commit
that referenced
this pull request
Dec 3, 2025
…ort version (#137431) (#138893) * ESQL: Fix enrich and lookup join resolution based on min transport version (#137431) When deciding which types are supported, we did not use the correct minimum transport version during the enrich resolution in case of CCS and ROW queries. What's more, the EnrichPolicyResolver did not account for the fact that the node requesting resolution might be on a version that doesn't support the types in the resolved mapping, which led to serialization bugs surfacing when trying to enable the DATE_RANGE type. - Initialize the minimum transport version with the minimum version from the cluster state before any resolution steps. That makes ROW queries correct. - Send the determined minimum transport version along the enrich resolution request so that remote clusters don't send un-deserializable data types back. - Add the determined minimum transport version to the profile. - Add a bunch of tests. (cherry picked from commit 4a14f83) # Conflicts: # server/src/main/resources/transport/upper_bounds/9.3.csv # x-pack/plugin/esql/qa/server/single-node/src/javaRestTest/java/org/elasticsearch/xpack/esql/qa/single_node/PushExpressionToLoadIT.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichPolicyResolver.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java # x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/IndexResolver.java * Align PreAnalysisResult with main Make pre-initialization of minimumTransportVersion consistent with main. * Use min version from the correct field caps In 9.2, this was in the forked field caps response, not the original field caps response. --------- Co-authored-by: elasticsearchmachine <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Relates #137534
When deciding which types are supported, we did not use the correct minimum transport version during the enrich resolution step. What's more, the
EnrichPolicyResolverdid not account for the fact that the node requesting resolution might be on a version that doesn't support the types in the resolved mapping, which led to serialization bugs surfacing when trying to enable theDATE_RANGEtype. We had inconsistent minimum transport version in case ofROW | ENRICH/ROW | LOOKUP JOINqueries andFROM remote:* | ENRICHqueries.For
ENRICH, this is a non-issue in practice, because the only affected data types wereAGGREGATE_METRIC_DOUBLEandDENSE_VECTOR, which cannot yet be used in enrich policies to begin with. (See #137699 and #127350).ROW | LOOKUP JOINqueries were broken (or at least potentially inconsistent) because a new coordinator might have assumed that the two new data types are supported, while an old lookup node doesn't actually support them.For both, enrich and lookup resolution, retrieve the determined minimum version and use it on the coordinator. This is important forROW | ENRICHandROW | LOOKUP JOINqueries.ROW | ENRICHandROW | LOOKUP JOINqueries don't just use the coordinator node's version (which may be newer than the version of a node executing enrich/lookup joins for us).FROM remote_only:*FROM | ENRICH, both for queries on the local cluster and in case of CCS.FROM | LOOKUP JOINon the local cluster + with CCS.