On Mon, Jan 24, 2022 at 9:28 AM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Wed, Aug 11, 2021 at 06:14:55PM +0530, Dilip Kumar wrote:
> > Right
>
> Amit, are you planning to look more at this patch? It has been a
> couple of months since the last update, and this is still a bug as far
> as I understand.
>
> FWIW, I find the API changes of HeapDetermineModifiedColumns() and
> ExtractReplicaIdentity() a bit grotty. Shouldn't we try to flatten
> the old tuple instead? There are things like
> toast_flatten_tuple_to_datum() for this purpose if a tuple satisfies
> HeapTupleHasExternal(), or just heap_copy_tuple_as_datum().
>
That can add overhead in cases where we don't need to log the toasted
values of the old tuple. We only need it for the case where we have
unchanged toasted replica identity columns. In the previous version
[1], we were doing something like you are suggesting and that seems to
have overhead as explained in the second paragraph of the email [2].
Also, Alvaro seems to have some reservations about that change. I
don't know if there is a better way to fix this but I could be missing
something.
[1] - https://www.postgresql.org/message-id/CAFiTN-sTS4bB7W3UJV3iUm%3DwKdr9EpOwyK97hNr77MzFQm_NBw%40mail.gmail.com
[2] - https://www.postgresql.org/message-id/CAA4eK1KgZr%3DQSBE_Qh0Qfb2ma1Tc6%2BZxkMaUHO7aC7b9WSCRaw%40mail.gmail.com
--
With Regards,
Amit Kapila.