Hi Kuroda-san, Thanks for addressing most of my v6-0001 review comments.
Below are some minor follow-up comments for v7-0001. ====== src/backend/access/transam/twophase.c 1. IsTwoPhaseTransactionGidForSubid +/* + * IsTwoPhaseTransactionGidForSubid + * Check whether the given GID is formed by TwoPhaseTransactionGid. + */ +static bool +IsTwoPhaseTransactionGidForSubid(Oid subid, char *gid) I think the function comment should mention something about 'subid'. SUGGESTION Check whether the given GID (as formed by TwoPhaseTransactionGid) is for the specified 'subid'. ====== src/backend/commands/subscriptioncmds.c 2. AlterSubscription + if (!opts.twophase && + form->subtwophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED && + LookupGXactBySubid(subid)) + /* Add error message */ + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot disable two_phase when uncommitted prepared transactions present"), + errhint("Resolve these transactions and try again"))); The comment "/* Add error message */" seems unnecessary. ====== Kind Regards, Peter Smith. Fujitsu Australia