Re: [PATCH]Comment improvement in publication.sql

On Mon, Aug 2, 2021 at 3:31 PM tanghy(dot)fnst(at)fujitsu(dot)com
<tanghy(dot)fnst(at)fujitsu(dot)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