database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-07-05 07:21:24 +03:00
Code Activity

Fix deletion of speculatively inserted TOAST on conflict

Browse Source 
INSERT ..  ON CONFLICT runs a pre-check of the possible conflicting
constraints before performing the actual speculative insertion.  In case
the inserted tuple included TOASTed columns the ON CONFLICT condition
would be handled correctly in case the conflict was caught by the
pre-check, but if two transactions entered the speculative insertion
phase at the same time, one would have to re-try, and the code for
aborting a speculative insertion did not handle deleting the
speculatively inserted TOAST datums correctly.

TOAST deletion would fail with "ERROR: attempted to delete invisible
tuple" as we attempted to remove the TOAST tuples using
simple_heap_delete which reasoned that the given tuples should not be
visible to the command that wrote them.

This commit updates the heap_abort_speculative() function which aborts
the conflicting tuple to use itself, via toast_delete, for deleting
associated TOAST datums.  Like before, the inserted toast rows are not
marked as being speculative.

This commit also adds a isolationtester spec test, exercising the
relevant code path. Unfortunately 9.5 cannot handle two waiting
sessions, and thus cannot execute this test.

Reported-By: Viren Negi, Oskari Saarenmaa
Author: Oskari Saarenmaa, edited a bit by me
Bug: #14150
Discussion: <20160519123338.12513.20271@wrigleys.postgresql.org>
Backpatch: 9.5, where ON CONFLICT was introduced
This commit is contained in:
Andres Freund
2016-08-17 17:03:36 -07:00
parent cf9b0fea5f
commit 07ef035129
7 changed files with 87 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/backend/access/heap/heapam.c
View File

@ -3335,7 +3335,7 @@ l1:
Assert(!HeapTupleHasExternal(&tp));
}
else if (HeapTupleHasExternal(&tp))
toast_delete(relation, &tp);
toast_delete(relation, &tp, false);
/*
* Mark tuple for invalidation from system caches at next command
@ -6057,7 +6057,8 @@ heap_finish_speculative(Relation relation, HeapTuple tuple)
* could deadlock with each other, which would not be acceptable.
*
* This is somewhat redundant with heap_delete, but we prefer to have a
* dedicated routine with stripped down requirements.
* dedicated routine with stripped down requirements. Note that this is also
* used to delete the TOAST tuples created during speculative insertion.
*
* This routine does not affect logical decoding as it only looks at
* confirmation records.
@ -6101,7 +6102,7 @@ heap_abort_speculative(Relation relation, HeapTuple tuple)
*/
if (tp.t_data->t_choice.t_heap.t_xmin != xid)
elog(ERROR, "attempted to kill a tuple inserted by another transaction");
if (!HeapTupleHeaderIsSpeculative(tp.t_data))
if (!(IsToastRelation(relation) || HeapTupleHeaderIsSpeculative(tp.t_data)))
elog(ERROR, "attempted to kill a non-speculative tuple");
Assert(!HeapTupleHeaderIsHeapOnly(tp.t_data));
@ -6171,7 +6172,10 @@ heap_abort_speculative(Relation relation, HeapTuple tuple)
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
if (HeapTupleHasExternal(&tp))
toast_delete(relation, &tp);
{
Assert(!IsToastRelation(relation));
toast_delete(relation, &tp, true);
}
/*
* Never need to mark tuple for invalidation, since catalogs don't support