Horton

Thread

Commits

  1. Fix duplicate primary keys in partitions

  1. Multiple primary key on partition table?

    Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com> — 2018-09-17T11:20:49Z

    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

  2. Re: Multiple primary key on partition table?

    Amul Sul <sulamul@gmail.com> — 2018-09-17T15:36:01Z

    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

  3. Re: Multiple primary key on partition table?

    Amul Sul <sulamul@gmail.com> — 2018-09-18T05:49:44Z

    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

  4. Re: Multiple primary key on partition table?

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-09-24T15:57:12Z

    [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

  5. Re: Multiple primary key on partition table?

    Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com> — 2018-10-01T06:39:14Z

    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
>

  6. Re: Multiple primary key on partition table?

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-10-04T14:55:03Z

    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

  7. Re: Multiple primary key on partition table?

    Amul Sul <sulamul@gmail.com> — 2018-10-05T04:50:45Z

    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