LookupResources2 follow-ups

internal/datastore/common/sql.go

@@ -44,8 +44,6 @@ var (
 	// ID.
 	SubObjectIDKey = attribute.Key("authzed.com/spicedb/sql/subObjectId")
 
-	limitKey = attribute.Key("authzed.com/spicedb/sql/limit")
-
 	tracer = otel.Tracer("spicedb/internal/datastore/common")
 )
 
@@ -104,15 +102,16 @@ type SchemaQueryFilterer struct {
 	schema                SchemaInformation
 	queryBuilder          sq.SelectBuilder
 	filteringColumnCounts map[string]int
-	tracerAttributes      []attribute.KeyValue
+	filterMaximumIDCount  uint16
 }
 
 // NewSchemaQueryFilterer creates a new SchemaQueryFilterer object.
-func NewSchemaQueryFilterer(schema SchemaInformation, initialQuery sq.SelectBuilder) SchemaQueryFilterer {
+func NewSchemaQueryFilterer(schema SchemaInformation, initialQuery sq.SelectBuilder, filterMaximumIDCount uint16) SchemaQueryFilterer {
 	return SchemaQueryFilterer{
 		schema:                schema,
 		queryBuilder:          initialQuery,
 		filteringColumnCounts: map[string]int{},
+		filterMaximumIDCount:  filterMaximumIDCount,
 	}
 }
 
@@ -249,7 +248,6 @@ func (sqf SchemaQueryFilterer) After(cursor *core.RelationTuple, order options.S
 // specified type.
 func (sqf SchemaQueryFilterer) FilterToResourceType(resourceType string) SchemaQueryFilterer {
 	sqf.queryBuilder = sqf.queryBuilder.Where(sq.Eq{sqf.schema.colNamespace: resourceType})
-	sqf.tracerAttributes = append(sqf.tracerAttributes, ObjNamespaceNameKey.String(resourceType))
 	sqf.recordColumnValue(sqf.schema.colNamespace)
 	return sqf
 }
@@ -267,7 +265,6 @@ func (sqf SchemaQueryFilterer) recordColumnValue(colName string) {
 // specified ID.
 func (sqf SchemaQueryFilterer) FilterToResourceID(objectID string) SchemaQueryFilterer {
 	sqf.queryBuilder = sqf.queryBuilder.Where(sq.Eq{sqf.schema.colObjectID: objectID})
-	sqf.tracerAttributes = append(sqf.tracerAttributes, ObjIDKey.String(objectID))
 	sqf.recordColumnValue(sqf.schema.colObjectID)
 	return sqf
 }
@@ -294,7 +291,6 @@ func (sqf SchemaQueryFilterer) FilterWithResourceIDPrefix(prefix string) (Schema
 	prefix = strings.ReplaceAll(prefix, "_", `\_`)
 
 	sqf.queryBuilder = sqf.queryBuilder.Where(sq.Like{sqf.schema.colObjectID: prefix + "%"})
-	sqf.tracerAttributes = append(sqf.tracerAttributes, ObjIDKey.String(prefix+"*"))
 
 	// NOTE: we do *not* record the use of the resource ID column here, because it is not used
 	// statically and thus is necessary for sorting operations.
@@ -312,38 +308,38 @@ func (sqf SchemaQueryFilterer) MustFilterWithResourceIDPrefix(prefix string) Sch
 // FilterToResourceIDs returns a new SchemaQueryFilterer that is limited to resources with any of the
 // specified IDs.
 func (sqf SchemaQueryFilterer) FilterToResourceIDs(resourceIds []string) (SchemaQueryFilterer, error) {
-	if len(resourceIds) > int(datastore.FilterMaximumIDCount) {
-		return sqf, spiceerrors.MustBugf("cannot have more than %d resources IDs in a single filter", datastore.FilterMaximumIDCount)
+	if len(resourceIds) > int(sqf.filterMaximumIDCount) {
+		return sqf, spiceerrors.MustBugf("cannot have more than %d resources IDs in a single filter", sqf.filterMaximumIDCount)
 	}
 
-	inClause := sqf.schema.colObjectID + " IN ("
+	var builder strings.Builder
+	builder.WriteString(sqf.schema.colObjectID)
+	builder.WriteString(" IN (")
 	args := make([]any, 0, len(resourceIds))
 
-	for index, resourceID := range resourceIds {
+	for _, resourceID := range resourceIds {
 		if len(resourceID) == 0 {
 			return sqf, spiceerrors.MustBugf("got empty resource ID")
 		}
 
-		if index > 0 {
-			inClause += ", "
-		}
-
-		inClause += "?"
-
 		args = append(args, resourceID)
-		sqf.tracerAttributes = append(sqf.tracerAttributes, ObjIDKey.String(resourceID))
 		sqf.recordColumnValue(sqf.schema.colObjectID)
 	}
 
-	sqf.queryBuilder = sqf.queryBuilder.Where(inClause+")", args...)
+	builder.WriteString("?")
+	if len(resourceIds) > 1 {
+		builder.WriteString(strings.Repeat(",?", len(resourceIds)-1))
+	}
+	builder.WriteString(")")
+
+	sqf.queryBuilder = sqf.queryBuilder.Where(builder.String(), args...)
 	return sqf, nil
 }
 
 // FilterToRelation returns a new SchemaQueryFilterer that is limited to resources with the
 // specified relation.
 func (sqf SchemaQueryFilterer) FilterToRelation(relation string) SchemaQueryFilterer {
 	sqf.queryBuilder = sqf.queryBuilder.Where(sq.Eq{sqf.schema.colRelation: relation})
-	sqf.tracerAttributes = append(sqf.tracerAttributes, ObjRelationNameKey.String(relation))
 	sqf.recordColumnValue(sqf.schema.colRelation)
 	return sqf
 }
@@ -424,35 +420,35 @@ func (sqf SchemaQueryFilterer) FilterWithSubjectsSelectors(selectors ...datastor
 
 		if len(selector.OptionalSubjectType) > 0 {
 			selectorClause = append(selectorClause, sq.Eq{sqf.schema.colUsersetNamespace: selector.OptionalSubjectType})
-			sqf.tracerAttributes = append(sqf.tracerAttributes, SubNamespaceNameKey.String(selector.OptionalSubjectType))
 			sqf.recordColumnValue(sqf.schema.colUsersetNamespace)
 		}
 
 		if len(selector.OptionalSubjectIds) > 0 {
-			if len(selector.OptionalSubjectIds) > int(datastore.FilterMaximumIDCount) {
-				return sqf, spiceerrors.MustBugf("cannot have more than %d subject IDs in a single filter", datastore.FilterMaximumIDCount)
+			if len(selector.OptionalSubjectIds) > int(sqf.filterMaximumIDCount) {
+				return sqf, spiceerrors.MustBugf("cannot have more than %d subject IDs in a single filter", sqf.filterMaximumIDCount)
 			}
 
-			inClause := sqf.schema.colUsersetObjectID + " IN ("
+			var builder strings.Builder
+			builder.WriteString(sqf.schema.colUsersetObjectID)
+			builder.WriteString(" IN (")
 			args := make([]any, 0, len(selector.OptionalSubjectIds))
 
-			for index, subjectID := range selector.OptionalSubjectIds {
+			for _, subjectID := range selector.OptionalSubjectIds {
 				if len(subjectID) == 0 {
 					return sqf, spiceerrors.MustBugf("got empty subject ID")
 				}
 
-				if index > 0 {
-					inClause += ", "
-				}
-
-				inClause += "?"
-
 				args = append(args, subjectID)
-				sqf.tracerAttributes = append(sqf.tracerAttributes, SubObjectIDKey.String(subjectID))
 				sqf.recordColumnValue(sqf.schema.colUsersetObjectID)
 			}
 
-			selectorClause = append(selectorClause, sq.Expr(inClause+")", args...))
+			builder.WriteString("?")
+			if len(selector.OptionalSubjectIds) > 1 {
+				builder.WriteString(strings.Repeat(",?", len(selector.OptionalSubjectIds)-1))
+			}
+
+			builder.WriteString(")")
+			selectorClause = append(selectorClause, sq.Expr(builder.String(), args...))
 		}
 
 		if !selector.RelationFilter.IsEmpty() {
@@ -471,15 +467,13 @@ func (sqf SchemaQueryFilterer) FilterWithSubjectsSelectors(selectors ...datastor
 
 				if len(relations) == 1 {
 					relName := relations[0]
-					sqf.tracerAttributes = append(sqf.tracerAttributes, SubRelationNameKey.String(relName))
 					selectorClause = append(selectorClause, sq.Eq{sqf.schema.colUsersetRelation: relName})
 					sqf.recordColumnValue(sqf.schema.colUsersetRelation)
 				} else {
 					orClause := sq.Or{}
 					for _, relationName := range relations {
 						dsRelationName := stringz.DefaultEmpty(relationName, datastore.Ellipsis)
 						orClause = append(orClause, sq.Eq{sqf.schema.colUsersetRelation: dsRelationName})
-						sqf.tracerAttributes = append(sqf.tracerAttributes, SubRelationNameKey.String(dsRelationName))
 						sqf.recordColumnValue(sqf.schema.colUsersetRelation)
 					}
 
@@ -499,20 +493,17 @@ func (sqf SchemaQueryFilterer) FilterWithSubjectsSelectors(selectors ...datastor
 // subjects that match the specified filter.
 func (sqf SchemaQueryFilterer) FilterToSubjectFilter(filter *v1.SubjectFilter) SchemaQueryFilterer {
 	sqf.queryBuilder = sqf.queryBuilder.Where(sq.Eq{sqf.schema.colUsersetNamespace: filter.SubjectType})
-	sqf.tracerAttributes = append(sqf.tracerAttributes, SubNamespaceNameKey.String(filter.SubjectType))
 	sqf.recordColumnValue(sqf.schema.colUsersetNamespace)
 
 	if filter.OptionalSubjectId != "" {
 		sqf.queryBuilder = sqf.queryBuilder.Where(sq.Eq{sqf.schema.colUsersetObjectID: filter.OptionalSubjectId})
-		sqf.tracerAttributes = append(sqf.tracerAttributes, SubObjectIDKey.String(filter.OptionalSubjectId))
 		sqf.recordColumnValue(sqf.schema.colUsersetObjectID)
 	}
 
 	if filter.OptionalRelation != nil {
 		dsRelationName := stringz.DefaultEmpty(filter.OptionalRelation.Relation, datastore.Ellipsis)
 
 		sqf.queryBuilder = sqf.queryBuilder.Where(sq.Eq{sqf.schema.colUsersetRelation: dsRelationName})
-		sqf.tracerAttributes = append(sqf.tracerAttributes, SubRelationNameKey.String(dsRelationName))
 		sqf.recordColumnValue(sqf.schema.colUsersetRelation)
 	}
 
@@ -521,15 +512,13 @@ func (sqf SchemaQueryFilterer) FilterToSubjectFilter(filter *v1.SubjectFilter) S
 
 func (sqf SchemaQueryFilterer) FilterWithCaveatName(caveatName string) SchemaQueryFilterer {
 	sqf.queryBuilder = sqf.queryBuilder.Where(sq.Eq{sqf.schema.colCaveatName: caveatName})
-	sqf.tracerAttributes = append(sqf.tracerAttributes, CaveatNameKey.String(caveatName))
 	sqf.recordColumnValue(sqf.schema.colCaveatName)
 	return sqf
 }
 
 // Limit returns a new SchemaQueryFilterer which is limited to the specified number of results.
 func (sqf SchemaQueryFilterer) limit(limit uint64) SchemaQueryFilterer {
 	sqf.queryBuilder = sqf.queryBuilder.Limit(limit)
-	sqf.tracerAttributes = append(sqf.tracerAttributes, limitKey.Int64(int64(limit)))
 	return sqf
 }
 

internal/datastore/common/sql_test.go

@@ -3,6 +3,8 @@ package common
 import (
 	"testing"
 
+	"github.com/google/uuid"
+
 	"github.com/authzed/spicedb/pkg/datastore/options"
 
 	"github.com/authzed/spicedb/pkg/tuple"
@@ -58,7 +60,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 			func(filterer SchemaQueryFilterer) SchemaQueryFilterer {
 				return filterer.MustFilterToResourceIDs([]string{"someresourceid", "anotherresourceid"})
 			},
-			"SELECT * WHERE object_id IN (?, ?)",
+			"SELECT * WHERE object_id IN (?,?)",
 			[]any{"someresourceid", "anotherresourceid"},
 			map[string]int{
 				"object_id": 2,
@@ -138,7 +140,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 					OptionalResourceIds:  []string{"someid", "anotherid"},
 				})
 			},
-			"SELECT * WHERE ns = ? AND object_id IN (?, ?)",
+			"SELECT * WHERE ns = ? AND object_id IN (?,?)",
 			[]any{"sometype", "someid", "anotherid"},
 			map[string]int{
 				"ns":        1,
@@ -218,7 +220,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 					OptionalSubjectIds:  []string{"somesubjectid", "anothersubjectid"},
 				})
 			},
-			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?, ?)))",
+			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?,?)))",
 			[]any{"somesubjectype", "somesubjectid", "anothersubjectid"},
 			map[string]int{
 				"subject_ns":        1,
@@ -294,7 +296,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 					RelationFilter:      datastore.SubjectRelationFilter{}.WithNonEllipsisRelation("somesubrel").WithEllipsisRelation(),
 				})
 			},
-			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?, ?) AND (subject_relation = ? OR subject_relation = ?)))",
+			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?,?) AND (subject_relation = ? OR subject_relation = ?)))",
 			[]any{"somesubjectype", "somesubjectid", "anothersubjectid", "...", "somesubrel"},
 			map[string]int{
 				"subject_ns":        1,
@@ -322,7 +324,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 					},
 				)
 			},
