Added support for named filters in top-level filter

Closes #3097
elastic · Aug 2, 2013 · 85b7efa · 85b7efa
1 parent bd32467
commit 85b7efa
Show file tree

Hide file tree

Showing 14 changed files with 157 additions and 42 deletions.
diff --git a/src/main/java/org/elasticsearch/index/aliases/IndexAliasesService.java b/src/main/java/org/elasticsearch/index/aliases/IndexAliasesService.java
@@ -134,7 +134,7 @@ private Filter parse(String alias, CompressedString filter) {
             byte[] filterSource = filter.uncompressed();
             XContentParser parser = XContentFactory.xContent(filterSource).createParser(filterSource);
             try {
-                return indexQueryParser.parseInnerFilter(parser);
+                return indexQueryParser.parseInnerFilter(parser).filter();
             } finally {
                 parser.close();
             }

diff --git a/src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java b/src/main/java/org/elasticsearch/index/query/IndexQueryParserService.java
@@ -20,7 +20,6 @@
 package org.elasticsearch.index.query;
 
 import com.google.common.collect.ImmutableMap;
-import org.apache.lucene.search.Filter;
 import org.apache.lucene.search.Query;
 import org.apache.lucene.util.CloseableThreadLocal;
 import org.elasticsearch.ElasticSearchException;
@@ -259,10 +258,10 @@ public ParsedQuery parse(XContentParser parser) {
     }
 
     @Nullable
-    public Filter parseInnerFilter(XContentParser parser) throws IOException {
+    public ParsedFilter parseInnerFilter(XContentParser parser) throws IOException {
         QueryParseContext context = cache.get();
         context.reset(parser);
-        return context.parseInnerFilter();
+        return new ParsedFilter(context.parseInnerFilter(), context.copyNamedFilters());
     }
 
     @Nullable

diff --git a/src/main/java/org/elasticsearch/index/query/ParsedFilter.java b/src/main/java/org/elasticsearch/index/query/ParsedFilter.java
@@ -0,0 +1,27 @@
+package org.elasticsearch.index.query;
+
+import com.google.common.collect.ImmutableMap;
+import org.apache.lucene.search.Filter;
+import org.elasticsearch.common.lucene.search.Queries;
+
+public class ParsedFilter {
+
+    public static final ParsedFilter EMPTY = new ParsedFilter(Queries.MATCH_NO_FILTER, ImmutableMap.<String, Filter>of());
+
+    private final Filter filter;
+
+    private final ImmutableMap<String, Filter> namedFilters;
+
+    public ParsedFilter(Filter filter, ImmutableMap<String, Filter> namedFilters) {
+        this.filter = filter;
+        this.namedFilters = namedFilters;
+    }
+
+    public Filter filter() {
+        return filter;
+    }
+
+    public ImmutableMap<String, Filter> namedFilters() {
+        return namedFilters;
+    }
+}
diff --git a/src/main/java/org/elasticsearch/percolator/PercolatorService.java b/src/main/java/org/elasticsearch/percolator/PercolatorService.java
@@ -254,7 +254,7 @@ private Tuple<ParsedDocument, Query> parsePercolate(IndexService documentIndexSe
                         if (query != null) {
                             throw new ElasticSearchParseException("Either specify query or filter, not both");
                         }
-                        Filter filter = documentIndexService.queryParserService().parseInnerFilter(parser);
+                        Filter filter = documentIndexService.queryParserService().parseInnerFilter(parser).filter();
                         query = new XConstantScoreQuery(filter);
                     }
                 } else if (token == null) {
@@ -323,7 +323,7 @@ private Query parseQueryOrFilter(IndexService documentIndexService, BytesReferen
                         if (query != null) {
                             throw new ElasticSearchParseException("Either specify query or filter, not both");
                         }
-                        Filter filter = documentIndexService.queryParserService().parseInnerFilter(parser);
+                        Filter filter = documentIndexService.queryParserService().parseInnerFilter(parser).filter();
                         query = new XConstantScoreQuery(filter);
                     }
                 } else if (token == null) {

diff --git a/src/main/java/org/elasticsearch/search/facet/FacetParseElement.java b/src/main/java/org/elasticsearch/search/facet/FacetParseElement.java
@@ -83,7 +83,7 @@ public void parse(XContentParser parser, SearchContext context) throws Exception
                         fieldName = parser.currentName();
                     } else if (token == XContentParser.Token.START_OBJECT) {
                         if ("facet_filter".equals(fieldName) || "facetFilter".equals(fieldName)) {
-                            filter = context.queryParserService().parseInnerFilter(parser);
+                            filter = context.queryParserService().parseInnerFilter(parser).filter();
                         } else {
                             FacetParser facetParser = facetParsers.parser(fieldName);
                             if (facetParser == null) {

diff --git a/src/main/java/org/elasticsearch/search/facet/filter/FilterFacetParser.java b/src/main/java/org/elasticsearch/search/facet/filter/FilterFacetParser.java
@@ -58,7 +58,7 @@ public FacetExecutor.Mode defaultGlobalMode() {
 
     @Override
     public FacetExecutor parse(String facetName, XContentParser parser, SearchContext context) throws IOException {
-        Filter facetFilter = context.queryParserService().parseInnerFilter(parser);
+        Filter facetFilter = context.queryParserService().parseInnerFilter(parser).filter();
         return new FilterFacetExecutor(facetFilter);
     }
 }
diff --git a/src/main/java/org/elasticsearch/search/fetch/matchedfilters/MatchedFiltersFetchSubPhase.java b/src/main/java/org/elasticsearch/search/fetch/matchedfilters/MatchedFiltersFetchSubPhase.java
@@ -56,13 +56,25 @@ public void hitsExecute(SearchContext context, InternalSearchHit[] hits) throws
 
     @Override
     public boolean hitExecutionNeeded(SearchContext context) {
-        return !context.parsedQuery().namedFilters().isEmpty();
+        return !context.parsedQuery().namedFilters().isEmpty()
+                || (context.parsedFilter() !=null && !context.parsedFilter().namedFilters().isEmpty());
     }
 
     @Override
     public void hitExecute(SearchContext context, HitContext hitContext) throws ElasticSearchException {
         List<String> matchedFilters = Lists.newArrayListWithCapacity(2);
-        for (Map.Entry<String, Filter> entry : context.parsedQuery().namedFilters().entrySet()) {
+
+        addMatchedFilters(hitContext, context.parsedQuery().namedFilters(), matchedFilters);
+
+        if (context.parsedFilter() != null) {
+            addMatchedFilters(hitContext, context.parsedFilter().namedFilters(), matchedFilters);
+        }
+
+        hitContext.hit().matchedFilters(matchedFilters.toArray(new String[matchedFilters.size()]));
+    }
+
+    private void addMatchedFilters(HitContext hitContext, ImmutableMap<String, Filter> namedFilters, List<String> matchedFilters) {
+        for (Map.Entry<String, Filter> entry : namedFilters.entrySet()) {
             String name = entry.getKey();
             Filter filter = entry.getValue();
             try {
@@ -77,6 +89,5 @@ public void hitExecute(SearchContext context, HitContext hitContext) throws Elas
                 // ignore
             }
         }
-        hitContext.hit().matchedFilters(matchedFilters.toArray(new String[matchedFilters.size()]));
     }
 }
diff --git a/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java b/src/main/java/org/elasticsearch/search/internal/ContextIndexSearcher.java
@@ -139,7 +139,7 @@ public void search(List<AtomicReaderContext> leaves, Weight weight, Collector co
                 // this will only get applied to the actual search collector and not
                 // to any scoped collectors, also, it will only be applied to the main collector
                 // since that is where the filter should only work
-                collector = new FilteredCollector(collector, searchContext.parsedFilter());
+                collector = new FilteredCollector(collector, searchContext.parsedFilter().filter());
             }
             if (queryCollectors != null && !queryCollectors.isEmpty()) {
                 collector = new MultiCollector(collector, queryCollectors.toArray(new Collector[queryCollectors.size()]));

diff --git a/src/main/java/org/elasticsearch/search/internal/SearchContext.java b/src/main/java/org/elasticsearch/search/internal/SearchContext.java
@@ -45,6 +45,7 @@
 import org.elasticsearch.index.mapper.FieldMappers;
 import org.elasticsearch.index.mapper.MapperService;
 import org.elasticsearch.index.query.IndexQueryParserService;
+import org.elasticsearch.index.query.ParsedFilter;
 import org.elasticsearch.index.query.ParsedQuery;
 import org.elasticsearch.index.query.QueryParseContext;
 import org.elasticsearch.index.service.IndexService;
@@ -159,7 +160,7 @@ public static interface Rewrite {
 
     private Query query;
 
-    private Filter filter;
+    private ParsedFilter filter;
 
     private Filter aliasFilter;
 
@@ -476,12 +477,12 @@ public boolean trackScores() {
         return this.trackScores;
     }
 
-    public SearchContext parsedFilter(Filter filter) {
+    public SearchContext parsedFilter(ParsedFilter filter) {
         this.filter = filter;
         return this;
     }
 
-    public Filter parsedFilter() {
+    public ParsedFilter parsedFilter() {
         return this.filter;
     }
 

diff --git a/src/main/java/org/elasticsearch/search/query/FilterParseElement.java b/src/main/java/org/elasticsearch/search/query/FilterParseElement.java
@@ -19,9 +19,8 @@
 
 package org.elasticsearch.search.query;
 
-import org.apache.lucene.search.Filter;
-import org.elasticsearch.common.lucene.search.Queries;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.index.query.ParsedFilter;
 import org.elasticsearch.search.SearchParseElement;
 import org.elasticsearch.search.internal.SearchContext;
 
@@ -32,9 +31,9 @@ public class FilterParseElement implements SearchParseElement {
 
     @Override
     public void parse(XContentParser parser, SearchContext context) throws Exception {
-        Filter filter = context.queryParserService().parseInnerFilter(parser);
+        ParsedFilter filter = context.queryParserService().parseInnerFilter(parser);
         if (filter == null) {
-            filter = Queries.MATCH_NO_FILTER;
+            filter = ParsedFilter.EMPTY;
         }
         context.parsedFilter(filter);
     }

diff --git a/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java b/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortParser.java
@@ -23,7 +23,6 @@
 import org.apache.lucene.search.SortField;
 import org.elasticsearch.ElasticSearchIllegalArgumentException;
 import org.elasticsearch.common.geo.GeoDistance;
-import org.elasticsearch.common.geo.GeoHashUtils;
 import org.elasticsearch.common.geo.GeoPoint;
 import org.elasticsearch.common.geo.GeoUtils;
 import org.elasticsearch.common.unit.DistanceUnit;
@@ -34,7 +33,6 @@
 import org.elasticsearch.index.fielddata.fieldcomparator.SortMode;
 import org.elasticsearch.index.mapper.FieldMapper;
 import org.elasticsearch.index.mapper.ObjectMappers;
-import org.elasticsearch.index.mapper.geo.GeoPointFieldMapper;
 import org.elasticsearch.index.mapper.object.ObjectMapper;
 import org.elasticsearch.index.search.nested.NestedFieldComparatorSource;
 import org.elasticsearch.index.search.nested.NonNestedDocsFilter;
@@ -75,7 +73,7 @@ public SortField parse(XContentParser parser, SearchContext context) throws Exce
             } else if (token == XContentParser.Token.START_OBJECT) {
                 // the json in the format of -> field : { lat : 30, lon : 12 }
                 if ("nested_filter".equals(currentName) || "nestedFilter".equals(currentName)) {
-                    nestedFilter = context.queryParserService().parseInnerFilter(parser);
+                    nestedFilter = context.queryParserService().parseInnerFilter(parser).filter();
                 } else {
                     fieldName = currentName;
                     GeoPoint.parse(parser, point);

diff --git a/src/main/java/org/elasticsearch/search/sort/ScriptSortParser.java b/src/main/java/org/elasticsearch/search/sort/ScriptSortParser.java
@@ -67,7 +67,7 @@ public SortField parse(XContentParser parser, SearchContext context) throws Exce
                 if ("params".equals(currentName)) {
                     params = parser.map();
                 } else if ("nested_filter".equals(currentName) || "nestedFilter".equals(currentName)) {
-                    nestedFilter = context.queryParserService().parseInnerFilter(parser);
+                    nestedFilter = context.queryParserService().parseInnerFilter(parser).filter();
                 }
             } else if (token.isValue()) {
                 if ("reverse".equals(currentName)) {

diff --git a/src/main/java/org/elasticsearch/search/sort/SortParseElement.java b/src/main/java/org/elasticsearch/search/sort/SortParseElement.java
@@ -161,7 +161,7 @@ private void addCompoundSortField(XContentParser parser, SearchContext context,
                                 }
                             } else if (token == XContentParser.Token.START_OBJECT) {
                                 if ("nested_filter".equals(innerJsonName) || "nestedFilter".equals(innerJsonName)) {
-                                    nestedFilter = context.queryParserService().parseInnerFilter(parser);
+                                    nestedFilter = context.queryParserService().parseInnerFilter(parser).filter();
                                 } else {
                                     throw new ElasticSearchIllegalArgumentException("sort option [" + innerJsonName + "] not supported");
                                 }

diff --git a/...st/java/org/elasticsearch/test/integration/search/matchedfilters/MatchedFiltersTests.java b/...st/java/org/elasticsearch/test/integration/search/matchedfilters/MatchedFiltersTests.java
@@ -19,33 +19,26 @@
 
 package org.elasticsearch.test.integration.search.matchedfilters;
 
-import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
-import static org.elasticsearch.index.query.FilterBuilders.orFilter;
-import static org.elasticsearch.index.query.FilterBuilders.rangeFilter;
-import static org.elasticsearch.index.query.QueryBuilders.filteredQuery;
-import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.hasItemInArray;
-
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.common.Priority;
 import org.elasticsearch.search.SearchHit;
 import org.elasticsearch.test.integration.AbstractSharedClusterTest;
 import org.junit.Test;
 
+import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
+import static org.elasticsearch.index.query.FilterBuilders.*;
+import static org.elasticsearch.index.query.QueryBuilders.filteredQuery;
+import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery;
+import static org.hamcrest.Matchers.*;
+
 /**
  *
  */
 public class MatchedFiltersTests extends AbstractSharedClusterTest {
 
     @Test
-    public void simpleMatchedFilter() throws Exception {
-        try {
-            client().admin().indices().prepareDelete("test").execute().actionGet();
-        } catch (Exception e) {
-            // ignore
-        }
+    public void simpleMatchedFilterFromFilteredQuery() throws Exception {
+
         client().admin().indices().prepareCreate("test").execute().actionGet();
         client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().execute().actionGet();
 
@@ -59,8 +52,6 @@ public void simpleMatchedFilter() throws Exception {
                 .field("number", 2)
                 .endObject()).execute().actionGet();
 
-        client().admin().indices().prepareFlush().execute().actionGet();
-
         client().prepareIndex("test", "type1", "3").setSource(jsonBuilder().startObject()
                 .field("name", "test3")
                 .field("number", 3)
@@ -72,6 +63,8 @@ public void simpleMatchedFilter() throws Exception {
                 .setQuery(filteredQuery(matchAllQuery(), orFilter(rangeFilter("number").lte(2).filterName("test1"), rangeFilter("number").gt(2).filterName("test2"))))
                 .execute().actionGet();
 
+
+        assertThat(searchResponse.getShardFailures().length, equalTo(0));
         assertThat(searchResponse.getHits().totalHits(), equalTo(3l));
         for (SearchHit hit : searchResponse.getHits()) {
             if (hit.id().equals("1") || hit.id().equals("2")) {
@@ -81,7 +74,94 @@ public void simpleMatchedFilter() throws Exception {
                 assertThat(hit.matchedFilters().length, equalTo(1));
                 assertThat(hit.matchedFilters(), hasItemInArray("test2"));
             } else {
-                assert false;
+                fail("Unexpected document returned with id " + hit.id());
+            }
+        }
+    }
+
+    @Test
+    public void simpleMatchedFilterFromTopLevelFilter() throws Exception {
+
+        client().admin().indices().prepareCreate("test").execute().actionGet();
+        client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().execute().actionGet();
+
+        client().prepareIndex("test", "type1", "1").setSource(jsonBuilder().startObject()
+                .field("name", "test")
+                .field("title", "title1")
+                .endObject()).execute().actionGet();
+
+        client().prepareIndex("test", "type1", "2").setSource(jsonBuilder().startObject()
+                .field("name", "test")
+                .endObject()).execute().actionGet();
+
+        client().prepareIndex("test", "type1", "3").setSource(jsonBuilder().startObject()
+                .field("name", "test")
+                .endObject()).execute().actionGet();
+
+        client().admin().indices().prepareRefresh().execute().actionGet();
+
+        SearchResponse searchResponse = client().prepareSearch()
+                .setQuery(matchAllQuery())
+                .setFilter(orFilter(
+                        termFilter("name", "test").filterName("name"),
+                        termFilter("title", "title1").filterName("title")))
+                .execute().actionGet();
+
+        assertThat(searchResponse.getShardFailures().length, equalTo(0));
+        assertThat(searchResponse.getHits().totalHits(), equalTo(3l));
+
+        for (SearchHit hit : searchResponse.getHits()) {
+            if (hit.id().equals("1")) {
+                assertThat(hit.matchedFilters().length, equalTo(2));
+                assertThat(hit.matchedFilters(), hasItemInArray("name"));
+                assertThat(hit.matchedFilters(), hasItemInArray("title"));
+            } else if (hit.id().equals("2") || hit.id().equals("3")) {
+                assertThat(hit.matchedFilters().length, equalTo(1));
+                assertThat(hit.matchedFilters(), hasItemInArray("name"));
+            } else {
+                fail("Unexpected document returned with id " + hit.id());
+            }
+        }
+    }
+
+    @Test
+    public void simpleMatchedFilterFromTopLevelFilterAndFilteredQuery() throws Exception {
+
+        client().admin().indices().prepareCreate("test").execute().actionGet();
+        client().admin().cluster().prepareHealth().setWaitForEvents(Priority.LANGUID).setWaitForGreenStatus().execute().actionGet();
+
+        client().prepareIndex("test", "type1", "1").setSource(jsonBuilder().startObject()
+                .field("name", "test")
+                .field("title", "title1")
+                .endObject()).execute().actionGet();
+
+        client().prepareIndex("test", "type1", "2").setSource(jsonBuilder().startObject()
+                .field("name", "test")
+                .field("title", "title2")
+                .endObject()).execute().actionGet();
+
+        client().prepareIndex("test", "type1", "3").setSource(jsonBuilder().startObject()
+                .field("name", "test")
+                .field("title", "title3")
+                .endObject()).execute().actionGet();
+
+        client().admin().indices().prepareRefresh().execute().actionGet();
+
+        SearchResponse searchResponse = client().prepareSearch()
+                .setQuery(filteredQuery(matchAllQuery(), termsFilter("title", "title1", "title2", "title3").filterName("title")))
+                        .setFilter(termFilter("name", "test").filterName("name"))
+                        .execute().actionGet();
+
+        assertThat(searchResponse.getShardFailures().length, equalTo(0));
+        assertThat(searchResponse.getHits().totalHits(), equalTo(3l));
+
+        for (SearchHit hit : searchResponse.getHits()) {
+            if (hit.id().equals("1") || hit.id().equals("2") || hit.id().equals("3")) {
+                assertThat(hit.matchedFilters().length, equalTo(2));
+                assertThat(hit.matchedFilters(), hasItemInArray("name"));
+                assertThat(hit.matchedFilters(), hasItemInArray("title"));
+            } else {
+                fail("Unexpected document returned with id " + hit.id());
             }
         }
     }