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/
