Andres Freund <and...@2ndquadrant.com> wrote: > On 2014-06-09 08:00:52 -0700, Kevin Grittner wrote:
> I tried to make things a bit clearer there - but I am not sure I've > succeed. I'm certainly willing to explain things further if you can tell > me which are is unclear. Thanks! IMO, something like this should be included in the code comments. > HEAPTUPLE_INSERT_IN_PROGRESS, /* inserting xact is still in progress */ > HEAPTUPLE_DELETE_IN_PROGRESS /* deleting xact is still in progress */ > the current code will return INSERT_IN_PROGRESS even if the tuple has > *also* been deleted in another xact... > I think the problem here is that there's simply no way to really > represent that case accurately with the current API. For purposes of predicate.c, if the "also deleted" activity might be rolled back without rolling back the insert, INSERT_IN_PROGRESS is the only correct value. If they will either both commit or neither will commit, predicate.c would be more efficient if HEAPTUPLE_RECENTLY_DEAD was returned, but I HEAPTUPLE_INSERT_IN_PROGRESS would be OK from a correctness PoV. >> Perhaps it would be good if you could provide a concise description >> of the conditions under which value could currently be returned on >> this (or the related) thread before we talk about what changes >> might be needed? Maybe this is clear to others involved in the >> discussion, but I am not confident that I fully understand what >> gets returned under what conditions. > > HEAPTUPLE_DEAD, /* tuple is dead and deletable */ > 1) xmin has committed, xmax has committed and wasn't just a locker. Xmax > precedes OldestXmin. Perfect. > HEAPTUPLE_LIVE, /* tuple is live (committed, no deleter) */ > 1) xmin has committed, xmax unset > 2) xmin has committed, xmax is locked only. Status of xmax is irrelevant > 3) xmin has committed, xmax has aborted. Perfect. > HEAPTUPLE_RECENTLY_DEAD, /* tuple is dead, but not deletable yet */ > 1) xmin has committed, xmax has committed and wasn't only a locker. But > xmax doesn't precede OldestXmin. For my purposes, it would be better if this also included: 2) xmin is in progress, xmax matches (or includes) xmin ... but that would be only a performance tweak. > HEAPTUPLE_INSERT_IN_PROGRESS, /* inserting xact is still in >progress > */ > new: > 1) xmin is in progress, xmin is the current backend, xmax is invalid > 2) xmin is in progress, xmin is the current backend, xmax only a locker > 3) xmin is in progress, xmin is the current backend, xmax aborted > 4) xmin is in progress, xmin is *not* current backend, xmax is irrelevant > old: > 1) xmin is in progress, xmax is invalid > 2) xmin is in progress, xmax is only a locker I think this is OK from a correctness PoV. There may be an opportunity to optimize. I will look more closely at whether it seems likely to matter much, and what sort of change in the return conditions or in predicate.c might be needed. > HEAPTUPLE_DELETE_IN_PROGRESS /* deleting xact is still in progress */ > new: > 1) xmin has committed, xmax is in progress, xmax is not just a locker > 2) xmin is in progress, xmin is the current backend, xmax is not just a > locker and in progress. I'm not clear on how 2) could happen unless xmax is the current backend or a subtransaction thereof. Could you clarify? > old: > 1) xmin has committed, xmax is in progress, xmax is not just a locker > 2) xmin is in progress, xmax is set and not not just a locker > > Note that the 2) case here never checked xmax's status. Again, I'm not sure how 2) could happen unless they involve the same top-level transaction. What am I missing? -- 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