-			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?, ?) AND (subject_relation = ? OR subject_relation = ?)) OR (subject_ns = ? AND subject_object_id IN (?, ?) AND (subject_relation = ? OR subject_relation = ?)) OR (subject_ns = ? AND subject_relation <> ?))",
+			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?,?) AND (subject_relation = ? OR subject_relation = ?)) OR (subject_ns = ? AND subject_object_id IN (?,?) AND (subject_relation = ? OR subject_relation = ?)) OR (subject_ns = ? AND subject_relation <> ?))",
 			[]any{"somesubjectype", "a", "b", "...", "somesubrel", "anothersubjecttype", "b", "c", "...", "anotherrel", "thirdsubjectype", "..."},
 			map[string]int{
 				"subject_ns":        3,
@@ -438,7 +440,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 					},
 				)
 			},
-			"SELECT * WHERE ns = ? AND relation = ? AND object_id IN (?, ?) AND ((subject_ns = ? AND subject_object_id IN (?, ?) AND (subject_relation = ? OR subject_relation = ?)))",
+			"SELECT * WHERE ns = ? AND relation = ? AND object_id IN (?,?) AND ((subject_ns = ? AND subject_object_id IN (?,?) AND (subject_relation = ? OR subject_relation = ?)))",
 			[]any{"someresourcetype", "somerelation", "someid", "anotherid", "somesubjectype", "somesubjectid", "anothersubjectid", "...", "somesubrel"},
 			map[string]int{
 				"ns":                1,
@@ -536,7 +538,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 					},
 				).After(tuple.MustParse("someresourcetype:foo#viewer@user:bar"), options.ByResource)
 			},
