Fix broken parent and child aggregator (backport #63811) (#63892)

In #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.

modules/parent-join/build.gradle

@@ -27,6 +27,6 @@ esplugin {
 
 restResources {
   restApi {
-    includeCore '_common', 'cluster', 'nodes', 'indices', 'index', 'search'
+    includeCore '_common', 'bulk', 'cluster', 'nodes', 'indices', 'index', 'search'
   }
 }

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

@@ -112,6 +112,11 @@ public abstract class ParentJoinAggregator extends BucketsAggregator implements
         };
     }
 
+    @Override
+    public void postCollection() throws IOException {
+        // Delaying until beforeBuildingBuckets
+    }
+
     @Override
     protected void beforeBuildingBuckets(long[] ordsToCollect) throws IOException {
         IndexReader indexReader = context().searcher().getIndexReader();
@@ -162,6 +167,7 @@ public abstract class ParentJoinAggregator extends BucketsAggregator implements
                 }
             }
         }
+        super.postCollection(); // Run post collection after collecting the sub-aggs
     }
 
     @Override

modules/parent-join/src/test/resources/rest-api-spec/test/40_unconfigred.yml

@@ -1,24 +0,0 @@
----
-"unconfigured":
-  - do:
-      index:
-        index: test
-        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
-        body:
-          size: 0
-          aggs:
-            to-answers:
-              children:
-                type: answer
-  - match: { hits.total.value: 1 }
-  - match: { aggregations.to-answers.doc_count: 0 }

modules/parent-join/src/yamlRestTest/resources/rest-api-spec/test/20_parent_join.yml

@@ -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
+        refresh: true
-        body:  { "id", "1", "join_field": { "name": "parent" } }
+        body:
-
+          - '{ "index": {"_id": "1"}}'
-  - do:
+          - '{ "id": 1, "join_field": { "name": "parent" } }'
-      index:
+          - '{ "index": {"_id": "2"}}'
-        index: test
+          - '{ "id": 2, "join_field": { "name": "parent" } }'
-        id:    2
+          - '{ "index": {"_id": "3", "routing": "1"}}'
-        body:  { "id", "2", "join_field": { "name": "parent" } }
+          - '{ "id": 3, "join_field": { "name": "child", "parent": "1" } }'
-
+          - '{ "index": {"_id": "4", "routing": "1"}}'
-  - do:
+          - '{ "id": 4, "join_field": { "name": "child", "parent": "1" } }'
-      index:
+          - '{ "index": {"_id": "5", "routing": "2"}}'
-        index: test
+          - '{ "id": 5, "join_field": { "name": "child", "parent": "2" } }'
-        id:    3
+          - '{ "index": {"_id": "6", "routing": "1"}}'
-        routing: 1
+          - '{ "id": 6, "join_field": { "name": "grand_child", "parent": "5" } }'
-        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: {}
 
 ---
 teardown:

modules/parent-join/src/yamlRestTest/resources/rest-api-spec/test/40_aggregations.yml

@@ -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 }

server/src/main/java/org/elasticsearch/search/aggregations/AggregatorBase.java

@@ -250,9 +250,12 @@ public abstract class AggregatorBase extends Aggregator {
 
     /**
      * 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();