Обсуждение: 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.
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)
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");
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: > > 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
Вложения
[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.
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