Some comments for v9-0002. ====== APPLY:
0. The patch does not apply cleanly: $ git apply ../patches_misc/v9-0002-Support-EXCEPT-for-ALL-SEQUENCES-in-ALTER-PUBLICA.patch error: patch failed: doc/src/sgml/ref/alter_publication.sgml:277 ====== doc/src/sgml/ref/alter_publication.sgml 1. + <para> + Reset the publication to be a <literal>ALL SEQUENCES</literal> publication + with no excluded sequences: Personally, I don't see how it is good to have "publication" 2x and "sequences" 2x in the same sentence. SUGGESTION Reset the publication to be <literal>ALL SEQUENCES</literal> with no exclusions: ~ I know you're only following the same pattern as "Reset the publication to be a FOR ALL TABLES publication with no excluded tables:". It is good to be consistent, but when the original text is poor, copying it doesn't make it better. Things like this fall into a grey-zone because, unless they get fixed "in-passing", nothing ever changes: a) I think a patch to only change the original wording would be rejected because it is too trivial. b) OTOH, when the original is used as a precedent, the poor wording just spreads further. ~ Anyway, if your chose not to reword this, then there is a typo /a ALL SEQUENCES publication/an ALL SEQUENCES publication/ ====== src/backend/commands/publicationcmds.c get_delete_rels: 2. + /* look up the cache for the old relmap */ /look up/Look up/ ~~~ 3. + if(found) + break; Missing space after 'if'. ~~~ 4. + oldrel = palloc0_object (PublicationRelInfo); Unwanted space after function name. ====== Kind Regards, Peter Smith. Fujitsu Australia
