mirror of
https://github.com/opencloud-eu/opencloud.git
synced 2026-09-12 21:58:58 -04:00
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 <term>` (the web Files filter) at the root, so the earlier mediatype:file group workaround is dropped.
This commit is contained in:
1 parent
5d9a255b16
commit
f6739f552d
7 files changed
+55
-22
No files matched your search
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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{
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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 <term>` 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")
|
||||
|
||||
@@ -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"},
|
||||
}))
|
||||
})
|
||||
|
||||
|
||||
@@ -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},
|
||||
}))
|
||||
})
|
||||
|
||||
|
||||
Reference in new issue
Block a user