From 95f175fe10a202dbf137a28fcdc64416f459f31a Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Thu, 13 Aug 2026 02:10:29 +0200 Subject: [PATCH] perf(search): prune the space fan-out for drive-pinned queries When top-level AND conjuncts pin the query to a single root (driveId/RootID restrictions), only that space's index is asked; the restriction itself stays in the query, so this is purely an optimization. Conservative by design: any top-level OR, negated or group-nested restriction leaves the fan-out untouched, searching a space too many is wasted work while skipping one would be wrong. Mountpoints are kept for result path mapping. Costs one extra parse of the query in the service; parsing once and handing the AST to the engines (which currently re-parse per space) is a follow-up that changes the engine interface. --- services/search/pkg/query/normalize.go | 17 +----- services/search/pkg/search/pinnedroot_test.go | 28 +++++++++ services/search/pkg/search/search.go | 61 +++++++++++++++++++ services/search/pkg/search/service.go | 14 +++++ services/search/pkg/search/service_test.go | 11 ++++ 5 files changed, 116 insertions(+), 15 deletions(-) create mode 100644 services/search/pkg/search/pinnedroot_test.go diff --git a/services/search/pkg/query/normalize.go b/services/search/pkg/query/normalize.go index 62577ca6fb..bce1fd1be6 100644 --- a/services/search/pkg/query/normalize.go +++ b/services/search/pkg/query/normalize.go @@ -6,10 +6,10 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "reflect" - "strings" "github.com/opencloud-eu/opencloud/pkg/ast" "github.com/opencloud-eu/opencloud/services/search/pkg/query/mimetype" + "github.com/opencloud-eu/opencloud/services/search/pkg/search" ) // Normalize is the shared KQL lowering pass between parse and compile: it @@ -43,7 +43,7 @@ func normalizeNodes(nodes []ast.Node, resolve func(string) string, defaultKey st node.Value = resourceType(node.Value) } if node.Key == "RootID" { - node.Value = completeRootID(node.Value) + node.Value = search.CompleteRootID(node.Value) } if exp := mimetype.Expand(node.Key, node.Value); exp != nil { out = append(out, normalizeNodes(exp, resolve, defaultKey)...) @@ -75,19 +75,6 @@ func normalizeNodes(nodes []ast.Node, resolve func(string) string, defaultKey st return out } -// completeRootID turns a driveId ("storage$space") into the full root -// resource id ("storage$space!space") stored in the index: a space root's -// opaque id is its space id. Full ids pass through untouched. -func completeRootID(v string) string { - if strings.Contains(v, "!") { - return v - } - if i := strings.LastIndex(v, "$"); i >= 0 && i+1 < len(v) { - return v + "!" + v[i+1:] - } - return v -} - // toPointer returns n as a pointer; the parser emits some nodes by value and the // in-place key rewrites would be lost on those. func toPointer(n ast.Node) ast.Node { diff --git a/services/search/pkg/search/pinnedroot_test.go b/services/search/pkg/search/pinnedroot_test.go new file mode 100644 index 0000000000..64aea028c4 --- /dev/null +++ b/services/search/pkg/search/pinnedroot_test.go @@ -0,0 +1,28 @@ +package search_test + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + + "github.com/opencloud-eu/opencloud/services/search/pkg/search" +) + +var _ = DescribeTable("PinnedRootID", + func(query, want string) { + Expect(search.PinnedRootID(query)).To(Equal(want)) + }, + Entry("bare driveId is completed to the root id", `driveId:"1$2"`, "1$2!2"), + Entry("full root id passes through", `driveId:"1$2!4" name:x`, "1$2!4"), + Entry("RootID works as well", `RootID:"1$2" AND name:x`, "1$2!2"), + Entry("top-level AND conjuncts pin", `driveId:"1$2" AND name:x AND tag:y`, "1$2!2"), + Entry("repeated identical AND restrictions pin", `driveId:"1$2" AND driveId:"1$2"`, "1$2!2"), + // KQL groups adjacent same-key restrictions into an implicit OR group + Entry("implicitly repeated restrictions do not pin", `driveId:"1$2" driveId:"1$2"`, ""), + Entry("no restriction, no pin", `name:x`, ""), + Entry("a top-level OR disables pruning", `name:x OR driveId:"1$2"`, ""), + Entry("OR between other terms disables pruning", `driveId:"1$2" AND (a OR b)`, "1$2!2"), + Entry("a negated restriction disables pruning", `NOT driveId:"1$2" AND name:x`, ""), + Entry("restrictions inside groups do not pin", `(driveId:"1$2") AND name:x`, ""), + Entry("two different roots disable pruning", `driveId:"1$2" AND driveId:"1$3"`, ""), + Entry("invalid query, no pin", `((`, ""), +) diff --git a/services/search/pkg/search/search.go b/services/search/pkg/search/search.go index fd7cb6c0d6..007813adf8 100644 --- a/services/search/pkg/search/search.go +++ b/services/search/pkg/search/search.go @@ -11,6 +11,8 @@ import ( gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/opencloud-eu/opencloud/pkg/ast" + "github.com/opencloud-eu/opencloud/pkg/kql" "github.com/opencloud-eu/reva/v2/pkg/conversions" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/opencloud-eu/reva/v2/pkg/storage/utils/grants" @@ -223,6 +225,65 @@ func convertToWebDAVPermissions(isShared, isMountpoint, isDir bool, p *provider. return b.String() } +// CompleteRootID completes a bare driveId ("storage$space") to the root +// resource id stored in the index; a root's opaque id is its space id. +func CompleteRootID(v string) string { + if strings.Contains(v, "!") { + return v + } + if i := strings.LastIndex(v, "$"); i >= 0 && i+1 < len(v) { + return v + "!" + v[i+1:] + } + return v +} + +// PinnedRootID returns the single space root the query is pinned to via +// top-level AND driveId/RootID conjuncts, or "". OR, NOT and group-nested +// restrictions never pin: skipping a space would be wrong. +func PinnedRootID(query string) string { + a, err := kql.Builder{}.Build(query) + if err != nil { + return "" + } + pinned := "" + negated := false + for _, n := range a.Nodes { + switch node := n.(type) { + case *ast.OperatorNode: + if strings.EqualFold(node.Value, "OR") { + return "" + } + negated = strings.EqualFold(node.Value, "NOT") + continue + case ast.OperatorNode: + if strings.EqualFold(node.Value, "OR") { + return "" + } + negated = strings.EqualFold(node.Value, "NOT") + continue + } + var key, value string + switch node := n.(type) { + case *ast.StringNode: + key, value = node.Key, node.Value + case ast.StringNode: + key, value = node.Key, node.Value + } + if strings.EqualFold(key, "driveid") || strings.EqualFold(key, "rootid") { + if negated { + return "" + } + v := CompleteRootID(value) + if pinned != "" && pinned != v { + return "" + } + pinned = v + } + negated = false + } + return pinned +} + // ParseScope extract a scope value from the query string and returns search, scope strings func ParseScope(query string) (string, string) { match := scopeRegex.FindStringSubmatch(query) diff --git a/services/search/pkg/search/service.go b/services/search/pkg/search/service.go index 0c6611be10..7c8ae567dc 100644 --- a/services/search/pkg/search/service.go +++ b/services/search/pkg/search/service.go @@ -167,6 +167,17 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se Path: gpRes.Path, } } + // drive-pinned queries only need that space's index; the restriction + // stays in the query, mountpoints are kept for path mapping + var pinnedRoot *provider.ResourceId + if req.Ref == nil { + if pinned := PinnedRootID(req.Query); pinned != "" { + if rid, err := storagespace.ParseID(pinned); err == nil { + pinnedRoot = &rid + } + } + } + filters := []*provider.ListStorageSpacesRequest_Filter{ { Type: provider.ListStorageSpacesRequest_Filter_TYPE_USER, @@ -195,6 +206,9 @@ func (s *Service) Search(ctx context.Context, req *searchsvc.SearchRequest) (*se // We still need the mountpoint in order to map the result paths to the according share continue } + if pinnedRoot != nil && space.SpaceType != _spaceTypeMountpoint && pinnedRoot.GetSpaceId() != space.Root.GetSpaceId() { + continue + } spaces = append(spaces, space) } diff --git a/services/search/pkg/search/service_test.go b/services/search/pkg/search/service_test.go index 3a75bd0c6e..4af3d014a3 100644 --- a/services/search/pkg/search/service_test.go +++ b/services/search/pkg/search/service_test.go @@ -498,6 +498,17 @@ var _ = Describe("Searchprovider", func() { }, nil) }) + It("prunes the fan-out when the query pins a drive", func() { + res, err := s.Search(ctx, &searchsvc.SearchRequest{ + Query: `driveId:"storageid$personalspace" AND foo`, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeNil()) + Expect(len(res.Matches)).To(Equal(1)) + Expect(res.Matches[0].Entity.Id.OpaqueId).To(Equal("foo-id")) + indexClient.AssertNumberOfCalls(GinkgoT(), "Search", 1) + }) + It("considers the search Ref parameter", func() { res, err := s.Search(ctx, &searchsvc.SearchRequest{ Query: "foo",