RE: Added schema level support for publication.

From: "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>
To: vignesh C <vignesh21(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>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Smith <smithpb2250(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(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>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: RE: Added schema level support for publication.
Date: 2021-10-16 05:57:08
Message-ID: OS0PR01MB57164EAC2B7703A24DA99EC894BA9@OS0PR01MB5716.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wednesday, October 13, 2021 10:49 AM Hou, Zhijie wrote:
> On Tuesday, October 12, 2021 9:15 PM vignesh C <vignesh21(at)gmail(dot)com>
> > Attached v40 patch has the fix for the above comments.
>
> Thanks for the update, I have some minor issues about partition related
> behavior.
>
> 1)
>
> Tang tested and discussed this issue with me.
> The testcase is:
> We publish a schema and there is a partition in the published schema. If
> publish_via_partition_root is on and the partition's parent table is not in the
> published schema, neither the change on the partition nor the parent table will
> not be published.
>
> But if we publish by FOR TABLE partition and set publish_via_partition_root to
> on, the change on the partition will be published. So, I think it'd be better to
> publish the change on partition for FOR ALL TABLES IN SCHEMA case if its parent
> table
> is not published in the same publication.
>
> It seems we should pass publication oid to the GetSchemaPublicationRelations()
> and add some check like the following:
> 2)
> I think a partitioned table could also be a partition which should not be
> appended to the list. I think we should also filter these cases here by same
> check in 1).

After some offline discussion with Vignesh and Amit.
I found my proposed fix can be improved because it brings some overhead to
functions which could be invoked many times for the same publication. Now, I
think it'd be better to add this check in GetAllSchemaPublicationRelations and
GetPublicationRelations.

Besides, I found we misunderstood the flag PUBLICATION_PART_ROOT it means:
"ROOT: only the table explicitly mentioned in the publication" We cannot use it
as a flag to judge whether do the partition filtering, I think we need to pass
the actual pubviaroot flag.

Based on the V40 patchset, attaching the Top-up patch which try to fix the
partition issue in a cleaner way.

Best regards,
Hou zj

Attachment Content-Type Size
v40-Topup-partition-fix.patch application/octet-stream 13.0 KB
v40-0001-Added-schema-level-support-for-publication.patch application/octet-stream 79.3 KB
v40-0002-Client-side-changes-to-support-FOR-ALL-TABLES-IN.patch application/octet-stream 21.6 KB
v40-0003-Tests-for-FOR-ALL-TABLES-IN-SCHEMA-publication.patch application/octet-stream 54.1 KB
v40-0004-Documentation-for-FOR-ALL-TABLES-IN-SCHEMA-publi.patch application/octet-stream 14.7 KB
v40-0005-Implemented-pg_publication_objects-view.patch application/octet-stream 6.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message houzj.fnst@fujitsu.com 2021-10-16 06:30:47 RE: Data is copied twice when specifying both child and parent table in publication
Previous Message Michael Paquier 2021-10-16 03:43:00 Re: Reset snapshot export state on the transaction abort