Hi, here are my review comments for v19-0001.
======
doc/src/sgml/protocol.sgml
nitpick - Now there is >1 option. /The following option is supported:/The following options are supported:/
======
src/backend/access/transam/twophase.c
TwoPhaseTransactionGid:
nitpick - renamed parameter /gid/gid_res/ to emphasize that this is returned by reference
~~~
1.
IsTwoPhaseTransactionGidForSubid
+ /* Construct the format GID based on the got xid */
+ TwoPhaseTransactionGid(subid, xid, gid_generated, sizeof(gid));
I think the wrong size is being passed here. It should be the buffer size -- e.g. sizeof(gid_generated).
~
nitpick - renamed a couple of vars for readability
nitpick - expanded some comments.
======
src/backend/commands/subscriptioncmds.c
2. AlterSubscription
+ /*
+ * slot_name and two_phase cannot be altered
+ * simultaneously. The latter part refers to the pre-set
+ * slot name and tries to modify the slot option, so
+ * changing both does not make sense.
+ */
I had given a v18-0002 nitpick suggestion to re-word all of this comment. But, as I wrote before [1], that fix belongs here in patch 0001 where the comment was first added.
======
[1]
https://www.postgresql.org/message-id/CAHut%2BPsqMRS3dcijo5jsTSbgV1-9So-YBC7PH7xg0%2BZ8hA7fDQ%40mail.gmail.comKind Regards,
Peter Smith.
Fujitsu Australia