Hi,
On 9/7/21 7:58 AM, Dilip Kumar wrote:
>> 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?
Yeah right, we can actually do that, it doesn't matter even if we are passing the size from outside.
Agree, if no objections, I'll prepare a patch with the modified approach of option 2) proposed by Amit (means passing the size from the outside in all the cases).
Thanks
Bertrand
