sql/schemachanger: implement DROP COLUMN

pkg/ccl/changefeedccl/schemafeed/testdata/add_column

@@ -26,9 +26,10 @@ t 4->5: Unknown
 t 5->6: Unknown
 t 6->7: Unknown
 t 7->8: Unknown
-t 8->9: AddColumnWithBackfill|PrimaryKeyChange
-t 9->10: Unknown
+t 8->9: Unknown
+t 9->10: AddColumnWithBackfill|PrimaryKeyChange
 t 10->11: Unknown
+t 11->12: Unknown
 
 exec
 SET use_declarative_schema_changer=off;
@@ -37,6 +38,6 @@ ALTER TABLE t ADD COLUMN l INT NOT NULL DEFAULT 42;
 
 pop f=1
 ----
-t 11->12: Unknown
 t 12->13: Unknown
-t 13->14: AddColumnWithBackfill
+t 13->14: Unknown
+t 14->15: AddColumnWithBackfill

pkg/ccl/kvccl/kvfollowerreadsccl/testdata/boundedstaleness/single_row

@@ -185,7 +185,7 @@ query idx=2 wait-until-follower-read
 SELECT * FROM t AS OF SYSTEM TIME with_max_staleness('10s', true) WHERE pk = 1
 ----
 1
-events (15 found):
+events (17 found):
  * event 1: colbatchscan trace on node_idx 2: local read
  * event 2: transaction retry on node_idx: 2
  * event 3: colbatchscan trace on node_idx 2: local read
@@ -200,13 +200,15 @@ events (15 found):
  * event 12: transaction retry on node_idx: 2
  * event 13: colbatchscan trace on node_idx 2: local read
  * event 14: transaction retry on node_idx: 2
- * event 15: colbatchscan trace on node_idx 2: local follower read
+ * event 15: colbatchscan trace on node_idx 2: local read
+ * event 16: transaction retry on node_idx: 2
+ * event 17: colbatchscan trace on node_idx 2: local follower read
 
 query idx=2
 SELECT * FROM t AS OF SYSTEM TIME with_min_timestamp(now() - '10s', true) WHERE pk = 1
 ----
 1
-events (15 found):
+events (17 found):
  * event 1: colbatchscan trace on node_idx 2: local read
  * event 2: transaction retry on node_idx: 2
  * event 3: colbatchscan trace on node_idx 2: local read
@@ -221,7 +223,9 @@ events (15 found):
  * event 12: transaction retry on node_idx: 2
  * event 13: colbatchscan trace on node_idx 2: local read
  * event 14: transaction retry on node_idx: 2
- * event 15: colbatchscan trace on node_idx 2: local follower read
+ * event 15: colbatchscan trace on node_idx 2: local read
+ * event 16: transaction retry on node_idx: 2
+ * event 17: colbatchscan trace on node_idx 2: local follower read
 
 # When creating a new table, ensure when nearest_only=True, we correctly error
 # with the schema not existing if none of the followers have caught up.

pkg/ccl/schemachangerccl/testdata/end_to_end/create_index

@@ -146,7 +146,7 @@ notified job registry to adopt jobs: [1]
 begin transaction #2
 commit transaction #2
 begin transaction #3
-## PostCommitPhase stage 1 of 6 with 3 MutationType ops
+## PostCommitPhase stage 1 of 7 with 3 MutationType ops
 upsert descriptor #104
   ...
      formatVersion: 3
@@ -168,14 +168,14 @@ upsert descriptor #104
      unexposedParentSchemaId: 101
   -  version: "2"
   +  version: "3"
-update progress of schema change job #1: "PostCommitPhase stage 2 of 6 with 1 BackfillType op pending"
+update progress of schema change job #1: "PostCommitPhase stage 2 of 7 with 1 BackfillType op pending"
 commit transaction #3
 begin transaction #4
-## PostCommitPhase stage 2 of 6 with 1 BackfillType op
+## PostCommitPhase stage 2 of 7 with 1 BackfillType op
 backfill indexes [2] from index #1 in table #104
 commit transaction #4
 begin transaction #5
-## PostCommitPhase stage 3 of 6 with 3 MutationType ops
+## PostCommitPhase stage 3 of 7 with 3 MutationType ops
 upsert descriptor #104
   ...
      formatVersion: 3
@@ -197,10 +197,10 @@ upsert descriptor #104
      unexposedParentSchemaId: 101
   -  version: "3"
   +  version: "4"
-update progress of schema change job #1: "PostCommitPhase stage 4 of 6 with 1 MutationType op pending"
+update progress of schema change job #1: "PostCommitPhase stage 4 of 7 with 1 MutationType op pending"
 commit transaction #5
 begin transaction #6
