Skip to content

Commit 8830eb6

Browse files
authored
Generalize how queries on _index are handled at rewrite time (#52486)
#49713 aims at introducing a new constant_keyword field which, like _index, always rewrites queries to a MatchAllQueryBuilder or a MatchNoneQueryBuilder in order to skip shards in the can_match phase. This change introduces a new ConstantFieldType marker class that helps get this functionality with any field and not just _index. Since this change refactors rewrites, I also took it as an opportunity to adrress #49254: instead of returning the same queries you would get on a keyword field when a field is unmapped, queries get rewritten to a MatchNoDocsQueryBuilder. This change exposed a couple bugs, like the fact that the percolator doesn't rewrite queries at query time, or that the significant_terms aggregation doesn't rewrite its inner filter, which I fixed. Closes #49254
1 parent 643ccd7 commit 8830eb6

File tree

37 files changed

+607
-334
lines changed

37 files changed

+607
-334
lines changed

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolateQueryBuilder.java

+14-5
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.apache.lucene.analysis.DelegatingAnalyzerWrapper;
2424
import org.apache.lucene.index.BinaryDocValues;
2525
import org.apache.lucene.index.DirectoryReader;
26+
import org.apache.lucene.index.IndexReader;
2627
import org.apache.lucene.index.IndexReaderContext;
2728
import org.apache.lucene.index.IndexWriter;
2829
import org.apache.lucene.index.IndexWriterConfig;
@@ -75,6 +76,7 @@
7576
import org.elasticsearch.index.query.QueryRewriteContext;
7677
import org.elasticsearch.index.query.QueryShardContext;
7778
import org.elasticsearch.index.query.QueryShardException;
79+
import org.elasticsearch.index.query.Rewriteable;
7880
import org.elasticsearch.indices.breaker.CircuitBreakerService;
7981
import org.elasticsearch.indices.breaker.NoneCircuitBreakerService;
8082

@@ -562,9 +564,9 @@ protected Analyzer getWrappedAnalyzer(String fieldName) {
562564
PercolatorFieldMapper.FieldType pft = (PercolatorFieldMapper.FieldType) fieldType;
563565
String name = this.name != null ? this.name : pft.name();
564566
QueryShardContext percolateShardContext = wrap(context);
567+
PercolatorFieldMapper.configureContext(percolateShardContext, pft.mapUnmappedFieldsAsText);;
565568
PercolateQuery.QueryStore queryStore = createStore(pft.queryBuilderField,
566-
percolateShardContext,
567-
pft.mapUnmappedFieldsAsText);
569+
percolateShardContext);
568570

569571
return pft.percolateQuery(name, queryStore, documents, docSearcher, excludeNestedDocuments, context.indexVersionCreated());
570572
}
@@ -607,8 +609,7 @@ static IndexSearcher createMultiDocumentSearcher(Analyzer analyzer, Collection<P
607609
}
608610

