Re: [BUG]Update Toast data failure in logical replication

On Thu, Jul 22, 2021 at 8:02 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Thu, Jul 22, 2021 at 4:11 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Jun 3, 2021 at 5:15 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > >
> > > On Wed, Jun 2, 2021 at 7:23 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > >
> > > > On Wed, Jun 2, 2021 at 7:20 PM Kuntal Ghosh <kuntalghosh(dot)2007(at)gmail(dot)com> wrote:
> > > > >
> > > > > On Wed, Jun 2, 2021 at 3:10 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> > > > > >
> > > > > > Yes, you are right. I will change it in the next version, along with
> > > > > > the test case.
> > > > > >
> > > > > + /*
> > > > > + * if the key hasn't changed and we're only logging the key, we're done.
> > > > > + * But if tuple has external data then we might have to detoast the key.
> > > > > + */
> > > > > This doesn't really mention why we need to detoast the key even when
> > > > > the key remains the same. I guess we can add some more details here.
> > > >
> > > > Noted, let's see what others have to say about fixing this, then I
> > > > will fix this along with one other pending comment and I will also add
> > > > the test case. Thanks for looking into this.
> > >
> > > I have fixed all the pending issues, I see there is already a test
> > > case for this so I have changed the output for that.
> > >
> >
> > IIUC, this issue occurs because we don't log the actual key value for
> > unchanged toast key. It is neither logged as part of old_key_tuple nor
> > for new tuple due to which we are not able to find it in the
> > apply-side when we searched it via FindReplTupleInLocalRel. Now, I
> > think it will work if we LOG the entire unchanged toasted value as you
> > have done in the patch but can we explore some other way to fix it. In
> > the subscriber-side, can we detect that the key column has toasted
> > value in the new tuple and try to first fetch it and then do the index
> > search for the fetched toasted value? I am not sure about the
> > feasibility of this but wanted to see if we can someway avoid logging
> > unchanged toasted key value as that might save us from additional WAL.
>
> Yeah if we can do this then it will be a better approach, I think as
> you mentioned we can avoid logging unchanged toast key data. I will
> investigate this next week and update the thread.
>

Okay, thanks. I think one point we need to consider here is that on
the subscriber side, we use dirtysnapshot to search the key, so we
need to ensure that we don't fetch the wrong data. I am not sure what
will happen when by the time we try to search the tuple in the
subscriber-side for the update, it has been removed and re-inserted
with the same values by the user. Do we find the newly inserted tuple
and update it? If so, can it also happen even if logged the unchanged
old_key_tuple as the patch is doing currently?

--
With Regards,
Amit Kapila.