Search: Modify query for better performance (#77576)

* Add missing `org_id` in query condition

* Update benchmarks

pkg/api/folder_bench_test.go

@@ -96,49 +96,97 @@ func BenchmarkFolderListAndSearch(b *testing.B) {
 		features    *featuremgmt.FeatureManager
 	}{
 		{
-			desc:        "get root folders with nested folders feature enabled",
+			desc:        "impl=default nested_folders=on get root folders",
+			url:         "/api/folders",
+			expectedLen: LEVEL0_FOLDER_NUM,
+			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=on get root folders",
 			url:         "/api/folders",
 			expectedLen: LEVEL0_FOLDER_NUM,
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
 		},
 		{
-			desc:        "get subfolders with nested folders feature enabled",
+			desc:        "impl=default nested_folders=on get subfolders",
+			url:         "/api/folders?parentUid=folder0",
+			expectedLen: LEVEL1_FOLDER_NUM,
+			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=on get subfolders",
 			url:         "/api/folders?parentUid=folder0",
 			expectedLen: LEVEL1_FOLDER_NUM,
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
 		},
 		{
-			desc:        "list all inherited dashboards with nested folders feature enabled",
+			desc:        "impl=default nested_folders=on list all inherited dashboards",
+			url:         "/api/search?type=dash-db&limit=5000",
+			expectedLen: withLimit(all),
+			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=on list all inherited dashboards",
 			url:         "/api/search?type=dash-db&limit=5000",
 			expectedLen: withLimit(all),
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
 		},
 		{
-			desc:        "search for pattern with nested folders feature enabled",
+			desc:        "impl=default nested_folders=on search for pattern",
+			url:         "/api/search?type=dash-db&query=dashboard_0_0&limit=5000",
+			expectedLen: withLimit(1 + LEVEL1_DASHBOARD_NUM + LEVEL2_FOLDER_NUM*LEVEL2_DASHBOARD_NUM),
+			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=on search for pattern",
 			url:         "/api/search?type=dash-db&query=dashboard_0_0&limit=5000",
 			expectedLen: withLimit(1 + LEVEL1_DASHBOARD_NUM + LEVEL2_FOLDER_NUM*LEVEL2_DASHBOARD_NUM),
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
 		},
 		{
-			desc:        "search for specific dashboard nested folders feature enabled",
+			desc:        "impl=default nested_folders=on search for specific dashboard",
+			url:         "/api/search?type=dash-db&query=dashboard_0_0_0_0",
+			expectedLen: 1,
+			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=on search for specific dashboard",
 			url:         "/api/search?type=dash-db&query=dashboard_0_0_0_0",
 			expectedLen: 1,
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
 		},
 		{
-			desc:        "get root folders with nested folders feature disabled",
+			desc:        "impl=default nested_folders=off get root folders",
+			url:         "/api/folders?limit=5000",
+			expectedLen: withLimit(LEVEL0_FOLDER_NUM),
+			features:    featuremgmt.WithFeatures(),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=off get root folders",
 			url:         "/api/folders?limit=5000",
 			expectedLen: withLimit(LEVEL0_FOLDER_NUM),
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),
 		},
 		{
-			desc:        "list all dashboards with nested folders feature disabled",
+			desc:        "impl=default nested_folders=off list all dashboards",
+			url:         "/api/search?type=dash-db&limit=5000",
+			expectedLen: withLimit(LEVEL0_FOLDER_NUM * LEVEL0_DASHBOARD_NUM),
+			features:    featuremgmt.WithFeatures(),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=off list all dashboards",
 			url:         "/api/search?type=dash-db&limit=5000",
 			expectedLen: withLimit(LEVEL0_FOLDER_NUM * LEVEL0_DASHBOARD_NUM),
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),
 		},
 		{
-			desc:        "search specific dashboard with nested folders feature disabled",
+			desc:        "impl=default nested_folders=off search specific dashboard",
+			url:         "/api/search?type=dash-db&query=dashboard_0_0",
+			expectedLen: 1,
+			features:    featuremgmt.WithFeatures(),
+		},
+		{
+			desc:        "impl=permissionsFilterRemoveSubquery nested_folders=off search specific dashboard",
 			url:         "/api/search?type=dash-db&query=dashboard_0_0",
 			expectedLen: 1,
 			features:    featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),

