On Fri, Dec 4, 2015 at 10:34 AM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> On Thu, Dec 3, 2015 at 1:10 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
>> I'll need to think about a fix.
>
> The problem was with the pointer we pass to ExecUpdate().
>
> It's a pointer to the target tuple in shared memory. So the field
> "tuple.t_data->t_ctid" within ExecOnConflictUpdate() starts out
> pointing to an ItemPointerData with the correct ctid (when it
> initially points to the current/target tuple, since as an
> about-to-be-upserted tuple the "t_ctid" field must be a pointer to the
> self-same tuple). Then, it is modified in-place in shared memory by
> heap_update(), within its critical section.
>
> The fix is to take a deep copy (pass a pointer to an ItemPointerData
> on the stack), as in the attached. I've also fixed up the tests, which
> should have caught this, but didn't. Mea culpa.
>
> Many thanks to Stanislav for the report! While I didn't adopt his
> suggestion, he certainly almost had it right.
Cool, that looks right and there are no assertion problems. In the
case of a plain UPDATE the old tuple referenced in both BEFORE/AFTER
triggers is the same, so things are indeed consistent this way
(attached is simple test case I played with to check manually the
patch).
--
Michael