Multiple primary key on partition table?
Hi,
I am able to create multiple primary key on partition table by executing
below statement.
[edb@localhost bin]$ ./psql postgres
psql (11beta3)
Type "help" for help.
postgres=# CREATE TABLE t1 (a int PRIMARY KEY,b int) PARTITION BY RANGE (a);
CREATE TABLE
postgres=# CREATE TABLE t1_p1 PARTITION OF t1 (b PRIMARY KEY) FOR VALUES
FROM (MINVALUE) TO (MAXVALUE);
CREATE TABLE
postgres=# \d+ t1_p1
Table "public.t1_p1"
Column | Type | Collation | Nullable | Default | Storage | Stats target
| Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain |
|
b | integer | | not null | | plain |
|
Partition of: t1 FOR VALUES FROM (MINVALUE) TO (MAXVALUE)
Partition constraint: (a IS NOT NULL)
Indexes:
"t1_p1_pkey" PRIMARY KEY, btree (a)
"t1_p1_pkey1" PRIMARY KEY, btree (b)
Is this fine to allow it?
eventually it cause to pg_upgrade failed with below error.
pg_restore: creating TABLE "public.t1"
pg_restore: creating TABLE "public.t1_p1"
pg_restore: INFO: partition constraint for table "t1_p1" is implied by
existing constraints
pg_restore: creating CONSTRAINT "public.t1 t1_pkey"
pg_restore: creating CONSTRAINT "public.t1_p1 t1_p1_pkey"
pg_restore: creating CONSTRAINT "public.t1_p1 t1_p1_pkey1"
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2920; 2606 16395
CONSTRAINT t1_p1 t1_p1_pkey1 edb
pg_restore: [archiver (db)] could not execute query: ERROR: multiple
primary keys for table "t1_p1" are not allowed
Command was:
-- For binary upgrade, must preserve pg_class oids
SELECT
pg_catalog.binary_upgrade_set_next_index_pg_class_oid('16394'::pg_catalog.oid);
ALTER TABLE ONLY "public"."t1_p1"
ADD CONSTRAINT "t1_p1_pkey1" PRIMARY KEY ("b");
Thanks & Regards,
Rajkumar Raghuwanshi
QMG, EnterpriseDB Corporation
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?
Regards,
Amul
On Mon, Sep 17, 2018 at 4:51 PM Rajkumar Raghuwanshi
<rajkumar.raghuwanshi@enterprisedb.com> wrote:
Show quoted text
Hi,
I am able to create multiple primary key on partition table by executing below statement.
[edb@localhost bin]$ ./psql postgres
psql (11beta3)
Type "help" for help.postgres=# CREATE TABLE t1 (a int PRIMARY KEY,b int) PARTITION BY RANGE (a);
CREATE TABLE
postgres=# CREATE TABLE t1_p1 PARTITION OF t1 (b PRIMARY KEY) FOR VALUES FROM (MINVALUE) TO (MAXVALUE);
CREATE TABLE
postgres=# \d+ t1_p1
Table "public.t1_p1"
Column | Type | Collation | Nullable | Default | Storage | Stats target | Description
--------+---------+-----------+----------+---------+---------+--------------+-------------
a | integer | | not null | | plain | |
b | integer | | not null | | plain | |
Partition of: t1 FOR VALUES FROM (MINVALUE) TO (MAXVALUE)
Partition constraint: (a IS NOT NULL)
Indexes:
"t1_p1_pkey" PRIMARY KEY, btree (a)
"t1_p1_pkey1" PRIMARY KEY, btree (b)Is this fine to allow it?
eventually it cause to pg_upgrade failed with below error.pg_restore: creating TABLE "public.t1"
pg_restore: creating TABLE "public.t1_p1"
pg_restore: INFO: partition constraint for table "t1_p1" is implied by existing constraints
pg_restore: creating CONSTRAINT "public.t1 t1_pkey"
pg_restore: creating CONSTRAINT "public.t1_p1 t1_p1_pkey"
pg_restore: creating CONSTRAINT "public.t1_p1 t1_p1_pkey1"
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2920; 2606 16395 CONSTRAINT t1_p1 t1_p1_pkey1 edb
pg_restore: [archiver (db)] could not execute query: ERROR: multiple primary keys for table "t1_p1" are not allowed
Command was:
-- For binary upgrade, must preserve pg_class oids
SELECT pg_catalog.binary_upgrade_set_next_index_pg_class_oid('16394'::pg_catalog.oid);ALTER TABLE ONLY "public"."t1_p1"
ADD CONSTRAINT "t1_p1_pkey1" PRIMARY KEY ("b");Thanks & Regards,
Rajkumar Raghuwanshi
QMG, EnterpriseDB Corporation
On Mon, Sep 17, 2018 at 9:06 PM amul sul <sulamul@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
Attachments:
0001-multiple-primary-key-restriction-for-a-partition-tab.patchapplication/x-patch; name=0001-multiple-primary-key-restriction-for-a-partition-tab.patchDownload
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
[Back from a week AFK ...]
On 2018-Sep-18, amul sul wrote:
On Mon, Sep 17, 2018 at 9:06 PM amul sul <sulamul@gmail.com> wrote:
Nice catch Rajkumar.
Here is the complete patch proposes the aforesaid fix with regression test.
Looks closely related to the one that was fixed in 1f8a3327a9db, but of
course it's a different code path. Will review this shortly, thanks.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Sep 18, 2018 at 11:20 AM amul sul <sulamul@gmail.com> wrote:
On Mon, Sep 17, 2018 at 9:06 PM amul sul <sulamul@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.
Thanks, This worked for me.
Show quoted text
Regards,
Amul
On 2018-Oct-01, Rajkumar Raghuwanshi wrote:
On Tue, Sep 18, 2018 at 11:20 AM amul sul <sulamul@gmail.com> wrote:
Here is the complete patch proposes the aforesaid fix with regression test.
Thanks, This worked for me.
Yeah, looks good to me, pushed. I added one more regression test to
ensure that the PRIMARY KEY clause in the partition is still accepted if
the parent does not have one.
Thanks
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Thu, Oct 4, 2018 at 8:25 PM Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
On 2018-Oct-01, Rajkumar Raghuwanshi wrote:
On Tue, Sep 18, 2018 at 11:20 AM amul sul <sulamul@gmail.com> wrote:
Here is the complete patch proposes the aforesaid fix with regression test.
Thanks, This worked for me.
Yeah, looks good to me, pushed. I added one more regression test to
ensure that the PRIMARY KEY clause in the partition is still accepted if
the parent does not have one.
Thanks a lot, for enhancing regression and committing the fix.
Regards,
Amul