sql/schemachanger: fix unusable secondary indexes during pk swap

Previously, the declarative schema changer during ALTER PRIMARY KEY, the
declarative schema changer would make the new primary index public and
then work on creating replacement secondary indexes. This could lead to
broken behaviour where you could be left with either unusable old
secondary indexes (causing trouble for DML queries) or potential
validation errors. To address this, this patch adjusts the rules so that
the new primary index is kept at a validated state, while replacement
secondary indexes are constructed. This guarantees that all secondary
indexes are usable during a PK swap, and the new primary key and
replacement secondary indexes are swapped in an atomic fashion.

Fixes: #133129
Fixes: #130165

Release note (bug fix): Addressed a bug where secondary indexes could be
unusable by DML while a primary key swap was occurring, if the new
primary key did not contain columns from the old primary key.

Author: fqazi

pkg/ccl/schemachangerccl/backup_base_generated_test.go

@@ -203,7 +203,7 @@ func init() {
 func init() {
 
 	registerDepRule(
-		"primary index with new columns should exist before secondary indexes",
+		"primary index with new columns should validated before secondary indexes",
 		scgraph.Precedence,
 		"primary-index", "secondary-index",
 		func(from, to NodeVars) rel.Clauses {
@@ -216,7 +216,27 @@ func init() {
 					to, screl.SourceIndexID,
 					"primary-index-id",
 				),
-				StatusesToPublicOrTransient(from, scpb.Status_PUBLIC, to, scpb.Status_BACKFILL_ONLY),
+				StatusesToPublicOrTransient(from, scpb.Status_VALIDATED, to, scpb.Status_BACKFILL_ONLY),
+			}
+		})
+
+	// This rule guarantees that the primary index can only go public, once the
+	// secondary index is ready to go public.
+	registerDepRule(
+		"secondary indexes should be in a validated state before primary indexes can go public",
+		scgraph.Precedence,
+		"secondary-index", "primary-index",
+		func(from, to NodeVars) rel.Clauses {
+			return rel.Clauses{
+				from.Type((*scpb.SecondaryIndex)(nil)),
+				to.Type((*scpb.PrimaryIndex)(nil)),
+				JoinOnDescID(from, to, "table-id"),
+				JoinOn(
+					from, screl.SourceIndexID,
+					to, screl.IndexID,
+					"primary-index-id",
+				),
+				StatusesToPublicOrTransient(from, scpb.Status_VALIDATED, to, scpb.Status_PUBLIC),
 			}
 		})
 
@@ -234,7 +254,7 @@ func init() {
 					to, screl.SourceIndexID,
 					"primary-index-id",
 				),
-				StatusesToPublicOrTransient(from, scpb.Status_PUBLIC, to, scpb.Status_DELETE_ONLY),
+				StatusesToPublicOrTransient(from, scpb.Status_VALIDATED, to, scpb.Status_DELETE_ONLY),
 			}
 		})
 }

pkg/sql/schemachanger/scplan/internal/rules/current/dep_add_index.go

@@ -4031,21 +4031,6 @@ deprules
     - $old-index-Node[CurrentStatus] = PUBLIC
     - joinTargetNode($new-index, $new-index-Target, $new-index-Node)
     - joinTargetNode($old-index, $old-index-Target, $old-index-Node)
-- name: primary index with new columns should exist before secondary indexes
-  from: primary-index-Node
-  kind: Precedence
-  to: secondary-index-Node
-  query:
-    - $primary-index[Type] = '*scpb.PrimaryIndex'
-    - $secondary-index[Type] = '*scpb.SecondaryIndex'
-    - joinOnDescID($primary-index, $secondary-index, $table-id)
-    - $primary-index[IndexID] = $primary-index-id
-    - $secondary-index[SourceIndexID] = $primary-index-id
-    - ToPublicOrTransient($primary-index-Target, $secondary-index-Target)
-    - $primary-index-Node[CurrentStatus] = PUBLIC
-    - $secondary-index-Node[CurrentStatus] = BACKFILL_ONLY
-    - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
-    - joinTargetNode($secondary-index, $secondary-index-Target, $secondary-index-Node)
 - name: primary index with new columns should exist before temp indexes
   from: primary-index-Node
   kind: Precedence
@@ -4057,10 +4042,25 @@ deprules
     - $primary-index[IndexID] = $primary-index-id
     - $temp-index[SourceIndexID] = $primary-index-id
     - ToPublicOrTransient($primary-index-Target, $temp-index-Target)
-    - $primary-index-Node[CurrentStatus] = PUBLIC
+    - $primary-index-Node[CurrentStatus] = VALIDATED
     - $temp-index-Node[CurrentStatus] = DELETE_ONLY
     - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
     - joinTargetNode($temp-index, $temp-index-Target, $temp-index-Node)
+- name: primary index with new columns should validated before secondary indexes
+  from: primary-index-Node
+  kind: Precedence
+  to: secondary-index-Node
+  query:
+    - $primary-index[Type] = '*scpb.PrimaryIndex'
+    - $secondary-index[Type] = '*scpb.SecondaryIndex'
+    - joinOnDescID($primary-index, $secondary-index, $table-id)
+    - $primary-index[IndexID] = $primary-index-id
+    - $secondary-index[SourceIndexID] = $primary-index-id
+    - ToPublicOrTransient($primary-index-Target, $secondary-index-Target)
+    - $primary-index-Node[CurrentStatus] = VALIDATED
+    - $secondary-index-Node[CurrentStatus] = BACKFILL_ONLY
+    - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
+    - joinTargetNode($secondary-index, $secondary-index-Target, $secondary-index-Node)
 - name: relation dropped before dependent column
   from: descriptor-Node
   kind: Precedence
