Skip to content

Commit 04e44c8

Browse files
committed
sql,optbuilder: route descriptor lookups for triggers through cache
Triggers have a `TG_TABLE_SCHEMA` argument which provides the schema name. Getting the schema name requires doing a descriptor lookup when the trigger is planned, which previously avoided the cache and always performed an expensive KV lookup. This commit plumbs `ResolveSchemaByID`, which allows the lookup to hit the descriptor cache. Fixes #144211 Release note (performance improvement): Triggers now perform the descriptor lookup for `TG_TABLE_SCHEMA` against a cache. This can significantly reduce trigger planning latency in multi-region databases.
1 parent bd46989 commit 04e44c8

File tree

9 files changed

+105
-9
lines changed

9 files changed

+105
-9
lines changed

pkg/bench/rttanalysis/testdata/benchmark_expectations

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ exp,benchmark
119119
1,SystemDatabaseQueries/select_system.users_with_empty_database_Name
120120
1,SystemDatabaseQueries/select_system.users_with_schema_Name
121121
1,SystemDatabaseQueries/select_system.users_without_schema_Name
122-
3,TriggerResolution/insert_into_table_with_trigger
122+
1,TriggerResolution/insert_into_table_with_trigger
123123
15,Truncate/truncate_1_column_0_rows
124124
15,Truncate/truncate_1_column_1_row
125125
15,Truncate/truncate_1_column_2_rows

