sql: account for duplicate partition names in subzone span gen

This patch fixes a bug during GenerateSubzoneSpans that does not correctly distinguish between partitions of the same name across indexes. The issue was introduced when we relaxed restrictions on partition name reuse across indexes of a table (#39332). Epic: CRDB-43310 Fixes: #128692 Release note (bug fix): Configuring replication controls on a partition name of an index that is not unique across all indexes will correctly impact only that partition.
cockroachdb · Jan 31, 2025 · c308da0 · c308da0
1 parent c794734
commit c308da0
Show file tree

Hide file tree

Showing 3 changed files with 63 additions and 18 deletions.
diff --git a/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/partitions b/pkg/ccl/spanconfigccl/spanconfigsqltranslatorccl/testdata/partitions
@@ -253,3 +253,26 @@ translate database=db table=part
 /Table/111/2/{51-60}                       range default
 /Table/111/2/6{0-1}                        num_replicas=11
 /Table/11{1/2/61-2}                        range default
+
+exec-sql
+CREATE TABLE db.foo(i INT PRIMARY KEY, j INT) PARTITION BY LIST (i) (
+  PARTITION p VALUES IN (1, 5)
+);
+CREATE INDEX idx ON db.foo(j) PARTITION BY LIST (j) (
+  PARTITION p VALUES IN (2, 4)
+);
+ALTER PARTITION p OF INDEX db.foo@foo_pkey CONFIGURE ZONE USING gc.ttlseconds = 2;
+ALTER PARTITION p OF INDEX db.foo@idx CONFIGURE ZONE USING gc.ttlseconds = 5;
+----
+
+translate database=db table=foo
+----
+/Table/112{-/1/1}                          range default
+/Table/112/1/{1-2}                         ttl_seconds=2
+/Table/112/1/{2-5}                         range default
+/Table/112/1/{5-6}                         ttl_seconds=2
+/Table/112/{1/6-2/2}                       range default
+/Table/112/2/{2-3}                         ttl_seconds=5
+/Table/112/2/{3-4}                         range default
+/Table/112/2/{4-5}                         ttl_seconds=5
+/Table/11{2/2/5-3}                         range default
diff --git a/pkg/sql/partition_utils.go b/pkg/sql/partition_utils.go
@@ -18,6 +18,13 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 )
 
+// partitionKey is used to group a partition's name and its index ID for
+// indexing into a map.
+type partitionKey struct {
+	indexID descpb.IndexID
+	name    string
+}
+
 // GenerateSubzoneSpans constructs from a TableDescriptor the entries mapping
 // zone config spans to subzones for use in the SubzoneSpans field of
 // zonepb.ZoneConfig. SubzoneSpans controls which splits are created, so only
