Re: Logical replication can lose an update after concurrent index invalidation

On Fri, 18 Sept 2026 at 18:25, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Mon, Sep 7, 2026 at 9:04 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Thu, 3 Sept 2026 at 16:15, Mihail Nikalayeu
> > <mihailnikalayeu(at)gmail(dot)com> wrote:
> > >
> > > Zhijie, Amit, thanks for the reviews!
> > >
> > > > we shall mention in the comments atop the old function that it should
> > > > not be used in new code anymore
> > >
> > > Done.
> > >
> >
> > Couple of minor comments:
> > 1) I was able to compile without this header inclusion:
> > --- a/src/backend/replication/logical/worker.c
> > +++ b/src/backend/replication/logical/worker.c
> > @@ -249,6 +249,7 @@
> >
> > #include "access/genam.h"
> > #include "access/commit_ts.h"
> > +#include "access/htup_details.h"
> > #include "access/table.h"
> >
>
> Fixed in the attached. Apart from this I changed multiple comments to
> make those clear. One notable change is, I moved the newly added
> boolean after localindexoid as it reads better there because then we
> don't need to forward reference the fields. For back-branches, if it
> needs to be moved to an earlier location then we can do that in those
> versions but for HEAD and 19, the new location seems better.

I felt we will have to use the earlier location in case of
back-branches to avoid offset changing.

> Also, shall we keep just one test, say Drop Index Concurrently instead
> of two as both tests do the same thing in a slightly different way? I
> have not done that but if you agree please update the patch
> accordingly.

Here is an updated version including the back-branches version.
This version also addresses Hou-san's agreed 1st comment from [1].

[1] - https://www.postgresql.org/message-id/CAFvd2n-NBbpLM5yhfxLdmpLy5vshpBNqbJ_vX4cj6Z3Wx5BwFw%40mail.gmail.com

Regards,
Vignesh