8000 Fix deletion of speculatively inserted TOAST on conflict · patchsoft/postgres@94bc307 · GitHub
[go: up one dir, main page]
Skip to content

Commit 94bc307

Browse files
anarazelanarazel
committed
Fix deletion of speculatively inserted TOAST on conflict
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
1 parent de396a1 commit 94bc307

File tree

4 files changed

+20
-13
lines changed

4 files changed

+20
-13
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3112,7 +3112,7 @@ heap_delete(Relation relation, ItemPointer tid,
31123112
Assert(!HeapTupleHasExternal(&tp));
31133113
}
31143114
else if (HeapTupleHasExternal(&tp))
3115-
toast_delete(relation, &tp);
3115+
toast_delete(relation, &tp, false);
31163116

31173117
/*
31183118
* Mark tuple for invalidation from system caches at next command
@@ -5723,7 +5723,8 @@ heap_finish_speculative(Relation relation, HeapTuple tuple)
57235723
* could deadlock with each other, which would not be acceptable.
57245724
*
57255725
* This is somewhat redundant with heap_delete, but we prefer to have a
5726-
* dedicated routine with stripped down requirements.
5726+
* dedicated routine with stripped down requirements. Note that this is also
5727+
* used to delete the TOAST tuples created during speculative insertion.
57275728
*
57285729
* This routine does not affect logical decoding as it only looks at
57295730
* confirmation records.
@@ -5767,7 +5768,7 @@ heap_abort_speculative(Relation relation, HeapTuple tuple)
57675768
*/
57685769
if (tp.t_data->t_choice.t_heap.t_xmin != xid)
57695770
elog(ERROR, "attempted to kill a tuple inserted by another transaction");
5770-
if (!HeapTupleHeaderIsSpeculative(tp.t_data))
5771+
if (!(IsToastRelation(relation) || HeapTupleHeaderIsSpeculative(tp.t_data)))
57715772
elog(ERROR, "attempted to kill a non-speculative tuple");
57725773
Assert(!HeapTupleHeaderIsHeapOnly(tp.t_data));
57735774

@@ -5837,7 +5838,10 @@ heap_abort_speculative(Relation relation, HeapTuple tuple)
58375838
LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
58385839

58395840
if (HeapTupleHasExternal(&tp))
5840-
toast_delete(relation, &tp);
5841+
{
5842+
Assert(!IsToastRelation(relation));
5843+
toast_delete(relation, &tp, true);
5844+
}
58415845

58425846
/*
58435847
* Never need to mark tuple for invalidation, since catalogs don't support

‎src/backend/access/heap/tuptoaster.c

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ typedef struct toast_compress_header
6666
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
6767
(((toast_compress_header *) (ptr))->rawsize = (len))
6868

69-
static void toast_delete_datum(Relation rel, Datum value);
69+
static void toast_delete_datum(Relation rel, Datum value, bool is_speculative);
7070
static Datum toast_save_datum(Relation rel, Datum value,
7171
struct varlena * oldexternal, int options);
7272
static bool toastrel_valueid_exists(Relation toastrel, Oid valueid);
@@ -459,7 +459,7 @@ toast_datum_size(Datum value)
459459
* ----------
460460
*/
461461
void
462-
toast_delete(Relation rel, HeapTuple oldtup)
462+
toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative)
463463
{
464464
TupleDesc tupleDesc;
465465
Form_pg_attribute *att;
@@ -506,7 +506,7 @@ toast_delete(Relation rel, HeapTuple oldtup)
506506
if (toast_isnull[i])
507507
continue;
508508
else if (VARATT_IS_EXTERNAL_ONDISK(PointerGetDatum(value)))
509-
toast_delete_datum(rel, value);
509+
toast_delete_datum(rel, value, is_speculative);
510510
}
511511
}
512512
}
@@ -1062,7 +1062,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
10621062
if (need_delold)
10631063
for (i = 0; i < numAttrs; i++)
10641064
if (toast_delold[i])
1065-
toast_delete_datum(rel, toast_oldvalues[i]);
1065+
toast_delete_datum(rel, toast_oldvalues[i], false);
10661066

10671067
return result_tuple;
10681068
}
@@ -1654,7 +1654,7 @@ toast_save_datum(Relation rel, Datum value,
16541654
* ----------
16551655
*/
16561656
static void
1657-
toast_delete_datum(Relation rel, Datum value)
1657+
toast_delete_datum(Relation rel, Datum value, bool is_speculative)
16581658
{
16591659
struct varlena *attr = (struct varlena *) DatumGetPointer(value);
16601660
struct varatt_external toast_pointer;
@@ -1703,7 +1703,10 @@ toast_delete_datum(Relation rel, Datum value)
17031703
/*
17041704
* Have a chunk, delete it
17051705
*/
1706-
simple_heap_delete(toastrel, &toasttup->t_self);
1706+
if (is_speculative)
1707+
heap_abort_speculative(toastrel, toasttup);
1708+
else
1709+
simple_heap_delete(toastrel, &toasttup->t_self);
17071710
}
17081711

17091712
/*

‎src/backend/utils/time/tqual.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -408,8 +408,8 @@ HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot,
408408

409409
/*
410410
* An invalid Xmin can be left behind by a speculative insertion that
411-
* is cancelled by super-deleting the tuple. We shouldn't see any of
412-
* those in TOAST tables, but better safe than sorry.
411+
* is canceled by super-deleting the tuple. This also applies to
412+
* TOAST tuples created during speculative insertion.
413413
*/
414414
else if (!TransactionIdIsValid(HeapTupleHeaderGetXmin(tuple)))
415415
return false;

‎src/include/access/tuptoaster.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ extern HeapTuple toast_insert_or_update(Relation rel,
142142
* Called by heap_delete().
143143
* ----------
144144
*/
145-
extern void toast_delete(Relation rel, HeapTuple oldtup);
145+
extern void toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative);
146146

147147
/* ----------
148148
* heap_tuple_fetch_attr() -

0 commit comments

Comments
 (0)
0