From 65c5e375717108f5e100ac1e0b9811c1f8968281 Mon Sep 17 00:00:00 2001
From: Solon Gordon <solon@cockroachlabs.com>
Date: Tue, 10 Sep 2019 12:15:58 -0400
Subject: [PATCH] sql: backward compat for ALTER PARTITION OF TABLE

When we made partition names index-scoped, we removed the ability to
specify secondary index partitions in the ALTER PARTITION ... OF TABLE
command. However I'm concerned this could break some backward
compatibility. I restored that behavior but added logic to throw an
error if there are multiple partitions with the same name.

Fixes #40425

Release note: None
---
 pkg/ccl/logictestccl/testdata/logic_test/zone | 28 +++++++++++++++++--
 pkg/sql/set_zone_config.go                    | 19 +++++++++++++
 pkg/sql/sqlbase/structured.go                 | 12 ++++++++
 3 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/pkg/ccl/logictestccl/testdata/logic_test/zone b/pkg/ccl/logictestccl/testdata/logic_test/zone
index 398c12770ef2..57280d0b6769 100644
--- a/pkg/ccl/logictestccl/testdata/logic_test/zone
+++ b/pkg/ccl/logictestccl/testdata/logic_test/zone
@@ -511,7 +511,7 @@ CREATE INDEX "my index" ON "my table" (x) PARTITION BY LIST (x) (
 ALTER DATABASE "my database" CONFIGURE ZONE USING num_replicas = 1;
 ALTER TABLE "my table" CONFIGURE ZONE USING num_replicas = 1;
 ALTER INDEX "my table"@"my index" CONFIGURE ZONE USING num_replicas = 1;
-ALTER PARTITION "my partition" OF TABLE "my table" CONFIGURE ZONE USING num_replicas = 1;
+ALTER PARTITION "my partition" OF INDEX "my table"@primary CONFIGURE ZONE USING num_replicas = 1;
 ALTER PARTITION "my partition" OF INDEX "my table"@"my index" CONFIGURE ZONE USING num_replicas = 1
 
 query TTTTTT
@@ -601,8 +601,11 @@ CREATE INDEX x1 ON t2 (x) PARTITION BY LIST (x) (
 );
 CREATE INDEX x2 ON t2 (x) PARTITION BY LIST (x) (
   PARTITION p1 VALUES IN (1),
-  PARTITION p2 VALUES IN (2)
-);
+  PARTITION p2 VALUES IN (2),
+  PARTITION p3 VALUES IN (3)
+)
+
+statement ok
 ALTER PARTITION p1 OF INDEX t2@* CONFIGURE ZONE USING num_replicas = 1
 
 query TT
@@ -615,6 +618,25 @@ PARTITION p1 OF INDEX "my database".public.t2@x1       ALTER PARTITION p1 OF IND
 PARTITION p1 OF INDEX "my database".public.t2@x2       ALTER PARTITION p1 OF INDEX "my database".public.t2@x2 CONFIGURE ZONE USING
                                                        num_replicas = 1
 
+# ALTER PARTITION ... OF TABLE should only succeed if the partition name is
+# unique across all indexes.
+statement error pq: partition "p1" exists on multiple indexes of table "t2"
+ALTER PARTITION p1 OF TABLE t2 CONFIGURE ZONE USING num_replicas = 1
+
+statement ok
+ALTER PARTITION p3 OF TABLE t2 CONFIGURE ZONE USING num_replicas = 1
+
+query TT
+SELECT * FROM [SHOW ALL ZONE CONFIGURATIONS] WHERE target LIKE '%t2@x2%'
+----
+PARTITION p1 OF INDEX "my database".public.t2@x2  ALTER PARTITION p1 OF INDEX "my database".public.t2@x2 CONFIGURE ZONE USING
+                                                  num_replicas = 1
+PARTITION p3 OF INDEX "my database".public.t2@x2  ALTER PARTITION p3 OF INDEX "my database".public.t2@x2 CONFIGURE ZONE USING
+                                                  num_replicas = 1
+
+statement error pq: partition "p4" does not exist on table "t2"
+ALTER PARTITION p4 OF TABLE t2 CONFIGURE ZONE USING num_replicas = 1
+
 # regression for #40417
 statement ok
 CREATE TABLE t40417 (x INT PRIMARY KEY)
diff --git a/pkg/sql/set_zone_config.go b/pkg/sql/set_zone_config.go
index 3d937a48decd..64f8064a750a 100644
--- a/pkg/sql/set_zone_config.go
+++ b/pkg/sql/set_zone_config.go
@@ -243,6 +243,25 @@ func (n *setZoneConfigNode) startExec(params runParams) error {
 		return err
 	}
 
+	if n.zoneSpecifier.TargetsPartition() && len(n.zoneSpecifier.TableOrIndex.Index) == 0 && !n.allIndexes {
+		// Backward compatibility for ALTER PARTITION ... OF TABLE. Determine which
+		// index has the specified partition.
+		partitionName := string(n.zoneSpecifier.Partition)
+		indexes := table.FindIndexesWithPartition(partitionName)
+		switch len(indexes) {
+		case 0:
+			return fmt.Errorf("partition %q does not exist on table %q", partitionName, table.Name)
+		case 1:
+			n.zoneSpecifier.TableOrIndex.Index = tree.UnrestrictedName(indexes[0].Name)
+		default:
+			err := fmt.Errorf(
+				"partition %q exists on multiple indexes of table %q", partitionName, table.Name)
+			err = pgerror.WithCandidateCode(err, pgcode.InvalidParameterValue)
+			err = errors.WithHint(err, "try ALTER PARTITION ... OF INDEX ...")
+			return err
+		}
+	}
+
 	// If this is an ALTER ALL PARTITIONS statement, we need to find all indexes
 	// with the specified partition name and apply the zone configuration to all
 	// of them.
diff --git a/pkg/sql/sqlbase/structured.go b/pkg/sql/sqlbase/structured.go
index 68bf9aa59d7c..138276618a8f 100644
--- a/pkg/sql/sqlbase/structured.go
+++ b/pkg/sql/sqlbase/structured.go
@@ -2097,6 +2097,18 @@ func (desc *TableDescriptor) validatePartitioningDescriptor(
 	return nil
 }
 
+// FindIndexesWithPartition returns all IndexDescriptors (potentially including
+// the primary index) which have a partition with the given name.
+func (desc *TableDescriptor) FindIndexesWithPartition(name string) []*IndexDescriptor {
+	var indexes []*IndexDescriptor
+	for _, idx := range desc.AllNonDropIndexes() {
+		if idx.FindPartitionByName(name) != nil {
+			indexes = append(indexes, idx)
+		}
+	}
+	return indexes
+}
+
 // validatePartitioning validates that any PartitioningDescriptors contained in
 // table indexes are well-formed. See validatePartitioningDesc for details.
 func (desc *TableDescriptor) validatePartitioning() error {