-## PostCommitPhase stage 4 of 6 with 3 MutationType ops
+## PostCommitPhase stage 4 of 7 with 3 MutationType ops
 upsert descriptor #104
   ...
      formatVersion: 3
@@ -214,25 +214,50 @@ upsert descriptor #104
          version: 4
        mutationId: 1
   -    state: DELETE_ONLY
-  +    state: DELETE_AND_WRITE_ONLY
+  +    state: MERGING
      - direction: ADD
        index:
   ...
        time: {}
      unexposedParentSchemaId: 101
   -  version: "4"
   +  version: "5"
-update progress of schema change job #1: "PostCommitPhase stage 5 of 6 with 1 BackfillType op pending"
+update progress of schema change job #1: "PostCommitPhase stage 5 of 7 with 1 BackfillType op pending"
 commit transaction #6
 begin transaction #7
-## PostCommitPhase stage 5 of 6 with 1 BackfillType op
+## PostCommitPhase stage 5 of 7 with 1 BackfillType op
 merge temporary indexes [3] into backfilled indexes [2] in table #104
 commit transaction #7
 begin transaction #8
-## PostCommitPhase stage 6 of 6 with 1 ValidationType op
-validate forward indexes [2] in table #104
+## PostCommitPhase stage 6 of 7 with 3 MutationType ops
+upsert descriptor #104
+  ...
+     formatVersion: 3
+     id: 104
+  -  modificationTime:
+  -    wallTime: "1640995200000000006"
+  +  modificationTime: {}
+     mutations:
+     - direction: ADD
+  ...
+         version: 4
+       mutationId: 1
+  -    state: MERGING
+  +    state: DELETE_AND_WRITE_ONLY
+     - direction: ADD
+       index:
+  ...
+       time: {}
+     unexposedParentSchemaId: 101
+  -  version: "5"
+  +  version: "6"
+update progress of schema change job #1: "PostCommitPhase stage 7 of 7 with 1 ValidationType op pending"
 commit transaction #8
 begin transaction #9
+## PostCommitPhase stage 7 of 7 with 1 ValidationType op
+validate forward indexes [2] in table #104
+commit transaction #9
+begin transaction #10
 ## PostCommitNonRevertiblePhase stage 1 of 2 with 5 MutationType ops
 upsert descriptor #104
   ...
@@ -245,7 +270,7 @@ upsert descriptor #104
      formatVersion: 3
      id: 104
   -  modificationTime:
-  -    wallTime: "1640995200000000006"
+  -    wallTime: "1640995200000000008"
   +  indexes:
   +  - constraintId: 2
   +    createdExplicitly: true
@@ -326,12 +351,12 @@ upsert descriptor #104
   ...
        time: {}
      unexposedParentSchemaId: 101
-  -  version: "5"
-  +  version: "6"
+  -  version: "6"
+  +  version: "7"
 update progress of schema change job #1: "PostCommitNonRevertiblePhase stage 2 of 2 with 2 MutationType ops pending"
 set schema change job #1 to non-cancellable
-commit transaction #9
-begin transaction #10
+commit transaction #10
+begin transaction #11
 ## PostCommitNonRevertiblePhase stage 2 of 2 with 4 MutationType ops
 upsert descriptor #104
   ...
@@ -358,7 +383,7 @@ upsert descriptor #104
        - money
        version: 4
   -  modificationTime:
-  -    wallTime: "1640995200000000009"
+  -    wallTime: "1640995200000000010"
   -  mutations:
   -  - direction: DROP
   -    index:
@@ -401,12 +426,12 @@ upsert descriptor #104
   ...
        time: {}
      unexposedParentSchemaId: 101
-  -  version: "6"
-  +  version: "7"
+  -  version: "7"
+  +  version: "8"
 write *eventpb.FinishSchemaChange to event log for descriptor 104
 create job #2 (non-cancelable: true): "GC for "
   descriptor IDs: [104]
 update progress of schema change job #1: "all stages completed"
-commit transaction #10
+commit transaction #11
 notified job registry to adopt jobs: [2]
 # end PostCommitPhase

pkg/ccl/schemachangerccl/testdata/explain/create_index

@@ -39,40 +39,47 @@ Schema change plan for CREATE INDEX ‹id1› ON ‹defaultdb›.‹public›.
  │              ├── SetJobStateOnDescriptor {"DescriptorID":104,"Initialize":true}
  │              └── CreateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."}
  ├── PostCommitPhase
