Skip to content

Commit 27e59ab

Browse files
imotovimotov
authored
[7.11] Revert "Remove aggregation's postCollect phase (#68942) (#69066)
This partially reverts #64016 and and adds #67839 and adds additional tests that would have caught issues with the changes in #64016. It's mostly Nik's code, I am just cleaning things up a bit. Co-authored-by: Nik Everett [email protected]
1 parent 18969cc commit 27e59ab

File tree

44 files changed

+703
-59
lines changed

Some content is hidden

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

44 files changed

+703
-59
lines changed

‎docs/reference/search/profile.asciidoc

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,9 @@ This yields the following aggregation profile output:
799799
"collect": 45786,
800800
"collect_count": 4,
801801
"build_leaf_collector": 18211,
802-
"build_leaf_collector_count": 1
802+
"build_leaf_collector_count": 1,
803+
"post_collection": 929,
804+
"post_collection_count": 1
803805
},
804806
"debug": {
805807
"total_buckets": 1,
@@ -820,7 +822,9 @@ This yields the following aggregation profile output:
820822
"collect": 69401,
821823
"collect_count": 4,
822824
"build_leaf_collector": 8150,
823-
"build_leaf_collector_count": 1
825+
"build_leaf_collector_count": 1,
826+
"post_collection": 1584,
827+
"post_collection_count": 1
824828
},
825829
"children": [
826830
{
@@ -837,7 +841,9 @@ This yields the following aggregation profile output:
837841
"collect": 61611,
838842
"collect_count": 4,
839843
"build_leaf_collector": 5564,
840-
"build_leaf_collector_count": 1
844+
"build_leaf_collector_count": 1,
845+
"post_collection": 471,
846+
"post_collection_count": 1
841847
},
842848
"debug": {
843849
"total_buckets": 1,

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,12 @@ public void collect(int docId, long owningBucketOrd) throws IOException {
102102
}
103103

104104
@Override
105-
protected void prepareSubAggs(long[] bucketOrdsToCollect) throws IOException {
105+
public void postCollection() throws IOException {
106+
// Delaying until beforeBuildingBuckets
107+
}
108+
109+
@Override
110+
protected void prepareSubAggs(long[] ordsToCollect) throws IOException {
106111
IndexReader indexReader = searcher().getIndexReader();
107112
for (LeafReaderContext ctx : indexReader.leaves()) {
108113
Scorer childDocsScorer = outFilter.scorer(ctx);
@@ -144,13 +149,14 @@ public int docID() {
144149
* structure that maps a primitive long to a list of primitive
145150
* longs.
146151
*/
147-
for (long o: bucketOrdsToCollect) {
148-
if (collectionStrategy.exists(o, globalOrdinal)) {
149-
collectBucket(sub, docId, o);
152+
for (long owningBucketOrd: ordsToCollect) {
153+
if (collectionStrategy.exists(owningBucketOrd, globalOrdinal)) {
154+
collectBucket(sub, docId, owningBucketOrd);
150155
}
151156
}
152157
}
153158
}
159+
super.postCollection(); // Run post collection after collecting the sub-aggs
154160
}
155161

156162
@Override

‎modules/parent-join/src/yamlRestTest/resources/rest-api-spec/test/50_order_by.yml

Lines changed: 238 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,238 @@
1+
---
2+
"order by sub agg containing join":
3+
- skip:
4+
reason: "It was fixed it 7.11.2"
5+
version: " - 7.11.2"
6+
- do:
7+
indices.create:
8+
index: test_1
9+
body:
10+
settings:
11+
number_of_shards: 1
12+
number_of_replicas: 0
13+
mappings:
14+
properties:
15+
name:
16+
type: keyword
17+
join:
18+
type: join
19+
relations:
20+
animal: feature
21+
22+
- do:
23+
bulk:
24+
index: test_1
25+
refresh: true
26+
body: |
27+
{ "index": {"_id": "sheep"} }
28+
{ "name": "sheep", "join": {"name": "animal"} }
29+
{ "index": {"_id": "cow"} }
30+
{ "name": "cow", "join": {"name": "animal"} }
31+
{ "index": {"_id": "pig"} }
32+
{ "name": "pig", "join": {"name": "animal"} }
33+
{ "index": {"routing": "sheep"} }
34+
{ "join": {"name": "feature", "parent": "sheep"}, "tag": "danger", "number": 1 }
35+
{ "index": {"routing": "sheep"} }
36+
{ "join": {"name": "feature", "parent": "sheep"}, "tag": "fluffiness", "number": 10 }
37+
{ "index": {"routing": "cow"} }
38+
{ "join": {"name": "feature", "parent": "cow"}, "tag": "danger", "number": 3 }
39+
{ "index": {"routing": "cow"} }
40+
{ "join": {"name": "feature", "parent": "cow"}, "tag": "fluffiness", "number": 1 }
41+
{ "index": {"routing": "pig"} }
42+
{ "join": {"name": "feature", "parent": "pig"}, "tag": "danger", "number": 100 }
43+
{ "index": {"routing": "pig"} }
44+
{ "join": {"name": "feature", "parent": "pig"}, "tag": "fluffiness", "number": 1 }
45+
46+
- do:
47+
search:
48+
index: test_1
49+
body:
50+
size: 0
51+
aggs:
52+
name:
53+
terms:
54+
size: 1
55+
shard_size: 1
56+
field: name
57+
order:
58+
- "children>max_number": desc
59+
aggs:
60+
children:
61+
children:
62+
type: feature
63+
aggs:
64+
max_number:
65+
max:
66+
field: number
67+
- length: { aggregations.name.buckets: 1 }
68+
- match: { aggregations.name.buckets.0.key: pig }
69+
- match: { aggregations.name.buckets.0.doc_count: 1 }
70+
- match: { aggregations.name.buckets.0.children.max_number.value: 100.0 }
71+
72+
---
73+
"order by sub agg containing join and nested":
74+
- skip:
75+
reason: "It was fixed it 7.11.2"
76+
version: " - 7.11.2"
77+
- do:
78+
indices.create:
79+
index: test_1
80+
body:
81+
settings:
82+
number_of_shards: 1
83+
number_of_replicas: 0
84+
mappings:
85+
properties:
86+
name:
87+
type: keyword
88+
join:
89+
type: join
90+
relations:
91+
animal: feature
92+
nested:
93+
type: nested
94+
properties:
95+
number:
96+
type: long
97+
98+
- do:
99+
bulk:
100+
index: test_1
101+
refresh: true
102+
body: |
103+
{ "index": {"_id": "sheep"} }
104+
{ "name": "sheep", "join": {"name": "animal"} }
105+
{ "index": {"_id": "cow"} }
106+
{ "name": "cow", "join": {"name": "animal"} }
107+
{ "index": {"_id": "pig"} }
108+
{ "name": "pig", "join": {"name": "animal"} }
109+
{ "index": {"routing": "sheep"} }
110+
{ "join": {"name": "feature", "parent": "sheep"}, "tag": "danger", "nested": [{"number": 1}] }
111+
{ "index": {"routing": "sheep"} }
112+
{ "join": {"name": "feature", "parent": "sheep"}, "tag": "fluffiness", "nested": [{"number": 10}] }
113+
{ "index": {"routing": "cow"} }
114+
{ "join": {"name": "feature", "parent": "cow"}, "tag": "danger", "nested": [{"number": 3}] }
115+
{ "index": {"routing": "cow"} }
116+
{ "join": {"name": "feature", "parent": "cow"}, "tag": "fluffiness", "nested": [{"number": 1}] }
117+
{ "index": {"routing": "pig"} }
118+
{ "join": {"name": "feature", "parent": "pig"}, "tag": "danger", "nested": [{"number": 100}, {"number": 1}] }
119+
{ "index": {"routing": "pig"} }
120+
{ "join": {"name": "feature", "parent": "pig"}, "tag": "fluffiness", "nested": [{"number": 1}] }
121+
122+
- do:
123+
search:
124+
index: test_1
125+
body:
126+
size: 0
127+
aggs:
128+
name:
129+
terms:
130+
size: 1
131+
shard_size: 1
132+
field: name
133+
order:
134+
- "children>nested>max_number": desc
135+
aggs:
136+
children:
137+
children:
138+
type: feature
139+
aggs:
140+
nested:
141+
nested:
142+
path: nested
143+
aggs:
144+
max_number:
145+
max:
146+
field: nested.number
147+
- length: { aggregations.name.buckets: 1 }
148+
- match: { aggregations.name.buckets.0.key: pig }
149+
- match: { aggregations.name.buckets.0.doc_count: 1 }
150+
- match: { aggregations.name.buckets.0.children.nested.max_number.value: 100.0 }
151+
152+
---
153+
"order by sub agg containing join and nested and filter":
154+
- skip:
155+
reason: "It was fixed it 7.11.2"
156+
version: " - 7.11.2"
157+
- do:
158+
indices.create:
159+
index: test_1
160+
body:
161+
settings:
162+
number_of_shards: 1
163+
number_of_replicas: 0
164+
mappings:
165+
properties:
166+
name:
167+
type: keyword
168+
join:
169+
type: join
170+
relations:
171+
animal: feature
172+
nested:
173+
type: nested
174+
properties:
175+
code:
176+
type:
177+
keyword
178+
number:
179+
type: long
180+
181+
- do:
182+
bulk:
183+
index: test_1
184+
refresh: true
185+
body: |
186+
{ "index": {"_id": "sheep"} }
187+
{ "name": "sheep", "join": {"name": "animal"} }
188+
{ "index": {"_id": "cow"} }
189+
{ "name": "cow", "join": {"name": "animal"} }
190+
{ "index": {"_id": "pig"} }
191+
{ "name": "pig", "join": {"name": "animal"} }
192+
{ "index": {"routing": "sheep"} }
193+
{ "join": {"name": "feature", "parent": "sheep"}, "tag": "danger", "nested": [{"code": "mighty", "number": 1}, {"code": "meek", "number": 100}] }
194+
{ "index": {"routing": "sheep"} }
195+
{ "join": {"name": "feature", "parent": "sheep"}, "tag": "fluffiness", "nested": [{"code": "mighty", "number": 10}, {"code": "meek", "number": 10}] }
196+
{ "index": {"routing": "cow"} }
197+
{ "join": {"name": "feature", "parent": "cow"}, "tag": "danger", "nested": [{"code": "mighty", "number": 3}, {"code": "meek", "number": 3}] }
198+
{ "index": {"routing": "cow"} }
199+
{ "join": {"name": "feature", "parent": "cow"}, "tag": "fluffiness", "nested": [{"code": "mighty", "number": 1}, {"code": "meek", "number": 1}] }
200+
{ "index": {"routing": "pig"} }
201+
{ "join": {"name": "feature", "parent": "pig"}, "tag": "danger", "nested": [{"code": "mighty", "number": 100}, {"code": "meek", "number": 1}] }
202+
{ "index": {"routing": "pig"} }
203+
{ "join": {"name": "feature", "parent": "pig"}, "tag": "fluffiness", "nested": [{"code": "mighty", "number": 1}, {"code": "meek", "number": 1}] }
204+
205+
- do:
206+
search:
207+
index: test_1
208+
body:
209+
size: 0
210+
aggs:
211+
name:
212+
terms:
213+
size: 1
214+
shard_size: 1
215+
field: name
216+
order:
217+
- "children>nested>filter>max_number": desc
218+
aggs:
219+
children:
220+
children:
221+
type: feature
222+
aggs:
223+
nested:
224+
nested:
225+
path: nested
226+
aggs:
227+
filter:
228+
filter:
229+
match:
230+
nested.code: mighty
231+
aggs:
232+
max_number:
233+
max:
234+
field: nested.number
235+
- length: { aggregations.name.buckets: 1 }
236+
- match: { aggregations.name.buckets.0.key: pig }
237+
- match: { aggregations.name.buckets.0.doc_count: 1 }
238+
- match: { aggregations.name.buckets.0.children.nested.filter.max_number.value: 100.0 }

‎rest-api-spec/src/main/resources/rest-api-spec/test/search.aggregation/170_cardinality_metric.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,7 @@ setup:
232232
- gt: { profile.shards.0.aggregations.0.breakdown.build_leaf_collector: 0 }
233233
- gt: { profile.shards.0.aggregations.0.breakdown.collect: 0 }
234234
- gt: { profile.shards.0.aggregations.0.breakdown.build_aggregation: 0 }
235+
- gt: { profile.shards.0.aggregations.0.breakdown.post_collection: 0 }
235236
- match: { profile.shards.0.aggregations.0.debug.empty_collectors_used: 0 }
236237
- gt: { profile.shards.0.aggregations.0.debug.numeric_collectors_used: 0 }
237238
- match: { profile.shards.0.aggregations.0.debug.ordinals_collectors_used: 0 }
@@ -257,6 +258,7 @@ setup:
257258
- gt: { profile.shards.0.aggregations.0.breakdown.build_leaf_collector: 0 }
258259
- gt: { profile.shards.0.aggregations.0.breakdown.collect: 0 }
259260
- gt: { profile.shards.0.aggregations.0.breakdown.build_aggregation: 0 }
261+
- gt: { profile.shards.0.aggregations.0.breakdown.post_collection: 0 }
260262
- match: { profile.shards.0.aggregations.0.debug.empty_collectors_used: 0 }
261263
- gt: { profile.shards.0.aggregations.0.debug.numeric_collectors_used: 0 }
262264
- match: { profile.shards.0.aggregations.0.debug.ordinals_collectors_used: 0 }
@@ -283,3 +285,4 @@ setup:
283285
- gt: { profile.shards.0.aggregations.0.breakdown.build_leaf_collector: 0 }
284286
- gt: { profile.shards.0.aggregations.0.breakdown.collect: 0 }
285287
- gt: { profile.shards.0.aggregations.0.breakdown.build_aggregation: 0 }
288+
- gt: { profile.shards.0.aggregations.0.breakdown.post_collection: 0 }

0 commit comments

Comments
 (0)