On Tue, Apr 20, 2021 at 9:00 AM osumi.takamichi@fujitsu.com
<osumi.takamichi@fujitsu.com> wrote:
>
> On Tuesday, April 20, 2021 10:53 AM Ajin Cherian <itsajin@gmail.com> wrote:
> >
> > I reviewed the patch, ran make check, no issues. One minor comment:
> >
> > Could you add the comment similar to RelationGetIndexAttrBitmap() on why
> > the redo, it's not very obvious to someone reading the code, why we are
> > refetching the index list here.
> >
> > + /* Check if we need to redo */
> >
> > + newindexoidlist = RelationGetIndexList(relation);
> Yeah, makes sense. Fixed.
I don't think here we need to restart to get a stable list of indexes
as we do in RelationGetIndexAttrBitmap. The reason is here we build
the cache entry using a historic snapshot and all the later changes
are absorbed while decoding WAL. I have updated that and modified few
comments in the attached patch. Can you please test this in
clobber_cache_always mode? I think just testing
subscription/t/010_truncate.pl would be sufficient.
Now, this bug exists in prior versions (>= v11) where we have
introduced decoding of Truncate. Do we want to backpatch this? As no
user has reported this till now apart from Tang who I think got it
while doing some other patch testing, we might want to just push it in
HEAD. I am fine either way. Petr, others, do you have any opinion on
this matter?
--
With Regards,
Amit Kapila.
