sql,tabledesc: add new IndexDescriptorVersion for primary indexes

Previously, the IndexDescriptorVersion type was only used to describe
the encoding of secondary indexes. This commit adds a new value for
use in primary indexes, PrimaryIndexWithStoredColumnsVersion, to signify
that the StoredColumnIDs and StoredColumnNames slices are populated
correctly.

Previously, these slices did not need to be populated at all. This is
because the set of columns comprising the primary index of a table is
assumed to be all non-virtual columns of that table. Our upcoming work on
the new schema changer will require us to violate that assumption
however. This commit is in preparation of that change.

Release note: None

pkg/ccl/logictestccl/testdata/logic_test/partitioning_implicit

@@ -588,7 +588,7 @@ CREATE TABLE public.t (
   u STRING NULL,
   e INT8 NOT NULL,
   CONSTRAINT "primary" PRIMARY KEY (pk2 ASC),
-  UNIQUE INDEX t_pk_key (pk ASC),
+  UNIQUE INDEX t_pk_key (pk ASC) STORING (pk2, a, b, c, d, j, u, e),
   UNIQUE INDEX t_u_key (u ASC),
   INDEX t_a_idx (a ASC),
   UNIQUE INDEX t_b_key (b ASC),

pkg/ccl/logictestccl/testdata/logic_test/regional_by_row

@@ -1743,7 +1743,7 @@ CREATE TABLE public.regional_by_row_table (
   j JSONB NULL,
   crdb_region public.crdb_internal_region NOT VISIBLE NOT NULL DEFAULT default_to_database_primary_region(gateway_region())::public.crdb_internal_region,
   CONSTRAINT "primary" PRIMARY KEY (pk2 ASC),
-  UNIQUE INDEX regional_by_row_table_pk_key (pk ASC),
+  UNIQUE INDEX regional_by_row_table_pk_key (pk ASC) STORING (pk2, a, b, j),
   INDEX regional_by_row_table_a_idx (a ASC),
   UNIQUE INDEX regional_by_row_table_b_key (b ASC),
   INVERTED INDEX regional_by_row_table_j_idx (j),

pkg/ccl/logictestccl/testdata/logic_test/zone

@@ -1107,7 +1107,7 @@ t  CREATE TABLE public.t (
    z INT8 NULL,
    w INT8 NULL,
    CONSTRAINT "primary" PRIMARY KEY (y ASC),
-   UNIQUE INDEX t_x_key (x ASC),
+   UNIQUE INDEX t_x_key (x ASC) STORING (y, z, w),
    INDEX i1 (z ASC) PARTITION BY LIST (z) (
      PARTITION p1 VALUES IN ((1), (2)),
      PARTITION p2 VALUES IN ((3), (4))

pkg/sql/add_column.go

@@ -164,6 +164,14 @@ func (p *planner) addColumnImpl(
 		col.ComputeExpr = &serializedExpr
 	}
 
+	if !col.Virtual {
+		// Add non-virtual column name and ID to primary index.
+		primaryIndex := n.tableDesc.GetPrimaryIndex().IndexDescDeepCopy()
+		primaryIndex.StoreColumnNames = append(primaryIndex.StoreColumnNames, col.Name)
+		primaryIndex.StoreColumnIDs = append(primaryIndex.StoreColumnIDs, col.ID)
+		n.tableDesc.SetPrimaryIndex(primaryIndex)
+	}
+
 	return nil
 }
 

pkg/sql/alter_column_type.go

@@ -384,6 +384,13 @@ func alterColumnTypeGeneral(
 	}
 
 	tableDesc.AddColumnMutation(&newCol, descpb.DescriptorMutation_ADD)
+	if !newCol.Virtual {
+		// Add non-virtual column name and ID to primary index.
+		primaryIndex := tableDesc.GetPrimaryIndex().IndexDescDeepCopy()
+		primaryIndex.StoreColumnNames = append(primaryIndex.StoreColumnNames, newCol.Name)
+		primaryIndex.StoreColumnIDs = append(primaryIndex.StoreColumnIDs, newCol.ID)
+		tableDesc.SetPrimaryIndex(primaryIndex)
+	}
 
 	if err := tableDesc.AllocateIDs(ctx); err != nil {
 		return err

pkg/sql/alter_index.go

@@ -107,7 +107,7 @@ func (n *alterIndexNode) startExec(params runParams) error {
 					"cannot ALTER INDEX and change the partitioning to contain implicit columns",
 				)
 			}
-			isIndexAltered := tabledesc.UpdateIndexPartitioning(&alteredIndexDesc, newImplicitCols, newPartitioning)
+			isIndexAltered := tabledesc.UpdateIndexPartitioning(&alteredIndexDesc, n.index.Primary(), newImplicitCols, newPartitioning)
 			if isIndexAltered {
 				oldPartitioning := n.index.GetPartitioning().DeepCopy()
 				if n.index.Primary() {

pkg/sql/alter_primary_key.go

@@ -240,6 +240,26 @@ func (p *planner) AlterPrimaryKey(
 	if err := newPrimaryIndexDesc.FillColumns(alterPKNode.Columns); err != nil {
 		return err
 	}
+
+	{
+		// Add all deletable non-virtual non-pk columns to new primary index.
+		names := make(map[string]struct{}, len(newPrimaryIndexDesc.KeyColumnNames))
+		for _, name := range newPrimaryIndexDesc.KeyColumnNames {
+			names[name] = struct{}{}
+		}
+		deletable := tableDesc.DeletableColumns()
+		newPrimaryIndexDesc.StoreColumnNames = make([]string, 0, len(deletable))
+		for _, col := range deletable {
+			if _, found := names[col.GetName()]; found || col.IsVirtual() {
+				continue
+			}
+			newPrimaryIndexDesc.StoreColumnNames = append(newPrimaryIndexDesc.StoreColumnNames, col.GetName())
+		}
+		if len(newPrimaryIndexDesc.StoreColumnNames) == 0 {
+			newPrimaryIndexDesc.StoreColumnNames = nil
+		}
+	}
+
 	if err := tableDesc.AddIndexMutation(newPrimaryIndexDesc, descpb.DescriptorMutation_ADD); err != nil {
 		return err
 	}
@@ -349,21 +369,15 @@ func (p *planner) AlterPrimaryKey(
 		if err != nil {
 			return err
 		}
-		tabledesc.UpdateIndexPartitioning(newPrimaryIndexDesc, newImplicitCols, newPartitioning)
+		tabledesc.UpdateIndexPartitioning(newPrimaryIndexDesc, true /* isIndexPrimary */, newImplicitCols, newPartitioning)
 	}
 
-	// Ensure that the new primary index stores all columns in the table.
-	tabledesc.PopulateAllStoreColumns(newPrimaryIndexDesc, tableDesc)
-	newPrimaryIndexDesc.KeySuffixColumnIDs = nil
-
 	// Create a new index that indexes everything the old primary index
 	// does, but doesn't store anything.
 	if shouldCopyPrimaryKey(tableDesc, newPrimaryIndexDesc, alterPrimaryKeyLocalitySwap) {
 		oldPrimaryIndexCopy := tableDesc.GetPrimaryIndex().IndexDescDeepCopy()
 		// 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 = descpb.InterleaveDescriptor{}
@@ -441,7 +455,7 @@ func (p *planner) AlterPrimaryKey(
 
 		// Drop any PARTITION ALL BY clause.
 		if dropPartitionAllBy {
-			tabledesc.UpdateIndexPartitioning(&newIndex, nil /* newImplicitCols */, descpb.PartitioningDescriptor{})
+			tabledesc.UpdateIndexPartitioning(&newIndex, idx.Primary(), nil /* newImplicitCols */, descpb.PartitioningDescriptor{})
 		}
 
 		// Create partitioning if we are newly adding a PARTITION BY ALL statement.
@@ -459,7 +473,7 @@ func (p *planner) AlterPrimaryKey(
 			if err != nil {
 				return err
 			}
-			tabledesc.UpdateIndexPartitioning(&newIndex, newImplicitCols, newPartitioning)
+			tabledesc.UpdateIndexPartitioning(&newIndex, idx.Primary(), newImplicitCols, newPartitioning)
 		}
 
 		newIndex.Name = tabledesc.GenerateUniqueName(basename, nameExists)

pkg/sql/alter_table.go

@@ -863,7 +863,7 @@ func (n *alterTableNode) startExec(params runParams) error {
 					"cannot ALTER TABLE and change the partitioning to contain implicit columns",
 				)
 			}
-			isIndexAltered := tabledesc.UpdateIndexPartitioning(&newPrimaryIndexDesc, newImplicitCols, newPartitioning)
+			isIndexAltered := tabledesc.UpdateIndexPartitioning(&newPrimaryIndexDesc, true /* isIndexPrimary */, newImplicitCols, newPartitioning)
 			if isIndexAltered {
 				n.tableDesc.SetPrimaryIndex(newPrimaryIndexDesc)
 				descriptorChanged = true

pkg/sql/catalog/descpb/structured.go

@@ -111,6 +111,14 @@ const (
 	// each column ID in the ColumnIDs, StoreColumnIDs and KeySuffixColumnIDs
 	// slices are unique within each slice, and the slices form disjoint sets.
 	StrictIndexColumnIDGuaranteesVersion
+	// PrimaryIndexWithStoredColumnsVersion corresponds to the encoding of
+	// primary indexes that is identical to the unspecified scheme previously in
+	// use (the IndexDescriptorVersion type was originally only used for
+	// secondary indexes) but with the guarantee that the StoreColumnIDs and
+	// StoreColumnNames slices are explicitly populated and maintained. Previously
+	// these were implicitly derived based on the set of non-virtual columns in
+	// the table.
+	PrimaryIndexWithStoredColumnsVersion
 )
 
 // ColumnID is a custom type for ColumnDescriptor IDs.

pkg/sql/catalog/descs/collection_test.go

@@ -107,7 +107,7 @@ func TestCollectionWriteDescToBatch(t *testing.T) {
 			NextFamilyID:   1,
 			NextIndexID:    2,
 			NextMutationID: 1,
-			FormatVersion:  descpb.FamilyFormatVersion,
+			FormatVersion:  descpb.InterleavedFormatVersion,
 		}).BuildCreatedMutableTable()
 		b := txn.NewBatch()
 

pkg/sql/catalog/systemschema/BUILD.bazel

@@ -16,6 +16,7 @@ go_library(
         "//pkg/sql/catalog/tabledesc",
         "//pkg/sql/privilege",
         "//pkg/sql/types",
+        "//pkg/util/log",
         "@com_github_cockroachdb_errors//:errors",
     ],
 )

pkg/sql/catalog/systemschema/system.go

@@ -11,6 +11,7 @@
 package systemschema
 
 import (
+	"context"
 	"math"
 	"time"
 
@@ -24,6 +25,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/tabledesc"
 	"github.com/cockroachdb/cockroach/pkg/sql/privilege"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/errors"
 )
 
@@ -376,7 +378,6 @@ func pk(name string) descpb.IndexDescriptor {
 		KeyColumnNames:      []string{name},
 		KeyColumnDirections: singleASC,
 		KeyColumnIDs:        singleID1,
-		Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 	}
 }
 
@@ -403,7 +404,13 @@ func MakeSystemDatabaseDesc() catalog.DatabaseDescriptor {
 }
 
 func makeTable(desc descpb.TableDescriptor) catalog.TableDescriptor {
-	return tabledesc.NewBuilder(&desc).BuildImmutableTable()
+	ctx := context.Background()
+	b := tabledesc.NewBuilder(&desc)
+	err := b.RunPostDeserializationChanges(ctx, nil /* DescGetter */)
+	if err != nil {
+		log.Fatalf(ctx, "Error when building descriptor of system table %q: %s", desc.Name, err)
+	}
+	return b.BuildImmutableTable()
 }
 
 // These system config descpb.TableDescriptor literals should match the descriptor
@@ -443,7 +450,6 @@ var (
 			KeyColumnNames:      []string{"parentID", "parentSchemaID", "name"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2, 3},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -538,7 +544,6 @@ var (
 			KeyColumnNames:      []string{"id"},
 			KeyColumnDirections: singleASC,
 			KeyColumnIDs:        []descpb.ColumnID{keys.ZonesTablePrimaryIndexID},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextFamilyID: 3,
 		NextIndexID:  2,
@@ -679,7 +684,6 @@ var (
 			KeyColumnNames:      []string{"descID", "version", "expiration", "nodeID"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2, 4, 3},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextFamilyID: 1,
 		NextIndexID:  2,
@@ -724,7 +728,6 @@ var (
 			KeyColumnNames:      []string{"timestamp", "uniqueID"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 6},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextFamilyID: 6,
 		NextIndexID:  2,
@@ -768,7 +771,6 @@ var (
 			KeyColumnNames:      []string{"timestamp", "uniqueID"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 7},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextFamilyID: 7,
 		NextIndexID:  2,
@@ -995,7 +997,6 @@ var (
 			KeyColumnNames:      []string{"tableID", "statisticID"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1036,7 +1037,6 @@ var (
 			KeyColumnNames:      []string{"localityKey", "localityValue"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1081,7 +1081,6 @@ var (
 			KeyColumnNames:      []string{"role", "member"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		Indexes: []descpb.IndexDescriptor{
 			{
@@ -1138,7 +1137,6 @@ var (
 			KeyColumnNames:      []string{"type", "object_id", "sub_id"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2, 3},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: newCommentPrivilegeDescriptor(
@@ -1176,7 +1174,6 @@ var (
 				descpb.IndexDescriptor_ASC,
 			},
 			KeyColumnIDs: []descpb.ColumnID{1},
-			Version:      descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1231,7 +1228,6 @@ var (
 				descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC,
 			},
 			KeyColumnIDs: []descpb.ColumnID{1, 2, 3, 4},
-			Version:      descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1281,7 +1277,6 @@ var (
 				descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC,
 			},
 			KeyColumnIDs: []descpb.ColumnID{1, 2, 3},
-			Version:      descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1334,7 +1329,6 @@ var (
 			KeyColumnNames:      []string{"zone_id", "subzone_id"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1380,7 +1374,6 @@ var (
 		PrimaryIndex: descpb.IndexDescriptor{
 			Name:           "primary",
 			ID:             1,
-			Version:        descpb.StrictIndexColumnIDGuaranteesVersion,
 			Unique:         true,
 			KeyColumnNames: []string{"singleton"},
 			KeyColumnIDs:   []descpb.ColumnID{1},
@@ -1422,7 +1415,6 @@ var (
 		PrimaryIndex: descpb.IndexDescriptor{
 			Name:           "primary",
 			ID:             1,
-			Version:        descpb.StrictIndexColumnIDGuaranteesVersion,
 			Unique:         true,
 			KeyColumnNames: []string{"id"},
 			KeyColumnIDs:   []descpb.ColumnID{1},
@@ -1466,7 +1458,6 @@ var (
 			KeyColumnNames:      []string{"username", "option"},
 			KeyColumnDirections: []descpb.IndexDescriptor_Direction{descpb.IndexDescriptor_ASC, descpb.IndexDescriptor_ASC},
 			KeyColumnIDs:        []descpb.ColumnID{1, 2},
-			Version:             descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1710,7 +1701,6 @@ var (
 				descpb.IndexDescriptor_ASC,
 			},
 			KeyColumnIDs: []descpb.ColumnID{1, 2, 3, 4},
-			Version:      descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(
@@ -1753,7 +1743,6 @@ var (
 				descpb.IndexDescriptor_ASC,
 			},
 			KeyColumnIDs: []descpb.ColumnID{1},
-			Version:      descpb.StrictIndexColumnIDGuaranteesVersion,
 		},
 		NextIndexID: 2,
 		Privileges: descpb.NewCustomSuperuserPrivilegeDescriptor(

pkg/sql/catalog/table_elements.go

@@ -154,12 +154,14 @@ type Index interface {
 
 	CollectKeyColumnIDs() TableColSet
 	CollectKeySuffixColumnIDs() TableColSet
+	CollectPrimaryStoredColumnIDs() TableColSet
 	CollectSecondaryStoredColumnIDs() TableColSet
 	CollectCompositeColumnIDs() TableColSet
 
 	InvertedColumnID() descpb.ColumnID
 	InvertedColumnName() string
 
+	NumPrimaryStoredColumns() int
 	NumSecondaryStoredColumns() int
 	GetStoredColumnID(storedColumnOrdinal int) descpb.ColumnID
 	GetStoredColumnName(storedColumnOrdinal int) string