Skip to content

Commit

Permalink
sql: clean up drop table / sequence inside new schema changer
Browse files Browse the repository at this point in the history 
Previous, drop sequence inside the new schema changer used
to refer to descriptor ID's as table_id's when it would be
cleaner to refer to sequence ID. Also clean up the code for
drop table at the same time. This isn't a functional change
and only cleans up the code to be more readable. These changes
also removed things from schema change execution interfaces
that shouldn't be shared across boundaries for schema change
execution.

Release note: None.
  • Loading branch information
@fqazi
fqazi committed Jun 7, 2021
1 parent f5e633f commit 4515923
Show file tree
Hide file tree
Showing 9 changed files with 319 additions and 305 deletions.
20 changes: 11 additions & 9 deletions pkg/sql/schemachanger/scbuild/sequence.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,15 @@ import (

// dropSequenceDesc builds targets and transformations using a descriptor.
func (b *buildContext) dropSequenceDesc(
ctx context.Context, table catalog.TableDescriptor, cascade tree.DropBehavior,
ctx context.Context, seq catalog.TableDescriptor, cascade tree.DropBehavior,
) {
// Check if there are dependencies.
err := table.ForeachDependedOnBy(func(dep *descpb.TableDescriptor_Reference) error {
err := seq.ForeachDependedOnBy(func(dep *descpb.TableDescriptor_Reference) error {
if cascade != tree.DropCascade {
return pgerror.Newf(
pgcode.DependentObjectsStillExist,
"cannot drop sequence %s because other objects depend on it",
table.GetName(),
seq.GetName(),
)
}
desc, err := b.Descs.GetImmutableTableByID(ctx, b.EvalCtx.Txn, dep.ID, tree.ObjectLookupFlagsWithRequired())
Expand Down Expand Up @@ -62,15 +62,17 @@ func (b *buildContext) dropSequenceDesc(
}

// Add a node to drop the sequence
sequenceNode := &scpb.Sequence{TableID: table.GetID()}
sequenceNode := &scpb.Sequence{SequenceID: seq.GetID()}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, sequenceNode); !exists {
b.addNode(scpb.Target_DROP, sequenceNode)
}
sequenceOwnedBy := &scpb.SequenceOwnedBy{
TableID: table.GetID(),
OwnerTableID: table.GetSequenceOpts().SequenceOwner.OwnerTableID}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, sequenceOwnedBy); !exists {
b.addNode(scpb.Target_DROP, sequenceOwnedBy)
if seq.GetSequenceOpts().SequenceOwner.OwnerTableID != descpb.InvalidID {
sequenceOwnedBy := &scpb.SequenceOwnedBy{
SequenceID: seq.GetID(),
OwnerTableID: seq.GetSequenceOpts().SequenceOwner.OwnerTableID}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, sequenceOwnedBy); !exists {
b.addNode(scpb.Target_DROP, sequenceOwnedBy)
}
}
}

Expand Down
306 changes: 167 additions & 139 deletions pkg/sql/schemachanger/scbuild/table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -574,160 +574,188 @@ func isTypeSupportedInVersion(v clusterversion.ClusterVersion, t *types.T) bool
return v.IsActive(minVersion)
}

func (b *buildContext) dropTable(ctx context.Context, n *tree.DropTable) {
// Find the table first.
for _, name := range n.Names {
table, err := resolver.ResolveExistingTableObject(ctx, b.Res, &name,
tree.ObjectLookupFlagsWithRequired())
if err != nil {
if errors.Is(err, catalog.ErrDescriptorNotFound) && n.IfExists {
return
}
panic(err)
}
if table == nil {
panic(errors.AssertionFailedf("Unable to resolve table %s",
name.FQString()))
}
// Interleaved tables not supported in new schema changer.
if table.IsInterleaved() {
panic(&notImplementedError{n: n, detail: "drop on interleaved table"})
}

// Drop dependent views
err = table.ForeachDependedOnBy(func(dep *descpb.TableDescriptor_Reference) error {
dependentDesc, err := b.Descs.GetImmutableTableByID(ctx, b.EvalCtx.Txn, dep.ID, tree.ObjectLookupFlagsWithRequired())
func (b *buildContext) maybeCleanTableSequenceRefs(
ctx context.Context, table catalog.TableDescriptor, behavior tree.DropBehavior,
) {
// Setup nodes for dropping sequences
// and cleaning up default expressions.
for _, col := range table.PublicColumns() {
// Loop over owned sequences
for seqIdx := 0; seqIdx < col.NumOwnsSequences(); seqIdx++ {
seqID := col.GetOwnsSequenceID(seqIdx)
table, err := b.Descs.GetMutableTableByID(ctx, b.EvalCtx.Txn, seqID, tree.ObjectLookupFlagsWithRequiredTableKind(tree.ResolveRequireSequenceDesc))
if err != nil {
panic(err)
}
if n.DropBehavior != tree.DropCascade {
return pgerror.Newf(
pgcode.DependentObjectsStillExist, "cannot drop table %q because view %q depends on it",
table.GetName(), dependentDesc.GetName())
if behavior != tree.DropCascade {
panic(pgerror.Newf(
pgcode.DependentObjectsStillExist,
"cannot drop table %s because other objects depend on it",
table.GetName(),
))
}
err = b.AuthAccessor.CheckPrivilege(ctx, dependentDesc, privilege.DROP)
err = b.AuthAccessor.CheckPrivilege(ctx, table, privilege.DROP)
if err != nil {
panic(err)
}
b.maybeDropViewAndDependents(ctx, dependentDesc, n.DropBehavior)
return nil
})
b.dropSequenceDesc(ctx, table, tree.DropCascade)
}
// Setup logic to clean up the default expression,
// only if sequences are depending on it.
if col.NumUsesSequences() > 0 {
b.addNode(scpb.Target_DROP,
&scpb.DefaultExpression{
DefaultExpr: col.GetDefaultExpr(),
TableID: table.GetID(),
UsesSequenceIDs: col.ColumnDesc().UsesSequenceIds,
ColumnID: col.GetID()})
// Drop the depends on within the sequence side.
for seqOrd := 0; seqOrd < col.NumUsesSequences(); seqOrd++ {
seqID := col.GetUsesSequenceID(seqOrd)
// Remove dependencies to this sequences.
dropDep := &scpb.RelationDependedOnBy{TableID: seqID,
DependedOnBy: table.GetID()}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, dropDep); !exists {
b.addNode(scpb.Target_DROP, dropDep)
}
}
}
}
}

func (b *buildContext) maybeCleanTableFKs(
ctx context.Context, table catalog.TableDescriptor, behavior tree.DropBehavior,
) {
// Loop through and update inbound and outbound
// foreign key references.
for _, fk := range table.GetInboundFKs() {
dependentTable, err := b.Descs.GetImmutableTableByID(ctx, b.EvalCtx.Txn, fk.OriginTableID, tree.ObjectLookupFlagsWithRequired())
if err != nil {
panic(err)
}
if behavior != tree.DropCascade {
panic(pgerror.Newf(
pgcode.DependentObjectsStillExist,
"%q is referenced by foreign key from table %q", fk.Name, dependentTable.GetName()))
}
err = b.AuthAccessor.CheckPrivilege(ctx, dependentTable, privilege.DROP)
if err != nil {
panic(err)
}
outFkNode := &scpb.OutboundForeignKey{
OriginID: fk.OriginTableID,
OriginColumns: fk.OriginColumnIDs,
ReferenceID: fk.ReferencedTableID,
ReferenceColumns: fk.ReferencedColumnIDs,
Name: fk.Name,
}
inFkNode := &scpb.InboundForeignKey{
OriginID: fk.ReferencedTableID,
OriginColumns: fk.ReferencedColumnIDs,
ReferenceID: fk.OriginTableID,
ReferenceColumns: fk.OriginColumnIDs,
Name: fk.Name,
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, outFkNode); !exists {
b.addNode(scpb.Target_DROP,
outFkNode)
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, inFkNode); !exists {
b.addNode(scpb.Target_DROP,
inFkNode)
}
}

// Loop through and update inbound and outbound
// foreign key references.
for _, fk := range table.GetInboundFKs() {
dependentTable, err := b.Descs.GetImmutableTableByID(ctx, b.EvalCtx.Txn, fk.OriginTableID, tree.ObjectLookupFlagsWithRequired())
if err != nil {
panic(err)
}
if n.DropBehavior != tree.DropCascade {
panic(pgerror.Newf(
pgcode.DependentObjectsStillExist,
"%q is referenced by foreign key from table %q", fk.Name, dependentTable.GetName()))
}
err = b.AuthAccessor.CheckPrivilege(ctx, dependentTable, privilege.DROP)
if err != nil {
panic(err)
}
outFkNode := &scpb.OutboundForeignKey{
OriginID: fk.OriginTableID,
OriginColumns: fk.OriginColumnIDs,
ReferenceID: fk.ReferencedTableID,
ReferenceColumns: fk.ReferencedColumnIDs,
Name: fk.Name,
}
inFkNode := &scpb.InboundForeignKey{
OriginID: fk.ReferencedTableID,
OriginColumns: fk.ReferencedColumnIDs,
ReferenceID: fk.OriginTableID,
ReferenceColumns: fk.OriginColumnIDs,
Name: fk.Name,
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, outFkNode); !exists {
b.addNode(scpb.Target_DROP,
outFkNode)
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, inFkNode); !exists {
b.addNode(scpb.Target_DROP,
inFkNode)
}
for _, fk := range table.GetOutboundFKs() {
outFkNode := &scpb.OutboundForeignKey{
OriginID: fk.OriginTableID,
OriginColumns: fk.OriginColumnIDs,
ReferenceID: fk.ReferencedTableID,
ReferenceColumns: fk.ReferencedColumnIDs,
Name: fk.Name,
}
inFkNode := &scpb.InboundForeignKey{
OriginID: fk.ReferencedTableID,
OriginColumns: fk.ReferencedColumnIDs,
ReferenceID: fk.OriginTableID,
ReferenceColumns: fk.OriginColumnIDs,
Name: fk.Name,
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, outFkNode); !exists {
b.addNode(scpb.Target_DROP,
outFkNode)
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, inFkNode); !exists {
b.addNode(scpb.Target_DROP,
inFkNode)
}
}
}

for _, fk := range table.GetOutboundFKs() {
outFkNode := &scpb.OutboundForeignKey{
OriginID: fk.OriginTableID,
OriginColumns: fk.OriginColumnIDs,
ReferenceID: fk.ReferencedTableID,
ReferenceColumns: fk.ReferencedColumnIDs,
Name: fk.Name,
}
inFkNode := &scpb.InboundForeignKey{
OriginID: fk.ReferencedTableID,
OriginColumns: fk.ReferencedColumnIDs,
ReferenceID: fk.OriginTableID,
ReferenceColumns: fk.OriginColumnIDs,
Name: fk.Name,
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, outFkNode); !exists {
b.addNode(scpb.Target_DROP,
outFkNode)
}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, inFkNode); !exists {
b.addNode(scpb.Target_DROP,
inFkNode)
}
func (b *buildContext) dropTableDesc(
ctx context.Context, table catalog.TableDescriptor, behavior tree.DropBehavior,
) {
// Interleaved tables not supported in new schema changer.
if table.IsInterleaved() {
panic(&notImplementedError{
n: &tree.DropTable{
Names: []tree.TableName{
tree.MakeUnqualifiedTableName(tree.Name(table.GetName())),
},
},
detail: "drop on interleaved table"})
}

// Drop dependent views
err := table.ForeachDependedOnBy(func(dep *descpb.TableDescriptor_Reference) error {
dependentDesc, err := b.Descs.GetImmutableTableByID(ctx, b.EvalCtx.Txn, dep.ID, tree.ObjectLookupFlagsWithRequired())
if err != nil {
panic(err)
}
// Setup nodes for dropping sequences
// and cleaning up default expressions.
for _, col := range table.PublicColumns() {
// Loop over owned sequences
for seqIdx := 0; seqIdx < col.NumOwnsSequences(); seqIdx++ {
seqID := col.GetOwnsSequenceID(seqIdx)
table, err := b.Descs.GetMutableTableByID(ctx, b.EvalCtx.Txn, seqID, tree.ObjectLookupFlagsWithRequiredTableKind(tree.ResolveRequireSequenceDesc))
if err != nil {
panic(err)
}
if n.DropBehavior != tree.DropCascade {
panic(pgerror.Newf(
pgcode.DependentObjectsStillExist,
"cannot drop table %s because other objects depend on it",
table.GetName(),
))
}
err = b.AuthAccessor.CheckPrivilege(ctx, table, privilege.DROP)
if err != nil {
panic(err)
}
b.dropSequenceDesc(ctx, table, tree.DropCascade)
}
// Setup logic to clean up the default expression,
// only if sequences are depending on it.
if col.NumUsesSequences() > 0 {
b.addNode(scpb.Target_DROP,
&scpb.DefaultExpression{
DefaultExpr: col.GetDefaultExpr(),
TableID: table.GetID(),
UsesSequenceIDs: col.ColumnDesc().UsesSequenceIds,
ColumnID: col.GetID()})
// Drop the depends on within the sequence side.
for seqOrd := 0; seqOrd < col.NumUsesSequences(); seqOrd++ {
seqID := col.GetUsesSequenceID(seqOrd)
// Remove dependencies to this sequences.
dropDep := &scpb.RelationDependedOnBy{TableID: seqID,
DependedOnBy: table.GetID()}
if exists, _ := b.checkIfNodeExists(scpb.Target_DROP, dropDep); !exists {
b.addNode(scpb.Target_DROP, dropDep)
}
}
}
if behavior != tree.DropCascade {
return pgerror.Newf(
pgcode.DependentObjectsStillExist, "cannot drop table %q because view %q depends on it",
table.GetName(), dependentDesc.GetName())
}
err = b.AuthAccessor.CheckPrivilege(ctx, dependentDesc, privilege.DROP)
if err != nil {
panic(err)
}
b.maybeDropViewAndDependents(ctx, dependentDesc, behavior)
return nil
})
if err != nil {
panic(err)
}

// Clean up foreign key references (both inbound
// and out bound).
b.maybeCleanTableFKs(ctx, table, behavior)

// Clean up type back references
b.removeTypeBackRefDeps(ctx, table)
b.addNode(scpb.Target_DROP,
&scpb.Table{TableID: table.GetID()})
// Clean up sequence references and ownerships.
b.maybeCleanTableSequenceRefs(ctx, table, behavior)

// Clean up type back references
b.removeTypeBackRefDeps(ctx, table)
b.addNode(scpb.Target_DROP,
&scpb.Table{TableID: table.GetID()})
}

func (b *buildContext) dropTable(ctx context.Context, n *tree.DropTable) {
// Find the table first.
for _, name := range n.Names {
table, err := resolver.ResolveExistingTableObject(ctx, b.Res, &name,
tree.ObjectLookupFlagsWithRequired())
if err != nil {
if errors.Is(err, catalog.ErrDescriptorNotFound) && n.IfExists {
return
}
panic(err)
}
if table == nil {
panic(errors.AssertionFailedf("Unable to resolve table %s",
name.FQString()))
}
b.dropTableDesc(ctx, table, n.DropBehavior)
}
}
16 changes: 2 additions & 14 deletions pkg/sql/schemachanger/scbuild/testdata/drop_sequence
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,7 @@ DROP SEQUENCE defaultdb.SQ1 CASCADE
direction: DROP
elementProto:
sequence:
tableId: 52
state: PUBLIC
- target:
direction: DROP
elementProto:
sequenceOwner:
tableId: 52
sequenceId: 52
state: PUBLIC

create-table
Expand Down Expand Up @@ -51,11 +45,5 @@ DROP SEQUENCE defaultdb.SQ1 CASCADE
direction: DROP
elementProto:
sequence:
tableId: 52
state: PUBLIC
- target:
direction: DROP
elementProto:
sequenceOwner:
tableId: 52
sequenceId: 52
state: PUBLIC
Loading

0 comments on commit 4515923

Please sign in to comment.