Re: Skipping logical replication transactions on subscriber side

On Mon, Aug 16, 2021 at 3:59 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Thu, Aug 12, 2021 1:53 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> > I've attached the updated patches. FYI I've included the patch
> > (v8-0005) that fixes the assertion failure during shared fileset cleanup to make
> > cfbot tests happy.
>
> Hi,
>
> Thanks for the new patches.
> I have a few comments on the v8-0001 patch.

Thank you for the comments!

>
>
> 2)
> +/*
> + * Get string representing LogicalRepMsgType.
> + */
> +char *
> +logicalrep_message_type(LogicalRepMsgType action)
> +{
> ...
> +
> +       elog(ERROR, "invalid logical replication message type \"%c\"", action);
> +}
>
> Some old compilers might complain that the function doesn't have a return value
> at the end of the function, maybe we can code like the following:
>
> +char *
> +logicalrep_message_type(LogicalRepMsgType action)
> +{
> +       switch (action)
> +       {
> +               case LOGICAL_REP_MSG_BEGIN:
> +                       return "BEGIN";
> ...
> +               default:
> +                       elog(ERROR, "invalid logical replication message type \"%c\"", action);
> +       }
> +       return NULL;                            /* keep compiler quiet */
> +}

Fixed.

>
>
> 3)
> Do we need to invoke set_apply_error_context_xact() in the function
> apply_handle_stream_prepare() to save the xid and timestamp ?

Yes. I think that v8-0001 patch already set xid and timestamp just
after parsing stream_prepare message. You meant it's not necessary?

I'll submit the updated patches soon.

Regards,

--
Masahiko Sawada
EDB:  https://www.enterprisedb.com/