On Wed, Oct 12, 2016 at 12:45 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
>> The test in ExecCheckHeapTupleVisible() seems wrong to me. It's
>> not immediately obvious what the proper fix is.
(prune old e-mail address from cc list)
I agree that the multi-value case is a bug.
> To identify what cases ExecCheckHeapTupleVisible() was meant to
> cover I commented out the body of the function to see which
> regression tests failed. None did. The failures shown on this
> thread are fixed by doing so. If there is really a need for this
> function, speak up now and provide a test case showing what is
> broken without it; otherwise if I can't find some justification for
> this function I will rip it (and the calls to it) out of the code.
> If you do have some test case showing what breaks without the
> function, let's get it added to the regression tests!
I am independently working on a bug to fix to
ExecCheckHeapTupleVisible(), concerning the fact that
HeapTupleSatisfiesMVCC() can be called without even a shared buffer
lock held (only a buffer pin) [1]. So, anything that we do here should
probably take care of that, while we're at it.
I think that it should be pretty obvious to you why the check exists
at all, Kevin. It exists because it would be improper to decide to
take the DO NOTHING path on the basis of some other committed tuple
existing that is not visible to the original MVCC snapshot, at higher
isolation levels. There is a similar consideration for DO UPDATE. I'm
slightly surprised that you're contemplating just ripping the check
out. Did I miss something?
[1] https://www.postgresql.org/message-id/57EE93C8.8080504@postgrespro.ru
--
Peter Geoghegan