-			"SELECT * WHERE ns = ? AND object_id IN (?, ?) AND (object_id,relation,subject_ns,subject_object_id,subject_relation) > (?,?,?,?,?)",
+			"SELECT * WHERE ns = ? AND object_id IN (?,?) AND (object_id,relation,subject_ns,subject_object_id,subject_relation) > (?,?,?,?,?)",
 			[]any{"someresourcetype", "one", "two", "foo", "viewer", "user", "bar", "..."},
 			map[string]int{
 				"ns":        1,
@@ -647,7 +649,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 					OptionalSubjectIds:  []string{"foo", "bar"},
 				}).After(tuple.MustParse("someresourcetype:someresource#viewer@user:next"), options.BySubject)
 			},
-			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?, ?))) AND (subject_object_id,ns,object_id,relation,subject_relation) > (?,?,?,?,?)",
+			"SELECT * WHERE ((subject_ns = ? AND subject_object_id IN (?,?))) AND (subject_object_id,ns,object_id,relation,subject_relation) > (?,?,?,?,?)",
 			[]any{"somesubjectype", "foo", "bar", "next", "someresourcetype", "someresource", "viewer", "..."},
 			map[string]int{"subject_ns": 1, "subject_object_id": 2},
 		},
@@ -667,7 +669,7 @@ func TestSchemaQueryFilterer(t *testing.T) {
 				"caveat",
 				TupleComparison,
 			)
