Skip to content

Commit

Permalink
sqk: clean up interfaces for new schema changer
Browse files Browse the repository at this point in the history 
Previously, we had a monolithic MutDescGetter interface,
which exposed different operations outside of jut descriptor
ones. This was inadequate because in the longer term it
would turn into a maintain once nightmare. This patch
turns it into Catalog interface with child operations.

Release note: None
  • Loading branch information
@fqazi
fqazi committed Jun 4, 2021
1 parent 41cb15e commit f5e633f
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 32 deletions.
2 changes: 1 addition & 1 deletion pkg/sql/schemachanger/scexec/mutation_desc_getter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,4 +112,4 @@ func (m *mutationDescGetter) RemoveObjectComments(ctx context.Context, id descpb
return err
}

var _ scmutationexec.MutableDescGetter = (*mutationDescGetter)(nil)
var _ scmutationexec.Catalog = (*mutationDescGetter)(nil)
80 changes: 49 additions & 31 deletions pkg/sql/schemachanger/scexec/scmutationexec/scmutationexec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,35 @@ import (
"github.com/cockroachdb/errors"
)

// MutableDescGetter encapsulates the logic to retrieve descriptors.
// All retrieved descriptors are modified.
type MutableDescGetter interface {
// DescriptorReader encapsulates logic used to retrieve
// and track modified descriptors.
type DescriptorReader interface {
GetMutableTypeByID(ctx context.Context, id descpb.ID) (*typedesc.Mutable, error)
GetImmutableDatabaseByID(ctx context.Context, id descpb.ID) (catalog.DatabaseDescriptor, error)
GetMutableTableByID(ctx context.Context, id descpb.ID) (*tabledesc.Mutable, error)
}

// NamespaceWriter encapsulates operations used to manipulate
// namespaces.
type NamespaceWriter interface {
AddDrainedName(id descpb.ID, nameInfo descpb.NameInfo)
SubmitDrainedNames(ctx context.Context, codec keys.SQLCodec, ba *kv.Batch) error
}

// CommentWriter encapsulates operations used to manipulate
// object comments.
type CommentWriter interface {
RemoveObjectComments(ctx context.Context, id descpb.ID) error
}

// Catalog encapsulates the logic to modify descriptors,
// namespaces, comments to help support schema changer mutations.
type Catalog interface {
DescriptorReader
NamespaceWriter
CommentWriter
}

// MutationJobs encapsulates the logic to create different types
// of jobs.
type MutationJobs interface {
Expand All @@ -50,19 +68,19 @@ type MutationJobs interface {
}

// NewMutationVisitor creates a new scop.MutationVisitor.
func NewMutationVisitor(descs MutableDescGetter, jobs MutationJobs) scop.MutationVisitor {
return &visitor{descs: descs, jobs: jobs}
func NewMutationVisitor(catalog Catalog, jobs MutationJobs) scop.MutationVisitor {
return &visitor{catalog: catalog, jobs: jobs}
}

type visitor struct {
descs MutableDescGetter
jobs MutationJobs
catalog Catalog
jobs MutationJobs
}

func (m *visitor) MakeAddedColumnDeleteAndWriteOnly(
ctx context.Context, op scop.MakeAddedColumnDeleteAndWriteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -77,7 +95,7 @@ func (m *visitor) MakeAddedColumnDeleteAndWriteOnly(

func (m *visitor) UpdateRelationDeps(ctx context.Context, op scop.UpdateRelationDeps) error {
// TODO(fqazi): Only implemented for sequences.
tableDesc, err := m.descs.GetMutableTableByID(ctx, op.TableID)
tableDesc, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand Down Expand Up @@ -124,7 +142,7 @@ func (m *visitor) RemoveColumnDefaultExpression(
ctx context.Context, op scop.RemoveColumnDefaultExpression,
) error {
// Remove the descriptors namespaces as the last stage
tableDesc, err := m.descs.GetMutableTableByID(ctx, op.TableID)
tableDesc, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -143,7 +161,7 @@ func (m *visitor) RemoveRelationDependedOnBy(
ctx context.Context, op scop.RemoveRelationDependedOnBy,
) error {
// Remove the descriptors namespaces as the last stage
tableDesc, err := m.descs.GetMutableTableByID(ctx, op.TableID)
tableDesc, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -157,7 +175,7 @@ func (m *visitor) RemoveRelationDependedOnBy(
}

func (m *visitor) RemoveSequenceOwnedBy(ctx context.Context, op scop.RemoveSequenceOwnedBy) error {
mutDesc, err := m.descs.GetMutableTableByID(ctx, op.TableID)
mutDesc, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -167,7 +185,7 @@ func (m *visitor) RemoveSequenceOwnedBy(ctx context.Context, op scop.RemoveSeque
}

func (m *visitor) RemoveTypeBackRef(ctx context.Context, op scop.RemoveTypeBackRef) error {
mutDesc, err := m.descs.GetMutableTypeByID(ctx, op.TypeID)
mutDesc, err := m.catalog.GetMutableTypeByID(ctx, op.TypeID)
if err != nil {
return err
}
Expand All @@ -194,7 +212,7 @@ func (m *visitor) CreateGcJobForDescriptor(
func (m *visitor) MarkDescriptorAsDropped(
ctx context.Context, op scop.MarkDescriptorAsDropped,
) error {
tableDesc, err := m.descs.GetMutableTableByID(ctx, op.TableID)
tableDesc, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -204,7 +222,7 @@ func (m *visitor) MarkDescriptorAsDropped(
}

func (m *visitor) DrainDescriptorName(ctx context.Context, op scop.DrainDescriptorName) error {
tableDesc, err := m.descs.GetMutableTableByID(ctx, op.TableID)
tableDesc, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -214,12 +232,12 @@ func (m *visitor) DrainDescriptorName(ctx context.Context, op scop.DrainDescript
ParentID: tableDesc.ParentID,
ParentSchemaID: parentSchemaID,
Name: tableDesc.Name}
m.descs.AddDrainedName(tableDesc.GetID(), nameDetails)
m.catalog.AddDrainedName(tableDesc.GetID(), nameDetails)
return nil
}

func (m *visitor) MakeColumnPublic(ctx context.Context, op scop.MakeColumnPublic) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -243,7 +261,7 @@ func (m *visitor) MakeColumnPublic(ctx context.Context, op scop.MakeColumnPublic
func (m *visitor) MakeDroppedNonPrimaryIndexDeleteAndWriteOnly(
ctx context.Context, op scop.MakeDroppedNonPrimaryIndexDeleteAndWriteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -266,7 +284,7 @@ func (m *visitor) MakeDroppedNonPrimaryIndexDeleteAndWriteOnly(
func (m *visitor) MakeDroppedColumnDeleteAndWriteOnly(
ctx context.Context, op scop.MakeDroppedColumnDeleteAndWriteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -289,7 +307,7 @@ func (m *visitor) MakeDroppedColumnDeleteAndWriteOnly(
func (m *visitor) MakeDroppedColumnDeleteOnly(
ctx context.Context, op scop.MakeDroppedColumnDeleteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -303,7 +321,7 @@ func (m *visitor) MakeDroppedColumnDeleteOnly(
}

func (m *visitor) MakeColumnAbsent(ctx context.Context, op scop.MakeColumnAbsent) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -324,7 +342,7 @@ func (m *visitor) MakeColumnAbsent(ctx context.Context, op scop.MakeColumnAbsent
func (m *visitor) MakeAddedIndexDeleteAndWriteOnly(
ctx context.Context, op scop.MakeAddedIndexDeleteAndWriteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -340,7 +358,7 @@ func (m *visitor) MakeAddedIndexDeleteAndWriteOnly(
func (m *visitor) MakeAddedColumnDeleteOnly(
ctx context.Context, op scop.MakeAddedColumnDeleteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand Down Expand Up @@ -380,7 +398,7 @@ func (m *visitor) MakeAddedColumnDeleteOnly(
func (m *visitor) MakeDroppedIndexDeleteOnly(
ctx context.Context, op scop.MakeDroppedIndexDeleteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -396,7 +414,7 @@ func (m *visitor) MakeDroppedIndexDeleteOnly(
func (m *visitor) MakeDroppedPrimaryIndexDeleteAndWriteOnly(
ctx context.Context, op scop.MakeDroppedPrimaryIndexDeleteAndWriteOnly,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -415,7 +433,7 @@ func (m *visitor) MakeAddedIndexDeleteOnly(
ctx context.Context, op scop.MakeAddedIndexDeleteOnly,
) error {

table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -432,7 +450,7 @@ func (m *visitor) MakeAddedIndexDeleteOnly(
}

func (m *visitor) AddCheckConstraint(ctx context.Context, op scop.AddCheckConstraint) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -454,7 +472,7 @@ func (m *visitor) AddCheckConstraint(ctx context.Context, op scop.AddCheckConstr
func (m *visitor) MakeAddedPrimaryIndexPublic(
ctx context.Context, op scop.MakeAddedPrimaryIndexPublic,
) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -471,7 +489,7 @@ func (m *visitor) MakeAddedPrimaryIndexPublic(
}

func (m *visitor) MakeIndexAbsent(ctx context.Context, op scop.MakeIndexAbsent) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -484,7 +502,7 @@ func (m *visitor) MakeIndexAbsent(ctx context.Context, op scop.MakeIndexAbsent)
}

func (m *visitor) AddColumnFamily(ctx context.Context, op scop.AddColumnFamily) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand All @@ -496,7 +514,7 @@ func (m *visitor) AddColumnFamily(ctx context.Context, op scop.AddColumnFamily)
}

func (m *visitor) DropForeignKeyRef(ctx context.Context, op scop.DropForeignKeyRef) error {
table, err := m.descs.GetMutableTableByID(ctx, op.TableID)
table, err := m.catalog.GetMutableTableByID(ctx, op.TableID)
if err != nil {
return err
}
Expand Down

0 comments on commit f5e633f

Please sign in to comment.