sql: bugfixes around writing the old primary key in pk changes

pkg/sql/alter_table.go

@@ -397,17 +397,15 @@ func (n *alterTableNode) startExec(params runParams) error {
 
 			// TODO (rohany): gate this behind a flag so it doesn't happen all the time.
 			// Create a new index that indexes everything the old primary index does, but doesn't store anything.
-			// TODO (rohany): is there an easier way of checking if the existing primary index was the
-			//  automatically created one?
-			if len(n.tableDesc.PrimaryIndex.ColumnNames) == 1 && n.tableDesc.PrimaryIndex.ColumnNames[0] != "rowid" {
+			if !n.tableDesc.IsPrimaryIndexDefaultRowID() {
 				oldPrimaryIndexCopy := protoutil.Clone(&n.tableDesc.PrimaryIndex).(*sqlbase.IndexDescriptor)
-				name := generateUniqueConstraintName(
-					"old_primary_key",
-					nameExists,
-				)
-				oldPrimaryIndexCopy.Name = name
+				// Clear the name of the index so that it gets generated by AllocateIDs.
+				oldPrimaryIndexCopy.Name = ""
 				oldPrimaryIndexCopy.StoreColumnIDs = nil
 				oldPrimaryIndexCopy.StoreColumnNames = nil
+				// Make the copy of the old primary index not-interleaved. This decision
+				// can be revisited based on user experience.
+				oldPrimaryIndexCopy.Interleave = sqlbase.InterleaveDescriptor{}
 				if err := addIndexMutationWithSpecificPrimaryKey(n.tableDesc, oldPrimaryIndexCopy, newPrimaryIndexDesc); err != nil {
 					return err
 				}

pkg/sql/logictest/testdata/logic_test/alter_primary_key

@@ -97,7 +97,7 @@ child  CREATE TABLE child (
            y INT8 NOT NULL,
            z INT8 NOT NULL,
            CONSTRAINT "primary" PRIMARY KEY (x ASC, y ASC, z ASC),
-           UNIQUE INDEX old_primary_key (x ASC),
+           UNIQUE INDEX child_x_key (x ASC),
            FAMILY fam_0_x_y_z (x, y, z)
        ) INTERLEAVE IN PARENT parent (x, y)
 
@@ -151,6 +151,7 @@ child  CREATE TABLE child (
        y INT8 NOT NULL,
        z INT8 NOT NULL,
        CONSTRAINT "primary" PRIMARY KEY (y ASC, z ASC),
+       UNIQUE INDEX child_x_y_z_key (x ASC, y ASC, z ASC),
        FAMILY fam_0_x_y_z (x, y, z)
 )
 
@@ -194,6 +195,7 @@ child  CREATE TABLE child (
          z INT8 NOT NULL,
          w INT8 NULL,
          CONSTRAINT "primary" PRIMARY KEY (x ASC, y ASC, z ASC),
+         UNIQUE INDEX child_x_y_key (x ASC, y ASC),
          INDEX i (x ASC, w ASC) INTERLEAVE IN PARENT parent (x),
          FAMILY fam_0_x_y_z_w (x, y, z, w)
        ) INTERLEAVE IN PARENT parent (x)
@@ -281,3 +283,20 @@ INSERT INTO t1 VALUES (100, 100, 100, 100)
 
 statement error insert on table "t4" violates foreign key constraint "fk3"
 INSERT INTO t4 VALUES (101)
+
+# Ensure that we still rewrite a primary index if the index column has name "rowid".
+statement ok
+DROP TABLE IF EXISTS t;
+CREATE TABLE t (rowid INT PRIMARY KEY, y INT NOT NULL, FAMILY (rowid, y));
+ALTER TABLE t ALTER PRIMARY KEY USING COLUMNS (y)
+
+query TT
+SHOW CREATE t
+----
+t  CREATE TABLE t (
+   rowid INT8 NOT NULL,
+   y INT8 NOT NULL,
+   CONSTRAINT "primary" PRIMARY KEY (y ASC),
+   UNIQUE INDEX t_rowid_key (rowid ASC),
+   FAMILY fam_0_rowid_y (rowid, y)
+)

pkg/sql/schema_changer_test.go

@@ -2322,7 +2322,7 @@ INSERT INTO t.test VALUES (1, 1, 1), (2, 2, 2), (3, 3, 3);
 	y INT8 NOT NULL,
 	z INT8 NULL,
 	CONSTRAINT "primary" PRIMARY KEY (y ASC),
-	UNIQUE INDEX old_primary_key (x ASC),
+	UNIQUE INDEX test_x_key (x ASC),
 	INDEX i (z ASC),
 	FAMILY "primary" (x, y, z)
 )`

pkg/sql/sqlbase/structured.go

@@ -2808,6 +2808,20 @@ func (desc *TableDescriptor) IsInterleaved() bool {
 	return false
 }
 
+// IsPrimaryIndexDefaultRowID returns whether or not the table's primary
+// index is the default primary key on the hidden rowid column.
+func (desc *TableDescriptor) IsPrimaryIndexDefaultRowID() bool {
+	if len(desc.PrimaryIndex.ColumnIDs) != 1 {
+		return false
+	}
+	col, err := desc.FindColumnByID(desc.PrimaryIndex.ColumnIDs[0])
+	if err != nil {
+		// Should never be in this case.
+		panic(err)
+	}
+	return col.Hidden && col.Name == "rowid"
+}
+
 // MakeMutationComplete updates the descriptor upon completion of a mutation.
 // There are three Validity types for the mutations:
 // Validated   - The constraint has already been added and validated, should