Skip to content

Commit 7b29cb3

Browse files
not-napoleonnot-napoleon
authored
Allow composite to be a sub-aggregation of filter (#71639)
1 parent bb2dd72 commit 7b29cb3

File tree

3 files changed

+183
-83
lines changed

3 files changed

+183
-83
lines changed

‎rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search.aggregation/230_composite.yml

Lines changed: 177 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,6 @@ setup:
1616
type: long
1717
geo_point:
1818
type: geo_point
19-
nested:
20-
type: nested
21-
properties:
22-
nested_long:
23-
type: long
2419

2520
- do:
2621
indices.create:
@@ -32,11 +27,6 @@ setup:
3227
type: date
3328
long:
3429
type: long
35-
nested:
36-
type: nested
37-
properties:
38-
nested_long:
39-
type: long
4030

4131
- do:
4232
indices.create:
@@ -61,23 +51,118 @@ setup:
6151
semester:
6252
type: keyword
6353

54+
- do:
55+
indices.create:
56+
index: nonesting
57+
body:
58+
mappings:
59+
properties:
60+
kw:
61+
type: keyword
62+
num:
63+
type: integer
64+
65+
- do:
66+
index:
67+
index: nonesting
68+
id: 1
69+
body: { "kw": "one", "num": 1 }
70+
71+
- do:
72+
index:
73+
index: nonesting
74+
id: 2
75+
body: { "kw": "two", "num": 2 }
76+
77+
- do:
78+
index:
79+
index: nonesting
80+
id: 3
81+
body: { "kw": "three", "num": 3 }
82+
6483
- do:
6584
index:
6685
index: verynested
6786
id: 1
68-
body: { "department": "compsci", "staff": 12, "courses": [ { "name": "Object Oriented Programming", "credits": 3, "sessions": [ { "semester": "spr2021", "students": 37 }, { "semester": "fall2020", "students": 45} ] }, { "name": "Theory of Computation", "credits": 4, "sessions": [ { "semester": "spr2021", "students": 19 }, { "semester": "fall2020", "students": 14 } ] } ] }
87+
body: {
88+
"department": "compsci",
89+
"staff": 12,
90+
"courses": [
91+
{
92+
"name": "Object Oriented Programming",
93+
"credits": 3,
94+
"sessions": [
95+
{
96+
"semester": "spr2021",
97+
"students": 37
98+
},
99+
{
100+
"semester": "fall2020",
101+
"students": 45
102+
}
103+
]
104+
},
105+
{
106+
"name": "Theory of Computation",
107+
"credits": 4,
108+
"sessions": [
109+
{
110+
"semester": "spr2021",
111+
"students": 19
112+
},
113+
{
114+
"semester": "fall2020",
115+
"students": 14
116+
}
117+
]
118+
}
119+
]
120+
}
69121

70122
- do:
71123
index:
72124
index: verynested
73125
id: 2
74-
body: { "department": "math", "staff": 20, "courses": [ { "name": "Precalculus", "credits": 1, "sessions": [ { "semester": "spr2021", "students": 100 }, { "semester": "fall2020", "students": 134 } ] }, { "name": "Linear Algebra", "credits": 3, "sessions": [ { "semester": "spr2021", "students": 29 }, { "semester": "fall2020", "students": 23 } ] } ] }
126+
body: {
127+
"department": "math",
128+
"staff": 20,
129+
"courses": [
130+
{
131+
"name": "Precalculus",
132+
"credits": 1,
133+
"sessions": [
134+
{
135+
"semester": "spr2021",
136+
"students": 100
137+
},
138+
{
139+
"semester": "fall2020",
140+
"students": 134
141+
}
142+
]
143+
},
144+
{
145+
"name": "Linear Algebra",
146+
"credits": 3,
147+
"sessions": [
148+
{
149+
"semester": "spr2021",
150+
"students": 29
151+
},
152+
{
153+
"semester": "fall2020",
154+
"students": 23
155+
}
156+
]
157+
}
158+
]
159+
}
75160

76161
- do:
77162
index:
78163
index: test
79164
id: 1
80-
body: { "keyword": "foo", "long": [10, 20], "geo_point": "37.2343,-115.8067", "nested": [{"nested_long": 10}, {"nested_long": 20}] }
165+
body: { "keyword": "foo", "long": [10, 20], "geo_point": "37.2343,-115.8067"}
81166

82167
- do:
83168
index:
@@ -89,13 +174,13 @@ setup:
89174
index:
90175
index: test
91176
id: 3
92-
body: { "keyword": "bar", "long": [100, 0], "geo_point": "90.0,0.0", "nested": [{"nested_long": 10}, {"nested_long": 0}] }
177+
body: { "keyword": "bar", "long": [100, 0], "geo_point": "90.0,0.0"}
93178

94179
- do:
95180
index:
96181
index: test
97182
id: 4
98-
body: { "keyword": "bar", "long": [1000, 0], "geo_point": "41.12,-71.34", "nested": [{"nested_long": 1000}, {"nested_long": 20}] }
183+
body: { "keyword": "bar", "long": [1000, 0], "geo_point": "41.12,-71.34"}
99184

100185
- do:
101186
index:
@@ -117,7 +202,7 @@ setup:
117202

118203
- do:
119204
indices.refresh:
120-
index: [test, other, verynested]
205+
index: [test, other, verynested, nonesting]
121206

122207
---
123208
"Simple Composite aggregation":
@@ -265,7 +350,7 @@ setup:
265350
terms:
266351
field: long
267352
aggs:
268-
nested:
353+
invalid_child:
269354
composite:
270355
sources: [
271356
{
@@ -485,67 +570,6 @@ setup:
485570
}
486571
]
487572

488-
---
489-
"Composite aggregation with nested parent":
490-
- do:
491-
search:
492-
rest_total_hits_as_int: true
493-
index: test
494-
body:
495-
aggregations:
496-
1:
497-
nested:
498-
path: nested
499-
aggs:
500-
2:
501-
composite:
502-
sources: [
503-
"nested": {
504-
"terms": {
505-
"field": "nested.nested_long"
506-
}
507-
}
508-
]
509-
510-
- match: {hits.total: 6}
511-
- length: { aggregations.1.2.buckets: 4 }
512-
- match: { aggregations.1.2.buckets.0.key.nested: 0 }
513-
- match: { aggregations.1.2.buckets.0.doc_count: 1 }
514-
- match: { aggregations.1.2.buckets.1.key.nested: 10 }
515-
- match: { aggregations.1.2.buckets.1.doc_count: 2 }
516-
- match: { aggregations.1.2.buckets.2.key.nested: 20 }
517-
- match: { aggregations.1.2.buckets.2.doc_count: 2 }
518-
- match: { aggregations.1.2.buckets.3.key.nested: 1000 }
519-
- match: { aggregations.1.2.buckets.3.doc_count: 1 }
520-
521-
- do:
522-
search:
523-
rest_total_hits_as_int: true
524-
index: test
525-
body:
526-
aggregations:
527-
1:
528-
nested:
529-
path: nested
530-
aggs:
531-
2:
532-
composite:
533-
after: { "nested": 10 }
534-
sources: [
535-
"nested": {
536-
"terms": {
537-
"field": "nested.nested_long"
538-
}
539-
}
540-
]
541-
542-
- match: {hits.total: 6}
543-
- length: { aggregations.1.2.buckets: 2 }
544-
- match: { aggregations.1.2.buckets.0.key.nested: 20 }
545-
- match: { aggregations.1.2.buckets.0.doc_count: 2 }
546-
- match: { aggregations.1.2.buckets.1.key.nested: 1000 }
547-
- match: { aggregations.1.2.buckets.1.doc_count: 1 }
548-
549573
---
550574
"Composite aggregation with unmapped field":
551575
- skip:
@@ -1120,3 +1144,78 @@ setup:
11201144
- match: { aggregations.courses.sessions.names.buckets.0.doc_count: 4}
11211145
- match: { aggregations.courses.sessions.names.buckets.1.key.kw: "spr2021" }
11221146
- match: { aggregations.courses.sessions.names.buckets.1.doc_count: 4}
1147+
1148+
---
1149+
"Nested then filter then nested then terms":
1150+
- skip:
1151+
version: " - 7.99.99"
1152+
reason: Filter support not backported yet
1153+
- do:
1154+
search:
1155+
rest_total_hits_as_int: true
1156+
index: verynested
1157+
body:
1158+
"aggregations": {
1159+
"courses": {
1160+
"nested": { "path": "courses" },
1161+
"aggregations": {
1162+
"highpass_filter": {
1163+
"filter": { "range": {"courses.credits": { "gt": 1 }}},
1164+
"aggregations": {
1165+
"sessions": {
1166+
"nested": { "path": "courses.sessions" },
1167+
"aggregations": {
1168+
"names": {
1169+
"composite": {
1170+
"sources": [
1171+
"kw": {"terms": { "field": "courses.sessions.semester" }}
1172+
]
1173+
}
1174+
}
1175+
}
1176+
}
1177+
}
1178+
}
1179+
}
1180+
}
1181+
}
1182+
- match: {hits.total: 2}
1183+
- match: {aggregations.courses.doc_count: 4}
1184+
- match: {aggregations.courses.highpass_filter.doc_count: 3}
1185+
- match: {aggregations.courses.highpass_filter.sessions.doc_count: 6}
1186+
- length: { aggregations.courses.highpass_filter.sessions.names.buckets: 2 }
1187+
- match: { aggregations.courses.highpass_filter.sessions.names.buckets.0.key.kw: "fall2020" }
1188+
- match: { aggregations.courses.highpass_filter.sessions.names.buckets.0.doc_count: 3}
1189+
- match: { aggregations.courses.highpass_filter.sessions.names.buckets.1.key.kw: "spr2021" }
1190+
- match: { aggregations.courses.highpass_filter.sessions.names.buckets.1.doc_count: 3}
1191+
1192+
---
1193+
"Filter without nesting":
1194+
- skip:
1195+
version: " - 7.99.99"
1196+
reason: Filter support not backported yet
1197+
- do:
1198+
search:
1199+
rest_total_hits_as_int: true
1200+
index: nonesting
1201+
body: {
1202+
"aggs": {
1203+
"not_one": {
1204+
"filter": { "range": {"num": {"gt": 1}} },
1205+
"aggs": {
1206+
"keez": {
1207+
"composite": {
1208+
"sources": [
1209+
"key": {"terms": {"field": "kw"}}
1210+
]
1211+
}
1212+
}
1213+
}
1214+
}
1215+
}
1216+
}
1217+
- match: {hits.total: 3}
1218+
- match: {aggregations.not_one.doc_count: 2}
1219+
- length: {aggregations.not_one.keez.buckets: 2}
1220+
- match: {aggregations.not_one.keez.buckets.0.key.key: "three"}
1221+
- match: {aggregations.not_one.keez.buckets.1.key.key: "two"}

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregationBuilder.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.search.aggregations.AggregationBuilder;
1818
import org.elasticsearch.search.aggregations.AggregatorFactories;
1919
import org.elasticsearch.search.aggregations.AggregatorFactory;
20+
import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregatorFactory;
2021
import org.elasticsearch.search.aggregations.bucket.nested.NestedAggregatorFactory;
2122
import org.elasticsearch.search.aggregations.support.AggregationContext;
2223
import org.elasticsearch.search.aggregations.support.ValuesSourceRegistry;
@@ -160,11 +161,11 @@ public BucketCardinality bucketCardinality() {
160161
* this aggregator or the instance of the parent's factory that is incompatible with
161162
* the composite aggregation.
162163
*/
163-
private AggregatorFactory checkParentIsNullOrNested(AggregatorFactory factory) {
164+
private AggregatorFactory validateParentAggregations(AggregatorFactory factory) {
164165
if (factory == null) {
165166
return null;
166-
} else if (factory instanceof NestedAggregatorFactory) {
167-
return checkParentIsNullOrNested(factory.getParent());
167+
} else if (factory instanceof NestedAggregatorFactory || factory instanceof FilterAggregatorFactory) {
168+
return validateParentAggregations(factory.getParent());
168169
} else {
169170
return factory;
170171
}
@@ -195,7 +196,7 @@ private static void validateSources(List<CompositeValuesSourceBuilder<?>> source
195196
@Override
196197
protected AggregatorFactory doBuild(AggregationContext context, AggregatorFactory parent,
197198
AggregatorFactories.Builder subfactoriesBuilder) throws IOException {
198-
AggregatorFactory invalid = checkParentIsNullOrNested(parent);
199+
AggregatorFactory invalid = validateParentAggregations(parent);
199200
if (invalid != null) {
200201
throw new IllegalArgumentException("[composite] aggregation cannot be used with a parent aggregation of" +
201202
" type: [" + invalid.getClass().getSimpleName() + "]");

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/CompositeAggregator.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ protected LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucket
382382
Sort indexSortPrefix = buildIndexSortPrefix(ctx);
383383
int sortPrefixLen = computeSortPrefixLen(indexSortPrefix);
384384

385-
SortedDocsProducer sortedDocsProducer = sortPrefixLen == 0 ?
385+
SortedDocsProducer sortedDocsProducer = (sortPrefixLen == 0 && parent == null) ?
386386
sources[0].createSortedDocsProducerOrNull(ctx.reader(), topLevelQuery()) : null;
387387
if (sortedDocsProducer != null) {
388388
// Visit documents sorted by the leading source of the composite definition and terminates

0 commit comments

Comments
 (0)