Re: Added schema level support for publication.

From: vignesh C <vignesh21(at)gmail(dot)com>
To: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
Cc: "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(dot)com>, Greg Nancarrow <gregn4422(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Rahila Syed <rahilasyed90(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Subject: Re: Added schema level support for publication.
Date: 2021-08-09 06:01:31
Message-ID: CALDaNm0BeDVN4nZ7XHSMV4dHWT=WBRML4-H7h+DfkhR_o0CwqA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 6, 2021 at 2:00 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Wed, Aug 4, 2021 at 12:08 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Tue, Aug 3, 2021 at 12:00 PM tanghy(dot)fnst(at)fujitsu(dot)com
> > <tanghy(dot)fnst(at)fujitsu(dot)com> wrote:
> > >
> > > On Monday, August 2, 2021 11:40 PM vignesh C <vignesh21(at)gmail(dot)com>wrote:
> > > >
> > > > Thanks for the comments, attached v17 patches has the fixes for the same.
> > >
> > > Thanks for your new patch.
> > >
> > > I saw the following warning when compiling. It seems we don't need this variable any more.
> > >
> > > publicationcmds.c: In function ‘AlterPublicationSchemas’:
> > > publicationcmds.c:592:15: warning: unused variable ‘oldlc’ [-Wunused-variable]
> > > ListCell *oldlc;
> > > ^~~~~
> >
> > Thanks for reporting this, this is fixed in the v18 patch attached.
>
> I've also started reviewing this patch. I've not looked at the patch
> yet but here are initial comments/questions based on using this
> feature:
>
> pg_publication catalog still has puballtables column but it's still
> necessary? IIUC since pubtype = 'a' means publishing all tables in the
> database puballtables seems no longer necessary.

I will remove puballtables in my next version of the patch.

> ---
> Suppose that a parent table and its child table are defined in
> different schemas, there is a publication for the schema where only
> the parent table is defined, and the subscriber subscribes to the
> publication, should changes for its child table be replicated to the
> subscriber?

I felt that in this case only the table data that is present in the
publish schema should be sent to the subscriber. Since the child table
schema is not part of the publication, I felt this child table data
should not be replicated. Thoughts?
I have kept the above same behavior in the case of publication created
using PUBLISH_VIA_PARTITION_ROOT option i.e the child table data will
not be sent. But now I'm feeling we should send the child table data
since it is being sent through the parent table which is part of the
publication. Also this way users can use this option if the user has
the table having partitions designed across the schemas. Thoughts?

> In FOR TABLE cases, i.g., where the subscriber subscribes to the
> publication that is only for the parent table, changes for its child
> table are replicated to the subscriber.
>
> As far as I tested v18 patch, changes for the child table are not
> replicated in FOR SCHEMA cases. Here is the test script:
>
> On publisher and subscriber:
> create schema p_schema;
> create schema c_schema;
> create table p_schema.p (a int) partition by list (a);
> create table c_schema.c partition of p_schema.p for values in (1);
>
> On publisher:
> create publication pub_p_schema for schema p_schema;
>
> On subscriber:
> create subscription pub connection 'dbname=postgres' publication pub_p_schema;
>
> On publisher:
> insert into p_schema.p values (1);
> select * from p_schema.p;
> a
> ---
> 1
> (1 row)
>
> On subscriber:
> select * from p_schema.p;
> a
> ---
>
> (0 rows)

I have kept this behavior intentionally, details explained above. Thoughts?

Regards,
Vignesh

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2021-08-09 07:05:08 Re: logical replication empty transactions
Previous Message Mahendra Singh Thalor 2021-08-09 05:46:01 Re: "ERROR: cache lookup failed for function %d" while dropping function from another session