Andres Freund <and...@2ndquadrant.com> wrote: > On 2014-06-09 11:24:22 -0700, Kevin Grittner wrote:
>> The only way that it could be a problem is if the DELETE is in a >> subtransaction which might get rolled back without rolling back the >> INSERT. > > The way I understand the code in that case the subxid in xmax would have > been resolved the toplevel xid. > > /* > * Find top level xid. Bail out if xid is too early to be a conflict, or > * if it's our own xid. > */ > if (TransactionIdEquals(xid, GetTopTransactionIdIfAny())) > return; > xid = SubTransGetTopmostTransaction(xid); > if (TransactionIdPrecedes(xid, TransactionXmin)) > return; > if (TransactionIdEquals(xid, GetTopTransactionIdIfAny())) > return; > > That should essentially make that case harmless, right? Hmm. Since the switch statement above doesn't limit the HEAPTUPLE_DELETE_IN_PROGRESS or HEAPTUPLE_INSERT_IN_PROGRESS cases by visibility, we are safe. I had been thinking that we had early returns based on visibility, like the we do for HEAPTUPLE_LIVE and HEAPTUPLE_RECENTLY_DEAD. Since we don't do that, there is no problem with the code either before or after your recent change. Apologies that I didn't notice that sooner. It might be possible that with more guarantees of what those return codes mean (possibly by adding the new one mentioned by Tom) we could add that early return in one or both cases, which would better optimize some corner cases involving subtransactions. But now doesn't seem like a good time to worry about that. Such a micro-optimization would not be a sane candidate for back-patching, or for introducing this late in a release cycle. So, nothing to see here, folks. Move along. predicate.c is neutral in this matter. Good spot, BTW! -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
