On Tue, Sep 7, 2021 at 11:08 AM Dilip Kumar <dilipbal...@gmail.com> wrote: > > On Tue, Sep 7, 2021 at 8:38 AM Amit Kapila <amit.kapil...@gmail.com> wrote: >> >> On Mon, Sep 6, 2021 at 9:14 PM Dilip Kumar <dilipbal...@gmail.com> wrote: >> > >> > On Mon, Sep 6, 2021 at 8:54 PM Drouvot, Bertrand <bdrou...@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()? > > > 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.
