Sfoglia il codice sorgente

Refactor: Add LeafBucketCollector#isNoop (#69937)

We're getting to the point where it'll be useful to check if the
sub-aggregator's collectors are noops. This adds a method we can call to
check.
Nik Everett 4 anni fa
parent
commit
9334ee915d

+ 12 - 0
server/src/main/java/org/elasticsearch/search/aggregations/LeafBucketCollector.java

@@ -30,6 +30,10 @@ public abstract class LeafBucketCollector implements LeafCollector {
         public void collect(int doc, long bucket) {
             // no-op
         }
+        @Override
+        public boolean isNoop() {
+            return true;
+        }
     };
 
     public static LeafBucketCollector wrap(Iterable<LeafBucketCollector> collectors) {
@@ -91,6 +95,14 @@ public abstract class LeafBucketCollector implements LeafCollector {
      */
     public abstract void collect(int doc, long owningBucketOrd) throws IOException;
 
+    /**
+     * Does this collector collect anything? If this returns true we can safely
+     * just never call {@link #collect}.
+     */
+    public boolean isNoop() {
+        return false;
+    }
+
     @Override
     public final void collect(int doc) throws IOException {
         collect(doc, 0);

+ 6 - 0
server/src/main/java/org/elasticsearch/search/aggregations/MultiBucketCollector.java

@@ -142,6 +142,12 @@ public class MultiBucketCollector extends BucketCollector {
         }
         switch (leafCollectors.size()) {
             case 0:
+                // TODO it's probably safer to return noop and let the caller throw if it wants to
+                /*
+                 * See MinAggregator which only throws if it has a parent.
+                 * That is because it doesn't want there to ever drop
+                 * to this case and throw, thus skipping calculating the parent.
+                 */
                 throw new CollectionTerminatedException();
             case 1:
                 return leafCollectors.get(0);

+ 1 - 1
server/src/main/java/org/elasticsearch/search/aggregations/bucket/terms/GlobalOrdinalsStringTermsAggregator.java

@@ -304,7 +304,7 @@ public class GlobalOrdinalsStringTermsAggregator extends AbstractStringTermsAggr
             }
             final SortedSetDocValues segmentOrds = valuesSource.ordinalsValues(ctx);
             segmentDocCounts = bigArrays().grow(segmentDocCounts, 1 + segmentOrds.getValueCount());
-            assert sub == LeafBucketCollector.NO_OP_COLLECTOR;
+            assert sub.isNoop();
             final SortedDocValues singleValues = DocValues.unwrapSingleton(segmentOrds);
             mapping = valuesSource.globalOrdinalsMapping(ctx);
             // Dense mode doesn't support include/exclude so we don't have to check it here.

+ 1 - 1
x-pack/plugin/analytics/src/main/java/org/elasticsearch/xpack/analytics/topmetrics/TopMetricsAggregator.java

@@ -118,7 +118,7 @@ class TopMetricsAggregator extends NumericMetricsAggregator.MultiValue {
 
     @Override
     public LeafBucketCollector getLeafCollector(LeafReaderContext ctx, LeafBucketCollector sub) throws IOException {
-        assert sub == LeafBucketCollector.NO_OP_COLLECTOR : "Expected noop but was " + sub.toString();
+        assert sub.isNoop() : "Expected noop but was " + sub.toString();
 
         BucketedSort.Leaf leafSort = sort.forLeaf(ctx);