Skip to content

Commit ac2e6a3

Browse files
martijnvgimotov
martijnvg
authored and
imotov
committed
Fixed nested facets with filters.
1 parent 24291d4 commit ac2e6a3

File tree

3 files changed

+93
-28
lines changed

3 files changed

+93
-28
lines changed

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

+31-5
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.elasticsearch.common.lucene.search.*;
3232
import org.elasticsearch.search.SearchParseElement;
3333
import org.elasticsearch.search.SearchPhase;
34+
import org.elasticsearch.search.facet.nested.NestedFacetExecutor;
3435
import org.elasticsearch.search.internal.SearchContext;
3536
import org.elasticsearch.search.query.QueryPhaseExecutionException;
3637

@@ -66,9 +67,21 @@ public void preProcess(SearchContext context) {
6667
continue;
6768
}
6869
if (entry.getMode() == FacetExecutor.Mode.COLLECTOR) {
70+
// TODO: We can pass the filter as param to collector method, then this filter wrapper logic can
71+
// be moved to NestedFacetExecutor impl, the other implementations would just wrap it into
72+
// FilteredCollector.
6973
Collector collector = entry.getFacetExecutor().collector();
74+
7075
if (entry.getFilter() != null) {
71-
collector = new FilteredCollector(collector, entry.getFilter());
76+
if (collector instanceof NestedFacetExecutor.Collector) {
77+
// We get rootDoc ids as hits in the collect method, so we need to first translate from
78+
// rootDoc hit to nested doc hit and then apply filter.
79+
collector = new NestedFacetExecutor.Collector((NestedFacetExecutor.Collector) collector, entry.getFilter());
80+
// If we would first apply the filter on the rootDoc level and then translate it back to the
81+
// nested docs we ignore the facet filter and all nested docs are passed to facet collector
82+
} else {
83+
collector = new FilteredCollector(collector, entry.getFilter());
84+
}
7285
}
7386
context.searcher().addMainQueryCollector(collector);
7487
} else if (entry.getMode() == FacetExecutor.Mode.POST) {
@@ -98,7 +111,11 @@ public void execute(SearchContext context) throws ElasticSearchException {
98111
if (entry.getMode() == FacetExecutor.Mode.POST) {
99112
FacetExecutor.Post post = entry.getFacetExecutor().post();
100113
if (entry.getFilter() != null) {
101-
post = new FacetExecutor.Post.Filtered(post, entry.getFilter());
114+
if (post instanceof NestedFacetExecutor.Post) {
115+
post = new NestedFacetExecutor.Post((NestedFacetExecutor.Post) post, entry.getFilter());
116+
} else {
117+
post = new FacetExecutor.Post.Filtered(post, entry.getFilter());
118+
}
102119
}
103120
try {
104121
post.executePost(context.searcher().mainDocIdSetCollector().docSets());
@@ -122,16 +139,25 @@ public void execute(SearchContext context) throws ElasticSearchException {
122139
try {
123140
FacetExecutor.Post post = entry.getFacetExecutor().post();
124141
if (entry.getFilter() != null) {
125-
post = new FacetExecutor.Post.Filtered(post, entry.getFilter());
142+
if (post instanceof NestedFacetExecutor.Post) {
143+
post = new NestedFacetExecutor.Post((NestedFacetExecutor.Post) post, entry.getFilter());
144+
} else {
145+
post = new FacetExecutor.Post.Filtered(post, entry.getFilter());
146+
}
126147
}
127148
post.executePost(globalDocSets);
128149
} catch (Exception e) {
129150
throw new QueryPhaseExecutionException(context, "Failed to execute facet [" + entry.getFacetName() + "]", e);
130151
}
131152
} else if (entry.getMode() == FacetExecutor.Mode.COLLECTOR) {
132153
Filter filter = Queries.MATCH_ALL_FILTER;
154+
Collector collector = entry.getFacetExecutor().collector();
133155
if (entry.getFilter() != null) {
134-
filter = entry.getFilter();
156+
if (collector instanceof NestedFacetExecutor.Collector) {
157+
collector = new NestedFacetExecutor.Collector((NestedFacetExecutor.Collector) collector, entry.getFilter());
158+
} else {
159+
collector = new FilteredCollector(collector, entry.getFilter());
160+
}
135161
}
136162
if (filtersByCollector == null) {
137163
filtersByCollector = Maps.newHashMap();
@@ -141,7 +167,7 @@ public void execute(SearchContext context) throws ElasticSearchException {
141167
list = new ArrayList<Collector>();
142168
filtersByCollector.put(filter, list);
143169
}
144-
list.add(entry.getFacetExecutor().collector());
170+
list.add(collector);
145171
}
146172
}
147173
}

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

+14
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.apache.lucene.util.FixedBitSet;
2929
import org.elasticsearch.common.lucene.docset.ContextDocIdSet;
3030
import org.elasticsearch.common.lucene.docset.DocIdSets;
31+
import org.elasticsearch.common.lucene.search.FilteredCollector;
3132
import org.elasticsearch.common.lucene.search.XCollector;
3233
import org.elasticsearch.index.mapper.MapperService;
3334
import org.elasticsearch.index.mapper.object.ObjectMapper;
@@ -101,6 +102,12 @@ public Post(FacetExecutor.Post post, Filter parentFilter, Filter childFilter) {
101102
this.childFilter = childFilter;
102103
}
103104

105+
public Post(Post post, Filter filter) {
106+
this.post = new FacetExecutor.Post.Filtered(post.post, filter);
107+
this.parentFilter = post.parentFilter;
108+
this.childFilter = post.childFilter;
109+
}
110+
104111
@Override
105112
public void executePost(List<ContextDocIdSet> docSets) throws IOException {
106113
List<ContextDocIdSet> nestedEntries = new ArrayList<ContextDocIdSet>(docSets.size());
@@ -151,6 +158,13 @@ public static class Collector extends FacetExecutor.Collector {
151158
private Bits childDocs;
152159
private FixedBitSet parentDocs;
153160

161+
// We can move
162+
public Collector(Collector collector, Filter filter) {
163+
this.collector = new FilteredCollector(collector.collector, filter);
164+
this.parentFilter = collector.parentFilter;
165+
this.childFilter = collector.childFilter;
166+
}
167+
154168
public Collector(org.apache.lucene.search.Collector collector, Filter parentFilter, Filter childFilter) {
155169
this.collector = collector;
156170
this.parentFilter = parentFilter;

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

+48-23
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.elasticsearch.index.query.FilterBuilders;
3030
import org.elasticsearch.index.query.QueryBuilders;
3131
import org.elasticsearch.search.facet.FacetBuilders;
32+
import org.elasticsearch.search.facet.filter.FilterFacet;
3233
import org.elasticsearch.search.facet.termsstats.TermsStatsFacet;
3334
import org.elasticsearch.test.integration.AbstractNodesTests;
3435
import org.testng.annotations.AfterClass;
@@ -39,7 +40,7 @@
3940

4041
import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilder;
4142
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
42-
import static org.elasticsearch.index.query.FilterBuilders.nestedFilter;
43+
import static org.elasticsearch.index.query.FilterBuilders.*;
4344
import static org.elasticsearch.index.query.QueryBuilders.*;
4445
import static org.hamcrest.MatcherAssert.assertThat;
4546
import static org.hamcrest.Matchers.equalTo;
@@ -437,28 +438,52 @@ private void testFacets(int numberOfShards) throws Exception {
437438
assertThat(termsStatsFacet.getEntries().get(3).getCount(), equalTo(1l));
438439
assertThat(termsStatsFacet.getEntries().get(3).getTotal(), equalTo(12d));
439440

440-
// test scope ones
441-
// searchResponse = client.prepareSearch("test")
442-
// .setQuery(
443-
// nestedQuery("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue"))
444-
// )
445-
// .addFacet(
446-
// FacetBuilders.termsStatsFacet("facet1")
447-
// .keyField("nested1.nested2.field2_1")
448-
// .valueField("nested1.nested2.field2_2")
449-
// .nested("nested1.nested2")
450-
// .facetFilter(nestedFilter("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue")).join(false))
451-
// )
452-
// .execute().actionGet();
453-
//
454-
// assertThat(Arrays.toString(searchResponse.shardFailures()), searchResponse.failedShards(), equalTo(0));
455-
// assertThat(searchResponse.hits().totalHits(), equalTo(2l));
456-
//
457-
// termsStatsFacet = searchResponse.facets().facet("facet1");
458-
// assertThat(termsStatsFacet.getEntries().size(), equalTo(1));
459-
// assertThat(termsStatsFacet.getEntries().get(0).getTerm().string(), equalTo("blue"));
460-
// assertThat(termsStatsFacet.getEntries().get(0).getCount(), equalTo(3l));
461-
// assertThat(termsStatsFacet.getEntries().get(0).getTotal(), equalTo(8d));
441+
// test scope ones (collector based)
442+
searchResponse = client.prepareSearch("test")
443+
.setQuery(
444+
nestedQuery("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue"))
445+
)
446+
.addFacet(
447+
FacetBuilders.termsStatsFacet("facet1")
448+
.keyField("nested1.nested2.field2_1")
449+
.valueField("nested1.nested2.field2_2")
450+
.nested("nested1.nested2")
451+
// Maybe remove the `join` option?
452+
// The following also works:
453+
// .facetFilter(termFilter("nested1.nested2.field2_1", "blue"))
454+
.facetFilter(nestedFilter("nested1.nested2", termFilter("nested1.nested2.field2_1", "blue")).join(false))
455+
)
456+
.execute().actionGet();
457+
458+
assertThat(Arrays.toString(searchResponse.getShardFailures()), searchResponse.getFailedShards(), equalTo(0));
459+
assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
460+
461+
termsStatsFacet = searchResponse.getFacets().facet("facet1");
462+
assertThat(termsStatsFacet.getEntries().size(), equalTo(1));
463+
assertThat(termsStatsFacet.getEntries().get(0).getTerm().string(), equalTo("blue"));
464+
assertThat(termsStatsFacet.getEntries().get(0).getCount(), equalTo(3l));
465+
assertThat(termsStatsFacet.getEntries().get(0).getTotal(), equalTo(8d));
466+
467+
// test scope ones (post based)
468+
searchResponse = client.prepareSearch("test")
469+
.setQuery(
470+
nestedQuery("nested1.nested2", termQuery("nested1.nested2.field2_1", "blue"))
471+
)
472+
.addFacet(
473+
FacetBuilders.filterFacet("facet1")
474+
.global(true)
475+
.filter(rangeFilter("nested1.nested2.field2_2").gte(0).lte(2))
476+
.nested("nested1.nested2")
477+
.facetFilter(nestedFilter("nested1.nested2", termFilter("nested1.nested2.field2_1", "blue")).join(false))
478+
)
479+
.execute().actionGet();
480+
481+
assertThat(Arrays.toString(searchResponse.getShardFailures()), searchResponse.getFailedShards(), equalTo(0));
482+
assertThat(searchResponse.getHits().totalHits(), equalTo(2l));
483+
484+
FilterFacet filterFacet = searchResponse.getFacets().facet("facet1");
485+
assertThat(filterFacet.getCount(), equalTo(2l));
486+
assertThat(filterFacet.getName(), equalTo("facet1"));
462487
}
463488

464489
@Test

0 commit comments

Comments
 (0)