From 31d8c224eb41fb3f5088aa3ce1688b5e7ffb3a19 Mon Sep 17 00:00:00 2001 From: Xiang Gu Date: Tue, 9 Aug 2022 16:22:40 -0400 Subject: [PATCH] sql/catalog/tabledesc: relaxed validation for virtual col in SUFFIX cols One of the validation rule says that "we don't allow virtual columns to be in the SUFFIX columns of a secondary index", except for one case: `ALTER PRIMARY KYE USING HASH`, where the implicitly created virtual, shard column, will need to appear in the SUFFIX columns of the implicitly created unique, secondary index of the old PK key columns ( which btw is a CRDB unique feature). The validation has logic to exempt us from this special case but it's written specifically to the legacy schema changer. Namely, it uses the canonical `PrimaryKeySwap` mutation type as the signal but we don't have that in the declarative schema changer. This PR addresses this issue and allows the validation logic to also exempt the exact same case but in the declarative schema changer. Release note: None --- pkg/sql/catalog/tabledesc/validate.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pkg/sql/catalog/tabledesc/validate.go b/pkg/sql/catalog/tabledesc/validate.go index b12c084929bd..3e2ef6e64623 100644 --- a/pkg/sql/catalog/tabledesc/validate.go +++ b/pkg/sql/catalog/tabledesc/validate.go @@ -1289,6 +1289,20 @@ func (desc *wrapper) validateTableIndexes( newPKColIDs.UnionWith(newPK.CollectKeyColumnIDs()) } } + if newPKColIDs.Empty() { + // Sadly, if the `ALTER PRIMARY KEY USING HASH` is from declarative schema changer, + // we won't find the `PrimaryKeySwap` mutation. In that case, we will attempt to + // find a mutation of adding a primary index and allow its key columns to be used + // as SUFFIX columns in other indexes, even if they are virtual. + for _, mut := range desc.Mutations { + if pidx := mut.GetIndex(); pidx != nil && + pidx.EncodingType == descpb.PrimaryIndexEncoding && + mut.Direction == descpb.DescriptorMutation_ADD && + !mut.Rollback { + newPKColIDs.UnionWith(catalog.MakeTableColSet(pidx.KeyColumnIDs...)) + } + } + } for _, colID := range idx.IndexDesc().KeySuffixColumnIDs { if !vea.IsActive(clusterversion.Start22_1) { if col := columnsByID[colID]; col != nil && col.IsVirtual() {