On Mon, Jun 21, 2021 at 4:37 PM Peter Smith <smithpb2...@gmail.com> wrote: > > Please find attached the latest patch set v88* >
Some minor comments: (1) v88-0002 doc/src/sgml/logicaldecoding.sgml "examples shows" is not correct. I think there is only ONE example being referred to. BEFORE: + The following examples shows how logical decoding is controlled over the AFTER: + The following example shows how logical decoding is controlled over the (2) v88 - 0003 doc/src/sgml/ref/create_subscription.sgml (i) BEFORE: + to the subscriber on the PREPARE TRANSACTION. By default, the transaction + prepared on publisher is decoded as a normal transaction at commit. AFTER: + to the subscriber on the PREPARE TRANSACTION. By default, the transaction + prepared on the publisher is decoded as a normal transaction at commit time. (ii) src/backend/access/transam/twophase.c The double-bracketing is unnecessary: BEFORE: + if ((gxact->valid && strcmp(gxact->gid, gid) == 0)) AFTER: + if (gxact->valid && strcmp(gxact->gid, gid) == 0) (iii) src/backend/replication/logical/snapbuild.c Need to add some commas to make the following easier to read, and change "needs" to "need": BEFORE: + * The prepared transactions that were skipped because previously + * two-phase was not enabled or are not covered by initial snapshot needs + * to be sent later along with commit prepared and they must be before + * this point. AFTER: + * The prepared transactions, that were skipped because previously + * two-phase was not enabled or are not covered by initial snapshot, need + * to be sent later along with commit prepared and they must be before + * this point. (iv) src/backend/replication/logical/tablesync.c I think the convention used in Postgres code is to check for empty Lists using "== NIL" and non-empty Lists using "!= NIL". BEFORE: + if (table_states_not_ready && !last_start_times) AFTER: + if (table_states_not_ready != NIL && !last_start_times) BEFORE: + else if (!table_states_not_ready && last_start_times) AFTER: + else if (table_states_not_ready == NIL && last_start_times) Regards, Greg Nancarrow Fujitsu Australia