On Thu, Jun 11, 2020 at 1:10 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is v3, which has an isolationtester test.
Nice repro. We have:
foo happened before bar, because it can't see bar_insert
bar happened before foo, because it can't see foo_insert (!)
Confirmed here that this anomaly is missed in master and detected with
the patch as shown, and check-world passes.
Explaining it to myself like I'm five:
The tuple foo_insert was concurrently inserted *and* deleted/updated.
Our coding screwed that case up. Your fix teaches the
HEAPTUPLE_DELETE_IN_PROGRESS case to check if it's also an insert we
can't see, and then handles it the same as non-visible HEAPTUPLE_LIVE
or HEAPTUPLE_INSERT_IN_PROGRESS. That is, by taking the xmin
(inserter), not the xmax (updater/deleter) for conflict-out checking.
(As mentioned on IM, I wondered for a bit why it's OK that we now lack
a check for conflict out to the deleter in the insert+delete case, but
I think that's OK because it certainly happened after the insert
committed.)
Then you realised that HEAPTUPLE_RECENTLY_DEAD has no reason to be
treated differently.
Minor nits (already mentioned via IM but FTR): bar_first isn't
necessary in the test, s/foo-insert/foo_insert/
Looks pretty good to me. Tricky problem, really excellent work.