Merge #156105

156105: sql/opt_catalog: Use index offset as tombstone index ordinal r=mw5h a=mw5h

Previously, when the optimizer stored the ordinal of an index to use for writing tombstones in non-SERIALIZABLE transactions, it would call the Ordinal() method, which returns the value from the table's descriptor. We would later use this value as an input to the Index() method to retrieve the index in question. However, Ordinal() can differ from the catalog's view of the schema during schema change, resulting in sometimes getting the wrong index ordinal from the optimizer's point of view, sometimes resulting in an out of bounds error.

This patch switches the ordinal we save to be the actual index used when creating the optTable, which feels janky, but is at least correct.

Fixes: #151477
Release note (bug fix): A bug which could occasionally cause DML on regional by row tables with unique indexes that don't reference the region to fail under READ-COMMITTED isolation has been corrected.

Co-authored-by: Matt White <[email protected]>

Author: craig[bot]

pkg/sql/opt_catalog.go

@@ -1157,12 +1157,13 @@ func newOptTable(
 				canUseTombstones := idx.ImplicitPartitioningColumnCount() == 1 &&
 					partitionColumn.GetType().Family() == types.EnumFamily
 				ot.uniqueConstraints = append(ot.uniqueConstraints, optUniqueConstraint{
-					name:                  idx.GetName(),
-					table:                 ot.ID(),
-					columns:               idx.IndexDesc().KeyColumnIDs[idx.IndexDesc().ExplicitColumnStartIdx():],
-					withoutIndex:          true,
-					canUseTombstones:      canUseTombstones,
-					tombstoneIndexOrdinal: idx.Ordinal(),
+					name:             idx.GetName(),
+					table:            ot.ID(),
+					columns:          idx.IndexDesc().KeyColumnIDs[idx.IndexDesc().ExplicitColumnStartIdx():],
+					withoutIndex:     true,
+					canUseTombstones: canUseTombstones,
+					// One would assume that this would be idx.Ordinal(), but they can differ during schema change
+					tombstoneIndexOrdinal: i,
 					predicate:             idx.GetPredicate(),
 					// TODO(rytaft): will we ever support an unvalidated unique constraint
 					// here?