Parent/child: If a p/c query is wrapped in a query filter then CustomQueryWrappingFilter must always be used and any filter wrapping the query filter must never be cached.
Closes #7685
This commit is contained in:
parent
3c8f8cc090
commit
91144fc92f
|
@ -21,6 +21,7 @@ package org.elasticsearch.common.lucene.search;
|
|||
|
||||
import org.apache.lucene.search.*;
|
||||
import org.elasticsearch.common.Nullable;
|
||||
import org.elasticsearch.index.query.QueryParseContext;
|
||||
import org.elasticsearch.index.search.child.CustomQueryWrappingFilter;
|
||||
|
||||
import java.util.List;
|
||||
|
@ -141,8 +142,14 @@ public class Queries {
|
|||
|
||||
}
|
||||
|
||||
public static Filter wrap(Query query) {
|
||||
return FACTORY.wrap(query);
|
||||
/**
|
||||
* Wraps a query in a filter.
|
||||
*
|
||||
* If a filter has an anti per segment execution / caching nature then @{@link CustomQueryWrappingFilter} is returned
|
||||
* otherwise the standard {@link org.apache.lucene.search.QueryWrapperFilter} is returned.
|
||||
*/
|
||||
public static Filter wrap(Query query, QueryParseContext context) {
|
||||
return FACTORY.wrap(query, context);
|
||||
}
|
||||
|
||||
private static final QueryWrapperFilterFactory FACTORY = new QueryWrapperFilterFactory();
|
||||
|
@ -151,8 +158,8 @@ public class Queries {
|
|||
// and potentially miss a forbidden API usage!
|
||||
private static final class QueryWrapperFilterFactory {
|
||||
|
||||
public Filter wrap(Query query) {
|
||||
if (CustomQueryWrappingFilter.shouldUseCustomQueryWrappingFilter(query)) {
|
||||
public Filter wrap(Query query, QueryParseContext context) {
|
||||
if (context.requireCustomQueryWrappingFilter() || CustomQueryWrappingFilter.shouldUseCustomQueryWrappingFilter(query)) {
|
||||
return new CustomQueryWrappingFilter(query);
|
||||
} else {
|
||||
return new QueryWrapperFilter(query);
|
||||
|
|
|
@ -25,7 +25,6 @@ import org.elasticsearch.common.inject.Inject;
|
|||
import org.elasticsearch.common.lucene.search.Queries;
|
||||
import org.elasticsearch.common.xcontent.XContentParser;
|
||||
import org.elasticsearch.index.cache.filter.support.CacheKeyFilter;
|
||||
import org.elasticsearch.index.search.child.CustomQueryWrappingFilter;
|
||||
|
||||
import java.io.IOException;
|
||||
|
||||
|
@ -86,7 +85,7 @@ public class FQueryFilterParser implements FilterParser {
|
|||
if (query == null) {
|
||||
return null;
|
||||
}
|
||||
Filter filter = Queries.wrap(query);
|
||||
Filter filter = Queries.wrap(query, parseContext);
|
||||
if (cache) {
|
||||
filter = parseContext.cacheFilter(filter, cacheKey);
|
||||
}
|
||||
|
|
|
@ -148,7 +148,7 @@ public class NestedFilterParser implements FilterParser {
|
|||
// expects FixedBitSet instances
|
||||
parentFilter = parseContext.fixedBitSetFilter(parentFilter);
|
||||
|
||||
Filter nestedFilter = Queries.wrap(new ToParentBlockJoinQuery(query, parentFilter, ScoreMode.None));
|
||||
Filter nestedFilter = Queries.wrap(new ToParentBlockJoinQuery(query, parentFilter, ScoreMode.None), parseContext);
|
||||
|
||||
if (cache) {
|
||||
nestedFilter = parseContext.cacheFilter(nestedFilter, cacheKey);
|
||||
|
|
|
@ -48,6 +48,6 @@ public class QueryFilterParser implements FilterParser {
|
|||
if (query == null) {
|
||||
return null;
|
||||
}
|
||||
return Queries.wrap(query);
|
||||
return Queries.wrap(query, parseContext);
|
||||
}
|
||||
}
|
|
@ -42,6 +42,7 @@ import org.elasticsearch.index.fielddata.IndexFieldData;
|
|||
import org.elasticsearch.index.mapper.FieldMapper;
|
||||
import org.elasticsearch.index.mapper.FieldMappers;
|
||||
import org.elasticsearch.index.mapper.MapperService;
|
||||
import org.elasticsearch.index.search.child.CustomQueryWrappingFilter;
|
||||
import org.elasticsearch.index.similarity.SimilarityService;
|
||||
import org.elasticsearch.script.ScriptService;
|
||||
import org.elasticsearch.search.internal.SearchContext;
|
||||
|
@ -79,6 +80,8 @@ public class QueryParseContext {
|
|||
|
||||
private boolean propagateNoCache = false;
|
||||
|
||||
private boolean requireCustomQueryWrappingFilter = false;
|
||||
|
||||
final IndexQueryParserService indexQueryParser;
|
||||
|
||||
private final Map<String, Filter> namedFilters = Maps.newHashMap();
|
||||
|
@ -118,6 +121,8 @@ public class QueryParseContext {
|
|||
this.lookup = null;
|
||||
this.parser = jp;
|
||||
this.namedFilters.clear();
|
||||
this.requireCustomQueryWrappingFilter = false;
|
||||
this.propagateNoCache = false;
|
||||
}
|
||||
|
||||
public Index index() {
|
||||
|
@ -200,7 +205,7 @@ public class QueryParseContext {
|
|||
}
|
||||
|
||||
public void addNamedQuery(String name, Query query) {
|
||||
namedFilters.put(name, Queries.wrap(query));
|
||||
namedFilters.put(name, Queries.wrap(query, this));
|
||||
}
|
||||
|
||||
public ImmutableMap<String, Filter> copyNamedFilters() {
|
||||
|
@ -240,6 +245,13 @@ public class QueryParseContext {
|
|||
// if we are at END_OBJECT, move to the next one...
|
||||
parser.nextToken();
|
||||
}
|
||||
if (CustomQueryWrappingFilter.shouldUseCustomQueryWrappingFilter(result)) {
|
||||
requireCustomQueryWrappingFilter = true;
|
||||
// If later on, either directly or indirectly this query gets wrapped in a query filter it must never
|
||||
// get cached even if a filter higher up the chain is configured to do this. This will happen, because
|
||||
// the result filter will be instance of NoCacheFilter (CustomQueryWrappingFilter) which will in
|
||||
// #executeFilterParser() set propagateNoCache to true.
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
|
@ -382,4 +394,8 @@ public class QueryParseContext {
|
|||
}
|
||||
return System.currentTimeMillis();
|
||||
}
|
||||
|
||||
public boolean requireCustomQueryWrappingFilter() {
|
||||
return requireCustomQueryWrappingFilter;
|
||||
}
|
||||
}
|
||||
|
|
|
@ -66,6 +66,7 @@ import static org.elasticsearch.common.settings.ImmutableSettings.settingsBuilde
|
|||
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
|
||||
import static org.elasticsearch.index.query.FilterBuilders.*;
|
||||
import static org.elasticsearch.index.query.QueryBuilders.*;
|
||||
import static org.elasticsearch.index.query.QueryBuilders.constantScoreQuery;
|
||||
import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.factorFunction;
|
||||
import static org.elasticsearch.index.query.functionscore.ScoreFunctionBuilders.scriptFunction;
|
||||
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.*;
|
||||
|
@ -1065,6 +1066,79 @@ public class SimpleChildQuerySearchTests extends ElasticsearchIntegrationTest {
|
|||
assertThat(searchResponse.getHits().hits()[0].id(), equalTo("2"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHasChildAndHasParentWrappedInAQueryFilter() throws Exception {
|
||||
assertAcked(prepareCreate("test")
|
||||
.addMapping("parent")
|
||||
.addMapping("child", "_parent", "type=parent"));
|
||||
ensureGreen();
|
||||
|
||||
// query filter in case for p/c shouldn't execute per segment, but rather
|
||||
client().prepareIndex("test", "parent", "1").setSource("p_field", 1).get();
|
||||
client().admin().indices().prepareFlush("test").setForce(true).get();
|
||||
client().prepareIndex("test", "child", "2").setParent("1").setSource("c_field", 1).get();
|
||||
refresh();
|
||||
|
||||
SearchResponse searchResponse = client().prepareSearch("test")
|
||||
.setQuery(filteredQuery(matchAllQuery(), queryFilter(hasChildQuery("child", matchQuery("c_field", 1))))).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("1"));
|
||||
|
||||
searchResponse = client().prepareSearch("test")
|
||||
.setQuery(filteredQuery(matchAllQuery(), queryFilter(topChildrenQuery("child", matchQuery("c_field", 1))))).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("1"));
|
||||
|
||||
searchResponse = client().prepareSearch("test")
|
||||
.setQuery(filteredQuery(matchAllQuery(), queryFilter(hasParentQuery("parent", matchQuery("p_field", 1))))).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("2"));
|
||||
|
||||
searchResponse = client().prepareSearch("test")
|
||||
.setQuery(filteredQuery(matchAllQuery(), queryFilter(boolQuery().must(hasChildQuery("child", matchQuery("c_field", 1)))))).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("1"));
|
||||
|
||||
searchResponse = client().prepareSearch("test")
|
||||
.setQuery(filteredQuery(matchAllQuery(), queryFilter(boolQuery().must(topChildrenQuery("child", matchQuery("c_field", 1)))))).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("1"));
|
||||
|
||||
searchResponse = client().prepareSearch("test")
|
||||
.setQuery(filteredQuery(matchAllQuery(), queryFilter(boolQuery().must(hasParentQuery("parent", matchQuery("p_field", 1)))))).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("2"));
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testHasChildAndHasParentWrappedInAQueryFilterShouldNeverGetCached() throws Exception {
|
||||
assertAcked(prepareCreate("test")
|
||||
.setSettings(ImmutableSettings.builder().put("index.cache.filter.type", "weighted"))
|
||||
.addMapping("parent")
|
||||
.addMapping("child", "_parent", "type=parent"));
|
||||
ensureGreen();
|
||||
|
||||
client().prepareIndex("test", "parent", "1").setSource("p_field", 1).get();
|
||||
client().prepareIndex("test", "child", "2").setParent("1").setSource("c_field", 1).get();
|
||||
refresh();
|
||||
|
||||
for (int i = 0; i < 10; i++) {
|
||||
SearchResponse searchResponse = client().prepareSearch("test")
|
||||
.setExplain(true)
|
||||
.setQuery(constantScoreQuery(boolFilter()
|
||||
.must(queryFilter(hasChildQuery("child", matchQuery("c_field", 1))))
|
||||
.cache(true)
|
||||
)).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("1"));
|
||||
// Can't start with ConstantScore(cache(BooleanFilter(
|
||||
assertThat(searchResponse.getHits().getAt(0).explanation().getDescription(), startsWith("ConstantScore(BooleanFilter("));
|
||||
|
||||
searchResponse = client().prepareSearch("test")
|
||||
.setExplain(true)
|
||||
.setQuery(constantScoreQuery(boolFilter()
|
||||
.must(queryFilter(boolQuery().must(matchAllQuery()).must(hasChildQuery("child", matchQuery("c_field", 1)))))
|
||||
.cache(true)
|
||||
)).get();
|
||||
assertSearchHit(searchResponse, 1, hasId("1"));
|
||||
// Can't start with ConstantScore(cache(BooleanFilter(
|
||||
assertThat(searchResponse.getHits().getAt(0).explanation().getDescription(), startsWith("ConstantScore(BooleanFilter("));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testSimpleQueryRewrite() throws Exception {
|
||||
assertAcked(prepareCreate("test")
|
||||
|
|
Loading…
Reference in New Issue