From 2bcab3c5ea88e42529cd79355e0e46271d577827 Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Wed, 1 Feb 2023 17:17:46 -0500 Subject: [PATCH] schemachanger: Implement `ADD FOREIGN KEY NOT VALID` and add tests --- .../backup_base_generated_test.go | 5 + .../logictest/testdata/logic_test/drop_table | 9 ++ .../internal/scbuildstmt/alter_table.go | 5 +- .../scbuildstmt/alter_table_add_constraint.go | 47 ++++--- .../testdata/alter_table_add_foreign_key | 12 ++ .../alter_table_add_foreign_key_not_valid | 12 ++ pkg/sql/schemachanger/scdecomp/decomp.go | 34 +++-- pkg/sql/schemachanger/scdecomp/testdata/table | 18 +++ .../scexec/scmutationexec/constraint.go | 34 +++++ .../schemachanger/scop/immediate_mutation.go | 14 ++ .../immediate_mutation_visitor_generated.go | 6 + pkg/sql/schemachanger/scpb/elements.proto | 12 ++ .../schemachanger/scpb/elements_generated.go | 31 +++++ pkg/sql/schemachanger/scpb/uml/table.puml | 12 ++ .../scplan/internal/opgen/BUILD.bazel | 1 + .../opgen_foreign_key_constraint_not_valid.go | 58 ++++++++ .../internal/rules/current/testdata/deprules | 18 +-- .../internal/rules/current/testdata/oprules | 2 +- pkg/sql/schemachanger/screl/attr.go | 5 + pkg/sql/schemachanger/screl/scalars.go | 3 +- .../schemachanger/sctest_generated_test.go | 25 ++++ .../alter_table_add_foreign_key_not_valid | 130 ++++++++++++++++++ .../alter_table_add_foreign_key_not_valid | 31 +++++ .../alter_table_add_foreign_key_not_valid | 86 ++++++++++++ 24 files changed, 569 insertions(+), 41 deletions(-) create mode 100644 pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key create mode 100644 pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key_not_valid create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_not_valid.go create mode 100644 pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid create mode 100644 pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key_not_valid create mode 100644 pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key_not_valid diff --git a/pkg/ccl/schemachangerccl/backup_base_generated_test.go b/pkg/ccl/schemachangerccl/backup_base_generated_test.go index 23cdcc068386..32da5c59d0b6 100644 --- a/pkg/ccl/schemachangerccl/backup_base_generated_test.go +++ b/pkg/ccl/schemachangerccl/backup_base_generated_test.go @@ -58,6 +58,11 @@ func TestBackup_base_alter_table_add_foreign_key(t *testing.T) { defer log.Scope(t).Close(t) sctest.Backup(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key", newCluster) } +func TestBackup_base_alter_table_add_foreign_key_not_valid(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.Backup(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid", newCluster) +} func TestBackup_base_alter_table_add_primary_key_drop_rowid(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) diff --git a/pkg/sql/logictest/testdata/logic_test/drop_table b/pkg/sql/logictest/testdata/logic_test/drop_table index f2b4c3e20366..d1a43b524334 100644 --- a/pkg/sql/logictest/testdata/logic_test/drop_table +++ b/pkg/sql/logictest/testdata/logic_test/drop_table @@ -120,6 +120,9 @@ subtest drop_table_with_not_valid_constraints statement ok CREATE TABLE t_with_not_valid_constraints_1 (i INT PRIMARY KEY, j INT); +statement ok +CREATE TABLE t_with_not_valid_constraints_2 (i INT PRIMARY KEY, j INT); + statement ok ALTER TABLE t_with_not_valid_constraints_1 ADD CHECK (i > 0) NOT VALID; @@ -127,5 +130,11 @@ statement ok SET experimental_enable_unique_without_index_constraints = true; ALTER TABLE t_with_not_valid_constraints_1 ADD UNIQUE WITHOUT INDEX (j) NOT VALID; +statement ok +ALTER TABLE t_with_not_valid_constraints_1 ADD FOREIGN KEY (i) REFERENCES t_with_not_valid_constraints_2 (i) NOT VALID; + statement ok DROP TABLE t_with_not_valid_constraints_1; + +statement ok +DROP TABLE t_with_not_valid_constraints_2; diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go index 63aaffd5d97c..8dd1b4726c91 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go @@ -65,8 +65,8 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{ return true } - // Support ALTER TABLE ... ADD CONSTRAINT FOREIGN KEY - if _, ok := t.ConstraintDef.(*tree.ForeignKeyConstraintTableDef); ok && t.ValidationBehavior == tree.ValidationDefault { + // Support ALTER TABLE ... ADD CONSTRAINT FOREIGN KEY [NOT VALID] + if _, ok := t.ConstraintDef.(*tree.ForeignKeyConstraintTableDef); ok { return true } @@ -86,6 +86,7 @@ var alterTableAddConstraintMinSupportedClusterVersion = map[string]clusterversio "ADD_UNIQUE_WITHOUT_INDEX_DEFAULT": clusterversion.V23_1Start, "ADD_CHECK_SKIP": clusterversion.V23_1, "ADD_UNIQUE_WITHOUT_INDEX_SKIP": clusterversion.V23_1, + "ADD_FOREIGN_KEY_SKIP": clusterversion.V23_1, } func init() { diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_constraint.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_constraint.go index 658ba72282a4..2cef85e8c07a 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_constraint.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table_add_constraint.go @@ -47,9 +47,7 @@ func alterTableAddConstraint( case *tree.CheckConstraintTableDef: alterTableAddCheck(b, tn, tbl, t) case *tree.ForeignKeyConstraintTableDef: - if t.ValidationBehavior == tree.ValidationDefault { - alterTableAddForeignKey(b, tn, tbl, t) - } + alterTableAddForeignKey(b, tn, tbl, t) } } @@ -173,7 +171,7 @@ func getIndexIDForValidationForConstraint(b BuildCtx, tableID catid.DescID) (ret } // alterTableAddForeignKey contains logic for building -// `ALTER TABLE ... ADD CONSTRAINT ... FOREIGN KEY`. +// `ALTER TABLE ... ADD FOREIGN KEY ... [NOT VALID]`. // It assumes `t` is such a command. func alterTableAddForeignKey( b BuildCtx, tn *tree.TableName, tbl *scpb.Table, t *tree.AlterTableAddConstraint, @@ -369,19 +367,34 @@ func alterTableAddForeignKey( // 12. (Finally!) Add a ForeignKey_Constraint, ConstraintName element to // builder state. constraintID := b.NextTableConstraintID(tbl.TableID) - fk := &scpb.ForeignKeyConstraint{ - TableID: tbl.TableID, - ConstraintID: constraintID, - ColumnIDs: originColIDs, - ReferencedTableID: referencedTableID, - ReferencedColumnIDs: referencedColIDs, - OnUpdateAction: tree.ForeignKeyReferenceActionValue[fkDef.Actions.Update], - OnDeleteAction: tree.ForeignKeyReferenceActionValue[fkDef.Actions.Delete], - CompositeKeyMatchMethod: tree.CompositeKeyMatchMethodValue[fkDef.Match], - IndexIDForValidation: getIndexIDForValidationForConstraint(b, tbl.TableID), - } - b.Add(fk) - b.LogEventForExistingTarget(fk) + if t.ValidationBehavior == tree.ValidationDefault { + fk := &scpb.ForeignKeyConstraint{ + TableID: tbl.TableID, + ConstraintID: constraintID, + ColumnIDs: originColIDs, + ReferencedTableID: referencedTableID, + ReferencedColumnIDs: referencedColIDs, + OnUpdateAction: tree.ForeignKeyReferenceActionValue[fkDef.Actions.Update], + OnDeleteAction: tree.ForeignKeyReferenceActionValue[fkDef.Actions.Delete], + CompositeKeyMatchMethod: tree.CompositeKeyMatchMethodValue[fkDef.Match], + IndexIDForValidation: getIndexIDForValidationForConstraint(b, tbl.TableID), + } + b.Add(fk) + b.LogEventForExistingTarget(fk) + } else { + fk := &scpb.ForeignKeyConstraintNotValid{ + TableID: tbl.TableID, + ConstraintID: constraintID, + ColumnIDs: originColIDs, + ReferencedTableID: referencedTableID, + ReferencedColumnIDs: referencedColIDs, + OnUpdateAction: tree.ForeignKeyReferenceActionValue[fkDef.Actions.Update], + OnDeleteAction: tree.ForeignKeyReferenceActionValue[fkDef.Actions.Delete], + CompositeKeyMatchMethod: tree.CompositeKeyMatchMethodValue[fkDef.Match], + } + b.Add(fk) + b.LogEventForExistingTarget(fk) + } b.Add(&scpb.ConstraintWithoutIndexName{ TableID: tbl.TableID, ConstraintID: constraintID, diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key new file mode 100644 index 000000000000..aa77fd74a243 --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key @@ -0,0 +1,12 @@ +setup +CREATE TABLE t2 (i INT PRIMARY KEY); +CREATE TABLE t1 (i INT PRIMARY KEY); +---- + +build +ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i); +---- +- [[ForeignKeyConstraint:{DescID: 105, IndexID: 0, ConstraintID: 2, ReferencedDescID: 104}, PUBLIC], ABSENT] + {columnIds: [1], constraintId: 2, referencedColumnIds: [1], referencedTableId: 104, tableId: 105} +- [[ConstraintWithoutIndexName:{DescID: 105, Name: t1_i_fkey, ConstraintID: 2}, PUBLIC], ABSENT] + {constraintId: 2, name: t1_i_fkey, tableId: 105} diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key_not_valid b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key_not_valid new file mode 100644 index 000000000000..f31ada8f8da2 --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key_not_valid @@ -0,0 +1,12 @@ +setup +CREATE TABLE t2 (i INT PRIMARY KEY); +CREATE TABLE t1 (i INT PRIMARY KEY); +---- + +build +ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i) NOT VALID; +---- +- [[ForeignKeyConstraintNotValid:{DescID: 105, ConstraintID: 2, ReferencedDescID: 104}, PUBLIC], ABSENT] + {columnIds: [1], constraintId: 2, referencedColumnIds: [1], referencedTableId: 104, tableId: 105} +- [[ConstraintWithoutIndexName:{DescID: 105, Name: t1_i_fkey, ConstraintID: 2}, PUBLIC], ABSENT] + {constraintId: 2, name: t1_i_fkey, tableId: 105} diff --git a/pkg/sql/schemachanger/scdecomp/decomp.go b/pkg/sql/schemachanger/scdecomp/decomp.go index 52725d01a20a..863702f91d5b 100644 --- a/pkg/sql/schemachanger/scdecomp/decomp.go +++ b/pkg/sql/schemachanger/scdecomp/decomp.go @@ -667,17 +667,29 @@ func (w *walkCtx) walkCheckConstraint(tbl catalog.TableDescriptor, c catalog.Che func (w *walkCtx) walkForeignKeyConstraint( tbl catalog.TableDescriptor, c catalog.ForeignKeyConstraint, ) { - // TODO(postamar): proper handling of constraint status - w.ev(scpb.Status_PUBLIC, &scpb.ForeignKeyConstraint{ - TableID: tbl.GetID(), - ConstraintID: c.GetConstraintID(), - ColumnIDs: c.ForeignKeyDesc().OriginColumnIDs, - ReferencedTableID: c.GetReferencedTableID(), - ReferencedColumnIDs: c.ForeignKeyDesc().ReferencedColumnIDs, - OnUpdateAction: c.OnUpdate(), - OnDeleteAction: c.OnDelete(), - CompositeKeyMatchMethod: c.Match(), - }) + if c.IsConstraintUnvalidated() && w.clusterVersion.IsActive(clusterversion.V23_1) { + w.ev(scpb.Status_PUBLIC, &scpb.ForeignKeyConstraintNotValid{ + TableID: tbl.GetID(), + ConstraintID: c.GetConstraintID(), + ColumnIDs: c.ForeignKeyDesc().OriginColumnIDs, + ReferencedTableID: c.GetReferencedTableID(), + ReferencedColumnIDs: c.ForeignKeyDesc().ReferencedColumnIDs, + OnUpdateAction: c.OnUpdate(), + OnDeleteAction: c.OnDelete(), + CompositeKeyMatchMethod: c.Match(), + }) + } else { + w.ev(scpb.Status_PUBLIC, &scpb.ForeignKeyConstraint{ + TableID: tbl.GetID(), + ConstraintID: c.GetConstraintID(), + ColumnIDs: c.ForeignKeyDesc().OriginColumnIDs, + ReferencedTableID: c.GetReferencedTableID(), + ReferencedColumnIDs: c.ForeignKeyDesc().ReferencedColumnIDs, + OnUpdateAction: c.OnUpdate(), + OnDeleteAction: c.OnDelete(), + CompositeKeyMatchMethod: c.Match(), + }) + } w.ev(scpb.Status_PUBLIC, &scpb.ConstraintWithoutIndexName{ TableID: tbl.GetID(), ConstraintID: c.GetConstraintID(), diff --git a/pkg/sql/schemachanger/scdecomp/testdata/table b/pkg/sql/schemachanger/scdecomp/testdata/table index 88c434f208a3..2ffac78cbfd2 100644 --- a/pkg/sql/schemachanger/scdecomp/testdata/table +++ b/pkg/sql/schemachanger/scdecomp/testdata/table @@ -18,6 +18,7 @@ COMMENT ON CONSTRAINT tbl_pkey ON tbl IS 'primary key'; ALTER TABLE tbl ADD CONSTRAINT mycheck2 CHECK (id < 10) NOT VALID; ALTER TABLE tbl ADD CONSTRAINT myuwi1 UNIQUE WITHOUT INDEX (price); ALTER TABLE tbl ADD CONSTRAINT myuwi2 UNIQUE WITHOUT INDEX (price) NOT VALID; +ALTER TABLE tbl ADD CONSTRAINT myfk2 FOREIGN KEY (id) REFERENCES parent (id) NOT VALID; ---- decompose @@ -378,6 +379,18 @@ ElementState: predicate: null tableId: 105 Status: PUBLIC +- ForeignKeyConstraintNotValid: + columnIds: + - 1 + compositeKeyMatchMethod: SIMPLE + constraintId: 7 + onDeleteAction: NO_ACTION + onUpdateAction: NO_ACTION + referencedColumnIds: + - 1 + referencedTableId: 104 + tableId: 105 + Status: PUBLIC - TableZoneConfig: tableId: 105 Status: PUBLIC @@ -678,6 +691,11 @@ ElementState: name: myuwi2 tableId: 105 Status: PUBLIC +- ConstraintWithoutIndexName: + constraintId: 7 + name: myfk2 + tableId: 105 + Status: PUBLIC - ConstraintComment: comment: must have a parent constraintId: 2 diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go b/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go index e6a9ea567b0f..c2921e602b7d 100644 --- a/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go +++ b/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go @@ -428,6 +428,40 @@ func (i *immediateVisitor) MakeAbsentForeignKeyConstraintWriteOnly( return nil } +func (i *immediateVisitor) MakeAbsentForeignKeyConstraintNotValidPublic( + ctx context.Context, op scop.MakeAbsentForeignKeyConstraintNotValidPublic, +) error { + out, err := i.checkOutTable(ctx, op.TableID) + if err != nil || out.Dropped() { + return err + } + if op.ConstraintID >= out.NextConstraintID { + out.NextConstraintID = op.ConstraintID + 1 + } + + // Enqueue a mutation in `out` to signal this mutation is now enforced. + fk := descpb.ForeignKeyConstraint{ + OriginTableID: op.TableID, + OriginColumnIDs: op.ColumnIDs, + ReferencedColumnIDs: op.ReferencedColumnIDs, + ReferencedTableID: op.ReferencedTableID, + Name: tabledesc.ConstraintNamePlaceholder(op.ConstraintID), + Validity: descpb.ConstraintValidity_Unvalidated, + OnDelete: op.OnDeleteAction, + OnUpdate: op.OnUpdateAction, + Match: op.CompositeKeyMatchMethod, + ConstraintID: op.ConstraintID, + } + out.OutboundFKs = append(out.OutboundFKs, fk) + // Add an entry in "InboundFKs" in the referenced table as company. + in, err := i.checkOutTable(ctx, op.ReferencedTableID) + if err != nil { + return err + } + in.InboundFKs = append(in.InboundFKs, fk) + return nil +} + func (i *immediateVisitor) MakeValidatedForeignKeyConstraintPublic( ctx context.Context, op scop.MakeValidatedForeignKeyConstraintPublic, ) error { diff --git a/pkg/sql/schemachanger/scop/immediate_mutation.go b/pkg/sql/schemachanger/scop/immediate_mutation.go index c76db2679f39..89b91983d551 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation.go @@ -341,6 +341,20 @@ type MakeAbsentForeignKeyConstraintWriteOnly struct { CompositeKeyMatchMethod semenumpb.Match } +// MakeAbsentForeignKeyConstraintNotValidPublic adds a non-existent, not-valid +// foreign key constraint to the table. +type MakeAbsentForeignKeyConstraintNotValidPublic struct { + immediateMutationOp + TableID descpb.ID + ConstraintID descpb.ConstraintID + ColumnIDs []descpb.ColumnID + ReferencedTableID descpb.ID + ReferencedColumnIDs []descpb.ColumnID + OnUpdateAction semenumpb.ForeignKeyAction + OnDeleteAction semenumpb.ForeignKeyAction + CompositeKeyMatchMethod semenumpb.Match +} + // MakeValidatedForeignKeyConstraintPublic moves a new, validated foreign key // constraint from mutation to public. type MakeValidatedForeignKeyConstraintPublic struct { diff --git a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go index 3b033ac05722..0439f79e213c 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go @@ -61,6 +61,7 @@ type ImmediateMutationVisitor interface { MakeValidatedCheckConstraintPublic(context.Context, MakeValidatedCheckConstraintPublic) error MakeValidatedColumnNotNullPublic(context.Context, MakeValidatedColumnNotNullPublic) error MakeAbsentForeignKeyConstraintWriteOnly(context.Context, MakeAbsentForeignKeyConstraintWriteOnly) error + MakeAbsentForeignKeyConstraintNotValidPublic(context.Context, MakeAbsentForeignKeyConstraintNotValidPublic) error MakeValidatedForeignKeyConstraintPublic(context.Context, MakeValidatedForeignKeyConstraintPublic) error MakePublicForeignKeyConstraintValidated(context.Context, MakePublicForeignKeyConstraintValidated) error RemoveForeignKeyConstraint(context.Context, RemoveForeignKeyConstraint) error @@ -313,6 +314,11 @@ func (op MakeAbsentForeignKeyConstraintWriteOnly) Visit(ctx context.Context, v I return v.MakeAbsentForeignKeyConstraintWriteOnly(ctx, op) } +// Visit is part of the ImmediateMutationOp interface. +func (op MakeAbsentForeignKeyConstraintNotValidPublic) Visit(ctx context.Context, v ImmediateMutationVisitor) error { + return v.MakeAbsentForeignKeyConstraintNotValidPublic(ctx, op) +} + // Visit is part of the ImmediateMutationOp interface. func (op MakeValidatedForeignKeyConstraintPublic) Visit(ctx context.Context, v ImmediateMutationVisitor) error { return v.MakeValidatedForeignKeyConstraintPublic(ctx, op) diff --git a/pkg/sql/schemachanger/scpb/elements.proto b/pkg/sql/schemachanger/scpb/elements.proto index 8fb59f3b6eb5..0f0c7a6d6659 100644 --- a/pkg/sql/schemachanger/scpb/elements.proto +++ b/pkg/sql/schemachanger/scpb/elements.proto @@ -77,6 +77,7 @@ message ElementProto { RowLevelTTL row_level_ttl = 29 [(gogoproto.customname) = "RowLevelTTL", (gogoproto.moretags) = "parent:\"Table\""]; CheckConstraintNotValid check_constraint_not_valid = 170 [(gogoproto.moretags) = "parent:\"Table\""]; UniqueWithoutIndexConstraintNotValid unique_without_index_constraint_not_valid = 171 [(gogoproto.moretags) = "parent:\"Table\""]; + ForeignKeyConstraintNotValid foreign_key_constraint_not_valid = 172 [(gogoproto.moretags) = "parent:\"Table\""]; TableZoneConfig table_zone_config = 121 [(gogoproto.moretags) = "parent:\"Table, View\""]; TableData table_data = 131 [(gogoproto.customname) = "TableData", (gogoproto.moretags) = "parent:\"Table, View, Sequence\""]; TablePartitioning table_partitioning = 132 [(gogoproto.customname) = "TablePartitioning", (gogoproto.moretags) = "parent:\"Table\""]; @@ -398,6 +399,17 @@ message ForeignKeyConstraint { uint32 index_id_for_validation = 9 [(gogoproto.customname) = "IndexIDForValidation", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.IndexID"]; } +message ForeignKeyConstraintNotValid { + uint32 table_id = 1 [(gogoproto.customname) = "TableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + uint32 constraint_id = 2 [(gogoproto.customname) = "ConstraintID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ConstraintID"]; + repeated uint32 column_ids = 3 [(gogoproto.customname) = "ColumnIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ColumnID"]; + uint32 referenced_table_id = 4 [(gogoproto.customname) = "ReferencedTableID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; + repeated uint32 referenced_column_ids = 5 [(gogoproto.customname) = "ReferencedColumnIDs", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.ColumnID"]; + cockroach.sql.sem.semenumpb.ForeignKeyAction on_update_action = 6 [(gogoproto.customname) = "OnUpdateAction"]; + cockroach.sql.sem.semenumpb.ForeignKeyAction on_delete_action = 7 [(gogoproto.customname) = "OnDeleteAction"]; + cockroach.sql.sem.semenumpb.Match composite_key_match_method = 8 [(gogoproto.customname) = "CompositeKeyMatchMethod"]; +} + message EnumType { uint32 type_id = 1 [(gogoproto.customname) = "TypeID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; uint32 array_type_id = 2 [(gogoproto.customname) = "ArrayTypeID", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.DescID"]; diff --git a/pkg/sql/schemachanger/scpb/elements_generated.go b/pkg/sql/schemachanger/scpb/elements_generated.go index d3a3b6224b3f..5d3472039ce0 100644 --- a/pkg/sql/schemachanger/scpb/elements_generated.go +++ b/pkg/sql/schemachanger/scpb/elements_generated.go @@ -761,6 +761,37 @@ func FindForeignKeyConstraint(b ElementStatusIterator) (current Status, target T return current, target, element } +func (e ForeignKeyConstraintNotValid) element() {} + +// ForEachForeignKeyConstraintNotValid iterates over elements of type ForeignKeyConstraintNotValid. +func ForEachForeignKeyConstraintNotValid( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *ForeignKeyConstraintNotValid), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*ForeignKeyConstraintNotValid); ok { + fn(current, target, elt) + } + }) +} + +// FindForeignKeyConstraintNotValid finds the first element of type ForeignKeyConstraintNotValid. +func FindForeignKeyConstraintNotValid(b ElementStatusIterator) (current Status, target TargetStatus, element *ForeignKeyConstraintNotValid) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*ForeignKeyConstraintNotValid); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + func (e Function) element() {} // ForEachFunction iterates over elements of type Function. diff --git a/pkg/sql/schemachanger/scpb/uml/table.puml b/pkg/sql/schemachanger/scpb/uml/table.puml index 1bd7bff0c2c8..e2f740cdb685 100644 --- a/pkg/sql/schemachanger/scpb/uml/table.puml +++ b/pkg/sql/schemachanger/scpb/uml/table.puml @@ -135,6 +135,17 @@ UniqueWithoutIndexConstraintNotValid : ConstraintID UniqueWithoutIndexConstraintNotValid : []ColumnIDs UniqueWithoutIndexConstraintNotValid : Predicate +object ForeignKeyConstraintNotValid + +ForeignKeyConstraintNotValid : TableID +ForeignKeyConstraintNotValid : ConstraintID +ForeignKeyConstraintNotValid : []ColumnIDs +ForeignKeyConstraintNotValid : ReferencedTableID +ForeignKeyConstraintNotValid : []ReferencedColumnIDs +ForeignKeyConstraintNotValid : OnUpdateAction +ForeignKeyConstraintNotValid : OnDeleteAction +ForeignKeyConstraintNotValid : CompositeKeyMatchMethod + object TableZoneConfig TableZoneConfig : TableID @@ -390,6 +401,7 @@ Sequence <|-- TableComment Table <|-- RowLevelTTL Table <|-- CheckConstraintNotValid Table <|-- UniqueWithoutIndexConstraintNotValid +Table <|-- ForeignKeyConstraintNotValid Table <|-- TableZoneConfig View <|-- TableZoneConfig Table <|-- TableData diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel index 77041d3da47b..0d71661e75bc 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel +++ b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel @@ -30,6 +30,7 @@ go_library( "opgen_enum_type.go", "opgen_enum_type_value.go", "opgen_foreign_key_constraint.go", + "opgen_foreign_key_constraint_not_valid.go", "opgen_function.go", "opgen_function_body.go", "opgen_function_leakproof.go", diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_not_valid.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_not_valid.go new file mode 100644 index 000000000000..791548a5f959 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_not_valid.go @@ -0,0 +1,58 @@ +// Copyright 2023 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.ForeignKeyConstraintNotValid)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.ForeignKeyConstraintNotValid) *scop.MakeAbsentForeignKeyConstraintNotValidPublic { + return &scop.MakeAbsentForeignKeyConstraintNotValidPublic{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + ColumnIDs: this.ColumnIDs, + ReferencedTableID: this.ReferencedTableID, + ReferencedColumnIDs: this.ReferencedColumnIDs, + OnUpdateAction: this.OnUpdateAction, + OnDeleteAction: this.OnDeleteAction, + CompositeKeyMatchMethod: this.CompositeKeyMatchMethod, + } + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + // TODO(postamar): remove revertibility constraint when possible + revertible(false), + emit(func(this *scpb.ForeignKeyConstraintNotValid) *scop.RemoveForeignKeyBackReference { + return &scop.RemoveForeignKeyBackReference{ + ReferencedTableID: this.ReferencedTableID, + OriginTableID: this.TableID, + OriginConstraintID: this.ConstraintID, + } + }), + emit(func(this *scpb.ForeignKeyConstraintNotValid) *scop.RemoveForeignKeyConstraint { + return &scop.RemoveForeignKeyConstraint{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + } + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules index faa6bed4667e..7e6d112be2d6 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules @@ -2077,7 +2077,7 @@ deprules kind: Precedence to: relation-Node query: - - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.Column', '*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TableData', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.ColumnNotNull', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.IndexData', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.DatabaseData', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] + - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.Column', '*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TableData', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.ColumnNotNull', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.IndexData', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.DatabaseData', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - $relation[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinOnDescID($dependent, $relation, $relation-id) - ToPublicOrTransient($dependent-Target, $relation-Target) @@ -2253,7 +2253,7 @@ deprules to: constraint-Node query: - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] - - $constraint[Type] = '*scpb.CheckConstraintNotValid' + - $constraint[Type] IN ['*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid'] - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) - toAbsent($dependents-Target, $constraint-Target) - $dependents-Node[CurrentStatus] = ABSENT @@ -2266,7 +2266,7 @@ deprules to: constraint-Node query: - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] - - $constraint[Type] = '*scpb.CheckConstraintNotValid' + - $constraint[Type] IN ['*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid'] - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) - transient($dependents-Target, $constraint-Target) - $dependents-Node[CurrentStatus] = TRANSIENT_ABSENT @@ -2279,7 +2279,7 @@ deprules to: constraint-Node query: - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] - - $constraint[Type] = '*scpb.CheckConstraintNotValid' + - $constraint[Type] IN ['*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid'] - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) - $dependents-Target[TargetStatus] = TRANSIENT_ABSENT - $dependents-Node[CurrentStatus] = TRANSIENT_ABSENT @@ -2293,7 +2293,7 @@ deprules to: constraint-Node query: - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] - - $constraint[Type] = '*scpb.CheckConstraintNotValid' + - $constraint[Type] IN ['*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid'] - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) - $dependents-Target[TargetStatus] = ABSENT - $dependents-Node[CurrentStatus] = ABSENT @@ -2307,7 +2307,7 @@ deprules to: referencing-via-attr-Node query: - $referenced-descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - - $referencing-via-attr[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaComment', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] + - $referencing-via-attr[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaComment', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - joinReferencedDescID($referencing-via-attr, $referenced-descriptor, $desc-id) - toAbsent($referenced-descriptor-Target, $referencing-via-attr-Target) - $referenced-descriptor-Node[CurrentStatus] = DROPPED @@ -2348,7 +2348,7 @@ deprules to: dependent-Node query: - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] + - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - joinOnDescID($descriptor, $dependent, $desc-id) - toAbsent($descriptor-Target, $dependent-Target) - $descriptor-Node[CurrentStatus] = DROPPED @@ -2387,7 +2387,7 @@ deprules to: dependent-Node query: - $relation[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.Column', '*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TableData', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.ColumnNotNull', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.IndexData', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.DatabaseData', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] + - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.Column', '*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid', '*scpb.TableZoneConfig', '*scpb.TableData', '*scpb.TablePartitioning', '*scpb.TableLocalityGlobal', '*scpb.TableLocalityPrimaryRegion', '*scpb.TableLocalitySecondaryRegion', '*scpb.TableLocalityRegionalByRow', '*scpb.ColumnName', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SequenceOwner', '*scpb.ColumnComment', '*scpb.ColumnNotNull', '*scpb.IndexName', '*scpb.IndexPartitioning', '*scpb.SecondaryIndexPartial', '*scpb.IndexComment', '*scpb.IndexColumn', '*scpb.IndexData', '*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*scpb.Namespace', '*scpb.Owner', '*scpb.UserPrivileges', '*scpb.DatabaseRegionConfig', '*scpb.DatabaseRoleSetting', '*scpb.DatabaseComment', '*scpb.DatabaseData', '*scpb.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - joinOnDescID($relation, $dependent, $relation-id) - ToPublicOrTransient($relation-Target, $dependent-Target) - $relation-Node[CurrentStatus] = DESCRIPTOR_ADDED @@ -3148,7 +3148,7 @@ deprules kind: SameStagePrecedence to: dependent-Node query: - - $simple-constraint[Type] = '*scpb.CheckConstraintNotValid' + - $simple-constraint[Type] IN ['*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid'] - $dependent[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] - joinOnConstraintID($simple-constraint, $dependent, $table-id, $constraint-id) - ToPublicOrTransient($simple-constraint-Target, $dependent-Target) diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules index c0011e508fc2..7f4417fac1c2 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules @@ -143,7 +143,7 @@ oprules from: constraint-Node query: - $relation[Type] IN ['*scpb.Table', '*scpb.View'] - - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint', '*scpb.CheckConstraintNotValid', '*scpb.ColumnNotNull'] + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint', '*scpb.CheckConstraintNotValid', '*scpb.UniqueWithoutIndexConstraintNotValid', '*scpb.ForeignKeyConstraintNotValid', '*scpb.ColumnNotNull'] - joinOnDescID($relation, $constraint, $relation-id) - joinTarget($relation, $relation-Target) - $relation-Target[TargetStatus] = ABSENT diff --git a/pkg/sql/schemachanger/screl/attr.go b/pkg/sql/schemachanger/screl/attr.go index 1f0bd4e4238c..9d83c74fc28c 100644 --- a/pkg/sql/schemachanger/screl/attr.go +++ b/pkg/sql/schemachanger/screl/attr.go @@ -201,6 +201,11 @@ var elementSchemaOptions = []rel.SchemaOption{ rel.EntityAttr(ConstraintID, "ConstraintID"), rel.EntityAttr(IndexID, "IndexIDForValidation"), ), + rel.EntityMapping(t((*scpb.ForeignKeyConstraintNotValid)(nil)), + rel.EntityAttr(DescID, "TableID"), + rel.EntityAttr(ReferencedDescID, "ReferencedTableID"), + rel.EntityAttr(ConstraintID, "ConstraintID"), + ), rel.EntityMapping(t((*scpb.RowLevelTTL)(nil)), rel.EntityAttr(DescID, "TableID"), ), diff --git a/pkg/sql/schemachanger/screl/scalars.go b/pkg/sql/schemachanger/screl/scalars.go index 6825c85d7b7a..5dcb6b6447b7 100644 --- a/pkg/sql/schemachanger/screl/scalars.go +++ b/pkg/sql/schemachanger/screl/scalars.go @@ -124,7 +124,8 @@ func MinVersion(el scpb.Element) clusterversion.Key { *scpb.Function, *scpb.FunctionName, *scpb.FunctionVolatility, *scpb.FunctionLeakProof, *scpb.FunctionNullInputBehavior, *scpb.FunctionBody, *scpb.FunctionParamDefaultExpression: return clusterversion.V23_1 - case *scpb.ColumnNotNull, *scpb.CheckConstraintNotValid, *scpb.UniqueWithoutIndexConstraintNotValid: + case *scpb.ColumnNotNull, *scpb.CheckConstraintNotValid, *scpb.UniqueWithoutIndexConstraintNotValid, + *scpb.ForeignKeyConstraintNotValid: return clusterversion.V23_1 default: panic(errors.AssertionFailedf("unknown element %T", el)) diff --git a/pkg/sql/schemachanger/sctest_generated_test.go b/pkg/sql/schemachanger/sctest_generated_test.go index a54335f775b3..1291ac6851a8 100644 --- a/pkg/sql/schemachanger/sctest_generated_test.go +++ b/pkg/sql/schemachanger/sctest_generated_test.go @@ -220,6 +220,31 @@ func TestRollback_alter_table_add_foreign_key(t *testing.T) { defer log.Scope(t).Close(t) sctest.Rollback(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key", sctest.SingleNodeCluster) } +func TestEndToEndSideEffects_alter_table_add_foreign_key_not_valid(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.EndToEndSideEffects(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid", sctest.SingleNodeCluster) +} +func TestExecuteWithDMLInjection_alter_table_add_foreign_key_not_valid(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.ExecuteWithDMLInjection(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid", sctest.SingleNodeCluster) +} +func TestGenerateSchemaChangeCorpus_alter_table_add_foreign_key_not_valid(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.GenerateSchemaChangeCorpus(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid", sctest.SingleNodeCluster) +} +func TestPause_alter_table_add_foreign_key_not_valid(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.Pause(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid", sctest.SingleNodeCluster) +} +func TestRollback_alter_table_add_foreign_key_not_valid(t *testing.T) { + defer leaktest.AfterTest(t)() + defer log.Scope(t).Close(t) + sctest.Rollback(t, "pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid", sctest.SingleNodeCluster) +} func TestEndToEndSideEffects_alter_table_add_primary_key_drop_rowid(t *testing.T) { defer leaktest.AfterTest(t)() defer log.Scope(t).Close(t) diff --git a/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid new file mode 100644 index 000000000000..108a63bb54fa --- /dev/null +++ b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_foreign_key_not_valid @@ -0,0 +1,130 @@ +setup +CREATE TABLE t1 (i INT PRIMARY KEY); +CREATE TABLE t2 (i INT PRIMARY KEY); +INSERT INTO t1 VALUES (0); +---- +... ++object {100 101 t1} -> 104 ++object {100 101 t2} -> 105 + +test +ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i) NOT VALID +---- +begin transaction #1 +# begin StatementPhase +checking for feature: ALTER TABLE +increment telemetry for sql.schema.alter_table +increment telemetry for sql.schema.alter_table.add_constraint +write *eventpb.AlterTable to event log: + mutationId: 1 + sql: + descriptorId: 104 + statement: ALTER TABLE ‹defaultdb›.‹public›.‹t1› ADD CONSTRAINT ‹t1_i_fkey› FOREIGN + KEY (‹i›) REFERENCES ‹defaultdb›.‹public›.‹t2› (‹i›) NOT VALID + tag: ALTER TABLE + user: root + tableName: defaultdb.public.t1 +## StatementPhase stage 1 of 1 with 2 MutationType ops +upsert descriptor #104 + ... + name: t1 + nextColumnId: 2 + - nextConstraintId: 2 + + nextConstraintId: 3 + nextFamilyId: 1 + nextIndexId: 2 + nextMutationId: 1 + + outboundFks: + + - constraintId: 2 + + name: t1_i_fkey + + originColumnIds: + + - 1 + + originTableId: 104 + + referencedColumnIds: + + - 1 + + referencedTableId: 105 + + validity: Unvalidated + parentId: 100 + primaryIndex: + ... + time: {} + unexposedParentSchemaId: 101 + - version: "1" + + version: "2" +upsert descriptor #105 + ... + formatVersion: 3 + id: 105 + + inboundFks: + + - constraintId: 2 + + name: t1_i_fkey + + originColumnIds: + + - 1 + + originTableId: 104 + + referencedColumnIds: + + - 1 + + referencedTableId: 105 + + validity: Unvalidated + modificationTime: {} + name: t2 + ... + time: {} + unexposedParentSchemaId: 101 + - version: "1" + + version: "2" +# end StatementPhase +# begin PreCommitPhase +## PreCommitPhase stage 1 of 2 with 1 MutationType op +undo all catalog changes within txn #1 +persist all catalog changes to storage +## PreCommitPhase stage 2 of 2 with 2 MutationType ops +upsert descriptor #104 + ... + name: t1 + nextColumnId: 2 + - nextConstraintId: 2 + + nextConstraintId: 3 + nextFamilyId: 1 + nextIndexId: 2 + nextMutationId: 1 + + outboundFks: + + - constraintId: 2 + + name: t1_i_fkey + + originColumnIds: + + - 1 + + originTableId: 104 + + referencedColumnIds: + + - 1 + + referencedTableId: 105 + + validity: Unvalidated + parentId: 100 + primaryIndex: + ... + time: {} + unexposedParentSchemaId: 101 + - version: "1" + + version: "2" +upsert descriptor #105 + ... + formatVersion: 3 + id: 105 + + inboundFks: + + - constraintId: 2 + + name: t1_i_fkey + + originColumnIds: + + - 1 + + originTableId: 104 + + referencedColumnIds: + + - 1 + + referencedTableId: 105 + + validity: Unvalidated + modificationTime: {} + name: t2 + ... + time: {} + unexposedParentSchemaId: 101 + - version: "1" + + version: "2" +persist all catalog changes to storage +# end PreCommitPhase +commit transaction #1 diff --git a/pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key_not_valid b/pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key_not_valid new file mode 100644 index 000000000000..e7ac582f654c --- /dev/null +++ b/pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key_not_valid @@ -0,0 +1,31 @@ +/* setup */ +CREATE TABLE t1 (i INT PRIMARY KEY); +CREATE TABLE t2 (i INT PRIMARY KEY); +INSERT INTO t1 VALUES (0); + +/* test */ +EXPLAIN (ddl) ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i) NOT VALID; +---- +Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t1› ADD CONSTRAINT ‹t1_i_fkey› FOREIGN KEY (‹i›) REFERENCES ‹defaultdb›.‹public›.‹t2› (‹i›) NOT VALID; + ├── StatementPhase + │ └── Stage 1 of 1 in StatementPhase + │ ├── 2 elements transitioning toward PUBLIC + │ │ ├── ABSENT → PUBLIC ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} + │ │ └── ABSENT → PUBLIC ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} + │ └── 2 Mutation operations + │ ├── MakeAbsentForeignKeyConstraintNotValidPublic {"ConstraintID":2,"ReferencedTableID":105,"TableID":104} + │ └── SetConstraintName {"ConstraintID":2,"Name":"t1_i_fkey","TableID":104} + └── PreCommitPhase + ├── Stage 1 of 2 in PreCommitPhase + │ ├── 2 elements transitioning toward PUBLIC + │ │ ├── PUBLIC → ABSENT ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} + │ │ └── PUBLIC → ABSENT ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} + │ └── 1 Mutation operation + │ └── UndoAllInTxnImmediateMutationOpSideEffects + └── Stage 2 of 2 in PreCommitPhase + ├── 2 elements transitioning toward PUBLIC + │ ├── ABSENT → PUBLIC ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} + │ └── ABSENT → PUBLIC ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} + └── 2 Mutation operations + ├── MakeAbsentForeignKeyConstraintNotValidPublic {"ConstraintID":2,"ReferencedTableID":105,"TableID":104} + └── SetConstraintName {"ConstraintID":2,"Name":"t1_i_fkey","TableID":104} diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key_not_valid b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key_not_valid new file mode 100644 index 000000000000..a1d832bda98c --- /dev/null +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key_not_valid @@ -0,0 +1,86 @@ +/* setup */ +CREATE TABLE t1 (i INT PRIMARY KEY); +CREATE TABLE t2 (i INT PRIMARY KEY); +INSERT INTO t1 VALUES (0); + +/* test */ +EXPLAIN (ddl, verbose) ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i) NOT VALID; +---- +• Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t1› ADD CONSTRAINT ‹t1_i_fkey› FOREIGN KEY (‹i›) REFERENCES ‹defaultdb›.‹public›.‹t2› (‹i›) NOT VALID; +│ +├── • StatementPhase +│ │ +│ └── • Stage 1 of 1 in StatementPhase +│ │ +│ ├── • 2 elements transitioning toward PUBLIC +│ │ │ +│ │ ├── • ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} +│ │ │ ABSENT → PUBLIC +│ │ │ +│ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} +│ │ │ ABSENT → PUBLIC +│ │ │ +│ │ └── • SameStagePrecedence dependency from PUBLIC ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} +│ │ rule: "simple constraint public right before its dependents" +│ │ +│ └── • 2 Mutation operations +│ │ +│ ├── • MakeAbsentForeignKeyConstraintNotValidPublic +│ │ ColumnIDs: +│ │ - 1 +│ │ ConstraintID: 2 +│ │ ReferencedColumnIDs: +│ │ - 1 +│ │ ReferencedTableID: 105 +│ │ TableID: 104 +│ │ +│ └── • SetConstraintName +│ ConstraintID: 2 +│ Name: t1_i_fkey +│ TableID: 104 +│ +└── • PreCommitPhase + │ + ├── • Stage 1 of 2 in PreCommitPhase + │ │ + │ ├── • 2 elements transitioning toward PUBLIC + │ │ │ + │ │ ├── • ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} + │ │ │ PUBLIC → ABSENT + │ │ │ + │ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} + │ │ PUBLIC → ABSENT + │ │ + │ └── • 1 Mutation operation + │ │ + │ └── • UndoAllInTxnImmediateMutationOpSideEffects + │ {} + │ + └── • Stage 2 of 2 in PreCommitPhase + │ + ├── • 2 elements transitioning toward PUBLIC + │ │ + │ ├── • ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} + │ │ ABSENT → PUBLIC + │ │ + │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} + │ │ ABSENT → PUBLIC + │ │ + │ └── • SameStagePrecedence dependency from PUBLIC ForeignKeyConstraintNotValid:{DescID: 104, ConstraintID: 2, ReferencedDescID: 105} + │ rule: "simple constraint public right before its dependents" + │ + └── • 2 Mutation operations + │ + ├── • MakeAbsentForeignKeyConstraintNotValidPublic + │ ColumnIDs: + │ - 1 + │ ConstraintID: 2 + │ ReferencedColumnIDs: + │ - 1 + │ ReferencedTableID: 105 + │ TableID: 104 + │ + └── • SetConstraintName + ConstraintID: 2 + Name: t1_i_fkey + TableID: 104