diff options
| -rw-r--r-- | src/backend/access/heap/heapam.c | 12 | ||||
| -rw-r--r-- | src/backend/access/heap/tuptoaster.c | 15 | ||||
| -rw-r--r-- | src/backend/utils/time/tqual.c | 4 | ||||
| -rw-r--r-- | src/include/access/tuptoaster.h | 2 | ||||
| -rw-r--r-- | src/test/isolation/expected/insert-conflict-toast.out | 15 | ||||
| -rw-r--r-- | src/test/isolation/isolation_schedule | 1 | ||||
| -rw-r--r-- | src/test/isolation/specs/insert-conflict-toast.spec | 51 |
7 files changed, 87 insertions, 13 deletions
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 24bd9be5e17..c63dfa0bafc 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -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 diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c index bbb26493717..fc4702ce6f3 100644 --- a/src/backend/access/heap/tuptoaster.c +++ b/src/backend/access/heap/tuptoaster.c @@ -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); } /* diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c index d99c847000e..26a3be3a618 100644 --- a/src/backend/utils/time/tqual.c +++ b/src/backend/utils/time/tqual.c @@ -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; diff --git a/src/include/access/tuptoaster.h b/src/include/access/tuptoaster.h index 7b5ae6245e6..011f5c19062 100644 --- a/src/include/access/tuptoaster.h +++ b/src/include/access/tuptoaster.h @@ -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() - diff --git a/src/test/isolation/expected/insert-conflict-toast.out b/src/test/isolation/expected/insert-conflict-toast.out new file mode 100644 index 00000000000..e86b98020f3 --- /dev/null +++ b/src/test/isolation/expected/insert-conflict-toast.out @@ -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> diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule index 87ab7742fc8..a96a3189871 100644 --- a/src/test/isolation/isolation_schedule +++ b/src/test/isolation/isolation_schedule @@ -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 diff --git a/src/test/isolation/specs/insert-conflict-toast.spec b/src/test/isolation/specs/insert-conflict-toast.spec new file mode 100644 index 00000000000..c5e39ef9e31 --- /dev/null +++ b/src/test/isolation/specs/insert-conflict-toast.spec @@ -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" |