From 7532d8fd07c65585416cf1fa6385e1d17166a9ca Mon Sep 17 00:00:00 2001 From: Marius Posta Date: Wed, 4 Jan 2023 12:37:11 -0500 Subject: [PATCH] sql: inline GetImmutableDescriptorByID Informs #87753. Release note: None --- pkg/spanconfig/spanconfigreconciler/reconciler.go | 5 ++--- .../spanconfigsqltranslator/sqltranslator.go | 7 ++----- pkg/sql/catalog/descs/descriptor.go | 7 ------- pkg/sql/create_function.go | 4 +--- pkg/sql/privileged_accessor.go | 14 ++++---------- pkg/sql/rename_table.go | 2 +- pkg/sql/repair.go | 6 +++--- pkg/sql/resolver.go | 13 +++++-------- pkg/sql/schema_change_plan_node.go | 8 +++----- pkg/sql/schema_changer.go | 3 +-- pkg/sql/schemachanger/scdeps/build_deps.go | 4 +--- pkg/sql/schemachanger/scdeps/exec_deps.go | 5 +---- pkg/sql/type_change.go | 6 +++--- 13 files changed, 27 insertions(+), 57 deletions(-) diff --git a/pkg/spanconfig/spanconfigreconciler/reconciler.go b/pkg/spanconfig/spanconfigreconciler/reconciler.go index db2c1bad9fcd..a9a26834d1e3 100644 --- a/pkg/spanconfig/spanconfigreconciler/reconciler.go +++ b/pkg/spanconfig/spanconfigreconciler/reconciler.go @@ -643,12 +643,11 @@ func (r *incrementalReconciler) filterForMissingTableIDs( continue // nothing to do } - desc, err := descsCol.GetImmutableDescriptorByID(ctx, txn, descriptorUpdate.ID, tree.CommonLookupFlags{ - Required: true, // we want to error out for missing descriptors + desc, err := descsCol.ByID(txn).WithFlags(tree.CommonLookupFlags{ IncludeDropped: true, IncludeOffline: true, AvoidLeased: true, // we want consistent reads - }) + }).Immutable().Desc(ctx, descriptorUpdate.ID) considerAsMissing := false if errors.Is(err, catalog.ErrDescriptorNotFound) { diff --git a/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go b/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go index 9d9ebcc4dc91..65814530aa7c 100644 --- a/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go +++ b/pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go @@ -182,9 +182,6 @@ func (s *SQLTranslator) Translate( // descLookupFlags is the set of look up flags used when fetching descriptors. var descLookupFlags = tree.CommonLookupFlags{ - // We act on errors being surfaced when the descriptor being looked up is - // not found. - Required: true, // We can (do) generate span configurations for dropped and offline tables. IncludeDropped: true, IncludeOffline: true, @@ -261,7 +258,7 @@ func (s *SQLTranslator) generateSpanConfigurations( } // We're dealing with a SQL object. - desc, err := descsCol.GetImmutableDescriptorByID(ctx, txn, id, descLookupFlags) + desc, err := descsCol.ByID(txn).WithFlags(descLookupFlags).Immutable().Desc(ctx, id) if err != nil { if errors.Is(err, catalog.ErrDescriptorNotFound) { return nil, nil // the descriptor has been deleted; nothing to do here @@ -542,7 +539,7 @@ func (s *SQLTranslator) findDescendantLeafIDs( func (s *SQLTranslator) findDescendantLeafIDsForDescriptor( ctx context.Context, id descpb.ID, txn *kv.Txn, descsCol *descs.Collection, ) (descpb.IDs, error) { - desc, err := descsCol.GetImmutableDescriptorByID(ctx, txn, id, descLookupFlags) + desc, err := descsCol.ByID(txn).WithFlags(descLookupFlags).Immutable().Desc(ctx, id) if err != nil { if errors.Is(err, catalog.ErrDescriptorNotFound) { return nil, nil // the descriptor has been deleted; nothing to do here diff --git a/pkg/sql/catalog/descs/descriptor.go b/pkg/sql/catalog/descs/descriptor.go index 94d8297699e0..400d4b9d3314 100644 --- a/pkg/sql/catalog/descs/descriptor.go +++ b/pkg/sql/catalog/descs/descriptor.go @@ -44,13 +44,6 @@ func (tc *Collection) GetMutableDescriptorByID( return tc.ByID(txn).Mutable().Desc(ctx, id) } -// GetImmutableDescriptorByID delegates to GetImmutableDescriptorsByID. -func (tc *Collection) GetImmutableDescriptorByID( - ctx context.Context, txn *kv.Txn, id descpb.ID, flags tree.CommonLookupFlags, -) (catalog.Descriptor, error) { - return tc.ByID(txn).WithFlags(flags).Immutable().Desc(ctx, id) -} - // GetComment fetches comment from uncommitted cache if it exists, otherwise from storage. func (tc *Collection) GetComment(key catalogkeys.CommentKey) (string, bool) { if cmt, hasCmt, cached := tc.uncommittedComments.getUncommitted(key); cached { diff --git a/pkg/sql/create_function.go b/pkg/sql/create_function.go index 910a3eae769d..c5260752c5c8 100644 --- a/pkg/sql/create_function.go +++ b/pkg/sql/create_function.go @@ -471,9 +471,7 @@ func makeFunctionParam( } func (p *planner) descIsTable(ctx context.Context, id descpb.ID) (bool, error) { - desc, err := p.Descriptors().GetImmutableDescriptorByID( - ctx, p.Txn(), id, tree.ObjectLookupFlagsWithRequired().CommonLookupFlags, - ) + desc, err := p.Descriptors().ByID(p.Txn()).WithFlags(tree.CommonLookupFlags{}).Immutable().Desc(ctx, id) if err != nil { return false, err } diff --git a/pkg/sql/privileged_accessor.go b/pkg/sql/privileged_accessor.go index b0d308146fcd..add06742ecb2 100644 --- a/pkg/sql/privileged_accessor.go +++ b/pkg/sql/privileged_accessor.go @@ -80,16 +80,10 @@ func (p *planner) LookupZoneConfigByNamespaceID( // to check the permissions of a descriptor given its ID, or the id given // is not a descriptor of a table or database. func (p *planner) checkDescriptorPermissions(ctx context.Context, id descpb.ID) error { - desc, err := p.Descriptors().GetImmutableDescriptorByID( - ctx, p.txn, id, - tree.CommonLookupFlags{ - IncludeDropped: true, - IncludeOffline: true, - // Note that currently the ByID API implies required regardless of whether it - // is set. Set it just to be explicit. - Required: true, - }, - ) + desc, err := p.Descriptors().ByID(p.txn).WithFlags(tree.CommonLookupFlags{ + IncludeDropped: true, + IncludeOffline: true, + }).Immutable().Desc(ctx, id) if err != nil { // Filter the error due to the descriptor not existing. if errors.Is(err, catalog.ErrDescriptorNotFound) { diff --git a/pkg/sql/rename_table.go b/pkg/sql/rename_table.go index 0ec5de6ccfa8..a1463bf52ea8 100644 --- a/pkg/sql/rename_table.go +++ b/pkg/sql/rename_table.go @@ -252,7 +252,7 @@ func (n *renameTableNode) Close(context.Context) {} func (p *planner) dependentError( ctx context.Context, typeName string, objName string, parentID descpb.ID, id descpb.ID, op string, ) error { - desc, err := p.Descriptors().GetImmutableDescriptorByID(ctx, p.txn, id, tree.CommonLookupFlags{Required: true}) + desc, err := p.Descriptors().ByID(p.txn).WithFlags(tree.CommonLookupFlags{}).Immutable().Desc(ctx, id) if err != nil { return err } diff --git a/pkg/sql/repair.go b/pkg/sql/repair.go index 3582227ce5bd..116533db7acf 100644 --- a/pkg/sql/repair.go +++ b/pkg/sql/repair.go @@ -422,7 +422,7 @@ func (p *planner) UnsafeUpsertNamespaceEntry( flags.IncludeDropped = true flags.IncludeOffline = true validateDescriptor := func() error { - desc, err := p.Descriptors().GetImmutableDescriptorByID(ctx, p.Txn(), descID, flags) + desc, err := p.Descriptors().ByID(p.Txn()).WithFlags(flags).Immutable().Desc(ctx, descID) if err != nil && descID != keys.PublicSchemaID { return errors.Wrapf(err, "failed to retrieve descriptor %d", descID) } @@ -456,7 +456,7 @@ func (p *planner) UnsafeUpsertNamespaceEntry( if parentID == descpb.InvalidID { return nil } - parent, err := p.Descriptors().GetImmutableDescriptorByID(ctx, p.Txn(), parentID, flags) + parent, err := p.Descriptors().ByID(p.Txn()).WithFlags(flags).Immutable().Desc(ctx, parentID) if err != nil { return errors.Wrapf(err, "failed to look up parent %d", parentID) } @@ -470,7 +470,7 @@ func (p *planner) UnsafeUpsertNamespaceEntry( if parentSchemaID == descpb.InvalidID || parentSchemaID == keys.PublicSchemaID { return nil } - schema, err := p.Descriptors().GetImmutableDescriptorByID(ctx, p.Txn(), parentSchemaID, flags) + schema, err := p.Descriptors().ByID(p.Txn()).WithFlags(flags).Immutable().Desc(ctx, parentSchemaID) if err != nil { return err } diff --git a/pkg/sql/resolver.go b/pkg/sql/resolver.go index 0f773f82e066..ba3c35474fdc 100644 --- a/pkg/sql/resolver.go +++ b/pkg/sql/resolver.go @@ -598,14 +598,11 @@ func (p *planner) getFullyQualifiedTableNamesFromIDs( ctx context.Context, ids []descpb.ID, ) (fullyQualifiedNames []string, _ error) { for _, id := range ids { - desc, err := p.Descriptors().GetImmutableDescriptorByID( - ctx, p.txn, id, - tree.CommonLookupFlags{ - AvoidLeased: true, - IncludeDropped: true, - IncludeOffline: true, - }, - ) + desc, err := p.Descriptors().ByID(p.txn).WithFlags(tree.CommonLookupFlags{ + AvoidLeased: true, + IncludeDropped: true, + IncludeOffline: true, + }).Immutable().Desc(ctx, id) if err != nil { return nil, err } diff --git a/pkg/sql/schema_change_plan_node.go b/pkg/sql/schema_change_plan_node.go index 6065b3d7b704..cd7fbea0332a 100644 --- a/pkg/sql/schema_change_plan_node.go +++ b/pkg/sql/schema_change_plan_node.go @@ -197,11 +197,9 @@ func (p *planner) waitForDescriptorSchemaChanges( if err := txn.SetFixedTimestamp(ctx, now); err != nil { return err } - desc, err := descriptors.GetImmutableDescriptorByID(ctx, txn, descID, - tree.CommonLookupFlags{ - Required: true, - AvoidLeased: true, - }) + desc, err := descriptors.ByID(txn).WithFlags(tree.CommonLookupFlags{ + AvoidLeased: true, + }).Immutable().Desc(ctx, descID) if err != nil { return err } diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go index 4ebda18e3de4..218d08d06494 100644 --- a/pkg/sql/schema_changer.go +++ b/pkg/sql/schema_changer.go @@ -585,11 +585,10 @@ func (sc *SchemaChanger) getTargetDescriptor(ctx context.Context) (catalog.Descr ) (err error) { flags := tree.CommonLookupFlags{ AvoidLeased: true, - Required: true, IncludeOffline: true, IncludeDropped: true, } - desc, err = descriptors.GetImmutableDescriptorByID(ctx, txn, sc.descID, flags) + desc, err = descriptors.ByID(txn).WithFlags(flags).Immutable().Desc(ctx, sc.descID) return err }); err != nil { return nil, err diff --git a/pkg/sql/schemachanger/scdeps/build_deps.go b/pkg/sql/schemachanger/scdeps/build_deps.go index e989c05d6071..e097ceb778c3 100644 --- a/pkg/sql/schemachanger/scdeps/build_deps.go +++ b/pkg/sql/schemachanger/scdeps/build_deps.go @@ -231,13 +231,11 @@ func (d *buildDeps) CurrentDatabase() string { // MustReadDescriptor implements the scbuild.CatalogReader interface. func (d *buildDeps) MustReadDescriptor(ctx context.Context, id descpb.ID) catalog.Descriptor { flags := tree.CommonLookupFlags{ - Required: true, - RequireMutable: false, AvoidLeased: true, IncludeOffline: true, IncludeDropped: true, } - desc, err := d.descsCollection.GetImmutableDescriptorByID(ctx, d.txn, id, flags) + desc, err := d.descsCollection.ByID(d.txn).WithFlags(flags).Immutable().Desc(ctx, id) if err != nil { panic(err) } diff --git a/pkg/sql/schemachanger/scdeps/exec_deps.go b/pkg/sql/schemachanger/scdeps/exec_deps.go index dec2b705285b..8ae8a952e0bc 100644 --- a/pkg/sql/schemachanger/scdeps/exec_deps.go +++ b/pkg/sql/schemachanger/scdeps/exec_deps.go @@ -144,15 +144,12 @@ func (d *txnDeps) MustReadImmutableDescriptors( // GetFullyQualifiedName implements the scmutationexec.CatalogReader interface func (d *txnDeps) GetFullyQualifiedName(ctx context.Context, id descpb.ID) (string, error) { flags := tree.CommonLookupFlags{ - Required: true, IncludeOffline: true, IncludeDropped: true, AvoidLeased: true, AvoidSynthetic: true, } - objectDesc, err := d.descsCollection.GetImmutableDescriptorByID( - ctx, d.txn, id, flags, - ) + objectDesc, err := d.descsCollection.ByID(d.txn).WithFlags(flags).Immutable().Desc(ctx, id) if err != nil { return "", err } diff --git a/pkg/sql/type_change.go b/pkg/sql/type_change.go index 6a00669b2b98..db4f4ff2cdcf 100644 --- a/pkg/sql/type_change.go +++ b/pkg/sql/type_change.go @@ -197,14 +197,14 @@ func (t *typeSchemaChanger) getTypeDescFromStore( ) (catalog.TypeDescriptor, error) { var typeDesc catalog.TypeDescriptor if err := DescsTxn(ctx, t.execCfg, func(ctx context.Context, txn *kv.Txn, col *descs.Collection) error { - flags := tree.ObjectLookupFlags{CommonLookupFlags: tree.CommonLookupFlags{ + flags := tree.CommonLookupFlags{ AvoidLeased: true, IncludeDropped: true, IncludeOffline: true, - }} + } // Avoid GetImmutableTypeByID, downstream logic relies on // catalog.ErrDescriptorNotFound. - desc, err := col.GetImmutableDescriptorByID(ctx, txn, t.typeID, flags.CommonLookupFlags) + desc, err := col.ByID(txn).WithFlags(flags).Immutable().Desc(ctx, t.typeID) if err != nil { return err }