inherited primary key misbehavior

Started by Amit Langotealmost 7 years ago5 messages
#1Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
1 attachment(s)

Hi,

It seems that ATExecDetachPartition misses to mark a child's primary key
constraint entry (if any) as local (conislocal=true, coninhcount=0), which
causes this:

create table p (a int primary key) partition by list (a);
create table p2 partition of p for values in (2);
alter table p detach partition p2;
alter table p2 drop constraint p2_pkey ;
ERROR: cannot drop inherited constraint "p2_pkey" of relation "p2"

select conname, conislocal, coninhcount from pg_constraint where conrelid
= 'p2'::regclass;
conname │ conislocal │ coninhcount
─────────┼────────────┼─────────────
p2_pkey │ f │ 1
(1 row)

How about the attached patch?

Thanks,
Amit

Attachments:

0001-Detach-primary-key-constraint-when-detaching-partiti.patchtext/plain; charset=UTF-8; name=0001-Detach-primary-key-constraint-when-detaching-partiti.patchDownload
From 42cb636ce6acee717fa19f6b69bf40aacc402852 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 23 Jan 2019 18:33:09 +0900
Subject: [PATCH] Detach primary key constraint when detaching partition

---
 src/backend/commands/tablecmds.c       | 11 +++++++++++
 src/test/regress/expected/indexing.out |  9 +++++++++
 src/test/regress/sql/indexing.sql      |  9 +++++++++
 3 files changed, 29 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 28a137bb53..897f74119d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15094,6 +15094,7 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	foreach(cell, indexes)
 	{
 		Oid			idxid = lfirst_oid(cell);
+		Oid			constrOid;
 		Relation	idx;
 
 		if (!has_superclass(idxid))
@@ -15106,6 +15107,16 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 		IndexSetParentIndex(idx, InvalidOid);
 		update_relispartition(classRel, idxid, false);
 		index_close(idx, NoLock);
+
+		/* Detach any associated constraint as well. */
+		if (!idx->rd_index->indisprimary)
+			continue;
+		constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
+													idxid);
+		if (!OidIsValid(constrOid))
+			elog(ERROR, "missing pg_constraint entry of index %u of %u",
+				 idxid, RelationGetRelid(partRel));
+		ConstraintSetParentConstraint(constrOid, InvalidOid);
 	}
 	table_close(classRel, RowExclusiveLock);
 
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 118f2c78df..ebf86d7bfc 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1414,3 +1414,12 @@ DETAIL:  Key (a)=(4) already exists.
 create unique index on covidxpart (b) include (a); -- should fail
 ERROR:  insufficient columns in UNIQUE constraint definition
 DETAIL:  UNIQUE constraint on table "covidxpart" lacks column "a" which is part of the partition key.
+-- check that detaching a partition also detaches the primary key constraint
+create table parted_pk_detach_test (a int primary key) partition by list (a);
+create table parted_pk_detach_test1 partition of parted_pk_detach_test for values in (1);
+alter table parted_pk_detach_test detach partition parted_pk_detach_test1;
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;
+alter table parted_pk_detach_test attach partition parted_pk_detach_test1 for values in (1);
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;	-- should fail
+ERROR:  cannot drop inherited constraint "parted_pk_detach_test1_pkey" of relation "parted_pk_detach_test1"
+drop table parted_pk_detach_test;
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index d4a64c18c7..c110f0bdb8 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -757,3 +757,12 @@ alter table covidxpart attach partition covidxpart4 for values in (4);
 insert into covidxpart values (4, 1);
 insert into covidxpart values (4, 1);
 create unique index on covidxpart (b) include (a); -- should fail
+
+-- check that detaching a partition also detaches the primary key constraint
+create table parted_pk_detach_test (a int primary key) partition by list (a);
+create table parted_pk_detach_test1 partition of parted_pk_detach_test for values in (1);
+alter table parted_pk_detach_test detach partition parted_pk_detach_test1;
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;
+alter table parted_pk_detach_test attach partition parted_pk_detach_test1 for values in (1);
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;	-- should fail
+drop table parted_pk_detach_test;
-- 
2.11.0

#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Amit Langote (#1)
Re: inherited primary key misbehavior

Hello

On 2019-Jan-23, Amit Langote wrote:

It seems that ATExecDetachPartition misses to mark a child's primary key
constraint entry (if any) as local (conislocal=true, coninhcount=0), which
causes this:

create table p (a int primary key) partition by list (a);
create table p2 partition of p for values in (2);
alter table p detach partition p2;
alter table p2 drop constraint p2_pkey ;
ERROR: cannot drop inherited constraint "p2_pkey" of relation "p2"

Ugh.

I suppose unique keys would have the same problem, so the check for
indisprimary is wrong. Other than that, looks correct.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#3Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Alvaro Herrera (#2)
1 attachment(s)
Re: inherited primary key misbehavior

On 2019/01/24 6:10, Alvaro Herrera wrote:

Hello

On 2019-Jan-23, Amit Langote wrote:

It seems that ATExecDetachPartition misses to mark a child's primary key
constraint entry (if any) as local (conislocal=true, coninhcount=0), which
causes this:

create table p (a int primary key) partition by list (a);
create table p2 partition of p for values in (2);
alter table p detach partition p2;
alter table p2 drop constraint p2_pkey ;
ERROR: cannot drop inherited constraint "p2_pkey" of relation "p2"

Ugh.

I suppose unique keys would have the same problem, so the check for
indisprimary is wrong.

