On Thu, Oct 13, 2016 at 8:45 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Wed, Oct 12, 2016 at 10:06 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
>
>> The test in ExecCheckHeapTupleVisible() seems wrong to me. It's
>> not immediately obvious what the proper fix is.
>
> 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!
Here's an isolation test that shows the distinction between a
transaction that reports a serialization failure because it crashed
into its own invisible tuples, and one that reports a serialization
failure because it crashed into a concurrent transaction's invisible
tuples. Surely Peter intended the latter to report an error, but the
former seems like an oversight.
Here's a patch that shows one way to fix it. I think it does make
sense to change this, because otherwise automatic
retry-on-serialization-failure strategies will be befuddle by this
doomed transaction. And as you and Vitaly have said, there is
literally no concurrent update.
--
Thomas Munro
http://www.enterprisedb.com