Date: Tue, 19 Jul 2022 17:13:08 +0900 (JST)
Message-Id: <20220719.171308.785327364847593895.horikyota.ntt@gmail.com>
To: sawada.mshk@gmail.com
Cc: amit.kapila16@gmail.com, osumi.takamichi@fujitsu.com,
 bdrouvot@amazon.com, pgsql-hackers@lists.postgresql.org, minsoo@amazon.com
Subject: Re: [BUG] Logical replication failure "ERROR: could not map
 filenode "base/13237/442428" to relation OID" with catalog modifying txns
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <20220719.165714.766068228275757655.horikyota.ntt@gmail.com>
References: 
 <CAA4eK1JWaY5t+20iteUnPZy0E+nJi4KPqH-wEYyKx7YkcHmEqA@mail.gmail.com>
	<CAD21AoC1E09F_swzOPQQE9P4tf3ADr0cyATesGHA3JrySAytNA@mail.gmail.com>
	<20220719.165714.766068228275757655.horikyota.ntt@gmail.com>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/20220719.171308.785327364847593895.horikyota.ntt%40gmail.com>
Precedence: bulk

At Tue, 19 Jul 2022 16:57:14 +0900 (JST), Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote in 
> At Tue, 19 Jul 2022 16:02:26 +0900, Masahiko Sawada <sawada.mshk@gmail.com> wrote in 
> > On Tue, Jul 19, 2022 at 1:47 PM Amit Kapila <amit.kapila16@gmail.com> 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.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center