MB-27666: Fix nested mode handling for field-agnostic queries #2272
Conversation
CascadingRadium
changed the title
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug in nested-mode search where field-agnostic queries (match_all, docID) were incorrectly filtered at the searcher level instead of the collector level. The issue caused zero matches when these queries were used in compound queries (e.g., Boolean filters) due to a level mismatch between searcher filtering and nested-level execution.
Changes:
- Introduced a
Global()flag in theFieldSetinterface to identify field-agnostic queries - Removed searcher-level nested document filtering from
MatchAllSearcher - Updated nested collector activation logic to trigger for global queries, ensuring consistent filtering at the collector level
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| search/util.go | Converted FieldSet from a map type to an interface with a private implementation, adding Global() flag to track field-agnostic queries |
| search/query/query.go | Updated ExtractFields to set the Global() flag for MatchAllQuery and DocIDQuery |
| search/searcher/search_match_all.go | Removed nested document filtering logic (isNested method) to unify filtering at collector level |
| index_impl.go | Enhanced nested collector activation logic to include global queries; refactored field iteration to use VisitFields |
| registry/nested.go | Adapted to new FieldSet interface using VisitFields method |
| index_alias_impl.go | Updated field iteration to use new FieldSet.VisitFields API |
| mapping/index.go | Updated method call from Slice() to Fields() |
| search_nested_test.go | Added comprehensive test cases for match_all, docID, and Boolean filter queries in nested mode |
| search/query/query_test.go | Extended test coverage for field extraction with expGlobal assertions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
CascadingRadium
requested review from
Likith101,
Thejas-bhat,
abhinavdangeti,
capemox and
maneuvertomars
abhinavdangeti
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
registry/nested.go:1
- Potential panic when accessing
ancestors[len(ancestors)-pos-1]ifancestorsis empty or ifpos >= len(ancestors). With the changes allowing global queries in nested contexts, this function may be called with empty ancestor chains from top-level documents matched by match_all queries. Add bounds checking to prevent panics.
// Copyright (c) 2026 Couchbase, Inc.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if fs == nil { | ||
| fs = search.NewFieldSet() | ||
| } | ||
| fs.SetGlobal() |
There was a problem hiding this comment.
MatchAll and DocID both go to the _id internal field - so they're not technically "field-agnostic".
|
|
||
| // SetGlobal sets the field-agnostic flag. | ||
| func (fs *FieldSet) SetGlobal() { | ||
| fs.global = true |
There was a problem hiding this comment.
Poor naming choice for field-agnostic flag - our definition of "field agnostic" search is going to the _all composite field and yet you're setting this for MatchAll and DocID which go to the _id internal field.
| fs.global = true | ||
| } | ||
|
|
||
| // Global returns true if the set contains any field-agnostic queries, |
There was a problem hiding this comment.
Need to refine this naming to avoid confusion here.
match_all,docID): These queries previously filtered nested documents at the searcher level, but sinceExtractFieldsreturned no fields, the nested collector was never activated. In compound queries (e.g., Boolean filters), this caused a level mismatch—searcher filtering vs nested-level execution—resulting in zero matches.Global()flag inFieldSetto track field-agnostic queries. Removed searcher-level filtering fromMatchAllSearcher.match_all queryas a sub-clause in aNestedConjunctionSearchercould cause a panic.