Skip to content

Commit 693ab3d

Browse files
kkrik-eskkrik-es
authored
Skip segment for MatchNoDocsQuery filters (#98295)
When a query of a filter gets rewritten to `MatchNoDocsQuery`, segments will not produce any results. We can therefore skip processing such segments. This applies to `FilterByFilterAggregator` and `FiltersAggregator`, as well as to `TermsAggregator` when it uses `StringTermsAggregatorFromFilters` internally; the latter is an adapter aggregator wrapping `FilterByFilterAggregator`. Fixes #94637
1 parent 297a013 commit 693ab3d

File tree

6 files changed

+246
-5
lines changed

6 files changed

+246
-5
lines changed

‎docs/changelog/98295.yaml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 98295
2+
summary: Skip segment for `MatchNoDocsQuery` filters
3+
area: Aggregations
4+
type: bug
5+
issues:
6+
- 94637

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FilterByFilterAggregator.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ private FilterByFilterAggregator(
233233
@Override
234234
protected LeafBucketCollector getLeafCollector(AggregationExecutionContext aggCtx, LeafBucketCollector sub) throws IOException {
235235
assert scoreMode().needsScores() == false;
236-
if (filters().size() == 0) {
236+
if (QueryToFilterAdapter.MatchesNoDocs(filters())) {
237237
return LeafBucketCollector.NO_OP_COLLECTOR;
238238
}
239239
Bits live = aggCtx.getLeafReaderContext().reader().getLiveDocs();

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregator.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,10 @@ static class Compatible extends FiltersAggregator {
289289

290290
@Override
291291
protected LeafBucketCollector getLeafCollector(AggregationExecutionContext aggCtx, LeafBucketCollector sub) throws IOException {
292+
if (QueryToFilterAdapter.MatchesNoDocs(filters()) && otherBucketKey == null) {
293+
return LeafBucketCollector.NO_OP_COLLECTOR;
294+
}
295+
292296
IntPredicate[] docFilters = new IntPredicate[filters().size()];
293297
for (int filterOrd = 0; filterOrd < filters().size(); filterOrd++) {
294298
docFilters[filterOrd] = filters().get(filterOrd).matchingDocIds(aggCtx.getLeafReaderContext());

‎server/src/main/java/org/elasticsearch/search/aggregations/bucket/filter/QueryToFilterAdapter.java‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.apache.lucene.search.IndexSearcher;
1818
import org.apache.lucene.search.IndexSortSortedNumericDocValuesRangeQuery;
1919
import org.apache.lucene.search.LeafCollector;
20+
import org.apache.lucene.search.MatchNoDocsQuery;
2021
import org.apache.lucene.search.PointRangeQuery;
2122
import org.apache.lucene.search.Query;
2223
import org.apache.lucene.search.ScoreMode;
@@ -28,6 +29,7 @@
2829
import org.elasticsearch.xcontent.XContentBuilder;
2930

3031
import java.io.IOException;
32+
import java.util.List;
3133
import java.util.function.BiConsumer;
3234
import java.util.function.IntPredicate;
3335

@@ -246,4 +248,19 @@ private Weight weight() throws IOException {
246248
}
247249
return weight;
248250
}
251+
252+
/**
253+
* Checks if all passed filters contain MatchNoDocsQuery queries. In this case, filter aggregation produces
254+
* no docs for the given segment.
255+
* @param filters list of filters to check
256+
* @return true if all filters match no docs, otherwise false
257+
*/
258+
static boolean MatchesNoDocs(List<QueryToFilterAdapter> filters) {
259+
for (QueryToFilterAdapter filter : filters) {
260+
if (filter.query() instanceof MatchNoDocsQuery == false) {
261+
return false;
262+
}
263+
}
264+
return true;
265+
}
249266
}

‎server/src/test/java/org/elasticsearch/search/aggregations/bucket/filter/FiltersAggregatorTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -919,14 +919,14 @@ public void testMatchNoneFilter() throws IOException {
919919
matchesMap().entry("segments_with_doc_count_field", 0)
920920
.entry("segments_with_deleted_docs", 0)
921921
.entry("segments_collected", 0)
922-
.entry("segments_counted", greaterThanOrEqualTo(1))
922+
.entry("segments_counted", 0)
923923
.entry(
924924
"filters",
925925
matchesList().item(
926926
matchesMap().entry(
927927
"query",
928928
"MatchNoDocsQuery(\"The \"range\" query was rewritten to a \"match_none\" query.\")"
929-
).entry("segments_counted_in_constant_time", greaterThan(0))
929+
).entry("segments_counted_in_constant_time", 0)
930930
)
931931
)
932932
)
@@ -958,14 +958,14 @@ public void testMatchNoneTopLevel() throws IOException {
958958
matchesMap().entry("segments_with_doc_count_field", 0)
959959
.entry("segments_with_deleted_docs", 0)
960960
.entry("segments_collected", 0)
961-
.entry("segments_counted", greaterThanOrEqualTo(1))
961+
.entry("segments_counted", 0)
962962
.entry(
963963
"filters",
964964
matchesList().item(
965965
matchesMap().entry(
966966
"query",
967967
"MatchNoDocsQuery(\"The \"range\" query was rewritten to a \"match_none\" query.\")"
968-
).entry("segments_counted_in_constant_time", greaterThan(0))
968+
).entry("segments_counted_in_constant_time", 0)
969969
)
970970
)
971971
)

‎x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/constant_keyword/10_basic.yml‎

Lines changed: 214 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,3 +199,217 @@ setup:
199199
- match: {hits.hits.0.fields.foo.0: bar }
200200
- match: {hits.hits.1.fields.foo.0: bar }
201201
- match: {hits.hits.2.fields.foo.0: baz }
202+
203+
---
204+
"Simple filter":
205+
- skip:
206+
version: " - 7.6.99"
207+
reason: "constant_keyword was added in 7.7"
208+
209+
- do:
210+
indices.create:
211+
index: bicycles
212+
body:
213+
settings:
214+
number_of_shards: 1
215+
mappings:
216+
properties:
217+
cycle_type:
218+
type: constant_keyword
219+
value: bicycle
220+
id:
221+
type: integer
222+
223+
- do:
224+
bulk:
225+
refresh: true
226+
body:
227+
- index:
228+
_index: bicycles
229+
_id: 1
230+
- id: 1
231+
- index:
232+
_index: bicycles
233+
_id: 2
234+
- id: 2
235+
- index:
236+
_index: bicycles
237+
_id: 3
238+
- id: 3
239+
240+
- do:
241+
search:
242+
index: bicycles
243+
body:
244+
size: 0
245+
aggs:
246+
f:
247+
filter:
248+
term:
249+
cycle_type: bicycle
250+
- match: { hits.total.value: 3 }
251+
- match: { aggregations.f.doc_count: 3 }
252+
253+
- do:
254+
search:
255+
index: bicycles
256+
body:
257+
size: 0
258+
aggs:
259+
f:
260+
filter:
261+
term:
262+
cycle_type: nomatch
263+
- match: { hits.total.value: 3 }
264+
- match: { aggregations.f.doc_count: 0 }
265+
266+
- do:
267+
search:
268+
index: bicycles
269+
body:
270+
size: 0
271+
aggs:
272+
f:
273+
filters:
274+
other_bucket_key: other_types
275+
filters:
276+
nomatch_type:
277+
match:
278+
cycle_type: nomatch
279+
- match: { hits.total.value: 3 }
280+
- match: { aggregations.f.buckets.nomatch_type.doc_count: 0 }
281+
- match: { aggregations.f.buckets.other_types.doc_count: 3 }
282+
283+
284+
---
285+
"Filter with histogram":
286+
- skip:
287+
version: " - 7.6.99"
288+
reason: "constant_keyword was added in 7.7"
289+
290+
- do:
291+
indices.create:
292+
index: bicycles
293+
body:
294+
settings:
295+
number_of_shards: 1
296+
mappings:
297+
properties:
298+
cycle_type:
299+
type: constant_keyword
300+
value: bicycle
301+
id:
302+
type: integer
303+
status:
304+
type: keyword
305+
price:
306+
type: integer
307+
308+
- do:
309+
bulk:
310+
refresh: true
311+
body:
312+
- index:
313+
_index: bicycles
314+
_id: 1
315+
- id: 1
316+
status: up
317+
price: 100
318+
- index:
319+
_index: bicycles
320+
_id: 2
321+
- id: 2
322+
status: up
323+
price: 150
324+
- index:
325+
_index: bicycles
326+
_id: 3
327+
- id: 3
328+
status: down
329+
price: 200
330+
331+
- do:
332+
search:
333+
index: bicycles
334+
body:
335+
size: 0
336+
aggs:
337+
agg:
338+
histogram:
339+
field: price
340+
interval: 50
341+
aggs:
342+
0-bucket:
343+
filter:
344+
bool:
345+
filter:
346+
- bool:
347+
filter:
348+
- bool:
349+
filter:
350+
- bool:
351+
should:
352+
- term:
353+
id: 1
354+
minimum_should_match: 1
355+
- bool:
356+
should:
357+
- term:
358+
cycle_type: bicycle
359+
minimum_should_match: 1
360+
- bool:
361+
should:
362+
- term:
363+
status: up
364+
minimum_should_match: 1
365+
- match: { hits.total.value: 3 }
366+
- length: { aggregations.agg.buckets: 3 }
367+
- match: { aggregations.agg.buckets.0.key: 100.0 }
368+
- match: { aggregations.agg.buckets.0.0-bucket.doc_count: 1 }
369+
- match: { aggregations.agg.buckets.1.key: 150.0 }
370+
- match: { aggregations.agg.buckets.1.0-bucket.doc_count: 0 }
371+
- match: { aggregations.agg.buckets.2.key: 200.0 }
372+
- match: { aggregations.agg.buckets.2.0-bucket.doc_count: 0 }
373+
374+
- do:
375+
search:
376+
index: bicycles
377+
body:
378+
size: 0
379+
aggs:
380+
agg:
381+
histogram:
382+
field: price
383+
interval: 50
384+
aggs:
385+
0-bucket:
386+
filter:
387+
bool:
388+
filter:
389+
- bool:
390+
filter:
391+
- bool:
392+
filter:
393+
- bool:
394+
should:
395+
- term:
396+
id: 1
397+
minimum_should_match: 1
398+
- bool:
399+
should:
400+
- term:
401+
cycle_type: nomatch
402+
minimum_should_match: 1
403+
- bool:
404+
should:
405+
- term:
406+
status: up
407+
minimum_should_match: 1
408+
- match: { hits.total.value: 3 }
409+
- length: { aggregations.agg.buckets: 3 }
410+
- match: { aggregations.agg.buckets.0.key: 100.0 }
411+
- match: { aggregations.agg.buckets.0.0-bucket.doc_count: 0 }
412+
- match: { aggregations.agg.buckets.1.key: 150.0 }
413+
- match: { aggregations.agg.buckets.1.0-bucket.doc_count: 0 }
414+
- match: { aggregations.agg.buckets.2.key: 200.0 }
415+
- match: { aggregations.agg.buckets.2.0-bucket.doc_count: 0 }

0 commit comments

Comments
 (0)