-			filterer := NewSchemaQueryFilterer(schema, base)
+			filterer := NewSchemaQueryFilterer(schema, base, 100)
 
 			ran := test.run(filterer)
 			require.Equal(t, test.expectedColumnCounts, ran.filteringColumnCounts)
@@ -679,3 +681,27 @@ func TestSchemaQueryFilterer(t *testing.T) {
 		})
 	}
 }
+
+func BenchmarkSchemaFilterer(b *testing.B) {
+	si := NewSchemaInformation(
+		"namespace",
+		"object_id",
+		"object_relation",
+		"resource_type",
+		"resource_id",
+		"resource_relation",
+		"caveat_name",
+		TupleComparison,
+	)
+	sqf := NewSchemaQueryFilterer(si, sq.Select("*"), 100)
+	var names []string
+	for i := 0; i < 500; i++ {
+		names = append(names, uuid.NewString())
+	}
+
+	b.ReportAllocs()
+	b.ResetTimer()
+	for i := 0; i < b.N; i++ {
+		_, _ = sqf.FilterToResourceIDs(names)
+	}
+}

internal/datastore/crdb/crdb.go

@@ -188,6 +188,7 @@ func newCRDBDatastore(ctx context.Context, url string, options ...Option) (datas
 		beginChangefeedQuery:    changefeedQuery,
 		transactionNowQuery:     transactionNowQuery,
 		analyzeBeforeStatistics: config.analyzeBeforeStatistics,
+		filterMaximumIDCount:    config.filterMaximumIDCount,
 	}
 	ds.RemoteClockRevisions.SetNowFunc(ds.headRevisionInternal)
 