@@ -4462,6 +4462,21 @@ deprules
     - isIndexKeyColumnKey(*scpb.IndexColumn)($index-column)
     - joinTargetNode($index, $index-Target, $index-Node)
     - joinTargetNode($column, $column-Target, $column-Node)
+- name: secondary indexes should be in a validated state before primary indexes can go public
+  from: secondary-index-Node
+  kind: Precedence
+  to: primary-index-Node
+  query:
+    - $secondary-index[Type] = '*scpb.SecondaryIndex'
+    - $primary-index[Type] = '*scpb.PrimaryIndex'
+    - joinOnDescID($secondary-index, $primary-index, $table-id)
+    - $secondary-index[SourceIndexID] = $primary-index-id
+    - $primary-index[IndexID] = $primary-index-id
+    - ToPublicOrTransient($secondary-index-Target, $primary-index-Target)
+    - $secondary-index-Node[CurrentStatus] = VALIDATED
+    - $primary-index-Node[CurrentStatus] = PUBLIC
+    - joinTargetNode($secondary-index, $secondary-index-Target, $secondary-index-Node)
+    - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
 - name: simple constraint public right before its dependents
   from: simple-constraint-Node
   kind: SameStagePrecedence
@@ -8687,21 +8702,6 @@ deprules
     - $old-index-Node[CurrentStatus] = PUBLIC
     - joinTargetNode($new-index, $new-index-Target, $new-index-Node)
     - joinTargetNode($old-index, $old-index-Target, $old-index-Node)
-- name: primary index with new columns should exist before secondary indexes
-  from: primary-index-Node
-  kind: Precedence
-  to: secondary-index-Node
-  query:
-    - $primary-index[Type] = '*scpb.PrimaryIndex'
-    - $secondary-index[Type] = '*scpb.SecondaryIndex'
-    - joinOnDescID($primary-index, $secondary-index, $table-id)
-    - $primary-index[IndexID] = $primary-index-id
-    - $secondary-index[SourceIndexID] = $primary-index-id
-    - ToPublicOrTransient($primary-index-Target, $secondary-index-Target)
-    - $primary-index-Node[CurrentStatus] = PUBLIC
-    - $secondary-index-Node[CurrentStatus] = BACKFILL_ONLY
-    - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
-    - joinTargetNode($secondary-index, $secondary-index-Target, $secondary-index-Node)
 - name: primary index with new columns should exist before temp indexes
   from: primary-index-Node
   kind: Precedence
@@ -8713,10 +8713,25 @@ deprules
     - $primary-index[IndexID] = $primary-index-id
     - $temp-index[SourceIndexID] = $primary-index-id
     - ToPublicOrTransient($primary-index-Target, $temp-index-Target)
-    - $primary-index-Node[CurrentStatus] = PUBLIC
+    - $primary-index-Node[CurrentStatus] = VALIDATED
     - $temp-index-Node[CurrentStatus] = DELETE_ONLY
     - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
     - joinTargetNode($temp-index, $temp-index-Target, $temp-index-Node)
+- name: primary index with new columns should validated before secondary indexes
+  from: primary-index-Node
+  kind: Precedence
+  to: secondary-index-Node
+  query:
+    - $primary-index[Type] = '*scpb.PrimaryIndex'
+    - $secondary-index[Type] = '*scpb.SecondaryIndex'
+    - joinOnDescID($primary-index, $secondary-index, $table-id)
+    - $primary-index[IndexID] = $primary-index-id
+    - $secondary-index[SourceIndexID] = $primary-index-id
+    - ToPublicOrTransient($primary-index-Target, $secondary-index-Target)
+    - $primary-index-Node[CurrentStatus] = VALIDATED
+    - $secondary-index-Node[CurrentStatus] = BACKFILL_ONLY
+    - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
+    - joinTargetNode($secondary-index, $secondary-index-Target, $secondary-index-Node)
 - name: relation dropped before dependent column
   from: descriptor-Node
   kind: Precedence
@@ -9118,6 +9133,21 @@ deprules
     - isIndexKeyColumnKey(*scpb.IndexColumn)($index-column)
     - joinTargetNode($index, $index-Target, $index-Node)
     - joinTargetNode($column, $column-Target, $column-Node)
+- name: secondary indexes should be in a validated state before primary indexes can go public
+  from: secondary-index-Node
+  kind: Precedence
+  to: primary-index-Node
+  query:
+    - $secondary-index[Type] = '*scpb.SecondaryIndex'
+    - $primary-index[Type] = '*scpb.PrimaryIndex'
+    - joinOnDescID($secondary-index, $primary-index, $table-id)
+    - $secondary-index[SourceIndexID] = $primary-index-id
+    - $primary-index[IndexID] = $primary-index-id
+    - ToPublicOrTransient($secondary-index-Target, $primary-index-Target)
+    - $secondary-index-Node[CurrentStatus] = VALIDATED
+    - $primary-index-Node[CurrentStatus] = PUBLIC
+    - joinTargetNode($secondary-index, $secondary-index-Target, $secondary-index-Node)
+    - joinTargetNode($primary-index, $primary-index-Target, $primary-index-Node)
 - name: simple constraint public right before its dependents
   from: simple-constraint-Node
   kind: SameStagePrecedence