On July 20, 2021 9:26 AM Masahiko Sawada <sawada.m...@gmail.com> wrote: > On Mon, Jul 19, 2021 at 8:38 PM houzj.f...@fujitsu.com > <houzj.f...@fujitsu.com> wrote: > > > > On July 19, 2021 2:40 PM Masahiko Sawada <sawada.m...@gmail.com> > wrote: > > > I've attached the updated version patch that incorporated all > > > comments I got so far except for the clearing error details part I > > > mentioned above. After getting a consensus on those parts, I'll > > > incorporate the idea into the patches. > > > > 3) For 0003 patch, if user set skip_xid to a wrong xid which have not been > > assigned, and then will the change be skipped when the xid is assigned in > > the future even if it doesn't cause any conflicts ? > > Yes. Currently, setting a correct xid is the user's responsibility. I think > it would > be better to disable it or emit WARNING/ERROR when the user mistakenly set > the wrong xid if we find out a convenient way to detect that.
Thanks for the explanation. As Amit suggested, it seems we can document the risk of misusing skip_xid. Besides, I found some minor things in the patch. 1) In 0002 patch + */ +static void +pgstat_recv_subscription_purge(PgStat_MsgSubscriptionPurge *msg, int len) +{ + if (subscriptionErrHash != NULL) + return; + +static void +pgstat_recv_subscription_error(PgStat_MsgSubscriptionErr *msg, int len) +{ the second paramater "len" seems not used in the function pgstat_recv_subscription_purge() and pgstat_recv_subscription_error(). 2) in 0003 patch * Helper function for apply_handle_commit and apply_handle_stream_commit. */ static void -apply_handle_commit_internal(StringInfo s, LogicalRepCommitData *commit_data) +apply_handle_commit_internal(LogicalRepCommitData *commit_data) { This looks like a separate change which remove unused paramater in existing code, maybe we can get this committed first ? Best regards, Houzj