From 99a7b47f2c41f1b25f94cc9b40ad014935c4103a Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Mon, 30 Jan 2023 17:19:18 -0500 Subject: [PATCH 1/9] schemachanger: implement `ADD CHECK NOT VALID` and add tests Not-valid constraints are like simple dependents: they don't have any intermediate state so it can transition directly between public and absent. This nature requires us not to enforce certains rules that are enforced on the normal constraints. To that effect, we introduce the concept/partition of "simple" vs. "complex" constraints: - "complex constraint": a constraint that needs to transition through an intermediate status; - "simple constraint": a constraint that does not need to transition through an intermediate status. They are the NOT VALID version of the complex constraints. This commit also slightly modified the fitlers on constraints to be more principled. --- .../backup_base_generated_test.go | 5 + .../logictest/testdata/logic_test/drop_table | 11 ++ .../internal/scbuildstmt/alter_table.go | 5 +- .../scbuildstmt/alter_table_add_constraint.go | 41 ++++--- .../scbuild/internal/scbuildstmt/process.go | 1 - .../scbuild/testdata/alter_table_add_check | 1 - .../alter_table_add_check_unvalidated | 11 ++ pkg/sql/schemachanger/scdecomp/decomp.go | 24 +++-- pkg/sql/schemachanger/scdecomp/testdata/table | 38 ++++++- .../scexec/scmutationexec/constraint.go | 19 ++-- .../schemachanger/scop/immediate_mutation.go | 7 +- .../immediate_mutation_visitor_generated.go | 6 +- pkg/sql/schemachanger/scpb/elements.proto | 10 +- .../schemachanger/scpb/elements_generated.go | 31 ++++++ pkg/sql/schemachanger/scpb/uml/table.puml | 8 ++ .../scplan/internal/opgen/BUILD.bazel | 1 + .../internal/opgen/opgen_check_constraint.go | 6 +- .../opgen_check_constraint_unvalidated.go | 83 ++++++++++++++ .../internal/rules/current/assertions_test.go | 36 +++---- .../rules/current/dep_add_constraint.go | 20 +++- .../current/dep_add_index_and_constraint.go | 2 +- .../rules/current/dep_drop_constraint.go | 26 ++++- .../internal/rules/current/dep_drop_object.go | 6 +- .../scplan/internal/rules/current/helpers.go | 59 ++++++---- .../scplan/internal/rules/current/op_drop.go | 2 +- .../internal/rules/current/testdata/deprules | 101 +++++++++++++++--- .../internal/rules/current/testdata/oprules | 2 +- .../scplan/testdata/alter_table_add_check | 8 +- .../schemachanger/scplan/testdata/drop_table | 28 ++++- pkg/sql/schemachanger/screl/attr.go | 6 ++ pkg/sql/schemachanger/screl/scalars.go | 2 +- .../schemachanger/sctest_generated_test.go | 25 +++++ .../alter_table_add_check_unvalidated | 80 ++++++++++++++ .../explain/alter_table_add_check_unvalidated | 30 ++++++ .../explain/alter_table_add_check_vanilla | 4 +- .../alter_table_add_check_with_seq_and_udt | 4 +- .../alter_table_add_check_unvalidated | 77 +++++++++++++ .../alter_table_add_check_vanilla | 8 +- .../alter_table_add_check_with_seq_and_udt | 8 +- .../alter_table_add_foreign_key | 2 +- .../alter_table_add_unique_without_index | 2 +- ...r_table_add_unique_without_index_not_valid | 74 +++++++++++++ 42 files changed, 777 insertions(+), 143 deletions(-) create mode 100644 pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check_unvalidated create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint_unvalidated.go create mode 100644 pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_unvalidated create mode 100644 pkg/sql/schemachanger/testdata/explain/alter_table_add_check_unvalidated create mode 100644 pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_unvalidated create mode 100644 pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid diff --git a/pkg/ccl/schemachangerccl/backup_base_generated_test.go b/pkg/ccl/schemachangerccl/backup_base_generated_test.go index 8db271e972e9..7ff06900a66e 100644 --- a/pkg/ccl/schemachangerccl/backup_base_generated_test.go +++ b/pkg/ccl/schemachangerccl/backup_base_generated_test.go @@ -38,6 +38,11 @@ func TestBackup_base_add_column_no_default(t *testing.T) { defer log.Scope(t).Close(t) sctest.Backup(t, "pkg/sql/schemachanger/testdata/end_to_end/add_column_no_default", newCluster) } +func TestBackup_base_alter_table_add_check_unvalidated(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_check_unvalidated", newCluster) +} func TestBackup_base_alter_table_add_check_vanilla(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 49c8388f13f0..655e2305dacd 100644 --- a/pkg/sql/logictest/testdata/logic_test/drop_table +++ b/pkg/sql/logictest/testdata/logic_test/drop_table @@ -114,3 +114,14 @@ COMMIT; statement error pgcode 42P01 relation "to_drop" does not exist DROP TABLE to_drop; + +subtest drop_table_with_not_valid_constraints + +statement ok +CREATE TABLE t_with_not_valid_constraints_1 (i INT PRIMARY KEY); + +statement ok +ALTER TABLE t_with_not_valid_constraints_1 ADD CHECK (i > 0) NOT VALID; + +statement ok +DROP TABLE t_with_not_valid_constraints_1; diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go index bab311fe92f8..64e238ee7d4d 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go @@ -59,8 +59,8 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{ return true } - // Support ALTER TABLE ... ADD CONSTRAINT CHECK - if _, ok := t.ConstraintDef.(*tree.CheckConstraintTableDef); ok && t.ValidationBehavior == tree.ValidationDefault { + // Support ALTER TABLE ... ADD CONSTRAINT CHECK [NOT VALID] + if _, ok := t.ConstraintDef.(*tree.CheckConstraintTableDef); ok { return true } @@ -83,6 +83,7 @@ var alterTableAddConstraintMinSupportedClusterVersion = map[string]clusterversio "ADD_CHECK_DEFAULT": clusterversion.V23_1Start, "ADD_FOREIGN_KEY_DEFAULT": clusterversion.V23_1Start, "ADD_UNIQUE_WITHOUT_INDEX_DEFAULT": clusterversion.V23_1Start, + "ADD_CHECK_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 df72176612d5..34b88922d60b 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 @@ -45,9 +45,7 @@ func alterTableAddConstraint( alterTableAddUniqueWithoutIndex(b, tn, tbl, t) } case *tree.CheckConstraintTableDef: - if t.ValidationBehavior == tree.ValidationDefault { - alterTableAddCheck(b, tn, tbl, t) - } + alterTableAddCheck(b, tn, tbl, t) case *tree.ForeignKeyConstraintTableDef: if t.ValidationBehavior == tree.ValidationDefault { alterTableAddForeignKey(b, tn, tbl, t) @@ -80,7 +78,7 @@ func alterTableAddPrimaryKey( } // alterTableAddCheck contains logic for building -// `ALTER TABLE ... ADD CONSTRAINT ... CHECK`. +// `ALTER TABLE ... ADD CHECK ... [NOT VALID]`. // It assumes `t` is such a command. func alterTableAddCheck( b BuildCtx, tn *tree.TableName, tbl *scpb.Table, t *tree.AlterTableAddConstraint, @@ -113,18 +111,31 @@ func alterTableAddCheck( panic(err) } - // 3. Add relevant check constraint element: CheckConstraint and ConstraintName. + // 3. Add relevant check constraint element: + // - CheckConstraint or CheckConstraintUnvalidated + // - ConstraintName constraintID := b.NextTableConstraintID(tbl.TableID) - ck := &scpb.CheckConstraint{ - TableID: tbl.TableID, - ConstraintID: constraintID, - ColumnIDs: colIDs.Ordered(), - Expression: *b.WrapExpression(tbl.TableID, typedCkExpr), - FromHashShardedColumn: ckDef.FromHashShardedColumn, - IndexIDForValidation: getIndexIDForValidationForConstraint(b, tbl.TableID), - } - b.Add(ck) - b.LogEventForExistingTarget(ck) + if t.ValidationBehavior == tree.ValidationDefault { + ck := &scpb.CheckConstraint{ + TableID: tbl.TableID, + ConstraintID: constraintID, + ColumnIDs: colIDs.Ordered(), + Expression: *b.WrapExpression(tbl.TableID, typedCkExpr), + FromHashShardedColumn: ckDef.FromHashShardedColumn, + IndexIDForValidation: getIndexIDForValidationForConstraint(b, tbl.TableID), + } + b.Add(ck) + b.LogEventForExistingTarget(ck) + } else { + ck := &scpb.CheckConstraintUnvalidated{ + TableID: tbl.TableID, + ConstraintID: constraintID, + ColumnIDs: colIDs.Ordered(), + Expression: *b.WrapExpression(tbl.TableID, typedCkExpr), + } + b.Add(ck) + b.LogEventForExistingTarget(ck) + } constraintName := string(ckDef.Name) if constraintName == "" { diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go index f92baae14ecc..b97abd8cf879 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/process.go @@ -197,5 +197,4 @@ func stmtSupportedInCurrentClusterVersion(b BuildCtx, n tree.Statement) bool { } minSupportedClusterVersion := supportedStatements[reflect.TypeOf(n)].minSupportedClusterVersion return b.EvalCtx().Settings.Version.IsActive(b, minSupportedClusterVersion) - } diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check index 8d931cb189ae..3cd0f25f3e65 100644 --- a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check +++ b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check @@ -4,7 +4,6 @@ CREATE TABLE t (i INT PRIMARY KEY) build ALTER TABLE t ADD CHECK (i > 0) ---- ---- - [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, PUBLIC], ABSENT] {columnIds: [1], constraintId: 2, expr: 'i > 0:::INT8', referencedColumnIds: [1], tableId: 104} diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check_unvalidated b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check_unvalidated new file mode 100644 index 000000000000..ac291db4324f --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_check_unvalidated @@ -0,0 +1,11 @@ +setup +CREATE TABLE t (i INT PRIMARY KEY) +---- + +build +ALTER TABLE t ADD CHECK (i > 0) NOT VALID +---- +- [[CheckConstraintUnvalidated:{DescID: 104, ConstraintID: 2}, PUBLIC], ABSENT] + {columnIds: [1], constraintId: 2, expr: 'i > 0:::INT8', referencedColumnIds: [1], tableId: 104} +- [[ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2}, PUBLIC], ABSENT] + {constraintId: 2, name: check_i, tableId: 104} diff --git a/pkg/sql/schemachanger/scdecomp/decomp.go b/pkg/sql/schemachanger/scdecomp/decomp.go index bf1e539817c7..7f0a7b7108bd 100644 --- a/pkg/sql/schemachanger/scdecomp/decomp.go +++ b/pkg/sql/schemachanger/scdecomp/decomp.go @@ -635,14 +635,22 @@ func (w *walkCtx) walkCheckConstraint(tbl catalog.TableDescriptor, c catalog.Che panic(errors.NewAssertionErrorWithWrappedErrf(err, "check constraint %q in table %q (%d)", c.GetName(), tbl.GetName(), tbl.GetID())) } - // TODO(postamar): proper handling of constraint status - w.ev(scpb.Status_PUBLIC, &scpb.CheckConstraint{ - TableID: tbl.GetID(), - ConstraintID: c.GetConstraintID(), - ColumnIDs: c.CollectReferencedColumnIDs().Ordered(), - Expression: *expr, - FromHashShardedColumn: c.IsHashShardingConstraint(), - }) + if c.IsConstraintUnvalidated() && w.clusterVersion.IsActive(clusterversion.V23_1) { + w.ev(scpb.Status_PUBLIC, &scpb.CheckConstraintUnvalidated{ + TableID: tbl.GetID(), + ConstraintID: c.GetConstraintID(), + ColumnIDs: c.CollectReferencedColumnIDs().Ordered(), + Expression: *expr, + }) + } else { + w.ev(scpb.Status_PUBLIC, &scpb.CheckConstraint{ + TableID: tbl.GetID(), + ConstraintID: c.GetConstraintID(), + ColumnIDs: c.CollectReferencedColumnIDs().Ordered(), + Expression: *expr, + FromHashShardedColumn: c.IsHashShardingConstraint(), + }) + } 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 797f583a947b..05560972b6d3 100644 --- a/pkg/sql/schemachanger/scdecomp/testdata/table +++ b/pkg/sql/schemachanger/scdecomp/testdata/table @@ -5,7 +5,8 @@ CREATE TABLE tbl ( name STRING NOT NULL, price DECIMAL(8,2), INDEX sec (name) STORING (price) WHERE (id > 0), - CONSTRAINT myfk FOREIGN KEY (id) REFERENCES parent (id) + CONSTRAINT myfk FOREIGN KEY (id) REFERENCES parent (id), + CONSTRAINT mycheck1 CHECK (id > 0) ); COMMENT ON TABLE tbl IS 'tbl is good table'; COMMENT ON INDEX tbl@tbl_pkey IS 'tbl_pkey is a primary key'; @@ -13,6 +14,7 @@ COMMENT ON COLUMN tbl.id IS 'id is a identifier'; COMMENT ON CONSTRAINT myfk ON tbl IS 'must have a parent'; ALTER TABLE tbl CONFIGURE ZONE USING gc.ttlseconds=10; COMMENT ON CONSTRAINT tbl_pkey ON tbl IS 'primary key'; +ALTER TABLE tbl ADD CONSTRAINT mycheck2 CHECK (id < 10) NOT VALID; ---- decompose @@ -317,6 +319,30 @@ ElementState: tableId: 105 temporaryIndexId: 0 Status: PUBLIC +- CheckConstraint: + columnIds: + - 1 + constraintId: 3 + expr: id > 0:::INT8 + fromHashShardedColumn: false + indexIdForValidation: 0 + referencedColumnIds: + - 1 + tableId: 105 + usesSequenceIds: [] + usesTypeIds: [] + Status: PUBLIC +- CheckConstraintUnvalidated: + columnIds: + - 1 + constraintId: 4 + expr: id < 10:::INT8 + referencedColumnIds: + - 1 + tableId: 105 + usesSequenceIds: [] + usesTypeIds: [] + Status: PUBLIC - ForeignKeyConstraint: columnIds: - 1 @@ -614,6 +640,16 @@ ElementState: name: myfk tableId: 105 Status: PUBLIC +- ConstraintWithoutIndexName: + constraintId: 3 + name: mycheck1 + tableId: 105 + Status: PUBLIC +- ConstraintWithoutIndexName: + constraintId: 4 + name: mycheck2 + 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 c616dbc55e70..3f6c3d2cad64 100644 --- a/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go +++ b/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go @@ -56,8 +56,8 @@ func (i *immediateVisitor) SetConstraintName(ctx context.Context, op scop.SetCon return nil } -func (i *immediateVisitor) MakeAbsentCheckConstraintWriteOnly( - ctx context.Context, op scop.MakeAbsentCheckConstraintWriteOnly, +func (i *immediateVisitor) AddCheckConstraint( + ctx context.Context, op scop.AddCheckConstraint, ) error { tbl, err := i.checkOutTable(ctx, op.TableID) if err != nil || tbl.Dropped() { @@ -67,21 +67,20 @@ func (i *immediateVisitor) MakeAbsentCheckConstraintWriteOnly( tbl.NextConstraintID = op.ConstraintID + 1 } - // We should have already validated that the check constraint - // is syntactically valid in the builder, so we just need to - // enqueue it to the descriptor's mutation slice. ck := &descpb.TableDescriptor_CheckConstraint{ Expr: string(op.CheckExpr), Name: tabledesc.ConstraintNamePlaceholder(op.ConstraintID), - Validity: descpb.ConstraintValidity_Validating, + Validity: op.Validity, ColumnIDs: op.ColumnIDs, FromHashShardedColumn: op.FromHashShardedColumn, ConstraintID: op.ConstraintID, } - enqueueNonIndexMutation(tbl, tbl.AddCheckMutation, ck, descpb.DescriptorMutation_ADD) - // Fast-forward the mutation state to WRITE_ONLY because this constraint - // is now considered as enforced. - tbl.Mutations[len(tbl.Mutations)-1].State = descpb.DescriptorMutation_WRITE_ONLY + if op.Validity == descpb.ConstraintValidity_Validating { + // A validating constraint needs to transition through an intermediate state + // so we enqueue a mutation for it and fast-forward it to WRITE_ONLY state. + enqueueNonIndexMutation(tbl, tbl.AddCheckMutation, ck, descpb.DescriptorMutation_ADD) + tbl.Mutations[len(tbl.Mutations)-1].State = descpb.DescriptorMutation_WRITE_ONLY + } tbl.Checks = append(tbl.Checks, ck) return nil } diff --git a/pkg/sql/schemachanger/scop/immediate_mutation.go b/pkg/sql/schemachanger/scop/immediate_mutation.go index 0523e3b03bd1..7676aba34a84 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation.go @@ -266,15 +266,16 @@ type RemoveColumnNotNull struct { ColumnID descpb.ColumnID } -// MakeAbsentCheckConstraintWriteOnly adds a non-existent check constraint -// to the table in the WRITE_ONLY state. -type MakeAbsentCheckConstraintWriteOnly struct { +// AddCheckConstraint adds a non-existent check constraint +// to the table. +type AddCheckConstraint struct { immediateMutationOp TableID descpb.ID ConstraintID descpb.ConstraintID ColumnIDs []descpb.ColumnID CheckExpr catpb.Expression FromHashShardedColumn bool + Validity descpb.ConstraintValidity } // MakeAbsentColumnNotNullWriteOnly adds a non-existent NOT NULL constraint, diff --git a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go index 9a312702c1ff..88d0c4f62879 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go @@ -53,7 +53,7 @@ type ImmediateMutationVisitor interface { RemoveSequenceOwner(context.Context, RemoveSequenceOwner) error RemoveCheckConstraint(context.Context, RemoveCheckConstraint) error RemoveColumnNotNull(context.Context, RemoveColumnNotNull) error - MakeAbsentCheckConstraintWriteOnly(context.Context, MakeAbsentCheckConstraintWriteOnly) error + AddCheckConstraint(context.Context, AddCheckConstraint) error MakeAbsentColumnNotNullWriteOnly(context.Context, MakeAbsentColumnNotNullWriteOnly) error MakePublicCheckConstraintValidated(context.Context, MakePublicCheckConstraintValidated) error MakePublicColumnNotNullValidated(context.Context, MakePublicColumnNotNullValidated) error @@ -272,8 +272,8 @@ func (op RemoveColumnNotNull) Visit(ctx context.Context, v ImmediateMutationVisi } // Visit is part of the ImmediateMutationOp interface. -func (op MakeAbsentCheckConstraintWriteOnly) Visit(ctx context.Context, v ImmediateMutationVisitor) error { - return v.MakeAbsentCheckConstraintWriteOnly(ctx, op) +func (op AddCheckConstraint) Visit(ctx context.Context, v ImmediateMutationVisitor) error { + return v.AddCheckConstraint(ctx, op) } // Visit is part of the ImmediateMutationOp interface. diff --git a/pkg/sql/schemachanger/scpb/elements.proto b/pkg/sql/schemachanger/scpb/elements.proto index a89c279d93ca..5538163718f3 100644 --- a/pkg/sql/schemachanger/scpb/elements.proto +++ b/pkg/sql/schemachanger/scpb/elements.proto @@ -72,6 +72,7 @@ message ElementProto { TemporaryIndex temporary_index = 24 [(gogoproto.moretags) = "parent:\"Table, View\""]; UniqueWithoutIndexConstraint unique_without_index_constraint = 25 [(gogoproto.moretags) = "parent:\"Table\""]; CheckConstraint check_constraint = 26 [(gogoproto.moretags) = "parent:\"Table\""]; + CheckConstraintUnvalidated check_constraint_unvalidated = 170 [(gogoproto.moretags) = "parent:\"Table\""]; ForeignKeyConstraint foreign_key_constraint = 27 [(gogoproto.moretags) = "parent:\"Table\""]; TableComment table_comment = 28 [(gogoproto.moretags) = "parent:\"Table, View, Sequence\""]; RowLevelTTL row_level_ttl = 29 [(gogoproto.customname) = "RowLevelTTL", (gogoproto.moretags) = "parent:\"Table\""]; @@ -139,7 +140,7 @@ message ElementProto { FunctionBody function_body = 164 [(gogoproto.moretags) = "parent:\"Function\""]; FunctionParamDefaultExpression function_param_default = 165 [(gogoproto.moretags) = "parent:\"Function\""]; - // Next element group start id: 170 + // Next element group start id: 180 } // TypeT is a wrapper for a types.T which contains its user-defined type ID @@ -367,6 +368,13 @@ message CheckConstraint { uint32 index_id_for_validation = 6 [(gogoproto.customname) = "IndexIDForValidation", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.IndexID"]; } +message CheckConstraintUnvalidated { + 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"]; + Expression embedded_expr = 4 [(gogoproto.nullable) = false, (gogoproto.embed) = true]; +} + message ForeignKeyConstraint { 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"]; diff --git a/pkg/sql/schemachanger/scpb/elements_generated.go b/pkg/sql/schemachanger/scpb/elements_generated.go index 86dcc41976a5..28f41d5137fa 100644 --- a/pkg/sql/schemachanger/scpb/elements_generated.go +++ b/pkg/sql/schemachanger/scpb/elements_generated.go @@ -79,6 +79,37 @@ func FindCheckConstraint(b ElementStatusIterator) (current Status, target Target return current, target, element } +func (e CheckConstraintUnvalidated) element() {} + +// ForEachCheckConstraintUnvalidated iterates over elements of type CheckConstraintUnvalidated. +func ForEachCheckConstraintUnvalidated( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *CheckConstraintUnvalidated), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*CheckConstraintUnvalidated); ok { + fn(current, target, elt) + } + }) +} + +// FindCheckConstraintUnvalidated finds the first element of type CheckConstraintUnvalidated. +func FindCheckConstraintUnvalidated(b ElementStatusIterator) (current Status, target TargetStatus, element *CheckConstraintUnvalidated) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*CheckConstraintUnvalidated); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + func (e Column) element() {} // ForEachColumn iterates over elements of type Column. diff --git a/pkg/sql/schemachanger/scpb/uml/table.puml b/pkg/sql/schemachanger/scpb/uml/table.puml index a74acc012794..71577376d69f 100644 --- a/pkg/sql/schemachanger/scpb/uml/table.puml +++ b/pkg/sql/schemachanger/scpb/uml/table.puml @@ -99,6 +99,13 @@ CheckConstraint : Expression CheckConstraint : FromHashShardedColumn CheckConstraint : IndexIDForValidation +object CheckConstraintUnvalidated + +CheckConstraintUnvalidated : TableID +CheckConstraintUnvalidated : ConstraintID +CheckConstraintUnvalidated : []ColumnIDs +CheckConstraintUnvalidated : Expression + object ForeignKeyConstraint ForeignKeyConstraint : TableID @@ -369,6 +376,7 @@ Table <|-- TemporaryIndex View <|-- TemporaryIndex Table <|-- UniqueWithoutIndexConstraint Table <|-- CheckConstraint +Table <|-- CheckConstraintUnvalidated Table <|-- ForeignKeyConstraint Table <|-- TableComment View <|-- TableComment diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel index 838df49bb5e8..5775b3cb85d3 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel +++ b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel @@ -8,6 +8,7 @@ go_library( "op_gen.go", "opgen_alias_type.go", "opgen_check_constraint.go", + "opgen_check_constraint_unvalidated.go", "opgen_column.go", "opgen_column_comment.go", "opgen_column_default_expression.go", diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go index 787e6b7b0964..00f7a7878edd 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint.go @@ -11,6 +11,7 @@ package opgen import ( + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" ) @@ -20,13 +21,14 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_WRITE_ONLY, - emit(func(this *scpb.CheckConstraint) *scop.MakeAbsentCheckConstraintWriteOnly { - return &scop.MakeAbsentCheckConstraintWriteOnly{ + emit(func(this *scpb.CheckConstraint) *scop.AddCheckConstraint { + return &scop.AddCheckConstraint{ TableID: this.TableID, ConstraintID: this.ConstraintID, ColumnIDs: this.ColumnIDs, CheckExpr: this.Expr, FromHashShardedColumn: this.FromHashShardedColumn, + Validity: descpb.ConstraintValidity_Validating, } }), emit(func(this *scpb.CheckConstraint) *scop.UpdateTableBackReferencesInTypes { diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint_unvalidated.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint_unvalidated.go new file mode 100644 index 000000000000..d198faf30b31 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_check_constraint_unvalidated.go @@ -0,0 +1,83 @@ +// 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/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.CheckConstraintUnvalidated)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.CheckConstraintUnvalidated) *scop.AddCheckConstraint { + return &scop.AddCheckConstraint{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + ColumnIDs: this.ColumnIDs, + CheckExpr: this.Expr, + Validity: descpb.ConstraintValidity_Unvalidated, + } + }), + emit(func(this *scpb.CheckConstraintUnvalidated) *scop.UpdateTableBackReferencesInTypes { + if len(this.UsesTypeIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInTypes{ + TypeIDs: this.UsesTypeIDs, + BackReferencedTableID: this.TableID, + } + }), + emit(func(this *scpb.CheckConstraintUnvalidated) *scop.UpdateTableBackReferencesInSequences { + if len(this.UsesSequenceIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInSequences{ + SequenceIDs: this.UsesSequenceIDs, + BackReferencedTableID: this.TableID, + } + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.CheckConstraintUnvalidated) *scop.RemoveCheckConstraint { + return &scop.RemoveCheckConstraint{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + } + }), + emit(func(this *scpb.CheckConstraintUnvalidated) *scop.UpdateTableBackReferencesInTypes { + if len(this.UsesTypeIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInTypes{ + TypeIDs: this.UsesTypeIDs, + BackReferencedTableID: this.TableID, + } + }), + emit(func(this *scpb.CheckConstraintUnvalidated) *scop.UpdateTableBackReferencesInSequences { + if len(this.UsesSequenceIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInSequences{ + SequenceIDs: this.UsesSequenceIDs, + BackReferencedTableID: this.TableID, + } + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/assertions_test.go b/pkg/sql/schemachanger/scplan/internal/rules/current/assertions_test.go index bd4d57f704ba..3ef90b26a7e0 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/assertions_test.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/assertions_test.go @@ -185,11 +185,16 @@ func checkIsConstraintDependent(e scpb.Element) error { return nil } -// Assert that any element with a "ConstraintID" attr is either a constraint -// or a constraintDependent. -// If it's a constraint, then -// - it's either a NonIndexBackedConstraint or isIndex -// - it's either a CrossDescriptorConstraint or it does not a referencedDescID|ReferencedSequenceIDs|ReferencedTypeIDs attr +// Assert the following partitions about constraints: +// 1. An element `e` with ConstraintID attr is either a constraint +// or a constraint dependent. +// 2. A constraint is either index-backed or non-index-backed. +// +// TODO (xiang): Add test for cross-descriptor partition. We currently +// cannot have them until we added referenced.*ID attr for +// UniqueWithoutIndex[NotValid] element, which is required to support +// partial unique without index constraint with a predicate that references +// other descriptors. func checkConstraintPartitions(e scpb.Element) error { _, err := screl.Schema.GetAttribute(screl.ConstraintID, e) if err != nil { @@ -199,27 +204,10 @@ func checkConstraintPartitions(e scpb.Element) error { return errors.New("has ConstraintID attr but is not a constraint nor a constraint dependent") } if isConstraintDependent(e) { - // The checks below only apply to constraints, so skip constraint dependents. return nil } - if isNonIndexBackedConstraint(e) == isIndex(e) { - if isIndex(e) { - return errors.New("verifies both isIndex and isNonIndexBackedConstraint") - } else { - return errors.New("doesn't verify isIndex nor isNonIndexBackedConstraint") - } - } - _, err1 := screl.Schema.GetAttribute(screl.ReferencedDescID, e) - _, err2 := screl.Schema.GetAttribute(screl.ReferencedSequenceIDs, e) - _, err3 := screl.Schema.GetAttribute(screl.ReferencedTypeIDs, e) - if isCrossDescriptorConstraint(e) { - if err1 != nil && err2 != nil && err3 != nil { - return errors.New("verifies isCrossDescriptorConstraint but does not have a Referenced.*ID attr") - } - } else { - if err1 == nil || err2 == nil || err3 == nil { - return errors.New("doesn't verify isCrossDescriptorConstraint but has a Referenced.*ID attr") - } + if !isNonIndexBackedConstraint(e) && !isIndex(e) { + return errors.New("verifies isConstraint but does not verify isNonIndexBackedConstraint nor isIndex") } return nil } diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_constraint.go b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_constraint.go index 81e89f3a8b79..5936f5d333fa 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_constraint.go @@ -21,13 +21,27 @@ import ( // name, etc. appear once the constraint reaches a suitable state. func init() { registerDepRule( - "constraint dependent public right before constraint", + "constraint dependent public right before complex constraint", scgraph.SameStagePrecedence, - "dependent", "constraint", + "dependent", "complex-constraint", func(from, to NodeVars) rel.Clauses { return rel.Clauses{ from.TypeFilter(rulesVersionKey, isConstraintDependent), - to.TypeFilter(rulesVersionKey, isConstraint), + to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, isSubjectTo2VersionInvariant), + JoinOnConstraintID(from, to, "table-id", "constraint-id"), + StatusesToPublicOrTransient(from, scpb.Status_PUBLIC, to, scpb.Status_PUBLIC), + } + }, + ) + + registerDepRule( + "simple constraint public right before its dependents", + scgraph.SameStagePrecedence, + "simple-constraint", "dependent", + func(from, to NodeVars) rel.Clauses { + return rel.Clauses{ + from.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, Not(isNonIndexBackedCrossDescriptorConstraint)), + to.TypeFilter(rulesVersionKey, isConstraintDependent), JoinOnConstraintID(from, to, "table-id", "constraint-id"), StatusesToPublicOrTransient(from, scpb.Status_PUBLIC, to, scpb.Status_PUBLIC), } diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_index_and_constraint.go b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_index_and_constraint.go index 2cae27f166bd..17a2519d9ad0 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_index_and_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_index_and_constraint.go @@ -28,7 +28,7 @@ func init() { func(from, to NodeVars) rel.Clauses { return rel.Clauses{ from.Type((*scpb.PrimaryIndex)(nil)), - to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint), + to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, isSubjectTo2VersionInvariant), JoinOnDescID(from, to, "table-id"), JoinOn( from, screl.IndexID, diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_constraint.go b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_constraint.go index c3a696572ee6..24f003668b43 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_constraint.go @@ -19,9 +19,6 @@ import ( // These rules ensure that constraint-dependent elements, like an constraint's // name, etc. disappear once the constraint reaches a suitable state. -// TODO (xiang): The dep rules here are not complete, as they are aimed specifically -// for check constraints only. Complete them when we properly support the -// other constraints: UniqueWithoutIndex, ForeignKey, Unique, and Not Null. func init() { registerDepRuleForDrop( @@ -31,7 +28,7 @@ func init() { scpb.Status_VALIDATED, scpb.Status_ABSENT, func(from, to NodeVars) rel.Clauses { return rel.Clauses{ - from.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint), + from.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, isSubjectTo2VersionInvariant), to.TypeFilter(rulesVersionKey, isConstraintDependent), JoinOnConstraintID(from, to, "table-id", "constraint-id"), } @@ -46,7 +43,26 @@ func init() { func(from, to NodeVars) rel.Clauses { return rel.Clauses{ from.TypeFilter(rulesVersionKey, isConstraintDependent), - to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint), + to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, isSubjectTo2VersionInvariant), + JoinOnConstraintID(from, to, "table-id", "constraint-id"), + } + }, + ) +} + +// These rules apply to simple constraints and ensure that their dependents, like +// their names, comments, etc., disappear right before the simple constraint. +func init() { + + registerDepRuleForDrop( + "dependents removed right before simple constraint", + scgraph.SameStagePrecedence, + "dependents", "constraint", + scpb.Status_ABSENT, scpb.Status_ABSENT, + func(from, to NodeVars) rel.Clauses { + return rel.Clauses{ + from.TypeFilter(rulesVersionKey, isConstraintDependent), + to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, Not(isSubjectTo2VersionInvariant)), JoinOnConstraintID(from, to, "table-id", "constraint-id"), } }, diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_object.go b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_object.go index 48c298daa725..0229b719bc07 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_object.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/dep_drop_object.go @@ -88,7 +88,7 @@ func init() { func(from, to NodeVars) rel.Clauses { return rel.Clauses{ from.Type((*scpb.Table)(nil)), - to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, Not(isCrossDescriptorConstraint)), + to.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, isSubjectTo2VersionInvariant, Not(isNonIndexBackedCrossDescriptorConstraint)), JoinOnDescID(from, to, "desc-id"), StatusesToAbsent(from, scpb.Status_DROPPED, to, scpb.Status_VALIDATED), } @@ -239,7 +239,7 @@ func init() { "cross-desc-constraint", "referenced-descriptor", func(from, to NodeVars) rel.Clauses { return rel.Clauses{ - from.TypeFilter(rulesVersionKey, isCrossDescriptorConstraint), + from.TypeFilter(rulesVersionKey, isNonIndexBackedCrossDescriptorConstraint, isSubjectTo2VersionInvariant), to.TypeFilter(rulesVersionKey, isDescriptor), JoinReferencedDescID(from, to, "desc-id"), StatusesToAbsent(from, scpb.Status_ABSENT, to, scpb.Status_DROPPED), @@ -253,7 +253,7 @@ func init() { "cross-desc-constraint", "referencing-descriptor", func(from, to NodeVars) rel.Clauses { return rel.Clauses{ - from.TypeFilter(rulesVersionKey, isCrossDescriptorConstraint), + from.TypeFilter(rulesVersionKey, isNonIndexBackedCrossDescriptorConstraint, isSubjectTo2VersionInvariant), to.TypeFilter(rulesVersionKey, isDescriptor), JoinOnDescID(from, to, "desc-id"), StatusesToAbsent(from, scpb.Status_ABSENT, to, scpb.Status_DROPPED), diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go index 7ee84882c1e2..171d6067a392 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go @@ -65,7 +65,15 @@ func isSubjectTo2VersionInvariant(e scpb.Element) bool { // TODO(ajwerner): This should include constraints and enum values but it // currently does not because we do not support dropping them unless we're // dropping the descriptor and we do not support adding them. - return isIndex(e) || isColumn(e) || isNonIndexBackedConstraint(e) + if isIndex(e) || isColumn(e) { + return true + } + switch e.(type) { + case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, *scpb.ForeignKeyConstraint, + *scpb.ColumnNotNull: + return true + } + return false } func isIndex(e scpb.Element) bool { @@ -133,6 +141,11 @@ func getExpression(element scpb.Element) (*scpb.Expression, error) { return nil, nil } return &e.Expression, nil + case *scpb.CheckConstraintUnvalidated: + if e == nil { + return nil, nil + } + return &e.Expression, nil case *scpb.FunctionParamDefaultExpression: if e == nil { return nil, nil @@ -184,36 +197,42 @@ func isIndexDependent(e scpb.Element) bool { return false } -// isNonIndexBackedConstraint a non-index-backed constraint. -func isNonIndexBackedConstraint(e scpb.Element) bool { - switch e.(type) { - case *scpb.CheckConstraint, *scpb.ForeignKeyConstraint, *scpb.UniqueWithoutIndexConstraint, - *scpb.ColumnNotNull: - return true - } - return false +// CRDB supports five constraints of two categories: +// - PK, Unique (index-backed) +// - Check, UniqueWithoutIndex, FK (non-index-backed) +func isConstraint(e scpb.Element) bool { + return isIndex(e) || isNonIndexBackedConstraint(e) } -func isConstraint(e scpb.Element) bool { +// isNonIndexBackedConstraint returns true if `e` is a non-index-backed constraint. +func isNonIndexBackedConstraint(e scpb.Element) bool { switch e.(type) { - case *scpb.PrimaryIndex, *scpb.SecondaryIndex, *scpb.TemporaryIndex: + case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, *scpb.ForeignKeyConstraint, + *scpb.ColumnNotNull: return true - case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, *scpb.ForeignKeyConstraint: + case *scpb.CheckConstraintUnvalidated: return true } return false } -// isCrossDescriptorConstraint are constraints that might reference -// other descriptors: -// - FKs can reference other table -// - Checks can reference other sequences/types +// isNonIndexBackedCrossDescriptorConstraint returns true if `e` is a +// non-index-backed constraint and it can potentially reference another +// descriptor. +// +// This filter exists because in general we need to drop the constraint first +// before dropping referencing/referenced descriptor. Read rules that use +// this filter for more details. // -// Those constraints need to be dropped first when we are dropping either -// the referencing or reference descriptor. -func isCrossDescriptorConstraint(e scpb.Element) bool { +// TODO (xiang): UniqueWithoutIndex and UniqueWithoutIndexNotValid should +// also be treated as cross-descriptor constraint because its partial predicate +// can references other descriptors. +func isNonIndexBackedCrossDescriptorConstraint(e scpb.Element) bool { switch e.(type) { - case *scpb.ForeignKeyConstraint, *scpb.CheckConstraint: + case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, + *scpb.ForeignKeyConstraint: + return true + case *scpb.CheckConstraintUnvalidated: return true } return false diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go b/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go index e10be4805636..a410c0c1f162 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/op_drop.go @@ -168,7 +168,7 @@ func init() { (*scpb.Table)(nil), (*scpb.View)(nil), ), - constraint.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint), + constraint.TypeFilter(rulesVersionKey, isNonIndexBackedConstraint, Not(isNonIndexBackedCrossDescriptorConstraint)), JoinOnDescID(relation, constraint, relationID), diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules index a7a242800857..bd8205c13243 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules @@ -1871,19 +1871,19 @@ deprules - $column-constraint-Node[CurrentStatus] = WRITE_ONLY - joinTargetNode($column, $column-Target, $column-Node) - joinTargetNode($column-constraint, $column-constraint-Target, $column-constraint-Node) -- name: constraint dependent public right before constraint +- name: constraint dependent public right before complex constraint from: dependent-Node kind: SameStagePrecedence - to: constraint-Node + to: complex-constraint-Node query: - $dependent[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] - - $constraint[Type] IN ['*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint'] - - joinOnConstraintID($dependent, $constraint, $table-id, $constraint-id) - - ToPublicOrTransient($dependent-Target, $constraint-Target) + - $complex-constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint', '*scpb.ColumnNotNull'] + - joinOnConstraintID($dependent, $complex-constraint, $table-id, $constraint-id) + - ToPublicOrTransient($dependent-Target, $complex-constraint-Target) - $dependent-Node[CurrentStatus] = PUBLIC - - $constraint-Node[CurrentStatus] = PUBLIC + - $complex-constraint-Node[CurrentStatus] = PUBLIC - joinTargetNode($dependent, $dependent-Target, $dependent-Node) - - joinTargetNode($constraint, $constraint-Target, $constraint-Node) + - joinTargetNode($complex-constraint, $complex-constraint-Target, $complex-constraint-Node) - name: constraint no longer public before dependents from: constraint-Node kind: Precedence @@ -1943,7 +1943,7 @@ deprules kind: Precedence to: referenced-descriptor-Node query: - - $cross-desc-constraint[Type] IN ['*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint'] + - $cross-desc-constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint'] - $referenced-descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinReferencedDescID($cross-desc-constraint, $referenced-descriptor, $desc-id) - toAbsent($cross-desc-constraint-Target, $referenced-descriptor-Target) @@ -1956,7 +1956,7 @@ deprules kind: Precedence to: referencing-descriptor-Node query: - - $cross-desc-constraint[Type] IN ['*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint'] + - $cross-desc-constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.ForeignKeyConstraint'] - $referencing-descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinOnDescID($cross-desc-constraint, $referencing-descriptor, $desc-id) - toAbsent($cross-desc-constraint-Target, $referencing-descriptor-Target) @@ -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.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.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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) @@ -2247,13 +2247,67 @@ deprules - $index-Node[CurrentStatus] = TRANSIENT_ABSENT - joinTargetNode($dependent, $dependent-Target, $dependent-Node) - joinTargetNode($index, $index-Target, $index-Node) +- name: dependents removed right before simple constraint + from: dependents-Node + kind: SameStagePrecedence + to: constraint-Node + query: + - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] + - $constraint[Type] = '*scpb.CheckConstraintUnvalidated' + - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) + - toAbsent($dependents-Target, $constraint-Target) + - $dependents-Node[CurrentStatus] = ABSENT + - $constraint-Node[CurrentStatus] = ABSENT + - joinTargetNode($dependents, $dependents-Target, $dependents-Node) + - joinTargetNode($constraint, $constraint-Target, $constraint-Node) +- name: dependents removed right before simple constraint + from: dependents-Node + kind: SameStagePrecedence + to: constraint-Node + query: + - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] + - $constraint[Type] = '*scpb.CheckConstraintUnvalidated' + - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) + - transient($dependents-Target, $constraint-Target) + - $dependents-Node[CurrentStatus] = TRANSIENT_ABSENT + - $constraint-Node[CurrentStatus] = TRANSIENT_ABSENT + - joinTargetNode($dependents, $dependents-Target, $dependents-Node) + - joinTargetNode($constraint, $constraint-Target, $constraint-Node) +- name: dependents removed right before simple constraint + from: dependents-Node + kind: SameStagePrecedence + to: constraint-Node + query: + - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] + - $constraint[Type] = '*scpb.CheckConstraintUnvalidated' + - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) + - $dependents-Target[TargetStatus] = TRANSIENT_ABSENT + - $dependents-Node[CurrentStatus] = TRANSIENT_ABSENT + - $constraint-Target[TargetStatus] = ABSENT + - $constraint-Node[CurrentStatus] = ABSENT + - joinTargetNode($dependents, $dependents-Target, $dependents-Node) + - joinTargetNode($constraint, $constraint-Target, $constraint-Node) +- name: dependents removed right before simple constraint + from: dependents-Node + kind: SameStagePrecedence + to: constraint-Node + query: + - $dependents[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] + - $constraint[Type] = '*scpb.CheckConstraintUnvalidated' + - joinOnConstraintID($dependents, $constraint, $table-id, $constraint-id) + - $dependents-Target[TargetStatus] = ABSENT + - $dependents-Node[CurrentStatus] = ABSENT + - $constraint-Target[TargetStatus] = TRANSIENT_ABSENT + - $constraint-Node[CurrentStatus] = TRANSIENT_ABSENT + - joinTargetNode($dependents, $dependents-Target, $dependents-Node) + - joinTargetNode($constraint, $constraint-Target, $constraint-Node) - name: descriptor drop right before removing dependent with attr ref from: referenced-descriptor-Node kind: SameStagePrecedence 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.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.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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 @@ -2268,7 +2322,7 @@ deprules - $referenced-descriptor[Type] = '*scpb.Sequence' - $referenced-descriptor[DescID] = $seqID - $referencing-via-expr[ReferencedSequenceIDs] CONTAINS $seqID - - $referencing-via-expr[Type] IN ['*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial', '*scpb.FunctionParamDefaultExpression'] + - $referencing-via-expr[Type] IN ['*scpb.CheckConstraintUnvalidated', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial', '*scpb.FunctionParamDefaultExpression'] - toAbsent($referenced-descriptor-Target, $referencing-via-expr-Target) - $referenced-descriptor-Node[CurrentStatus] = DROPPED - $referencing-via-expr-Node[CurrentStatus] = ABSENT @@ -2282,7 +2336,7 @@ deprules - $referenced-descriptor[Type] IN ['*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType'] - $referenced-descriptor[DescID] = $fromDescID - $referencing-via-type[ReferencedTypeIDs] CONTAINS $fromDescID - - $referencing-via-type[Type] IN ['*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial', '*scpb.FunctionParamDefaultExpression'] + - $referencing-via-type[Type] IN ['*scpb.CheckConstraintUnvalidated', '*scpb.ColumnType', '*scpb.ColumnDefaultExpression', '*scpb.ColumnOnUpdateExpression', '*scpb.SecondaryIndexPartial', '*scpb.FunctionParamDefaultExpression'] - toAbsent($referenced-descriptor-Target, $referencing-via-type-Target) - $referenced-descriptor-Node[CurrentStatus] = DROPPED - $referencing-via-type-Node[CurrentStatus] = ABSENT @@ -2294,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.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.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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 @@ -2333,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.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.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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 @@ -2691,7 +2745,7 @@ deprules kind: Precedence to: descriptor-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.TableZoneConfig', '*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.ConstraintWithoutIndexName', '*scpb.ConstraintComment', '*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.Column', '*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.ColumnNotNull', '*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.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinOnDescID($dependent, $descriptor, $desc-id) - toAbsent($dependent-Target, $descriptor-Target) @@ -2887,7 +2941,7 @@ deprules to: constraint-Node query: - $descriptor[Type] = '*scpb.Table' - - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraint', '*scpb.ColumnNotNull'] + - $constraint[Type] = '*scpb.ColumnNotNull' - joinOnDescID($descriptor, $constraint, $desc-id) - toAbsent($descriptor-Target, $constraint-Target) - $descriptor-Node[CurrentStatus] = DROPPED @@ -3102,6 +3156,19 @@ deprules - isIndexKeyColumnKey(*scpb.IndexColumn)($index-column) - joinTargetNode($index, $index-Target, $index-Node) - joinTargetNode($column, $column-Target, $column-Node) +- name: simple constraint public right before its dependents + from: simple-constraint-Node + kind: SameStagePrecedence + to: dependent-Node + query: + - $simple-constraint[Type] = '*scpb.ColumnNotNull' + - $dependent[Type] IN ['*scpb.ConstraintWithoutIndexName', '*scpb.ConstraintComment'] + - joinOnConstraintID($simple-constraint, $dependent, $table-id, $constraint-id) + - ToPublicOrTransient($simple-constraint-Target, $dependent-Target) + - $simple-constraint-Node[CurrentStatus] = PUBLIC + - $dependent-Node[CurrentStatus] = PUBLIC + - joinTargetNode($simple-constraint, $simple-constraint-Target, $simple-constraint-Node) + - joinTargetNode($dependent, $dependent-Target, $dependent-Node) - name: swapped primary index public before column from: index-Node kind: Precedence diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/oprules index bf6b67837417..7d75810491d8 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.ColumnNotNull'] + - $constraint[Type] = '*scpb.ColumnNotNull' - joinOnDescID($relation, $constraint, $relation-id) - joinTarget($relation, $relation-Target) - $relation-Target[TargetStatus] = ABSENT diff --git a/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check b/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check index 3ca67250a759..39424720aedd 100644 --- a/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check +++ b/pkg/sql/schemachanger/scplan/testdata/alter_table_add_check @@ -9,12 +9,13 @@ StatementPhase stage 1 of 1 with 1 MutationType op transitions: [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, PUBLIC], ABSENT] -> WRITE_ONLY ops: - *scop.MakeAbsentCheckConstraintWriteOnly + *scop.AddCheckConstraint CheckExpr: i > 0:::INT8 ColumnIDs: - 1 ConstraintID: 2 TableID: 104 + Validity: 2 PreCommitPhase stage 1 of 2 with 1 MutationType op transitions: [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, PUBLIC], WRITE_ONLY] -> ABSENT @@ -25,12 +26,13 @@ PreCommitPhase stage 2 of 2 with 3 MutationType ops transitions: [[CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, PUBLIC], ABSENT] -> WRITE_ONLY ops: - *scop.MakeAbsentCheckConstraintWriteOnly + *scop.AddCheckConstraint CheckExpr: i > 0:::INT8 ColumnIDs: - 1 ConstraintID: 2 TableID: 104 + Validity: 2 *scop.SetJobStateOnDescriptor DescriptorID: 104 Initialize: true @@ -91,4 +93,4 @@ ALTER TABLE t ADD CHECK (i > 0) - from: [ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2}, PUBLIC] to: [CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2}, PUBLIC] kind: SameStagePrecedence - rule: constraint dependent public right before constraint + rule: constraint dependent public right before complex constraint diff --git a/pkg/sql/schemachanger/scplan/testdata/drop_table b/pkg/sql/schemachanger/scplan/testdata/drop_table index 8cf6e09b015a..57d74d0bdfd1 100644 --- a/pkg/sql/schemachanger/scplan/testdata/drop_table +++ b/pkg/sql/schemachanger/scplan/testdata/drop_table @@ -29,7 +29,7 @@ COMMENT ON CONSTRAINT fk_customers ON defaultdb.shipments IS 'customer is not go ops DROP TABLE defaultdb.shipments CASCADE; ---- -StatementPhase stage 1 of 1 with 5 MutationType ops +StatementPhase stage 1 of 1 with 7 MutationType ops transitions: [[ForeignKeyConstraint:{DescID: 109, IndexID: 0, ConstraintID: 2, ReferencedDescID: 104}, ABSENT], PUBLIC] -> VALIDATED [[ConstraintWithoutIndexName:{DescID: 109, Name: fk_customers, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT @@ -55,9 +55,15 @@ StatementPhase stage 1 of 1 with 5 MutationType ops [[ColumnName:{DescID: 111, Name: tableoid, ColumnID: 4294967294}, ABSENT], PUBLIC] -> ABSENT [[ColumnType:{DescID: 111, ColumnFamilyID: 0, ColumnID: 4294967294}, ABSENT], PUBLIC] -> ABSENT ops: + *scop.MakePublicForeignKeyConstraintValidated + ConstraintID: 2 + TableID: 109 *scop.RemoveConstraintComment ConstraintID: 2 TableID: 109 + *scop.MakePublicForeignKeyConstraintValidated + ConstraintID: 3 + TableID: 109 *scop.MarkDescriptorAsDropped DescriptorID: 111 *scop.RemoveBackReferencesInRelations @@ -101,7 +107,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 47 MutationType ops +PreCommitPhase stage 2 of 2 with 49 MutationType ops transitions: [[Namespace:{DescID: 109, Name: shipments, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 109}, ABSENT], PUBLIC] -> ABSENT @@ -181,9 +187,15 @@ PreCommitPhase stage 2 of 2 with 47 MutationType ops [[ColumnName:{DescID: 111, Name: tableoid, ColumnID: 4294967294}, ABSENT], PUBLIC] -> ABSENT [[ColumnType:{DescID: 111, ColumnFamilyID: 0, ColumnID: 4294967294}, ABSENT], PUBLIC] -> ABSENT ops: + *scop.MakePublicForeignKeyConstraintValidated + ConstraintID: 2 + TableID: 109 *scop.RemoveConstraintComment ConstraintID: 2 TableID: 109 + *scop.MakePublicForeignKeyConstraintValidated + ConstraintID: 3 + TableID: 109 *scop.MarkDescriptorAsDropped DescriptorID: 110 *scop.RemoveObjectParent @@ -1481,11 +1493,14 @@ CREATE TABLE defaultdb.greeter ( ops DROP TABLE defaultdb.greeter ---- -StatementPhase stage 1 of 1 with no ops +StatementPhase stage 1 of 1 with 1 MutationType op transitions: [[CheckConstraint:{DescID: 114, ReferencedTypeIDs: [112 113], IndexID: 0, ConstraintID: 2}, ABSENT], PUBLIC] -> VALIDATED [[ConstraintWithoutIndexName:{DescID: 114, Name: check, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT - no ops + ops: + *scop.MakePublicCheckConstraintValidated + ConstraintID: 2 + TableID: 114 PreCommitPhase stage 1 of 2 with 1 MutationType op transitions: [[CheckConstraint:{DescID: 114, ReferencedTypeIDs: [112 113], IndexID: 0, ConstraintID: 2}, ABSENT], VALIDATED] -> PUBLIC @@ -1493,7 +1508,7 @@ PreCommitPhase stage 1 of 2 with 1 MutationType op ops: *scop.UndoAllInTxnImmediateMutationOpSideEffects {} -PreCommitPhase stage 2 of 2 with 23 MutationType ops +PreCommitPhase stage 2 of 2 with 24 MutationType ops transitions: [[Namespace:{DescID: 114, Name: greeter, ReferencedDescID: 100}, ABSENT], PUBLIC] -> ABSENT [[Owner:{DescID: 114}, ABSENT], PUBLIC] -> ABSENT @@ -1534,6 +1549,9 @@ PreCommitPhase stage 2 of 2 with 23 MutationType ops [[CheckConstraint:{DescID: 114, ReferencedTypeIDs: [112 113], IndexID: 0, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT [[ConstraintWithoutIndexName:{DescID: 114, Name: check, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT ops: + *scop.MakePublicCheckConstraintValidated + ConstraintID: 2 + TableID: 114 *scop.RemoveCheckConstraint ConstraintID: 2 TableID: 114 diff --git a/pkg/sql/schemachanger/screl/attr.go b/pkg/sql/schemachanger/screl/attr.go index 48ba02405ae2..99a103a833c2 100644 --- a/pkg/sql/schemachanger/screl/attr.go +++ b/pkg/sql/schemachanger/screl/attr.go @@ -185,6 +185,12 @@ var elementSchemaOptions = []rel.SchemaOption{ rel.EntityAttr(ReferencedTypeIDs, "UsesTypeIDs"), rel.EntityAttr(IndexID, "IndexIDForValidation"), ), + rel.EntityMapping(t((*scpb.CheckConstraintUnvalidated)(nil)), + rel.EntityAttr(DescID, "TableID"), + rel.EntityAttr(ConstraintID, "ConstraintID"), + rel.EntityAttr(ReferencedSequenceIDs, "UsesSequenceIDs"), + rel.EntityAttr(ReferencedTypeIDs, "UsesTypeIDs"), + ), rel.EntityMapping(t((*scpb.ForeignKeyConstraint)(nil)), rel.EntityAttr(DescID, "TableID"), rel.EntityAttr(ReferencedDescID, "ReferencedTableID"), diff --git a/pkg/sql/schemachanger/screl/scalars.go b/pkg/sql/schemachanger/screl/scalars.go index 82ab6df42a08..3c3832e81d9c 100644 --- a/pkg/sql/schemachanger/screl/scalars.go +++ b/pkg/sql/schemachanger/screl/scalars.go @@ -124,7 +124,7 @@ 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: + case *scpb.ColumnNotNull, *scpb.CheckConstraintUnvalidated: 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 3ee99c25c063..00129cbff071 100644 --- a/pkg/sql/schemachanger/sctest_generated_test.go +++ b/pkg/sql/schemachanger/sctest_generated_test.go @@ -120,6 +120,31 @@ func TestRollback_add_column_no_default(t *testing.T) { defer log.Scope(t).Close(t) sctest.Rollback(t, "pkg/sql/schemachanger/testdata/end_to_end/add_column_no_default", sctest.SingleNodeCluster) } +func TestEndToEndSideEffects_alter_table_add_check_unvalidated(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_check_unvalidated", sctest.SingleNodeCluster) +} +func TestExecuteWithDMLInjection_alter_table_add_check_unvalidated(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_check_unvalidated", sctest.SingleNodeCluster) +} +func TestGenerateSchemaChangeCorpus_alter_table_add_check_unvalidated(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_check_unvalidated", sctest.SingleNodeCluster) +} +func TestPause_alter_table_add_check_unvalidated(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_check_unvalidated", sctest.SingleNodeCluster) +} +func TestRollback_alter_table_add_check_unvalidated(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_check_unvalidated", sctest.SingleNodeCluster) +} func TestEndToEndSideEffects_alter_table_add_check_vanilla(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_check_unvalidated b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_unvalidated new file mode 100644 index 000000000000..0aa1883245fd --- /dev/null +++ b/pkg/sql/schemachanger/testdata/end_to_end/alter_table_add_check_unvalidated @@ -0,0 +1,80 @@ +setup +CREATE TABLE t (i INT PRIMARY KEY); +INSERT INTO t VALUES (0); +---- +... ++object {100 101 t} -> 104 + + +test +ALTER TABLE t ADD CHECK (i > 0) 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›.‹t› ADD CHECK (‹i› > ‹0›) NOT VALID + tag: ALTER TABLE + user: root + tableName: defaultdb.public.t +## StatementPhase stage 1 of 1 with 2 MutationType ops +upsert descriptor #104 + table: + + checks: + + - columnIds: + + - 1 + + constraintId: 2 + + expr: i > 0:::INT8 + + name: check_i + + validity: Unvalidated + columns: + - id: 1 + ... + name: t + nextColumnId: 2 + - nextConstraintId: 2 + + nextConstraintId: 3 + nextFamilyId: 1 + nextIndexId: 2 + ... + 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 + table: + + checks: + + - columnIds: + + - 1 + + constraintId: 2 + + expr: i > 0:::INT8 + + name: check_i + + validity: Unvalidated + columns: + - id: 1 + ... + name: t + nextColumnId: 2 + - nextConstraintId: 2 + + nextConstraintId: 3 + nextFamilyId: 1 + nextIndexId: 2 + ... + 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_check_unvalidated b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_unvalidated new file mode 100644 index 000000000000..7c7cc1290860 --- /dev/null +++ b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_unvalidated @@ -0,0 +1,30 @@ +/* setup */ +CREATE TABLE t (i INT PRIMARY KEY); +INSERT INTO t VALUES (0); + +/* test */ +EXPLAIN (ddl) ALTER TABLE t ADD CHECK (i > 0) NOT VALID; +---- +Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHECK (‹i› > ‹0›) NOT VALID; + ├── StatementPhase + │ └── Stage 1 of 1 in StatementPhase + │ ├── 2 elements transitioning toward PUBLIC + │ │ ├── ABSENT → PUBLIC CheckConstraintUnvalidated:{DescID: 104, ConstraintID: 2} + │ │ └── ABSENT → PUBLIC ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} + │ └── 2 Mutation operations + │ ├── AddCheckConstraint {"CheckExpr":"i \u003e 0:::INT8","ConstraintID":2,"TableID":104,"Validity":1} + │ └── SetConstraintName {"ConstraintID":2,"Name":"check_i","TableID":104} + └── PreCommitPhase + ├── Stage 1 of 2 in PreCommitPhase + │ ├── 2 elements transitioning toward PUBLIC + │ │ ├── PUBLIC → ABSENT CheckConstraintUnvalidated:{DescID: 104, ConstraintID: 2} + │ │ └── PUBLIC → ABSENT ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} + │ └── 1 Mutation operation + │ └── UndoAllInTxnImmediateMutationOpSideEffects + └── Stage 2 of 2 in PreCommitPhase + ├── 2 elements transitioning toward PUBLIC + │ ├── ABSENT → PUBLIC CheckConstraintUnvalidated:{DescID: 104, ConstraintID: 2} + │ └── ABSENT → PUBLIC ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} + └── 2 Mutation operations + ├── AddCheckConstraint {"CheckExpr":"i \u003e 0:::INT8","ConstraintID":2,"TableID":104,"Validity":1} + └── SetConstraintName {"ConstraintID":2,"Name":"check_i","TableID":104} diff --git a/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_vanilla b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_vanilla index 14d256a4d305..fc4103302277 100644 --- a/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_vanilla +++ b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_vanilla @@ -11,7 +11,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2} │ └── 1 Mutation operation - │ └── MakeAbsentCheckConstraintWriteOnly {"CheckExpr":"i \u003e 0:::INT8","ConstraintID":2,"TableID":104} + │ └── AddCheckConstraint {"CheckExpr":"i \u003e 0:::INT8","ConstraintID":2,"TableID":104,"Validity":2} ├── PreCommitPhase │ ├── Stage 1 of 2 in PreCommitPhase │ │ ├── 1 element transitioning toward PUBLIC @@ -22,7 +22,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2} │ └── 3 Mutation operations - │ ├── MakeAbsentCheckConstraintWriteOnly {"CheckExpr":"i \u003e 0:::INT8","ConstraintID":2,"TableID":104} + │ ├── AddCheckConstraint {"CheckExpr":"i \u003e 0:::INT8","ConstraintID":2,"TableID":104,"Validity":2} │ ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true} │ └── CreateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."} └── PostCommitPhase diff --git a/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_with_seq_and_udt b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_with_seq_and_udt index fa287f7a365e..b772bfb84161 100644 --- a/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_with_seq_and_udt +++ b/pkg/sql/schemachanger/testdata/explain/alter_table_add_check_with_seq_and_udt @@ -12,7 +12,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 107, ReferencedTypeIDs: [105 106], IndexID: 0, ConstraintID: 2, ReferencedSequenceIDs: [104]} │ └── 3 Mutation operations - │ ├── MakeAbsentCheckConstraintWriteOnly {"CheckExpr":"(i \u003e nextval(104...","ConstraintID":2,"TableID":107} + │ ├── AddCheckConstraint {"CheckExpr":"(i \u003e nextval(104...","ConstraintID":2,"TableID":107,"Validity":2} │ ├── UpdateTableBackReferencesInTypes {"BackReferencedTableID":107} │ └── UpdateTableBackReferencesInSequences {"BackReferencedTableID":107} ├── PreCommitPhase @@ -25,7 +25,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHEC │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY CheckConstraint:{DescID: 107, ReferencedTypeIDs: [105 106], IndexID: 0, ConstraintID: 2, ReferencedSequenceIDs: [104]} │ └── 8 Mutation operations - │ ├── MakeAbsentCheckConstraintWriteOnly {"CheckExpr":"(i \u003e nextval(104...","ConstraintID":2,"TableID":107} + │ ├── AddCheckConstraint {"CheckExpr":"(i \u003e nextval(104...","ConstraintID":2,"TableID":107,"Validity":2} │ ├── UpdateTableBackReferencesInTypes {"BackReferencedTableID":107} │ ├── UpdateTableBackReferencesInSequences {"BackReferencedTableID":107} │ ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true} diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_unvalidated b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_unvalidated new file mode 100644 index 000000000000..54c2d0acc07a --- /dev/null +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_unvalidated @@ -0,0 +1,77 @@ +/* setup */ +CREATE TABLE t (i INT PRIMARY KEY); +INSERT INTO t VALUES (0); + +/* test */ +EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > 0) NOT VALID; +---- +• Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CHECK (‹i› > ‹0›) NOT VALID; +│ +├── • StatementPhase +│ │ +│ └── • Stage 1 of 1 in StatementPhase +│ │ +│ ├── • 2 elements transitioning toward PUBLIC +│ │ │ +│ │ ├── • CheckConstraintUnvalidated:{DescID: 104, ConstraintID: 2} +│ │ │ ABSENT → PUBLIC +│ │ │ +│ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} +│ │ ABSENT → PUBLIC +│ │ +│ └── • 2 Mutation operations +│ │ +│ ├── • AddCheckConstraint +│ │ CheckExpr: i > 0:::INT8 +│ │ ColumnIDs: +│ │ - 1 +│ │ ConstraintID: 2 +│ │ TableID: 104 +│ │ Validity: 1 +│ │ +│ └── • SetConstraintName +│ ConstraintID: 2 +│ Name: check_i +│ TableID: 104 +│ +└── • PreCommitPhase + │ + ├── • Stage 1 of 2 in PreCommitPhase + │ │ + │ ├── • 2 elements transitioning toward PUBLIC + │ │ │ + │ │ ├── • CheckConstraintUnvalidated:{DescID: 104, ConstraintID: 2} + │ │ │ PUBLIC → ABSENT + │ │ │ + │ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} + │ │ PUBLIC → ABSENT + │ │ + │ └── • 1 Mutation operation + │ │ + │ └── • UndoAllInTxnImmediateMutationOpSideEffects + │ {} + │ + └── • Stage 2 of 2 in PreCommitPhase + │ + ├── • 2 elements transitioning toward PUBLIC + │ │ + │ ├── • CheckConstraintUnvalidated:{DescID: 104, ConstraintID: 2} + │ │ ABSENT → PUBLIC + │ │ + │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} + │ ABSENT → PUBLIC + │ + └── • 2 Mutation operations + │ + ├── • AddCheckConstraint + │ CheckExpr: i > 0:::INT8 + │ ColumnIDs: + │ - 1 + │ ConstraintID: 2 + │ TableID: 104 + │ Validity: 1 + │ + └── • SetConstraintName + ConstraintID: 2 + Name: check_i + TableID: 104 diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_vanilla b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_vanilla index d5353238abb5..101b147d5492 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_vanilla +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_vanilla @@ -21,12 +21,13 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > 0) │ │ │ └── • 1 Mutation operation │ │ -│ └── • MakeAbsentCheckConstraintWriteOnly +│ └── • AddCheckConstraint │ CheckExpr: i > 0:::INT8 │ ColumnIDs: │ - 1 │ ConstraintID: 2 │ TableID: 104 +│ Validity: 2 │ ├── • PreCommitPhase │ │ @@ -54,12 +55,13 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > 0) │ │ │ └── • 3 Mutation operations │ │ -│ ├── • MakeAbsentCheckConstraintWriteOnly +│ ├── • AddCheckConstraint │ │ CheckExpr: i > 0:::INT8 │ │ ColumnIDs: │ │ - 1 │ │ ConstraintID: 2 │ │ TableID: 104 +│ │ Validity: 2 │ │ │ ├── • SetJobStateOnDescriptor │ │ DescriptorID: 104 @@ -106,7 +108,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > 0) │ │ │ rule: "CheckConstraint transitions to PUBLIC uphold 2-version invariant: VALIDATED->PUBLIC" │ │ │ │ │ └── • SameStagePrecedence dependency from PUBLIC ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} - │ │ rule: "constraint dependent public right before constraint" + │ │ rule: "constraint dependent public right before complex constraint" │ │ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: check_i, ConstraintID: 2} │ ABSENT → PUBLIC diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_with_seq_and_udt b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_with_seq_and_udt index 7ef1c09073c8..d9ed6731285c 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_with_seq_and_udt +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_check_with_seq_and_udt @@ -22,13 +22,14 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > nextval('s') OR j::typ = 'a' │ │ │ └── • 3 Mutation operations │ │ -│ ├── • MakeAbsentCheckConstraintWriteOnly +│ ├── • AddCheckConstraint │ │ CheckExpr: (i > nextval(104:::REGCLASS)) OR (j::@100105 = b'@':::@100105) │ │ ColumnIDs: │ │ - 1 │ │ - 2 │ │ ConstraintID: 2 │ │ TableID: 107 +│ │ Validity: 2 │ │ │ ├── • UpdateTableBackReferencesInTypes │ │ BackReferencedTableID: 107 @@ -67,13 +68,14 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > nextval('s') OR j::typ = 'a' │ │ │ └── • 8 Mutation operations │ │ -│ ├── • MakeAbsentCheckConstraintWriteOnly +│ ├── • AddCheckConstraint │ │ CheckExpr: (i > nextval(104:::REGCLASS)) OR (j::@100105 = b'@':::@100105) │ │ ColumnIDs: │ │ - 1 │ │ - 2 │ │ ConstraintID: 2 │ │ TableID: 107 +│ │ Validity: 2 │ │ │ ├── • UpdateTableBackReferencesInTypes │ │ BackReferencedTableID: 107 @@ -147,7 +149,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD CHECK (i > nextval('s') OR j::typ = 'a' │ │ │ rule: "CheckConstraint transitions to PUBLIC uphold 2-version invariant: VALIDATED->PUBLIC" │ │ │ │ │ └── • SameStagePrecedence dependency from PUBLIC ConstraintWithoutIndexName:{DescID: 107, Name: check_i_j, ConstraintID: 2} - │ │ rule: "constraint dependent public right before constraint" + │ │ rule: "constraint dependent public right before complex constraint" │ │ │ └── • ConstraintWithoutIndexName:{DescID: 107, Name: check_i_j, ConstraintID: 2} │ ABSENT → PUBLIC diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key index 2a44850c363e..09940e034d6b 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key @@ -116,7 +116,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i); │ │ │ rule: "ForeignKeyConstraint transitions to PUBLIC uphold 2-version invariant: VALIDATED->PUBLIC" │ │ │ │ │ └── • SameStagePrecedence dependency from PUBLIC ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} - │ │ rule: "constraint dependent public right before constraint" + │ │ rule: "constraint dependent public right before complex constraint" │ │ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: t1_i_fkey, ConstraintID: 2} │ ABSENT → PUBLIC diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index index 2fe495788b0c..ba7a056dd550 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index @@ -105,7 +105,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD UNIQUE WITHOUT INDEX (j); │ │ │ rule: "UniqueWithoutIndexConstraint transitions to PUBLIC uphold 2-version invariant: VALIDATED->PUBLIC" │ │ │ │ │ └── • SameStagePrecedence dependency from PUBLIC ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} - │ │ rule: "constraint dependent public right before constraint" + │ │ rule: "constraint dependent public right before complex constraint" │ │ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} │ ABSENT → PUBLIC diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid new file mode 100644 index 000000000000..4266f17877ef --- /dev/null +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid @@ -0,0 +1,74 @@ +/* setup */ +SET experimental_enable_unique_without_index_constraints = true; +CREATE TABLE t (i INT PRIMARY KEY, j INT); +INSERT INTO t VALUES (0,0), (1,0); + +/* test */ +EXPLAIN (ddl, verbose) ALTER TABLE t ADD UNIQUE WITHOUT INDEX (j) NOT VALID; +---- +• Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CONSTRAINT ‹unique_j› UNIQUE WITHOUT INDEX (‹j›) NOT VALID; +│ +├── • StatementPhase +│ │ +│ └── • Stage 1 of 1 in StatementPhase +│ │ +│ ├── • 2 elements transitioning toward PUBLIC +│ │ │ +│ │ ├── • UniqueWithoutIndexConstraintNotValid:{DescID: 104, ConstraintID: 2} +│ │ │ ABSENT → PUBLIC +│ │ │ +│ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} +│ │ ABSENT → PUBLIC +│ │ +│ └── • 2 Mutation operations +│ │ +│ ├── • MakeAbsentUniqueWithoutIndexConstraintNotValidPublic +│ │ ColumnIDs: +│ │ - 2 +│ │ ConstraintID: 2 +│ │ TableID: 104 +│ │ +│ └── • SetConstraintName +│ ConstraintID: 2 +│ Name: unique_j +│ TableID: 104 +│ +└── • PreCommitPhase + │ + ├── • Stage 1 of 2 in PreCommitPhase + │ │ + │ ├── • 2 elements transitioning toward PUBLIC + │ │ │ + │ │ ├── • UniqueWithoutIndexConstraintNotValid:{DescID: 104, ConstraintID: 2} + │ │ │ PUBLIC → ABSENT + │ │ │ + │ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} + │ │ PUBLIC → ABSENT + │ │ + │ └── • 1 Mutation operation + │ │ + │ └── • UndoAllInTxnImmediateMutationOpSideEffects + │ {} + │ + └── • Stage 2 of 2 in PreCommitPhase + │ + ├── • 2 elements transitioning toward PUBLIC + │ │ + │ ├── • UniqueWithoutIndexConstraintNotValid:{DescID: 104, ConstraintID: 2} + │ │ ABSENT → PUBLIC + │ │ + │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} + │ ABSENT → PUBLIC + │ + └── • 2 Mutation operations + │ + ├── • MakeAbsentUniqueWithoutIndexConstraintNotValidPublic + │ ColumnIDs: + │ - 2 + │ ConstraintID: 2 + │ TableID: 104 + │ + └── • SetConstraintName + ConstraintID: 2 + Name: unique_j + TableID: 104 From e7d394b3400cd125ce5798f1c1aa3cb9768d8b90 Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Wed, 1 Feb 2023 15:35:34 -0500 Subject: [PATCH 2/9] schemachanger: Implement `ADD UNIQUE WITHOUT INDEX NOT VALID` and add tests --- .../logictest/testdata/logic_test/drop_table | 6 +- pkg/sql/schemachanger/scbuild/builder_test.go | 1 + .../internal/scbuildstmt/alter_table.go | 11 ++- .../scbuildstmt/alter_table_add_constraint.go | 38 +++++--- .../alter_table_add_unique_without_index | 11 +++ ...table_add_unique_without_index_unvalidated | 11 +++ pkg/sql/schemachanger/scdecomp/decomp.go | 28 ++++-- pkg/sql/schemachanger/scdecomp/testdata/table | 28 ++++++ .../scdeps/sctestutils/sctestutils.go | 1 + .../scexec/scmutationexec/constraint.go | 14 +-- .../schemachanger/scop/immediate_mutation.go | 7 +- .../immediate_mutation_visitor_generated.go | 6 +- pkg/sql/schemachanger/scpb/elements.proto | 9 ++ .../schemachanger/scpb/elements_generated.go | 31 +++++++ pkg/sql/schemachanger/scpb/uml/table.puml | 8 ++ .../scplan/internal/opgen/BUILD.bazel | 1 + .../opgen_unique_without_index_constraint.go | 6 +- ...ue_without_index_constraint_unvalidated.go | 88 +++++++++++++++++++ .../scplan/internal/rules/current/helpers.go | 4 +- .../internal/rules/current/testdata/deprules | 18 ++-- pkg/sql/schemachanger/screl/attr.go | 4 + pkg/sql/schemachanger/screl/scalars.go | 3 +- ...r_table_add_unique_without_index_not_valid | 74 ---------------- 23 files changed, 285 insertions(+), 123 deletions(-) create mode 100644 pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index create mode 100644 pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index_unvalidated create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint_unvalidated.go delete mode 100644 pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid diff --git a/pkg/sql/logictest/testdata/logic_test/drop_table b/pkg/sql/logictest/testdata/logic_test/drop_table index 655e2305dacd..f2b4c3e20366 100644 --- a/pkg/sql/logictest/testdata/logic_test/drop_table +++ b/pkg/sql/logictest/testdata/logic_test/drop_table @@ -118,10 +118,14 @@ DROP TABLE to_drop; subtest drop_table_with_not_valid_constraints statement ok -CREATE TABLE t_with_not_valid_constraints_1 (i INT PRIMARY KEY); +CREATE TABLE t_with_not_valid_constraints_1 (i INT PRIMARY KEY, j INT); statement ok ALTER TABLE t_with_not_valid_constraints_1 ADD CHECK (i > 0) NOT VALID; +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 DROP TABLE t_with_not_valid_constraints_1; diff --git a/pkg/sql/schemachanger/scbuild/builder_test.go b/pkg/sql/schemachanger/scbuild/builder_test.go index 6812504ff741..a07e96057991 100644 --- a/pkg/sql/schemachanger/scbuild/builder_test.go +++ b/pkg/sql/schemachanger/scbuild/builder_test.go @@ -93,6 +93,7 @@ func TestBuildDataDriven(t *testing.T) { // changer will allow non-fully implemented operations. sd.NewSchemaChangerMode = sessiondatapb.UseNewSchemaChangerUnsafe sd.ApplicationName = "" + sd.EnableUniqueWithoutIndexConstraints = true }, ), ), diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go index 64e238ee7d4d..63aaffd5d97c 100644 --- a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go +++ b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/alter_table.go @@ -52,10 +52,11 @@ var supportedAlterTableStatements = map[reflect.Type]supportedAlterTableCommand{ reflect.TypeOf((*tree.AlterTableAddConstraint)(nil)): {fn: alterTableAddConstraint, on: true, extraChecks: func( t *tree.AlterTableAddConstraint, ) bool { - // Support ALTER TABLE ... ADD PRIMARY KEY if d, ok := t.ConstraintDef.(*tree.UniqueConstraintTableDef); ok && d.PrimaryKey && t.ValidationBehavior == tree.ValidationDefault { + // Support ALTER TABLE ... ADD PRIMARY KEY return true - } else if ok && d.WithoutIndex && t.ValidationBehavior == tree.ValidationDefault { + } else if ok && d.WithoutIndex { + // Support ALTER TABLE ... ADD UNIQUE WITHOUT INDEX [NOT VALID] return true } @@ -84,6 +85,7 @@ var alterTableAddConstraintMinSupportedClusterVersion = map[string]clusterversio "ADD_FOREIGN_KEY_DEFAULT": clusterversion.V23_1Start, "ADD_UNIQUE_WITHOUT_INDEX_DEFAULT": clusterversion.V23_1Start, "ADD_CHECK_SKIP": clusterversion.V23_1, + "ADD_UNIQUE_WITHOUT_INDEX_SKIP": clusterversion.V23_1, } func init() { @@ -187,7 +189,10 @@ func alterTableAddConstraintSupportedInCurrentClusterVersion( cmdKey += "_SKIP" } - minSupportedClusterVersion := alterTableAddConstraintMinSupportedClusterVersion[cmdKey] + minSupportedClusterVersion, ok := alterTableAddConstraintMinSupportedClusterVersion[cmdKey] + if !ok { + return false + } return b.EvalCtx().Settings.Version.IsActive(b, minSupportedClusterVersion) } 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 34b88922d60b..faf567c38a29 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 @@ -41,7 +41,7 @@ func alterTableAddConstraint( case *tree.UniqueConstraintTableDef: if d.PrimaryKey && t.ValidationBehavior == tree.ValidationDefault { alterTableAddPrimaryKey(b, tn, tbl, t) - } else if d.WithoutIndex && t.ValidationBehavior == tree.ValidationDefault { + } else if d.WithoutIndex { alterTableAddUniqueWithoutIndex(b, tn, tbl, t) } case *tree.CheckConstraintTableDef: @@ -389,7 +389,8 @@ func alterTableAddForeignKey( }) } -// alterTableAddUniqueWithoutIndex contains logic for building ALTER TABLE ... ADD CONSTRAINT ... UNIQUE WITHOUT INDEX. +// alterTableAddUniqueWithoutIndex contains logic for building +// `ALTER TABLE ... ADD UNIQUE WITHOUT INDEX ... [NOT VALID]`. // It assumes `t` is such a command. func alterTableAddUniqueWithoutIndex( b BuildCtx, tn *tree.TableName, tbl *scpb.Table, t *tree.AlterTableAddConstraint, @@ -473,17 +474,30 @@ func alterTableAddUniqueWithoutIndex( // 5. (Finally!) Add a UniqueWithoutIndex, ConstraintName element to builder state. constraintID := b.NextTableConstraintID(tbl.TableID) - uwi := &scpb.UniqueWithoutIndexConstraint{ - TableID: tbl.TableID, - ConstraintID: constraintID, - ColumnIDs: colIDs, - IndexIDForValidation: getIndexIDForValidationForConstraint(b, tbl.TableID), - } - if d.Predicate != nil { - uwi.Predicate = b.WrapExpression(tbl.TableID, d.Predicate) + if t.ValidationBehavior == tree.ValidationDefault { + uwi := &scpb.UniqueWithoutIndexConstraint{ + TableID: tbl.TableID, + ConstraintID: constraintID, + ColumnIDs: colIDs, + IndexIDForValidation: getIndexIDForValidationForConstraint(b, tbl.TableID), + } + if d.Predicate != nil { + uwi.Predicate = b.WrapExpression(tbl.TableID, d.Predicate) + } + b.Add(uwi) + b.LogEventForExistingTarget(uwi) + } else { + uwi := &scpb.UniqueWithoutIndexConstraintUnvalidated{ + TableID: tbl.TableID, + ConstraintID: constraintID, + ColumnIDs: colIDs, + } + if d.Predicate != nil { + uwi.Predicate = b.WrapExpression(tbl.TableID, d.Predicate) + } + b.Add(uwi) + b.LogEventForExistingTarget(uwi) } - b.Add(uwi) - b.LogEventForExistingTarget(uwi) b.Add(&scpb.ConstraintWithoutIndexName{ TableID: tbl.TableID, ConstraintID: constraintID, diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index new file mode 100644 index 000000000000..3d65dfaf783e --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index @@ -0,0 +1,11 @@ +setup +CREATE TABLE t (i INT PRIMARY KEY, j INT); +---- + +build +ALTER TABLE t ADD UNIQUE WITHOUT INDEX (j); +---- +- [[UniqueWithoutIndexConstraint:{DescID: 104, ConstraintID: 2}, PUBLIC], ABSENT] + {columnIds: [2], constraintId: 2, tableId: 104} +- [[ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2}, PUBLIC], ABSENT] + {constraintId: 2, name: unique_j, tableId: 104} diff --git a/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index_unvalidated b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index_unvalidated new file mode 100644 index 000000000000..b9352bd40e4a --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_unique_without_index_unvalidated @@ -0,0 +1,11 @@ +setup +CREATE TABLE t (i INT PRIMARY KEY, j INT); +---- + +build +ALTER TABLE t ADD UNIQUE WITHOUT INDEX (j) NOT VALID; +---- +- [[UniqueWithoutIndexConstraintUnvalidated:{DescID: 104, ConstraintID: 2}, PUBLIC], ABSENT] + {columnIds: [2], constraintId: 2, tableId: 104} +- [[ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2}, PUBLIC], ABSENT] + {constraintId: 2, name: unique_j, tableId: 104} diff --git a/pkg/sql/schemachanger/scdecomp/decomp.go b/pkg/sql/schemachanger/scdecomp/decomp.go index 7f0a7b7108bd..a876d607f502 100644 --- a/pkg/sql/schemachanger/scdecomp/decomp.go +++ b/pkg/sql/schemachanger/scdecomp/decomp.go @@ -601,20 +601,32 @@ func (w *walkCtx) walkIndex(tbl catalog.TableDescriptor, idx catalog.Index) { func (w *walkCtx) walkUniqueWithoutIndexConstraint( tbl catalog.TableDescriptor, c catalog.UniqueWithoutIndexConstraint, ) { - uwi := &scpb.UniqueWithoutIndexConstraint{ - TableID: tbl.GetID(), - ConstraintID: c.GetConstraintID(), - ColumnIDs: c.CollectKeyColumnIDs().Ordered(), - } + var expr *scpb.Expression + var err error if c.IsPartial() { - expr, err := w.newExpression(c.GetPredicate()) + expr, err = w.newExpression(c.GetPredicate()) if err != nil { panic(errors.NewAssertionErrorWithWrappedErrf(err, "unique without index constraint %q in table %q (%d)", c.GetName(), tbl.GetName(), tbl.GetID())) } - uwi.Predicate = expr } - w.ev(scpb.Status_PUBLIC, uwi) + if c.IsConstraintUnvalidated() && w.clusterVersion.IsActive(clusterversion.V23_1) { + uwi := &scpb.UniqueWithoutIndexConstraintUnvalidated{ + TableID: tbl.GetID(), + ConstraintID: c.GetConstraintID(), + ColumnIDs: c.CollectKeyColumnIDs().Ordered(), + Predicate: expr, + } + w.ev(scpb.Status_PUBLIC, uwi) + } else { + uwi := &scpb.UniqueWithoutIndexConstraint{ + TableID: tbl.GetID(), + ConstraintID: c.GetConstraintID(), + ColumnIDs: c.CollectKeyColumnIDs().Ordered(), + Predicate: expr, + } + w.ev(scpb.Status_PUBLIC, uwi) + } 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 05560972b6d3..18be84bb527d 100644 --- a/pkg/sql/schemachanger/scdecomp/testdata/table +++ b/pkg/sql/schemachanger/scdecomp/testdata/table @@ -1,4 +1,5 @@ setup +SET experimental_enable_unique_without_index_constraints = true; CREATE TABLE parent (id INT PRIMARY KEY); CREATE TABLE tbl ( id INT PRIMARY KEY, @@ -15,6 +16,8 @@ COMMENT ON CONSTRAINT myfk ON tbl IS 'must have a parent'; ALTER TABLE tbl CONFIGURE ZONE USING gc.ttlseconds=10; 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; ---- decompose @@ -319,6 +322,21 @@ ElementState: tableId: 105 temporaryIndexId: 0 Status: PUBLIC +- UniqueWithoutIndexConstraint: + columnIds: + - 3 + constraintId: 5 + indexIdForValidation: 0 + predicate: null + tableId: 105 + Status: PUBLIC +- UniqueWithoutIndexConstraintUnvalidated: + columnIds: + - 3 + constraintId: 6 + predicate: null + tableId: 105 + Status: PUBLIC - CheckConstraint: columnIds: - 1 @@ -650,6 +668,16 @@ ElementState: name: mycheck2 tableId: 105 Status: PUBLIC +- ConstraintWithoutIndexName: + constraintId: 5 + name: myuwi1 + tableId: 105 + Status: PUBLIC +- ConstraintWithoutIndexName: + constraintId: 6 + name: myuwi2 + tableId: 105 + Status: PUBLIC - ConstraintComment: comment: must have a parent constraintId: 2 diff --git a/pkg/sql/schemachanger/scdeps/sctestutils/sctestutils.go b/pkg/sql/schemachanger/scdeps/sctestutils/sctestutils.go index faf2a0becdd6..7b59944ccbb7 100644 --- a/pkg/sql/schemachanger/scdeps/sctestutils/sctestutils.go +++ b/pkg/sql/schemachanger/scdeps/sctestutils/sctestutils.go @@ -79,6 +79,7 @@ func WithBuilderDependenciesFromTestServer( // For setting up a builder inside tests we will ensure that the new schema // changer will allow non-fully implemented operations. planner.SessionData().NewSchemaChangerMode = sessiondatapb.UseNewSchemaChangerUnsafe + planner.SessionData().EnableUniqueWithoutIndexConstraints = true fn(scdeps.NewBuilderDependencies( execCfg.NodeInfo.LogicalClusterID(), execCfg.Codec, diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go b/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go index 3f6c3d2cad64..0eb1c236f414 100644 --- a/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go +++ b/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go @@ -493,8 +493,8 @@ func (i *immediateVisitor) MakePublicForeignKeyConstraintValidated( return errors.AssertionFailedf("failed to find FK constraint %d in descriptor %v", op.ConstraintID, out) } -func (i *immediateVisitor) MakeAbsentUniqueWithoutIndexConstraintWriteOnly( - ctx context.Context, op scop.MakeAbsentUniqueWithoutIndexConstraintWriteOnly, +func (i *immediateVisitor) AddUniqueWithoutIndexConstraint( + ctx context.Context, op scop.AddUniqueWithoutIndexConstraint, ) error { tbl, err := i.checkOutTable(ctx, op.TableID) if err != nil || tbl.Dropped() { @@ -508,13 +508,17 @@ func (i *immediateVisitor) MakeAbsentUniqueWithoutIndexConstraintWriteOnly( TableID: op.TableID, ColumnIDs: op.ColumnIDs, Name: tabledesc.ConstraintNamePlaceholder(op.ConstraintID), - Validity: descpb.ConstraintValidity_Validating, + Validity: op.Validity, ConstraintID: op.ConstraintID, Predicate: string(op.PartialExpr), } + if op.Validity == descpb.ConstraintValidity_Unvalidated { + // Unvalidated constraint doesn't need to transition through an intermediate + // state, so we don't enqueue a mutation for it. + tbl.UniqueWithoutIndexConstraints = append(tbl.UniqueWithoutIndexConstraints, *uwi) + return nil + } enqueueNonIndexMutation(tbl, tbl.AddUniqueWithoutIndexMutation, uwi, descpb.DescriptorMutation_ADD) - // Fast-forward the mutation state to WRITE_ONLY because this constraint - // is now considered as enforced. tbl.Mutations[len(tbl.Mutations)-1].State = descpb.DescriptorMutation_WRITE_ONLY return nil } diff --git a/pkg/sql/schemachanger/scop/immediate_mutation.go b/pkg/sql/schemachanger/scop/immediate_mutation.go index 7676aba34a84..429d09ed413e 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation.go @@ -365,14 +365,15 @@ type RemoveForeignKeyBackReference struct { OriginConstraintID descpb.ConstraintID } -// MakeAbsentUniqueWithoutIndexConstraintWriteOnly adds a non-existent -// unique_without_index constraint to the table in the WRITE_ONLY state. -type MakeAbsentUniqueWithoutIndexConstraintWriteOnly struct { +// AddUniqueWithoutIndexConstraint adds a non-existent +// unique_without_index constraint to the table. +type AddUniqueWithoutIndexConstraint struct { immediateMutationOp TableID descpb.ID ConstraintID descpb.ConstraintID ColumnIDs []descpb.ColumnID PartialExpr catpb.Expression + Validity descpb.ConstraintValidity } // MakeValidatedUniqueWithoutIndexConstraintPublic moves a new, validated unique_without_index diff --git a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go index 88d0c4f62879..563b09718047 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go @@ -64,7 +64,7 @@ type ImmediateMutationVisitor interface { MakePublicForeignKeyConstraintValidated(context.Context, MakePublicForeignKeyConstraintValidated) error RemoveForeignKeyConstraint(context.Context, RemoveForeignKeyConstraint) error RemoveForeignKeyBackReference(context.Context, RemoveForeignKeyBackReference) error - MakeAbsentUniqueWithoutIndexConstraintWriteOnly(context.Context, MakeAbsentUniqueWithoutIndexConstraintWriteOnly) error + AddUniqueWithoutIndexConstraint(context.Context, AddUniqueWithoutIndexConstraint) error MakeValidatedUniqueWithoutIndexConstraintPublic(context.Context, MakeValidatedUniqueWithoutIndexConstraintPublic) error MakePublicUniqueWithoutIndexConstraintValidated(context.Context, MakePublicUniqueWithoutIndexConstraintValidated) error RemoveUniqueWithoutIndexConstraint(context.Context, RemoveUniqueWithoutIndexConstraint) error @@ -327,8 +327,8 @@ func (op RemoveForeignKeyBackReference) Visit(ctx context.Context, v ImmediateMu } // Visit is part of the ImmediateMutationOp interface. -func (op MakeAbsentUniqueWithoutIndexConstraintWriteOnly) Visit(ctx context.Context, v ImmediateMutationVisitor) error { - return v.MakeAbsentUniqueWithoutIndexConstraintWriteOnly(ctx, op) +func (op AddUniqueWithoutIndexConstraint) Visit(ctx context.Context, v ImmediateMutationVisitor) error { + return v.AddUniqueWithoutIndexConstraint(ctx, op) } // Visit is part of the ImmediateMutationOp interface. diff --git a/pkg/sql/schemachanger/scpb/elements.proto b/pkg/sql/schemachanger/scpb/elements.proto index 5538163718f3..534382c0c283 100644 --- a/pkg/sql/schemachanger/scpb/elements.proto +++ b/pkg/sql/schemachanger/scpb/elements.proto @@ -71,6 +71,7 @@ message ElementProto { SecondaryIndex secondary_index = 23 [(gogoproto.moretags) = "parent:\"Table, View\""]; TemporaryIndex temporary_index = 24 [(gogoproto.moretags) = "parent:\"Table, View\""]; UniqueWithoutIndexConstraint unique_without_index_constraint = 25 [(gogoproto.moretags) = "parent:\"Table\""]; + UniqueWithoutIndexConstraintUnvalidated unique_without_index_constraint_unvalidated = 171 [(gogoproto.moretags) = "parent:\"Table\""]; CheckConstraint check_constraint = 26 [(gogoproto.moretags) = "parent:\"Table\""]; CheckConstraintUnvalidated check_constraint_unvalidated = 170 [(gogoproto.moretags) = "parent:\"Table\""]; ForeignKeyConstraint foreign_key_constraint = 27 [(gogoproto.moretags) = "parent:\"Table\""]; @@ -356,6 +357,14 @@ message UniqueWithoutIndexConstraint { uint32 index_id_for_validation = 5 [(gogoproto.customname) = "IndexIDForValidation", (gogoproto.casttype) = "github.com/cockroachdb/cockroach/pkg/sql/sem/catid.IndexID"]; } +message UniqueWithoutIndexConstraintUnvalidated { + 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"]; + // Predicate, if non-nil, means a partial uniqueness constraint. + Expression predicate = 4 [(gogoproto.customname) = "Predicate"]; +} + message CheckConstraint { 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"]; diff --git a/pkg/sql/schemachanger/scpb/elements_generated.go b/pkg/sql/schemachanger/scpb/elements_generated.go index 28f41d5137fa..1d14a6d75368 100644 --- a/pkg/sql/schemachanger/scpb/elements_generated.go +++ b/pkg/sql/schemachanger/scpb/elements_generated.go @@ -1846,6 +1846,37 @@ func FindUniqueWithoutIndexConstraint(b ElementStatusIterator) (current Status, return current, target, element } +func (e UniqueWithoutIndexConstraintUnvalidated) element() {} + +// ForEachUniqueWithoutIndexConstraintUnvalidated iterates over elements of type UniqueWithoutIndexConstraintUnvalidated. +func ForEachUniqueWithoutIndexConstraintUnvalidated( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *UniqueWithoutIndexConstraintUnvalidated), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*UniqueWithoutIndexConstraintUnvalidated); ok { + fn(current, target, elt) + } + }) +} + +// FindUniqueWithoutIndexConstraintUnvalidated finds the first element of type UniqueWithoutIndexConstraintUnvalidated. +func FindUniqueWithoutIndexConstraintUnvalidated(b ElementStatusIterator) (current Status, target TargetStatus, element *UniqueWithoutIndexConstraintUnvalidated) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*UniqueWithoutIndexConstraintUnvalidated); ok { + element = elt + current = c + target = t + } + }) + return current, target, element +} + func (e UserPrivileges) element() {} // ForEachUserPrivileges iterates over elements of type UserPrivileges. diff --git a/pkg/sql/schemachanger/scpb/uml/table.puml b/pkg/sql/schemachanger/scpb/uml/table.puml index 71577376d69f..001290e00a04 100644 --- a/pkg/sql/schemachanger/scpb/uml/table.puml +++ b/pkg/sql/schemachanger/scpb/uml/table.puml @@ -90,6 +90,13 @@ UniqueWithoutIndexConstraint : []ColumnIDs UniqueWithoutIndexConstraint : Predicate UniqueWithoutIndexConstraint : IndexIDForValidation +object UniqueWithoutIndexConstraintUnvalidated + +UniqueWithoutIndexConstraintUnvalidated : TableID +UniqueWithoutIndexConstraintUnvalidated : ConstraintID +UniqueWithoutIndexConstraintUnvalidated : []ColumnIDs +UniqueWithoutIndexConstraintUnvalidated : Predicate + object CheckConstraint CheckConstraint : TableID @@ -375,6 +382,7 @@ View <|-- SecondaryIndex Table <|-- TemporaryIndex View <|-- TemporaryIndex Table <|-- UniqueWithoutIndexConstraint +Table <|-- UniqueWithoutIndexConstraintUnvalidated Table <|-- CheckConstraint Table <|-- CheckConstraintUnvalidated Table <|-- ForeignKeyConstraint diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel index 5775b3cb85d3..daae91b2e229 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel +++ b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel @@ -65,6 +65,7 @@ go_library( "opgen_table_zone_config.go", "opgen_temporary_index.go", "opgen_unique_without_index_constraint.go", + "opgen_unique_without_index_constraint_unvalidated.go", "opgen_user_privileges.go", "opgen_view.go", "register.go", diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go index 741021f58e69..1155d15c1ae4 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint.go @@ -12,6 +12,7 @@ package opgen import ( "github.com/cockroachdb/cockroach/pkg/sql/catalog/catpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" ) @@ -21,16 +22,17 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_WRITE_ONLY, - emit(func(this *scpb.UniqueWithoutIndexConstraint) *scop.MakeAbsentUniqueWithoutIndexConstraintWriteOnly { + emit(func(this *scpb.UniqueWithoutIndexConstraint) *scop.AddUniqueWithoutIndexConstraint { var partialExpr catpb.Expression if this.Predicate != nil { partialExpr = this.Predicate.Expr } - return &scop.MakeAbsentUniqueWithoutIndexConstraintWriteOnly{ + return &scop.AddUniqueWithoutIndexConstraint{ TableID: this.TableID, ConstraintID: this.ConstraintID, ColumnIDs: this.ColumnIDs, PartialExpr: partialExpr, + Validity: descpb.ConstraintValidity_Validating, } }), emit(func(this *scpb.UniqueWithoutIndexConstraint) *scop.UpdateTableBackReferencesInTypes { diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint_unvalidated.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint_unvalidated.go new file mode 100644 index 000000000000..db9516951028 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_unique_without_index_constraint_unvalidated.go @@ -0,0 +1,88 @@ +// 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/catalog/catpb" + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.UniqueWithoutIndexConstraintUnvalidated)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.UniqueWithoutIndexConstraintUnvalidated) *scop.AddUniqueWithoutIndexConstraint { + var partialExpr catpb.Expression + if this.Predicate != nil { + partialExpr = this.Predicate.Expr + } + return &scop.AddUniqueWithoutIndexConstraint{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + ColumnIDs: this.ColumnIDs, + PartialExpr: partialExpr, + Validity: descpb.ConstraintValidity_Unvalidated, + } + }), + emit(func(this *scpb.UniqueWithoutIndexConstraintUnvalidated) *scop.UpdateTableBackReferencesInTypes { + if this.Predicate == nil || this.Predicate.UsesTypeIDs == nil || len(this.Predicate.UsesTypeIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInTypes{ + TypeIDs: this.Predicate.UsesTypeIDs, + BackReferencedTableID: this.TableID, + } + }), + emit(func(this *scpb.UniqueWithoutIndexConstraintUnvalidated) *scop.UpdateTableBackReferencesInSequences { + if this.Predicate == nil || this.Predicate.UsesSequenceIDs == nil || len(this.Predicate.UsesSequenceIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInSequences{ + SequenceIDs: this.Predicate.UsesSequenceIDs, + BackReferencedTableID: this.TableID, + } + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.UniqueWithoutIndexConstraintUnvalidated) *scop.RemoveUniqueWithoutIndexConstraint { + return &scop.RemoveUniqueWithoutIndexConstraint{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + } + }), + emit(func(this *scpb.UniqueWithoutIndexConstraintUnvalidated) *scop.UpdateTableBackReferencesInTypes { + if this.Predicate == nil || this.Predicate.UsesTypeIDs == nil || len(this.Predicate.UsesTypeIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInTypes{ + TypeIDs: this.Predicate.UsesTypeIDs, + BackReferencedTableID: this.TableID, + } + }), + emit(func(this *scpb.UniqueWithoutIndexConstraintUnvalidated) *scop.UpdateTableBackReferencesInSequences { + if this.Predicate == nil || this.Predicate.UsesSequenceIDs == nil || len(this.Predicate.UsesSequenceIDs) == 0 { + return nil + } + return &scop.UpdateTableBackReferencesInSequences{ + SequenceIDs: this.Predicate.UsesSequenceIDs, + BackReferencedTableID: this.TableID, + } + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go index 171d6067a392..4791006c5be6 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go @@ -210,7 +210,7 @@ func isNonIndexBackedConstraint(e scpb.Element) bool { case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, *scpb.ForeignKeyConstraint, *scpb.ColumnNotNull: return true - case *scpb.CheckConstraintUnvalidated: + case *scpb.CheckConstraintUnvalidated, *scpb.UniqueWithoutIndexConstraintUnvalidated: return true } return false @@ -232,7 +232,7 @@ func isNonIndexBackedCrossDescriptorConstraint(e scpb.Element) bool { case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, *scpb.ForeignKeyConstraint: return true - case *scpb.CheckConstraintUnvalidated: + case *scpb.CheckConstraintUnvalidated, *scpb.UniqueWithoutIndexConstraintUnvalidated: return true } return false diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules index bd8205c13243..57789337eef5 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.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.CheckConstraintUnvalidated' + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] - 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.CheckConstraintUnvalidated' + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] - 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.CheckConstraintUnvalidated' + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] - 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.CheckConstraintUnvalidated' + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] - 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.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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 @@ -2745,7 +2745,7 @@ deprules kind: Precedence to: descriptor-Node query: - - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.Column', '*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.ColumnNotNull', '*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.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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.ColumnNotNull', '*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.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinOnDescID($dependent, $descriptor, $desc-id) - toAbsent($dependent-Target, $descriptor-Target) diff --git a/pkg/sql/schemachanger/screl/attr.go b/pkg/sql/schemachanger/screl/attr.go index 99a103a833c2..8fdfcab2a083 100644 --- a/pkg/sql/schemachanger/screl/attr.go +++ b/pkg/sql/schemachanger/screl/attr.go @@ -178,6 +178,10 @@ var elementSchemaOptions = []rel.SchemaOption{ rel.EntityAttr(DescID, "TableID"), rel.EntityAttr(ConstraintID, "ConstraintID"), ), + rel.EntityMapping(t((*scpb.UniqueWithoutIndexConstraintUnvalidated)(nil)), + rel.EntityAttr(DescID, "TableID"), + rel.EntityAttr(ConstraintID, "ConstraintID"), + ), rel.EntityMapping(t((*scpb.CheckConstraint)(nil)), rel.EntityAttr(DescID, "TableID"), rel.EntityAttr(ConstraintID, "ConstraintID"), diff --git a/pkg/sql/schemachanger/screl/scalars.go b/pkg/sql/schemachanger/screl/scalars.go index 3c3832e81d9c..732d6128f5c3 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.CheckConstraintUnvalidated: + case *scpb.ColumnNotNull, *scpb.CheckConstraintUnvalidated, + *scpb.UniqueWithoutIndexConstraintUnvalidated: return clusterversion.V23_1 default: panic(errors.AssertionFailedf("unknown element %T", el)) diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid deleted file mode 100644 index 4266f17877ef..000000000000 --- a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index_not_valid +++ /dev/null @@ -1,74 +0,0 @@ -/* setup */ -SET experimental_enable_unique_without_index_constraints = true; -CREATE TABLE t (i INT PRIMARY KEY, j INT); -INSERT INTO t VALUES (0,0), (1,0); - -/* test */ -EXPLAIN (ddl, verbose) ALTER TABLE t ADD UNIQUE WITHOUT INDEX (j) NOT VALID; ----- -• Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CONSTRAINT ‹unique_j› UNIQUE WITHOUT INDEX (‹j›) NOT VALID; -│ -├── • StatementPhase -│ │ -│ └── • Stage 1 of 1 in StatementPhase -│ │ -│ ├── • 2 elements transitioning toward PUBLIC -│ │ │ -│ │ ├── • UniqueWithoutIndexConstraintNotValid:{DescID: 104, ConstraintID: 2} -│ │ │ ABSENT → PUBLIC -│ │ │ -│ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} -│ │ ABSENT → PUBLIC -│ │ -│ └── • 2 Mutation operations -│ │ -│ ├── • MakeAbsentUniqueWithoutIndexConstraintNotValidPublic -│ │ ColumnIDs: -│ │ - 2 -│ │ ConstraintID: 2 -│ │ TableID: 104 -│ │ -│ └── • SetConstraintName -│ ConstraintID: 2 -│ Name: unique_j -│ TableID: 104 -│ -└── • PreCommitPhase - │ - ├── • Stage 1 of 2 in PreCommitPhase - │ │ - │ ├── • 2 elements transitioning toward PUBLIC - │ │ │ - │ │ ├── • UniqueWithoutIndexConstraintNotValid:{DescID: 104, ConstraintID: 2} - │ │ │ PUBLIC → ABSENT - │ │ │ - │ │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} - │ │ PUBLIC → ABSENT - │ │ - │ └── • 1 Mutation operation - │ │ - │ └── • UndoAllInTxnImmediateMutationOpSideEffects - │ {} - │ - └── • Stage 2 of 2 in PreCommitPhase - │ - ├── • 2 elements transitioning toward PUBLIC - │ │ - │ ├── • UniqueWithoutIndexConstraintNotValid:{DescID: 104, ConstraintID: 2} - │ │ ABSENT → PUBLIC - │ │ - │ └── • ConstraintWithoutIndexName:{DescID: 104, Name: unique_j, ConstraintID: 2} - │ ABSENT → PUBLIC - │ - └── • 2 Mutation operations - │ - ├── • MakeAbsentUniqueWithoutIndexConstraintNotValidPublic - │ ColumnIDs: - │ - 2 - │ ConstraintID: 2 - │ TableID: 104 - │ - └── • SetConstraintName - ConstraintID: 2 - Name: unique_j - TableID: 104 From b9a0471e49c4c0c57d8f2fa6e72306921d0ac55d Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Wed, 1 Feb 2023 17:17:46 -0500 Subject: [PATCH 3/9] schemachanger: Implement `ADD FOREIGN KEY NOT VALID` and add tests --- .../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_unvalidated | 12 ++++ pkg/sql/schemachanger/scdecomp/decomp.go | 34 +++++++---- pkg/sql/schemachanger/scdecomp/testdata/table | 18 ++++++ .../scexec/scmutationexec/constraint.go | 26 ++++++--- .../scexec/scmutationexec/references.go | 26 ++++++++- .../schemachanger/scop/immediate_mutation.go | 7 ++- .../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/opgen_foreign_key_constraint.go | 6 +- ...pgen_foreign_key_constraint_unvalidated.go | 58 +++++++++++++++++++ .../scplan/internal/rules/current/helpers.go | 6 +- .../internal/rules/current/testdata/deprules | 18 +++--- pkg/sql/schemachanger/screl/attr.go | 5 ++ pkg/sql/schemachanger/screl/scalars.go | 2 +- .../explain/alter_table_add_foreign_key | 4 +- .../alter_table_add_unique_without_index | 4 +- .../alter_table_add_foreign_key | 6 +- .../alter_table_add_unique_without_index | 6 +- 25 files changed, 305 insertions(+), 68 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_unvalidated create mode 100644 pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_unvalidated.go 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 faf567c38a29..e139d46b85ed 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.ForeignKeyConstraintUnvalidated{ + 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_unvalidated b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key_unvalidated new file mode 100644 index 000000000000..27171d7ef64a --- /dev/null +++ b/pkg/sql/schemachanger/scbuild/testdata/alter_table_add_foreign_key_unvalidated @@ -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; +---- +- [[ForeignKeyConstraintUnvalidated:{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 a876d607f502..c49519af15f7 100644 --- a/pkg/sql/schemachanger/scdecomp/decomp.go +++ b/pkg/sql/schemachanger/scdecomp/decomp.go @@ -680,17 +680,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.ForeignKeyConstraintUnvalidated{ + 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 18be84bb527d..d811c9994f6d 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 @@ -374,6 +375,18 @@ ElementState: referencedTableId: 104 tableId: 105 Status: PUBLIC +- ForeignKeyConstraintUnvalidated: + columnIds: + - 1 + compositeKeyMatchMethod: SIMPLE + constraintId: 7 + onDeleteAction: NO_ACTION + onUpdateAction: NO_ACTION + referencedColumnIds: + - 1 + referencedTableId: 104 + tableId: 105 + Status: PUBLIC - TableComment: comment: tbl is good table tableId: 105 @@ -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 0eb1c236f414..7e7d01b1cbbf 100644 --- a/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go +++ b/pkg/sql/schemachanger/scexec/scmutationexec/constraint.go @@ -367,8 +367,8 @@ func (i *immediateVisitor) RemoveUniqueWithoutIndexConstraint( return nil } -func (i *immediateVisitor) MakeAbsentForeignKeyConstraintWriteOnly( - ctx context.Context, op scop.MakeAbsentForeignKeyConstraintWriteOnly, +func (i *immediateVisitor) AddForeignKeyConstraint( + ctx context.Context, op scop.AddForeignKeyConstraint, ) error { out, err := i.checkOutTable(ctx, op.TableID) if err != nil || out.Dropped() { @@ -378,21 +378,27 @@ func (i *immediateVisitor) MakeAbsentForeignKeyConstraintWriteOnly( 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_Validating, + Validity: op.Validity, OnDelete: op.OnDeleteAction, OnUpdate: op.OnUpdateAction, Match: op.CompositeKeyMatchMethod, ConstraintID: op.ConstraintID, } - enqueueNonIndexMutation(out, out.AddForeignKeyMutation, fk, descpb.DescriptorMutation_ADD) - out.Mutations[len(out.Mutations)-1].State = descpb.DescriptorMutation_WRITE_ONLY + if op.Validity == descpb.ConstraintValidity_Unvalidated { + // Unvalidated constraint doesn't need to transition through an intermediate + // state, so we don't enqueue a mutation for it. + out.OutboundFKs = append(out.OutboundFKs, *fk) + } else { + enqueueNonIndexMutation(out, out.AddForeignKeyMutation, fk, descpb.DescriptorMutation_ADD) + out.Mutations[len(out.Mutations)-1].State = descpb.DescriptorMutation_WRITE_ONLY + } + // Add an entry in "InboundFKs" in the referenced table as company. in, err := i.checkOutTable(ctx, op.ReferencedTableID) if err != nil { @@ -454,14 +460,16 @@ func (i *immediateVisitor) MakePublicForeignKeyConstraintValidated( ctx context.Context, op scop.MakePublicForeignKeyConstraintValidated, ) error { // A helper function to update the inbound FK in referenced table to DROPPING. - updateInboundFKAsDropping := func(referencedTableID descpb.ID) error { + // Note that the constraintID in the referencedTable might not match that in + // in the referencing table, so we pass in the FK name as identifier. + updateInboundFKAsDropping := func(referencedTableID descpb.ID, fkName string) error { foundInReferencedTable := false in, err := i.checkOutTable(ctx, referencedTableID) if err != nil { return err } for idx, inboundFk := range in.InboundFKs { - if inboundFk.OriginTableID == op.TableID && inboundFk.ConstraintID == op.ConstraintID { + if inboundFk.OriginTableID == op.TableID && inboundFk.Name == fkName { in.InboundFKs[idx].Validity = descpb.ConstraintValidity_Dropping foundInReferencedTable = true break @@ -486,7 +494,7 @@ func (i *immediateVisitor) MakePublicForeignKeyConstraintValidated( } fk.Validity = descpb.ConstraintValidity_Dropping enqueueNonIndexMutation(out, out.AddForeignKeyMutation, &fk, descpb.DescriptorMutation_DROP) - return updateInboundFKAsDropping(fk.ReferencedTableID) + return updateInboundFKAsDropping(fk.ReferencedTableID, fk.Name) } } diff --git a/pkg/sql/schemachanger/scexec/scmutationexec/references.go b/pkg/sql/schemachanger/scexec/scmutationexec/references.go index 9fff00e5d619..86ad14bb3b3d 100644 --- a/pkg/sql/schemachanger/scexec/scmutationexec/references.go +++ b/pkg/sql/schemachanger/scexec/scmutationexec/references.go @@ -20,6 +20,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sem/catid" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/iterutil" + "github.com/cockroachdb/errors" ) func (i *immediateVisitor) RemoveSchemaParent( @@ -73,6 +74,29 @@ func (i *immediateVisitor) RemoveForeignKeyBackReference( // Exit early if the foreign key back-reference holder is getting dropped. return err } + // Retrieve foreign key name in origin table to identify it in the referenced + // table. + var name string + { + out, err := i.getDescriptor(ctx, op.OriginTableID) + if err != nil { + return err + } + tbl, err := catalog.AsTableDescriptor(out) + if err != nil { + return err + } + for _, fk := range tbl.OutboundForeignKeys() { + if fk.GetConstraintID() == op.OriginConstraintID { + name = fk.GetName() + break + } + } + if name == "" { + return errors.AssertionFailedf("foreign key with ID %d not found in origin table %q (%d)", + op.OriginConstraintID, out.GetName(), out.GetID()) + } + } // Attempt to remove back reference. // Note how we // 1. only check to remove from `in.InboundFKs` but not from `in.Mutations`: @@ -83,7 +107,7 @@ func (i *immediateVisitor) RemoveForeignKeyBackReference( // this is because if we roll back before the adding FK is published in `out`, // such a back-reference won't exist in `in` yet. for idx, fk := range in.InboundFKs { - if fk.OriginTableID == op.OriginTableID && fk.ConstraintID == op.OriginConstraintID { + if fk.OriginTableID == op.OriginTableID && fk.Name == name { in.InboundFKs = append(in.InboundFKs[:idx], in.InboundFKs[idx+1:]...) if len(in.InboundFKs) == 0 { in.InboundFKs = nil diff --git a/pkg/sql/schemachanger/scop/immediate_mutation.go b/pkg/sql/schemachanger/scop/immediate_mutation.go index 429d09ed413e..32952992e494 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation.go @@ -318,9 +318,9 @@ type MakeValidatedColumnNotNullPublic struct { ColumnID descpb.ColumnID } -// MakeAbsentForeignKeyConstraintWriteOnly adds a non-existent foreign key -// constraint to the table in the WRITE_ONLY state. -type MakeAbsentForeignKeyConstraintWriteOnly struct { +// AddForeignKeyConstraint adds a non-existent foreign key +// constraint to the table. +type AddForeignKeyConstraint struct { immediateMutationOp TableID descpb.ID ConstraintID descpb.ConstraintID @@ -330,6 +330,7 @@ type MakeAbsentForeignKeyConstraintWriteOnly struct { OnUpdateAction semenumpb.ForeignKeyAction OnDeleteAction semenumpb.ForeignKeyAction CompositeKeyMatchMethod semenumpb.Match + Validity descpb.ConstraintValidity } // MakeValidatedForeignKeyConstraintPublic moves a new, validated foreign key diff --git a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go index 563b09718047..15dfa957054e 100644 --- a/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go +++ b/pkg/sql/schemachanger/scop/immediate_mutation_visitor_generated.go @@ -59,7 +59,7 @@ type ImmediateMutationVisitor interface { MakePublicColumnNotNullValidated(context.Context, MakePublicColumnNotNullValidated) error MakeValidatedCheckConstraintPublic(context.Context, MakeValidatedCheckConstraintPublic) error MakeValidatedColumnNotNullPublic(context.Context, MakeValidatedColumnNotNullPublic) error - MakeAbsentForeignKeyConstraintWriteOnly(context.Context, MakeAbsentForeignKeyConstraintWriteOnly) error + AddForeignKeyConstraint(context.Context, AddForeignKeyConstraint) error MakeValidatedForeignKeyConstraintPublic(context.Context, MakeValidatedForeignKeyConstraintPublic) error MakePublicForeignKeyConstraintValidated(context.Context, MakePublicForeignKeyConstraintValidated) error RemoveForeignKeyConstraint(context.Context, RemoveForeignKeyConstraint) error @@ -302,8 +302,8 @@ func (op MakeValidatedColumnNotNullPublic) Visit(ctx context.Context, v Immediat } // Visit is part of the ImmediateMutationOp interface. -func (op MakeAbsentForeignKeyConstraintWriteOnly) Visit(ctx context.Context, v ImmediateMutationVisitor) error { - return v.MakeAbsentForeignKeyConstraintWriteOnly(ctx, op) +func (op AddForeignKeyConstraint) Visit(ctx context.Context, v ImmediateMutationVisitor) error { + return v.AddForeignKeyConstraint(ctx, op) } // Visit is part of the ImmediateMutationOp interface. diff --git a/pkg/sql/schemachanger/scpb/elements.proto b/pkg/sql/schemachanger/scpb/elements.proto index 534382c0c283..21b180a43af5 100644 --- a/pkg/sql/schemachanger/scpb/elements.proto +++ b/pkg/sql/schemachanger/scpb/elements.proto @@ -75,6 +75,7 @@ message ElementProto { CheckConstraint check_constraint = 26 [(gogoproto.moretags) = "parent:\"Table\""]; CheckConstraintUnvalidated check_constraint_unvalidated = 170 [(gogoproto.moretags) = "parent:\"Table\""]; ForeignKeyConstraint foreign_key_constraint = 27 [(gogoproto.moretags) = "parent:\"Table\""]; + ForeignKeyConstraintUnvalidated foreign_key_constraint_not_valid = 172 [(gogoproto.moretags) = "parent:\"Table\""]; TableComment table_comment = 28 [(gogoproto.moretags) = "parent:\"Table, View, Sequence\""]; RowLevelTTL row_level_ttl = 29 [(gogoproto.customname) = "RowLevelTTL", (gogoproto.moretags) = "parent:\"Table\""]; TableZoneConfig table_zone_config = 121 [(gogoproto.moretags) = "parent:\"Table, View\""]; @@ -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 ForeignKeyConstraintUnvalidated { + 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 1d14a6d75368..e3f66c53e8f1 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 ForeignKeyConstraintUnvalidated) element() {} + +// ForEachForeignKeyConstraintUnvalidated iterates over elements of type ForeignKeyConstraintUnvalidated. +func ForEachForeignKeyConstraintUnvalidated( + b ElementStatusIterator, fn func(current Status, target TargetStatus, e *ForeignKeyConstraintUnvalidated), +) { + if b == nil { + return + } + b.ForEachElementStatus(func(current Status, target TargetStatus, e Element) { + if elt, ok := e.(*ForeignKeyConstraintUnvalidated); ok { + fn(current, target, elt) + } + }) +} + +// FindForeignKeyConstraintUnvalidated finds the first element of type ForeignKeyConstraintUnvalidated. +func FindForeignKeyConstraintUnvalidated(b ElementStatusIterator) (current Status, target TargetStatus, element *ForeignKeyConstraintUnvalidated) { + if b == nil { + return current, target, element + } + b.ForEachElementStatus(func(c Status, t TargetStatus, e Element) { + if elt, ok := e.(*ForeignKeyConstraintUnvalidated); 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 001290e00a04..9e98c7c7acc2 100644 --- a/pkg/sql/schemachanger/scpb/uml/table.puml +++ b/pkg/sql/schemachanger/scpb/uml/table.puml @@ -125,6 +125,17 @@ ForeignKeyConstraint : OnDeleteAction ForeignKeyConstraint : CompositeKeyMatchMethod ForeignKeyConstraint : IndexIDForValidation +object ForeignKeyConstraintUnvalidated + +ForeignKeyConstraintUnvalidated : TableID +ForeignKeyConstraintUnvalidated : ConstraintID +ForeignKeyConstraintUnvalidated : []ColumnIDs +ForeignKeyConstraintUnvalidated : ReferencedTableID +ForeignKeyConstraintUnvalidated : []ReferencedColumnIDs +ForeignKeyConstraintUnvalidated : OnUpdateAction +ForeignKeyConstraintUnvalidated : OnDeleteAction +ForeignKeyConstraintUnvalidated : CompositeKeyMatchMethod + object TableComment TableComment : TableID @@ -386,6 +397,7 @@ Table <|-- UniqueWithoutIndexConstraintUnvalidated Table <|-- CheckConstraint Table <|-- CheckConstraintUnvalidated Table <|-- ForeignKeyConstraint +Table <|-- ForeignKeyConstraintUnvalidated Table <|-- TableComment View <|-- TableComment Sequence <|-- TableComment diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel b/pkg/sql/schemachanger/scplan/internal/opgen/BUILD.bazel index daae91b2e229..bd7ba8d0a1a8 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_unvalidated.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.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint.go index 52d514a6ee72..bd9ea483f661 100644 --- a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint.go +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint.go @@ -11,6 +11,7 @@ package opgen import ( + "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" ) @@ -20,8 +21,8 @@ func init() { toPublic( scpb.Status_ABSENT, to(scpb.Status_WRITE_ONLY, - emit(func(this *scpb.ForeignKeyConstraint) *scop.MakeAbsentForeignKeyConstraintWriteOnly { - return &scop.MakeAbsentForeignKeyConstraintWriteOnly{ + emit(func(this *scpb.ForeignKeyConstraint) *scop.AddForeignKeyConstraint { + return &scop.AddForeignKeyConstraint{ TableID: this.TableID, ConstraintID: this.ConstraintID, ColumnIDs: this.ColumnIDs, @@ -30,6 +31,7 @@ func init() { OnUpdateAction: this.OnUpdateAction, OnDeleteAction: this.OnDeleteAction, CompositeKeyMatchMethod: this.CompositeKeyMatchMethod, + Validity: descpb.ConstraintValidity_Validating, } }), ), diff --git a/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_unvalidated.go b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_unvalidated.go new file mode 100644 index 000000000000..3d268b7d9b67 --- /dev/null +++ b/pkg/sql/schemachanger/scplan/internal/opgen/opgen_foreign_key_constraint_unvalidated.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/catalog/descpb" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop" + "github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb" +) + +func init() { + opRegistry.register((*scpb.ForeignKeyConstraintUnvalidated)(nil), + toPublic( + scpb.Status_ABSENT, + to(scpb.Status_PUBLIC, + emit(func(this *scpb.ForeignKeyConstraintUnvalidated) *scop.AddForeignKeyConstraint { + return &scop.AddForeignKeyConstraint{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + ColumnIDs: this.ColumnIDs, + ReferencedTableID: this.ReferencedTableID, + ReferencedColumnIDs: this.ReferencedColumnIDs, + OnUpdateAction: this.OnUpdateAction, + OnDeleteAction: this.OnDeleteAction, + CompositeKeyMatchMethod: this.CompositeKeyMatchMethod, + Validity: descpb.ConstraintValidity_Unvalidated, + } + }), + ), + ), + toAbsent( + scpb.Status_PUBLIC, + to(scpb.Status_ABSENT, + emit(func(this *scpb.ForeignKeyConstraintUnvalidated) *scop.RemoveForeignKeyBackReference { + return &scop.RemoveForeignKeyBackReference{ + ReferencedTableID: this.ReferencedTableID, + OriginTableID: this.TableID, + OriginConstraintID: this.ConstraintID, + } + }), + emit(func(this *scpb.ForeignKeyConstraintUnvalidated) *scop.RemoveForeignKeyConstraint { + return &scop.RemoveForeignKeyConstraint{ + TableID: this.TableID, + ConstraintID: this.ConstraintID, + } + }), + ), + ), + ) +} diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go index 4791006c5be6..43cdb8d94332 100644 --- a/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go +++ b/pkg/sql/schemachanger/scplan/internal/rules/current/helpers.go @@ -210,7 +210,8 @@ func isNonIndexBackedConstraint(e scpb.Element) bool { case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, *scpb.ForeignKeyConstraint, *scpb.ColumnNotNull: return true - case *scpb.CheckConstraintUnvalidated, *scpb.UniqueWithoutIndexConstraintUnvalidated: + case *scpb.CheckConstraintUnvalidated, *scpb.UniqueWithoutIndexConstraintUnvalidated, + *scpb.ForeignKeyConstraintUnvalidated: return true } return false @@ -232,7 +233,8 @@ func isNonIndexBackedCrossDescriptorConstraint(e scpb.Element) bool { case *scpb.CheckConstraint, *scpb.UniqueWithoutIndexConstraint, *scpb.ForeignKeyConstraint: return true - case *scpb.CheckConstraintUnvalidated, *scpb.UniqueWithoutIndexConstraintUnvalidated: + case *scpb.CheckConstraintUnvalidated, *scpb.UniqueWithoutIndexConstraintUnvalidated, + *scpb.ForeignKeyConstraintUnvalidated: return true } return false diff --git a/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules b/pkg/sql/schemachanger/scplan/internal/rules/current/testdata/deprules index 57789337eef5..7c7307930c72 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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraintUnvalidated'] - 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] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraintUnvalidated'] - 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] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraintUnvalidated'] - 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] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated'] + - $constraint[Type] IN ['*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraintUnvalidated'] - 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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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 @@ -2745,7 +2745,7 @@ deprules kind: Precedence to: descriptor-Node query: - - $dependent[Type] IN ['*scpb.ColumnFamily', '*scpb.Column', '*scpb.PrimaryIndex', '*scpb.SecondaryIndex', '*scpb.TemporaryIndex', '*scpb.UniqueWithoutIndexConstraint', '*scpb.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.ColumnNotNull', '*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.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.UniqueWithoutIndexConstraintUnvalidated', '*scpb.CheckConstraint', '*scpb.CheckConstraintUnvalidated', '*scpb.ForeignKeyConstraint', '*scpb.ForeignKeyConstraintUnvalidated', '*scpb.TableComment', '*scpb.RowLevelTTL', '*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.ColumnNotNull', '*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.SchemaParent', '*scpb.SchemaComment', '*scpb.ObjectParent', '*scpb.EnumTypeValue', '*scpb.CompositeTypeAttrType', '*scpb.CompositeTypeAttrName', '*scpb.FunctionName', '*scpb.FunctionVolatility', '*scpb.FunctionLeakProof', '*scpb.FunctionNullInputBehavior', '*scpb.FunctionBody', '*scpb.FunctionParamDefaultExpression'] - $descriptor[Type] IN ['*scpb.Database', '*scpb.Schema', '*scpb.View', '*scpb.Sequence', '*scpb.Table', '*scpb.EnumType', '*scpb.AliasType', '*scpb.CompositeType', '*scpb.Function'] - joinOnDescID($dependent, $descriptor, $desc-id) - toAbsent($dependent-Target, $descriptor-Target) diff --git a/pkg/sql/schemachanger/screl/attr.go b/pkg/sql/schemachanger/screl/attr.go index 8fdfcab2a083..4d9bb152e8c6 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.ForeignKeyConstraintUnvalidated)(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 732d6128f5c3..95330515b9de 100644 --- a/pkg/sql/schemachanger/screl/scalars.go +++ b/pkg/sql/schemachanger/screl/scalars.go @@ -125,7 +125,7 @@ func MinVersion(el scpb.Element) clusterversion.Key { *scpb.FunctionNullInputBehavior, *scpb.FunctionBody, *scpb.FunctionParamDefaultExpression: return clusterversion.V23_1 case *scpb.ColumnNotNull, *scpb.CheckConstraintUnvalidated, - *scpb.UniqueWithoutIndexConstraintUnvalidated: + *scpb.UniqueWithoutIndexConstraintUnvalidated, *scpb.ForeignKeyConstraintUnvalidated: return clusterversion.V23_1 default: panic(errors.AssertionFailedf("unknown element %T", el)) diff --git a/pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key b/pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key index 10fe4e9f0bc0..f01f6f97a85f 100644 --- a/pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key +++ b/pkg/sql/schemachanger/testdata/explain/alter_table_add_foreign_key @@ -11,7 +11,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t1› ADD CON │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY ForeignKeyConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2, ReferencedDescID: 105} │ └── 1 Mutation operation - │ └── MakeAbsentForeignKeyConstraintWriteOnly {"ConstraintID":2,"ReferencedTableID":105,"TableID":104} + │ └── AddForeignKeyConstraint {"ConstraintID":2,"ReferencedTableID":105,"TableID":104,"Validity":2} ├── PreCommitPhase │ ├── Stage 1 of 2 in PreCommitPhase │ │ ├── 1 element transitioning toward PUBLIC @@ -22,7 +22,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t1› ADD CON │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY ForeignKeyConstraint:{DescID: 104, IndexID: 0, ConstraintID: 2, ReferencedDescID: 105} │ └── 4 Mutation operations - │ ├── MakeAbsentForeignKeyConstraintWriteOnly {"ConstraintID":2,"ReferencedTableID":105,"TableID":104} + │ ├── AddForeignKeyConstraint {"ConstraintID":2,"ReferencedTableID":105,"TableID":104,"Validity":2} │ ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true} │ ├── SetJobStateOnDescriptor {"DescriptorID":105,"Initialize":true} │ └── CreateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."} diff --git a/pkg/sql/schemachanger/testdata/explain/alter_table_add_unique_without_index b/pkg/sql/schemachanger/testdata/explain/alter_table_add_unique_without_index index e2bb7f8bc22b..886e161a8cbe 100644 --- a/pkg/sql/schemachanger/testdata/explain/alter_table_add_unique_without_index +++ b/pkg/sql/schemachanger/testdata/explain/alter_table_add_unique_without_index @@ -11,7 +11,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CONS │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY UniqueWithoutIndexConstraint:{DescID: 104, ConstraintID: 2} │ └── 1 Mutation operation - │ └── MakeAbsentUniqueWithoutIndexConstraintWriteOnly {"ConstraintID":2,"TableID":104} + │ └── AddUniqueWithoutIndexConstraint {"ConstraintID":2,"TableID":104,"Validity":2} ├── PreCommitPhase │ ├── Stage 1 of 2 in PreCommitPhase │ │ ├── 1 element transitioning toward PUBLIC @@ -22,7 +22,7 @@ Schema change plan for ALTER TABLE ‹defaultdb›.‹public›.‹t› ADD CONS │ ├── 1 element transitioning toward PUBLIC │ │ └── ABSENT → WRITE_ONLY UniqueWithoutIndexConstraint:{DescID: 104, ConstraintID: 2} │ └── 3 Mutation operations - │ ├── MakeAbsentUniqueWithoutIndexConstraintWriteOnly {"ConstraintID":2,"TableID":104} + │ ├── AddUniqueWithoutIndexConstraint {"ConstraintID":2,"TableID":104,"Validity":2} │ ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true} │ └── CreateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."} └── PostCommitPhase diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key index 09940e034d6b..b3941af0a06e 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_foreign_key @@ -21,7 +21,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i); │ │ │ └── • 1 Mutation operation │ │ -│ └── • MakeAbsentForeignKeyConstraintWriteOnly +│ └── • AddForeignKeyConstraint │ ColumnIDs: │ - 1 │ ConstraintID: 2 @@ -29,6 +29,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i); │ - 1 │ ReferencedTableID: 105 │ TableID: 104 +│ Validity: 2 │ ├── • PreCommitPhase │ │ @@ -56,7 +57,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i); │ │ │ └── • 4 Mutation operations │ │ -│ ├── • MakeAbsentForeignKeyConstraintWriteOnly +│ ├── • AddForeignKeyConstraint │ │ ColumnIDs: │ │ - 1 │ │ ConstraintID: 2 @@ -64,6 +65,7 @@ EXPLAIN (ddl, verbose) ALTER TABLE t1 ADD FOREIGN KEY (i) REFERENCES t2(i); │ │ - 1 │ │ ReferencedTableID: 105 │ │ TableID: 104 +│ │ Validity: 2 │ │ │ ├── • SetJobStateOnDescriptor │ │ DescriptorID: 104 diff --git a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index index ba7a056dd550..6d113045ede4 100644 --- a/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index +++ b/pkg/sql/schemachanger/testdata/explain_verbose/alter_table_add_unique_without_index @@ -21,11 +21,12 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD UNIQUE WITHOUT INDEX (j); │ │ │ └── • 1 Mutation operation │ │ -│ └── • MakeAbsentUniqueWithoutIndexConstraintWriteOnly +│ └── • AddUniqueWithoutIndexConstraint │ ColumnIDs: │ - 2 │ ConstraintID: 2 │ TableID: 104 +│ Validity: 2 │ ├── • PreCommitPhase │ │ @@ -53,11 +54,12 @@ EXPLAIN (ddl, verbose) ALTER TABLE t ADD UNIQUE WITHOUT INDEX (j); │ │ │ └── • 3 Mutation operations │ │ -│ ├── • MakeAbsentUniqueWithoutIndexConstraintWriteOnly +│ ├── • AddUniqueWithoutIndexConstraint │ │ ColumnIDs: │ │ - 2 │ │ ConstraintID: 2 │ │ TableID: 104 +│ │ Validity: 2 │ │ │ ├── • SetJobStateOnDescriptor │ │ DescriptorID: 104 From 8e149bf9fc953a2684061ffa2a03e4f4566477a3 Mon Sep 17 00:00:00 2001 From: Jane Xing Date: Sun, 5 Feb 2023 23:31:00 -0500 Subject: [PATCH 4/9] sql: use `descs.Txn` for `schemachanger.txn` This commit is to use the new internal executor interface introduced by #93218 for `schemachanger.txn()`. informs #91004 Release Note: None --- pkg/ccl/backupccl/restore_job.go | 4 +- pkg/jobs/registry.go | 4 +- pkg/sql/alter_table.go | 2 +- pkg/sql/backfill.go | 129 ++++++++++------------ pkg/sql/catalog/descs/collection.go | 2 +- pkg/sql/schema_changer.go | 118 ++++++++------------ pkg/sql/schemachanger/scdeps/validator.go | 2 +- 7 files changed, 113 insertions(+), 148 deletions(-) diff --git a/pkg/ccl/backupccl/restore_job.go b/pkg/ccl/backupccl/restore_job.go index 8aebb9576135..256ca0cd3af3 100644 --- a/pkg/ccl/backupccl/restore_job.go +++ b/pkg/ccl/backupccl/restore_job.go @@ -1917,8 +1917,8 @@ func revalidateIndexes( // We don't actually need the 'historical' read the way the schema change does // since our table is offline. runner := descs.NewHistoricalInternalExecTxnRunner(hlc.Timestamp{}, func(ctx context.Context, fn descs.InternalExecFn) error { - return execCfg.InternalDB.Txn(ctx, func(ctx context.Context, txn isql.Txn) error { - return fn(ctx, txn, descs.FromTxn(txn)) + return execCfg.InternalDB.DescsTxn(ctx, func(ctx context.Context, txn descs.Txn) error { + return fn(ctx, txn) }) }) diff --git a/pkg/jobs/registry.go b/pkg/jobs/registry.go index a8f47562e251..2f54afd45587 100644 --- a/pkg/jobs/registry.go +++ b/pkg/jobs/registry.go @@ -111,8 +111,8 @@ type Registry struct { adoptionCh chan adoptionNotice sqlInstance sqlliveness.Instance - // sessionBoundInternalExecutorFactory provides a way for jobs to create - // internal executors. This is rarely needed, and usually job resumers should + // internalDB provides a way for jobs to create internal executors. + // This is rarely needed, and usually job resumers should // use the internal executor from the JobExecCtx. The intended user of this // interface is the schema change job resumer, which needs to set the // tableCollectionModifier on the internal executor to different values in diff --git a/pkg/sql/alter_table.go b/pkg/sql/alter_table.go index 13287ece04da..465f6b4219dd 100644 --- a/pkg/sql/alter_table.go +++ b/pkg/sql/alter_table.go @@ -545,7 +545,7 @@ func (n *alterTableNode) startExec(params runParams) error { ck.CheckDesc().Validity = descpb.ConstraintValidity_Validated } else if fk := c.AsForeignKey(); fk != nil { if err := validateFkInTxn( - params.ctx, params.p.InternalSQLTxn(), params.p.descCollection, n.tableDesc, name, + params.ctx, params.p.InternalSQLTxn(), n.tableDesc, name, ); err != nil { return err } diff --git a/pkg/sql/backfill.go b/pkg/sql/backfill.go index 3edce37697c3..8f1e4fa0de10 100644 --- a/pkg/sql/backfill.go +++ b/pkg/sql/backfill.go @@ -140,7 +140,7 @@ func (sc *SchemaChanger) getChunkSize(chunkSize int64) int64 { } // scTxnFn is the type of functions that operates using transactions in the backfiller. -type scTxnFn func(ctx context.Context, txn isql.Txn, evalCtx *extendedEvalContext) error +type scTxnFn func(ctx context.Context, txn descs.Txn, evalCtx *extendedEvalContext) error // historicalTxnRunner is the type of the callback used by the various // helper functions to run checks at a fixed timestamp (logically, at @@ -152,12 +152,10 @@ func (sc *SchemaChanger) makeFixedTimestampRunner(readAsOf hlc.Timestamp) histor runner := func(ctx context.Context, retryable scTxnFn) error { return sc.fixedTimestampTxnWithExecutor(ctx, readAsOf, func( ctx context.Context, - txn isql.Txn, - sd *sessiondata.SessionData, - descriptors *descs.Collection, + txn descs.Txn, ) error { // We need to re-create the evalCtx since the txn may retry. - evalCtx := createSchemaChangeEvalCtx(ctx, sc.execCfg, sd, readAsOf, descriptors) + evalCtx := createSchemaChangeEvalCtx(ctx, sc.execCfg, txn.SessionData(), readAsOf, txn.Descriptors()) return retryable(ctx, txn, &evalCtx) }) } @@ -173,11 +171,9 @@ func (sc *SchemaChanger) makeFixedTimestampInternalExecRunner( ) error { return sc.fixedTimestampTxnWithExecutor(ctx, readAsOf, func( ctx context.Context, - txn isql.Txn, - _ *sessiondata.SessionData, - descriptors *descs.Collection, + txn descs.Txn, ) error { - return retryable(ctx, txn, descriptors) + return retryable(ctx, txn) }) }) } @@ -185,13 +181,12 @@ func (sc *SchemaChanger) makeFixedTimestampInternalExecRunner( func (sc *SchemaChanger) fixedTimestampTxn( ctx context.Context, readAsOf hlc.Timestamp, - retryable func(ctx context.Context, txn isql.Txn, descriptors *descs.Collection) error, + retryable func(ctx context.Context, txn descs.Txn) error, ) error { return sc.fixedTimestampTxnWithExecutor(ctx, readAsOf, func( - ctx context.Context, txn isql.Txn, _ *sessiondata.SessionData, - descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { - return retryable(ctx, txn, descriptors) + return retryable(ctx, txn) }) } @@ -200,18 +195,16 @@ func (sc *SchemaChanger) fixedTimestampTxnWithExecutor( readAsOf hlc.Timestamp, retryable func( ctx context.Context, - txn isql.Txn, - sd *sessiondata.SessionData, - descriptors *descs.Collection, + txn descs.Txn, ) error, ) error { return sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { if err := txn.KV().SetFixedTimestamp(ctx, readAsOf); err != nil { return err } - return retryable(ctx, txn, txn.SessionData(), descriptors) + return retryable(ctx, txn) }) } @@ -446,8 +439,8 @@ func (sc *SchemaChanger) dropConstraints( } // Create update closure for the table and all other tables with backreferences. - if err := sc.txn(ctx, func(ctx context.Context, txn isql.Txn, descsCol *descs.Collection) error { - scTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + if err := sc.txn(ctx, func(ctx context.Context, txn descs.Txn) error { + scTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } @@ -477,7 +470,7 @@ func (sc *SchemaChanger) dropConstraints( if def.Name != constraint.GetName() { continue } - backrefTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) + backrefTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) if err != nil { return err } @@ -486,7 +479,7 @@ func (sc *SchemaChanger) dropConstraints( ); err != nil { return err } - if err := descsCol.WriteDescToBatch( + if err := txn.Descriptors().WriteDescToBatch( ctx, true /* kvTrace */, backrefTable, b, ); err != nil { return err @@ -525,7 +518,7 @@ func (sc *SchemaChanger) dropConstraints( } } } - if err := descsCol.WriteDescToBatch( + if err := txn.Descriptors().WriteDescToBatch( ctx, true /* kvTrace */, scTable, b, ); err != nil { return err @@ -538,13 +531,13 @@ func (sc *SchemaChanger) dropConstraints( log.Info(ctx, "finished dropping constraints") tableDescs := make(map[descpb.ID]catalog.TableDescriptor, len(fksByBackrefTable)+1) if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) (err error) { - if tableDescs[sc.descID], err = descsCol.ByIDWithLeased(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID); err != nil { + if tableDescs[sc.descID], err = txn.Descriptors().ByIDWithLeased(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID); err != nil { return err } for id := range fksByBackrefTable { - desc, err := descsCol.ByIDWithLeased(txn.KV()).WithoutOffline().Get().Table(ctx, id) + desc, err := txn.Descriptors().ByIDWithLeased(txn.KV()).WithoutOffline().Get().Table(ctx, id) if err != nil { return err } @@ -581,9 +574,9 @@ func (sc *SchemaChanger) addConstraints( // Create update closure for the table and all other tables with backreferences if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { - scTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + scTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } @@ -632,7 +625,7 @@ func (sc *SchemaChanger) addConstraints( } if !foundExisting { scTable.OutboundFKs = append(scTable.OutboundFKs, *fk.ForeignKeyDesc()) - backrefTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) + backrefTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) if err != nil { return err } @@ -656,7 +649,7 @@ func (sc *SchemaChanger) addConstraints( // the last put will win but it's perhaps not ideal. We could add // code to deduplicate but it doesn't seem worth the hassle. if backrefTable != scTable { - if err := descsCol.WriteDescToBatch( + if err := txn.Descriptors().WriteDescToBatch( ctx, true /* kvTrace */, backrefTable, b, ); err != nil { return err @@ -686,7 +679,7 @@ func (sc *SchemaChanger) addConstraints( } } } - if err := descsCol.WriteDescToBatch( + if err := txn.Descriptors().WriteDescToBatch( ctx, true /* kvTrace */, scTable, b, ); err != nil { return err @@ -727,9 +720,9 @@ func (sc *SchemaChanger) validateConstraints( var tableDesc catalog.TableDescriptor if err := sc.fixedTimestampTxn(ctx, readAsOf, func( - ctx context.Context, txn isql.Txn, descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { - tableDesc, err = descriptors.ByID(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID) + tableDesc, err = txn.Descriptors().ByID(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID) return err }); err != nil { return err @@ -756,7 +749,7 @@ func (sc *SchemaChanger) validateConstraints( } desc := descI.(*tabledesc.Mutable) // Each check operates at the historical timestamp. - return runHistoricalTxn(ctx, func(ctx context.Context, txn isql.Txn, evalCtx *extendedEvalContext) error { + return runHistoricalTxn(ctx, func(ctx context.Context, txn descs.Txn, evalCtx *extendedEvalContext) error { // If the constraint is a check constraint that fails validation, we // need a semaContext set up that can resolve types in order to pretty // print the check expression back to the user. @@ -782,7 +775,7 @@ func (sc *SchemaChanger) validateConstraints( return err } } else if c.AsForeignKey() != nil { - if err := validateFkInTxn(ctx, txn, collection, desc, c.GetName()); err != nil { + if err := validateFkInTxn(ctx, txn, desc, c.GetName()); err != nil { return err } } else if c.AsUniqueWithoutIndex() != nil { @@ -922,10 +915,10 @@ func (sc *SchemaChanger) distIndexBackfill( var mutationIdx int if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, col *descs.Collection, + ctx context.Context, txn descs.Txn, ) (err error) { todoSpans, _, mutationIdx, err = rowexec.GetResumeSpans( - ctx, sc.jobRegistry, txn, sc.execCfg.Codec, col, sc.descID, + ctx, sc.jobRegistry, txn, sc.execCfg.Codec, txn.Descriptors(), sc.descID, sc.mutationID, filter, ) return err @@ -960,7 +953,7 @@ func (sc *SchemaChanger) distIndexBackfill( const pageSize = 10000 noop := func(_ []kv.KeyValue) error { return nil } if err := sc.fixedTimestampTxn(ctx, writeAsOf, func( - ctx context.Context, txn isql.Txn, _ *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { for _, span := range targetSpans { // TODO(dt): a Count() request would be nice here if the target isn't @@ -976,7 +969,7 @@ func (sc *SchemaChanger) distIndexBackfill( } log.Infof(ctx, "persisting target safe write time %v...", writeAsOf) if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, _ *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { details := sc.job.Details().(jobspb.SchemaChangeDetails) details.WriteTimestamp = writeAsOf @@ -1003,7 +996,7 @@ func (sc *SchemaChanger) distIndexBackfill( // The txn is used to fetch a tableDesc, partition the spans and set the // evalCtx ts all of which is during planning of the DistSQL flow. if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { // It is okay to release the lease on the descriptor before running the @@ -1017,12 +1010,12 @@ func (sc *SchemaChanger) distIndexBackfill( // clear what this buys us in terms of checking the descriptors validity. // Thus, in favor of simpler code and no correctness concerns we release // the lease once the flow is planned. - tableDesc, err := sc.getTableVersion(ctx, txn.KV(), descriptors, version) + tableDesc, err := sc.getTableVersion(ctx, txn.KV(), txn.Descriptors(), version) if err != nil { return err } sd := NewFakeSessionData(sc.execCfg.SV()) - evalCtx = createSchemaChangeEvalCtx(ctx, sc.execCfg, sd, txn.KV().ReadTimestamp(), descriptors) + evalCtx = createSchemaChangeEvalCtx(ctx, sc.execCfg, sd, txn.KV().ReadTimestamp(), txn.Descriptors()) planCtx = sc.distSQLPlanner.NewPlanningCtx(ctx, &evalCtx, nil, /* planner */ txn.KV(), DistributionTypeSystemTenantOnly) indexBatchSize := indexBackfillBatchSize.Get(&sc.execCfg.Settings.SV) @@ -1302,10 +1295,10 @@ func (sc *SchemaChanger) distColumnBackfill( // variable and assign to todoSpans after committing. var updatedTodoSpans []roachpb.Span if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { updatedTodoSpans = todoSpans - tableDesc, err := sc.getTableVersion(ctx, txn.KV(), descriptors, version) + tableDesc, err := sc.getTableVersion(ctx, txn.KV(), txn.Descriptors(), version) if err != nil { return err } @@ -1318,7 +1311,7 @@ func (sc *SchemaChanger) distColumnBackfill( } cbw := MetadataCallbackWriter{rowResultWriter: &errOnlyResultWriter{}, fn: metaFn} sd := NewFakeSessionData(sc.execCfg.SV()) - evalCtx := createSchemaChangeEvalCtx(ctx, sc.execCfg, sd, txn.KV().ReadTimestamp(), descriptors) + evalCtx := createSchemaChangeEvalCtx(ctx, sc.execCfg, sd, txn.KV().ReadTimestamp(), txn.Descriptors()) recv := MakeDistSQLReceiver( ctx, &cbw, @@ -1432,9 +1425,9 @@ func (sc *SchemaChanger) validateIndexes(ctx context.Context) error { readAsOf := sc.clock.Now() var tableDesc catalog.TableDescriptor if err := sc.fixedTimestampTxn(ctx, readAsOf, func( - ctx context.Context, txn isql.Txn, descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) (err error) { - tableDesc, err = descriptors.ByID(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID) + tableDesc, err = txn.Descriptors().ByID(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID) return err }); err != nil { return err @@ -1530,14 +1523,14 @@ func ValidateConstraint( // The check operates at the historical timestamp. return runHistoricalTxn.Exec(ctx, func( - ctx context.Context, txn isql.Txn, descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { // Use a schema resolver because we need to resolve types by ID and table by name. - resolver := NewSkippingCacheSchemaResolver(descriptors, sessiondata.NewStack(sessionData), txn.KV(), nil /* authAccessor */) + resolver := NewSkippingCacheSchemaResolver(txn.Descriptors(), sessiondata.NewStack(sessionData), txn.KV(), nil /* authAccessor */) semaCtx := tree.MakeSemaContext() semaCtx.TypeResolver = resolver semaCtx.TableNameResolver = resolver - defer func() { descriptors.ReleaseAll(ctx) }() + defer func() { txn.Descriptors().ReleaseAll(ctx) }() switch catalog.GetConstraintType(constraint) { case catconstants.ConstraintTypeCheck: @@ -1565,7 +1558,7 @@ func ValidateConstraint( ) case catconstants.ConstraintTypeFK: fk := constraint.AsForeignKey() - targetTable, err := descriptors.ByID(txn.KV()).Get().Table(ctx, fk.GetReferencedTableID()) + targetTable, err := txn.Descriptors().ByID(txn.KV()).Get().Table(ctx, fk.GetReferencedTableID()) if err != nil { return err } @@ -1651,7 +1644,7 @@ func ValidateInvertedIndexes( key := span.Key endKey := span.EndKey if err := runHistoricalTxn.Exec(ctx, func( - ctx context.Context, txn isql.Txn, _ *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { for { kvs, err := txn.KV().Scan(ctx, key, endKey, 1000000) @@ -1760,7 +1753,7 @@ func countExpectedRowsForInvertedIndex( var expectedCount int64 if err := runHistoricalTxn.Exec(ctx, func( - ctx context.Context, txn isql.Txn, _ *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { var stmt string geoConfig := idx.GetGeoConfig() @@ -1946,7 +1939,7 @@ func populateExpectedCounts( } var tableRowCount int64 if err := runHistoricalTxn.Exec(ctx, func( - ctx context.Context, txn isql.Txn, _ *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { var s strings.Builder for _, idx := range indexes { @@ -2059,7 +2052,7 @@ func countIndexRowsAndMaybeCheckUniqueness( // Retrieve the row count in the index. var idxLen int64 if err := runHistoricalTxn.Exec(ctx, func( - ctx context.Context, txn isql.Txn, _ *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { query := fmt.Sprintf(`SELECT count(1) FROM [%d AS t]@[%d]`, desc.GetID(), idx.GetID()) // If the index is a partial index the predicate must be added @@ -2266,10 +2259,10 @@ func (sc *SchemaChanger) mergeFromTemporaryIndex( ) error { var tbl *tabledesc.Mutable if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { var err error - tbl, err = descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + tbl, err = txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) return err }); err != nil { return err @@ -2288,9 +2281,9 @@ func (sc *SchemaChanger) mergeFromTemporaryIndex( func (sc *SchemaChanger) runStateMachineAfterTempIndexMerge(ctx context.Context) error { var runStatus jobs.RunningStatus return sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { - tbl, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + tbl, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } @@ -2320,7 +2313,7 @@ func (sc *SchemaChanger) runStateMachineAfterTempIndexMerge(ctx context.Context) if runStatus == "" || tbl.Dropped() { return nil } - if err := descsCol.WriteDesc( + if err := txn.Descriptors().WriteDesc( ctx, true /* kvTrace */, tbl, txn.KV(), ); err != nil { return err @@ -2663,11 +2656,7 @@ func validateCheckInTxn( } func getTargetTablesAndFk( - ctx context.Context, - srcTable *tabledesc.Mutable, - txn *kv.Txn, - descsCol *descs.Collection, - fkName string, + ctx context.Context, srcTable *tabledesc.Mutable, txn descs.Txn, fkName string, ) ( syntheticDescs []catalog.Descriptor, fk *descpb.ForeignKeyConstraint, @@ -2688,7 +2677,7 @@ func getTargetTablesAndFk( if fk == nil { return nil, nil, nil, errors.AssertionFailedf("foreign key %s does not exist", fkName) } - targetTable, err = descsCol.ByID(txn).Get().Table(ctx, fk.ReferencedTableID) + targetTable, err = txn.Descriptors().ByID(txn.KV()).Get().Table(ctx, fk.ReferencedTableID) if err != nil { return nil, nil, nil, err } @@ -2714,13 +2703,9 @@ func getTargetTablesAndFk( // It operates entirely on the current goroutine and is thus able to // reuse an existing kv.Txn safely. func validateFkInTxn( - ctx context.Context, - txn isql.Txn, - descsCol *descs.Collection, - srcTable *tabledesc.Mutable, - fkName string, + ctx context.Context, txn descs.Txn, srcTable *tabledesc.Mutable, fkName string, ) error { - syntheticDescs, fk, targetTable, err := getTargetTablesAndFk(ctx, srcTable, txn.KV(), descsCol, fkName) + syntheticDescs, fk, targetTable, err := getTargetTablesAndFk(ctx, srcTable, txn, fkName) if err != nil { return err } diff --git a/pkg/sql/catalog/descs/collection.go b/pkg/sql/catalog/descs/collection.go index a03175bc0950..b1fe4cd26487 100644 --- a/pkg/sql/catalog/descs/collection.go +++ b/pkg/sql/catalog/descs/collection.go @@ -1206,7 +1206,7 @@ func MakeTestCollection(ctx context.Context, leaseManager *lease.Manager) Collec } // InternalExecFn is the type of functions that operates using an internalExecutor. -type InternalExecFn func(ctx context.Context, txn isql.Txn, descriptors *Collection) error +type InternalExecFn func(ctx context.Context, txn Txn) error // HistoricalInternalExecTxnRunnerFn callback for executing with the internal executor // at a fixed timestamp. diff --git a/pkg/sql/schema_changer.go b/pkg/sql/schema_changer.go index 68f5ab736115..2d976eb66699 100644 --- a/pkg/sql/schema_changer.go +++ b/pkg/sql/schema_changer.go @@ -440,8 +440,8 @@ func (sc *SchemaChanger) maybeMakeAddTablePublic( } log.Info(ctx, "making table public") - return sc.txn(ctx, func(ctx context.Context, txn isql.Txn, descsCol *descs.Collection) error { - mut, err := descsCol.MutableByID(txn.KV()).Table(ctx, table.GetID()) + return sc.txn(ctx, func(ctx context.Context, txn descs.Txn) error { + mut, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, table.GetID()) if err != nil { return err } @@ -449,7 +449,7 @@ func (sc *SchemaChanger) maybeMakeAddTablePublic( return nil } mut.State = descpb.DescriptorState_PUBLIC - return descsCol.WriteDesc(ctx, true /* kvTrace */, mut, txn.KV()) + return txn.Descriptors().WriteDesc(ctx, true /* kvTrace */, mut, txn.KV()) }) } @@ -481,8 +481,8 @@ func (sc *SchemaChanger) ignoreRevertedDropIndex( if !table.IsPhysicalTable() { return nil } - return sc.txn(ctx, func(ctx context.Context, txn isql.Txn, descsCol *descs.Collection) error { - mut, err := descsCol.MutableByID(txn.KV()).Table(ctx, table.GetID()) + return sc.txn(ctx, func(ctx context.Context, txn descs.Txn) error { + mut, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, table.GetID()) if err != nil { return err } @@ -500,7 +500,7 @@ func (sc *SchemaChanger) ignoreRevertedDropIndex( mutationsModified = true } if mutationsModified { - return descsCol.WriteDesc(ctx, true /* kvTrace */, mut, txn.KV()) + return txn.Descriptors().WriteDesc(ctx, true /* kvTrace */, mut, txn.KV()) } return nil }) @@ -574,9 +574,9 @@ func (sc *SchemaChanger) getTargetDescriptor(ctx context.Context) (catalog.Descr // Retrieve the descriptor that is being changed. var desc catalog.Descriptor if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descriptors *descs.Collection, + ctx context.Context, txn descs.Txn, ) (err error) { - desc, err = descriptors.ByID(txn.KV()).Get().Desc(ctx, sc.descID) + desc, err = txn.Descriptors().ByID(txn.KV()).Get().Desc(ctx, sc.descID) return err }); err != nil { return nil, err @@ -894,8 +894,8 @@ func (sc *SchemaChanger) handlePermanentSchemaChangeError( // initialize the job running status. func (sc *SchemaChanger) initJobRunningStatus(ctx context.Context) error { - return sc.txn(ctx, func(ctx context.Context, txn isql.Txn, descriptors *descs.Collection) error { - desc, err := descriptors.ByID(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID) + return sc.txn(ctx, func(ctx context.Context, txn descs.Txn) error { + desc, err := txn.Descriptors().ByID(txn.KV()).WithoutNonPublic().Get().Table(ctx, sc.descID) if err != nil { return err } @@ -1034,8 +1034,8 @@ func (sc *SchemaChanger) rollbackSchemaChange(ctx context.Context, err error) er // table was in the ADD state and the schema change failed, then we need to // clean up the descriptor. gcJobID := sc.jobRegistry.MakeJobID() - if err := sc.txn(ctx, func(ctx context.Context, txn isql.Txn, descsCol *descs.Collection) error { - scTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + if err := sc.txn(ctx, func(ctx context.Context, txn descs.Txn) error { + scTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } @@ -1046,17 +1046,17 @@ func (sc *SchemaChanger) rollbackSchemaChange(ctx context.Context, err error) er b := txn.KV().NewBatch() // For views, we need to clean up and references that exist to tables. if scTable.IsView() { - if err := sc.dropViewDeps(ctx, descsCol, txn.KV(), b, scTable); err != nil { + if err := sc.dropViewDeps(ctx, txn.Descriptors(), txn.KV(), b, scTable); err != nil { return err } } scTable.SetDropped() scTable.DropTime = timeutil.Now().UnixNano() const kvTrace = false - if err := descsCol.WriteDescToBatch(ctx, kvTrace, scTable, b); err != nil { + if err := txn.Descriptors().WriteDescToBatch(ctx, kvTrace, scTable, b); err != nil { return err } - if err := descsCol.DeleteNamespaceEntryToBatch(ctx, kvTrace, scTable, b); err != nil { + if err := txn.Descriptors().DeleteNamespaceEntryToBatch(ctx, kvTrace, scTable, b); err != nil { return err } @@ -1094,13 +1094,13 @@ func (sc *SchemaChanger) RunStateMachineBeforeBackfill(ctx context.Context) erro var runStatus jobs.RunningStatus if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { - tbl, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + tbl, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } - dbDesc, err := descsCol.ByID(txn.KV()).WithoutNonPublic().Get().Database(ctx, tbl.GetParentID()) + dbDesc, err := txn.Descriptors().ByID(txn.KV()).WithoutNonPublic().Get().Database(ctx, tbl.GetParentID()) if err != nil { return err } @@ -1143,7 +1143,7 @@ func (sc *SchemaChanger) RunStateMachineBeforeBackfill(ctx context.Context) erro tbl, m, false, // isDone - descsCol, + txn.Descriptors(), ); err != nil { return err } @@ -1151,7 +1151,7 @@ func (sc *SchemaChanger) RunStateMachineBeforeBackfill(ctx context.Context) erro if doNothing := runStatus == "" || tbl.Dropped(); doNothing { return nil } - if err := descsCol.WriteDesc( + if err := txn.Descriptors().WriteDesc( ctx, true /* kvTrace */, tbl, txn.KV(), ); err != nil { return err @@ -1200,9 +1200,9 @@ func (sc *SchemaChanger) stepStateMachineAfterIndexBackfill(ctx context.Context) var runStatus jobs.RunningStatus if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { - tbl, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + tbl, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } @@ -1232,7 +1232,7 @@ func (sc *SchemaChanger) stepStateMachineAfterIndexBackfill(ctx context.Context) if runStatus == "" || tbl.Dropped() { return nil } - if err := descsCol.WriteDesc( + if err := txn.Descriptors().WriteDesc( ctx, true /* kvTrace */, tbl, txn.KV(), ); err != nil { return err @@ -1342,24 +1342,24 @@ func (sc *SchemaChanger) done(ctx context.Context) error { var depMutationJobs []jobspb.JobID var otherJobIDs []jobspb.JobID err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { depMutationJobs = depMutationJobs[:0] otherJobIDs = otherJobIDs[:0] var err error - scTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + scTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } - dbDesc, err := descsCol.ByID(txn.KV()).WithoutNonPublic().Get().Database(ctx, scTable.GetParentID()) + dbDesc, err := txn.Descriptors().ByID(txn.KV()).WithoutNonPublic().Get().Database(ctx, scTable.GetParentID()) if err != nil { return err } collectReferencedTypeIDs := func() (catalog.DescriptorIDSet, error) { typeLookupFn := func(id descpb.ID) (catalog.TypeDescriptor, error) { - desc, err := descsCol.ByIDWithLeased(txn.KV()).WithoutNonPublic().Get().Type(ctx, id) + desc, err := txn.Descriptors().ByIDWithLeased(txn.KV()).WithoutNonPublic().Get().Type(ctx, id) if err != nil { return nil, err } @@ -1415,13 +1415,13 @@ func (sc *SchemaChanger) done(ctx context.Context) error { if fk := m.AsForeignKey(); fk != nil && fk.Adding() && fk.GetConstraintValidity() == descpb.ConstraintValidity_Unvalidated { // Add backreference on the referenced table (which could be the same table) - backrefTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) + backrefTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) if err != nil { return err } backrefTable.InboundFKs = append(backrefTable.InboundFKs, *fk.ForeignKeyDesc()) if backrefTable != scTable { - if err := descsCol.WriteDescToBatch(ctx, kvTrace, backrefTable, b); err != nil { + if err := txn.Descriptors().WriteDescToBatch(ctx, kvTrace, backrefTable, b); err != nil { return err } } @@ -1437,7 +1437,7 @@ func (sc *SchemaChanger) done(ctx context.Context) error { scTable, m, true, // isDone - descsCol, + txn.Descriptors(), ); err != nil { return err } @@ -1594,7 +1594,7 @@ func (sc *SchemaChanger) done(ctx context.Context) error { } if err := setNewLocalityConfig( - ctx, txn.KV(), descsCol, b, scTable, localityConfigToSwapTo, kvTrace, + ctx, txn.KV(), txn.Descriptors(), b, scTable, localityConfigToSwapTo, kvTrace, ); err != nil { return err } @@ -1690,7 +1690,7 @@ func (sc *SchemaChanger) done(ctx context.Context) error { // Update the set of back references. for id, isAddition := range update { - typ, err := descsCol.MutableByID(txn.KV()).Type(ctx, id) + typ, err := txn.Descriptors().MutableByID(txn.KV()).Type(ctx, id) if err != nil { return err } @@ -1699,7 +1699,7 @@ func (sc *SchemaChanger) done(ctx context.Context) error { } else { typ.RemoveReferencingDescriptorID(scTable.ID) } - if err := descsCol.WriteDescToBatch(ctx, kvTrace, typ, b); err != nil { + if err := txn.Descriptors().WriteDescToBatch(ctx, kvTrace, typ, b); err != nil { return err } } @@ -1731,12 +1731,12 @@ func (sc *SchemaChanger) done(ctx context.Context) error { // Update the set of back references. for id, colIDSet := range update { - tbl, err := descsCol.MutableByID(txn.KV()).Table(ctx, id) + tbl, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, id) if err != nil { return err } tbl.UpdateColumnsDependedOnBy(scTable.ID, colIDSet) - if err := descsCol.WriteDescToBatch(ctx, kvTrace, tbl, b); err != nil { + if err := txn.Descriptors().WriteDescToBatch(ctx, kvTrace, tbl, b); err != nil { return err } } @@ -1745,7 +1745,7 @@ func (sc *SchemaChanger) done(ctx context.Context) error { // Clean up any comments related to the mutations, specifically if we need // to drop them. for _, comment := range commentsToDelete { - if err := descsCol.DeleteCommentInBatch( + if err := txn.Descriptors().DeleteCommentInBatch( ctx, false /* kvTrace */, b, catalogkeys.MakeCommentKey(uint32(comment.id), uint32(comment.subID), comment.commentType), ); err != nil { return err @@ -1753,23 +1753,23 @@ func (sc *SchemaChanger) done(ctx context.Context) error { } for _, comment := range commentsToSwap { - cmt, found := descsCol.GetComment(catalogkeys.MakeCommentKey(uint32(comment.id), uint32(comment.oldSubID), comment.commentType)) + cmt, found := txn.Descriptors().GetComment(catalogkeys.MakeCommentKey(uint32(comment.id), uint32(comment.oldSubID), comment.commentType)) if !found { continue } - if err := descsCol.DeleteCommentInBatch( + if err := txn.Descriptors().DeleteCommentInBatch( ctx, false /* kvTrace */, b, catalogkeys.MakeCommentKey(uint32(comment.id), uint32(comment.oldSubID), comment.commentType), ); err != nil { return err } - if err := descsCol.WriteCommentToBatch( + if err := txn.Descriptors().WriteCommentToBatch( ctx, false /* kvTrace */, b, catalogkeys.MakeCommentKey(uint32(comment.id), uint32(comment.newSubID), comment.commentType), cmt, ); err != nil { return err } } - if err := descsCol.WriteDescToBatch(ctx, kvTrace, scTable, b); err != nil { + if err := txn.Descriptors().WriteDescToBatch(ctx, kvTrace, scTable, b); err != nil { return err } if err := txn.KV().Run(ctx, b); err != nil { @@ -1924,8 +1924,8 @@ func (sc *SchemaChanger) maybeReverseMutations(ctx context.Context, causingError // Get the other tables whose foreign key backreferences need to be removed. alreadyReversed := false const kvTrace = true // TODO(ajwerner): figure this out - err := sc.txn(ctx, func(ctx context.Context, txn isql.Txn, descsCol *descs.Collection) error { - scTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + err := sc.txn(ctx, func(ctx context.Context, txn descs.Txn) error { + scTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } @@ -1998,7 +1998,7 @@ func (sc *SchemaChanger) maybeReverseMutations(ctx context.Context, causingError } // Get the foreign key backreferences to remove. if fk := constraint.AsForeignKey(); fk != nil { - backrefTable, err := descsCol.MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) + backrefTable, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, fk.GetReferencedTableID()) if err != nil { return err } @@ -2009,7 +2009,7 @@ func (sc *SchemaChanger) maybeReverseMutations(ctx context.Context, causingError log.Infof(ctx, "error attempting to remove backreference %s during rollback: %s", constraint.GetName(), err) } - if err := descsCol.WriteDescToBatch(ctx, kvTrace, backrefTable, b); err != nil { + if err := txn.Descriptors().WriteDescToBatch(ctx, kvTrace, backrefTable, b); err != nil { return err } } @@ -2030,7 +2030,7 @@ func (sc *SchemaChanger) maybeReverseMutations(ctx context.Context, causingError // Read the table descriptor from the store. The Version of the // descriptor has already been incremented in the transaction and // this descriptor can be modified without incrementing the version. - if err := descsCol.WriteDescToBatch(ctx, kvTrace, scTable, b); err != nil { + if err := txn.Descriptors().WriteDescToBatch(ctx, kvTrace, scTable, b); err != nil { return err } if err := txn.KV().Run(ctx, b); err != nil { @@ -2447,27 +2447,7 @@ type SchemaChangerTestingKnobs struct { func (*SchemaChangerTestingKnobs) ModuleTestingKnobs() {} // txn is a convenient wrapper around descs.Txn(). -// -// TODO(ajwerner): Replace this with direct calls to DescsTxn. -func (sc *SchemaChanger) txn( - ctx context.Context, f func(context.Context, isql.Txn, *descs.Collection) error, -) error { - return sc.txnWithExecutor(ctx, func( - ctx context.Context, txn isql.Txn, _ *sessiondata.SessionData, - collection *descs.Collection, - ) error { - return f(ctx, txn, collection) - }) -} - -// txnWithExecutor is to run internal executor within a txn. -func (sc *SchemaChanger) txnWithExecutor( - ctx context.Context, - f func( - context.Context, isql.Txn, *sessiondata.SessionData, - *descs.Collection, - ) error, -) error { +func (sc *SchemaChanger) txn(ctx context.Context, f func(context.Context, descs.Txn) error) error { if fn := sc.testingKnobs.RunBeforeDescTxn; fn != nil { if err := fn(sc.job.ID()); err != nil { return err @@ -2476,7 +2456,7 @@ func (sc *SchemaChanger) txnWithExecutor( return sc.execCfg.InternalDB.DescsTxn(ctx, func( ctx context.Context, txn descs.Txn, ) error { - return f(ctx, txn, txn.SessionData(), txn.Descriptors()) + return f(ctx, txn) }) } @@ -3085,9 +3065,9 @@ func (sc *SchemaChanger) getDependentMutationsJobs( func (sc *SchemaChanger) preSplitHashShardedIndexRanges(ctx context.Context) error { if err := sc.txn(ctx, func( - ctx context.Context, txn isql.Txn, descsCol *descs.Collection, + ctx context.Context, txn descs.Txn, ) error { - tableDesc, err := descsCol.MutableByID(txn.KV()).Table(ctx, sc.descID) + tableDesc, err := txn.Descriptors().MutableByID(txn.KV()).Table(ctx, sc.descID) if err != nil { return err } diff --git a/pkg/sql/schemachanger/scdeps/validator.go b/pkg/sql/schemachanger/scdeps/validator.go index 8c6009b2dc83..e63ac296a671 100644 --- a/pkg/sql/schemachanger/scdeps/validator.go +++ b/pkg/sql/schemachanger/scdeps/validator.go @@ -136,7 +136,7 @@ func (vd validator) makeHistoricalInternalExecTxnRunner() descs.HistoricalIntern if err := txn.KV().SetFixedTimestamp(ctx, now); err != nil { return err } - return fn(ctx, txn, txn.Descriptors()) + return fn(ctx, txn) }) }) } From f01c861457d9e5bae8cc08b0d5f66e0fbf62e37d Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Thu, 26 Jan 2023 10:12:09 +0000 Subject: [PATCH 5/9] kvserver: use narrow checkpoints in consistency checker This commit modifies the consistency checker to save partial checkpoints instead of full ones. The partial checkpoints contain the data for the inconsistent range and its immediate left and right neighbour in the Store. Only the inconsistent range's Replica is locked at the time of snapshotting the store before creating the checkpoint, other ranges can be out of sync. They are checkpointed too, to give more context when e.g. debugging non-trivial split/merge scenarios. Release note (ops change): In the rare event of Range inconsistency, the consistency checker saves a storage checkpoint on each storage the Range belongs to. Previously, this was a full checkpoint, so its cost could quickly escalate on the nodes that went on running. This change makes the checkpoints partial, i.e. they now only contain the relevant range and its neighbours. This eliminates the time pressure on the cluster operator to remove the checkpoints. Epic: none --- pkg/kv/kvserver/replica_consistency.go | 31 +++++------ pkg/kv/kvserver/store.go | 74 +++++++++++++++++++++++--- pkg/kv/kvserver/store_replica_btree.go | 16 ++++++ pkg/server/debug/replay/replay.go | 2 +- pkg/storage/engine.go | 5 +- pkg/storage/engine_test.go | 4 +- pkg/storage/pebble.go | 31 ++++++++++- 7 files changed, 132 insertions(+), 31 deletions(-) diff --git a/pkg/kv/kvserver/replica_consistency.go b/pkg/kv/kvserver/replica_consistency.go index 1e90400f3234..3781f4a2846e 100644 --- a/pkg/kv/kvserver/replica_consistency.go +++ b/pkg/kv/kvserver/replica_consistency.go @@ -665,8 +665,10 @@ func (r *Replica) computeChecksumPostApply( } // NB: the names here will match on all nodes, which is nice for debugging. tag := fmt.Sprintf("r%d_at_%d", r.RangeID, as.RaftAppliedIndex) - if dir, err := r.store.checkpoint(ctx, tag); err != nil { - log.Warningf(ctx, "unable to create checkpoint %s: %+v", dir, err) + spans := r.store.checkpointSpans(&desc) + log.Warningf(ctx, "creating checkpoint %s with spans %+v", tag, spans) + if dir, err := r.store.checkpoint(tag, spans); err != nil { + log.Warningf(ctx, "unable to create checkpoint %s: %+v", tag, err) } else { log.Warningf(ctx, "created checkpoint %s", dir) } @@ -721,7 +723,6 @@ func (r *Replica) computeChecksumPostApply( } r.computeChecksumDone(c, result) } - var shouldFatal bool for _, rDesc := range cc.Terminate { if rDesc.StoreID == r.store.StoreID() && rDesc.ReplicaID == r.replicaID { @@ -757,23 +758,19 @@ A file preventing this node from restarting was placed at: Checkpoints are created on each node/store hosting this range, to help investigate the cause. Only nodes that are more likely to have incorrect data -are terminated, and usually a majority of replicas continue running. - -The storage checkpoint directory MUST be deleted or moved away timely, on the -nodes that continue operating. Over time the storage engine gets updated and -compacted, which leads to checkpoints becoming a full copy of a past state. Even -with no writes to the database, on these stores disk consumption may double in a -matter of hours/days, depending on compaction schedule. +are terminated, and usually a majority of replicas continue running. Checkpoints +are partial, i.e. contain only the data from to the inconsistent range, and +possibly its neighbouring ranges. -Checkpoints are very helpful in debugging this issue, so before deleting them, -please consider alternative actions: +The storage checkpoint directories can/should be deleted when no longer needed. +They are very helpful in debugging this issue, so before deleting them, please +consider alternative actions: -- If the store has enough capacity, hold off deleting the checkpoint until CRDB - staff has diagnosed the issue. -- Consider backing up the checkpoints before removing them, e.g. by snapshotting - the disk. +- If the store has enough capacity, hold off the deletion until CRDB staff has + diagnosed the issue. +- Back up the checkpoints for later investigation. - If the stores are nearly full, but the cluster has enough capacity, consider - gradually decomissioning the affected nodes, to retain the checkpoints. + gradually decommissioning the affected nodes, to retain the checkpoints. To inspect the checkpoints, one can use the cockroach debug range-data tool, and command line tools like diff. For example: diff --git a/pkg/kv/kvserver/store.go b/pkg/kv/kvserver/store.go index 4d9f7fcc829c..06dd59d52255 100644 --- a/pkg/kv/kvserver/store.go +++ b/pkg/kv/kvserver/store.go @@ -50,6 +50,7 @@ import ( "github.com/cockroachdb/cockroach/pkg/kv/kvserver/multiqueue" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/raftentry" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/rangefeed" + "github.com/cockroachdb/cockroach/pkg/kv/kvserver/rditer" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/tenantrate" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/tscache" "github.com/cockroachdb/cockroach/pkg/kv/kvserver/txnrecovery" @@ -2879,18 +2880,77 @@ func (s *Store) checkpointsDir() string { return filepath.Join(s.engine.GetAuxiliaryDir(), "checkpoints") } -// checkpoint creates a RocksDB checkpoint in the auxiliary directory with the -// provided tag used in the filepath. The filepath for the checkpoint directory -// is returned. -func (s *Store) checkpoint(ctx context.Context, tag string) (string, error) { +// checkpointSpans returns key spans containing the given range. The spans may +// be wider, and contain a few extra ranges that surround the given range. The +// extension of the spans gives more information for debugging consistency or +// storage issues, e.g. in situations when a recent reconfiguration like split +// or merge occurred. +func (s *Store) checkpointSpans(desc *roachpb.RangeDescriptor) []roachpb.Span { + // Find immediate left and right neighbours by range ID. + var prevID, nextID roachpb.RangeID + s.mu.replicasByRangeID.Range(func(r *Replica) { + if id, our := r.RangeID, desc.RangeID; id < our && id > prevID { + prevID = id + } else if id > our && (nextID == 0 || id < nextID) { + nextID = id + } + }) + if prevID == 0 { + prevID = desc.RangeID + } + if nextID == 0 { + nextID = desc.RangeID + } + + // Find immediate left and right neighbours by user key. + s.mu.RLock() + left := s.mu.replicasByKey.LookupPrecedingReplica(context.Background(), desc.StartKey) + right := s.mu.replicasByKey.LookupNextReplica(context.Background(), desc.EndKey) + s.mu.RUnlock() + + // Cover all range IDs (prevID, desc.RangeID, nextID) using a continuous span. + spanRangeIDs := func(first, last roachpb.RangeID) roachpb.Span { + return roachpb.Span{ + Key: keys.MakeRangeIDPrefix(first), + EndKey: keys.MakeRangeIDPrefix(last).PrefixEnd(), + } + } + spans := []roachpb.Span{spanRangeIDs(prevID, nextID)} + + userKeys := desc.RSpan() + // Include the rangeID-local data comprising ranges left, desc, and right. + if left != nil { + userKeys.Key = left.Desc().StartKey + // Skip this range ID if it was already covered by [prevID, nextID]. + if id := left.RangeID; id < prevID || id > nextID { + spans = append(spans, spanRangeIDs(id, id)) + } + } + if right != nil { + userKeys.EndKey = right.Desc().EndKey + // Skip this range ID if it was already covered by [prevID, nextID]. + if id := right.RangeID; id < prevID || id > nextID { + spans = append(spans, spanRangeIDs(id, id)) + } + } + // Include replicated user key span containing ranges left, desc, and right. + // TODO(tbg): rangeID is ignored here, make a rangeID-agnostic helper. + spans = append(spans, rditer.Select(0, rditer.SelectOpts{ReplicatedBySpan: userKeys})...) + + return spans +} + +// checkpoint creates a Pebble checkpoint in the auxiliary directory with the +// provided tag used in the filepath. Returns the path to the created checkpoint +// directory. The checkpoint includes only files that intersect with either of +// the provided key spans. If spans is empty, it includes the entire store. +func (s *Store) checkpoint(tag string, spans []roachpb.Span) (string, error) { checkpointBase := s.checkpointsDir() _ = s.engine.MkdirAll(checkpointBase) - checkpointDir := filepath.Join(checkpointBase, tag) - if err := s.engine.CreateCheckpoint(checkpointDir); err != nil { + if err := s.engine.CreateCheckpoint(checkpointDir, spans); err != nil { return "", err } - return checkpointDir, nil } diff --git a/pkg/kv/kvserver/store_replica_btree.go b/pkg/kv/kvserver/store_replica_btree.go index 45825392db36..1530511dc2a9 100644 --- a/pkg/kv/kvserver/store_replica_btree.go +++ b/pkg/kv/kvserver/store_replica_btree.go @@ -89,6 +89,22 @@ func (b *storeReplicaBTree) LookupPrecedingReplica(ctx context.Context, key roac return repl } +// LookupNextReplica returns the first / leftmost Replica (if any) whose start +// key is greater or equal to the provided key. Placeholders are ignored. +func (b *storeReplicaBTree) LookupNextReplica(ctx context.Context, key roachpb.RKey) *Replica { + var repl *Replica + if err := b.ascendRange(ctx, key, roachpb.RKeyMax, func(_ context.Context, it replicaOrPlaceholder) error { + if it.repl != nil { + repl = it.repl + return iterutil.StopIteration() + } + return nil + }); err != nil { + panic(err) + } + return repl +} + // VisitKeyRange invokes the visitor on all the replicas for ranges that // overlap [startKey, endKey), or until the visitor returns false, in the // specific order. (The items in the btree are non-overlapping). diff --git a/pkg/server/debug/replay/replay.go b/pkg/server/debug/replay/replay.go index f0ea6342dbff..7a9adec314c2 100644 --- a/pkg/server/debug/replay/replay.go +++ b/pkg/server/debug/replay/replay.go @@ -85,7 +85,7 @@ func (h *HTTPHandler) HandleRequest(w http.ResponseWriter, req *http.Request) { } if !wc.IsRunning() { wc.Start(captureFS, actionJSON.CaptureDirectory) - err = s.Engine().CreateCheckpoint(captureFS.PathJoin(actionJSON.CaptureDirectory, "checkpoint")) + err = s.Engine().CreateCheckpoint(captureFS.PathJoin(actionJSON.CaptureDirectory, "checkpoint"), nil) } if err != nil { return err diff --git a/pkg/storage/engine.go b/pkg/storage/engine.go index 5cbb8001195d..92e6f9ef9f0d 100644 --- a/pkg/storage/engine.go +++ b/pkg/storage/engine.go @@ -951,8 +951,9 @@ type Engine interface { fs.FS // CreateCheckpoint creates a checkpoint of the engine in the given directory, // which must not exist. The directory should be on the same file system so - // that hard links can be used. - CreateCheckpoint(dir string) error + // that hard links can be used. If spans is not empty, the checkpoint excludes + // SSTs that don't overlap with any of these key spans. + CreateCheckpoint(dir string, spans []roachpb.Span) error // SetMinVersion is used to signal to the engine the current minimum // version that it must maintain compatibility with. diff --git a/pkg/storage/engine_test.go b/pkg/storage/engine_test.go index b3ae15ed3f92..14ef62c9f0d0 100644 --- a/pkg/storage/engine_test.go +++ b/pkg/storage/engine_test.go @@ -1008,12 +1008,12 @@ func TestCreateCheckpoint(t *testing.T) { dir = filepath.Join(dir, "checkpoint") assert.NoError(t, err) - assert.NoError(t, db.CreateCheckpoint(dir)) + assert.NoError(t, db.CreateCheckpoint(dir, nil)) assert.DirExists(t, dir) m, err := filepath.Glob(dir + "/*") assert.NoError(t, err) assert.True(t, len(m) > 0) - if err := db.CreateCheckpoint(dir); !testutils.IsError(err, "exists") { + if err := db.CreateCheckpoint(dir, nil); !testutils.IsError(err, "exists") { t.Fatal(err) } } diff --git a/pkg/storage/pebble.go b/pkg/storage/pebble.go index e555d148fea3..1b32fecf23f1 100644 --- a/pkg/storage/pebble.go +++ b/pkg/storage/pebble.go @@ -1886,9 +1886,36 @@ func (p *Pebble) Stat(name string) (os.FileInfo, error) { return p.fs.Stat(name) } +func checkpointSpansNote(spans []roachpb.Span) []byte { + note := "CRDB spans:\n" + for _, span := range spans { + note += span.String() + "\n" + } + return []byte(note) +} + // CreateCheckpoint implements the Engine interface. -func (p *Pebble) CreateCheckpoint(dir string) error { - return p.db.Checkpoint(dir, pebble.WithFlushedWAL()) +func (p *Pebble) CreateCheckpoint(dir string, spans []roachpb.Span) error { + opts := []pebble.CheckpointOption{ + pebble.WithFlushedWAL(), + } + if l := len(spans); l > 0 { + s := make([]pebble.CheckpointSpan, 0, l) + for _, span := range spans { + s = append(s, pebble.CheckpointSpan{Start: span.Key, End: span.EndKey}) + } + opts = append(opts, pebble.WithRestrictToSpans(s)) + } + if err := p.db.Checkpoint(dir, opts...); err != nil { + return err + } + + // TODO(#90543, cockroachdb/pebble#2285): move spans info to Pebble manifest. + if len(spans) > 0 { + return fs.SafeWriteToFile(p.fs, dir, p.fs.PathJoin(dir, "checkpoint.txt"), + checkpointSpansNote(spans)) + } + return nil } // SetMinVersion implements the Engine interface. From 87e8de9727e7b3c620af5286f2d349d4452d9ffe Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Mon, 30 Jan 2023 15:28:42 +0000 Subject: [PATCH 6/9] roachtest,kvserver: brush up consistency check tests Release note: none Epic: none --- pkg/cmd/roachtest/tests/inconsistency.go | 61 +++++++++-------------- pkg/kv/kvserver/consistency_queue_test.go | 6 +-- 2 files changed, 26 insertions(+), 41 deletions(-) diff --git a/pkg/cmd/roachtest/tests/inconsistency.go b/pkg/cmd/roachtest/tests/inconsistency.go index 164d6912e8de..e1a78819f91f 100644 --- a/pkg/cmd/roachtest/tests/inconsistency.go +++ b/pkg/cmd/roachtest/tests/inconsistency.go @@ -32,11 +32,9 @@ func registerInconsistency(r registry.Registry) { } func runInconsistency(ctx context.Context, t test.Test, c cluster.Cluster) { - startOps := option.DefaultStartOpts() - nodes := c.Range(1, 3) c.Put(ctx, t.Cockroach(), "./cockroach", nodes) - c.Start(ctx, t.L(), startOps, install.MakeClusterSettings(), nodes) + c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), nodes) { db := c.Conn(ctx, t.L(), 1) @@ -45,19 +43,18 @@ func runInconsistency(ctx context.Context, t test.Test, c cluster.Cluster) { // to expect it. _, err := db.ExecContext(ctx, `SET CLUSTER SETTING server.consistency_check.interval = '0'`) require.NoError(t, err) - err = WaitFor3XReplication(ctx, t, db) - require.NoError(t, err) - _, db = db.Close(), nil + require.NoError(t, WaitFor3XReplication(ctx, t, db)) + require.NoError(t, db.Close()) } // Stop the cluster "gracefully" by letting each node initiate a "hard - // shutdown" This will prevent any potential problems in which data isn't + // shutdown". This will prevent any potential problems in which data isn't // synced. It seems (remotely) possible (see #64602) that without this, we // sometimes let the inconsistency win by ending up replicating it to all // nodes. This has not been conclusively proven, though. // - // First SIGINT initiates graceful shutdown, second one initiates a - // "hard" (i.e. don't shed leases, etc) shutdown. + // First SIGINT initiates graceful shutdown, second one initiates a "hard" + // (i.e. don't shed leases, etc) shutdown. stopOpts := option.DefaultStopOpts() stopOpts.RoachprodOpts.Wait = false stopOpts.RoachprodOpts.Sig = 2 @@ -95,7 +92,8 @@ func runInconsistency(ctx context.Context, t test.Test, c cluster.Cluster) { // If the consistency check "fails to fail", the verbose logging will help // determine why. startOpts := option.DefaultStartOpts() - startOpts.RoachprodOpts.ExtraArgs = append(startOpts.RoachprodOpts.ExtraArgs, "--vmodule=consistency_queue=5,replica_consistency=5,queue=5") + startOpts.RoachprodOpts.ExtraArgs = append(startOpts.RoachprodOpts.ExtraArgs, + "--vmodule=consistency_queue=5,replica_consistency=5,queue=5") c.Start(ctx, t.L(), startOpts, install.MakeClusterSettings(), nodes) m.Go(func(ctx context.Context) error { select { @@ -107,53 +105,40 @@ func runInconsistency(ctx context.Context, t test.Test, c cluster.Cluster) { time.Sleep(10 * time.Second) // wait for n1-n3 to all be known as live to each other - // set an aggressive consistency check interval, but only now (that we're + // Set an aggressive consistency check interval, but only now (that we're // reasonably sure all nodes are live, etc). This makes sure that the consistency // check runs against all three nodes. If it targeted only two nodes, a random // one would fatal - not what we want. { db := c.Conn(ctx, t.L(), 2) _, err := db.ExecContext(ctx, `SET CLUSTER SETTING server.consistency_check.interval = '10ms'`) - if err != nil { - t.Fatal(err) - } - _ = db.Close() - } - - if err := m.WaitE(); err == nil { - t.Fatal("expected a node to crash") + require.NoError(t, err) + require.NoError(t, db.Close()) } + require.Error(t, m.WaitE(), "expected a node to crash") time.Sleep(20 * time.Second) // wait for liveness to time out for dead nodes db := c.Conn(ctx, t.L(), 2) rows, err := db.Query(`SELECT node_id FROM crdb_internal.gossip_nodes WHERE is_live = false;`) - if err != nil { - t.Fatal(err) - } + require.NoError(t, err) var ids []int for rows.Next() { var id int - if err := rows.Scan(&id); err != nil { - t.Fatal(err) - } + require.NoError(t, rows.Scan(&id)) ids = append(ids, id) } - if err := rows.Err(); err != nil { - t.Fatal(err) - } - if len(ids) != 1 { - t.Fatalf("expected one dead NodeID, got %v", ids) - } + require.NoError(t, rows.Err()) + require.Len(t, ids, 1, "expected one dead NodeID") + const expr = "This.node.is.terminating.because.a.replica.inconsistency.was.detected" - c.Run(ctx, c.Node(1), "grep "+ - expr+" "+"{log-dir}/cockroach.log") + c.Run(ctx, c.Node(1), "grep "+expr+" {log-dir}/cockroach.log") - if err := c.StartE(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.Node(1)); err == nil { - // NB: we can't easily verify the error because there's a lot of output - // which isn't fully included in the error returned from StartE. - t.Fatalf("node restart should have failed") - } + // NB: we can't easily verify the error because there's a lot of output which + // isn't fully included in the error returned from StartE. + require.Error(t, c.StartE( + ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.Node(1), + ), "node restart should have failed") // roachtest checks that no nodes are down when the test finishes, but in this // case we have a down node that we can't restart. Remove the data dir, which diff --git a/pkg/kv/kvserver/consistency_queue_test.go b/pkg/kv/kvserver/consistency_queue_test.go index 993c64705e98..6f670bd51392 100644 --- a/pkg/kv/kvserver/consistency_queue_test.go +++ b/pkg/kv/kvserver/consistency_queue_test.go @@ -258,9 +258,9 @@ func TestCheckConsistencyInconsistent(t *testing.T) { stickyEngineRegistry := server.NewStickyInMemEnginesRegistry() defer stickyEngineRegistry.CloseAllStickyInMemEngines() - // The cluster has 3 node, with 1 store per node. The test writes a few KVs to - // a range, which gets replicated to all 3 stores. Then it manually replaces - // an entry in s2. The consistency check must detect this and terminate n2/s2. + // The cluster has 3 nodes, one store per node. The test writes a few KVs to a + // range, which gets replicated to all 3 stores. Then it manually replaces an + // entry in s2. The consistency check must detect this and terminate n2/s2. const numStores = 3 testKnobs := kvserver.StoreTestingKnobs{DisableConsistencyQueue: true} var tc *testcluster.TestCluster From 690cebeb727b49023b6ee148e3090b7c6d6f2bd9 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Tue, 31 Jan 2023 13:48:57 +0000 Subject: [PATCH 7/9] roachtest: inspect checkpoint in consistency check test This commit modifies the consistency check failure roachtest to verify that all the nodes of an inconsistent range create a checkpoint that can be opened with tooling. Release note: none Epic: none --- pkg/cmd/roachtest/tests/inconsistency.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/pkg/cmd/roachtest/tests/inconsistency.go b/pkg/cmd/roachtest/tests/inconsistency.go index e1a78819f91f..9ca222f1c4e2 100644 --- a/pkg/cmd/roachtest/tests/inconsistency.go +++ b/pkg/cmd/roachtest/tests/inconsistency.go @@ -134,6 +134,22 @@ func runInconsistency(ctx context.Context, t test.Test, c cluster.Cluster) { const expr = "This.node.is.terminating.because.a.replica.inconsistency.was.detected" c.Run(ctx, c.Node(1), "grep "+expr+" {log-dir}/cockroach.log") + // Make sure that every node creates a checkpoint. + for n := 1; n <= 3; n++ { + // Notes it in the log. + const expr = "creating.checkpoint.*with.spans" + c.Run(ctx, c.Node(n), "grep "+expr+" {log-dir}/cockroach.log") + // Creates at least one checkpoint directory (in rare cases it can be + // multiple if multiple consistency checks fail in close succession), and + // puts spans information into the checkpoint.txt file in it. + c.Run(ctx, c.Node(n), "find {store-dir}/auxiliary/checkpoints -name checkpoint.txt") + // The checkpoint can be inspected by the tooling. + c.Run(ctx, c.Node(n), "./cockroach debug range-descriptors "+ + "$(find {store-dir}/auxiliary/checkpoints/* -type d -depth 0 | head -n1)") + c.Run(ctx, c.Node(n), "./cockroach debug range-data --limit 10 "+ + "$(find {store-dir}/auxiliary/checkpoints/* -type d -depth 0 | head -n1) 1") + } + // NB: we can't easily verify the error because there's a lot of output which // isn't fully included in the error returned from StartE. require.Error(t, c.StartE( From 88f4f51b7e026acacf9e0b8b0900fa034066f800 Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Tue, 31 Jan 2023 19:04:53 +0000 Subject: [PATCH 8/9] kvserver: add LookupNextReplica test Release note: none Epic: none --- pkg/kv/kvserver/store_replica_btree_test.go | 40 +++++++++++---------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/pkg/kv/kvserver/store_replica_btree_test.go b/pkg/kv/kvserver/store_replica_btree_test.go index 2a89c101910c..3b13f004f3c9 100644 --- a/pkg/kv/kvserver/store_replica_btree_test.go +++ b/pkg/kv/kvserver/store_replica_btree_test.go @@ -87,7 +87,7 @@ func TestStoreReplicaBTree_VisitKeyRange(t *testing.T) { }) } -func TestStoreReplicaBTree_LookupPrecedingReplica(t *testing.T) { +func TestStoreReplicaBTree_LookupPrecedingAndNextReplica(t *testing.T) { defer leaktest.AfterTest(t)() ctx := context.Background() @@ -116,25 +116,29 @@ func TestStoreReplicaBTree_LookupPrecedingReplica(t *testing.T) { require.Zero(t, b.ReplaceOrInsertReplica(ctx, repl5)) for i, tc := range []struct { - key string - expRepl *Replica + key string + preRepl *Replica + nextRepl *Replica }{ - {"", nil}, - {"a", nil}, - {"aa", nil}, - {"b", repl2}, - {"bb", repl2}, - {"c", repl3}, - {"cc", repl3}, - {"d", repl3}, - {"dd", repl3}, - {"e", repl3}, - {"ee", repl3}, - {"f", repl5}, - {"\xff\xff", repl5}, + {"", nil, repl2}, + {"a", nil, repl2}, + {"aa", nil, repl3}, + {"b", repl2, repl3}, + {"bb", repl2, repl5}, + {"c", repl3, repl5}, + {"cc", repl3, repl5}, + {"d", repl3, repl5}, + {"dd", repl3, repl5}, + {"e", repl3, repl5}, + {"ee", repl3, nil}, + {"f", repl5, nil}, + {"\xff\xff", repl5, nil}, } { - if repl := b.LookupPrecedingReplica(ctx, roachpb.RKey(tc.key)); repl != tc.expRepl { - t.Errorf("%d: expected replica %v; got %v", i, tc.expRepl, repl) + if got, want := b.LookupPrecedingReplica(ctx, roachpb.RKey(tc.key)), tc.preRepl; got != want { + t.Errorf("%d: expected preceding replica %v; got %v", i, want, got) + } + if got, want := b.LookupNextReplica(ctx, roachpb.RKey(tc.key)), tc.nextRepl; got != want { + t.Errorf("%d: expected next replica %v; got %v", i, want, got) } } } From c87ca2d56c6bb4d0a6168afbf8e8ed118653456f Mon Sep 17 00:00:00 2001 From: Pavel Kalinnikov Date: Mon, 6 Feb 2023 12:44:51 +0000 Subject: [PATCH 9/9] kvserver: test Store.checkpointSpans Release note: none Epic: none --- pkg/kv/kvserver/replica_consistency_test.go | 86 +++++++++++++++++++++ 1 file changed, 86 insertions(+) diff --git a/pkg/kv/kvserver/replica_consistency_test.go b/pkg/kv/kvserver/replica_consistency_test.go index 018b27834689..78626f5ee4b2 100644 --- a/pkg/kv/kvserver/replica_consistency_test.go +++ b/pkg/kv/kvserver/replica_consistency_test.go @@ -79,6 +79,92 @@ func TestReplicaChecksumVersion(t *testing.T) { }) } +func TestStoreCheckpointSpans(t *testing.T) { + defer leaktest.AfterTest(t)() + + s := Store{} + s.mu.replicasByKey = newStoreReplicaBTree() + s.mu.replicaPlaceholders = map[roachpb.RangeID]*ReplicaPlaceholder{} + + makeDesc := func(rangeID roachpb.RangeID, start, end string) roachpb.RangeDescriptor { + desc := roachpb.RangeDescriptor{RangeID: rangeID} + if start != "" { + desc.StartKey = roachpb.RKey(start) + desc.EndKey = roachpb.RKey(end) + } + return desc + } + var descs []roachpb.RangeDescriptor + addReplica := func(rangeID roachpb.RangeID, start, end string) { + desc := makeDesc(rangeID, start, end) + r := &Replica{RangeID: rangeID, startKey: desc.StartKey} + r.mu.state.Desc = &desc + r.isInitialized.Set(desc.IsInitialized()) + require.NoError(t, s.addToReplicasByRangeIDLocked(r)) + if r.IsInitialized() { + require.NoError(t, s.addToReplicasByKeyLocked(r)) + descs = append(descs, desc) + } + } + addPlaceholder := func(rangeID roachpb.RangeID, start, end string) { + require.NoError(t, s.addPlaceholderLocked( + &ReplicaPlaceholder{rangeDesc: makeDesc(rangeID, start, end)}, + )) + } + + addReplica(1, "a", "b") + addReplica(4, "b", "c") + addPlaceholder(5, "c", "d") + addReplica(2, "e", "f") + addReplica(3, "", "") // uninitialized + + want := [][]string{{ + // r1 with keys [a, b). The checkpoint includes range-ID replicated and + // unreplicated keyspace for ranges 1-2 and 4. Range 2 is included because + // it's a neighbour of r1 by range ID. The checkpoint also includes + // replicated user keyspace {a-c} owned by ranges 1 and 4. + "/Local/RangeID/{1\"\"-3\"\"}", + "/Local/RangeID/{4\"\"-5\"\"}", + "/Local/Range\"{a\"-c\"}", + "/Local/Lock/Intent/Local/Range\"{a\"-c\"}", + "/Local/Lock/Intent\"{a\"-c\"}", + "{a-c}", + }, { + // r4 with keys [b, c). The checkpoint includes range-ID replicated and + // unreplicated keyspace for ranges 3-4, 1 and 2. Range 3 is included + // because it's a neighbour of r4 by range ID. The checkpoint also includes + // replicated user keyspace {a-f} owned by ranges 1, 4, and 2. + "/Local/RangeID/{3\"\"-5\"\"}", + "/Local/RangeID/{1\"\"-2\"\"}", + "/Local/RangeID/{2\"\"-3\"\"}", + "/Local/Range\"{a\"-f\"}", + "/Local/Lock/Intent/Local/Range\"{a\"-f\"}", + "/Local/Lock/Intent\"{a\"-f\"}", + "{a-f}", + }, { + // r2 with keys [e, f). The checkpoint includes range-ID replicated and + // unreplicated keyspace for ranges 1-3 and 4. Ranges 1 and 3 are included + // because they are neighbours of r2 by range ID. The checkpoint also + // includes replicated user keyspace {b-f} owned by ranges 4 and 2. + "/Local/RangeID/{1\"\"-4\"\"}", + "/Local/RangeID/{4\"\"-5\"\"}", + "/Local/Range\"{b\"-f\"}", + "/Local/Lock/Intent/Local/Range\"{b\"-f\"}", + "/Local/Lock/Intent\"{b\"-f\"}", + "{b-f}", + }} + + require.Len(t, want, len(descs)) + for i, desc := range descs { + spans := s.checkpointSpans(&desc) + got := make([]string, 0, len(spans)) + for _, s := range spans { + got = append(got, s.String()) + } + require.Equal(t, want[i], got, i) + } +} + func TestGetChecksumNotSuccessfulExitConditions(t *testing.T) { defer leaktest.AfterTest(t)()