Skip to content

Commit

Permalink
schemachanger: drop schedule ID on DROP TABLE
Browse files Browse the repository at this point in the history
This commit drops the schedule ID on the new schema changer during a
DROP TABLE.

Release note: None
  • Loading branch information
otan committed Feb 9, 2022
1 parent 8f34541 commit 5b564b9
Show file tree
Hide file tree
Showing 18 changed files with 147 additions and 12 deletions.
13 changes: 13 additions & 0 deletions pkg/sql/descmetadata/metadata_updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,3 +225,16 @@ func (mu metadataUpdater) DeleteDatabaseRoleSettings(
return descriptors.WriteDesc(ctx, false /*kvTrace*/, desc, txn)
})
}

// DeleteSchedule implement scexec.DescriptorMetadataUpdater.
func (mu metadataUpdater) DeleteSchedule(ctx context.Context, scheduleID int64) error {
_, err := mu.ie.ExecEx(
ctx,
"delete-schedule",
mu.txn,
sessiondata.InternalExecutorOverride{User: security.RootUserName()},
"DELETE FROM system.scheduled_jobs WHERE schedule_id = $1",
scheduleID,
)
return err
}
3 changes: 0 additions & 3 deletions pkg/sql/logictest/testdata/logic_test/row_level_ttl
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
# LogicTest: !local-declarative-schema
# TODO(#75428): fix DROP TABLE for declarative schema changer

statement error value of "ttl_expire_after" must be an interval
CREATE TABLE tbl (id INT PRIMARY KEY, text TEXT) WITH (ttl_expire_after = ' xx invalid interval xx')

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,5 +165,11 @@ func dropTableDependents(b BuildCtx, tbl catalog.TableDescriptor, behavior tree.
seq := c.MustReadTable(sequenceOwnedBy.SequenceID)
cleanSequenceOwnedBy(seq)
})
if ttl := tbl.GetRowLevelTTL(); ttl != nil {
b.EnqueueDrop(&scpb.RowLevelTTL{
TableID: tbl.GetID(),
RowLevelTTL: ttl,
})
}
}
}
6 changes: 6 additions & 0 deletions pkg/sql/schemachanger/scdeps/sctestdeps/test_deps.go
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,12 @@ func (s *TestState) DeleteDatabaseRoleSettings(
return nil
}

// DeleteSchedule implements scexec.DescriptorMetadataUpdater
func (s *TestState) DeleteSchedule(ctx context.Context, id int64) error {
s.LogSideEffectf("delete schedule ID %d", id)
return nil
}

