Support Spanner's UNION syntax, which needs to be UNION DISTINCT or UNION ALL. (#101768)

* Support Spanner's UNION syntax, which needs to be UNION DISTINCT or UNION ALL.
This commit is contained in:
Peter Štibraný 2025-03-10 12:33:52 +01:00 committed by GitHub
parent 607d39b573
commit fd6a4908f1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 46 additions and 23 deletions

View File

@ -72,7 +72,7 @@ func (sb *SQLBuilder) WriteDashboardPermissionFilter(user identity.Requester, pe
leftJoin string
)
filterRBAC := permissions.NewAccessControlDashboardPermissionFilter(user, permission, queryType, sb.features, sb.recursiveQueriesAreSupported)
filterRBAC := permissions.NewAccessControlDashboardPermissionFilter(user, permission, queryType, sb.features, sb.recursiveQueriesAreSupported, sb.dialect)
leftJoin = filterRBAC.LeftJoin()
sql, params = filterRBAC.Where()
recQry, recQryParams = filterRBAC.With()

View File

@ -6,9 +6,10 @@ import (
"strconv"
"strings"
"go.opentelemetry.io/otel"
"github.com/grafana/grafana/pkg/infra/db"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"go.opentelemetry.io/otel"
)
var tracer = otel.Tracer("github.com/grafana/grafana/pkg/services/accesscontrol/database")
@ -58,7 +59,7 @@ func (s *AccessControlStore) GetUserPermissions(ctx context.Context, query acces
return nil
}
filter, params := accesscontrol.UserRolesFilter(query.OrgID, query.UserID, query.TeamIDs, query.Roles)
filter, params := accesscontrol.UserRolesFilter(query.OrgID, query.UserID, query.TeamIDs, query.Roles, s.sql.GetDialect())
q := `
SELECT

View File

@ -6,6 +6,7 @@ import (
"strings"
"github.com/grafana/grafana/pkg/apimachinery/identity"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
)
var sqlIDAcceptList = map[string]struct{}{
@ -127,7 +128,7 @@ func SetAcceptListForTest(list map[string]struct{}) func() {
}
}
func UserRolesFilter(orgID, userID int64, teamIDs []int64, roles []string) (string, []any) {
func UserRolesFilter(orgID, userID int64, teamIDs []int64, roles []string, dialect migrator.Dialect) (string, []any) {
var params []any
builder := strings.Builder{}
@ -145,7 +146,7 @@ func UserRolesFilter(orgID, userID int64, teamIDs []int64, roles []string) (stri
if len(teamIDs) > 0 {
if builder.Len() > 0 {
builder.WriteString("UNION")
builder.WriteString(dialect.UnionDistinct())
}
builder.WriteString(`
SELECT tr.role_id FROM team_role as tr
@ -160,7 +161,7 @@ func UserRolesFilter(orgID, userID int64, teamIDs []int64, roles []string) (stri
if len(roles) != 0 {
if builder.Len() > 0 {
builder.WriteString("UNION")
builder.WriteString(dialect.UnionDistinct())
}
builder.WriteString(`

View File

@ -121,7 +121,7 @@ func (authz *AuthService) dashboardsWithVisibleAnnotations(ctx context.Context,
}
filters := []any{
permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, dashboardaccess.PERMISSION_VIEW, filterType, authz.features, recursiveQueriesSupported),
permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, dashboardaccess.PERMISSION_VIEW, filterType, authz.features, recursiveQueriesSupported, authz.db.GetDialect()),
searchstore.OrgFilter{OrgId: query.OrgID},
}

View File

@ -1002,7 +1002,7 @@ func (d *dashboardStore) FindDashboards(ctx context.Context, query *dashboards.F
}
if !query.SkipAccessControlFilter {
filters = append(filters, permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, query.Permission, query.Type, d.features, recursiveQueriesAreSupported))
filters = append(filters, permissions.NewAccessControlDashboardPermissionFilter(query.SignedInUser, query.Permission, query.Type, d.features, recursiveQueriesAreSupported, d.store.GetDialect()))
}
filters = append(filters, searchstore.DeletedFilter{Deleted: query.IsDeleted})

View File

@ -455,7 +455,9 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI
writeSearchStringSQL(query, l.SQLStore, &builder)
writeExcludeSQL(query, &builder)
writeTypeFilterSQL(typeFilter, &builder)
builder.Write(" UNION ")
builder.Write(" ")
builder.Write(l.SQLStore.GetDialect().UnionDistinct())
builder.Write(" ")
}
builder.Write(selectLibraryElementDTOWithMeta)
builder.Write(", le.folder_uid as folder_uid ")
@ -544,7 +546,9 @@ func (l *LibraryElementService) getAllLibraryElements(c context.Context, signedI
writeSearchStringSQL(query, l.SQLStore, &countBuilder)
writeExcludeSQL(query, &countBuilder)
writeTypeFilterSQL(typeFilter, &countBuilder)
countBuilder.Write(" UNION ")
countBuilder.Write(" ")
countBuilder.Write(l.SQLStore.GetDialect().UnionDistinct())
countBuilder.Write(" ")
}
countBuilder.Write(selectLibraryElementDTOWithMeta)
countBuilder.Write(getFromLibraryElementDTOWithMeta(l.SQLStore.GetDialect()))

View File

@ -32,6 +32,8 @@ type Dialect interface {
BooleanStr(bool) string
DateTimeFunc(string) string
BatchSize() int
UnionDistinct() string // this is the default UNION type
UnionAll() string
OrderBy(order string) string
@ -467,3 +469,11 @@ func (b *BaseDialect) Update(ctx context.Context, tx *session.SessionTx, tableNa
func (b *BaseDialect) Concat(strs ...string) string {
return fmt.Sprintf("CONCAT(%s)", strings.Join(strs, ", "))
}
func (b *BaseDialect) UnionDistinct() string {
return "UNION"
}
func (b *BaseDialect) UnionAll() string {
return "UNION ALL"
}

View File

@ -325,3 +325,7 @@ func confToClientOptions(connectorConfig spannerdriver.ConnectorConfig) []option
}
return opts
}
func (s *SpannerDialect) UnionDistinct() string {
return "UNION DISTINCT"
}

View File

@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
"github.com/grafana/grafana/pkg/services/sqlstore/searchstore"
)
@ -44,14 +45,14 @@ type PermissionsFilter interface {
With() (string, []any)
Where() (string, []any)
buildClauses()
buildClauses(dialect migrator.Dialect)
nestedFoldersSelectors(permSelector string, permSelectorArgs []any, leftTable string, Col string, rightTableCol string, orgID int64) (string, []any)
}
// NewAccessControlDashboardPermissionFilter creates a new AccessControlDashboardPermissionFilter that is configured with specific actions calculated based on the dashboardaccess.PermissionType and query type
// The filter is configured to use the new permissions filter (without subqueries) if the feature flag is enabled
// The filter is configured to use the old permissions filter (with subqueries) if the feature flag is disabled
func NewAccessControlDashboardPermissionFilter(user identity.Requester, permissionLevel dashboardaccess.PermissionType, queryType string, features featuremgmt.FeatureToggles, recursiveQueriesAreSupported bool) PermissionsFilter {
func NewAccessControlDashboardPermissionFilter(user identity.Requester, permissionLevel dashboardaccess.PermissionType, queryType string, features featuremgmt.FeatureToggles, recursiveQueriesAreSupported bool, dialect migrator.Dialect) PermissionsFilter {
needEdit := permissionLevel > dashboardaccess.PERMISSION_VIEW
var folderAction string
@ -129,7 +130,7 @@ func NewAccessControlDashboardPermissionFilter(user identity.Requester, permissi
features: features, recursiveQueriesAreSupported: recursiveQueriesAreSupported,
}
}
f.buildClauses()
f.buildClauses(dialect)
return f
}
@ -157,7 +158,7 @@ func (f *accessControlDashboardPermissionFilter) hasRequiredActions() bool {
return false
}
func (f *accessControlDashboardPermissionFilter) buildClauses() {
func (f *accessControlDashboardPermissionFilter) buildClauses(dialect migrator.Dialect) {
if f.user == nil || f.user.IsNil() || !f.hasRequiredActions() {
f.where = clause{string: "(1 = 0)"}
return
@ -171,7 +172,7 @@ func (f *accessControlDashboardPermissionFilter) buildClauses() {
}
orgID := f.user.GetOrgID()
filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user), dialect)
rolesFilter := " AND role_id IN(SELECT id FROM role " + filter + ") "
var args []any
builder := strings.Builder{}

View File

@ -10,6 +10,7 @@ import (
"github.com/grafana/grafana/pkg/services/featuremgmt"
"github.com/grafana/grafana/pkg/services/folder"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/sqlstore/migrator"
)
type accessControlDashboardPermissionFilterNoFolderSubquery struct {
@ -25,7 +26,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) LeftJoin() stri
return " dashboard AS folder ON dashboard.org_id = folder.org_id AND dashboard.folder_id = folder.id"
}
func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses() {
func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses(dialect migrator.Dialect) {
if f.user == nil || f.user.IsNil() || len(f.user.GetPermissions()) == 0 {
f.where = clause{string: "(1 = 0)"}
return
@ -39,7 +40,7 @@ func (f *accessControlDashboardPermissionFilterNoFolderSubquery) buildClauses()
}
orgID := f.user.GetOrgID()
filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user))
filter, params := accesscontrol.UserRolesFilter(orgID, userID, f.user.GetTeams(), accesscontrol.GetOrgRoles(f.user), dialect)
rolesFilter := " AND role_id IN(SELECT id FROM role " + filter + ") "
var args []any
builder := strings.Builder{}

View File

@ -183,7 +183,7 @@ func TestIntegration_DashboardPermissionFilter(t *testing.T) {
keys = append(keys, k)
}
t.Run(tt.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) {
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported, store.GetDialect())
var result int
err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
@ -355,7 +355,7 @@ func TestIntegration_DashboardPermissionFilter_WithSelfContainedPermissions(t *t
keys = append(keys, k)
}
t.Run(tt.desc+" with features "+strings.Join(keys, ","), func(t *testing.T) {
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tt.permission, tt.queryType, features, recursiveQueriesAreSupported, store.GetDialect())
var result int
err = store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
@ -470,7 +470,7 @@ func TestIntegration_DashboardNestedPermissionFilter(t *testing.T) {
db := setupNestedTest(t, usr, tc.permissions, orgID, features)
recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported()
require.NoError(t, err)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported, db.GetDialect())
var result []string
err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
@ -588,7 +588,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithSelfContainedPermission
db := setupNestedTest(t, helperUser, []accesscontrol.Permission{}, orgID, features)
recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported()
require.NoError(t, err)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported, db.GetDialect())
var result []string
err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()
@ -707,7 +707,7 @@ func TestIntegration_DashboardNestedPermissionFilter_WithActionSets(t *testing.T
db := setupNestedTest(t, usr, tc.signedInUserPermissions, orgID, features)
recursiveQueriesAreSupported, err := db.RecursiveQueriesAreSupported()
require.NoError(t, err)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported)
filter := permissions.NewAccessControlDashboardPermissionFilter(usr, tc.permission, tc.queryType, features, recursiveQueriesAreSupported, db.GetDialect())
var result []string
err = db.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()

View File

@ -56,7 +56,7 @@ func benchmarkDashboardPermissionFilter(b *testing.B, numUsers, numDashboards, n
b.ResetTimer()
for i := 0; i < b.N; i++ {
filter := permissions.NewAccessControlDashboardPermissionFilter(&usr, dashboardaccess.PERMISSION_VIEW, "", features, recursiveQueriesAreSupported)
filter := permissions.NewAccessControlDashboardPermissionFilter(&usr, dashboardaccess.PERMISSION_VIEW, "", features, recursiveQueriesAreSupported, store.GetDialect())
var result int
err := store.WithDbSession(context.Background(), func(sess *sqlstore.DBSession) error {
q, params := filter.Where()

View File

@ -334,6 +334,7 @@ func TestBuilder_RBAC(t *testing.T) {
"",
tc.features,
recursiveQueriesAreSupported,
store.GetDialect(),
),
},
Dialect: store.GetDialect(),