On Tue, Jul 9, 2024 at 6:23 PM Hayato Kuroda (Fujitsu) <kuroda.hay...@fujitsu.com> wrote: > > Previous patch set could not be accepted due to the initialization miss. > PSA new version. >
Few minor comments: ================= 0001-patch 1. .git/rebase-apply/patch:253: space before tab in indent. errmsg("slot_name and two_phase cannot be altered at the same time"))); warning: 1 line adds whitespace errors. White space issue as shown by git am command. 2. +/* + * Common checks for altering failover and two_phase option + */ +static void +CommonChecksForFailoverAndTwophase(Subscription *sub, const char *option, + bool isTopLevel) The function name looks odd to me. How about something along the lines of CheckAlterSubOption()? 3. + ereport(ERROR, + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), + errmsg("cannot disable two_phase when uncommitted prepared transactions present"), We can slightly change the above error message to: "cannot disable two_phase when prepared transactions are present". 0003-patch Alter the altering from + <literal>true</literal> to <literal>false</literal>, the publisher will + replicate transactions again when they are committed. The beginning of the sentence sounds awkward. -- With Regards, Amit Kapila.