Re: [BUG] Failed Assertion in ReorderBufferChangeMemoryUpdate()

On Mon, Sep 6, 2021 at 9:14 PM Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Mon, Sep 6, 2021 at 8:54 PM Drouvot, Bertrand <bdrouvot@amazon.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()?

> Yet another option could be to create a new API say ReorderBufferReplaceChangeMemoryUpdate(), which takes, 2 parameters, oldchange, and newchange as inputs, it will compute the difference and add/subtract that size.  Logically, that is what we are actually trying to do right? i.e. replacing old change with the new change.
>

Note that in ReorderBufferToastReplace(), the new tuple is replaced in
change so by the time we want to do this computation oldchange won't
be preserved.