Skip to content

Fix union types in CCS #128111

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 20, 2025
Merged

Fix union types in CCS #128111

merged 5 commits into from
May 20, 2025

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented May 17, 2025

Currently, union types in CCS is broken. For example, FROM *:remote-indices | EVAL port = TO_INT(port) returns all nulls if the types of the port field conflict. This happens because converters are a map of the fully qualified cluster:index -name (defined in MultiTypeEsField), but we are looking up the converter using only the index name, which leads to a wrong or missing converter on remote clusters. Our tests didn't catch this because MultiClusterSpecIT generates the same index for both clusters, allowing the local converter to be used for remote indices.

@elasticsearchmachine
Copy link
Collaborator

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

@dnhatn dnhatn force-pushed the fix-ccs-multi-types branch from 0733fc0 to 954e580 Compare May 17, 2025 01:05
@elasticsearchmachine
Copy link
Collaborator

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

@dnhatn dnhatn requested review from craigtaverner and nik9000 May 17, 2025 01:25
@dnhatn dnhatn marked this pull request as ready for review May 17, 2025 01:25
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 17, 2025
@@ -157,7 +157,8 @@ private BlockLoader getBlockLoaderFor(int shardId, Attribute attr, MappedFieldTy
BlockLoader blockLoader = shardContext.blockLoader(getFieldName(attr), isUnsupported, fieldExtractPreference);
MultiTypeEsField unionTypes = findUnionTypes(attr);
if (unionTypes != null) {
String indexName = shardContext.ctx.index().getName();
// Use the fully qualified name `cluster:index-name` because multiple types are resolved on coordinator with the cluster prefix
String indexName = shardContext.ctx.getFullyQualifiedIndex().getName();
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the fix.

@dnhatn dnhatn added auto-backport Automatically create backport pull requests when merged v8.19.0 and removed v8.17.7 labels May 20, 2025
@dnhatn
Copy link
Member Author

dnhatn commented May 20, 2025

Thanks Nik!

@dnhatn dnhatn merged commit c2561b5 into elastic:main May 20, 2025
17 checks passed
@dnhatn dnhatn deleted the fix-ccs-multi-types branch May 20, 2025 05:47
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request May 20, 2025
Currently, union types in CCS is broken. For example, FROM 
*:remote-indices | EVAL port = TO_INT(port) returns all nulls if the
types of the port field conflict. This happens because converters are a
map of the fully qualified cluster:index -name (defined in
MultiTypeEsField), but we are looking up the converter using only the
index name, which leads to a wrong or missing converter on remote
clusters. Our tests didn't catch this because MultiClusterSpecIT
generates the same index for both clusters, allowing the local converter
to be used for remote indices.
@elasticsearchmachine
Copy link
Collaborator

elasticsearchmachine commented May 20, 2025

Copy link
Contributor

@craigtaverner craigtaverner left a comment

Choose a reason for hiding this comment

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

Lovely fix! Just a one-word fix, and then a lot of testing!

@dnhatn
Copy link
Member Author

dnhatn commented May 20, 2025

@craigtaverner Thanks for looking :)

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request May 20, 2025
Currently, union types in CCS is broken. For example, FROM
*:remote-indices | EVAL port = TO_INT(port) returns all nulls if the
types of the port field conflict. This happens because converters are a
map of the fully qualified cluster:index -name (defined in
MultiTypeEsField), but we are looking up the converter using only the
index name, which leads to a wrong or missing converter on remote
clusters. Our tests didn't catch this because MultiClusterSpecIT
generates the same index for both clusters, allowing the local converter
to be used for remote indices.
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request May 20, 2025
Currently, union types in CCS is broken. For example, FROM
*:remote-indices | EVAL port = TO_INT(port) returns all nulls if the
types of the port field conflict. This happens because converters are a
map of the fully qualified cluster:index -name (defined in
MultiTypeEsField), but we are looking up the converter using only the
index name, which leads to a wrong or missing converter on remote
clusters. Our tests didn't catch this because MultiClusterSpecIT
generates the same index for both clusters, allowing the local converter
to be used for remote indices.
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request May 20, 2025
Currently, union types in CCS is broken. For example, FROM
*:remote-indices | EVAL port = TO_INT(port) returns all nulls if the
types of the port field conflict. This happens because converters are a
map of the fully qualified cluster:index -name (defined in
MultiTypeEsField), but we are looking up the converter using only the
index name, which leads to a wrong or missing converter on remote
clusters. Our tests didn't catch this because MultiClusterSpecIT
generates the same index for both clusters, allowing the local converter
to be used for remote indices.
elasticsearchmachine pushed a commit that referenced this pull request May 20, 2025
Currently, union types in CCS is broken. For example, FROM
*:remote-indices | EVAL port = TO_INT(port) returns all nulls if the
types of the port field conflict. This happens because converters are a
map of the fully qualified cluster:index -name (defined in
MultiTypeEsField), but we are looking up the converter using only the
index name, which leads to a wrong or missing converter on remote
clusters. Our tests didn't catch this because MultiClusterSpecIT
generates the same index for both clusters, allowing the local converter
to be used for remote indices.
elasticsearchmachine added a commit that referenced this pull request May 20, 2025
* Fix union types in CCS (#128111)

Currently, union types in CCS is broken. For example, FROM
*:remote-indices | EVAL port = TO_INT(port) returns all nulls if the
types of the port field conflict. This happens because converters are a
map of the fully qualified cluster:index -name (defined in
MultiTypeEsField), but we are looking up the converter using only the
index name, which leads to a wrong or missing converter on remote
clusters. Our tests didn't catch this because MultiClusterSpecIT
generates the same index for both clusters, allowing the local converter
to be used for remote indices.

* [CI] Auto commit changes from spotless

---------

Co-authored-by: elasticsearchmachine <[email protected]>
dnhatn added a commit that referenced this pull request May 20, 2025
Backport #128111 to 9.0

Currently, union types in CCS is broken. For example, FROM
*:remote-indices | EVAL port = TO_INT(port) returns all nulls if the
types of the port field conflict. This happens because converters are a
map of the fully qualified cluster:index -name (defined in
MultiTypeEsField), but we are looking up the converter using only the
index name, which leads to a wrong or missing converter on remote
clusters. Our tests didn't catch this because MultiClusterSpecIT
generates the same index for both clusters, allowing the local converter
to be used for remote indices.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.18.2 v8.19.0 v9.0.2 v9.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants