Make sure non-collecting aggs include sub-aggs (backport of #64214) (#64247)

Now that we're consistently using `cat_match` to filter which shards we
run on we can get this confusing case:
1. You have a search with, say, a range and a sub-agg.
2. That search has a query that `can_match` can recognize will match no
   docs. On *any* shard.
3. So we dutifully run it on a single shard so it can produce the
   "empty" aggs.
4. The shard we pick happens to not have the target of the range mapped.
5. This kicks in the special range aggregator that doesn't collect any
   documents.
6. Before this commit, that range aggregator *also* never produced any
   sub-aggs.

So, without this change, it was quite possible for a search that
happened to match no documents to "throw away" the sub-aggs of a range
and a few other aggs.

We've had this problem for a long, long time but it is more confusing
now because `can_match` is really kicking in and causing us to see cases
where it looks like you are targeting a lot of shards but you really are
only targeting a couple. It used to be that to get the "no sub-aggs"
behavior you had to explicitly target only shards that didn't map the
target field of the `range` agg. And, like, in that case it isn't too
bad because you targeted a sort of degenerate shard. But now that
`can_match` is doing its thing you can end up with the confusing steps
above. It took me several hours to track down what what happening I know
how the individual pieces of all of this works. It took four hours to
figure out how they fit together in this case....

Anyway! This replaces all the aggregator implementations that throw out
the sub-aggregators with ones that keep them. I think this'll be less
confusing in the future.

Closes #64142

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

@@ -60,7 +60,7 @@ public class ChildrenAggregatorFactory extends ValuesSourceAggregatorFactory {
 
     @Override
     protected Aggregator createUnmapped(SearchContext searchContext, Aggregator parent, Map<String, Object> metadata) throws IOException {
-        return new NonCollectingAggregator(name, searchContext, parent, metadata) {
+        return new NonCollectingAggregator(name, searchContext, parent, factories, metadata) {
             @Override
             public InternalAggregation buildEmptyAggregation() {
                 return new InternalChildren(name, 0, buildEmptySubAggregations(), metadata());

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

@@ -60,7 +60,7 @@ public class ParentAggregatorFactory extends ValuesSourceAggregatorFactory {
 
     @Override
     protected Aggregator createUnmapped(SearchContext searchContext, Aggregator parent, Map<String, Object> metadata) throws IOException {
-        return new NonCollectingAggregator(name, searchContext, parent, metadata) {
+        return new NonCollectingAggregator(name, searchContext, parent, factories, metadata) {
             @Override
             public InternalAggregation buildEmptyAggregation() {
                 return new InternalParent(name, 0, buildEmptySubAggregations(), metadata());

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

@@ -387,3 +387,43 @@ setup:
   - match: { aggregations.age_groups.buckets.2.key: "Generation Y" }
 
   - match: { aggregations.age_groups.buckets.2.doc_count: 2 }
+
+
+---
+"Date range unmapped with children":
+  - skip:
+      version: " - 7.9.99"
+      reason: Fixed in 7.10.0
+
+  - do:
+      indices.create:
+          index: test_a_unmapped
+          body:
+            settings:
+              number_of_shards: 1
+              number_of_replicas: 0
+  - do:
+      search:
+        index: test_a_unmapped
+        body:
+          size: 0
+          query:
+            terms:
+              animal: []
+          aggs:
+            date_range:
+              date_range:
+                field: date
+                ranges:
+                 - from: 2020-01-01T00:00:00Z
+              aggs:
+                sounds:
+                  cardinality:
+                    field: sound.keyword
+
+  - match: { hits.total.value: 0 }
+  - length: { aggregations.date_range.buckets: 1 }
+  - match: { aggregations.date_range.buckets.0.doc_count: 0 }
+  - match: { aggregations.date_range.buckets.0.key: "2020-01-01T00:00:00.000Z-*" }
+  - is_false: aggregations.date_range.buckets.0.to
+  - match: { aggregations.date_range.buckets.0.sounds.value: 0 }

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

@@ -38,14 +38,6 @@ public abstract class NonCollectingAggregator extends AggregatorBase {
         super(name, subFactories, context, parent, CardinalityUpperBound.NONE, metadata);
     }
 
-    /**
-     * Build a {@linkplain NonCollectingAggregator} for an aggregator without sub-aggregators.
-     */
-    protected NonCollectingAggregator(String name, SearchContext context, Aggregator parent,
-            Map<String, Object> metadata) throws IOException {
-        this(name, context, parent, AggregatorFactories.EMPTY, metadata);
-    }
-
     @Override
     public final LeafBucketCollector getLeafCollector(LeafReaderContext reader, LeafBucketCollector sub) {
         // the framework will automatically eliminate it

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoHashGridAggregatorFactory.java

@@ -63,7 +63,7 @@ public class GeoHashGridAggregatorFactory extends ValuesSourceAggregatorFactory
                                             Aggregator parent,
                                             Map<String, Object> metadata) throws IOException {
         final InternalAggregation aggregation = new InternalGeoHashGrid(name, requiredSize, emptyList(), metadata);
-        return new NonCollectingAggregator(name, searchContext, parent, metadata) {
+        return new NonCollectingAggregator(name, searchContext, parent, factories, metadata) {
             @Override
             public InternalAggregation buildEmptyAggregation() {
                 return aggregation;

server/src/main/java/org/elasticsearch/search/aggregations/bucket/geogrid/GeoTileGridAggregatorFactory.java

@@ -61,7 +61,7 @@ public class GeoTileGridAggregatorFactory extends ValuesSourceAggregatorFactory
                                             Aggregator parent,
                                             Map<String, Object> metadata) throws IOException {
         final InternalAggregation aggregation = new InternalGeoTileGrid(name, requiredSize, Collections.emptyList(), metadata);
-        return new NonCollectingAggregator(name, searchContext, parent, metadata) {
+        return new NonCollectingAggregator(name, searchContext, parent, factories, metadata) {
             @Override
             public InternalAggregation buildEmptyAggregation() {
                 return aggregation;

server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/NestedAggregatorFactory.java

@@ -51,7 +51,7 @@ public class NestedAggregatorFactory extends AggregatorFactory {
                                         CardinalityUpperBound cardinality,
                                         Map<String, Object> metadata) throws IOException {
         if (childObjectMapper == null) {
-            return new Unmapped(name, searchContext, parent, metadata);
+            return new Unmapped(name, searchContext, parent, factories, metadata);
         }
         return new NestedAggregator(name, factories, parentObjectMapper, childObjectMapper, searchContext, parent,
             cardinality, metadata);
@@ -62,8 +62,9 @@ public class NestedAggregatorFactory extends AggregatorFactory {
         Unmapped(String name,
                     SearchContext context,
                     Aggregator parent,
+                    AggregatorFactories factories,
                     Map<String, Object> metadata) throws IOException {
-            super(name, context, parent, metadata);
+            super(name, context, parent, factories, metadata);
         }
 
         @Override

server/src/main/java/org/elasticsearch/search/aggregations/bucket/nested/ReverseNestedAggregatorFactory.java

@@ -52,7 +52,7 @@ public class ReverseNestedAggregatorFactory extends AggregatorFactory {
                                         CardinalityUpperBound cardinality,
                                         Map<String, Object> metadata) throws IOException {
         if (unmapped) {
-            return new Unmapped(name, searchContext, parent, metadata);
+            return new Unmapped(name, searchContext, parent, factories, metadata);
         } else {
             return new ReverseNestedAggregator(name, factories, parentObjectMapper,
                 searchContext, parent, cardinality, metadata);
@@ -64,8 +64,9 @@ public class ReverseNestedAggregatorFactory extends AggregatorFactory {
         Unmapped(String name,
                     SearchContext context,
                     Aggregator parent,
+                    AggregatorFactories factories,
                     Map<String, Object> metadata) throws IOException {
-            super(name, context, parent, metadata);
+            super(name, context, parent, factories, metadata);
         }
 
         @Override

server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/AbstractRangeAggregatorFactory.java

@@ -75,7 +75,7 @@ public class AbstractRangeAggregatorFactory<R extends Range> extends ValuesSourc
     protected Aggregator createUnmapped(SearchContext searchContext,
                                             Aggregator parent,
                                             Map<String, Object> metadata) throws IOException {
-        return new Unmapped<>(name, ranges, keyed, config.format(), searchContext, parent, rangeFactory, metadata);
+        return new Unmapped<>(name, factories, ranges, keyed, config.format(), searchContext, parent, rangeFactory, metadata);
     }
 
     @Override

server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/GeoDistanceRangeAggregatorFactory.java

@@ -107,7 +107,7 @@ public class GeoDistanceRangeAggregatorFactory extends ValuesSourceAggregatorFac
     protected Aggregator createUnmapped(SearchContext searchContext,
                                             Aggregator parent,
                                             Map<String, Object> metadata) throws IOException {
-        return new RangeAggregator.Unmapped<>(name, ranges, keyed, config.format(), searchContext, parent,
+        return new RangeAggregator.Unmapped<>(name, factories, ranges, keyed, config.format(), searchContext, parent,
             rangeFactory, metadata);
     }
 

server/src/main/java/org/elasticsearch/search/aggregations/bucket/range/RangeAggregator.java

@@ -353,11 +353,19 @@ public class RangeAggregator extends BucketsAggregator {
         private final InternalRange.Factory factory;
         private final DocValueFormat format;
 
-        public Unmapped(String name, R[] ranges, boolean keyed, DocValueFormat format, SearchContext context, Aggregator parent,
+        public Unmapped(
-                InternalRange.Factory factory, Map<String, Object> metadata)
+            String name,
-                throws IOException {
+            AggregatorFactories factories,
+            R[] ranges,
+            boolean keyed,
+            DocValueFormat format,
+            SearchContext context,
+            Aggregator parent,
+            InternalRange.Factory factory,
+            Map<String, Object> metadata
+        ) throws IOException {
 
-            super(name, context, parent, metadata);
+            super(name, context, parent, factories, metadata);
             this.ranges = ranges;
             this.keyed = keyed;
             this.format = format;

server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/SignificantTermsAggregatorFactory.java

@@ -186,7 +186,7 @@ public class SignificantTermsAggregatorFactory extends ValuesSourceAggregatorFac
                                             Map<String, Object> metadata) throws IOException {
         final InternalAggregation aggregation = new UnmappedSignificantTerms(name, bucketCountThresholds.getRequiredSize(),
                 bucketCountThresholds.getMinDocCount(), metadata);
-        return new NonCollectingAggregator(name, searchContext, parent, metadata) {
+        return new NonCollectingAggregator(name, searchContext, parent, factories, metadata) {
             @Override
             public InternalAggregation buildEmptyAggregation() {
                 return aggregation;