On Tue, Aug 06, 2019 at 03:08:48PM +0200, Daniel Gustafsson wrote:
> Thanks, this is a much better approach and it passes tests for me. +1 on this
> version (regardless of outcome of the other patch as this is separate).
I had an extra lookup at this stuff this morning, and applied the
patch. Please note that I have kept the assertion on tupledata which
cannot be NULL and added a comment about that because it is not
possible to finish yet in a state where we do not have tuple data in
this context, but it actually could be the case if we begin to use
multi-inserts with system catalogs, so the assertion is here to make
future patch authors careful about that. We could in this case bypass
DecodeMultiInsert() if tupledata is NULL and assert that
XLH_INSERT_CONTAINS_NEW_TUPLE should not be set, or we could just
bypass the logic if XLH_INSERT_CONTAINS_NEW_TUPLE is not set at all.
Let's sort that out in your other patch.
--
Michael