Fix ClassCastException in mixed rollup+raw avg aggregation reduce

[vaibhoag]

Description

When an avg aggregation is executed across a mix of rollup and raw indices under the same aggregation name, the Index Management (ISM) rollup plugin rewrites avg on the rollup shards into a scripted_metric that emits ScriptedAvg(sum, count), so those shards return an InternalScriptedMetric. The raw-index shards still return a plain InternalAvg.

At coordinator reduce, InternalScriptedMetric.reduce() unconditionally cast every per-shard aggregation to InternalScriptedMetric, throwing:

java.lang.ClassCastException: class org.opensearch.search.aggregations.metrics.InternalAvg cannot be cast to class org.opensearch.search.aggregations.metrics.InternalScriptedMetric

which surfaces to the user as an HTTP 500 and fails the entire query.

Fix: in the reduce() collection loop, handle the InternalAvg case by converting it into a ScriptedAvg(sum, count) — the same object the rollup combine step emits — so it is folded into the scripted-metric reduce script (sum += a.getSum(); count += a.getCount(); return sum/count) and the raw contribution is blended into the final average instead.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

• Before: mixed rollup+raw avg search → HTTP 500 ClassCastException.
• After: HTTP 200 with the correct blended average across all tiers.

Check List

• Functionality includes testing.
• API changes companion pull request created, if applicable.
• Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🧪 PR contains tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Tight Coupling / Layering Violation InternalScriptedMetric.reduce() now has explicit knowledge of InternalAvg and a ScriptedAvg type, which is a rollup-plugin-specific concept. Baking this special-case into a core aggregation class couples core code to a plugin's data model and is fragile: it only works if a reduce script exists that understands ScriptedAvg, and it does nothing for other metric types (sum, min, max, stats, etc.) that could hit the same mixed-shard scenario. This suggests the fix belongs in the rollup plugin (e.g., by wrapping raw-shard results into InternalScriptedMetric before reduce) rather than here. if (aggregation instanceof InternalScriptedMetric mapReduceAggregation) { aggregationObjects.addAll(mapReduceAggregation.aggregations); } else if (aggregation instanceof InternalAvg avg) { aggregationObjects.add(new ScriptedAvg(avg.getSum(), avg.getCount())); } Silent Drop of Unknown Types The new if/else-if chain omits an else branch, so any InternalAggregation subtype that is neither InternalScriptedMetric nor InternalAvg will be silently dropped from aggregationObjects instead of failing loudly. Previously an unexpected type produced a ClassCastException which at least surfaced the problem; now results may be silently incorrect. Consider throwing an explicit exception in the else branch. for (InternalAggregation aggregation : aggregations) { if (aggregation instanceof InternalScriptedMetric mapReduceAggregation) { aggregationObjects.addAll(mapReduceAggregation.aggregations); } else if (aggregation instanceof InternalAvg avg) { aggregationObjects.add(new ScriptedAvg(avg.getSum(), avg.getCount())); } } Potential ClassCastException Remains Line 108 still does ((InternalScriptedMetric) aggregations.get(0)). If the first shard's aggregation is an InternalAvg (order of shards is not guaranteed to place the scripted-metric first), this cast will throw the same ClassCastException the PR is trying to fix. The reduce should locate the first InternalScriptedMetric in the list rather than assuming index 0. InternalScriptedMetric firstAggregation = ((InternalScriptedMetric) aggregations.get(0));

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Avoid unsafe cast on first aggregation The code assumes aggregations.get(0) is an InternalScriptedMetric, but with the new mixed-type handling, the first element could be an InternalAvg, causing a ClassCastException. Find the first InternalScriptedMetric in the list to safely obtain reduceScript and metadata, or handle the case where none exists. server/src/main/java/org/opensearch/search/aggregations/metrics/InternalScriptedMetric.java [108] if (aggregation instanceof InternalScriptedMetric mapReduceAggregation) { aggregationObjects.addAll(mapReduceAggregation.aggregations); } else if (aggregation instanceof InternalAvg avg) { aggregationObjects.add(new ScriptedAvg(avg.getSum(), avg.getCount())); } } - InternalScriptedMetric firstAggregation = ((InternalScriptedMetric) aggregations.get(0)); + InternalScriptedMetric firstAggregation = aggregations.stream() + .filter(a -> a instanceof InternalScriptedMetric) + .map(a -> (InternalScriptedMetric) a) + .findFirst() + .orElseThrow(() -> new IllegalStateException("No InternalScriptedMetric found in aggregations to reduce")); Suggestion importance[1-10]: 8 __ Why: Valid concern: with mixed aggregation types now supported, aggregations.get(0) could be an InternalAvg, causing a ClassCastException. This addresses a real bug introduced by the PR changes.	Medium
General	Fail fast on unexpected aggregation types Silently ignoring unknown aggregation types could hide bugs and produce incorrect reductions. Add an explicit else branch that throws an IllegalStateException when an unexpected aggregation type is encountered, to fail fast rather than silently dropping data. server/src/main/java/org/opensearch/search/aggregations/metrics/InternalScriptedMetric.java [102-106] if (aggregation instanceof InternalScriptedMetric mapReduceAggregation) { aggregationObjects.addAll(mapReduceAggregation.aggregations); } else if (aggregation instanceof InternalAvg avg) { aggregationObjects.add(new ScriptedAvg(avg.getSum(), avg.getCount())); + } else { + throw new IllegalStateException("Unexpected aggregation type during reduce: " + aggregation.getClass().getName()); } Suggestion importance[1-10]: 5 __ Why: Adding an explicit else branch improves robustness by failing fast on unexpected types rather than silently dropping data, though it's a defensive improvement rather than a critical fix.	Low

[github-actions]

❌ Gradle check result for 3995dea: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

[mgodwan]

Should we throw in else condition?