Skip to content

Commit bf9ae5f

Browse files
authored
Fix profiling of ordered terms aggs (#31814)
This change fixes the profiling of `terms` aggregation ordered by a sub aggregation. Closes #22123
1 parent a81dfbc commit bf9ae5f

File tree

3 files changed

+23
-6
lines changed

3 files changed

+23
-6
lines changed

server/src/main/java/org/elasticsearch/search/aggregations/support/AggregationPath.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.elasticsearch.search.aggregations.bucket.SingleBucketAggregator;
2929
import org.elasticsearch.search.aggregations.metrics.InternalNumericMetricsAggregation;
3030
import org.elasticsearch.search.aggregations.metrics.NumericMetricsAggregator;
31+
import org.elasticsearch.search.profile.aggregation.ProfilingAggregator;
3132

3233
import java.util.ArrayList;
3334
import java.util.List;
@@ -256,7 +257,7 @@ public Aggregator resolveAggregator(Aggregator root) {
256257
Aggregator aggregator = root;
257258
for (int i = 0; i < pathElements.size(); i++) {
258259
AggregationPath.PathElement token = pathElements.get(i);
259-
aggregator = aggregator.subAggregator(token.name);
260+
aggregator = ProfilingAggregator.unwrap(aggregator.subAggregator(token.name));
260261
assert (aggregator instanceof SingleBucketAggregator && i <= pathElements.size() - 1)
261262
|| (aggregator instanceof NumericMetricsAggregator && i == pathElements.size() - 1) :
262263
"this should be picked up before aggregation execution - on validate";
@@ -272,7 +273,7 @@ public Aggregator resolveAggregator(Aggregator root) {
272273
*/
273274
public Aggregator resolveTopmostAggregator(Aggregator root) {
274275
AggregationPath.PathElement token = pathElements.get(0);
275-
Aggregator aggregator = root.subAggregator(token.name);
276+
Aggregator aggregator = ProfilingAggregator.unwrap(root.subAggregator(token.name));
276277
assert (aggregator instanceof SingleBucketAggregator )
277278
|| (aggregator instanceof NumericMetricsAggregator) : "this should be picked up before aggregation execution - on validate";
278279
return aggregator;
@@ -287,7 +288,7 @@ public Aggregator resolveTopmostAggregator(Aggregator root) {
287288
public void validate(Aggregator root) throws AggregationExecutionException {
288289
Aggregator aggregator = root;
289290
for (int i = 0; i < pathElements.size(); i++) {
290-
aggregator = aggregator.subAggregator(pathElements.get(i).name);
291+
aggregator = ProfilingAggregator.unwrap(aggregator.subAggregator(pathElements.get(i).name));
291292
if (aggregator == null) {
292293
throw new AggregationExecutionException("Invalid aggregator order path [" + this + "]. Unknown aggregation ["
293294
+ pathElements.get(i).name + "]");

server/src/main/java/org/elasticsearch/search/profile/aggregation/ProfilingAggregator.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,4 +114,11 @@ public void postCollection() throws IOException {
114114
public String toString() {
115115
return delegate.toString();
116116
}
117+
118+
public static Aggregator unwrap(Aggregator agg) {
119+
if (agg instanceof ProfilingAggregator) {
120+
return ((ProfilingAggregator) agg).delegate;
121+
}
122+
return agg;
123+
}
117124
}

server/src/test/java/org/elasticsearch/search/profile/aggregation/AggregationProfilerIT.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.elasticsearch.action.index.IndexRequestBuilder;
2323
import org.elasticsearch.action.search.SearchResponse;
2424
import org.elasticsearch.search.aggregations.Aggregator.SubAggCollectionMode;
25+
import org.elasticsearch.search.aggregations.BucketOrder;
2526
import org.elasticsearch.search.aggregations.bucket.sampler.DiversifiedOrdinalsSamplerAggregator;
2627
import org.elasticsearch.search.aggregations.bucket.terms.GlobalOrdinalsStringTermsAggregator;
2728
import org.elasticsearch.search.aggregations.metrics.avg.AvgAggregator;
@@ -120,9 +121,17 @@ public void testSimpleProfile() {
120121

121122
public void testMultiLevelProfile() {
122123
SearchResponse response = client().prepareSearch("idx").setProfile(true)
123-
.addAggregation(histogram("histo").field(NUMBER_FIELD).interval(1L)
124-
.subAggregation(terms("terms").field(TAG_FIELD)
125-
.subAggregation(avg("avg").field(NUMBER_FIELD)))).get();
124+
.addAggregation(
125+
histogram("histo")
126+
.field(NUMBER_FIELD)
127+
.interval(1L)
128+
.subAggregation(
129+
terms("terms")
130+
.field(TAG_FIELD)
131+
.order(BucketOrder.aggregation("avg", false))
132+
.subAggregation(avg("avg").field(NUMBER_FIELD))
133+
)
134+
).get();
126135
assertSearchResponse(response);
127136
Map<String, ProfileShardResult> profileResults = response.getProfileResults();
128137
assertThat(profileResults, notNullValue());

0 commit comments

Comments
 (0)