pkg/services/sqlstore/permissions/dashboard.go

@@ -41,7 +41,7 @@ type PermissionsFilter interface {
 	Where() (string, []any)
 
 	buildClauses()
-	nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string) (string, []any)
+	nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string, orgID int64) (string, []any)
 }
 
 // NewAccessControlDashboardPermissionFilter creates a new AccessControlDashboardPermissionFilter that is configured with specific actions calculated based on the dashboards.PermissionType and query type
@@ -126,7 +126,8 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
 		userID, _ = identity.IntIdentifier(namespaceID, identifier)
 	}
 
-	filter, params := accesscontrol.UserRolesFilter(f.user.GetOrgID(), userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
+	orgID := f.user.GetOrgID()
+	filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
 	rolesFilter := " AND role_id IN(SELECT id FROM role " + filter + ") "
 	var args []any
 	builder := strings.Builder{}
@@ -208,9 +209,10 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
 						builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
 						recQueryName := fmt.Sprintf("RecQry%d", len(f.recQueries))
 						f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs)
-						builder.WriteString(fmt.Sprintf("WHERE d.uid IN (SELECT uid FROM %s)", recQueryName))
+						builder.WriteString(fmt.Sprintf("WHERE d.org_id = ? AND d.uid IN (SELECT uid FROM %s)", recQueryName))
+						args = append(args, orgID)
 					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.folder_id", "d.id")
+						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.folder_id", "d.id", orgID)
 						builder.WriteRune('(')
 						builder.WriteString(nestedFoldersSelectors)
 						args = append(args, nestedFoldersArgs...)
@@ -222,7 +224,8 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
 			default:
 				builder.WriteString("(dashboard.folder_id IN (SELECT d.id FROM dashboard as d ")
 				if len(permSelectorArgs) > 0 {
-					builder.WriteString("WHERE d.uid IN ")
+					builder.WriteString("WHERE d.org_id = ? AND d.uid IN ")
+					args = append(args, orgID)
 					builder.WriteString(permSelector.String())
 					args = append(args, permSelectorArgs...)
 				} else {
@@ -287,7 +290,7 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
 						builder.WriteString("(dashboard.uid IN ")
 						builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
 					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "d.uid")
+						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "d.uid", orgID)
 						builder.WriteRune('(')
 						builder.WriteString(nestedFoldersSelectors)
 						builder.WriteRune(')')
@@ -372,7 +375,7 @@ func actionsToCheck(actions []string, permissions map[string][]string, wildcards
 	return toCheck
 }
 
-func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string) (string, []any) {
+func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, rightTableCol string, orgID int64) (string, []any) {
 	wheres := make([]string, 0, folder.MaxNestedFolderDepth+1)
 	args := make([]any, 0, len(permSelectorArgs)*(folder.MaxNestedFolderDepth+1))
 
@@ -387,7 +390,8 @@ func (f *accessControlDashboardPermissionFilter) nestedFoldersSelectors(permSele
 		s := fmt.Sprintf(tmpl, t, prev, onCol, t, prev, t)
 		joins = append(joins, s)
 
-		wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT %s FROM dashboard d %s WHERE %s.uid IN %s)", leftTableCol, rightTableCol, strings.Join(joins, " "), t, permSelector))
+		wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT %s FROM dashboard d %s WHERE %s.org_id = ? AND %s.uid IN %s)", leftTableCol, rightTableCol, strings.Join(joins, " "), t, t, permSelector))
+		args = append(args, orgID)
 		args = append(args, permSelectorArgs...)
 
 		prev = t

pkg/services/sqlstore/permissions/dashboard_filter_no_subquery.go

@@ -40,7 +40,8 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
 		userID, _ = identity.IntIdentifier(namespaceID, identifier)
 	}
 