- │    ├── Stage 1 of 6 in PostCommitPhase
+ │    ├── Stage 1 of 7 in PostCommitPhase
  │    │    ├── 1 element transitioning toward TRANSIENT_ABSENT
  │    │    │    └── DELETE_ONLY → WRITE_ONLY TemporaryIndex:{DescID: 104, IndexID: 3, SourceIndexID: 1}
  │    │    └── 3 Mutation operations
  │    │         ├── MakeAddedIndexDeleteAndWriteOnly {"IndexID":3,"TableID":104}
  │    │         ├── SetJobStateOnDescriptor {"DescriptorID":104}
  │    │         └── UpdateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."}
- │    ├── Stage 2 of 6 in PostCommitPhase
+ │    ├── Stage 2 of 7 in PostCommitPhase
  │    │    ├── 1 element transitioning toward PUBLIC
  │    │    │    └── BACKFILL_ONLY → BACKFILLED SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
  │    │    └── 1 Backfill operation
  │    │         └── BackfillIndex {"IndexID":2,"SourceIndexID":1,"TableID":104}
- │    ├── Stage 3 of 6 in PostCommitPhase
+ │    ├── Stage 3 of 7 in PostCommitPhase
  │    │    ├── 1 element transitioning toward PUBLIC
  │    │    │    └── BACKFILLED → DELETE_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
  │    │    └── 3 Mutation operations
  │    │         ├── MakeBackfillingIndexDeleteOnly {"IndexID":2,"TableID":104}
  │    │         ├── SetJobStateOnDescriptor {"DescriptorID":104}
  │    │         └── UpdateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."}
- │    ├── Stage 4 of 6 in PostCommitPhase
+ │    ├── Stage 4 of 7 in PostCommitPhase
  │    │    ├── 1 element transitioning toward PUBLIC
  │    │    │    └── DELETE_ONLY → MERGE_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
  │    │    └── 3 Mutation operations
- │    │         ├── MakeAddedIndexDeleteAndWriteOnly {"IndexID":2,"TableID":104}
+ │    │         ├── MakeBackfilledIndexMerging {"IndexID":2,"TableID":104}
  │    │         ├── SetJobStateOnDescriptor {"DescriptorID":104}
  │    │         └── UpdateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."}
- │    ├── Stage 5 of 6 in PostCommitPhase
+ │    ├── Stage 5 of 7 in PostCommitPhase
  │    │    ├── 1 element transitioning toward PUBLIC
  │    │    │    └── MERGE_ONLY → MERGED SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
  │    │    └── 1 Backfill operation
  │    │         └── MergeIndex {"BackfilledIndexID":2,"TableID":104,"TemporaryIndexID":3}
- │    └── Stage 6 of 6 in PostCommitPhase
+ │    ├── Stage 6 of 7 in PostCommitPhase
+ │    │    ├── 1 element transitioning toward PUBLIC
+ │    │    │    └── MERGED → WRITE_ONLY SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
+ │    │    └── 3 Mutation operations
+ │    │         ├── MakeMergedIndexWriteOnly {"IndexID":2,"TableID":104}
+ │    │         ├── SetJobStateOnDescriptor {"DescriptorID":104}
+ │    │         └── UpdateSchemaChangerJob {"RunningStatus":"PostCommitPhase ..."}
+ │    └── Stage 7 of 7 in PostCommitPhase
  │         ├── 1 element transitioning toward PUBLIC
- │         │    └── MERGED → VALIDATED SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
+ │         │    └── WRITE_ONLY → VALIDATED SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
  │         └── 1 Validation operation
  │              └── ValidateUniqueIndex {"IndexID":2,"TableID":104}
  └── PostCommitNonRevertiblePhase

pkg/ccl/schemachangerccl/testdata/explain_verbose/create_index

@@ -172,7 +172,7 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │                 DescriptorIDs:
 │                 - 104
 │                 JobID: 1
-│                 RunningStatus: PostCommitPhase stage 1 of 6 with 1 MutationType op pending
+│                 RunningStatus: PostCommitPhase stage 1 of 7 with 1 MutationType op pending
 │                 Statements:
 │                 - statement: CREATE INDEX id1 ON defaultdb.t1 (id, name) STORING (money) PARTITION
 │                     BY LIST (id) (PARTITION p1 VALUES IN (1))
@@ -182,7 +182,7 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │
 ├── • PostCommitPhase
 │   │
-│   ├── • Stage 1 of 6 in PostCommitPhase
+│   ├── • Stage 1 of 7 in PostCommitPhase
 │   │   │
 │   │   ├── • 1 element transitioning toward TRANSIENT_ABSENT
 │   │   │   │
@@ -209,9 +209,9 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │   │       │
 │   │       └── • UpdateSchemaChangerJob
 │   │             JobID: 1
-│   │             RunningStatus: PostCommitPhase stage 2 of 6 with 1 BackfillType op pending
+│   │             RunningStatus: PostCommitPhase stage 2 of 7 with 1 BackfillType op pending
 │   │
