Repository navigation
Conversation
BooleanQuery.Filter built a FilterFunc closure that captured the filter searcher. FilteringSearcher never owned that searcher, so it was never closed, and FilterFunc could only return bool, so errors from the filter searcher's Next/Advance were silently turned into "no match". Add searcher.FilterSearcher, which owns the filter searcher (closing it together with the child) and propagates its errors. BooleanQuery.Searcher now uses it. FilteringSearcher and FilterFunc are unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2384.
What changed
BooleanQuery.Filterhad two defects, both stemming from the filter searcher beingcaptured in a
FilterFuncclosure thatFilteringSearcherneither owned nor couldreturn errors from:
TotTermSearchersStartedaboveTotTermSearchersFinishedand drops the filter'sbytes from
SearchResult.Cost.FilterFuncreturns only abool, so an error from the filter searcher'sNext/Advance(a segment read or decompression failure, for example) was turnedinto "this document does not match" and the search reported success with a partial
or empty result.
This fixes both without touching any exported API:
searcher.FilterSearcherandsearcher.NewFilterSearcher. It keeps only thechild documents that are also matched by a caller-supplied
filterSearcher, which itowns: the filter searcher is closed together with the child, and an error from its
Next/Advanceaborts the search instead of being treated as a non-match.BooleanQuery.Searchernow builds the filter with the disabled-scoring options italready used and wraps the child in
NewFilterSearcherinstead of constructing aFilterFunc. The filter still does not contribute to the score.FilteringSearcherandFilterFuncare unchanged, so existing customFilterFuncimplementations and the geo searchers keep working as-is.
The matching logic is the same lockstep merge as before (both searchers yield documents
in internal-ID order), so results are unchanged; only ownership and error handling
differ.
Design note
An alternative would be to change
FilterFuncto return(bool, error)and giveFilteringSearcheran owned filter searcher. That changes an exported signature, sothis PR keeps the public API stable and moves ownership + error propagation into a
dedicated searcher used only by
BooleanQuery.Filter.How it was verified
Ran the reproducer from #2384 (scorch index plus a query whose searcher fails on
Next/Advance) againstmasterand against this branch:BooleanQuery{Must,Filter}term searchersConjunctionQuerywith the same clauses was already correct and stays correct.New tests in
search/searcher/search_filter_test.gocover:FilterSearcherkeeps only documents matched by the filter searcher (interleaved,superset, disjoint, empty filter, filter ahead), through both
NextandAdvance;FilterSearcher.NextandAdvancereturn the filter searcher's error;FilterSearcher.Closecloses both the child and the filter searcher, preferring thechild's error;
DocumentMatchPoolSizeaccounts for both searchers.Existing end-to-end coverage (
TestFilteredBooleanQuery,search_nested_test.go,search_test.go) continues to pass.