@@ -76,10 +83,11 @@ func GenerateSubzoneSpans(
 	a := &tree.DatumAlloc{}
 
 	subzoneIndexByIndexID := make(map[descpb.IndexID]int32)
-	subzoneIndexByPartition := make(map[string]int32)
+	subzoneIndexByPartition := make(map[partitionKey]int32)
 	for i, subzone := range subzones {
 		if len(subzone.PartitionName) > 0 {
-			subzoneIndexByPartition[subzone.PartitionName] = int32(i)
+			partKey := partitionKey{indexID: descpb.IndexID(subzone.IndexID), name: subzone.PartitionName}
+			subzoneIndexByPartition[partKey] = int32(i)
 		} else {
 			subzoneIndexByIndexID[descpb.IndexID(subzone.IndexID)] = int32(i)
 		}
@@ -140,7 +148,8 @@ func GenerateSubzoneSpans(
 		}
 		var ok bool
 		if subzone := payloads[0].(zonepb.Subzone); len(subzone.PartitionName) > 0 {
-			subzoneSpan.SubzoneIndex, ok = subzoneIndexByPartition[subzone.PartitionName]
+			partKey := partitionKey{indexID: descpb.IndexID(subzone.IndexID), name: subzone.PartitionName}
+			subzoneSpan.SubzoneIndex, ok = subzoneIndexByPartition[partKey]
 		} else {
 			subzoneSpan.SubzoneIndex, ok = subzoneIndexByIndexID[descpb.IndexID(subzone.IndexID)]
 		}
@@ -165,7 +174,7 @@ func indexCoveringsForPartitioning(
 	tableDesc catalog.TableDescriptor,
 	idx catalog.Index,
 	part catalog.Partitioning,
-	relevantPartitions map[string]int32,
+	relevantPartitions map[partitionKey]int32,
 	prefixDatums []tree.Datum,
 ) ([]covering.Covering, error) {
 	if part.NumColumns() == 0 {
@@ -191,10 +200,11 @@ func indexCoveringsForPartitioning(
 				if err != nil {
 					return err
 				}
-				if _, ok := relevantPartitions[name]; ok {
+				partKey := partitionKey{indexID: idx.GetID(), name: name}
+				if _, ok := relevantPartitions[partKey]; ok {
 					listCoverings[len(t.Datums)] = append(listCoverings[len(t.Datums)], covering.Range{
 						Start: keyPrefix, End: roachpb.Key(keyPrefix).PrefixEnd(),
-						Payload: zonepb.Subzone{PartitionName: name},
+						Payload: zonepb.Subzone{IndexID: uint32(idx.GetID()), PartitionName: name},
 					})
 				}
 				newPrefixDatums := append(prefixDatums, t.Datums...)
@@ -219,7 +229,8 @@ func indexCoveringsForPartitioning(
 
 	if part.NumRanges() > 0 {
 		err := part.ForEachRange(func(name string, from, to []byte) error {
-			if _, ok := relevantPartitions[name]; !ok {
+			partKey := partitionKey{indexID: idx.GetID(), name: name}
+			if _, ok := relevantPartitions[partKey]; !ok {
 				return nil
 			}
 			_, fromKey, err := rowenc.DecodePartitionTuple(
@@ -232,10 +243,10 @@ func indexCoveringsForPartitioning(
 			if err != nil {
 				return err
 			}
-			if _, ok := relevantPartitions[name]; ok {
+			if _, ok := relevantPartitions[partKey]; ok {
 				coverings = append(coverings, covering.Covering{{
 					Start: fromKey, End: toKey,
-					Payload: zonepb.Subzone{PartitionName: name},
+					Payload: zonepb.Subzone{IndexID: uint32(idx.GetID()), PartitionName: name},
 				}})
 			}
 			return nil

diff --git a/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go b/pkg/sql/schemachanger/scbuild/internal/scbuildstmt/zone_config_helpers.go
@@ -710,6 +710,13 @@ func accumulateNewUniqueConstraints(currentZone, newZone *zonepb.ZoneConfig) []z
 	return retConstraints
 }
 
+// partitionKey is used to group a partition's name and its index ID for
+// indexing into a map.
+type partitionKey struct {
+	indexID descpb.IndexID
+	name    string
+}
+
 // generateSubzoneSpans constructs from a TableID the entries mapping
 // zone config spans to subzones for use in the SubzoneSpans field of
 // zonepb.ZoneConfig. SubzoneSpans controls which splits are created, so only
@@ -764,10 +771,11 @@ func generateSubzoneSpans(
 	}
 
 	subzoneIndexByIndexID := make(map[descpb.IndexID]int32)
-	subzoneIndexByPartition := make(map[string]int32)
+	subzoneIndexByPartition := make(map[partitionKey]int32)
 	for i, subzone := range subzones {
 		if len(subzone.PartitionName) > 0 {
-			subzoneIndexByPartition[subzone.PartitionName] = int32(i)
+			partKey := partitionKey{indexID: descpb.IndexID(subzone.IndexID), name: subzone.PartitionName}
+			subzoneIndexByPartition[partKey] = int32(i)
 		} else {
 			subzoneIndexByIndexID[descpb.IndexID(subzone.IndexID)] = int32(i)
 		}
@@ -827,7 +835,8 @@ func generateSubzoneSpans(
 		}
 		var ok bool
 		if subzone := payloads[0].(zonepb.Subzone); len(subzone.PartitionName) > 0 {
-			subzoneSpan.SubzoneIndex, ok = subzoneIndexByPartition[subzone.PartitionName]
+			partKey := partitionKey{indexID: descpb.IndexID(subzone.IndexID), name: subzone.PartitionName}
+			subzoneSpan.SubzoneIndex, ok = subzoneIndexByPartition[partKey]
 		} else {
 			subzoneSpan.SubzoneIndex, ok = subzoneIndexByIndexID[descpb.IndexID(subzone.IndexID)]
 		}
@@ -853,7 +862,7 @@ func indexCoveringsForPartitioning(
 	indexID catid.IndexID,
 	index []*scpb.IndexColumn,
 	part catalog.Partitioning,
-	relevantPartitions map[string]int32,
+	relevantPartitions map[partitionKey]int32,
 	prefixDatums []tree.Datum,
 ) ([]covering.Covering, error) {
 	if part.NumColumns() == 0 {
@@ -879,10 +888,11 @@ func indexCoveringsForPartitioning(
 				if err != nil {
 					return err
 				}
-				if _, ok := relevantPartitions[name]; ok {
+				partKey := partitionKey{indexID: indexID, name: name}
+				if _, ok := relevantPartitions[partKey]; ok {
 					listCoverings[len(t.Datums)] = append(listCoverings[len(t.Datums)], covering.Range{
 						Start: keyPrefix, End: roachpb.Key(keyPrefix).PrefixEnd(),
-						Payload: zonepb.Subzone{PartitionName: name},
+						Payload: zonepb.Subzone{IndexID: uint32(indexID), PartitionName: name},
 					})
 				}
 				newPrefixDatums := append(prefixDatums, t.Datums...)
@@ -907,7 +917,8 @@ func indexCoveringsForPartitioning(
 
 	if part.NumRanges() > 0 {
 		err := part.ForEachRange(func(name string, from, to []byte) error {
-			if _, ok := relevantPartitions[name]; !ok {
+			partKey := partitionKey{indexID: indexID, name: name}
+			if _, ok := relevantPartitions[partKey]; !ok {
 				return nil
 			}
 			_, fromKey, err := decodePartitionTuple(
@@ -920,10 +931,10 @@ func indexCoveringsForPartitioning(
 			if err != nil {
 				return err
 			}
-			if _, ok := relevantPartitions[name]; ok {
+			if _, ok := relevantPartitions[partKey]; ok {
 				coverings = append(coverings, covering.Covering{{
 					Start: fromKey, End: toKey,
-					Payload: zonepb.Subzone{PartitionName: name},
+					Payload: zonepb.Subzone{IndexID: uint32(indexID), PartitionName: name},
 				}})
 			}
 			return nil