Re: Added schema level support for publication.
vignesh C <vignesh21@gmail.com>
From: vignesh C <vignesh21@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: "tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>,
Greg Nancarrow <gregn4422@gmail.com>, Ajin Cherian <itsajin@gmail.com>,
"houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Rahila Syed <rahilasyed90@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Date: 2021-08-09T06:01:31Z
Lists: pgsql-hackers
On Fri, Aug 6, 2021 at 2:00 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote: > > On Wed, Aug 4, 2021 at 12:08 AM vignesh C <vignesh21@gmail.com> wrote: > > > > On Tue, Aug 3, 2021 at 12:00 PM tanghy.fnst@fujitsu.com > > <tanghy.fnst@fujitsu.com> wrote: > > > > > > On Monday, August 2, 2021 11:40 PM vignesh C <vignesh21@gmail.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
Commits
-
Include schema/table publications even with exclude options in dump.
- 4aa6fa3cd0a2 18.0 landed
-
Rename some enums to use TABLE instead of REL.
- b3812d0b9bcf 15.0 landed
-
Add tap tests for the schema publications.
- 6b0f6f79eef2 15.0 landed
-
Allow publishing the tables of schema.
- 5a2832465fd8 15.0 landed
-
In pg_dump, use simplehash.h to look up dumpable objects by OID.
- 92316a4582a5 15.0 cited