Skip to content
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

Fix ClassCastException in Significant Terms #108429

Merged
merged 4 commits into from
May 9, 2024

Conversation

not-napoleon
Copy link
Member

Resolves #108427

Prior to this PR, if a SignificantTerms aggregation targeted a field existing on two indices (that were included in the aggregation) but mapped to different field types, the query would fail at reduce time with a somewhat obscure ClassCastException. This change brings the behavior in line with the Terms aggregation, which returns a 400 class IllegalArgumentException with a useful message in this situation.

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 8, 2024
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine
Copy link
Collaborator

Hi @not-napoleon, I've created a changelog YAML for you.

@Override
public void accept(InternalAggregation aggregation) {
@SuppressWarnings("unchecked")
final InternalSignificantTerms<A, B> terms = (InternalSignificantTerms<A, B>) aggregation;
if (referenceAgg == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You need to skip Unmapped fields. Adding at the beginning should be enough:

             if (aggregation.canLeadReduction() == false) {
                    return;
                }

@not-napoleon not-napoleon merged commit 9f438ed into elastic:main May 9, 2024
15 checks passed
markjhoy pushed a commit to markjhoy/elasticsearch that referenced this pull request May 9, 2024
Prior to this PR, if a SignificantTerms aggregation targeted a field existing on two indices (that were included in the aggregation) but mapped to different field types, the query would fail at reduce time with a somewhat obscure ClassCastException. This change brings the behavior in line with the Terms aggregation, which returns a 400 class IllegalArgumentException with a useful message in this situation.

Resolves elastic#108427
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Aggregations Aggregations >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.15.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClassCastException in SignificantTermsAggregation
3 participants