public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Kapila <[email protected]>
To: Kyotaro Horiguchi <[email protected]>
Cc: Sawada Masahiko <[email protected]>
Cc: Osumi, Takamichi/大墨 昂道 <[email protected]>
Cc: Drouvot, Bertrand <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Oh, Mike <[email protected]>
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Date: Tue, 19 Jul 2022 17:26:53 +0530
Message-ID: <CAA4eK1+QjoDLcPi0gZBQLCar47A-ya_kx_gvi+UqiWiAnpckxQ@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAA4eK1JWaY5t+20iteUnPZy0E+nJi4KPqH-wEYyKx7YkcHmEqA@mail.gmail.com>
<CAD21AoC1E09F_swzOPQQE9P4tf3ADr0cyATesGHA3JrySAytNA@mail.gmail.com>
<[email protected]>
<[email protected]>
On Tue, Jul 19, 2022 at 1:43 PM Kyotaro Horiguchi
<[email protected]> wrote:
>
> At Tue, 19 Jul 2022 16:57:14 +0900 (JST), Kyotaro Horiguchi <[email protected]> wrote in
> > At Tue, 19 Jul 2022 16:02:26 +0900, Masahiko Sawada <[email protected]> wrote in
> > > On Tue, Jul 19, 2022 at 1:47 PM Amit Kapila <[email protected]> wrote:
> > > > Good work. I wonder without comments this may create a problem in the
> > > > future. OTOH, I don't see adding a check "catchange.xcnt > 0" before
> > > > freeing the memory any less robust. Also, for consistency, we can use
> > > > a similar check based on xcnt in the SnapBuildRestore to free the
> > > > memory in the below code:
> > > > + /* set catalog modifying transactions */
> > > > + if (builder->catchange.xip)
> > > > + pfree(builder->catchange.xip);
> > >
> > > I would hesitate to add comments about preventing the particular
> > > optimization. I think we do null-pointer-check-then-pfree many place.
> > > It seems to me that checking the array length before memcpy is more
> > > natural than checking both the array length and the array existence
> > > before pfree.
> >
> > Anyway according to commit message of 46ab07ffda, POSIX forbits
> > memcpy(NULL, NULL, 0). It seems to me that it is the cause of the
> > false (or over) optimization. So if we add some comment, it would be
> > for memcpy, not pfree..
>
> For clarilty, I meant that I don't think we need that comment.
>
Fair enough. I think commit 46ab07ffda clearly explains why it is a
good idea to add a check as Sawada-San did in his latest version. I
also agree that we don't any comment for this change.
--
With Regards,
Amit Kapila.
view thread (136+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
In-Reply-To: <CAA4eK1+QjoDLcPi0gZBQLCar47A-ya_kx_gvi+UqiWiAnpckxQ@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox