On Thu, Oct 13, 2016 at 8:45 AM, Kevin Grittner <kgrittn@xxxxxxxxx> wrote: > On Wed, Oct 12, 2016 at 10:06 AM, Kevin Grittner <kgrittn@xxxxxxxxx> 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@xxxxxxxxxxxxxx) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-general
