Fixed nested facets with filters.

src/main/java/org/elasticsearch/search/facet/FacetPhase.java

@@ -31,6 +31,7 @@ import org.elasticsearch.common.lucene.docset.ContextDocIdSet;
 import org.elasticsearch.common.lucene.search.*;
 import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.SearchPhase;
+import org.elasticsearch.search.facet.nested.NestedFacetExecutor;
 import org.elasticsearch.search.internal.SearchContext;
 import org.elasticsearch.search.query.QueryPhaseExecutionException;
 
@@ -66,10 +67,22 @@ public class FacetPhase implements SearchPhase {
                     continue;
                 }
                 if (entry.getMode() == FacetExecutor.Mode.COLLECTOR) {
+                    // TODO: We can pass the filter as param to collector method, then this filter wrapper logic can
+                    // be moved to NestedFacetExecutor impl, the other implementations would just wrap it into
+                    // FilteredCollector.
                     Collector collector = entry.getFacetExecutor().collector();
+
                     if (entry.getFilter() != null) {
+                        if (collector instanceof NestedFacetExecutor.Collector) {
+                            // We get rootDoc ids as hits in the collect method, so we need to first translate from
+                            // rootDoc hit to nested doc hit and then apply filter.
+                            collector = new NestedFacetExecutor.Collector((NestedFacetExecutor.Collector) collector, entry.getFilter());
+                            // If we would first apply the filter on the rootDoc level and then translate it back to the
+                            // nested docs we ignore the facet filter and all nested docs are passed to facet collector
+                        } else {
                             collector = new FilteredCollector(collector, entry.getFilter());
                         }
+                    }
                     context.searcher().addMainQueryCollector(collector);
                 } else if (entry.getMode() == FacetExecutor.Mode.POST) {
                     context.searcher().enableMainDocIdSetCollector();
@@ -98,8 +111,12 @@ public class FacetPhase implements SearchPhase {
                 if (entry.getMode() == FacetExecutor.Mode.POST) {
                     FacetExecutor.Post post = entry.getFacetExecutor().post();
                     if (entry.getFilter() != null) {
+                        if (post instanceof NestedFacetExecutor.Post) {
+                            post = new NestedFacetExecutor.Post((NestedFacetExecutor.Post) post, entry.getFilter());
+                        } else {
                             post = new FacetExecutor.Post.Filtered(post, entry.getFilter());
                         }
+                    }
                     try {
                         post.executePost(context.searcher().mainDocIdSetCollector().docSets());
                     } catch (Exception e) {
@@ -122,16 +139,25 @@ public class FacetPhase implements SearchPhase {
                     try {
                         FacetExecutor.Post post = entry.getFacetExecutor().post();
                         if (entry.getFilter() != null) {
+                            if (post instanceof NestedFacetExecutor.Post) {
+                                post = new NestedFacetExecutor.Post((NestedFacetExecutor.Post) post, entry.getFilter());
+                            } else {
                                 post = new FacetExecutor.Post.Filtered(post, entry.getFilter());
                             }
+                        }
                         post.executePost(globalDocSets);
                     } catch (Exception e) {
                         throw new QueryPhaseExecutionException(context, "Failed to execute facet [" + entry.getFacetName() + "]", e);
                     }
                 } else if (entry.getMode() == FacetExecutor.Mode.COLLECTOR) {
                     Filter filter = Queries.MATCH_ALL_FILTER;
+                    Collector collector = entry.getFacetExecutor().collector();
                     if (entry.getFilter() != null) {
-                        filter = entry.getFilter();
+                        if (collector instanceof NestedFacetExecutor.Collector) {
+                            collector = new NestedFacetExecutor.Collector((NestedFacetExecutor.Collector) collector, entry.getFilter());
+                        } else {
+                            collector = new FilteredCollector(collector, entry.getFilter());
+                        }
                     }
                     if (filtersByCollector == null) {
                         filtersByCollector = Maps.newHashMap();
@@ -141,7 +167,7 @@ public class FacetPhase implements SearchPhase {
                         list = new ArrayList<Collector>();
                         filtersByCollector.put(filter, list);
                     }
-                    list.add(entry.getFacetExecutor().collector());
+                    list.add(collector);
                 }
             }
         }

src/main/java/org/elasticsearch/search/facet/nested/NestedFacetExecutor.java

@@ -28,6 +28,7 @@ import org.apache.lucene.util.Bits;
 import org.apache.lucene.util.FixedBitSet;
 import org.elasticsearch.common.lucene.docset.ContextDocIdSet;
 import org.elasticsearch.common.lucene.docset.DocIdSets;
+import org.elasticsearch.common.lucene.search.FilteredCollector;
 import org.elasticsearch.common.lucene.search.XCollector;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.mapper.object.ObjectMapper;
@@ -101,6 +102,12 @@ public class NestedFacetExecutor extends FacetExecutor {
             this.childFilter = childFilter;
         }
 
+        public Post(Post post, Filter filter) {
+            this.post = new FacetExecutor.Post.Filtered(post.post, filter);
+            this.parentFilter = post.parentFilter;
+            this.childFilter = post.childFilter;
+        }
+
         @Override
         public void executePost(List<ContextDocIdSet> docSets) throws IOException {
             List<ContextDocIdSet> nestedEntries = new ArrayList<ContextDocIdSet>(docSets.size());
@@ -151,6 +158,13 @@ public class NestedFacetExecutor extends FacetExecutor {
         private Bits childDocs;
         private FixedBitSet parentDocs;
 
+        // We can move
+        public Collector(Collector collector, Filter filter) {
+            this.collector = new FilteredCollector(collector.collector, filter);
+            this.parentFilter = collector.parentFilter;
+            this.childFilter = collector.childFilter;
+        }
+
         public Collector(org.apache.lucene.search.Collector collector, Filter parentFilter, Filter childFilter) {
             this.collector = collector;
             this.parentFilter = parentFilter;

src/test/java/org/elasticsearch/test/integration/nested/SimpleNestedTests.java

@@ -29,6 +29,7 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.index.query.FilterBuilders;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.search.facet.FacetBuilders;
+import org.elasticsearch.search.facet.filter.FilterFacet;
 import org.elasticsearch.search.facet.termsstats.TermsStatsFacet;
 import org.elasticsearch.test.integration.AbstractNodesTests;
 import org.testng.annotations.AfterClass;
@@ -39,7 +40,7 @@ import java.util.Arrays;
 
 import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
 import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
-import static org.elasticsearch.index.query.FilterBuilders.nestedFilter;
+import static org.elasticsearch.index.query.FilterBuilders.*;
 import static org.elasticsearch.index.query.QueryBuilders.*;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
@@ -437,28 +438,52 @@ public class SimpleNestedTests extends AbstractNodesTests {
         assertThat(termsStatsFacet.getEntries().get(3).getCount(), equalTo(1l));
         assertThat(termsStatsFacet.getEntries().get(3).getTotal(), equalTo(12d));
 
-        // test scope ones
+        // test scope ones (collector based)
-//        searchResponse = client.prepareSearch("test")
+        searchResponse = client.prepareSearch("test")
-//                .setQuery(
+                .setQuery(
-//                        nestedQuery("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue"))
+                        nestedQuery("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue"))
-//                )
+                )
-//                .addFacet(
+                .addFacet(
-//                        FacetBuilders.termsStatsFacet("facet1")
+                        FacetBuilders.termsStatsFacet("facet1")
-//                                .keyField("nested1.nested2.field2_1")
+                                .keyField("nested1.nested2.field2_1")
-//                                .valueField("nested1.nested2.field2_2")
+                                .valueField("nested1.nested2.field2_2")
-//                                .nested("nested1.nested2")
+                                .nested("nested1.nested2")
-//                                .facetFilter(nestedFilter("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue")).join(false))
+                                        // Maybe remove the `join` option?
-//                )
+                                        // The following also works:
-//                .execute().actionGet();
+                                        // .facetFilter(termFilter("nested1.nested2.field2_1", "blue"))
-//
+                                .facetFilter(nestedFilter("nested1.nested2", termFilter("nested1.nested2.field2_1", "blue")).join(false))
-//        assertThat(Arrays.toString(searchResponse.shardFailures()), searchResponse.failedShards(), equalTo(0));
+                )
-//        assertThat(searchResponse.hits().totalHits(), equalTo(2l));
+                .execute().actionGet();
-//
+
-//        termsStatsFacet = searchResponse.facets().facet("facet1");
+        assertThat(Arrays.toString(searchResponse.getShardFailures()), searchResponse.getFailedShards(), equalTo(0));
-//        assertThat(termsStatsFacet.getEntries().size(), equalTo(1));
+        assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
-//        assertThat(termsStatsFacet.getEntries().get(0).getTerm().string(), equalTo("blue"));
+
-//        assertThat(termsStatsFacet.getEntries().get(0).getCount(), equalTo(3l));
+        termsStatsFacet = searchResponse.getFacets().facet("facet1");
-//        assertThat(termsStatsFacet.getEntries().get(0).getTotal(), equalTo(8d));
+        assertThat(termsStatsFacet.getEntries().size(), equalTo(1));
+        assertThat(termsStatsFacet.getEntries().get(0).getTerm().string(), equalTo("blue"));
+        assertThat(termsStatsFacet.getEntries().get(0).getCount(), equalTo(3l));
+        assertThat(termsStatsFacet.getEntries().get(0).getTotal(), equalTo(8d));
+
+        // test scope ones (post based)
+        searchResponse = client.prepareSearch("test")
+                .setQuery(
+                        nestedQuery("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue"))
+                )
+                .addFacet(
+                        FacetBuilders.filterFacet("facet1")
+                                .global(true)
+                                .filter(rangeFilter("nested1.nested2.field2_2").gte(0).lte(2))
+                                .nested("nested1.nested2")
+                                .facetFilter(nestedFilter("nested1.nested2", termFilter("nested1.nested2.field2_1", "blue")).join(false))
+                )
+                .execute().actionGet();
+
+        assertThat(Arrays.toString(searchResponse.getShardFailures()), searchResponse.getFailedShards(), equalTo(0));
+        assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
+
+        FilterFacet filterFacet = searchResponse.getFacets().facet("facet1");
+        assertThat(filterFacet.getCount(), equalTo(2l));
+        assertThat(filterFacet.getName(), equalTo("facet1"));
     }
 
     @Test