Skip to content

Commit

Permalink
sql,descs,*: rewrite by-ID and by-name descriptor lookups
Browse files Browse the repository at this point in the history 
This commit replaces all Get(Mutable|Immutable)*By(ID|Name) methods on the
descs.Collection and their usages with the recently-introduced
By(ID|Name)Getter methods.

While this commit shouldn't fundamentally change anything as far as
functionality is concerned, it isn't strictly-speaking a refactor
because the default behaviors have been changed slightly to make them
consistent accross similar kinds of lookups. Here's an illustrative
example: previously, GetImmutableDatabaseByID and GetImmutableSchemaByID
would honor the Required flag while the other GetImmutable*ByID methods
would ignore it and return an error when the descriptor is not found,
presently all immutable by-ID lookups always return an error when the
descriptor is not found.

The new API does away with the tree.(Common|Object)LookupFlags and these
have been cleaned up because now only the resolver uses them.

Fixes #87753.

Release note: None
  • Loading branch information
Marius Posta committed Jan 6, 2023
1 parent 57dd150 commit af6a72a
Show file tree
Hide file tree
Showing 153 changed files with 914 additions and 2,336 deletions.
2 changes: 1 addition & 1 deletion pkg/bench/rttanalysis/testdata/benchmark_expectations
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ exp,benchmark
5,GenerateObjects/generate_50000_tables
5,GenerateObjects/generate_1000_tables_-_this_test_should_use_the_same_number_of_RTTs_as_for_10_tables
5,GenerateObjects/generate_10_tables
10,GenerateObjects/generate_100_tables_from_template
12,GenerateObjects/generate_100_tables_from_template
16,GenerateObjects/generate_10x10_schemas_and_tables_in_existing_db
9,Grant/grant_all_on_1_table
9,Grant/grant_all_on_2_tables
Expand Down
64 changes: 24 additions & 40 deletions pkg/ccl/backupccl/backup_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10448,34 +10448,30 @@ $$;
udfID, err := strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, srcServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1")
require.NoError(t, err)
require.Equal(t, 104, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 106, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 107, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 108, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 110, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 111, int(fnDesc.GetID()))
require.Equal(t, 104, int(fnDesc.GetParentID()))
Expand Down Expand Up @@ -10504,34 +10500,30 @@ $$;
udfID, err = strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, srcServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1_new", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1_new")
require.NoError(t, err)
require.Equal(t, 112, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 114, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1_new", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 115, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1_new", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 116, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1_new", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 118, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 119, int(fnDesc.GetID()))
require.Equal(t, 112, int(fnDesc.GetParentID()))
Expand Down Expand Up @@ -10595,34 +10587,30 @@ $$;
udfID, err := strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, srcServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1")
require.NoError(t, err)
require.Equal(t, 104, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 106, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 107, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 108, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 110, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 111, int(fnDesc.GetID()))
require.Equal(t, 104, int(fnDesc.GetParentID()))
Expand Down Expand Up @@ -10653,34 +10641,30 @@ $$;
udfID, err = strconv.Atoi(rows[0][0])
require.NoError(t, err)
err = sql.TestingDescsTxn(ctx, tgtServer, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
dbDesc, err := col.GetImmutableDatabaseByName(ctx, txn, "db1", tree.DatabaseLookupFlags{Required: true})
dbDesc, err := col.ByNameWithLeased(txn).Get().Database(ctx, "db1")
require.NoError(t, err)
require.Equal(t, 107, int(dbDesc.GetID()))

scDesc, err := col.GetImmutableSchemaByName(ctx, txn, dbDesc, "sc1", tree.SchemaLookupFlags{Required: true})
scDesc, err := col.ByNameWithLeased(txn).Get().Schema(ctx, dbDesc, "sc1")
require.NoError(t, err)
require.Equal(t, 125, int(scDesc.GetID()))

tbName := tree.MakeTableNameWithSchema("db1", "sc1", "tbl1")
_, tbDesc, err := col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err := descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 126, int(tbDesc.GetID()))

typName := tree.MakeQualifiedTypeName("db1", "sc1", "enum1")
_, typDesc, err := col.GetImmutableTypeByName(ctx, txn, &typName, tree.ObjectLookupFlagsWithRequired())
_, typDesc, err := descs.PrefixAndType(ctx, col.ByNameWithLeased(txn).Get(), &typName)
require.NoError(t, err)
require.Equal(t, 127, int(typDesc.GetID()))

tbName = tree.MakeTableNameWithSchema("db1", "sc1", "sq1")
_, tbDesc, err = col.GetImmutableTableByName(
ctx, txn, &tbName, tree.ObjectLookupFlagsWithRequired(),
)
_, tbDesc, err = descs.PrefixAndTable(ctx, col.ByNameWithLeased(txn).Get(), &tbName)
require.NoError(t, err)
require.Equal(t, 129, int(tbDesc.GetID()))

fnDesc, err := col.GetImmutableFunctionByID(ctx, txn, descpb.ID(udfID), tree.ObjectLookupFlagsWithRequired())
fnDesc, err := col.ByIDWithLeased(txn).WithoutNonPublic().Get().Function(ctx, descpb.ID(udfID))
require.NoError(t, err)
require.Equal(t, 130, int(fnDesc.GetID()))
require.Equal(t, 107, int(fnDesc.GetParentID()))
Expand Down
73 changes: 16 additions & 57 deletions pkg/ccl/backupccl/restore_job.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1073,7 +1073,7 @@ func createImportingDescriptors(
// Write the updated databases.
for dbID, schemas := range existingDBsWithNewSchemas {
log.Infof(ctx, "writing %d schema entries to database %d", len(schemas), dbID)
desc, err := descsCol.GetMutableDescriptorByID(ctx, txn, dbID)
desc, err := descsCol.MutableByID(txn).Desc(ctx, dbID)
if err != nil {
return err
}
Expand All @@ -1093,12 +1093,7 @@ func createImportingDescriptors(
// to the new tables being restored.
for _, table := range mutableTables {
// Collect all types used by this table.
_, dbDesc, err := descsCol.GetImmutableDatabaseByID(
ctx, txn, table.GetParentID(), tree.DatabaseLookupFlags{
Required: true,
AvoidLeased: true,
IncludeOffline: true,
})
dbDesc, err := descsCol.ByID(txn).WithoutDropped().Get().Database(ctx, table.GetParentID())
if err != nil {
return err
}
Expand All @@ -1120,7 +1115,7 @@ func createImportingDescriptors(
continue
}
// Otherwise, add a backreference to this table.
typDesc, err := descsCol.GetMutableTypeVersionByID(ctx, txn, id)
typDesc, err := descsCol.MutableByID(txn).Type(ctx, id)
if err != nil {
return err
}
Expand All @@ -1144,16 +1139,7 @@ func createImportingDescriptors(
if details.DescriptorCoverage != tree.AllDescriptors {
for _, table := range tableDescs {
if lc := table.GetLocalityConfig(); lc != nil {
_, desc, err := descsCol.GetImmutableDatabaseByID(
ctx,
txn,
table.ParentID,
tree.DatabaseLookupFlags{
Required: true,
AvoidLeased: true,
IncludeOffline: true,
},
)
desc, err := descsCol.ByID(txn).WithoutDropped().Get().Database(ctx, table.ParentID)
if err != nil {
return err
}
Expand All @@ -1163,7 +1149,7 @@ func createImportingDescriptors(
table.ID, table.ParentID)
}

mutTable, err := descsCol.GetMutableTableVersionByID(ctx, table.GetID(), txn)
mutTable, err := descsCol.MutableByID(txn).Table(ctx, table.GetID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2242,7 +2228,7 @@ func prefetchDescriptors(
// and we're going to write them to KV very soon as part of a
// single batch).
ids := allDescIDs.Ordered()
got, err := descsCol.GetMutableDescriptorsByID(ctx, txn, ids...)
got, err := descsCol.MutableByID(txn).Descs(ctx, ids)
if err != nil {
return nstree.Catalog{}, errors.Wrap(err, "prefetch descriptors")
}
Expand Down Expand Up @@ -2374,7 +2360,7 @@ func (r *restoreResumer) dropDescriptors(
mutableTables := make([]*tabledesc.Mutable, len(details.TableDescs))
for i := range details.TableDescs {
var err error
mutableTables[i], err = descsCol.GetMutableTableVersionByID(ctx, details.TableDescs[i].ID, txn)
mutableTables[i], err = descsCol.MutableByID(txn).Table(ctx, details.TableDescs[i].ID)
if err != nil {
return err
}
Expand Down Expand Up @@ -2453,12 +2439,7 @@ func (r *restoreResumer) dropDescriptors(
// TypeDescriptors don't have a GC job process, so we can just write them
// as dropped here.
typDesc := details.TypeDescs[i]
mutType, err := descsCol.GetMutableTypeByID(ctx, txn, typDesc.ID, tree.ObjectLookupFlags{
CommonLookupFlags: tree.CommonLookupFlags{
AvoidLeased: true,
IncludeOffline: true,
},
})
mutType, err := descsCol.MutableByID(txn).Type(ctx, typDesc.ID)
if err != nil {
return err
}
Expand All @@ -2474,12 +2455,7 @@ func (r *restoreResumer) dropDescriptors(

for i := range details.FunctionDescs {
fnDesc := details.FunctionDescs[i]
mutFn, err := descsCol.GetMutableFunctionByID(ctx, txn, fnDesc.ID, tree.ObjectLookupFlags{
CommonLookupFlags: tree.CommonLookupFlags{
AvoidLeased: true,
IncludeOffline: true,
},
})
mutFn, err := descsCol.MutableByID(txn).Function(ctx, fnDesc.ID)
if err != nil {
return err
}
Expand Down Expand Up @@ -2551,13 +2527,13 @@ func (r *restoreResumer) dropDescriptors(
continue
}

mutSchema, err := descsCol.GetMutableDescriptorByID(ctx, txn, schemaDesc.GetID())
mutSchema, err := descsCol.MutableByID(txn).Desc(ctx, schemaDesc.GetID())
if err != nil {
return err
}
entry, hasEntry := dbsWithDeletedSchemas[schemaDesc.GetParentID()]
if !hasEntry {
mutParent, err := descsCol.GetMutableDescriptorByID(ctx, txn, schemaDesc.GetParentID())
mutParent, err := descsCol.MutableByID(txn).Desc(ctx, schemaDesc.GetParentID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2628,7 +2604,7 @@ func (r *restoreResumer) dropDescriptors(
continue
}

db, err := descsCol.GetMutableDescriptorByID(ctx, txn, dbDesc.GetID())
db, err := descsCol.MutableByID(txn).Desc(ctx, dbDesc.GetID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2682,24 +2658,12 @@ func setGCTTLForDroppingTable(
log.VInfof(ctx, 2, "lowering TTL for table %q (%d)", tableToDrop.GetName(), tableToDrop.GetID())
// We get a mutable descriptor here because we are going to construct a
// synthetic descriptor collection in which they are online.
_, dbDesc, err := descsCol.GetImmutableDatabaseByID(ctx, txn, tableToDrop.GetParentID(),
tree.DatabaseLookupFlags{
Required: true,
IncludeOffline: true,
IncludeDropped: true,
AvoidLeased: true,
})
dbDesc, err := descsCol.ByID(txn).Get().Database(ctx, tableToDrop.GetParentID())
if err != nil {
return err
}

schemaDesc, err := descsCol.GetImmutableSchemaByID(ctx, txn, tableToDrop.GetParentSchemaID(),
tree.SchemaLookupFlags{
Required: true,
IncludeDropped: true,
IncludeOffline: true,
AvoidLeased: true,
})
schemaDesc, err := descsCol.ByID(txn).Get().Schema(ctx, tableToDrop.GetParentSchemaID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2758,20 +2722,15 @@ func (r *restoreResumer) removeExistingTypeBackReferences(
return restored, nil
}
// Finally, look it up using the transaction.
typ, err := descsCol.GetMutableTypeVersionByID(ctx, txn, id)
typ, err := descsCol.MutableByID(txn).Type(ctx, id)
if err != nil {
return nil, err
}
existingTypes[typ.GetID()] = typ
return typ, nil
}

_, dbDesc, err := descsCol.GetImmutableDatabaseByID(
ctx, txn, tbl.GetParentID(), tree.DatabaseLookupFlags{
Required: true,
AvoidLeased: true,
IncludeOffline: true,
})
dbDesc, err := descsCol.ByID(txn).WithoutDropped().Get().Database(ctx, tbl.GetParentID())
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit af6a72a

Please sign in to comment.