From 688e0185d0552b9998bd050ae5a6429b722c614f Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 2 Jul 2024 19:06:44 +0200
Subject: [PATCH v3 1/3] Cope at DETACH time, for 12 - 16

---
 src/backend/commands/tablecmds.c          | 18 +++++++++---
 src/test/regress/expected/constraints.out | 31 ++++++++++++++++++++
 src/test/regress/sql/constraints.sql      | 35 +++++++++++++++++++++++
 3 files changed, 80 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index dbfe0d6b1c..37faa2a39a 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -19281,22 +19281,32 @@ DetachPartitionFinalize(Relation rel, Relation partRel, bool concurrent,
 	foreach(cell, indexes)
 	{
 		Oid			idxid = lfirst_oid(cell);
+		Oid			parentidx;
 		Relation	idx;
 		Oid			constrOid;
+		Oid			parentConstrOid;
 
 		if (!has_superclass(idxid))
 			continue;
 
-		Assert((IndexGetRelation(get_partition_parent(idxid, false), false) ==
-				RelationGetRelid(rel)));
+		parentidx = get_partition_parent(idxid, false);
+		Assert((IndexGetRelation(parentidx, false) == RelationGetRelid(rel)));
 
 		idx = index_open(idxid, AccessExclusiveLock);
 		IndexSetParentIndex(idx, InvalidOid);
 
-		/* If there's a constraint associated with the index, detach it too */
+		/*
+		 * If there's a constraint associated with the index, detach it too.
+		 * Careful: in releases prior to 17, it was possible for a constraint
+		 * index in a partition to be the child of a non-constraint index, so
+		 * we verify whether the parent index does actually have a constraint
+		 * first.
+		 */
 		constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
 													idxid);
-		if (OidIsValid(constrOid))
+		parentConstrOid = get_relation_idx_constraint_oid(RelationGetRelid(rel),
+														  parentidx);
+		if (OidIsValid(parentConstrOid) && OidIsValid(constrOid))
 			ConstraintSetParentConstraint(constrOid, InvalidOid, InvalidOid);
 
 		index_close(idx, NoLock);
diff --git a/src/test/regress/expected/constraints.out b/src/test/regress/expected/constraints.out
index e6f6602d95..47aa754965 100644
--- a/src/test/regress/expected/constraints.out
+++ b/src/test/regress/expected/constraints.out
@@ -626,6 +626,37 @@ SELECT conname FROM pg_constraint WHERE conrelid = 'parted_fk_naming_1'::regclas
 (1 row)
 
 DROP TABLE parted_fk_naming;
+--
+-- Test various ways to create primary keys on partitions, linked to unique
+-- indexes on the partitioned table.  These should all fail, but we don't dare
+-- change released behavior, so instead cope with it at DETACH time.
+CREATE TABLE t (a integer) PARTITION BY LIST (a);
+CREATE TABLE tp (a integer PRIMARY KEY);
+ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
+CREATE UNIQUE INDEX t_a_idx ON t (a);
+ALTER INDEX t_a_idx ATTACH PARTITION tp_pkey;
+ALTER TABLE t DETACH PARTITION tp;
+DROP TABLE t, tp;
+CREATE TABLE t (a integer) PARTITION BY LIST (a);
+CREATE TABLE tp (a integer PRIMARY KEY);
+CREATE UNIQUE INDEX t_a_idx ON t (a);
+ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
+ALTER TABLE t DETACH PARTITION tp;
+DROP TABLE t, tp;
+CREATE TABLE t (a integer) PARTITION BY LIST (a);
+CREATE TABLE tp (a integer PRIMARY KEY);
+CREATE UNIQUE INDEX t_a_idx ON ONLY t (a);
+ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
+ALTER TABLE t DETACH PARTITION tp;
+DROP TABLE t, tp;
+CREATE TABLE regress_constr_partitioned (a integer) PARTITION BY LIST (a);
+CREATE TABLE regress_constr_partition1 PARTITION OF regress_constr_partitioned FOR VALUES IN (1);
+ALTER TABLE regress_constr_partition1 ADD PRIMARY KEY (a);
+CREATE UNIQUE INDEX ON regress_constr_partitioned (a);
+BEGIN;
+ALTER TABLE regress_constr_partitioned DETACH PARTITION regress_constr_partition1;
+ROLLBACK;
+--  Leave this one in funny state for pg_upgrade testing
 -- test a HOT update that invalidates the conflicting tuple.
 -- the trigger should still fire and catch the violation
 BEGIN;
diff --git a/src/test/regress/sql/constraints.sql b/src/test/regress/sql/constraints.sql
index 5ffcd4ffc7..04fdf51a4a 100644
--- a/src/test/regress/sql/constraints.sql
+++ b/src/test/regress/sql/constraints.sql
@@ -449,6 +449,41 @@ ALTER TABLE parted_fk_naming ATTACH PARTITION parted_fk_naming_1 FOR VALUES IN (
 SELECT conname FROM pg_constraint WHERE conrelid = 'parted_fk_naming_1'::regclass AND contype = 'f';
 DROP TABLE parted_fk_naming;
 
+--
+-- Test various ways to create primary keys on partitions, linked to unique
+-- indexes on the partitioned table.  These should all fail, but we don't dare
+-- change released behavior, so instead cope with it at DETACH time.
+CREATE TABLE t (a integer) PARTITION BY LIST (a);
+CREATE TABLE tp (a integer PRIMARY KEY);
+ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
+CREATE UNIQUE INDEX t_a_idx ON t (a);
+ALTER INDEX t_a_idx ATTACH PARTITION tp_pkey;
+ALTER TABLE t DETACH PARTITION tp;
+DROP TABLE t, tp;
+
+CREATE TABLE t (a integer) PARTITION BY LIST (a);
+CREATE TABLE tp (a integer PRIMARY KEY);
+CREATE UNIQUE INDEX t_a_idx ON t (a);
+ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
+ALTER TABLE t DETACH PARTITION tp;
+DROP TABLE t, tp;
+
+CREATE TABLE t (a integer) PARTITION BY LIST (a);
+CREATE TABLE tp (a integer PRIMARY KEY);
+CREATE UNIQUE INDEX t_a_idx ON ONLY t (a);
+ALTER TABLE t ATTACH PARTITION tp FOR VALUES IN (1);
+ALTER TABLE t DETACH PARTITION tp;
+DROP TABLE t, tp;
+
+CREATE TABLE regress_constr_partitioned (a integer) PARTITION BY LIST (a);
+CREATE TABLE regress_constr_partition1 PARTITION OF regress_constr_partitioned FOR VALUES IN (1);
+ALTER TABLE regress_constr_partition1 ADD PRIMARY KEY (a);
+CREATE UNIQUE INDEX ON regress_constr_partitioned (a);
+BEGIN;
+ALTER TABLE regress_constr_partitioned DETACH PARTITION regress_constr_partition1;
+ROLLBACK;
+--  Leave this one in funny state for pg_upgrade testing
+
 -- test a HOT update that invalidates the conflicting tuple.
 -- the trigger should still fire and catch the violation
 
-- 
2.39.2

