Skip to content

Commit 45bdbf6

Browse files
josephschorrjosephschorr
committed
feat: Have read relationships use dynamic index forcing
This should ensure that, for recognized query shapes, the best index and sort order is selected for ReadRels
1 parent 9029960 commit 45bdbf6

File tree

12 files changed

+330
-33
lines changed

12 files changed

+330
-33
lines changed

‎internal/datastore/common/index.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
package common
22

3-
import "github.com/authzed/spicedb/pkg/datastore/queryshape"
3+
import (
4+
"github.com/authzed/spicedb/pkg/datastore/options"
5+
"github.com/authzed/spicedb/pkg/datastore/queryshape"
6+
)
47

58
// IndexDefinition is a definition of an index for a datastore.
69
type IndexDefinition struct {
@@ -15,6 +18,9 @@ type IndexDefinition struct {
1518

1619
// IsDeprecated is true if this index is deprecated and should not be used.
1720
IsDeprecated bool
21+
22+
// PreferredSortOrder is the preferred sort order for this index, if any.
23+
PreferredSortOrder options.SortOrder
1824
}
1925

2026
// matchesShape returns true if the index matches the given shape.

‎internal/datastore/common/sql.go‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,9 @@ type IndexingHint interface {
119119

120120
// FromSQLSuffix returns the suffix to be used for the indexing hint, if any.
121121
FromSQLSuffix() (string, error)
122+
123+
// SortOrder returns the preferred sort order for this index, if any.
124+
SortOrder() options.SortOrder
122125
}
123126

124127
// NewSchemaQueryFiltererForRelationshipsSelect creates a new SchemaQueryFilterer object for selecting
@@ -702,15 +705,22 @@ func (exc QueryRelationshipsExecutor) ExecuteQuery(
702705
queryOpts := options.NewQueryOptionsWithOptions(opts...)
703706

704707
// Add sort order.
705-
query = query.TupleOrder(queryOpts.Sort)
708+
sort := queryOpts.Sort
709+
if sort == options.ChooseEfficient && query.indexingHint != nil {
710+
hintSort := query.indexingHint.SortOrder()
711+
if hintSort != options.Unsorted {
712+
sort = hintSort
713+
}
714+
}
715+
query = query.TupleOrder(sort)
706716

707717
// Add cursor.
708718
if queryOpts.After != nil {
709-
if queryOpts.Sort == options.Unsorted {
719+
if sort == options.Unsorted {
710720
return nil, datastore.ErrCursorsWithoutSorting
711721
}
712722

713-
q, err := query.After(queryOpts.After, queryOpts.Sort)
723+
q, err := query.After(queryOpts.After, sort)
714724
if err != nil {
715725
return nil, err
716726
}

‎internal/datastore/common/sql_test.go‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,10 @@ func (f *fakeIndexingHint) SQLPrefix() (string, error) {
14711471
return "/*+ IndexHint */", nil
14721472
}
14731473

1474+
func (f *fakeIndexingHint) SortOrder() options.SortOrder {
1475+
return options.ByResource
1476+
}
1477+
14741478
var _ IndexingHint = &fakeIndexingHint{}
14751479

14761480
func TestIndexHint(t *testing.T) {
@@ -1519,6 +1523,134 @@ func TestIndexHint(t *testing.T) {
15191523
require.True(t, wasRun)
15201524
}
15211525

1526+
func TestIndexHintWithPreferredSort(t *testing.T) {
1527+
tests := []struct {
1528+
name string
1529+
hintSortOrder options.SortOrder
1530+
querySortOption options.QueryOptionsOption
1531+
expectedOrderBy string
1532+
expectedNoOrderBy bool
1533+
}{
1534+
{
1535+
name: "ChooseEfficient with hint ByResource uses hint's sort order",
1536+
hintSortOrder: options.ByResource,
1537+
querySortOption: options.WithSort(options.ChooseEfficient),
1538+
expectedOrderBy: " ORDER BY ns, object_id, relation, subject_ns, subject_object_id, subject_relation",
1539+
},
1540+
{
1541+
name: "ChooseEfficient with hint BySubject uses hint's sort order",
1542+
hintSortOrder: options.BySubject,
1543+
querySortOption: options.WithSort(options.ChooseEfficient),
1544+
expectedOrderBy: " ORDER BY subject_ns, subject_object_id, subject_relation, ns, object_id, relation",
1545+
},
1546+
{
1547+
name: "ChooseEfficient with hint Unsorted does not add ordering",
1548+
hintSortOrder: options.Unsorted,
1549+
querySortOption: options.WithSort(options.ChooseEfficient),
1550+
expectedNoOrderBy: true,
1551+
},
1552+
{
1553+
name: "Explicit ByResource overrides hint's BySubject",
1554+
hintSortOrder: options.BySubject,
1555+
querySortOption: options.WithSort(options.ByResource),
1556+
expectedOrderBy: " ORDER BY ns, object_id, relation, subject_ns, subject_object_id, subject_relation",
1557+
},
1558+
{
1559+
name: "Explicit BySubject overrides hint's ByResource",
1560+
hintSortOrder: options.ByResource,
1561+
querySortOption: options.WithSort(options.BySubject),
1562+
expectedOrderBy: " ORDER BY subject_ns, subject_object_id, subject_relation, ns, object_id, relation",
1563+
},
1564+
{
1565+
name: "Explicit Unsorted overrides hint's ByResource",
1566+
hintSortOrder: options.ByResource,
1567+
querySortOption: options.WithSort(options.Unsorted),
1568+
expectedNoOrderBy: true,
1569+
},
1570+
{
1571+
name: "No sort option with hint ByResource keeps unsorted",
1572+
hintSortOrder: options.ByResource,
1573+
querySortOption: nil,
1574+
expectedNoOrderBy: true,
1575+
},
1576+
}
1577+
1578+
for _, test := range tests {
1579+
t.Run(test.name, func(t *testing.T) {
1580+
schema := NewSchemaInformationWithOptions(
1581+
WithRelationshipTableName("relationtuples"),
1582+
WithColNamespace("ns"),
1583+
WithColObjectID("object_id"),
1584+
WithColRelation("relation"),
1585+
WithColUsersetNamespace("subject_ns"),
1586+
WithColUsersetObjectID("subject_object_id"),
1587+
WithColUsersetRelation("subject_relation"),
1588+
WithColCaveatName("caveat"),
1589+
WithColCaveatContext("caveat_context"),
1590+
WithColExpiration("expiration"),
1591+
WithPlaceholderFormat(sq.Question),
1592+
WithPaginationFilterType(TupleComparison),
1593+
WithColumnOptimization(ColumnOptimizationOptionStaticValues),
1594+
WithNowFunction("NOW"),
1595+
WithExpirationDisabled(true),
1596+
)
1597+
1598+
hint := &fakeIndexingHintWithSort{sortOrder: test.hintSortOrder}
1599+
filterer := NewSchemaQueryFiltererForRelationshipsSelect(*schema, 100)
1600+
filterer = filterer.WithIndexingHint(hint)
1601+
filterer = filterer.FilterToResourceType("sometype")
1602+
1603+
var wasRun bool
1604+
fake := QueryRelationshipsExecutor{
1605+
Executor: func(ctx context.Context, builder RelationshipsQueryBuilder) (datastore.RelationshipIterator, error) {
1606+
sql, _, err := builder.SelectSQL()
1607+
require.NoError(t, err)
1608+
1609+
if test.expectedNoOrderBy {
1610+
require.NotContains(t, sql, "ORDER BY", "Expected no ORDER BY clause in SQL")
1611+
} else {
1612+
require.Contains(t, sql, test.expectedOrderBy, "Expected ORDER BY clause not found in SQL")
1613+
}
1614+
1615+
wasRun = true
1616+
return nil, nil
1617+
},
1618+
}
1619+
1620+
var opts []options.QueryOptionsOption
1621+
if test.querySortOption != nil {
1622+
opts = []options.QueryOptionsOption{test.querySortOption}
1623+
}
1624+
1625+
_, err := fake.ExecuteQuery(t.Context(), filterer, opts...)
1626+
require.NoError(t, err)
1627+
require.True(t, wasRun)
1628+
})
1629+
}
1630+
}
1631+
1632+
type fakeIndexingHintWithSort struct {
1633+
sortOrder options.SortOrder
1634+
}
1635+
1636+
func (f *fakeIndexingHintWithSort) FromSQLSuffix() (string, error) {
1637+
return "", nil
1638+
}
1639+
1640+
func (f *fakeIndexingHintWithSort) FromTable(existingTableName string) (string, error) {
1641+
return existingTableName, nil
1642+
}
1643+
1644+
func (f *fakeIndexingHintWithSort) SQLPrefix() (string, error) {
1645+
return "", nil
1646+
}
1647+
1648+
func (f *fakeIndexingHintWithSort) SortOrder() options.SortOrder {
1649+
return f.sortOrder
1650+
}
1651+
1652+
var _ IndexingHint = &fakeIndexingHintWithSort{}
1653+
15221654
func TestBuildLikeCla(t *testing.T) {
15231655
tcs := []struct {
15241656
prefix string

‎internal/datastore/crdb/reader.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,11 @@ func (cr *crdbReader) QueryRelationships(
248248
}
249249

250250
builtOpts := options.NewQueryOptionsWithOptions(opts...)
251-
indexingHint := schema.IndexingHintForQueryShape(cr.schema, builtOpts.QueryShape)
251+
indexingHint, err := schema.IndexingHintForQueryShape(cr.schema, builtOpts.QueryShape, &filter)
252+
if err != nil {
253+
return nil, err
254+
}
255+
252256
qBuilder = qBuilder.WithIndexingHint(indexingHint)
253257

254258
if spiceerrors.DebugAssertionsEnabled {
@@ -277,7 +281,11 @@ func (cr *crdbReader) ReverseQueryRelationships(
277281
FilterToRelation(queryOpts.ResRelation.Relation)
278282
}
279283

280-
indexingHint := schema.IndexingHintForQueryShape(cr.schema, queryOpts.QueryShapeForReverse)
284+
indexingHint, err := schema.IndexingHintForQueryShape(cr.schema, queryOpts.QueryShapeForReverse, nil)
285+
if err != nil {
286+
return nil, err
287+
}
288+
281289
qBuilder = qBuilder.WithIndexingHint(indexingHint)
282290

283291
eopts := []options.QueryOptionsOption{

‎internal/datastore/crdb/schema/forcedindex.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package schema
22

33
import (
44
"github.com/authzed/spicedb/internal/datastore/common"
5+
"github.com/authzed/spicedb/pkg/datastore/options"
56
"github.com/authzed/spicedb/pkg/spiceerrors"
67
)
78

@@ -28,4 +29,8 @@ func (f forcedIndex) SQLPrefix() (string, error) {
2829
return "", nil
2930
}
3031

32+
func (f forcedIndex) SortOrder() options.SortOrder {
33+
return f.index.PreferredSortOrder
34+
}
35+
3136
var _ common.IndexingHint = forcedIndex{}

‎internal/datastore/crdb/schema/indexes.go‎

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package schema
33
import (
44
"github.com/authzed/spicedb/internal/datastore/common"
55
"github.com/authzed/spicedb/pkg/datastore"
6+
"github.com/authzed/spicedb/pkg/datastore/options"
67
"github.com/authzed/spicedb/pkg/datastore/queryshape"
78
)
89

@@ -16,6 +17,7 @@ var IndexPrimaryKey = common.IndexDefinition{
1617
queryshape.FindResourceOfType,
1718
queryshape.AllSubjectsForResources,
1819
},
20+
PreferredSortOrder: options.ByResource,
1921
}
2022

2123
// IndexRelationshipBySubject is an index for looking up relationships by subject.
@@ -25,13 +27,15 @@ var IndexRelationshipBySubject = common.IndexDefinition{
2527
Shapes: []queryshape.Shape{
2628
queryshape.MatchingResourcesForSubject,
2729
},
30+
PreferredSortOrder: options.BySubject,
2831
}
2932

3033
// IndexRelationshipBySubjectRelation is an index for looking up relationships by subject type and relation.
3134
// Used by schema delta checking.
3235
var IndexRelationshipBySubjectRelation = common.IndexDefinition{
33-
Name: "ix_relation_tuple_by_subject_relation",
34-
ColumnsSQL: `relation_tuple (userset_namespace, userset_relation, namespace, relation)`,
36+
Name: "ix_relation_tuple_by_subject_relation",
37+
ColumnsSQL: `relation_tuple (userset_namespace, userset_relation, namespace, relation)`,
38+
PreferredSortOrder: options.BySubject,
3539
}
3640

3741
// IndexRelationshipWithIntegrity is an index for looking up relationships with integrity.
@@ -42,6 +46,7 @@ var IndexRelationshipWithIntegrity = common.IndexDefinition{
4246
queryshape.CheckPermissionSelectDirectSubjects,
4347
queryshape.CheckPermissionSelectIndirectSubjects,
4448
},
49+
PreferredSortOrder: options.ByResource,
4550
}
4651

4752
var crdbAllIndexes = []common.IndexDefinition{
@@ -65,39 +70,53 @@ var crdbWithIntegrityIndexes = []common.IndexDefinition{
6570
var NoIndexingHint common.IndexingHint = nil
6671

6772
// IndexingHintForQueryShape returns an indexing hint for the given query shape, if any.
68-
func IndexingHintForQueryShape(schema common.SchemaInformation, qs queryshape.Shape) common.IndexingHint {
73+
func IndexingHintForQueryShape(schema common.SchemaInformation, qs queryshape.Shape, optionalFilter *datastore.RelationshipsFilter) (common.IndexingHint, error) {
6974
if schema.IntegrityEnabled {
7075
// Don't force anything since we don't have the other indexes.
71-
return NoIndexingHint
76+
return NoIndexingHint, nil
7277
}
7378

7479
switch qs {
7580
case queryshape.CheckPermissionSelectDirectSubjects:
76-
return forcedIndex{IndexPrimaryKey}
81+
return forcedIndex{IndexPrimaryKey}, nil
7782

7883
case queryshape.CheckPermissionSelectIndirectSubjects:
79-
return forcedIndex{IndexPrimaryKey}
84+
return forcedIndex{IndexPrimaryKey}, nil
8085

8186
case queryshape.AllSubjectsForResources:
82-
return forcedIndex{IndexPrimaryKey}
87+
return forcedIndex{IndexPrimaryKey}, nil
8388

8489
case queryshape.MatchingResourcesForSubject:
85-
return forcedIndex{IndexRelationshipBySubject}
90+
return forcedIndex{IndexRelationshipBySubject}, nil
8691

8792
case queryshape.FindResourceOfType:
88-
return forcedIndex{IndexPrimaryKey}
93+
return forcedIndex{IndexPrimaryKey}, nil
8994

9095
case queryshape.FindResourceAndSubjectWithRelations:
91-
return forcedIndex{IndexRelationshipBySubjectRelation}
96+
return forcedIndex{IndexRelationshipBySubjectRelation}, nil
9297

9398
case queryshape.FindSubjectOfTypeAndRelation:
94-
return forcedIndex{IndexRelationshipBySubjectRelation}
99+
return forcedIndex{IndexRelationshipBySubjectRelation}, nil
95100

96101
case queryshape.FindResourceRelationForSubjectRelation:
97-
return forcedIndex{IndexRelationshipBySubjectRelation}
102+
return forcedIndex{IndexRelationshipBySubjectRelation}, nil
98103

99104
default:
100-
return nil
105+
if optionalFilter != nil {
106+
// If we have a filter, see if there's a forced index for it.
107+
index, err := IndexForFilter(schema, *optionalFilter)
108+
if err != nil {
109+
return nil, err
110+
}
111+
if index != nil {
112+
return forcedIndex{*index}, nil
113+
}
114+
115+
// No forced index for the filter.
116+
return NoIndexingHint, nil
117+
}
118+
119+
return NoIndexingHint, nil
101120
}
102121
}
103122

0 commit comments

Comments
 (0)