Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()

On Tue, Sep 7, 2021 at 11:08 AM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Tue, Sep 7, 2021 at 8:38 AM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>
>> On Mon, Sep 6, 2021 at 9:14 PM Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>> >
>> > On Mon, Sep 6, 2021 at 8:54 PM Drouvot, Bertrand <bdrouvot(at)amazon(dot)com> wrote:
>> >>
>> >> Thanks for your feedback!
>> >>
>> >> That seems indeed more logical, so I see 3 options to do so:
>> >>
>> >> 1) Add a new API say ReorderBufferChangeMemorySubstractSize() (with a Size as one parameter) and make use of it in ReorderBufferToastReplace()
>> >>
>> >> 2) Add a new "Size" parameter to ReorderBufferChangeMemoryUpdate(), so that if this parameter is > 0 then it would be used instead of "sz = ReorderBufferChangeSize(change)"
>> >>
>> >> 3) Do the substraction directly into ReorderBufferToastReplace() without any API
>> >>
>> >> I'm inclined to go for option 2), what do you think?
>> >
>>
>> Isn't it better if we use option 2) at all places as then we won't
>> need any special check inside ReorderBufferChangeMemoryUpdate()?
>
>
> If we want to do this then be careful about REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID change. Basically, ReorderBufferChangeMemoryUpdate() ignores this type of change whereas ReorderBufferChangeSize(), consider at least sizeof(ReorderBufferChange) bytes to this change. So if we compute the size using ReorderBufferChangeSize() outside of ReorderBufferChangeMemoryUpdate(), then total size will be different from what we have now. Logically, we should be ignoring/asserting REORDER_BUFFER_CHANGE_INTERNAL_TUPLECID in ReorderBufferChangeSize(), because ReorderBufferChangeMemoryUpdate() is the only caller for this function.
>

Why can't we simply ignore it in ReorderBufferChangeMemoryUpdate() as
we are doing now?

--
With Regards,
Amit Kapila.