schemachanger: enable adding/dropping path of constraint name

pkg/sql/catalog/descpb/constraint.go

@@ -125,3 +125,22 @@ func (c *ConstraintDetail) GetConstraintName() string {
 	}
 	return ""
 }
+
+// SetConstraintName sets correct constraint name base on the constraint
+// type.
+func (c *ConstraintDetail) SetConstraintName(name string) {
+	switch c.Kind {
+	case ConstraintTypePK:
+		c.Index.Name = name
+	case ConstraintTypeUnique:
+		if c.Index != nil {
+			c.Index.Name = name
+		} else {
+			c.UniqueWithoutIndexConstraint.Name = name
+		}
+	case ConstraintTypeFK:
+		c.FK.Name = name
+	case ConstraintTypeCheck:
+		c.CheckConstraint.Name = name
+	}
+}

pkg/sql/schemachanger/scexec/scmutationexec/index.go

@@ -347,6 +347,37 @@ func (m *visitor) SetIndexName(ctx context.Context, op scop.SetIndexName) error
 	return nil
 }
 
+func (m *visitor) SetConstraintName(ctx context.Context, op scop.SetConstraintName) error {
+	tbl, err := m.checkOutTable(ctx, op.TableID)
+	if err != nil {
+		return err
+	}
+	constraint, err := tbl.FindConstraintWithID(op.ConstraintID)
+	if err != nil {
+		return err
+	}
+	isIndexBackedConstraint := false
+	if constraint.IsPrimaryKey() || constraint.IsUniqueConstraint() {
+		isIndexBackedConstraint = true
+	}
+	if isIndexBackedConstraint {
+		constraintProtoBuf := constraint.IndexDesc()
+		constraintProtoBuf.Name = op.Name
+	} else {
+		constraintProtoBuf := constraint.ConstraintToUpdateDesc()
+		constraintProtoBuf.Name = op.Name
+		switch constraintProtoBuf.ConstraintType {
+		case descpb.ConstraintToUpdate_CHECK, descpb.ConstraintToUpdate_NOT_NULL:
+			constraintProtoBuf.Check.Name = op.Name
+		case descpb.ConstraintToUpdate_FOREIGN_KEY:
+			constraintProtoBuf.ForeignKey.Name = op.Name
+		case descpb.ConstraintToUpdate_UNIQUE_WITHOUT_INDEX:
+			constraintProtoBuf.UniqueWithoutIndexConstraint.Name = op.Name
+		}
+	}
+	return nil
+}
+
 func (m *visitor) AddColumnToIndex(ctx context.Context, op scop.AddColumnToIndex) error {
 	tbl, err := m.checkOutTable(ctx, op.TableID)
 	if err != nil {

pkg/sql/schemachanger/scexec/scmutationexec/references.go

@@ -165,9 +165,8 @@ func (m *visitor) MakePublicCheckConstraintValidated(
 	if err != nil {
 		return err
 	}
-	for i, ck := range tbl.Checks {
+	for _, ck := range tbl.Checks {
 		if ck.ConstraintID == op.ConstraintID {
-			tbl.Checks = append(tbl.Checks[:i], tbl.Checks[i+1:]...)
 			ck.Validity = descpb.ConstraintValidity_Dropping
 			return enqueueDropCheckConstraintMutation(tbl, ck)
 		}

pkg/sql/schemachanger/scop/mutation.go

@@ -440,6 +440,14 @@ type SetIndexName struct {
 	Name    string
 }
 
+// SetConstraintName renames a constraint.
+type SetConstraintName struct {
+	mutationOp
+	TableID      descpb.ID
+	ConstraintID descpb.ConstraintID
+	Name         string
+}
+
 // DeleteDescriptor deletes a descriptor.
 type DeleteDescriptor struct {
 	mutationOp

pkg/sql/schemachanger/scplan/internal/opgen/opgen_constraint_name.go

@@ -11,6 +11,7 @@
 package opgen
 
 import (
+	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scop"
 	"github.com/cockroachdb/cockroach/pkg/sql/schemachanger/scpb"
 )
@@ -20,18 +21,24 @@ func init() {
 		toPublic(
 			scpb.Status_ABSENT,
 			to(scpb.Status_PUBLIC,
-				emit(func(this *scpb.ConstraintName) *scop.NotImplemented {
-					return notImplemented(this)
+				emit(func(this *scpb.ConstraintName) *scop.SetConstraintName {
+					return &scop.SetConstraintName{
+						TableID:      this.TableID,
+						ConstraintID: this.ConstraintID,
+						Name:         this.Name,
+					}
 				}),
 			),
 		),
 		toAbsent(
 			scpb.Status_PUBLIC,
 			to(scpb.Status_ABSENT,
-				// TODO(postamar): remove revertibility constraint when possible
-				revertible(false),
-				emit(func(this *scpb.ConstraintName) *scop.NotImplemented {
-					return notImplemented(this)
+				emit(func(this *scpb.ConstraintName) *scop.SetConstraintName {
+					return &scop.SetConstraintName{
+						TableID:      this.TableID,
+						ConstraintID: this.ConstraintID,
+						Name:         tabledesc.ConstraintNamePlaceholder(this.ConstraintID),
+					}
 				}),
 			),
 		),

pkg/sql/schemachanger/scplan/testdata/drop_index

@@ -369,13 +369,14 @@ DROP INDEX idx2 CASCADE
 ops
 DROP INDEX idx3 CASCADE
 ----
-StatementPhase stage 1 of 1 with 6 MutationType ops
+StatementPhase stage 1 of 1 with 7 MutationType ops
   transitions:
     [[Column:{DescID: 104, ColumnID: 5}, ABSENT], PUBLIC] -> WRITE_ONLY
     [[ColumnName:{DescID: 104, Name: crdb_internal_i_shard_16, ColumnID: 5}, ABSENT], PUBLIC] -> ABSENT
     [[SecondaryIndex:{DescID: 104, IndexID: 6, ConstraintID: 0}, ABSENT], PUBLIC] -> VALIDATED
     [[IndexName:{DescID: 104, Name: idx3, IndexID: 6}, ABSENT], PUBLIC] -> ABSENT
     [[CheckConstraint:{DescID: 104, ConstraintID: 2}, ABSENT], PUBLIC] -> VALIDATED
+    [[ConstraintName:{DescID: 104, Name: check_crdb_internal_i_shard_16, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT
   ops:
     *scop.MakePublicSecondaryIndexWriteOnly
       IndexID: 6
@@ -387,6 +388,10 @@ StatementPhase stage 1 of 1 with 6 MutationType ops
     *scop.MakePublicCheckConstraintValidated
       ConstraintID: 2
       TableID: 104
+    *scop.SetConstraintName
+      ConstraintID: 2
+      Name: crdb_internal_constraint_2_name_placeholder
+      TableID: 104
     *scop.MakePublicColumnWriteOnly
       ColumnID: 5
       TableID: 104
@@ -410,15 +415,12 @@ StatementPhase stage 1 of 1 with 6 MutationType ops
       ColumnID: 5
       Name: crdb_internal_column_5_name_placeholder
       TableID: 104
-PreCommitPhase stage 1 of 1 with 3 MutationType ops
+PreCommitPhase stage 1 of 1 with 2 MutationType ops
   transitions:
     [[IndexColumn:{DescID: 104, ColumnID: 5, IndexID: 6}, ABSENT], PUBLIC] -> ABSENT
     [[IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 6}, ABSENT], PUBLIC] -> ABSENT
     [[IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 6}, ABSENT], PUBLIC] -> ABSENT
-    [[ConstraintName:{DescID: 104, Name: check_crdb_internal_i_shard_16, ConstraintID: 2}, ABSENT], PUBLIC] -> ABSENT
   ops:
-    *scop.NotImplemented
-      ElementType: scpb.ConstraintName
     *scop.SetJobStateOnDescriptor
       DescriptorID: 104
       Initialize: true

pkg/sql/schemachanger/testdata/end_to_end/drop_index_hash_sharded_index

@@ -24,19 +24,20 @@ begin transaction #1
 # begin StatementPhase
 checking for feature: DROP INDEX
 increment telemetry for sql.schema.drop_index
-## StatementPhase stage 1 of 1 with 6 MutationType ops
+## StatementPhase stage 1 of 1 with 7 MutationType ops
 upsert descriptor #104
-   table:
-  -  checks:
-  -  - columnIds:
-  -    - 3
-  -    constraintId: 2
+  ...
+       - 3
+       constraintId: 2
   -    expr: crdb_internal_j_shard_16 IN (0:::INT8, 1:::INT8, 2:::INT8, 3:::INT8, 4:::INT8,
   -      5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8, 10:::INT8, 11:::INT8, 12:::INT8,
   -      13:::INT8, 14:::INT8, 15:::INT8)
-  -    fromHashShardedColumn: true
-  -    name: check_crdb_internal_j_shard_16
-  +  checks: []
+  +    expr: crdb_internal_column_3_name_placeholder IN (0:::INT8, 1:::INT8, 2:::INT8,
+  +      3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8, 10:::INT8,
+  +      11:::INT8, 12:::INT8, 13:::INT8, 14:::INT8, 15:::INT8)
+       fromHashShardedColumn: true
+       name: check_crdb_internal_j_shard_16
+  +    validity: Dropping
      columns:
      - id: 1
   ...
@@ -155,7 +156,7 @@ upsert descriptor #104
   +  version: "9"
 # end StatementPhase
 # begin PreCommitPhase
-## PreCommitPhase stage 1 of 1 with 3 MutationType ops
+## PreCommitPhase stage 1 of 1 with 2 MutationType ops
 upsert descriptor #104
   ...
      createAsOfTime:
@@ -190,6 +191,20 @@ commit transaction #2
 begin transaction #3
 ## PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops
 upsert descriptor #104
+   table:
+  -  checks:
+  -  - columnIds:
+  -    - 3
+  -    constraintId: 2
+  -    expr: crdb_internal_column_3_name_placeholder IN (0:::INT8, 1:::INT8, 2:::INT8,
+  -      3:::INT8, 4:::INT8, 5:::INT8, 6:::INT8, 7:::INT8, 8:::INT8, 9:::INT8, 10:::INT8,
+  -      11:::INT8, 12:::INT8, 13:::INT8, 14:::INT8, 15:::INT8)
+  -    fromHashShardedColumn: true
+  -    name: check_crdb_internal_j_shard_16
+  -    validity: Dropping
+  +  checks: []
+     columns:
+     - id: 1
   ...
          version: 3
        mutationId: 2

pkg/sql/schemachanger/testdata/explain/drop_index_hash_sharded_index

@@ -8,28 +8,28 @@ EXPLAIN (ddl) DROP INDEX idx CASCADE;
 Schema change plan for DROP INDEX ‹defaultdb›.‹public›.‹t›@‹idx› CASCADE; 
  ├── StatementPhase
  │    └── Stage 1 of 1 in StatementPhase
- │         ├── 5 elements transitioning toward ABSENT
+ │         ├── 6 elements transitioning toward ABSENT
  │         │    ├── PUBLIC → WRITE_ONLY Column:{DescID: 104, ColumnID: 3}
  │         │    ├── PUBLIC → ABSENT     ColumnName:{DescID: 104, Name: crdb_internal_j_shard_16, ColumnID: 3}
  │         │    ├── PUBLIC → VALIDATED  SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0}
  │         │    ├── PUBLIC → ABSENT     IndexName:{DescID: 104, Name: idx, IndexID: 2}
- │         │    └── PUBLIC → VALIDATED  CheckConstraint:{DescID: 104, ConstraintID: 2}
- │         └── 6 Mutation operations
+ │         │    ├── PUBLIC → VALIDATED  CheckConstraint:{DescID: 104, ConstraintID: 2}
+ │         │    └── PUBLIC → ABSENT     ConstraintName:{DescID: 104, Name: check_crdb_internal_j_shard_16, ConstraintID: 2}
+ │         └── 7 Mutation operations
  │              ├── MakePublicSecondaryIndexWriteOnly {"IndexID":2,"TableID":104}
  │              ├── SetIndexName {"IndexID":2,"Name":"crdb_internal_in...","TableID":104}
  │              ├── MakePublicCheckConstraintValidated {"ConstraintID":2,"TableID":104}
+ │              ├── SetConstraintName {"ConstraintID":2,"Name":"crdb_internal_co...","TableID":104}
  │              ├── MakePublicColumnWriteOnly {"ColumnID":3,"TableID":104}
  │              ├── LogEvent {"TargetStatus":1}
  │              └── SetColumnName {"ColumnID":3,"Name":"crdb_internal_co...","TableID":104}
  ├── PreCommitPhase
  │    └── Stage 1 of 1 in PreCommitPhase
- │         ├── 4 elements transitioning toward ABSENT
+ │         ├── 3 elements transitioning toward ABSENT
  │         │    ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 2}
  │         │    ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 2, IndexID: 2}
- │         │    ├── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 2}
- │         │    └── PUBLIC → ABSENT ConstraintName:{DescID: 104, Name: check_crdb_internal_j_shard_16, ConstraintID: 2}
- │         └── 3 Mutation operations
- │              ├── NotImplemented {"ElementType":"scpb.ConstraintN..."}
+ │         │    └── PUBLIC → ABSENT IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 2}
+ │         └── 2 Mutation operations
  │              ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true}
  │              └── CreateSchemaChangerJob {"NonCancelable":true,"RunningStatus":"PostCommitNonRev..."}
  └── PostCommitNonRevertiblePhase

pkg/sql/schemachanger/testdata/explain_verbose/drop_index_hash_sharded_index

@@ -11,7 +11,7 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE;
 │   │
 │   └── • Stage 1 of 1 in StatementPhase
 │       │
-│       ├── • 5 elements transitioning toward ABSENT
+│       ├── • 6 elements transitioning toward ABSENT
 │       │   │
 │       │   ├── • Column:{DescID: 104, ColumnID: 3}
 │       │   │   │ PUBLIC → WRITE_ONLY
@@ -40,13 +40,19 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE;
 │       │   │   └── • Precedence dependency from VALIDATED SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0}
 │       │   │         rule: "index no longer public before dependents"
 │       │   │
-│       │   └── • CheckConstraint:{DescID: 104, ConstraintID: 2}
-│       │       │ PUBLIC → VALIDATED
+│       │   ├── • CheckConstraint:{DescID: 104, ConstraintID: 2}
+│       │   │   │ PUBLIC → VALIDATED
+│       │   │   │
+│       │   │   └── • PreviousTransactionPrecedence dependency from PUBLIC CheckConstraint:{DescID: 104, ConstraintID: 2}
+│       │   │         rule: "CheckConstraint transitions to ABSENT uphold 2-version invariant: PUBLIC->VALIDATED"
+│       │   │
+│       │   └── • ConstraintName:{DescID: 104, Name: check_crdb_internal_j_shard_16, ConstraintID: 2}
+│       │       │ PUBLIC → ABSENT
 │       │       │
-│       │       └── • PreviousTransactionPrecedence dependency from PUBLIC CheckConstraint:{DescID: 104, ConstraintID: 2}
-│       │             rule: "CheckConstraint transitions to ABSENT uphold 2-version invariant: PUBLIC->VALIDATED"
+│       │       └── • Precedence dependency from VALIDATED CheckConstraint:{DescID: 104, ConstraintID: 2}
+│       │             rule: "constraint no longer public before dependents"
 │       │
-│       └── • 6 Mutation operations
+│       └── • 7 Mutation operations
 │           │
 │           ├── • MakePublicSecondaryIndexWriteOnly
 │           │     IndexID: 2
@@ -61,6 +67,11 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE;
 │           │     ConstraintID: 2
 │           │     TableID: 104
 │           │
+│           ├── • SetConstraintName
+│           │     ConstraintID: 2
+│           │     Name: crdb_internal_constraint_2_name_placeholder
+│           │     TableID: 104
+│           │
 │           ├── • MakePublicColumnWriteOnly
 │           │     ColumnID: 3
 │           │     TableID: 104
@@ -91,7 +102,7 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE;
 │   │
 │   └── • Stage 1 of 1 in PreCommitPhase
 │       │
-│       ├── • 4 elements transitioning toward ABSENT
+│       ├── • 3 elements transitioning toward ABSENT
 │       │   │
 │       │   ├── • IndexColumn:{DescID: 104, ColumnID: 3, IndexID: 2}
 │       │   │   │ PUBLIC → ABSENT
@@ -114,25 +125,16 @@ EXPLAIN (ddl, verbose) DROP INDEX idx CASCADE;
 │       │   │   └── • skip PUBLIC → ABSENT operations
 │       │   │         rule: "skip index-column removal ops on index removal"
 │       │   │
-│       │   ├── • IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 2}
-│       │   │   │ PUBLIC → ABSENT
-│       │   │   │
-│       │   │   ├── • Precedence dependency from VALIDATED SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0}
-│       │   │   │     rule: "index no longer public before dependents"
-│       │   │   │
-│       │   │   └── • skip PUBLIC → ABSENT operations
-│       │   │         rule: "skip index-column removal ops on index removal"
-│       │   │
-│       │   └── • ConstraintName:{DescID: 104, Name: check_crdb_internal_j_shard_16, ConstraintID: 2}
+│       │   └── • IndexColumn:{DescID: 104, ColumnID: 1, IndexID: 2}
 │       │       │ PUBLIC → ABSENT
 │       │       │
-│       │       └── • Precedence dependency from VALIDATED CheckConstraint:{DescID: 104, ConstraintID: 2}
-│       │             rule: "constraint no longer public before dependents"
+│       │       ├── • Precedence dependency from VALIDATED SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0}
+│       │       │     rule: "index no longer public before dependents"
+│       │       │
+│       │       └── • skip PUBLIC → ABSENT operations
+│       │             rule: "skip index-column removal ops on index removal"
 │       │
-│       └── • 3 Mutation operations
-│           │
-│           ├── • NotImplemented
-│           │     ElementType: scpb.ConstraintName
+│       └── • 2 Mutation operations
 │           │
 │           ├── • SetJobStateOnDescriptor
 │           │     DescriptorID: 104