Skip to content

Commit

Permalink
sql: inline GetMutableTable*ByID
Browse files Browse the repository at this point in the history
Informs #87753.

Release note: None
  • Loading branch information
Marius Posta committed Jan 4, 2023
1 parent 837672c commit 9fc8c24
Show file tree
Hide file tree
Showing 32 changed files with 71 additions and 159 deletions.
4 changes: 2 additions & 2 deletions pkg/ccl/backupccl/restore_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -1149,7 +1149,7 @@ func createImportingDescriptors(
table.ID, table.ParentID)
}

mutTable, err := descsCol.GetMutableTableVersionByID(ctx, table.GetID(), txn)
mutTable, err := descsCol.ByID(txn).Mutable().Table(ctx, table.GetID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2360,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.ByID(txn).Mutable().Table(ctx, details.TableDescs[i].ID)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/alter_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -1493,7 +1493,7 @@ func (p *planner) updateFKBackReferenceName(
if tableDesc.ID == ref.ReferencedTableID {
referencedTableDesc = tableDesc
} else {
lookup, err := p.Descriptors().GetMutableTableVersionByID(ctx, ref.ReferencedTableID, p.txn)
lookup, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, ref.ReferencedTableID)
if err != nil {
return errors.Wrapf(err, "error resolving referenced table ID %d", ref.ReferencedTableID)
}
Expand Down
14 changes: 7 additions & 7 deletions pkg/sql/backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ func (sc *SchemaChanger) dropConstraints(

// Create update closure for the table and all other tables with backreferences.
if err := sc.txn(ctx, func(ctx context.Context, txn *kv.Txn, descsCol *descs.Collection) error {
scTable, err := descsCol.GetMutableTableVersionByID(ctx, sc.descID, txn)
scTable, err := descsCol.ByID(txn).Mutable().Table(ctx, sc.descID)
if err != nil {
return err
}
Expand Down Expand Up @@ -480,7 +480,7 @@ func (sc *SchemaChanger) dropConstraints(
if def.Name != constraint.GetName() {
continue
}
backrefTable, err := descsCol.GetMutableTableVersionByID(ctx, fk.GetReferencedTableID(), txn)
backrefTable, err := descsCol.ByID(txn).Mutable().Table(ctx, fk.GetReferencedTableID())
if err != nil {
return err
}
Expand Down Expand Up @@ -586,7 +586,7 @@ func (sc *SchemaChanger) addConstraints(
if err := sc.txn(ctx, func(
ctx context.Context, txn *kv.Txn, descsCol *descs.Collection,
) error {
scTable, err := descsCol.GetMutableTableVersionByID(ctx, sc.descID, txn)
scTable, err := descsCol.ByID(txn).Mutable().Table(ctx, sc.descID)
if err != nil {
return err
}
Expand Down Expand Up @@ -635,7 +635,7 @@ func (sc *SchemaChanger) addConstraints(
}
if !foundExisting {
scTable.OutboundFKs = append(scTable.OutboundFKs, *fk.ForeignKeyDesc())
backrefTable, err := descsCol.GetMutableTableVersionByID(ctx, fk.GetReferencedTableID(), txn)
backrefTable, err := descsCol.ByID(txn).Mutable().Table(ctx, fk.GetReferencedTableID())
if err != nil {
return err
}
Expand Down Expand Up @@ -2208,7 +2208,7 @@ func (sc *SchemaChanger) mergeFromTemporaryIndex(
ctx context.Context, txn *kv.Txn, descsCol *descs.Collection,
) error {
var err error
tbl, err = descsCol.GetMutableTableVersionByID(ctx, sc.descID, txn)
tbl, err = descsCol.ByID(txn).Mutable().Table(ctx, sc.descID)
return err
}); err != nil {
return err
Expand All @@ -2229,7 +2229,7 @@ func (sc *SchemaChanger) runStateMachineAfterTempIndexMerge(ctx context.Context)
return sc.txn(ctx, func(
ctx context.Context, txn *kv.Txn, descsCol *descs.Collection,
) error {
tbl, err := descsCol.GetMutableTableVersionByID(ctx, sc.descID, txn)
tbl, err := descsCol.ByID(txn).Mutable().Table(ctx, sc.descID)
if err != nil {
return err
}
Expand Down Expand Up @@ -2534,7 +2534,7 @@ func runSchemaChangesInTxn(
if selfReference {
referencedTableDesc = tableDesc
} else {
lookup, err := planner.Descriptors().GetMutableTableVersionByID(ctx, fk.GetReferencedTableID(), planner.Txn())
lookup, err := planner.Descriptors().ByID(planner.Txn()).Mutable().Table(ctx, fk.GetReferencedTableID())
if err != nil {
return errors.Wrapf(err, "error resolving referenced table ID %d", fk.GetReferencedTableID())
}
Expand Down
14 changes: 4 additions & 10 deletions pkg/sql/catalog/descs/collection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ func TestSyntheticDescriptorResolution(t *testing.T) {
require.Equal(t, "bar", desc.PublicColumns()[0].GetName())

// Attempting to resolve the table mutably is not allowed.
_, err = descriptors.GetMutableTableByID(ctx, txn, tableID, tree.ObjectLookupFlags{})
_, err = descriptors.ByID(txn).Mutable().Table(ctx, tableID)
require.EqualError(t, err, fmt.Sprintf("attempted mutable access of synthetic descriptor %d", tableID))

return nil
Expand Down Expand Up @@ -1157,9 +1157,7 @@ SELECT id

// Modify the table to have the name "bar", synthetically
{
tab, err := descriptors.GetMutableTableByID(
ctx, txn, tabID, tree.ObjectLookupFlagsWithRequired(),
)
tab, err := descriptors.ByID(txn).Mutable().Table(ctx, tabID)
if err != nil {
return err
}
Expand All @@ -1176,17 +1174,13 @@ SELECT id
return err
}
// Attempt to retrieve the mutable descriptor, validate the error.
_, err := descriptors.GetMutableTableByID(
ctx, txn, tabID, tree.ObjectLookupFlagsWithRequired(),
)
_, err := descriptors.ByID(txn).Mutable().Table(ctx, tabID)
require.Regexp(t, `attempted mutable access of synthetic descriptor \d+`, err)
descriptors.ResetSyntheticDescriptors()
// Retrieve the mutable descriptor, find the unmodified "foo".
// Then modify the name to "baz" and write it.
{
tabMut, err := descriptors.GetMutableTableByID(
ctx, txn, tabID, tree.ObjectLookupFlagsWithRequired(),
)
tabMut, err := descriptors.ByID(txn).Mutable().Table(ctx, tabID)
require.NoError(t, err)
require.Equal(t, "foo", tabMut.GetName())
tabMut.Name = "baz"
Expand Down
20 changes: 0 additions & 20 deletions pkg/sql/catalog/descs/table.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,23 +86,3 @@ func (tc *Collection) GetUncommittedMutableTableByID(
}
return original.(catalog.TableDescriptor), mut.(*tabledesc.Mutable), nil
}

// GetMutableTableByID returns a mutable table descriptor with
// properties according to the provided lookup flags. RequireMutable is ignored.
// Required is ignored, and an error is always returned if no descriptor with
// the ID exists.
func (tc *Collection) GetMutableTableByID(
ctx context.Context, txn *kv.Txn, tableID descpb.ID, flags tree.ObjectLookupFlags,
) (*tabledesc.Mutable, error) {
return tc.ByID(txn).WithFlags(flags.CommonLookupFlags).Mutable().Table(ctx, tableID)
}

// GetMutableTableVersionByID is a variant of sqlbase.getTableDescFromID which returns a mutable
// table descriptor of the table modified in the same transaction.
// TODO (lucy): Usages should be replaced with GetMutableTableByID, but this
// needs a careful look at what flags should be passed in at each call site.
func (tc *Collection) GetMutableTableVersionByID(
ctx context.Context, tableID descpb.ID, txn *kv.Txn,
) (*tabledesc.Mutable, error) {
return tc.ByID(txn).Mutable().Table(ctx, tableID)
}
6 changes: 2 additions & 4 deletions pkg/sql/catalog/lease/lease_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,7 @@ func init() {
ctx context.Context, t *testing.T, s serverutils.TestServerInterface, id descpb.ID,
) {
require.NoError(t, sql.TestingDescsTxn(ctx, s, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error {
t, err := col.GetMutableTableByID(
ctx, txn, id, tree.ObjectLookupFlagsWithRequired(),
)
t, err := col.ByID(txn).Mutable().Table(ctx, id)
if err != nil {
return err
}
Expand Down Expand Up @@ -2439,7 +2437,7 @@ func TestLeaseWithOfflineTables(t *testing.T) {
flags := tree.ObjectLookupFlagsWithRequiredTableKind(tree.ResolveRequireTableDesc)
flags.CommonLookupFlags.IncludeOffline = true
flags.CommonLookupFlags.IncludeDropped = true
desc, err := descsCol.GetMutableTableByID(ctx, txn, testTableID(), flags)
desc, err := descsCol.ByID(txn).Mutable().Table(ctx, testTableID())
require.NoError(t, err)
require.Equal(t, desc.State, expected)
desc.State = next
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,7 @@ func (p *planner) validateTTLScheduledJobInTable(

// RepairTTLScheduledJobForTable is part of the EvalPlanner interface.
func (p *planner) RepairTTLScheduledJobForTable(ctx context.Context, tableID int64) error {
tableDesc, err := p.Descriptors().GetMutableTableByID(ctx, p.txn, descpb.ID(tableID), tree.ObjectLookupFlagsWithRequired())
tableDesc, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, descpb.ID(tableID))
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestValidateTTLScheduledJobs(t *testing.T) {
require.NoError(t, sql.TestingDescsTxn(ctx, s, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) (err error) {
// We need the collection to read the descriptor from storage for
// the subsequent write to succeed.
tableDesc, err = col.GetMutableTableByID(ctx, txn, tableDesc.GetID(), tree.ObjectLookupFlagsWithRequired())
tableDesc, err = col.ByID(txn).Mutable().Table(ctx, tableDesc.GetID())
tableDesc.RowLevelTTL.ScheduleID = 0
tableDesc.Version++
if err != nil {
Expand Down
8 changes: 2 additions & 6 deletions pkg/sql/create_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,7 @@ func (n *createFunctionNode) replaceFunction(udfDesc *funcdesc.Mutable, params r

// Removing all existing references before adding new references.
for _, id := range udfDesc.DependsOn {
backRefMutable, err := params.p.Descriptors().GetMutableTableByID(
params.ctx, params.p.txn, id, tree.ObjectLookupFlagsWithRequired(),
)
backRefMutable, err := params.p.Descriptors().ByID(params.p.txn).Mutable().Table(params.ctx, id)
if err != nil {
return err
}
Expand Down Expand Up @@ -328,9 +326,7 @@ func (n *createFunctionNode) addUDFReferences(udfDesc *funcdesc.Mutable, params
// Read all referenced tables and update their dependencies.
backRefMutables := make(map[descpb.ID]*tabledesc.Mutable)
for _, id := range backrefTblIDs.Ordered() {
backRefMutable, err := params.p.Descriptors().GetMutableTableByID(
params.ctx, params.p.txn, id, tree.ObjectLookupFlagsWithRequired(),
)
backRefMutable, err := params.p.Descriptors().ByID(params.p.txn).Mutable().Table(params.ctx, id)
if err != nil {
return err
}
Expand Down
8 changes: 3 additions & 5 deletions pkg/sql/create_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,7 @@ func (n *createViewNode) startExec(params runParams) error {
return err
}
for id := range n.planDeps {
backRefMutable, err := params.p.Descriptors().GetMutableTableByID(
params.ctx, params.p.Txn(), id, tree.ObjectLookupFlagsWithRequired(),
)
backRefMutable, err := params.p.Descriptors().ByID(params.p.Txn()).Mutable().Table(params.ctx, id)
if err != nil {
return err
}
Expand Down Expand Up @@ -160,7 +158,7 @@ func (n *createViewNode) startExec(params runParams) error {
if err != nil {
return err
}
desc, err := params.p.Descriptors().GetMutableTableVersionByID(params.ctx, id, params.p.txn)
desc, err := params.p.Descriptors().ByID(params.p.txn).Mutable().Table(params.ctx, id)
if err != nil {
return err
}
Expand Down Expand Up @@ -619,7 +617,7 @@ func (p *planner) replaceViewDesc(
desc, ok := backRefMutables[id]
if !ok {
var err error
desc, err = p.Descriptors().GetMutableTableVersionByID(ctx, id, p.txn)
desc, err = p.Descriptors().ByID(p.txn).Mutable().Table(ctx, id)
if err != nil {
return nil, err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/sql/delete_preserving_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,7 @@ func TestMergeProcessor(t *testing.T) {

require.NoError(t, sql.DescsTxn(ctx, &execCfg, func(
ctx context.Context, txn *kv.Txn, descriptors *descs.Collection) error {
mut, err := descriptors.GetMutableTableByID(ctx, txn, tableDesc.GetID(), tree.ObjectLookupFlags{})
mut, err := descriptors.ByID(txn).Mutable().Table(ctx, tableDesc.GetID())
if err != nil {
return err
}
Expand Down Expand Up @@ -710,7 +710,7 @@ func TestMergeProcessor(t *testing.T) {

require.NoError(t, sql.DescsTxn(ctx, &execCfg, func(
ctx context.Context, txn *kv.Txn, descriptors *descs.Collection) error {
mut, err := descriptors.GetMutableTableByID(ctx, txn, tableDesc.GetID(), tree.ObjectLookupFlags{})
mut, err := descriptors.ByID(txn).Mutable().Table(ctx, tableDesc.GetID())
if err != nil {
return err
}
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/descmetadata/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ go_library(
"//pkg/sql/catalog/descpb",
"//pkg/sql/catalog/descs",
"//pkg/sql/schemachanger/scexec",
"//pkg/sql/sem/tree",
"//pkg/sql/sessiondata",
"//pkg/sql/sessioninit",
"//pkg/sql/sqlutil",
Expand Down
12 changes: 1 addition & 11 deletions pkg/sql/descmetadata/metadata_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
"github.com/cockroachdb/cockroach/pkg/sql/catalog/descs"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scexec"
"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
"github.com/cockroachdb/cockroach/pkg/sql/sessiondata"
"github.com/cockroachdb/cockroach/pkg/sql/sessioninit"
"github.com/cockroachdb/cockroach/pkg/sql/sqlutil"
Expand Down Expand Up @@ -80,16 +79,7 @@ func (mu metadataUpdater) DeleteDatabaseRoleSettings(ctx context.Context, dbID d
return nil
}
// Bump the table version for the role settings table when we modify it.
desc, err := mu.descriptors.GetMutableTableByID(
ctx,
mu.txn,
keys.DatabaseRoleSettingsTableID,
tree.ObjectLookupFlags{
CommonLookupFlags: tree.CommonLookupFlags{
Required: true,
RequireMutable: true,
},
})
desc, err := mu.descriptors.ByID(mu.txn).Mutable().Table(ctx, keys.DatabaseRoleSettingsTableID)
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/drop_database.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ func (p *planner) accumulateOwnedSequences(
) error {
for colID := range desc.GetColumns() {
for _, seqID := range desc.GetColumns()[colID].OwnsSequenceIds {
ownedSeqDesc, err := p.Descriptors().GetMutableTableVersionByID(ctx, seqID, p.txn)
ownedSeqDesc, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, seqID)
if err != nil {
// Special case error swallowing for #50711 and #50781, which can
// cause columns to own sequences that have been dropped/do not
Expand Down
4 changes: 1 addition & 3 deletions pkg/sql/drop_function.go
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,7 @@ func (p *planner) dropFunctionImpl(ctx context.Context, fnMutable *funcdesc.Muta
// TODO(chengxiong): remove backreference from UDFs that this UDF has
// reference to. This is needed when we allow UDFs being referenced by
// UDFs.
refMutable, err := p.Descriptors().GetMutableTableByID(
ctx, p.txn, id, tree.ObjectLookupFlagsWithRequired(),
)
refMutable, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, id)
if err != nil {
return err
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/sql/drop_table.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (p *planner) canDropTable(
func (p *planner) canRemoveFKBackreference(
ctx context.Context, from string, ref catalog.ForeignKeyConstraint, behavior tree.DropBehavior,
) error {
table, err := p.Descriptors().GetMutableTableVersionByID(ctx, ref.GetOriginTableID(), p.txn)
table, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, ref.GetOriginTableID())
if err != nil {
return err
}
Expand Down Expand Up @@ -459,7 +459,7 @@ func (p *planner) removeFKForBackReference(
if tableDesc.ID == ref.GetOriginTableID() {
originTableDesc = tableDesc
} else {
lookup, err := p.Descriptors().GetMutableTableVersionByID(ctx, ref.GetOriginTableID(), p.txn)
lookup, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, ref.GetOriginTableID())
if err != nil {
return errors.Wrapf(err, "error resolving origin table ID %d", ref.GetOriginTableID())
}
Expand Down Expand Up @@ -523,7 +523,7 @@ func (p *planner) removeFKBackReference(
if tableDesc.ID == ref.ReferencedTableID {
referencedTableDesc = tableDesc
} else {
lookup, err := p.Descriptors().GetMutableTableVersionByID(ctx, ref.ReferencedTableID, p.txn)
lookup, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, ref.ReferencedTableID)
if err != nil {
return errors.Wrapf(err, "error resolving referenced table ID %d", ref.ReferencedTableID)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/sql/drop_view.go
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ func (p *planner) dropViewImpl(
// Remove back-references from the tables/views this view depends on.
dependedOn := append([]descpb.ID(nil), viewDesc.DependsOn...)
for _, depID := range dependedOn {
dependencyDesc, err := p.Descriptors().GetMutableTableVersionByID(ctx, depID, p.txn)
dependencyDesc, err := p.Descriptors().ByID(p.txn).Mutable().Table(ctx, depID)
if err != nil {
return cascadeDroppedViews,
errors.Wrapf(err, "error resolving dependency relation ID %d", depID)
Expand Down
1 change: 0 additions & 1 deletion pkg/sql/gcjob/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ go_library(
"//pkg/sql/oppurpose",
"//pkg/sql/pgwire/pgcode",
"//pkg/sql/pgwire/pgerror",
"//pkg/sql/sem/tree",
"//pkg/sql/sqlerrors",
"//pkg/storage",
"//pkg/util/admission/admissionpb",
Expand Down
Loading

0 comments on commit 9fc8c24

Please sign in to comment.