-	filter, params := accesscontrol.UserRolesFilter(f.user.GetOrgID(), userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
+	orgID := f.user.GetOrgID()
+	filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
 	rolesFilter := " AND role_id IN(SELECT id FROM role " + filter + ") "
 	var args []any
 	builder := strings.Builder{}
@@ -124,7 +125,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
 						f.addRecQry(recQueryName, permSelector.String(), permSelectorArgs)
 						builder.WriteString("(folder.uid IN (SELECT uid FROM " + recQueryName)
 					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder.uid", "")
+						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "folder.uid", "", orgID)
 						builder.WriteRune('(')
 						builder.WriteString(nestedFoldersSelectors)
 						args = append(args, nestedFoldersArgs...)
@@ -202,7 +203,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
 						builder.WriteString("(dashboard.uid IN ")
 						builder.WriteString(fmt.Sprintf("(SELECT uid FROM %s)", recQueryName))
 					default:
-						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "")
+						nestedFoldersSelectors, nestedFoldersArgs := f.nestedFoldersSelectors(permSelector.String(), permSelectorArgs, "dashboard.uid", "", orgID)
 						builder.WriteRune('(')
 						builder.WriteString(nestedFoldersSelectors)
 						builder.WriteRune(')')
@@ -230,7 +231,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
 	f.where = clause{string: builder.String(), params: args}
 }
 
-func (f *accessControlDashboardPermissionFilterNoFolderSubquery) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, _ string) (string, []any) {
+func (f *accessControlDashboardPermissionFilterNoFolderSubquery) nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTableCol string, _ string, orgID int64) (string, []any) {
 	wheres := make([]string, 0, folder.MaxNestedFolderDepth+1)
 	args := make([]any, 0, len(permSelectorArgs)*(folder.MaxNestedFolderDepth+1))
 
@@ -247,7 +248,8 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) nestedFoldersSe
 		s := fmt.Sprintf(tmpl, t, prev, t, prev, t)
 		joins = append(joins, s)
 
-		wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT f1.uid FROM folder f1 %s WHERE %s.uid IN %s)", leftTableCol, strings.Join(joins, " "), t, permSelector))
+		wheres = append(wheres, fmt.Sprintf("(%s IN (SELECT f1.uid FROM folder f1 %s WHERE %s.org_id = ? AND %s.uid IN %s)", leftTableCol, strings.Join(joins, " "), t, t, permSelector))
+		args = append(args, orgID)
 		args = append(args, permSelectorArgs...)
 
 		prev = t

pkg/services/sqlstore/searchstore/search_test.go

