On Mon, Aug 2, 2021 at 3:31 PM tanghy.f...@fujitsu.com <tanghy.f...@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
