From f6739f552d53d20eb3b321d867b01a3fb22c6244 Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Tue, 11 Aug 2026 09:48:05 +0200 Subject: [PATCH] fix(search): bind a leading NOT to the term right after it, on both backends A leading NOT next to an operator was miscompiled: the bleve compiler left the consumed term in `next`, so `NOT x AND y` dropped `y` and produced a self-contradicting clause; the OpenSearch transpiler checked nextOp==AND before prevOp==NOT, so the negated term landed in `must` instead of `must_not`. NOT is unary and binds to the node directly after it regardless of what follows. This also fixes `mediatype:file AND ` (the web Files filter) at the root, so the earlier mediatype:file group workaround is dropped. --- services/search/pkg/bleve/backend_test.go | 12 +++++++++++ .../internal/convert/kql_transpile.go | 14 ++++++++++--- .../internal/convert/kql_transpile_test.go | 20 +++++++++++++++++++ services/search/pkg/query/bleve/compiler.go | 5 ++++- .../search/pkg/query/mimetype/mimetype.go | 9 ++------- .../pkg/query/mimetype/mimetype_test.go | 7 ++----- services/search/pkg/query/normalize_test.go | 10 ++++------ 7 files changed, 55 insertions(+), 22 deletions(-) diff --git a/services/search/pkg/bleve/backend_test.go b/services/search/pkg/bleve/backend_test.go index 16422b809b..417c6c50ad 100644 --- a/services/search/pkg/bleve/backend_test.go +++ b/services/search/pkg/bleve/backend_test.go @@ -196,6 +196,18 @@ var _ = Describe("Bleve", func() { assertDocCount(rootResource.ID, "tag:missing", 0) }) + It("binds a leading NOT to the term right after it, combined with AND", func() { + // regression: a leading NOT next to AND dropped the AND'd term, so + // `NOT tag:x AND name:y` matched nothing (a self-contradicting clause). + parentResource.Document.Tags = []string{"physik"} + childResource.Document.Tags = []string{"mathe"} + Expect(eng.Upsert(parentResource.ID, parentResource)).To(Succeed()) + Expect(eng.Upsert(childResource.ID, childResource)).To(Succeed()) + + assertDocCount(rootResource.ID, "NOT tag:physik AND name:child.pdf", 1) // the mathe child + assertDocCount(rootResource.ID, "NOT tag:mathe AND name:parent*", 1) // the physik parent + }) + It("finds files by size", func() { parentResource.Document.Size = 12345 err := eng.Upsert(parentResource.ID, parentResource) diff --git a/services/search/pkg/opensearch/internal/convert/kql_transpile.go b/services/search/pkg/opensearch/internal/convert/kql_transpile.go index 368f1ad9be..0e74dd96c0 100644 --- a/services/search/pkg/opensearch/internal/convert/kql_transpile.go +++ b/services/search/pkg/opensearch/internal/convert/kql_transpile.go @@ -49,13 +49,21 @@ func (t kqlOpensearchTranspiler) transpile(nodes []ast.Node) (osu.Builder, error nextOp := t.getOperatorValueAt(nodes, i+1) prevOp := t.getOperatorValueAt(nodes, i-1) + // A preceding NOT negates this node regardless of what follows (NOT x AND y + // is (NOT x) AND y), so it must win over nextOp. The prevOp AND/OR cases + // give the right operand its own bucket instead of inheriting the previous + // one, which matters right after a NOT (its MustNot must not carry over). switch { + case prevOp == kql.BoolNOT: + boolQueryAdd = boolQuery.MustNot case nextOp == kql.BoolOR: boolQueryAdd = boolQuery.Should case nextOp == kql.BoolAND: boolQueryAdd = boolQuery.Must - case prevOp == kql.BoolNOT: - boolQueryAdd = boolQuery.MustNot + case prevOp == kql.BoolOR: + boolQueryAdd = boolQuery.Should + case prevOp == kql.BoolAND: + boolQueryAdd = boolQuery.Must } builder, err := t.toBuilder(node) @@ -72,7 +80,7 @@ func (t kqlOpensearchTranspiler) transpile(nodes []ast.Node) (osu.Builder, error continue } - if nextOp == kql.BoolOR { + if nextOp == kql.BoolOR || prevOp == kql.BoolOR { // if there are should clauses, we set the minimum should match to 1 boolQueryParams.MinimumShouldMatch = 1 } diff --git a/services/search/pkg/opensearch/internal/convert/kql_transpile_test.go b/services/search/pkg/opensearch/internal/convert/kql_transpile_test.go index 8cfbf54472..85996976d7 100644 --- a/services/search/pkg/opensearch/internal/convert/kql_transpile_test.go +++ b/services/search/pkg/opensearch/internal/convert/kql_transpile_test.go @@ -279,6 +279,26 @@ func TestTranspileKQLToOpenSearch(t *testing.T) { osu.NewTermQuery[string]("age").Value("32"), ), }, + { + // NOT binds to the node directly after it, not to whatever operator + // follows that node: NOT x AND y is (NOT x) AND y. + Name: "[NOT * AND *]", + Got: &ast.Ast{ + Nodes: []ast.Node{ + &ast.OperatorNode{Value: "NOT"}, + &ast.StringNode{Key: "age", Value: "32"}, + &ast.OperatorNode{Value: "AND"}, + &ast.StringNode{Key: "Name", Value: "openCloud"}, + }, + }, + Want: osu.NewBoolQuery(). + MustNot( + osu.NewTermQuery[string]("age").Value("32"), + ). + Must( + osu.NewTermQuery[string]("Name").Value("openCloud"), + ), + }, { Name: "[* OR * OR *]", Got: &ast.Ast{ diff --git a/services/search/pkg/query/bleve/compiler.go b/services/search/pkg/query/bleve/compiler.go index 5a57d2bc78..4a536d3776 100644 --- a/services/search/pkg/query/bleve/compiler.go +++ b/services/search/pkg/query/bleve/compiler.go @@ -190,8 +190,11 @@ func walk(offset int, nodes []ast.Node) (bleveQuery.Query, int, error) { q := bleve.NewBooleanQuery() q.AddMustNot(next) if prev == nil { - // unary in the beginning + // unary at the beginning: the term was consumed into the + // MustNot via nextNode, so clear next, otherwise a following + // operator would bind the stale term (NOT x AND y drops y). prev = q + next = nil } else { next = q } diff --git a/services/search/pkg/query/mimetype/mimetype.go b/services/search/pkg/query/mimetype/mimetype.go index 06700049e9..ca61e651ac 100644 --- a/services/search/pkg/query/mimetype/mimetype.go +++ b/services/search/pkg/query/mimetype/mimetype.go @@ -23,14 +23,9 @@ func Expand(key, value string) []ast.Node { value = strings.ToLower(value) switch value { case "file": - // Group the negation so it stays atomic next to an operator: a bare - // `NOT ` sequence spliced inline miscompiles as the left of an AND - // (mediatype:file AND name:x would drop name:x). return []ast.Node{ - &ast.GroupNode{Nodes: []ast.Node{ - &ast.OperatorNode{Value: kql.BoolNOT}, - &ast.StringNode{Key: field, Value: "httpd/unix-directory"}, - }}, + &ast.OperatorNode{Value: kql.BoolNOT}, + &ast.StringNode{Key: field, Value: "httpd/unix-directory"}, } case "folder": return term("httpd/unix-directory") diff --git a/services/search/pkg/query/mimetype/mimetype_test.go b/services/search/pkg/query/mimetype/mimetype_test.go index 51fe24e357..d48fb44e6a 100644 --- a/services/search/pkg/query/mimetype/mimetype_test.go +++ b/services/search/pkg/query/mimetype/mimetype_test.go @@ -55,12 +55,9 @@ var _ = Describe("Expand", func() { }) It("expands file to not-a-folder", func() { - // grouped so the negation stays atomic next to an operator. Expect(mimetype.Expand("mediatype", "file")).To(Equal([]ast.Node{ - &ast.GroupNode{Nodes: []ast.Node{ - &ast.OperatorNode{Value: "NOT"}, - &ast.StringNode{Key: "MimeType", Value: "httpd/unix-directory"}, - }}, + &ast.OperatorNode{Value: "NOT"}, + &ast.StringNode{Key: "MimeType", Value: "httpd/unix-directory"}, })) }) diff --git a/services/search/pkg/query/normalize_test.go b/services/search/pkg/query/normalize_test.go index 617283277a..3fec574da1 100644 --- a/services/search/pkg/query/normalize_test.go +++ b/services/search/pkg/query/normalize_test.go @@ -61,12 +61,10 @@ var _ = Describe("Normalize", func() { &ast.OperatorNode{Value: "AND"}, &ast.StringNode{Key: "photo.cameraMake", Value: "canon"}, &ast.OperatorNode{Value: "AND"}, - &ast.GroupNode{Nodes: []ast.Node{ - &ast.OperatorNode{Value: "NOT"}, - &ast.StringNode{Key: "MimeType", Value: "httpd/unix-directory"}, - &ast.OperatorNode{Value: "AND"}, - &ast.NumberNode{Key: "Size", Value: 100}, - }}, + &ast.OperatorNode{Value: "NOT"}, + &ast.StringNode{Key: "MimeType", Value: "httpd/unix-directory"}, + &ast.OperatorNode{Value: "AND"}, + &ast.NumberNode{Key: "Size", Value: 100}, })) })