Skip to content

Commit e58ad9f

Browse files
nik9000nik9000
authored
Clean up how pipeline aggs check for multi-bucket (backport of elastic#54161) (elastic#54379)
Pipeline aggregations like `stats_bucket`, `sum_bucket`, and `percentiles_bucket` only operate on buckets that have multiple buckets. This adds support for those aggregations to `geo_distance`, `ip_range`, `auto_date_histogram`, and `rare_terms`. This all happened because we used a marker interface to mark compatible aggs, `MultiBucketAggregationBuilder` and it was fairly easy to forget to implement the interface. This replaces the marker interface with an abstract method in `AggregationBuilder`, `bucketCardinality` which makes you return `NONE`, `ONE`, or `MANY`. The `bucket` aggregations can check for `MANY`. At this point `ONE` and `NONE` amount to about the same thing, but I suspect that'll be a useful distinction when validating bucket sorts. Closes elastic#53215
1 parent 39b3010 commit e58ad9f

File tree

45 files changed

+547
-124
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

45 files changed

+547
-124
lines changed

‎client/rest-high-level/src/main/java/org/elasticsearch/client/analytics/StringStatsAggregationBuilder.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ protected void innerWriteTo(StreamOutput out) throws IOException {
8686
throw new UnsupportedOperationException();
8787
}
8888

89+
@Override
90+
public BucketCardinality bucketCardinality() {
91+
return BucketCardinality.NONE;
92+
}
93+
8994
@Override
9095
protected ValuesSourceAggregatorFactory<Bytes> innerBuild(QueryShardContext queryShardContext, ValuesSourceConfig<Bytes> config,
9196
AggregatorFactory parent, Builder subFactoriesBuilder) throws IOException {

‎client/rest-high-level/src/main/java/org/elasticsearch/client/analytics/TopMetricsAggregationBuilder.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,11 @@ protected void doWriteTo(StreamOutput out) throws IOException {
9494
throw new UnsupportedOperationException();
9595
}
9696

97+
@Override
98+
public BucketCardinality bucketCardinality() {
99+
return BucketCardinality.NONE;
100+
}
101+
97102
@Override
98103
protected AggregatorFactory doBuild(QueryShardContext queryShardContext, AggregatorFactory parent, Builder subfactoriesBuilder)
99104
throws IOException {

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/matrix/stats/MatrixStatsAggregationBuilder.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import java.util.Map;
3939

4040
public class MatrixStatsAggregationBuilder
41-
extends ArrayValuesSourceAggregationBuilder.LeafOnly<ValuesSource.Numeric, MatrixStatsAggregationBuilder> {
41+
extends ArrayValuesSourceAggregationBuilder.LeafOnly<ValuesSource.Numeric, MatrixStatsAggregationBuilder> {
4242
public static final String NAME = "matrix_stats";
4343

4444
private MultiValueMode multiValueMode = MultiValueMode.AVG;

‎modules/aggs-matrix-stats/src/main/java/org/elasticsearch/search/aggregations/support/ArrayValuesSourceAggregationBuilder.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,11 @@ public AB subAggregations(Builder subFactories) {
8484
throw new AggregationInitializationException("Aggregator [" + name + "] of type [" +
8585
getType() + "] cannot accept sub-aggregations");
8686
}
87+
88+
@Override
89+
public final BucketCardinality bucketCardinality() {
90+
return BucketCardinality.NONE;
91+
}
8792
}
8893

8994
private final ValuesSourceType valuesSourceType;

‎modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ChildrenAggregationBuilder.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,16 @@ protected void innerWriteTo(StreamOutput out) throws IOException {
9494
out.writeString(childType);
9595
}
9696

97+
@Override
98+
public BucketCardinality bucketCardinality() {
99+
return BucketCardinality.ONE;
100+
}
101+
97102
@Override
98103
protected ValuesSourceAggregatorFactory<WithOrdinals> innerBuild(QueryShardContext queryShardContext,
99-
ValuesSourceConfig<WithOrdinals> config,
100-
AggregatorFactory parent,
101-
Builder subFactoriesBuilder) throws IOException {
104+
ValuesSourceConfig<WithOrdinals> config,
105+
AggregatorFactory parent,
106+
Builder subFactoriesBuilder) throws IOException {
102107
return new ChildrenAggregatorFactory(name, config, childFilter, parentFilter, queryShardContext, parent,
103108
subFactoriesBuilder, metaData);
104109
}

‎modules/parent-join/src/main/java/org/elasticsearch/join/aggregations/ParentAggregationBuilder.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,11 +94,16 @@ protected void innerWriteTo(StreamOutput out) throws IOException {
9494
out.writeString(childType);
9595
}
9696

97+
@Override
98+
public BucketCardinality bucketCardinality() {
99+
return BucketCardinality.ONE;
100+
}
101+
97102
@Override
98103
protected ValuesSourceAggregatorFactory<WithOrdinals> innerBuild(QueryShardContext queryShardContext,
99-
ValuesSourceConfig<WithOrdinals> config,
100-
AggregatorFactory parent,
101-
Builder subFactoriesBuilder) throws IOException {
104+
ValuesSourceConfig<WithOrdinals> config,
105+
AggregatorFactory parent,
106+
Builder subFactoriesBuilder) throws IOException {
102107
return new ParentAggregatorFactory(name, config, childFilter, parentFilter, queryShardContext, parent,
103108
subFactoriesBuilder, metaData);
104109
}

‎rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/280_rare_terms.yml‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -360,3 +360,64 @@ setup:
360360
- is_false: aggregations.str_terms.buckets.0.key_as_string
361361
- match: { aggregations.str_terms.buckets.0.doc_count: 1 }
362362
- match: { aggregations.str_terms.buckets.0.max_n.value: 3.0 }
363+
364+
---
365+
"avg_bucket":
366+
- skip:
367+
version: " - 7.7.99"
368+
reason: Fixed in 7.8.0
369+
- do:
370+
indices.create:
371+
index: test
372+
body:
373+
settings:
374+
number_of_replicas: 0
375+
mappings:
376+
properties:
377+
str:
378+
type: keyword
379+
- do:
380+
bulk:
381+
index: test
382+
refresh: true
383+
body:
384+
- '{"index": {}}'
385+
- '{"str": "foo", "v": 1}'
386+
- '{"index": {}}'
387+
- '{"str": "foo", "v": 2}'
388+
- '{"index": {}}'
389+
- '{"str": "foo", "v": 3}'
390+
- '{"index": {}}'
391+
- '{"str": "bar", "v": 4}'
392+
- '{"index": {}}'
393+
- '{"str": "bar", "v": 5}'
394+
- '{"index": {}}'
395+
- '{"str": "baz", "v": 6}'
396+
397+
- do:
398+
search:
399+
index: test
400+
body:
401+
size: 0
402+
aggs:
403+
str_terms:
404+
rare_terms:
405+
field: str
406+
max_doc_count: 2
407+
aggs:
408+
v:
409+
sum:
410+
field: v
411+
str_terms_avg_v:
412+
avg_bucket:
413+
buckets_path: str_terms.v
414+
415+
- match: { hits.total.value: 6 }
416+
- length: { aggregations.str_terms.buckets: 2 }
417+
- match: { aggregations.str_terms.buckets.0.key: baz }
418+
- match: { aggregations.str_terms.buckets.0.doc_count: 1 }
419+
- match: { aggregations.str_terms.buckets.0.v.value: 6 }
420+
- match: { aggregations.str_terms.buckets.1.key: bar }
421+
- match: { aggregations.str_terms.buckets.1.doc_count: 2 }
422+
- match: { aggregations.str_terms.buckets.1.v.value: 9 }
423+
- match: { aggregations.str_terms_avg_v.value: 7.5 }

‎rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/330_auto_date_histogram.yml‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
setup:
2+
- do:
3+
indices.create:
4+
index: test
5+
body:
6+
settings:
7+
number_of_replicas: 0
8+
mappings:
9+
properties:
10+
date:
11+
type: date
12+
- do:
13+
bulk:
14+
refresh: true
15+
index: test
16+
body:
17+
- '{"index": {}}'
18+
- '{"date": "2020-03-01", "v": 1}'
19+
- '{"index": {}}'
20+
- '{"date": "2020-03-02", "v": 2}'
21+
- '{"index": {}}'
22+
- '{"date": "2020-03-08", "v": 3}'
23+
- '{"index": {}}'
24+
- '{"date": "2020-03-09", "v": 4}'
25+
26+
---
27+
"basic":
28+
- skip:
29+
version: " - 7.7.99"
30+
reason: Tracked in https://github.com/elastic/elasticsearch/issues/54382
31+
- do:
32+
search:
33+
body:
34+
size: 0
35+
aggs:
36+
histo:
37+
auto_date_histogram:
38+
field: date
39+
buckets: 2
40+
- match: { hits.total.value: 4 }
41+
- length: { aggregations.histo.buckets: 2 }
42+
- match: { aggregations.histo.buckets.0.key_as_string: "2020-03-01T00:00:00.000Z" }
43+
- match: { aggregations.histo.buckets.0.doc_count: 2 }
44+
- match: { aggregations.histo.buckets.1.key_as_string: "2020-03-08T00:00:00.000Z" }
45+
- match: { aggregations.histo.buckets.1.doc_count: 2 }
46+
47+
---
48+
"avg_bucket":
49+
- skip:
50+
version: " - 7.7.99"
51+
reason: Fixed in 7.8.0
52+
- do:
53+
search:
54+
body:
55+
size: 0
56+
aggs:
57+
histo:
58+
auto_date_histogram:
59+
field: date
60+
buckets: 2
61+
aggs:
62+
v:
63+
sum:
64+
field: v
65+
histo_avg_v:
66+
avg_bucket:
67+
buckets_path: histo.v
68+
- match: { hits.total.value: 4 }
69+
- length: { aggregations.histo.buckets: 2 }
70+
- match: { aggregations.histo.buckets.0.key_as_string: "2020-03-01T00:00:00.000Z" }
71+
- match: { aggregations.histo.buckets.0.doc_count: 2 }
72+
- match: { aggregations.histo.buckets.0.v.value: 3 }
73+
- match: { aggregations.histo.buckets.1.key_as_string: "2020-03-08T00:00:00.000Z" }
74+
- match: { aggregations.histo.buckets.1.doc_count: 2 }
75+
- match: { aggregations.histo.buckets.1.v.value: 7 }
76+
- match: { aggregations.histo_avg_v.value: 5 }

‎rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/340_geo_distance.yml‎

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
setup:
2+
- do:
3+
indices.create:
4+
index: test
5+
body:
6+
mappings:
7+
properties:
8+
location:
9+
type: geo_point
10+
- do:
11+
bulk:
12+
index: test
13+
refresh: true
14+
body:
15+
- '{"index": {}}'
16+
- '{"location": {"lat" : 40.7128, "lon" : -74.0060}, "name": "New York", "population": 8623000}'
17+
- '{"index": {}}'
18+
- '{"location": {"lat" : 34.0522, "lon" : -118.2437}, "name": "Los Angeles", "population": 4000000}'
19+
- '{"index": {}}'
20+
- '{"location": {"lat" : 41.8781, "lon" : -87.6298}, "name": "Chicago", "population": 2716000}'
21+
22+
---
23+
"basic":
24+
- do:
25+
search:
26+
rest_total_hits_as_int: true
27+
body:
28+
size: 0
29+
aggs:
30+
distance:
31+
geo_distance:
32+
field: location
33+
origin: "35.7796, -78.6382"
34+
ranges:
35+
- to: 1000000
36+
- from: 1000000
37+
to: 5000000
38+
- from: 5000000
39+
- match: { hits.total: 3 }
40+
- length: { aggregations.distance.buckets: 3 }
41+
- match: { aggregations.distance.buckets.0.key: "*-1000000.0" }
42+
- match: { aggregations.distance.buckets.0.doc_count: 1 }
43+
- match: { aggregations.distance.buckets.1.key: "1000000.0-5000000.0" }
44+
- match: { aggregations.distance.buckets.1.doc_count: 2 }
45+
- match: { aggregations.distance.buckets.2.key: "5000000.0-*" }
46+
- match: { aggregations.distance.buckets.2.doc_count: 0 }
47+
48+
---
49+
"avg_bucket":
50+
- skip:
51+
version: " - 7.7.99"
52+
reason: Fixed in 7.8.0
53+
- do:
54+
search:
55+
body:
56+
size: 0
57+
aggs:
58+
distance:
59+
geo_distance:
60+
field: location
61+
origin: "35.7796, -78.6382"
62+
ranges:
63+
- to: 1000000
64+
- from: 1000000
65+
to: 5000000
66+
- from: 5000000
67+
aggs:
68+
total_population:
69+
sum:
70+
field: population
71+
avg_total_population:
72+
avg_bucket:
73+
buckets_path: distance.total_population
74+
- match: { hits.total.value: 3 }
75+
- length: { aggregations.distance.buckets: 3 }
76+
- match: { aggregations.distance.buckets.0.key: "*-1000000.0" }
77+
- match: { aggregations.distance.buckets.0.doc_count: 1 }
78+
- match: { aggregations.distance.buckets.0.total_population.value: 8623000.0 }
79+
- match: { aggregations.distance.buckets.1.key: "1000000.0-5000000.0" }
80+
- match: { aggregations.distance.buckets.1.doc_count: 2 }
81+
- match: { aggregations.distance.buckets.1.total_population.value: 6716000.0 }
82+
- match: { aggregations.distance.buckets.2.key: "5000000.0-*" }
83+
- match: { aggregations.distance.buckets.2.doc_count: 0 }
84+
- match: { aggregations.distance.buckets.2.total_population.value: 0 }
85+
- match: { aggregations.avg_total_population.value: 7669500.0 }

‎rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/40_range.yml‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,58 @@ setup:
224224
- match: { aggregations.ip_range.buckets.1.key: "192.168.0.0-192.169.0.0" }
225225
- match: { aggregations.ip_range.buckets.2.key: "192.169.0.0-*" }
226226

227+
---
228+
"IP Range avg_bucket":
229+
- skip:
230+
version: " - 7.7.99"
231+
reason: Fixed in 7.8.0
232+
- do:
233+
bulk:
234+
refresh: true
235+
index: test
236+
body:
237+
- '{"index": {}}'
238+
- '{"ip": "::1", "v": 1}'
239+
- '{"index": {}}'
240+
- '{"ip": "192.168.0.1", "v": 2}'
241+
- '{"index": {}}'
242+
- '{"ip": "192.168.0.7", "v": 3}'
243+
244+
- do:
245+
search:
246+
index: test
247+
body:
248+
size: 0
249+
aggs:
250+
range:
251+
ip_range:
252+
field: ip
253+
ranges:
254+
- to: 192.168.0.0
255+
- from: 192.168.0.0
256+
to: 192.169.0.0
257+
- from: 192.169.0.0
258+
aggs:
259+
v:
260+
sum:
261+
field: v
262+
range_avg_v:
263+
avg_bucket:
264+
buckets_path: range.v
265+
266+
- match: { hits.total.value: 3 }
267+
- length: { aggregations.range.buckets: 3 }
268+
- match: { aggregations.range.buckets.0.key: "*-192.168.0.0" }
269+
- match: { aggregations.range.buckets.0.doc_count: 1 }
270+
- match: { aggregations.range.buckets.0.v.value: 1 }
271+
- match: { aggregations.range.buckets.1.key: "192.168.0.0-192.169.0.0" }
272+
- match: { aggregations.range.buckets.1.doc_count: 2 }
273+
- match: { aggregations.range.buckets.1.v.value: 5 }
274+
- match: { aggregations.range.buckets.2.key: "192.169.0.0-*" }
275+
- match: { aggregations.range.buckets.2.doc_count: 0 }
276+
- match: { aggregations.range.buckets.2.v.value: 0 }
277+
- match: { aggregations.range_avg_v.value: 3 }
278+
227279
---
228280
"Date range":
229281
- do:

0 commit comments

Comments
 (0)