pkg/sql/opt/cat/catalog.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,10 @@ type Catalog interface {
153153
// be safely copied or used across goroutines.
154154
ResolveSchema(ctx context.Context, flags Flags, name *SchemaName) (Schema, SchemaName, error)
155155

156+
// ResolveSchemaByID is similar to ResolveSchema, except that it locates a
157+
// schema by its StableID. See the comment for StableID for more details.
158+
ResolveSchemaByID(ctx context.Context, flags Flags, id StableID) (Schema, error)
159+
156160
// GetAllSchemaNamesForDB Gets all the SchemaNames for a database.
157161
GetAllSchemaNamesForDB(ctx context.Context, dbName string) ([]SchemaName, error)
158162

pkg/sql/opt/cat/table.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,10 @@ type Table interface {
175175
// owning database could not be determined.
176176
GetDatabaseID() descpb.ID
177177

178+
// GetSchemaID returns the owning schema id of the table, or zero, if the
179+
// owning schema could not be determined.
180+
GetSchemaID() descpb.ID
181+
178182
// IsHypothetical returns true if this is a hypothetical table (used when
179183
// searching for index recommendations).
180184
IsHypothetical() bool

pkg/sql/opt/exec/explain/plan_gist_factory.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,11 @@ func (u *unknownTable) GetDatabaseID() descpb.ID {
650650
return 0
651651
}
652652

653+
// GetSchemaID is part of the cat.Table interface.
654+
func (u *unknownTable) GetSchemaID() descpb.ID {
655+
return 0
656+
}
657+
653658
// IsHypothetical is part of the cat.Table interface.
654659
func (u *unknownTable) IsHypothetical() bool {
655660
return false

pkg/sql/opt/optbuilder/trigger.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -220,11 +220,13 @@ func (mb *mutationBuilder) buildTriggerFunctionArgs(
220220
tgOp := tree.NewDString(eventType.String())
221221
tgRelID := tree.NewDOid(oid.Oid(mb.tab.ID()))
222222
tgTableName := tree.NewDString(string(mb.tab.Name()))
223-
fqName, err := mb.b.catalog.FullyQualifiedName(mb.b.ctx, mb.tab)
223+
schema, err := mb.b.catalog.ResolveSchemaByID(
224+
mb.b.ctx, cat.Flags{}, cat.StableID(mb.tab.GetSchemaID()),
225+
)
224226
if err != nil {
225227
panic(err)
226228
}
227-
tgTableSchema := tree.NewDString(fqName.Schema())
229+
tgTableSchema := tree.NewDString(schema.Name().Schema())
228230
tgNumArgs := tree.NewDInt(tree.DInt(len(trigger.FuncArgs())))
229231
tgArgV := tree.NewDArray(types.String)
230232
for _, arg := range trigger.FuncArgs() {

pkg/sql/opt/testutils/testcat/create_table.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func (tc *Catalog) CreateTable(stmt *tree.CreateTable) *Table {
8080
isRbr = stmt.Locality.LocalityLevel == tree.LocalityLevelRow
8181
isRbt = stmt.Locality.LocalityLevel == tree.LocalityLevelTable
8282
}
83-
tab := &Table{TabID: tc.nextStableID(), TabName: stmt.Table, Catalog: tc}
83+
tab := &Table{TabID: tc.nextStableID(), SchemaID: testSchemaID, TabName: stmt.Table, Catalog: tc}
8484

8585
if isRbt && stmt.Locality.TableRegion != "" {
8686
tab.multiRegion = true
@@ -490,7 +490,13 @@ func (tc *Catalog) CreateTableAs(name tree.TableName, columns []cat.Column) *Tab
490490
// Update the table name to include catalog and schema if not provided.
491491
tc.qualifyTableName(&name)
492492

493-
tab := &Table{TabID: tc.nextStableID(), TabName: name, Catalog: tc, Columns: columns}
493+
tab := &Table{
494+
TabID: tc.nextStableID(),
495+
SchemaID: testSchemaID,
496+
TabName: name,
497+
Catalog: tc,
498+
Columns: columns,
499+
}
494500

495501
var rowid cat.Column
496502
ordinal := len(columns)

pkg/sql/opt/testutils/testcat/test_catalog.go

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,8 @@ import (
4343

4444
const (
4545
// testDB is the default current database for testing purposes.
46-
testDB = "t"
46+
testDB = "t"
47+
testSchemaID = 1
4748
)
4849

4950
// Catalog implements the cat.Catalog interface for testing purposes.
@@ -78,7 +79,7 @@ func New() *Catalog {
7879

7980
return &Catalog{
8081
testSchema: Schema{
81-
SchemaID: 1,
82+
SchemaID: testSchemaID,
8283
SchemaName: cat.SchemaName{
8384
CatalogName: testDB,
8485
SchemaName: catconstants.PublicSchemaName,
@@ -135,6 +136,17 @@ func (tc *Catalog) ResolveSchema(
135136
return tc.resolveSchema(&toResolve)
136137
}
137138

139+
// ResolveSchemaByID is part of the cat.Catalog interface.
140+
func (tc *Catalog) ResolveSchemaByID(
141+
_ context.Context, _ cat.Flags, id cat.StableID,
142+
) (cat.Schema, error) {
143+
if id != testSchemaID {
144+
return nil, pgerror.Newf(pgcode.InvalidSchemaName,
145+
"target database or schema does not exist")
146+
}
147+
return &tc.testSchema, nil
148+
}
149+
138150
// GetAllSchemaNamesForDB is part of the cat.Catalog interface.
139151
func (tc *Catalog) GetAllSchemaNamesForDB(
140152
ctx context.Context, dbName string,
@@ -844,6 +856,7 @@ func (tv *View) Trigger(i int) cat.Trigger {
844856
type Table struct {
845857
TabID cat.StableID
846858
DatabaseID descpb.ID
859+
SchemaID descpb.ID
847860
TabVersion int
848861
TabName tree.TableName
849862
Columns []cat.Column
@@ -1085,6 +1098,11 @@ func (tt *Table) GetDatabaseID() descpb.ID {
10851098
return tt.DatabaseID
10861099
}
10871100

1101+
// GetSchemaID is part of the cat.Table interface.
1102+
func (tt *Table) GetSchemaID() descpb.ID {
1103+
return tt.SchemaID
1104+
}
1105+
10881106
// IsHypothetical is part of the cat.Table interface.
10891107
func (tt *Table) IsHypothetical() bool {
10901108
return false

pkg/sql/opt_catalog.go

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,32 @@ func (oc *optCatalog) LookupDatabaseName(
183183
return tree.Name(name), nil
184184
}
185185

186+
func (oc *optCatalog) ResolveSchemaByID(
187+
ctx context.Context, flags cat.Flags, schemaID cat.StableID,
188+
) (cat.Schema, error) {
189+
if flags.AvoidDescriptorCaches {
190+
defer func(prev bool) {
191+
oc.planner.skipDescriptorCache = prev
192+
}(oc.planner.skipDescriptorCache)
193+
oc.planner.skipDescriptorCache = true
194+
}
195+
196+
schemaLookup, err := oc.planner.LookupSchemaByID(ctx, descpb.ID(schemaID))
197+
if err != nil {
198+
return nil, err
199+
}
200+
databaseLookup, err := oc.planner.LookupDatabaseByID(ctx, schemaLookup.GetParentID())
201+
if err != nil {
202+
return nil, err
203+
}
204+
return &optSchema{
205+
planner: oc.planner,
206+
database: databaseLookup,
207+
schema: schemaLookup,
208+
name: oc.tn.ObjectNamePrefix,
209+
}, nil
210+
}
211+
186212
// ResolveSchema is part of the cat.Catalog interface.
187213
func (oc *optCatalog) ResolveSchema(
188214
ctx context.Context, flags cat.Flags, name *cat.SchemaName,
@@ -1523,6 +1549,11 @@ func (ot *optTable) GetDatabaseID() descpb.ID {
15231549
return ot.desc.GetParentID()
15241550
}
15251551

1552+
// GetSchemaID is part of the cat.Table interface.
1553+
func (ot *optTable) GetSchemaID() descpb.ID {
1554+
return ot.desc.GetParentSchemaID()
1555+
}
1556+
15261557
// IsHypothetical is part of the cat.Table interface.
15271558
func (ot *optTable) IsHypothetical() bool {
15281559
return false
@@ -2639,6 +2670,11 @@ func (ot *optVirtualTable) GetDatabaseID() descpb.ID {
26392670
return 0
26402671
}
26412672

2673+
// GetSchemaID is part of the cat.Table interface.
2674+
func (ot *optVirtualTable) GetSchemaID() descpb.ID {
2675+
return ot.desc.GetParentSchemaID()
2676+
}
2677+
26422678
// IsHypothetical is part of the cat.Table interface.
26432679
func (ot *optVirtualTable) IsHypothetical() bool {
26442680
return false

pkg/sql/planner.go

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -741,8 +741,7 @@ func (p *planner) CheckPrivilegeForTableID(
741741
return p.CheckPrivilegeForUser(ctx, desc, privilege, p.User())
742742
}
743743

744-
// LookupTableByID looks up a table, by the given descriptor ID. Based on the
745-
// CommonLookupFlags, it could use or skip the Collection cache.
744+
// LookupTableByID looks up a table, by the given descriptor ID.
746745
func (p *planner) LookupTableByID(
747746
ctx context.Context, tableID descpb.ID,
748747
) (catalog.TableDescriptor, error) {
@@ -753,6 +752,28 @@ func (p *planner) LookupTableByID(
753752
return table, nil
754753
}
755754

755+
// LookupSchemaByID looks up a schema by the given descriptor ID.
756+
func (p *planner) LookupSchemaByID(
757+
ctx context.Context, schemaID descpb.ID,
758+
) (catalog.SchemaDescriptor, error) {
759+
schema, err := p.byIDGetterBuilder().WithoutNonPublic().Get().Schema(ctx, schemaID)
760+
if err != nil {
761+
return nil, err
762+
}
763+
return schema, nil
764+
}
765+
766+
// LookupDatabaseByID looks up a database by the given descriptor ID.
767+
func (p *planner) LookupDatabaseByID(
768+
ctx context.Context, databaseID descpb.ID,
769+
) (catalog.DatabaseDescriptor, error) {
770+
database, err := p.byIDGetterBuilder().WithoutNonPublic().Get().Database(ctx, databaseID)
771+
if err != nil {
772+
return nil, err
773+
}
774+
return database, nil
775+
}
776+
756777
// SessionData is part of the PlanHookState interface.
757778
func (p *planner) SessionData() *sessiondata.SessionData {
758779
return p.EvalContext().SessionData()

0 commit comments

Comments
 (0)