Skip to content

BooleanQuery.Filter: filter searcher isn't closed, and its errors are swallowed #2384

Description

@pstibrany

The Filter clause of BooleanQuery has two problems. Both come from the same place: in search/query/boolean.go the filter searcher is captured inside a closure (filterFunc) that FilteringSearcher doesn't own and can't return errors from.

Seen on v2.5.7, v2.6.0, and current master (991a4506f4f3).

1. The filter searcher is never closed

BooleanQuery.Searcher passes only the scoring searcher to NewFilteringSearcher, and FilteringSearcher.Close() just calls child.Close(). The filter searcher, hidden in the closure, never gets closed.

On scorch this shows up two ways, both handled in the term reader's Close():

  • its bytes are left out of SearchResult.Cost, and
  • TotTermSearchersStarted ends up higher than TotTermSearchersFinished.

The same query as a ConjunctionQuery is fine, because ConjunctionSearcher.Close() closes every child.

This isn't a file-descriptor or memory leak (the reader doesn't AddRef the snapshot, and it's garbage-collected after the request) — just a wrong cost number and a counter that never balances.

2. Errors from the filter searcher are swallowed

FilterFunc returns only a bool, so when the filter searcher's Next/Advance returns an error (say a segment read or decompression failure), the closure turns it into "this document doesn't match" and moves on. Nothing propagates up to SearchInContext, so the search reports success with a partial or empty result instead of failing.

Again, as a conjunct the error propagates correctly through ConjunctionSearcher.Next. (Context cancellation is unaffected — the collector checks that separately.)

Reproducer

Output is identical on v2.5.7 and master:

=== Issue A: filter searcher Close() is never called ===
  BooleanQuery{Must,Filter}: started +2 finished +1
  => BUG: 1 term searcher(s) started but never closed (the Filter searcher)
  ConjunctionQuery (contrast): started +2 finished +2 => balanced

=== Issue B: filter searcher errors are silently swallowed ===
  BooleanQuery Filter: NO error, 0 hit(s) => BUG: error swallowed, result silently partial/empty
  ConjunctionQuery (contrast): error (as expected): injected filter searcher error (e.g. segment read failure)
Self-contained main.gogo get github.com/blevesearch/bleve/v2@v2.5.7 && go run .
// Reproduces two issues in bleve's BooleanQuery.Filter (search/query/boolean.go).
// A: the filter searcher is never Close()d, so on scorch
//    TotTermSearchersStarted outpaces Finished and its bytes are dropped from Cost.
// B: FilterFunc returns only bool, so an error from the filter searcher's
//    Next/Advance is turned into "no match" and the search reports success.
// Each is contrasted with the equivalent ConjunctionQuery.
package main

import (
	"context"
	"errors"
	"fmt"
	"os"

	"github.com/blevesearch/bleve/v2"
	"github.com/blevesearch/bleve/v2/index/scorch"
	"github.com/blevesearch/bleve/v2/mapping"
	"github.com/blevesearch/bleve/v2/search"
	"github.com/blevesearch/bleve/v2/search/query"
	index "github.com/blevesearch/bleve_index_api"
)

func main() {
	idx, err := bleve.NewUsing(mustTemp(), mapping.NewIndexMapping(), scorch.Name, scorch.Name, nil)
	check(err)
	defer idx.Close()
	for _, d := range []map[string]any{
		{"title": "alpha beta", "kind": "dashboard"},
		{"title": "beta gamma", "kind": "dashboard"},
		{"title": "alpha gamma", "kind": "folder"},
	} {
		check(idx.Index(d["title"].(string), d))
	}

	title := func(t string) *query.MatchQuery { q := bleve.NewMatchQuery(t); q.SetField("title"); return q }
	kind := func(t string) *query.MatchQuery { q := bleve.NewMatchQuery(t); q.SetField("kind"); return q }

	// Issue A: filter searcher Close() is never called.
	fmt.Println("=== Issue A: filter searcher Close() is never called ===")
	bq := bleve.NewBooleanQuery()
	bq.AddMust(title("alpha"))
	bq.AddFilter(kind("dashboard"))
	s, f := searchStats(idx, bq)
	fmt.Printf("  BooleanQuery{Must,Filter}: started +%d finished +%d\n", s, f)
	if s != f {
		fmt.Printf("  => BUG: %d term searcher(s) started but never closed (the Filter searcher)\n", s-f)
	}
	s, f = searchStats(idx, bleve.NewConjunctionQuery(title("alpha"), kind("dashboard")))
	fmt.Printf("  ConjunctionQuery (contrast): started +%d finished +%d => balanced\n", s, f)

	// Issue B: filter searcher errors are silently swallowed.
	fmt.Println("\n=== Issue B: filter searcher errors are silently swallowed ===")
	faulty := &faultyQuery{kind("dashboard")}
	bq = bleve.NewBooleanQuery()
	bq.AddMust(title("alpha"))
	bq.AddFilter(faulty)
	if res, err := idx.Search(bleve.NewSearchRequest(bq)); err != nil {
		fmt.Printf("  BooleanQuery Filter: error (good): %v\n", err)
	} else {
		fmt.Printf("  BooleanQuery Filter: NO error, %d hit(s) => BUG: error swallowed, result silently partial/empty\n", res.Total)
	}
	if _, err := idx.Search(bleve.NewSearchRequest(bleve.NewConjunctionQuery(title("alpha"), faulty))); err != nil {
		fmt.Printf("  ConjunctionQuery (contrast): error (as expected): %v\n", err)
	}
}

// searchStats runs q and returns the deltas in scorch's term-searcher counters.
func searchStats(idx bleve.Index, q query.Query) (started, finished uint64) {
	adv, _ := idx.Advanced()
	st := adv.(*scorch.Scorch).Stats().(*scorch.Stats)
	s0, f0 := st.TotTermSearchersStarted, st.TotTermSearchersFinished
	check1(idx.Search(bleve.NewSearchRequest(q)))
	return st.TotTermSearchersStarted - s0, st.TotTermSearchersFinished - f0
}

var errInjected = errors.New("injected filter searcher error (e.g. segment read failure)")

// faultyQuery wraps a real query but its searcher errors on Next/Advance,
// mimicking a segment read/decompression/corruption error during iteration.
type faultyQuery struct{ query.Query }

func (fq *faultyQuery) Searcher(ctx context.Context, i index.IndexReader, m mapping.IndexMapping, o search.SearcherOptions) (search.Searcher, error) {
	s, err := fq.Query.Searcher(ctx, i, m, o)
	return &faultySearcher{s}, err
}

type faultySearcher struct{ search.Searcher }

func (faultySearcher) Next(*search.SearchContext) (*search.DocumentMatch, error) {
	return nil, errInjected
}
func (faultySearcher) Advance(*search.SearchContext, index.IndexInternalID) (*search.DocumentMatch, error) {
	return nil, errInjected
}

func mustTemp() string { d, err := os.MkdirTemp("", "blevererepro"); check(err); return d }
func check(err error) {
	if err != nil {
		panic(err)
	}
}
func check1(_ any, err error) { check(err) }

Suggested fix

Let FilteringSearcher hold the filter searcher and close it in Close(), and give FilterFunc an error return (func(...) (bool, error)) so filter errors can surface — the same approach already used for CustomFilterFunc/CustomScoreFunc in #2344.

Found by @RafaelPaulovic while using BooleanQuery.Filter (added in #2220) for non-scoring filter/label clauses in Grafana's search backend.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions