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

On Tue, Jan 25, 2022 at 11:59 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Tue, Jan 25, 2022 at 12:26 AM Euler Taveira <euler(at)eulerto(dot)com> wrote:
> >
>
> I am not sure if your proposal is much different compared to v4 or how
> much it improves the situation? I see you didn't consider
> 'check_external_attr' parameter and I think that is important to know
> if the key has any external toast value. Overall, I see your point
> that the change of APIs looks a bit ugly. But, I guess that is more
> due to their names and current purpose. I think it could be better if
> we bring all the code of heap_tuple_attr_equals in its only caller
> HeapDetermineModifiedColumns or at least part of the code where we get
> attr value and can determine whether the value is stored externally.
> Then change name of HeapDetermineModifiedColumns to
> HeapDetermineColumnsInfo with additional parameters.

I think the best way is to do some refactoring and renaming of the
function, because as part of HeapDetermineModifiedColumns we are
already processing the tuple so we can not put extra overhead of
reprocessing it again. In short I like the idea of renaming the
HeapDetermineModifiedColumns and moving part of heap_tuple_attr_equals
code into the caller. Here is the patch set for the same. I have
divided it into two patches which can eventually be merged, 0001- for
refactoring 0002- does the actual work.

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com