Skip to content

Commit

Permalink
Fix broken parent and child aggregator (elastic#63811)
Browse files Browse the repository at this point in the history
In elastic#57892 I broke *some* sub-aggregations inside of the `parent` and
`child` aggregator, specifically any sub-aggregations that do work in
the `postCollect` phase. This fixes it by delaying the post collect
phase of aggs under `parent` and `child` until `beforeBuildingBuckets`
because, well, we haven't done *any* collection until after that phase.
  • Loading branch information
nik9000 committed Oct 19, 2020
1 parent b516ddb commit 06acc10
Show file tree
Hide file tree
Showing 6 changed files with 147 additions and 66 deletions.
2 changes: 1 addition & 1 deletion modules/parent-join/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ esplugin {

restResources {
restApi {
includeCore '_common', 'cluster', 'nodes', 'indices', 'index', 'search'
includeCore '_common', 'bulk', 'cluster', 'nodes', 'indices', 'index', 'search'
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ public void collect(int docId, long owningBucketOrd) throws IOException {
};
}

@Override
public void postCollection() throws IOException {
// Delaying until beforeBuildingBuckets
}

@Override
protected void beforeBuildingBuckets(long[] ordsToCollect) throws IOException {
IndexReader indexReader = context().searcher().getIndexReader();
Expand Down Expand Up @@ -162,6 +167,7 @@ public int docID() {
}
}
}
super.postCollection(); // Run post collection after collecting the sub-aggs
}

@Override
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -3,53 +3,30 @@ setup:
indices.create:
index: test
body:
settings:
number_of_shards: 2 #Two shards proves that we route properly
mappings:
properties:
join_field: { "type": "join", "relations": { "parent": "child", "child": "grand_child" } }
id: { "type": "keyword" }

- do:
index:
bulk:
index: test
id: 1
body: { "id", "1", "join_field": { "name": "parent" } }

- do:
index:
index: test
id: 2
body: { "id", "2", "join_field": { "name": "parent" } }

- do:
index:
index: test
id: 3
routing: 1
body: { "id", "3", "join_field": { "name": "child", "parent": "1" } }

- do:
index:
index: test
id: 4
routing: 1
body: { "id", "4", "join_field": { "name": "child", "parent": "1" } }

- do:
index:
index: test
id: 5
routing: 1
body: { "id", "5", "join_field": { "name": "child", "parent": "2" } }

- do:
index:
index: test
id: 6
routing: 1
body: { "id", "6", "join_field": { "name": "grand_child", "parent": "5" } }

- do:
indices.refresh: {}
refresh: true
body:
- '{ "index": {"_id": "1"}}'
- '{ "id": 1, "join_field": { "name": "parent" } }'
- '{ "index": {"_id": "2"}}'
- '{ "id": 2, "join_field": { "name": "parent" } }'
- '{ "index": {"_id": "3", "routing": "1"}}'
- '{ "id": 3, "join_field": { "name": "child", "parent": "1" } }'
- '{ "index": {"_id": "4", "routing": "1"}}'
- '{ "id": 4, "join_field": { "name": "child", "parent": "1" } }'
- '{ "index": {"_id": "5", "routing": "2"}}'
- '{ "id": 5, "join_field": { "name": "child", "parent": "2" } }'
- '{ "index": {"_id": "6", "routing": "1"}}'
- '{ "id": 6, "join_field": { "name": "grand_child", "parent": "5" } }'

---
teardown:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
setup:
- do:
indices.create:
index: test
body:
settings:
number_of_shards: 2 #Two shards proves that we route properly
mappings:
properties:
join_field: { "type": "join", "relations": { "parent": "child", "child": "grand_child" } }
id: { "type": "keyword" }

- do:
bulk:
index: test
refresh: true
body:
- '{ "index": {"_id": "1"}}'
- '{ "tag": "bump", "join_field": { "name": "parent" } }'
- '{ "index": {"_id": "2"}}'
- '{ "tag": "jumble", "join_field": { "name": "parent" } }'
- '{ "index": {"_id": "3", "routing": "1"}}'
- '{ "animal": "dog", "join_field": { "name": "child", "parent": "1" } }'
- '{ "index": {"_id": "4", "routing": "1"}}'
- '{ "animal": "cat", "join_field": { "name": "child", "parent": "1" } }'
- '{ "index": {"_id": "5", "routing": "2"}}'
- '{ "animal": "dog", "join_field": { "name": "child", "parent": "2" } }'
- '{ "index": {"_id": "6", "routing": "1"}}'
- '{ "join_field": { "name": "grand_child", "parent": "5" } }'

---
unconfigured:
- do:
index:
index: test_unconfigured
refresh: true
body:
join:
name: question
body: <p>I have Windows 2003 server and i bought a new Windows 2008 server...,
title: Whats the best way to file transfer my site from server to a newer one?,
tags: [windows-server-2003, windows-server-2008, file-transfer]

- do:
search:
index: test_unconfigured
body:
size: 0
aggs:
to-answers:
children:
type: answer
- match: { hits.total.value: 1 }
- match: { aggregations.to-answers.doc_count: 0 }

---
children:
- do:
search:
index: test
body:
size: 0
aggs:
children:
children:
type: child
- match: { hits.total.value: 6 }
- match: { aggregations.children.doc_count: 3 }

---
children containing cardinality:
- do:
search:
index: test
body:
size: 0
aggs:
children:
children:
type: child
aggs:
cardinality:
cardinality:
field: animal.keyword
- match: { hits.total.value: 6 }
- match: { aggregations.children.doc_count: 3 }
- match: { aggregations.children.cardinality.value: 2 }

---
parent:
- do:
search:
index: test
body:
size: 0
aggs:
parent:
parent:
type: child
- match: { hits.total.value: 6 }
- match: { aggregations.parent.doc_count: 2 }

---
parent cardinality:
- do:
search:
index: test
body:
size: 0
aggs:
parent:
parent:
type: child
aggs:
cardinality:
cardinality:
field: tag.keyword
- match: { hits.total.value: 6 }
- match: { aggregations.parent.doc_count: 2 }
Original file line number Diff line number Diff line change
Expand Up @@ -250,9 +250,12 @@ public SearchContext context() {

/**
* Called after collection of all document is done.
* <p>
* Warning: this is not final only to allow the parent join aggregator
* to delay this until building buckets.
*/
@Override
public final void postCollection() throws IOException {
public void postCollection() throws IOException {
// post-collect this agg before subs to make it possible to buffer and then replay in postCollection()
doPostCollection();
collectableSubAggregators.postCollection();
Expand Down

0 comments on commit 06acc10

Please sign in to comment.