RE: Skipping logical replication transactions on subscriber side

On July 20, 2021 9:26 AM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> On Mon, Jul 19, 2021 at 8:38 PM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > On July 19, 2021 2:40 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)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