-│   ├── • Stage 2 of 6 in PostCommitPhase
+│   ├── • Stage 2 of 7 in PostCommitPhase
 │   │   │
 │   │   ├── • 1 element transitioning toward PUBLIC
 │   │   │   │
@@ -237,7 +237,7 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │   │             SourceIndexID: 1
 │   │             TableID: 104
 │   │
-│   ├── • Stage 3 of 6 in PostCommitPhase
+│   ├── • Stage 3 of 7 in PostCommitPhase
 │   │   │
 │   │   ├── • 1 element transitioning toward PUBLIC
 │   │   │   │
@@ -255,9 +255,9 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │   │       │
 │   │       └── • UpdateSchemaChangerJob
 │   │             JobID: 1
-│   │             RunningStatus: PostCommitPhase stage 4 of 6 with 1 MutationType op pending
+│   │             RunningStatus: PostCommitPhase stage 4 of 7 with 1 MutationType op pending
 │   │
-│   ├── • Stage 4 of 6 in PostCommitPhase
+│   ├── • Stage 4 of 7 in PostCommitPhase
 │   │   │
 │   │   ├── • 1 element transitioning toward PUBLIC
 │   │   │   │
@@ -266,7 +266,7 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │   │   │
 │   │   └── • 3 Mutation operations
 │   │       │
-│   │       ├── • MakeAddedIndexDeleteAndWriteOnly
+│   │       ├── • MakeBackfilledIndexMerging
 │   │       │     IndexID: 2
 │   │       │     TableID: 104
 │   │       │
@@ -275,9 +275,9 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │   │       │
 │   │       └── • UpdateSchemaChangerJob
 │   │             JobID: 1
-│   │             RunningStatus: PostCommitPhase stage 5 of 6 with 1 BackfillType op pending
+│   │             RunningStatus: PostCommitPhase stage 5 of 7 with 1 BackfillType op pending
 │   │
-│   ├── • Stage 5 of 6 in PostCommitPhase
+│   ├── • Stage 5 of 7 in PostCommitPhase
 │   │   │
 │   │   ├── • 1 element transitioning toward PUBLIC
 │   │   │   │
@@ -291,12 +291,32 @@ EXPLAIN (ddl, verbose) CREATE INDEX id1
 │   │             TableID: 104
 │   │             TemporaryIndexID: 3
 │   │
-│   └── • Stage 6 of 6 in PostCommitPhase
+│   ├── • Stage 6 of 7 in PostCommitPhase
+│   │   │
+│   │   ├── • 1 element transitioning toward PUBLIC
+│   │   │   │
+│   │   │   └── • SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
+│   │   │         MERGED → WRITE_ONLY
+│   │   │
+│   │   └── • 3 Mutation operations
+│   │       │
+│   │       ├── • MakeMergedIndexWriteOnly
+│   │       │     IndexID: 2
+│   │       │     TableID: 104
+│   │       │
+│   │       ├── • SetJobStateOnDescriptor
+│   │       │     DescriptorID: 104
+│   │       │
+│   │       └── • UpdateSchemaChangerJob
+│   │             JobID: 1
+│   │             RunningStatus: PostCommitPhase stage 7 of 7 with 1 ValidationType op pending
+│   │
+│   └── • Stage 7 of 7 in PostCommitPhase
 │       │
 │       ├── • 1 element transitioning toward PUBLIC
 │       │   │
 │       │   └── • SecondaryIndex:{DescID: 104, IndexID: 2, ConstraintID: 0, TemporaryIndexID: 3, SourceIndexID: 1}
-│       │         MERGED → VALIDATED
+│       │         WRITE_ONLY → VALIDATED
 │       │
 │       └── • 1 Validation operation
 │           │

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

@@ -12,6 +12,7 @@ package scmutationexec
 
 import (
 	"context"
+	"sort"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
@@ -118,6 +119,18 @@ func (m *visitor) MakeColumnPublic(ctx context.Context, op scop.MakeColumnPublic
 	// that okay?
 	tbl.Columns = append(tbl.Columns,
 		*(protoutil.Clone(mut.GetColumn())).(*descpb.ColumnDescriptor))
+
+	// Ensure that the column is added in the right location. This is important
+	// when rolling back dropped columns.
+	getID := func(col *descpb.ColumnDescriptor) int {
+		if col.PGAttributeNum != 0 {
+			return int(col.PGAttributeNum)
+		}
+		return int(col.ID)
+	}
+	sort.Slice(tbl.Columns, func(i, j int) bool {
+		return getID(&tbl.Columns[i]) < getID(&tbl.Columns[j])
+	})
 	return nil
 }