Re: Added schema level support for publication.

vignesh C <vignesh21@gmail.com>

From: vignesh C <vignesh21@gmail.com>
To: "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>
Cc: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Greg Nancarrow <gregn4422@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Peter Eisentraut <peter.eisentraut@enterprisedb.com>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Smith <smithpb2250@gmail.com>, Ajin Cherian <itsajin@gmail.com>, Rahila Syed <rahilasyed90@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2021-09-30T10:09:07Z
Lists: pgsql-hackers

Attachments

On Wed, Sep 29, 2021 at 8:47 AM tanghy.fnst@fujitsu.com
<tanghy.fnst@fujitsu.com> wrote:
>
> On Monday, Tuesday, September 28, 2021 10:49 PM, vignesh C <vignesh21@gmail.com> wrote:
> >
> > Yes this should not be supported, we should throw an error in this case.
> > This is handled in the v34 patch attached at [1].
> > [1] - https://www.postgresql.org/message-
> > id/CALDaNm2Z9TfuoCf09YGKfwy7F1NwC4iCXJGTaZS%3DchH6VHtadQ%40mail.g
> > mail.com
> >
>
> Thanks for fixing it. I confirmed the error can be output as expected.
>
> Here is a problem related to publish_via_partition_root option when using this
> patch. With this option on, I think pg_get_publication_tables function gave an
> unexcepted result and the subscriber would get dual data during table sync.
>
>
> For example:
> (I used pg_publication_tables view to make it looks clearer)
>
> create schema sch1;
> create table sch1.tbl1 (a int) partition by range ( a );
> create table sch1.tbl1_part1 partition of sch1.tbl1 for values from (1) to (10);
> create table sch1.tbl1_part2 partition of sch1.tbl1 for values from (10) to (20);
> create table sch1.tbl1_part3 partition of sch1.tbl1 for values from (20) to (30);
> create publication pub for all tables in schema sch1 with(publish_via_partition_root=1);
>
> postgres=# select * from pg_publication_tables where pubname='pub';
>  pubname | schemaname | tablename
> ---------+------------+------------
>  pub     | sch1       | tbl1_part1
>  pub     | sch1       | tbl1_part2
>  pub     | sch1       | tbl1_part3
>  pub     | sch1       | tbl1
> (4 rows)
>
>
> It shows both the partitioned table and its leaf partitions. But the result of
> FOR ALL TABLES publication couldn't show the leaf partitions.
>
>
> postgres=# create publication pub_all for all tables with(publish_via_partition_root=1);
> CREATE PUBLICATION
> postgres=# select * from pg_publication_tables where pubname='pub_all';
>  pubname | schemaname | tablename
> ---------+------------+-----------
>  pub_all | sch1       | tbl1
> (1 row)
>
>
> How about make the following change to avoid it? I tried it and it also fixed dual
> data issue during table sync.
>
>
> diff --git a/src/backend/catalog/pg_publication.c b/src/backend/catalog/pg_publication.c
> index 04e785b192..4e8ccdabc6 100644
> --- a/src/backend/catalog/pg_publication.c
> +++ b/src/backend/catalog/pg_publication.c
> @@ -632,7 +632,8 @@ GetSchemaPublicationRelations(Oid schemaid, PublicationPartOpt pub_partopt)
>                 Form_pg_class relForm = (Form_pg_class) GETSTRUCT(tuple);
>                 Oid                     relid = relForm->oid;
>
> -               if (is_publishable_class(relid, relForm))
> +               if (is_publishable_class(relid, relForm) &&
> +                       !(relForm->relispartition && pub_partopt == PUBLICATION_PART_ROOT))
>                         result = lappend_oid(result, relid);
>         }

The suggested change works, I have modified it in the attached patch.

Regards,
Vignesh

Commits

  1. Include schema/table publications even with exclude options in dump.

  2. Rename some enums to use TABLE instead of REL.

  3. Add tap tests for the schema publications.

  4. Allow publishing the tables of schema.

  5. In pg_dump, use simplehash.h to look up dumpable objects by OID.