609611
static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldType,
610-
QueryShardContext context,
611-
boolean mapUnmappedFieldsAsString) {
612+
QueryShardContext context) {
612613
Version indexVersion = context.indexVersionCreated();
613614
NamedWriteableRegistry registry = context.getWriteableRegistry();
614615
return ctx -> {
@@ -634,7 +635,8 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy
634635
assert valueLength > 0;
635636
QueryBuilder queryBuilder = input.readNamedWriteable(QueryBuilder.class);
636637
assert in.read() == -1;
637-
return PercolatorFieldMapper.toQuery(context, mapUnmappedFieldsAsString, queryBuilder);
638+
queryBuilder = Rewriteable.rewrite(queryBuilder, context);
639+
return queryBuilder.toQuery(context);
638640
}
639641
}
640642
} else {
@@ -647,6 +649,13 @@ static PercolateQuery.QueryStore createStore(MappedFieldType queryBuilderFieldTy
647649
static QueryShardContext wrap(QueryShardContext shardContext) {
648650
return new QueryShardContext(shardContext) {
649651

652+
@Override
653+
public IndexReader getIndexReader() {
654+
// The reader that matters in this context is not the reader of the shard but
655+
// the reader of the MemoryIndex. We just use `null` for simplicity.
656+
return null;
657+
}
658+
650659
@Override
651660
public BitSetProducer bitsetFilter(Query query) {
652661
return context -> {

modules/percolator/src/main/java/org/elasticsearch/percolator/PercolatorFieldMapper.java

+5-7
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,8 @@ public void parse(ParseContext context) throws IOException {
394394
throw new IllegalArgumentException("a document can only contain one percolator query");
395395
}
396396

397+
configureContext(queryShardContext, isMapUnmappedFieldAsText());
398+
397399
XContentParser parser = context.parser();
398400
QueryBuilder queryBuilder = parseQueryBuilder(
399401
parser, parser.getTokenLocation()
@@ -407,7 +409,8 @@ public void parse(ParseContext context) throws IOException {
407409
Version indexVersion = context.mapperService().getIndexSettings().getIndexVersionCreated();
408410
createQueryBuilderField(indexVersion, queryBuilderField, queryBuilder, context);
409411

410-
Query query = toQuery(queryShardContext, isMapUnmappedFieldAsText(), queryBuilder);
412+
QueryBuilder queryBuilderForProcessing = queryBuilder.rewrite(new QueryShardContext(queryShardContext));
413+
Query query = queryBuilderForProcessing.toQuery(queryShardContext);
411414
processQuery(query, context);
412415
}
413416

@@ -465,11 +468,7 @@ void processQuery(Query query, ParseContext context) {
465468
doc.add(new NumericDocValuesField(minimumShouldMatchFieldMapper.name(), result.minimumShouldMatch));
466469
}
467470

468-
static Query parseQuery(QueryShardContext context, boolean mapUnmappedFieldsAsString, XContentParser parser) throws IOException {
469-
return toQuery(context, mapUnmappedFieldsAsString, parseQueryBuilder(parser, parser.getTokenLocation()));
470-
}
471-
472-
static Query toQuery(QueryShardContext context, boolean mapUnmappedFieldsAsString, QueryBuilder queryBuilder) throws IOException {
471+
static void configureContext(QueryShardContext context, boolean mapUnmappedFieldsAsString) {
473472
// This means that fields in the query need to exist in the mapping prior to registering this query
474473
// The reason that this is required, is that if a field doesn't exist then the query assumes defaults, which may be undesired.
475474
//
@@ -484,7 +483,6 @@ static Query toQuery(QueryShardContext context, boolean mapUnmappedFieldsAsStrin
484483
// as an analyzed string.
485484
context.setAllowUnmappedFields(false);
486485
context.setMapUnmappedFieldAsString(mapUnmappedFieldsAsString);
487-
return queryBuilder.toQuery(context);
488486
}
489487

490488
private static QueryBuilder parseQueryBuilder(XContentParser parser, XContentLocation location) {

modules/percolator/src/test/java/org/elasticsearch/percolator/QueryBuilderStoreTests.java

+10-1
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,12 @@
3838
import org.elasticsearch.index.fielddata.plain.BytesBinaryDVIndexFieldData;
3939
import org.elasticsearch.index.mapper.BinaryFieldMapper;
4040
import org.elasticsearch.index.mapper.ContentPath;
41+
import org.elasticsearch.index.mapper.KeywordFieldMapper;
4142
import org.elasticsearch.index.mapper.Mapper;
4243
import org.elasticsearch.index.mapper.ParseContext;
4344
import org.elasticsearch.index.query.QueryShardContext;
4445
import org.elasticsearch.index.query.TermQueryBuilder;
46+
import org.elasticsearch.mock.orig.Mockito;
4547
import org.elasticsearch.search.SearchModule;
4648
import org.elasticsearch.test.ESTestCase;
4749

@@ -93,7 +95,14 @@ public void testStoringQueryBuilders() throws IOException {
9395
when(queryShardContext.getXContentRegistry()).thenReturn(xContentRegistry());
9496
when(queryShardContext.getForField(fieldMapper.fieldType()))
9597
.thenReturn(new BytesBinaryDVIndexFieldData(new Index("index", "uuid"), fieldMapper.name()));
96-
PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore(fieldMapper.fieldType(), queryShardContext, false);
98+
when(queryShardContext.fieldMapper(Mockito.anyString())).thenAnswer(invocation -> {
99+
final String fieldName = (String) invocation.getArguments()[0];
100+
KeywordFieldMapper.KeywordFieldType ft = new KeywordFieldMapper.KeywordFieldType();
101+
ft.setName(fieldName);
102+
ft.freeze();
103+
return ft;
104+
});
105+
PercolateQuery.QueryStore queryStore = PercolateQueryBuilder.createStore(fieldMapper.fieldType(), queryShardContext);
97106

98107
try (IndexReader indexReader = DirectoryReader.open(directory)) {
99108
LeafReaderContext leafContext = indexReader.leaves().get(0);
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.elasticsearch.index.mapper;
21+
22+
import org.apache.lucene.search.MatchAllDocsQuery;
23+
import org.apache.lucene.search.MatchNoDocsQuery;
24+
import org.apache.lucene.search.MultiTermQuery;
25+
import org.apache.lucene.search.Query;
26+
import org.apache.lucene.util.BytesRef;
27+
import org.elasticsearch.common.Nullable;
28+
import org.elasticsearch.common.lucene.search.Queries;
29+
import org.elasticsearch.common.regex.Regex;
30+
import org.elasticsearch.index.query.QueryShardContext;
31+
32+
import java.util.List;
33+
34+
/**
35+
* A {@link MappedFieldType} that has the same value for all documents.
36+
* Factory methods for queries are called at rewrite time so they should be
37+
* cheap. In particular they should not read data from disk or perform a
38+
* network call. Furthermore they may only return a {@link MatchAllDocsQuery}
39+
* or a {@link MatchNoDocsQuery}.
40+
*/
41+
public abstract class ConstantFieldType extends MappedFieldType {
42+
43+
public ConstantFieldType() {
44+
super();
45+
}
46+
47+
public ConstantFieldType(ConstantFieldType other) {
48+
super(other);
49+
}
50+
51+
@Override
52+
public final boolean isSearchable() {
53+
return true;
54+
}
55+
56+
@Override
57+
public final boolean isAggregatable() {
58+
return true;
59+
}
60+
61+
@Override
62+
public final Query existsQuery(QueryShardContext context) {
63+
return new MatchAllDocsQuery();
64+
}
65+
66+
/**
67+
* Return whether the constant value of this field matches the provided {@code pattern}
68+
* as documented in {@link Regex#simpleMatch}.
69+
*/
70+
protected abstract boolean matches(String pattern, QueryShardContext context);
71+
72+
private static String valueToString(Object value) {
73+
return value instanceof BytesRef
74+
? ((BytesRef) value).utf8ToString()
75+
: value.toString();
76+
}
77+
78+
@Override
79+
public final Query termQuery(Object value, QueryShardContext context) {
80+
String pattern = valueToString(value);
81+
if (matches(pattern, context)) {
82+
return Queries.newMatchAllQuery();
83+
} else {
84+
return new MatchNoDocsQuery();
85+
}
86+
}
87+
88+
@Override
89+
public final Query termsQuery(List<?> values, QueryShardContext context) {
90+
for (Object value : values) {
91+
String pattern = valueToString(value);
92+
if (matches(pattern, context)) {
93+
// `terms` queries are a disjunction, so one matching term is enough
94+
return Queries.newMatchAllQuery();
95+
}
96+
}
97+
return new MatchNoDocsQuery();
98+
}
99+
100+
@Override
101+
public final Query prefixQuery(String prefix,
102+
@Nullable MultiTermQuery.RewriteMethod method,
103+
QueryShardContext context) {
104+
String pattern = prefix + "*";
105+
if (matches(pattern, context)) {
106+
return Queries.newMatchAllQuery();
107+
} else {
108+
return new MatchNoDocsQuery();
109+
}
110+
}
111+
112+
@Override
113+
public final Query wildcardQuery(String value,
114+
@Nullable MultiTermQuery.RewriteMethod method,
115+
QueryShardContext context) {
116+
if (matches(value, context)) {
117+
return Queries.newMatchAllQuery();
118+
} else {
119+
return new MatchNoDocsQuery();
120+
}
121+
}
122+
123+
}

server/src/main/java/org/elasticsearch/index/mapper/IndexFieldMapper.java

+3-82
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,7 @@
2121

2222
import org.apache.lucene.index.IndexOptions;
2323
import org.apache.lucene.index.IndexableField;
24-
import org.apache.lucene.search.MatchAllDocsQuery;
25-
import org.apache.lucene.search.MultiTermQuery;
26-
import org.apache.lucene.search.Query;
27-
import org.apache.lucene.util.BytesRef;
28-
import org.elasticsearch.common.Nullable;
2924
import org.elasticsearch.common.lucene.Lucene;
30-
import org.elasticsearch.common.lucene.search.Queries;
3125
import org.elasticsearch.common.settings.Settings;
3226
import org.elasticsearch.common.xcontent.XContentBuilder;
3327
import org.elasticsearch.index.fielddata.IndexFieldData;
@@ -91,7 +85,7 @@ public MetadataFieldMapper getDefault(ParserContext context) {
9185
}
9286
}
9387

94-
static final class IndexFieldType extends MappedFieldType {
88+
static final class IndexFieldType extends ConstantFieldType {
9589

9690
IndexFieldType() {}
9791

@@ -110,81 +104,8 @@ public String typeName() {
110104
}
111105

112106
@Override
113-
public boolean isSearchable() {
114-
// The _index field is always searchable.
115-
return true;
116-
}
117-
118-
@Override
119-
public Query existsQuery(QueryShardContext context) {
120-
return new MatchAllDocsQuery();
121-
}
122-
123-
/**
124-
* This termQuery impl looks at the context to determine the index that
125-
* is being queried and then returns a MATCH_ALL_QUERY or MATCH_NO_QUERY
126-
* if the value matches this index. This can be useful if aliases or
127-
* wildcards are used but the aim is to restrict the query to specific
128-
* indices
129-
*/
130-
@Override
131-
public Query termQuery(Object value, @Nullable QueryShardContext context) {
132-
String pattern = value instanceof BytesRef
133-
? ((BytesRef) value).utf8ToString()
134-
: value.toString();
135-
if (context.indexMatches(pattern)) {
136-
// No need to OR these clauses - we can only logically be
137-
// running in the context of just one of these index names.
138-
return Queries.newMatchAllQuery();
139-
} else {
140-
return Queries.newMatchNoDocsQuery("The index [" + context.getFullyQualifiedIndex().getName() +
141-
"] doesn't match the provided value [" + value + "].");
142-
}
143-
}
144-
145-
@Override
146-
public Query termsQuery(List values, QueryShardContext context) {
147-
if (context == null) {
148-
return super.termsQuery(values, context);
149-
}
150-
for (Object value : values) {
151-
String pattern = value instanceof BytesRef
152-
? ((BytesRef) value).utf8ToString()
153-
: value.toString();
154-
if (context.indexMatches(pattern)) {
155-
// No need to OR these clauses - we can only logically be
156-
// running in the context of just one of these index names.
157-
return Queries.newMatchAllQuery();
158-
}
159-
}
160-
// None of the listed index names are this one
161-
return Queries.newMatchNoDocsQuery("The index [" + context.getFullyQualifiedIndex().getName() +
162-
"] doesn't match the provided values [" + values + "].");
163-
}
164-
165-
@Override
166-
public Query prefixQuery(String value,
167-
@Nullable MultiTermQuery.RewriteMethod method,
168-
QueryShardContext context) {
169-
String pattern = value + "*";
170-
if (context.indexMatches(pattern)) {
171-
return Queries.newMatchAllQuery();
172-
} else {
173-
return Queries.newMatchNoDocsQuery("The index [" + context.getFullyQualifiedIndex().getName() +
174-
"] doesn't match the provided prefix [" + value + "].");
175-
}
176-
}
177-
178-
@Override
179-
public Query wildcardQuery(String value,
180-
@Nullable MultiTermQuery.RewriteMethod method,
181-
QueryShardContext context) {
182-
if (context.indexMatches(value)) {
183-
return Queries.newMatchAllQuery();
184-
} else {
185-
return Queries.newMatchNoDocsQuery("The index [" + context.getFullyQualifiedIndex().getName()
186-
+ "] doesn't match the provided pattern [" + value + "].");
187-
}
107+
protected boolean matches(String pattern, QueryShardContext context) {
108+
return context.indexMatches(pattern);
188109
}
189110

190111
@Override

server/src/main/java/org/elasticsearch/index/query/AbstractQueryBuilder.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
package org.elasticsearch.index.query;
2121

2222
import org.apache.lucene.search.BoostQuery;
23+
import org.apache.lucene.search.MatchNoDocsQuery;
2324
import org.apache.lucene.search.Query;
2425
import org.apache.lucene.search.spans.SpanBoostQuery;
2526
import org.apache.lucene.search.spans.SpanQuery;
@@ -103,7 +104,7 @@ public final Query toQuery(QueryShardContext context) throws IOException {
103104
if (boost != DEFAULT_BOOST) {
104105
if (query instanceof SpanQuery) {
105106
query = new SpanBoostQuery((SpanQuery) query, boost);
106-
} else {
107+
} else if (query instanceof MatchNoDocsQuery == false) {
107108
query = new BoostQuery(query, boost);
108109
}
109110
}
@@ -232,7 +233,7 @@ static Collection<Query> toQueries(Collection<QueryBuilder> queryBuilders, Query
232233
IOException {
233234
List<Query> queries = new ArrayList<>(queryBuilders.size());
234235
for (QueryBuilder queryBuilder : queryBuilders) {
235-
Query query = queryBuilder.toQuery(context);
236+
Query query = queryBuilder.rewrite(context).toQuery(context);
236237
if (query != null) {
237238
queries.add(query);
238239
}

0 commit comments

Comments
 (0)