@@ -3,7 +3,6 @@ package searchstore_test
 
 import (
 	"context"
-	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -120,12 +119,12 @@ func TestBuilder_RBAC(t *testing.T) {
 	testsCases := []struct {
 		desc            string
 		userPermissions []accesscontrol.Permission
-		features        []any
+		features        featuremgmt.FeatureToggles
 		expectedParams  []any
 	}{
 		{
 			desc:     "no user permissions",
-			features: []any{},
+			features: featuremgmt.WithFeatures(),
 			expectedParams: []any{
 				int64(1),
 			},
@@ -135,7 +134,83 @@ func TestBuilder_RBAC(t *testing.T) {
 			userPermissions: []accesscontrol.Permission{
 				{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
 			},
-			features: []any{},
+			features: featuremgmt.WithFeatures(),
+			expectedParams: []any{
+				int64(1),
+				int64(1),
+				int64(1),
+				0,
+				"Viewer",
+				int64(1),
+				0,
+				"dashboards:read",
+				"dashboards:write",
+				2,
+				int64(1),
+				int64(1),
+				int64(1),
+				0,
+				"Viewer",
+				int64(1),
+				0,
+				"dashboards:read",
+				"dashboards:write",
+				2,
+				int64(1),
+				int64(1),
+				0,
+				"Viewer",
+				int64(1),
+				0,
+				"folders:read",
+				"dashboards:create",
+				2,
+			},
+		},
+		{
+			desc: "user with view permission with nesting",
+			userPermissions: []accesscontrol.Permission{
+				{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
+			},
+			features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders),
+			expectedParams: []any{
+				int64(1),
+				int64(1),
+				0,
+				"Viewer",
+				int64(1),
+				0,
+				"dashboards:read",
+				"dashboards:write",
+				2,
+				int64(1),
+				int64(1),
+				0,
+				"Viewer",
+				int64(1),
+				0,
+				"folders:read",
+				"dashboards:create",
+				2,
+				int64(1),
+				int64(1),
+				int64(1),
+				0,
+				"Viewer",
+				int64(1),
+				0,
+				"dashboards:read",
+				"dashboards:write",
+				2,
+				int64(1),
+			},
+		},
+		{
+			desc: "user with view permission with remove subquery",
+			userPermissions: []accesscontrol.Permission{
+				{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
+			},
+			features: featuremgmt.WithFeatures(featuremgmt.FlagPermissionsFilterRemoveSubquery),
 			expectedParams: []any{
 				int64(1),
 				int64(1),
@@ -168,11 +243,11 @@ func TestBuilder_RBAC(t *testing.T) {
 			},
 		},
 		{
-			desc: "user with view permission with nesting",
+			desc: "user with view permission with nesting and remove subquery",
 			userPermissions: []accesscontrol.Permission{
 				{Action: dashboards.ActionDashboardsRead, Scope: "dashboards:uid:1"},
 			},
-			features: []any{featuremgmt.FlagNestedFolders},
+			features: featuremgmt.WithFeatures(featuremgmt.FlagNestedFolders, featuremgmt.FlagPermissionsFilterRemoveSubquery),
 			expectedParams: []any{
 				int64(1),
 				int64(1),
@@ -219,48 +294,39 @@ func TestBuilder_RBAC(t *testing.T) {
 	require.NoError(t, err)
 
 	for _, tc := range testsCases {
-		for _, features := range []*featuremgmt.FeatureManager{featuremgmt.WithFeatures(tc.features...), featuremgmt.WithFeatures(append(tc.features, featuremgmt.FlagPermissionsFilterRemoveSubquery)...)} {
+		t.Run(tc.desc, func(t *testing.T) {
-			m := features.GetEnabled(context.Background())
+			if len(tc.userPermissions) > 0 {
-			keys := make([]string, 0, len(m))
+				user.Permissions = map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tc.userPermissions)}
-			for k := range m {
-				keys = append(keys, k)
 			}
 
-			t.Run(tc.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) {
+			level := dashboards.PERMISSION_EDIT
-				if len(tc.userPermissions) > 0 {
-					user.Permissions = map[int64]map[string][]string{1: accesscontrol.GroupScopesByAction(tc.userPermissions)}
-				}
 
-				level := dashboards.PERMISSION_EDIT
+			builder := &searchstore.Builder{
+				Filters: []any{
+					searchstore.OrgFilter{OrgId: user.OrgID},
+					searchstore.TitleSorter{},
+					permissions.NewAccessControlDashboardPermissionFilter(
+						user,
+						level,
+						"",
+						tc.features,
+						recursiveQueriesAreSupported,
+					),
+				},
+				Dialect:  store.GetDialect(),
+				Features: tc.features,
+			}
 
-				builder := &searchstore.Builder{
+			res := []dashboards.DashboardSearchProjection{}
-					Filters: []any{
+			err := store.WithDbSession(context.Background(), func(sess *db.Session) error {
-						searchstore.OrgFilter{OrgId: user.OrgID},
+				sql, params := builder.ToSQL(limit, page)
-						searchstore.TitleSorter{},
+				assert.Equal(t, tc.expectedParams, params)
-						permissions.NewAccessControlDashboardPermissionFilter(
+				return sess.SQL(sql, params...).Find(&res)
-							user,
-							level,
-							"",
-							features,
-							recursiveQueriesAreSupported,
-						),
-					},
-					Dialect:  store.GetDialect(),
-					Features: features,
-				}
-
-				res := []dashboards.DashboardSearchProjection{}
-				err := store.WithDbSession(context.Background(), func(sess *db.Session) error {
-					sql, params := builder.ToSQL(limit, page)
-					// TODO: replace with a proper test
-					assert.Equal(t, tc.expectedParams, params)
-					return sess.SQL(sql, params...).Find(&res)
-				})
-				require.NoError(t, err)
-
-				assert.Len(t, res, 0)
 			})
-		}
+			require.NoError(t, err)
+
+			assert.Len(t, res, 0)
+		})
 	}
 }