On Thu, Oct 13, 2016 at 8:45 AM, Kevin Grittner <kgri...@gmail.com> wrote:
> On Wed, Oct 12, 2016 at 10:06 AM, Kevin Grittner <kgri...@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

Attachment: isolation-test.patch
Description: Binary data

Attachment: check-self-inserted.patch
Description: Binary data

-- 
Sent via pgsql-general mailing list (pgsql-general@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-general

Reply via email to