Re: Remembering bug #6123

"Kevin Grittner" <Kevin(dot)Grittner(at)wicourts(dot)gov> writes:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> 3. I modified heap_lock_tuple to also return a
>> HeapUpdateFailureData, principally on the grounds that its API
>> should be largely parallel to heap_update, but having done that I
>> can't help wondering whether its callers are okay with skipping
>> self-updated tuples. I see that you changed EvalPlanQualFetch,
>> but I'm not entirely sure that that is right; shouldn't it
>> continue to ignore rows modified by the current command itself?

> I made that change when working on the approach where "safe"
> conflicts (like a DELETE from within the BEFORE DELETE trigger) were
> quietly ignored. Without that change, it didn't see the end of the
> ctid chain, and didn't behave correctly. I've been wondering if it
> is still needed. I had been planning to create a test case to try
> to show that it was. Maybe an UPDATE with a join to force multiple
> row updates from the "primary" statement, followed by a triggered
> UPDATE to the row. If that doesn't need the EvalPlanQualFetch
> change, I don't know what would.

The EvalPlanQual code isn't really exercised by any existing regression
tests, because it is only triggered when two sessions concurrently
update the same row, something that we avoid for reproducibility's sake
in the regression tests. I think we could now test it using the
isolation test scaffolding, and maybe it would be a good idea to add
some tests there. I did verify that whether that part of your patch
is included or not makes no difference to any existing test.

regards, tom lane