mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-08-28 14:57:20 +02:00
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
This commit is contained in:
parent
8cb23dba8d
commit
e79aaebccd
@ -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
|
||||
|
@ -67,7 +67,7 @@ typedef struct toast_compress_header
|
||||
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
|
||||
(((toast_compress_header *) (ptr))->rawsize = (len))
|
||||
|
||||
static void toast_delete_datum(Relation rel, Datum value);
|
||||
static void toast_delete_datum(Relation rel, Datum value, bool is_speculative);
|
||||
static Datum toast_save_datum(Relation rel, Datum value,
|
||||
struct varlena * oldexternal, int options);
|
||||
static bool toastrel_valueid_exists(Relation toastrel, Oid valueid);
|
||||
@ -461,7 +461,7 @@ toast_datum_size(Datum value)
|
||||
* ----------
|
||||
*/
|
||||
void
|
||||
toast_delete(Relation rel, HeapTuple oldtup)
|
||||
toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative)
|
||||
{
|
||||
TupleDesc tupleDesc;
|
||||
Form_pg_attribute *att;
|
||||
@ -508,7 +508,7 @@ toast_delete(Relation rel, HeapTuple oldtup)
|
||||
if (toast_isnull[i])
|
||||
continue;
|
||||
else if (VARATT_IS_EXTERNAL_ONDISK(PointerGetDatum(value)))
|
||||
toast_delete_datum(rel, value);
|
||||
toast_delete_datum(rel, value, is_speculative);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -1064,7 +1064,7 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
|
||||
if (need_delold)
|
||||
for (i = 0; i < numAttrs; i++)
|
||||
if (toast_delold[i])
|
||||
toast_delete_datum(rel, toast_oldvalues[i]);
|
||||
toast_delete_datum(rel, toast_oldvalues[i], false);
|
||||
|
||||
return result_tuple;
|
||||
}
|
||||
@ -1656,7 +1656,7 @@ toast_save_datum(Relation rel, Datum value,
|
||||
* ----------
|
||||
*/
|
||||
static void
|
||||
toast_delete_datum(Relation rel, Datum value)
|
||||
toast_delete_datum(Relation rel, Datum value, bool is_speculative)
|
||||
{
|
||||
struct varlena *attr = (struct varlena *) DatumGetPointer(value);
|
||||
struct varatt_external toast_pointer;
|
||||
@ -1707,7 +1707,10 @@ toast_delete_datum(Relation rel, Datum value)
|
||||
/*
|
||||
* Have a chunk, delete it
|
||||
*/
|
||||
simple_heap_delete(toastrel, &toasttup->t_self);
|
||||
if (is_speculative)
|
||||
heap_abort_speculative(toastrel, toasttup);
|
||||
else
|
||||
simple_heap_delete(toastrel, &toasttup->t_self);
|
||||
}
|
||||
|
||||
/*
|
||||
|
@ -418,8 +418,8 @@ HeapTupleSatisfiesToast(HeapTuple htup, Snapshot snapshot,
|
||||
|
||||
/*
|
||||
* An invalid Xmin can be left behind by a speculative insertion that
|
||||
* is canceled by super-deleting the tuple. We shouldn't see any of
|
||||
* those in TOAST tables, but better safe than sorry.
|
||||
* is canceled by super-deleting the tuple. This also applies to
|
||||
* TOAST tuples created during speculative insertion.
|
||||
*/
|
||||
else if (!TransactionIdIsValid(HeapTupleHeaderGetXmin(tuple)))
|
||||
return false;
|
||||
|
@ -142,7 +142,7 @@ extern HeapTuple toast_insert_or_update(Relation rel,
|
||||
* Called by heap_delete().
|
||||
* ----------
|
||||
*/
|
||||
extern void toast_delete(Relation rel, HeapTuple oldtup);
|
||||
extern void toast_delete(Relation rel, HeapTuple oldtup, bool is_speculative);
|
||||
|
||||
/* ----------
|
||||
* heap_tuple_fetch_attr() -
|
||||
|
15
src/test/isolation/expected/insert-conflict-toast.out
Normal file
15
src/test/isolation/expected/insert-conflict-toast.out
Normal file
@ -0,0 +1,15 @@
|
||||
Parsed test spec with 3 sessions
|
||||
|
||||
starting permutation: s2insert s3insert s1commit
|
||||
pg_advisory_xact_lock
|
||||
|
||||
|
||||
step s2insert:
|
||||
INSERT INTO ctoast (key, val) VALUES (1, ctoast_large_val()) ON CONFLICT DO NOTHING;
|
||||
<waiting ...>
|
||||
step s3insert:
|
||||
INSERT INTO ctoast (key, val) VALUES (1, ctoast_large_val()) ON CONFLICT DO NOTHING;
|
||||
<waiting ...>
|
||||
step s1commit: COMMIT;
|
||||
step s2insert: <... completed>
|
||||
step s3insert: <... completed>
|
@ -28,6 +28,7 @@ test: insert-conflict-do-nothing
|
||||
test: insert-conflict-do-update
|
||||
test: insert-conflict-do-update-2
|
||||
test: insert-conflict-do-update-3
|
||||
test: insert-conflict-toast
|
||||
test: delete-abort-savept
|
||||
test: delete-abort-savept-2
|
||||
test: aborted-keyrevoke
|
||||
|
51
src/test/isolation/specs/insert-conflict-toast.spec
Normal file
51
src/test/isolation/specs/insert-conflict-toast.spec
Normal file
@ -0,0 +1,51 @@
|
||||
# INSERT...ON CONFLICT test on table with TOAST
|
||||
#
|
||||
# This test verifies that speculatively inserted toast rows do not
|
||||
# cause conflicts. It does so by using expression index over a
|
||||
# function which acquires an advisory lock, triggering two index
|
||||
# insertions to happen almost at the same time. This is not guaranteed
|
||||
# to lead to a failed speculative insertion, but makes one quite
|
||||
# likely.
|
||||
|
||||
setup
|
||||
{
|
||||
CREATE TABLE ctoast (key int primary key, val text);
|
||||
CREATE OR REPLACE FUNCTION ctoast_lock_func(int) RETURNS INT IMMUTABLE LANGUAGE SQL AS 'select pg_advisory_xact_lock_shared(1); select $1;';
|
||||
CREATE OR REPLACE FUNCTION ctoast_large_val() RETURNS TEXT LANGUAGE SQL AS 'select array_agg(md5(g::text))::text from generate_series(1, 256) g';
|
||||
CREATE UNIQUE INDEX ctoast_lock_idx ON ctoast (ctoast_lock_func(key));
|
||||
}
|
||||
|
||||
teardown
|
||||
{
|
||||
DROP TABLE ctoast;
|
||||
DROP FUNCTION ctoast_lock_func(int);
|
||||
DROP FUNCTION ctoast_large_val();
|
||||
}
|
||||
|
||||
session "s1"
|
||||
setup
|
||||
{
|
||||
BEGIN ISOLATION LEVEL READ COMMITTED;
|
||||
SELECT pg_advisory_xact_lock(1);
|
||||
}
|
||||
step "s1commit" { COMMIT; }
|
||||
|
||||
session "s2"
|
||||
setup
|
||||
{
|
||||
SET default_transaction_isolation = 'read committed';
|
||||
}
|
||||
step "s2insert" {
|
||||
INSERT INTO ctoast (key, val) VALUES (1, ctoast_large_val()) ON CONFLICT DO NOTHING;
|
||||
}
|
||||
|
||||
session "s3"
|
||||
setup
|
||||
{
|
||||
SET default_transaction_isolation = 'read committed';
|
||||
}
|
||||
step "s3insert" {
|
||||
INSERT INTO ctoast (key, val) VALUES (1, ctoast_large_val()) ON CONFLICT DO NOTHING;
|
||||
}
|
||||
|
||||
permutation "s2insert" "s3insert" "s1commit"
|
Loading…
Reference in New Issue
Block a user