// DescriptorMetadataUpdater implement scexec.Dependencies.
func (s *TestState) DescriptorMetadataUpdater(
ctx context.Context,
Expand Down
3 changes: 3 additions & 0 deletions pkg/sql/schemachanger/scexec/dependencies.go
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ type DescriptorMetadataUpdater interface {

// DeleteDatabaseRoleSettings deletes role settings associated with a database.
DeleteDatabaseRoleSettings(ctx context.Context, database catalog.DatabaseDescriptor) error

// DeleteSchedule deletes the given schedule.
DeleteSchedule(ctx context.Context, id int64) error
}

// DescriptorMetadataUpdaterFactory is used to construct a DescriptorMetadataUpdater for a given
Expand Down
22 changes: 16 additions & 6 deletions pkg/sql/schemachanger/scexec/exec_mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,35 +138,35 @@ func executeDescriptorMutationOps(ctx context.Context, deps Dependencies, ops []
}
}
}
commentUpdater := deps.DescriptorMetadataUpdater(ctx)
metadataUpdater := deps.DescriptorMetadataUpdater(ctx)
for _, comment := range mvs.commentsToUpdate {
if len(comment.comment) > 0 {
if err := commentUpdater.UpsertDescriptorComment(
if err := metadataUpdater.UpsertDescriptorComment(
comment.id, comment.subID, comment.commentType, comment.comment); err != nil {
return err
}
} else {
if err := commentUpdater.DeleteDescriptorComment(
if err := metadataUpdater.DeleteDescriptorComment(
comment.id, comment.subID, comment.commentType); err != nil {
return err
}
}
}
for _, comment := range mvs.constraintCommentsToUpdate {
if len(comment.comment) > 0 {
if err := commentUpdater.UpsertConstraintComment(
if err := metadataUpdater.UpsertConstraintComment(
comment.tbl, comment.schemaName, comment.constraintName, comment.constraintType, comment.comment); err != nil {
return err
}
} else {
if err := commentUpdater.DeleteConstraintComment(
if err := metadataUpdater.DeleteConstraintComment(
comment.tbl, comment.schemaName, comment.constraintName, comment.constraintType); err != nil {
return err
}
}
}
for _, dbRoleSetting := range mvs.databaseRoleSettingsToDelete {
err := commentUpdater.DeleteDatabaseRoleSettings(ctx, dbRoleSetting.database)
err := metadataUpdater.DeleteDatabaseRoleSettings(ctx, dbRoleSetting.database)
if err != nil {
return err
}
Expand All @@ -191,6 +191,11 @@ func executeDescriptorMutationOps(ctx context.Context, deps Dependencies, ops []
return err
}
}
for _, scheduleID := range mvs.scheduleIDsToDelete {
if err := metadataUpdater.DeleteSchedule(ctx, scheduleID); err != nil {
return err
}
}
return b.ValidateAndRun(ctx)
}

Expand Down Expand Up @@ -290,6 +295,7 @@ type mutationVisitorState struct {
schemaChangerJob *jobs.Record
schemaChangerJobUpdates map[jobspb.JobID]schemaChangerJobUpdate
eventsByStatement map[uint32][]eventPayload
scheduleIDsToDelete []int64
}

type constraintCommentToUpdate struct {
Expand Down Expand Up @@ -412,6 +418,10 @@ func (mvs *mutationVisitorState) DeleteDatabaseRoleSettings(
return nil
}

func (mvs *mutationVisitorState) DeleteSchedule(scheduleID int64) {
mvs.scheduleIDsToDelete = append(mvs.scheduleIDsToDelete, scheduleID)
}

func (mvs *mutationVisitorState) AddDrainedName(id descpb.ID, nameInfo descpb.NameInfo) {
if _, ok := mvs.drainedNames[id]; !ok {
mvs.drainedNames[id] = []descpb.NameInfo{nameInfo}
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/schemachanger/scexec/executor_external_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,11 @@ func (noopMetadataUpdater) DeleteDatabaseRoleSettings(
return nil
}

// DeleteScheduleID implements scexec.DescriptorMetadataUpdater
func (noopMetadataUpdater) DeleteSchedule(ctx context.Context, scheduleID int64) error {
return nil
}

var _ scexec.Backfiller = noopBackfiller{}
var _ scexec.IndexValidator = noopIndexValidator{}
var _ scmutationexec.Partitioner = noopPartitioner{}
Expand Down
8 changes: 8 additions & 0 deletions pkg/sql/schemachanger/scexec/scmutationexec/scmutationexec.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ type MutationVisitorStateUpdater interface {
db catalog.DatabaseDescriptor,
) error

// DeleteSchedule deletes a scheduled job.
DeleteSchedule(scheduleID int64)

// AddNewGCJobForTable enqueues a GC job for the given table.
AddNewGCJobForTable(descriptor catalog.TableDescriptor)

Expand Down Expand Up @@ -1090,4 +1093,9 @@ func (m *visitor) RemoveDatabaseRoleSettings(
return m.s.DeleteDatabaseRoleSettings(ctx, db.(catalog.DatabaseDescriptor))
}

func (m *visitor) DeleteSchedule(ctx context.Context, op scop.DeleteSchedule) error {
m.s.DeleteSchedule(op.ScheduleID)
return nil
}

var _ scop.MutationVisitor = (*visitor)(nil)
6 changes: 6 additions & 0 deletions pkg/sql/schemachanger/scop/mutation.go
Original file line number Diff line number Diff line change
Expand Up @@ -413,3 +413,9 @@ type RemoveDatabaseRoleSettings struct {
mutationOp
DatabaseID descpb.ID
}

// DeleteSchedule is used to delete a schedule ID from the database.
type DeleteSchedule struct {
mutationOp
ScheduleID int64
}
6 changes: 6 additions & 0 deletions pkg/sql/schemachanger/scop/mutation_visitor_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 6 additions & 2 deletions pkg/sql/schemachanger/scpb/element_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,10 @@ func ForEach{{ . }}(
// the names of the types of its members.
func getElementNames(inProtoFile string) (names []string, _ error) {
var (
elementProtoBufMeta = `(\s+\[\([A-z\.]+\)\s+=\s+\"[A-z\:\",\s]+\])?`
// e.g.: (gogoproto.customname) = 'field'
elementProtoBufMetaField = `\([A-z\.]+\)\s+=\s+\"[A-z\:\",\s]+`
// e.g.: [ (gogoproto.a) = b, (gogoproto.customname) = 'c' ]
elementProtoBufMeta = `(\s+\[(` + elementProtoBufMetaField + `)*\](\s+,\s+(` + elementProtoBufMetaField + `))*)?`
elementFieldPat = `\s*(?P<type>\w+)\s+(?P<name>\w+)\s+=\s+\d+` +
elementProtoBufMeta + `;`
elementProtoRegexp = regexp.MustCompile(`(?s)message ElementProto {
Expand All @@ -109,7 +112,8 @@ func getElementNames(inProtoFile string) (names []string, _ error) {
}
submatch := elementProtoRegexp.FindSubmatch(got)
if submatch == nil {
return nil, fmt.Errorf("failed to find ElementProto in %s: %s",
return nil, fmt.Errorf(""+
"failed to find ElementProto in %s: %s",
inProtoFile, elementProtoRegexp)
}
fieldMatches := elementFieldRegexp.FindAllSubmatch(submatch[elementFieldsIdx], -1)
Expand Down
6 changes: 6 additions & 0 deletions pkg/sql/schemachanger/scpb/elements.proto
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ message ElementProto {
ColumnComment columnComment = 38 [(gogoproto.moretags) = "parent:\"Column\""];
ConstraintComment constraintComment = 39 [(gogoproto.moretags) = "parent:\"PrimaryIndex, SecondaryIndex, ForeignKey, UniqueConstraint, CheckConstraint\""];
DatabaseRoleSetting database_role_setting = 40 [(gogoproto.moretags) = "parent:\"Database\""];
RowLevelTTL row_level_ttl = 41 [(gogoproto.customname) = "RowLevelTTL", (gogoproto.moretags) = "parent:\"Table\""];
}

message Column {
Expand Down Expand Up @@ -321,6 +322,11 @@ message Locality {
cockroach.sql.catalog.catpb.LocalityConfig Locality = 2 [(gogoproto.customname) = "Locality", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb.LocalityConfig"];
}

message RowLevelTTL {
uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
cockroach.sql.catalog.catpb.RowLevelTTL row_level_ttl = 2 [(gogoproto.customname) = "RowLevelTTL"];
}

message ColumnName {
option (gogoproto.equal) = true;
uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"];
Expand Down
13 changes: 13 additions & 0 deletions pkg/sql/schemachanger/scpb/elements_generated.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions pkg/sql/schemachanger/scpb/uml/table.puml
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,11 @@ object DatabaseRoleSetting
DatabaseRoleSetting : DatabaseID
DatabaseRoleSetting : RoleName

object RowLevelTTL

RowLevelTTL : TableID
RowLevelTTL : RowLevelTTL

Table <|-- Column
Table <|-- PrimaryIndex
Table <|-- SecondaryIndex
Expand Down Expand Up @@ -333,4 +338,5 @@ ForeignKey <|-- ConstraintComment
UniqueConstraint <|-- ConstraintComment
CheckConstraint <|-- ConstraintComment
Database <|-- DatabaseRoleSetting
Table <|-- RowLevelTTL
@enduml
1 change: 1 addition & 0 deletions pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ go_library(
"opgen_partitioning.go",
"opgen_primary_index.go",
"opgen_relation_depended_on_by.go",
"opgen_row_level_ttl.go",
"opgen_schema.go",
"opgen_schema_comment.go",
"opgen_secondary_index.go",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ func init() {
minPhase(scop.PreCommitPhase),
revertible(false),
emit(func(this *scpb.RelationDependedOnBy) scop.Op {
return &scop.RemoveRelationDependedOnBy{
return &scop.
RemoveRelationDependedOnBy{
TableID: this.TableID,
DependedOnBy: this.DependedOnBy,
}
Expand Down
41 changes: 41 additions & 0 deletions pkg/sql/schemachanger/scplan/internal/opgen/opgen_row_level_ttl.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// Copyright 2022 The Cockroach Authors.
//
// Use of this software is governed by the Business Source License
// included in the file licenses/BSL.txt.
//
// As of the Change Date specified in that file, in accordance with
// the Business Source License, use of this software will be governed
// by the Apache License, Version 2.0, included in the file
// licenses/APL.txt.

package opgen

import (
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
)

func init() {
opRegistry.register((*scpb.RowLevelTTL)(nil),
toPublic(
scpb.Status_ABSENT,
to(scpb.Status_PUBLIC,
emit(func(this *scpb.RowLevelTTL) scop.Op {
return notImplemented(this)
}),
),
),
toAbsent(
scpb.Status_PUBLIC,
to(scpb.Status_ABSENT,
minPhase(scop.PostCommitNonRevertiblePhase),
revertible(false),
emit(func(this *scpb.RowLevelTTL) scop.Op {
return &scop.DeleteSchedule{
ScheduleID: this.RowLevelTTL.ScheduleID,
}
}),
),
),
)
}
3 changes: 3 additions & 0 deletions pkg/sql/schemachanger/screl/attr.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ var Schema = rel.MustSchema("screl",
rel.EntityAttr(DescID, "DatabaseID"),
rel.EntityAttr(RoleName, "RoleName"),
),
rel.EntityMapping(t((*scpb.RowLevelTTL)(nil)),
rel.EntityAttr(DescID, "TableID"),
),
)

// JoinTargetNode generates a clause that joins the target and node vars
Expand Down

0 comments on commit 5b564b9

Please sign in to comment.