I wrote:
> Andres Freund <andres@xxxxxxxxxxx> writes:
>> Cool. And damn: I can't immediately think of a way to optimize this to
>> not require this kind of hack in the future.
> Maybe the same thing we do with user tables, ie not give up the lock
> when we close a toast rel? As long as the internal lock counters
> are 64-bit, we'd not have to worry about overflowing them.
Like this? This passes check-world, modulo the one very-unsurprising
regression test change. I've not tried to do any performance testing.
regards, tom lane
diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
index 545a6b8867..3826b4d93e 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -367,7 +367,7 @@ toast_fetch_datum(struct varlena *attr)
* case. */
/*
- * Open the toast relation and its indexes
+ * Open the toast relation (but its indexes are dealt with by the AM)
*/
toastrel = table_open(toast_pointer.va_toastrelid, AccessShareLock);
@@ -376,7 +376,7 @@ toast_fetch_datum(struct varlena *attr)
attrsize, 0, attrsize, result);
/* Close toast table */
- table_close(toastrel, AccessShareLock);
+ table_close(toastrel, NoLock);
return result;
}
@@ -457,7 +457,7 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset,
result);
/* Close toast table */
- table_close(toastrel, AccessShareLock);
+ table_close(toastrel, NoLock);
return result;
}
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index 730cd04a2d..f0d0251ce0 100644
--- a/src/backend/access/common/toast_internals.c
+++ b/src/backend/access/common/toast_internals.c
@@ -27,7 +27,7 @@
#include "utils/rel.h"
#include "utils/snapmgr.h"
-static bool toastrel_valueid_exists(Relation toastrel, Oid valueid);
+static bool toastrel_valueid_exists(Relation toastrel, Oid valueid, LOCKMODE lock);
static bool toastid_valueid_exists(Oid toastrelid, Oid valueid);
/* ----------
@@ -264,7 +264,8 @@ toast_save_datum(Relation rel, Datum value,
* be reclaimed by VACUUM.
*/
if (toastrel_valueid_exists(toastrel,
- toast_pointer.va_valueid))
+ toast_pointer.va_valueid,
+ RowExclusiveLock))
{
/* Match, so short-circuit the data storage loop below */
data_todo = 0;
@@ -359,8 +360,8 @@ toast_save_datum(Relation rel, Datum value,
/*
* Done - close toast relation and its indexes
*/
- toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
- table_close(toastrel, RowExclusiveLock);
+ toast_close_indexes(toastidxs, num_indexes, NoLock);
+ table_close(toastrel, NoLock);
/*
* Create the TOAST pointer value that we'll return
@@ -440,8 +441,8 @@ toast_delete_datum(Relation rel, Datum value, bool is_speculative)
* End scan and close relations
*/
systable_endscan_ordered(toastscan);
- toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
- table_close(toastrel, RowExclusiveLock);
+ toast_close_indexes(toastidxs, num_indexes, NoLock);
+ table_close(toastrel, NoLock);
}
/* ----------
@@ -453,7 +454,7 @@ toast_delete_datum(Relation rel, Datum value, bool is_speculative)
* ----------
*/
static bool
-toastrel_valueid_exists(Relation toastrel, Oid valueid)
+toastrel_valueid_exists(Relation toastrel, Oid valueid, LOCKMODE lock)
{
bool result = false;
ScanKeyData toastkey;
@@ -464,7 +465,7 @@ toastrel_valueid_exists(Relation toastrel, Oid valueid)
/* Fetch a valid index relation */
validIndex = toast_open_indexes(toastrel,
- RowExclusiveLock,
+ lock,
&toastidxs,
&num_indexes);
@@ -489,7 +490,7 @@ toastrel_valueid_exists(Relation toastrel, Oid valueid)
systable_endscan(toastscan);
/* Clean up */
- toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
+ toast_close_indexes(toastidxs, num_indexes, NoLock);
return result;
}
@@ -508,9 +509,9 @@ toastid_valueid_exists(Oid toastrelid, Oid valueid)
toastrel = table_open(toastrelid, AccessShareLock);
- result = toastrel_valueid_exists(toastrel, valueid);
+ result = toastrel_valueid_exists(toastrel, valueid, AccessShareLock);
- table_close(toastrel, AccessShareLock);
+ table_close(toastrel, NoLock);
return result;
}
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c
index 55bbe1d584..c3bfb97c62 100644
--- a/src/backend/access/heap/heaptoast.c
+++ b/src/backend/access/heap/heaptoast.c
@@ -789,5 +789,5 @@ heap_fetch_toast_slice(Relation toastrel, Oid valueid, int32 attrsize,
/* End scan and close indexes. */
systable_endscan_ordered(toastscan);
- toast_close_indexes(toastidxs, num_indexes, AccessShareLock);
+ toast_close_indexes(toastidxs, num_indexes, NoLock);
}
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ec14b060a6..b7a007904e 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2610,7 +2610,9 @@ select * from my_locks order by 1;
relname | max_lockmode
-----------+--------------------------
alterlock | ShareUpdateExclusiveLock
-(1 row)
+ pg_toast | AccessShareLock
+ pg_toast | AccessShareLock
+(3 rows)
rollback;
begin; alter table alterlock cluster on alterlock_pkey;