Fixed in the attached. We don't support inheriting EXCLUSION constraints
yet, so no need to consider their indexes.

Thanks,
Amit

Attachments:

v2-0001-Detach-primary-key-constraint-when-detaching-part.patchtext/plain; charset=UTF-8; name=v2-0001-Detach-primary-key-constraint-when-detaching-part.patchDownload
From d794906e8d2a0839f548f20be5817f306b833b15 Mon Sep 17 00:00:00 2001
From: amit <amitlangote09@gmail.com>
Date: Wed, 23 Jan 2019 18:33:09 +0900
Subject: [PATCH v2] Detach primary key constraint when detaching partition

---
 src/backend/commands/tablecmds.c       | 16 ++++++++++++++++
 src/test/regress/expected/indexing.out | 15 +++++++++++++++
 src/test/regress/sql/indexing.sql      | 14 ++++++++++++++
 3 files changed, 45 insertions(+)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 28a137bb53..e3baf73e7b 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -15094,6 +15094,7 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 	foreach(cell, indexes)
 	{
 		Oid			idxid = lfirst_oid(cell);
+		Oid			constrOid;
 		Relation	idx;
 
 		if (!has_superclass(idxid))
@@ -15106,6 +15107,21 @@ ATExecDetachPartition(Relation rel, RangeVar *name)
 		IndexSetParentIndex(idx, InvalidOid);
 		update_relispartition(classRel, idxid, false);
 		index_close(idx, NoLock);
+
+		/*
+		 * Detach any constraints associated with the index too.  Only UNIQUE
+		 * and PRIMARY KEY index constraints can be inherited.
+		 */
+		if (!idx->rd_index->indisprimary && !idx->rd_index->indisunique)
+			continue;
+
+		constrOid = get_relation_idx_constraint_oid(RelationGetRelid(partRel),
+													idxid);
+		if (!OidIsValid(constrOid))
+			elog(ERROR, "missing pg_constraint entry of index %u of %u",
+				 idxid, RelationGetRelid(partRel));
+
+		ConstraintSetParentConstraint(constrOid, InvalidOid);
 	}
 	table_close(classRel, RowExclusiveLock);
 
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index 118f2c78df..b344351ee7 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -1414,3 +1414,18 @@ DETAIL:  Key (a)=(4) already exists.
 create unique index on covidxpart (b) include (a); -- should fail
 ERROR:  insufficient columns in UNIQUE constraint definition
 DETAIL:  UNIQUE constraint on table "covidxpart" lacks column "a" which is part of the partition key.
+-- check that detaching a partition also detaches the primary key constraint
+create table parted_pk_detach_test (a int primary key) partition by list (a);
+create table parted_pk_detach_test1 partition of parted_pk_detach_test for values in (1);
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;	-- should fail
+ERROR:  cannot drop inherited constraint "parted_pk_detach_test1_pkey" of relation "parted_pk_detach_test1"
+alter table parted_pk_detach_test detach partition parted_pk_detach_test1;
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;
+drop table parted_pk_detach_test, parted_pk_detach_test1;
+create table parted_uniq_detach_test (a int unique) partition by list (a);
+create table parted_uniq_detach_test1 partition of parted_uniq_detach_test for values in (1);
+alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;	-- should fail
+ERROR:  cannot drop inherited constraint "parted_uniq_detach_test1_a_key" of relation "parted_uniq_detach_test1"
+alter table parted_uniq_detach_test detach partition parted_uniq_detach_test1;
+alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;
+drop table parted_uniq_detach_test, parted_uniq_detach_test1;
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index d4a64c18c7..3d43a57da2 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -757,3 +757,17 @@ alter table covidxpart attach partition covidxpart4 for values in (4);
 insert into covidxpart values (4, 1);
 insert into covidxpart values (4, 1);
 create unique index on covidxpart (b) include (a); -- should fail
+
+-- check that detaching a partition also detaches the primary key constraint
+create table parted_pk_detach_test (a int primary key) partition by list (a);
+create table parted_pk_detach_test1 partition of parted_pk_detach_test for values in (1);
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;	-- should fail
+alter table parted_pk_detach_test detach partition parted_pk_detach_test1;
+alter table parted_pk_detach_test1 drop constraint parted_pk_detach_test1_pkey;
+drop table parted_pk_detach_test, parted_pk_detach_test1;
+create table parted_uniq_detach_test (a int unique) partition by list (a);
+create table parted_uniq_detach_test1 partition of parted_uniq_detach_test for values in (1);
+alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;	-- should fail
+alter table parted_uniq_detach_test detach partition parted_uniq_detach_test1;
+alter table parted_uniq_detach_test1 drop constraint parted_uniq_detach_test1_a_key;
+drop table parted_uniq_detach_test, parted_uniq_detach_test1;
-- 
2.11.0

#4Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Amit Langote (#3)
Re: inherited primary key misbehavior

On 2019-Jan-24, Amit Langote wrote:

Fixed in the attached. We don't support inheriting EXCLUSION constraints
yet, so no need to consider their indexes.

Looks good, pushed.

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#5Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Alvaro Herrera (#4)
Re: inherited primary key misbehavior

On 2019/01/24 12:03, Alvaro Herrera wrote:

On 2019-Jan-24, Amit Langote wrote:

Fixed in the attached. We don't support inheriting EXCLUSION constraints
yet, so no need to consider their indexes.

Looks good, pushed.

Thank you.

Regards,
Amit