Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns

On Fri, Jul 15, 2022 at 3:32 PM shiy.fnst@fujitsu.com
<shiy.fnst@fujitsu.com> wrote:
>
> On Mon, Jul 11, 2022 9:54 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> >
> > I've attached an updated patch, please review it.
> >
>
> Thanks for your patch. Here are some comments for the REL14-v1 patch.
>
> 1.
> +               Size            sz = sizeof(TransactionId) * nxacts;;
>
> There is a redundant semicolon at the end.
>
> 2.
> +       workspace = MemoryContextAlloc(rb->context, rb->n_initial_running_xacts);
>
> Should it be:
> +       workspace = MemoryContextAlloc(rb->context, sizeof(TransactionId) * rb->n_initial_running_xacts);
>
> 3.
> +       /* bound check if there is at least one transaction to be removed */
> +       if (NormalTransactionIdPrecedes(rb->initial_running_xacts[0],
> +                                                                       running->oldestRunningXid))
> +               return;
> +
>
> Here, I think it should return if rb->initial_running_xacts[0] is older than
> oldestRunningXid, right? Should it be changed to:
>
> +       if (!NormalTransactionIdPrecedes(rb->initial_running_xacts[0],
> +                                                                       running->oldestRunningXid))
> +               return;
>
> 4.
> +       if ((parsed->xinfo & XACT_XINFO_HAS_INVALS) != 0)
>
> Maybe we can change it like the following, to be consistent with other places in
> this file. It's also fine if you don't change it.
>
> +       if (parsed->xinfo & XACT_XINFO_HAS_INVALS)

Thank you for the comments!

I've attached patches for all supported branches including the master.

Regards,

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