@@ -275,9 +276,10 @@ type crdbDatastore struct {
 
 	featureGroup singleflight.Group[string, *datastore.Features]
 
-	pruneGroup *errgroup.Group
-	ctx        context.Context
-	cancel     context.CancelFunc
+	pruneGroup           *errgroup.Group
+	ctx                  context.Context
+	cancel               context.CancelFunc
+	filterMaximumIDCount uint16
 }
 
 func (cds *crdbDatastore) SnapshotReader(rev datastore.Revision) datastore.Reader {
@@ -289,7 +291,7 @@ func (cds *crdbDatastore) SnapshotReader(rev datastore.Revision) datastore.Reade
 		return query.From(fromStr + " AS OF SYSTEM TIME " + rev.String())
 	}
 
-	return &crdbReader{cds.readPool, executor, noOverlapKeyer, nil, fromBuilder}
+	return &crdbReader{cds.readPool, executor, noOverlapKeyer, nil, fromBuilder, cds.filterMaximumIDCount}
 }
 
 func (cds *crdbDatastore) ReadWriteTx(
@@ -319,6 +321,7 @@ func (cds *crdbDatastore) ReadWriteTx(
 				func(query sq.SelectBuilder, fromStr string) sq.SelectBuilder {
 					return query.From(fromStr)
 				},
+				cds.filterMaximumIDCount,
 			},
 			tx,
 			0,

internal/datastore/crdb/options.go

@@ -22,8 +22,8 @@ type crdbOptions struct {
 	overlapKey                  string
 	enableConnectionBalancing   bool
 	analyzeBeforeStatistics     bool
-
-	enablePrometheusStats bool
+	filterMaximumIDCount        uint16
+	enablePrometheusStats       bool
 }
 
 const (
@@ -48,6 +48,7 @@ const (
 	defaultEnablePrometheusStats     = false
 	defaultEnableConnectionBalancing = true
 	defaultConnectRate               = 100 * time.Millisecond
+	defaultFilterMaximumIDCount      = 100
 )
 
 // Option provides the facility to configure how clients within the CRDB
@@ -68,6 +69,7 @@ func generateConfig(options []Option) (crdbOptions, error) {
 		enablePrometheusStats:       defaultEnablePrometheusStats,
 		enableConnectionBalancing:   defaultEnableConnectionBalancing,
 		connectRate:                 defaultConnectRate,
+		filterMaximumIDCount:        defaultFilterMaximumIDCount,
 	}
 
 	for _, option := range options {
@@ -306,3 +308,8 @@ func WithEnableConnectionBalancing(connectionBalancing bool) Option {
 func DebugAnalyzeBeforeStatistics() Option {
 	return func(po *crdbOptions) { po.analyzeBeforeStatistics = true }
 }
+
+// FilterMaximumIDCount is the maximum number of IDs that can be used to filter IDs in queries
+func FilterMaximumIDCount(filterMaximumIDCount uint16) Option {
+	return func(po *crdbOptions) { po.filterMaximumIDCount = filterMaximumIDCount }
+}