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: Greg Nancarrow <gregn4422(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(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>, "tanghy(dot)fnst(at)fujitsu(dot)com" <tanghy(dot)fnst(at)fujitsu(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>
Subject: RE: Added schema level support for publication.
Date: 2021-09-29 03:37:27
Message-ID: OS3PR01MB5718DDEACB51A10D4A338C0C94A99@OS3PR01MB5718.jpnprd01.prod.outlook.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tues, Sep 28, 2021 10:46 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> Attached v34 patch has the changes for the same.

Thanks for updating the patch.
Here are a few comments.

1)
+ * ALL TABLES IN SCHEMA schema [[, ...]

[[ -> [

2)
+ /* ALTER PUBLICATION ... ADD/DROP TABLE/ALL TABLES IN SCHEMA parameters */

The two '/' seems a bit unclear and it doesn't mention the SET case.
Maybe we can write like:

/* parameters used for ALTER PUBLICATION ... ADD/DROP/SET publication objects */

3)
+ /*
+ * Check if setting the relation to a different schema will result in the
+ * publication having schema and same schema's table in the publication.
+ */
+ if (stmt->objectType == OBJECT_TABLE)
+ {
+ ListCell *lc;
+ List *schemaPubids = GetSchemaPublications(nspOid);
+ foreach(lc, schemaPubids)
+ {
+ Oid pubid = lfirst_oid(lc);
+ if (list_member_oid(GetPublicationRelations(pubid, PUBLICATION_PART_ALL),
+ relid))
+ ereport(ERROR,

How about we check this case like the following ?

List *schemaPubids = GetSchemaPublications(nspOid);
List *relPubids = GetRelationPublications(RelationGetRelid(rel));
if (list_intersection(schemaPubids, relPubids))
ereport(ERROR, ...

Best regards,
Hou zj

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bharath Rupireddy 2021-09-29 03:48:00 Re: how to distinguish between using the server as a standby or for executing a targeted recovery in PG 11?
Previous Message Thomas Munro 2021-09-29 03:29:16 Re: Make relfile tombstone files conditional on WAL level