From aecadfcc61a0182f233d0ba3a1a3ba23c0c9b01c Mon Sep 17 00:00:00 2001 From: Martijn van Groningen Date: Tue, 11 Mar 2014 18:15:27 +0700 Subject: [PATCH] Invoke postCollection on aggregation collectors. Also cleanup how facet and aggs collector are used inside the QueryCollector Closes #5387 --- .../percolator/PercolatorService.java | 2 +- .../percolator/QueryCollector.java | 46 ++++++++----------- 2 files changed, 19 insertions(+), 29 deletions(-) diff --git a/src/main/java/org/elasticsearch/percolator/PercolatorService.java b/src/main/java/org/elasticsearch/percolator/PercolatorService.java index 142d07bbb1e..60ea74997cc 100644 --- a/src/main/java/org/elasticsearch/percolator/PercolatorService.java +++ b/src/main/java/org/elasticsearch/percolator/PercolatorService.java @@ -777,7 +777,7 @@ public class PercolatorService extends AbstractComponent { FilteredQuery query = new FilteredQuery(context.percolateQuery(), percolatorTypeFilter); percolatorSearcher.searcher().search(query, percolateCollector); - for (Collector queryCollector : percolateCollector.facetCollectors) { + for (Collector queryCollector : percolateCollector.facetAndAggregatorCollector) { if (queryCollector instanceof XCollector) { ((XCollector) queryCollector).postCollection(); } diff --git a/src/main/java/org/elasticsearch/percolator/QueryCollector.java b/src/main/java/org/elasticsearch/percolator/QueryCollector.java index a0a43091680..65293411b6b 100644 --- a/src/main/java/org/elasticsearch/percolator/QueryCollector.java +++ b/src/main/java/org/elasticsearch/percolator/QueryCollector.java @@ -19,6 +19,7 @@ package org.elasticsearch.percolator; import com.carrotsearch.hppc.FloatArrayList; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.apache.lucene.index.AtomicReaderContext; import org.apache.lucene.search.*; @@ -61,8 +62,7 @@ abstract class QueryCollector extends Collector { BytesValues values; - final List facetCollectors = new ArrayList(); - final Collector facetAndAggregatorCollector; + final List facetAndAggregatorCollector; QueryCollector(ESLogger logger, PercolateContext context) { this.logger = logger; @@ -71,6 +71,7 @@ abstract class QueryCollector extends Collector { final FieldMapper idMapper = context.mapperService().smartNameFieldMapper(IdFieldMapper.NAME); this.idFieldData = context.fieldData().getForField(idMapper); + ImmutableList.Builder facetAggCollectorBuilder = ImmutableList.builder(); if (context.facets() != null) { for (SearchContextFacets.Entry entry : context.facets().entries()) { if (entry.isGlobal()) { @@ -84,11 +85,10 @@ abstract class QueryCollector extends Collector { collector = new FilteredCollector(collector, entry.getFilter()); } } - facetCollectors.add(collector); + facetAggCollectorBuilder.add(collector); } } - List collectors = new ArrayList(facetCollectors); if (context.aggregations() != null) { AggregationContext aggregationContext = new AggregationContext(context); context.aggregations().aggregationContext(aggregationContext); @@ -105,24 +105,22 @@ abstract class QueryCollector extends Collector { } context.aggregations().aggregators(aggregators); if (!aggregatorCollectors.isEmpty()) { - collectors.add(new AggregationPhase.AggregationsCollector(aggregatorCollectors, aggregationContext)); + facetAggCollectorBuilder.add(new AggregationPhase.AggregationsCollector(aggregatorCollectors, aggregationContext)); } } + facetAndAggregatorCollector = facetAggCollectorBuilder.build(); + } - int size = collectors.size(); - if (size == 0) { - facetAndAggregatorCollector = null; - } else if (size == 1) { - facetAndAggregatorCollector = collectors.get(0); - } else { - facetAndAggregatorCollector = MultiCollector.wrap(collectors.toArray(new Collector[collectors.size()])); + public void postMatch(int doc) throws IOException { + for (Collector collector : facetAndAggregatorCollector) { + collector.collect(doc); } } @Override public void setScorer(Scorer scorer) throws IOException { - if (facetAndAggregatorCollector != null) { - facetAndAggregatorCollector.setScorer(scorer); + for (Collector collector : facetAndAggregatorCollector) { + collector.setScorer(scorer); } } @@ -130,8 +128,8 @@ abstract class QueryCollector extends Collector { public void setNextReader(AtomicReaderContext context) throws IOException { // we use the UID because id might not be indexed values = idFieldData.load(context).getBytesValues(true); - if (facetAndAggregatorCollector != null) { - facetAndAggregatorCollector.setNextReader(context); + for (Collector collector : facetAndAggregatorCollector) { + collector.setNextReader(context); } } @@ -214,9 +212,7 @@ abstract class QueryCollector extends Collector { } } counter++; - if (facetAndAggregatorCollector != null) { - facetAndAggregatorCollector.collect(doc); - } + postMatch(doc); } } catch (IOException e) { logger.warn("[" + spare.bytes.utf8ToString() + "] failed to execute query", e); @@ -259,9 +255,7 @@ abstract class QueryCollector extends Collector { searcher.search(query, collector); if (collector.exists()) { topDocsCollector.collect(doc); - if (facetAndAggregatorCollector != null) { - facetAndAggregatorCollector.collect(doc); - } + postMatch(doc); } } catch (IOException e) { logger.warn("[" + spare.bytes.utf8ToString() + "] failed to execute query", e); @@ -333,9 +327,7 @@ abstract class QueryCollector extends Collector { } } counter++; - if (facetAndAggregatorCollector != null) { - facetAndAggregatorCollector.collect(doc); - } + postMatch(doc); } } catch (IOException e) { logger.warn("[" + spare.bytes.utf8ToString() + "] failed to execute query", e); @@ -385,9 +377,7 @@ abstract class QueryCollector extends Collector { searcher.search(query, collector); if (collector.exists()) { counter++; - if (facetAndAggregatorCollector != null) { - facetAndAggregatorCollector.collect(doc); - } + postMatch(doc); } } catch (IOException e) { logger.warn("[" + spare.bytes.utf8ToString() + "] failed to execute query", e);