On Mon, Sep 17, 2018 at 9:06 PM amul sul <sula...@gmail.com> wrote:
>
> Nice catch Rajkumar.
>
> In index_check_primary_key(), relationHasPrimaryKey() called only for the an
> alter command but I think we need to call in this case as well, like this:
>
> diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
> index 7eb3e35166..c8714395fe 100644
> --- a/src/backend/catalog/index.c
> +++ b/src/backend/catalog/index.c
> @@ -223,7 +223,7 @@ index_check_primary_key(Relation heapRel,
> * and CREATE INDEX doesn't have a way to say PRIMARY KEY, so it's no
> * problem either.
> */
> - if (is_alter_table &&
> + if ((is_alter_table || heapRel->rd_rel->relispartition) &&
> relationHasPrimaryKey(heapRel))
> {
> ereport(ERROR,
>
> Thoughts?
>
Here is the complete patch proposes the aforesaid fix with regression test.
Regards,
Amul
From e936138f4befce29f5dfda1df37f1ab9acf0c8de Mon Sep 17 00:00:00 2001
From: Amul Sul <amul.sul@enterprisedb.com>
Date: Tue, 18 Sep 2018 01:40:38 -0400
Subject: [PATCH] multiple primary key restriction for a partition table
---
src/backend/catalog/index.c | 10 +++++-----
src/test/regress/expected/indexing.out | 3 +++
src/test/regress/sql/indexing.sql | 2 ++
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 7eb3e35166..a18816f312 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -218,12 +218,12 @@ index_check_primary_key(Relation heapRel,
int i;
/*
- * If ALTER TABLE, check that there isn't already a PRIMARY KEY. In CREATE
- * TABLE, we have faith that the parser rejected multiple pkey clauses;
- * and CREATE INDEX doesn't have a way to say PRIMARY KEY, so it's no
- * problem either.
+ * If ALTER TABLE and CREATE TABLE .. PARTITION OF, check that there isn't
+ * already a PRIMARY KEY. In CREATE TABLE for an ordinary relations, we
+ * have faith that the parser rejected multiple pkey clauses; and CREATE
+ * INDEX doesn't have a way to say PRIMARY KEY, so it's no problem either.
*/
- if (is_alter_table &&
+ if ((is_alter_table || heapRel->rd_rel->relispartition) &&
relationHasPrimaryKey(heapRel))
{
ereport(ERROR,
diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out
index b9297c98d2..b9d11bf397 100644
--- a/src/test/regress/expected/indexing.out
+++ b/src/test/regress/expected/indexing.out
@@ -781,6 +781,9 @@ Indexes:
"idxpart_pkey" PRIMARY KEY, btree (a)
Number of partitions: 0
+-- multiple primary key on child should fail
+create table failpart partition of idxpart (b primary key) for values from (0) to (100);
+ERROR: multiple primary keys for table "failpart" are not allowed
drop table idxpart;
-- but not if you fail to use the full partition key
create table idxpart (a int unique, b int) partition by range (a, b);
diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql
index 2091a87ff5..25224b6924 100644
--- a/src/test/regress/sql/indexing.sql
+++ b/src/test/regress/sql/indexing.sql
@@ -399,6 +399,8 @@ drop table idxpart;
-- Verify that it works to add primary key / unique to partitioned tables
create table idxpart (a int primary key, b int) partition by range (a);
\d idxpart
+-- multiple primary key on child should fail
+create table failpart partition of idxpart (b primary key) for values from (0) to (100);
drop table idxpart;
-- but not if you fail to use the full partition key
--
2.18.0
