Re: [PATCH]Comment improvement in publication.sql - Mailing list pgsql-hackers

From vignesh C
Subject Re: [PATCH]Comment improvement in publication.sql
Date
Msg-id CALDaNm2f+NW6Na8RA05hLrmskH3E9kgnLSdWYBEfWuwtPiY3Vw@mail.gmail.com
Whole thread Raw
In response to [PATCH]Comment improvement in publication.sql  ("tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>)
Responses RE: [PATCH]Comment improvement in publication.sql  ("tanghy.fnst@fujitsu.com" <tanghy.fnst@fujitsu.com>)
List pgsql-hackers
On Mon, Aug 2, 2021 at 3:31 PM tanghy.fnst@fujitsu.com
<tanghy.fnst@fujitsu.com> wrote:
>
> Hi Hackers
>
> When review and test another patch at [1], I found some comments in existing test code of "
src/test/regress/sql/publication.sql" is a little bit confused.
 
> Attached a patch to fix them, please take a check.
>
> Here is the detail:
>
> Existing code:
> CREATE TABLE testpub_tbl2 (id serial primary key, data text);
> -- fail - can't add to for all tables publication
> ALTER PUBLICATION testpub_foralltables ADD TABLE testpub_tbl2;
> -- fail - can't drop from all tables publication
> ALTER PUBLICATION testpub_foralltables DROP TABLE testpub_tbl2;
> -- fail - can't add to for all tables publication
> ALTER PUBLICATION testpub_foralltables SET TABLE pub_test.testpub_nopk;
>
> After patch:
> CREATE TABLE testpub_tbl2 (id serial primary key, data text);
> -- fail - tables can't be added to or dropped form FOR ALL TABLES publications
> ALTER PUBLICATION testpub_foralltables ADD TABLE testpub_tbl2;
> ALTER PUBLICATION testpub_foralltables DROP TABLE testpub_tbl2;
> ALTER PUBLICATION testpub_foralltables SET TABLE pub_test.testpub_nopk;
>
> You see the comment for SET TABLE is not appropriate.
> And above three operations(ADD DROP SET) output the same message as below:
> "DETAIL:  Tables cannot be added to or dropped from FOR ALL TABLES publications."
>
> So maybe we can combine the existing three comments to one, thoughts?
>
> Besides, another comment in the same file is not clear enough to me:
> -- fail - already added
> CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1;
>
> Maybe it will be better if we use 'already exists'. Thoughts?
>
> [1]
https://www.postgresql.org/message-id/OS0PR01MB6113CC160D0F134448567FDDFBE99%40OS0PR01MB6113.jpnprd01.prod.outlook.com

Few minor suggestions:
1) Should we change below to "fail - tables can't be added, dropped or
set to "FOR ALL TABLES" publications"
--- fail - can't add to for all tables publication
+-- fail - tables can't be added to or dropped from FOR ALL TABLES publications

2) Should we change this to "--fail - already existing publication"
--- fail - already added
+-- fail - already exists

Regards,
Vignesh



pgsql-hackers by date:

Previous
From: vignesh C
Date:
Subject: Re: Corrected documentation of data type for the logical replication message formats.
Next
From: Andres Freund
Date:
Subject: Re: Minimal logical decoding on standbys