From: Nathan Bossart <nathandbossart@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jonathan S. Katz <jkatz@postgresql.org>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Large expressions in indexes can't be stored (non-TOASTable)
Date: Wed, 16 Oct 2024 10:54:45 -0500
Message-ID: <Zw_hxaDoC3TzQYjj@nathan> (raw)
In-Reply-To: <Zw8V0ClvZbUi2QQl@paquier.xyz>
References: <e012fb69-ca34-7e0e-3a33-b16d9737e526@gmail.com>
<ZvGOPdGAronxOfmf@nathan>
<ZvI-WXcaFXuj1Dvp@paquier.xyz>
<ZvMSUPOqUU-VNADN@nathan>
<ZvOMBhlb5zrBCG5p@paquier.xyz>
<ZwV_DBQa8Qw6kKY2@nathan>
<Zw14zoBCg-zGu1mi@nathan>
<Zw8E73_nCk0DbO18@paquier.xyz>
<Zw8U0YU_fu2pcXs6@nathan>
<Zw8V0ClvZbUi2QQl@paquier.xyz>
On Wed, Oct 16, 2024 at 10:24:32AM +0900, Michael Paquier wrote:
> On Tue, Oct 15, 2024 at 08:20:17PM -0500, Nathan Bossart wrote:
>> I assume all of this will get compiled out in non-USE_ASSERT_CHECKING
>> builds as-is, but I see no problem with surrounding it with an #ifdef to be
>> sure.
>
> Yeah, I'm not sure that that would always be the case when optimized.
> Code generated can be dumb sometimes even if compilers got much
> smarter in the last 10 years or so (not compiler guy here).
Here is a new version of the patch set with this #ifdef added. I plan to
give each of the code paths adjusted by 0001 a closer look, but otherwise
I'm hoping to commit these soon.
--
nathan
Attachments:download all (3 files, .tar.gz)v4-0001-Ensure-we-have-a-snapshot-when-updating-various-s.patch (text/plain, 5.5K) download
From b9444fd22d8dcc05e3a27817943be7f5296503fa Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 14 Oct 2024 14:43:26 -0500
Subject: [PATCH v4 1/3] Ensure we have a snapshot when updating various system
catalogs.
Reported-by: Alexander Lakhin
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/18127-fe54b6a667f29658%40postgresql.org
Discussion: https://postgr.es/m/18309-c0bf914950c46692%40postgresql.org
Discussion: https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan
Backpatch-through: FIXME
---
src/backend/commands/tablecmds.c | 13 +++++++++++
src/backend/replication/logical/tablesync.c | 21 ++++++++++++++++++
src/backend/replication/logical/worker.c | 24 +++++++++++++++++++++
3 files changed, 58 insertions(+)
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex 1ccc80087c..c6f82e93b9 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -19283,9 +19283,22 @@ ATExecDetachPartition(List **wqueue, AlteredTableInfo *tab, Relation rel,
tab->rel = rel;
}
+ /*+ * Detaching the partition might involve TOAST table access, so ensure we+ * have a valid snapshot. We only expect to get here without a snapshot+ * in the concurrent path.+ */+ if (concurrent)+ PushActiveSnapshot(GetTransactionSnapshot());+ else+ Assert(HaveRegisteredOrActiveSnapshot());+
/* Do the final part of detaching */
DetachPartitionFinalize(rel, partRel, concurrent, defaultPartOid);
+ if (concurrent)+ PopActiveSnapshot();+
ObjectAddressSet(address, RelationRelationId, RelationGetRelid(partRel));
/* keep our lock until commit */
diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.cindex e03e761392..d3fbb18438 100644--- a/src/backend/replication/logical/tablesync.c+++ b/src/backend/replication/logical/tablesync.c@@ -377,6 +377,12 @@ process_syncing_tables_for_sync(XLogRecPtr current_lsn)
replorigin_session_origin_lsn = InvalidXLogRecPtr;
replorigin_session_origin_timestamp = 0;
+ /*+ * Updating pg_replication_origin might involve TOAST table access, so+ * ensure we have a valid snapshot.+ */+ PushActiveSnapshot(GetTransactionSnapshot());+
/*
* Drop the tablesync's origin tracking if exists.
*
@@ -387,6 +393,8 @@ process_syncing_tables_for_sync(XLogRecPtr current_lsn)
*/
replorigin_drop_by_name(originname, true, false);
+ PopActiveSnapshot();+
finish_sync_worker();
}
else
@@ -483,6 +491,12 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn)
started_tx = true;
}
+ /*+ * Updating pg_replication_origin might involve TOAST table+ * access, so ensure we have a valid snapshot.+ */+ PushActiveSnapshot(GetTransactionSnapshot());+
/*
* Remove the tablesync origin tracking if exists.
*
@@ -500,6 +514,8 @@ process_syncing_tables_for_apply(XLogRecPtr current_lsn)
sizeof(originname));
replorigin_drop_by_name(originname, true, false);
+ PopActiveSnapshot();+
/*
* Update the state to READY only after the origin cleanup.
*/
@@ -1455,8 +1471,13 @@ LogicalRepSyncTableStart(XLogRecPtr *origin_startpos)
* Then advance to the LSN got from walrcv_create_slot. This is WAL
* logged for the purpose of recovery. Locks are to prevent the
* replication origin from vanishing while advancing.
+ *+ * Updating pg_replication_origin might involve TOAST table access, so+ * ensure we have a valid snapshot.
*/
+ PushActiveSnapshot(GetTransactionSnapshot());
originid = replorigin_create(originname);
+ PopActiveSnapshot();
LockRelationOid(ReplicationOriginRelationId, RowExclusiveLock);
replorigin_advance(originid, *origin_startpos, InvalidXLogRecPtr,
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.cindex 925dff9cc4..804e5dbb75 100644--- a/src/backend/replication/logical/worker.c+++ b/src/backend/replication/logical/worker.c@@ -4604,8 +4604,16 @@ run_apply_worker()
walrcv_startstreaming(LogRepWorkerWalRcvConn, &options);
StartTransactionCommand();
++ /*+ * Updating pg_subscription might involve TOAST table access, so+ * ensure we have a valid snapshot.+ */+ PushActiveSnapshot(GetTransactionSnapshot());+
UpdateTwoPhaseState(MySubscription->oid, LOGICALREP_TWOPHASE_STATE_ENABLED);
MySubscription->twophasestate = LOGICALREP_TWOPHASE_STATE_ENABLED;
+ PopActiveSnapshot();
CommitTransactionCommand();
}
else
@@ -4821,7 +4829,15 @@ DisableSubscriptionAndExit(void)
/* Disable the subscription */
StartTransactionCommand();
++ /*+ * Updating pg_subscription might involve TOAST table access, so ensure we+ * have a valid snapshot.+ */+ PushActiveSnapshot(GetTransactionSnapshot());+
DisableSubscription(MySubscription->oid);
+ PopActiveSnapshot();
CommitTransactionCommand();
/* Ensure we remove no-longer-useful entry for worker's start time */
@@ -4925,6 +4941,12 @@ clear_subscription_skip_lsn(XLogRecPtr finish_lsn)
started_tx = true;
}
+ /*+ * Updating pg_subscription might involve TOAST table access, so ensure we+ * have a valid snapshot.+ */+ PushActiveSnapshot(GetTransactionSnapshot());+
/*
* Protect subskiplsn of pg_subscription from being concurrently updated
* while clearing it.
@@ -4983,6 +5005,8 @@ clear_subscription_skip_lsn(XLogRecPtr finish_lsn)
heap_freetuple(tup);
table_close(rel, NoLock);
+ PopActiveSnapshot();+
if (started_tx)
CommitTransactionCommand();
}
--
2.39.5 (Apple Git-154)
v4-0002-Introduce-RelationGetToastRelid-macro.patch (text/plain, 12.8K) download
From eb289ef83fb9e3c83bcef614fc3a8f7e3d7e1dce Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 14 Oct 2024 14:50:59 -0500
Subject: [PATCH v4 2/3] Introduce RelationGetToastRelid macro.
Discussion: https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan
---
contrib/amcheck/verify_heapam.c | 6 +++---
src/backend/access/common/toast_internals.c | 2 +-
src/backend/access/heap/heaptoast.c | 6 +++---
src/backend/catalog/heap.c | 2 +-
src/backend/catalog/index.c | 2 +-
src/backend/catalog/toasting.c | 2 +-
src/backend/commands/cluster.c | 19 ++++++++++---------
src/backend/commands/indexcmds.c | 4 ++--
src/backend/commands/tablecmds.c | 8 ++++----
src/backend/commands/vacuum.c | 2 +-
.../replication/logical/reorderbuffer.c | 4 ++--
src/backend/utils/adt/dbsize.c | 4 ++--
src/include/utils/rel.h | 6 ++++++
13 files changed, 37 insertions(+), 30 deletions(-)
diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.cindex f2526ed63a..78316daacd 100644--- a/contrib/amcheck/verify_heapam.c+++ b/contrib/amcheck/verify_heapam.c@@ -367,12 +367,12 @@ verify_heapam(PG_FUNCTION_ARGS)
}
/* Optionally open the toast relation, if any. */
- if (ctx.rel->rd_rel->reltoastrelid && check_toast)+ if (RelationGetToastRelid(ctx.rel) && check_toast)
{
int offset;
/* Main relation has associated toast relation */
- ctx.toast_rel = table_open(ctx.rel->rd_rel->reltoastrelid,+ ctx.toast_rel = table_open(RelationGetToastRelid(ctx.rel),
AccessShareLock);
offset = toast_open_indexes(ctx.toast_rel,
AccessShareLock,
@@ -1721,7 +1721,7 @@ check_tuple_attribute(HeapCheckContext *ctx)
}
/* The relation better have a toast table */
- if (!ctx->rel->rd_rel->reltoastrelid)+ if (!OidIsValid(RelationGetToastRelid(ctx->rel)))
{
report_corruption(ctx,
psprintf("toast value %u is external but relation has no toast relation",
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.cindex 90d0654e62..39d93d12f2 100644--- a/src/backend/access/common/toast_internals.c+++ b/src/backend/access/common/toast_internals.c@@ -151,7 +151,7 @@ toast_save_datum(Relation rel, Datum value,
* uniqueness of the OID we assign to the toasted item, even though it has
* additional columns besides OID.
*/
- toastrel = table_open(rel->rd_rel->reltoastrelid, RowExclusiveLock);+ toastrel = table_open(RelationGetToastRelid(rel), RowExclusiveLock);
toasttupDesc = toastrel->rd_att;
/* Open all the toast indexes and look for the valid one */
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.cindex a420e16530..cf6f3dff53 100644--- a/src/backend/access/heap/heaptoast.c+++ b/src/backend/access/heap/heaptoast.c@@ -213,7 +213,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
* XXX maybe the threshold should be less than maxDataLen?
*/
if (toast_attr[biggest_attno].tai_size > maxDataLen &&
- rel->rd_rel->reltoastrelid != InvalidOid)+ OidIsValid(RelationGetToastRelid(rel)))
toast_tuple_externalize(&ttc, biggest_attno, options);
}
@@ -224,7 +224,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
*/
while (heap_compute_data_size(tupleDesc,
toast_values, toast_isnull) > maxDataLen &&
- rel->rd_rel->reltoastrelid != InvalidOid)+ OidIsValid(RelationGetToastRelid(rel)))
{
int biggest_attno;
@@ -259,7 +259,7 @@ heap_toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup,
while (heap_compute_data_size(tupleDesc,
toast_values, toast_isnull) > maxDataLen &&
- rel->rd_rel->reltoastrelid != InvalidOid)+ OidIsValid(RelationGetToastRelid(rel)))
{
int biggest_attno;
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.cindex 0078a12f26..1b912488d6 100644--- a/src/backend/catalog/heap.c+++ b/src/backend/catalog/heap.c@@ -3073,7 +3073,7 @@ heap_truncate_one_rel(Relation rel)
RelationTruncateIndexes(rel);
/* If there is a toast table, truncate that too */
- toastrelid = rel->rd_rel->reltoastrelid;+ toastrelid = RelationGetToastRelid(rel);
if (OidIsValid(toastrelid))
{
Relation toastrel = table_open(toastrelid, AccessExclusiveLock);
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.cindex 12822d0b14..3eab421a16 100644--- a/src/backend/catalog/index.c+++ b/src/backend/catalog/index.c@@ -3930,7 +3930,7 @@ reindex_relation(const ReindexStmt *stmt, Oid relid, int flags,
get_namespace_name(RelationGetNamespace(rel)),
RelationGetRelationName(rel));
- toast_relid = rel->rd_rel->reltoastrelid;+ toast_relid = RelationGetToastRelid(rel);
/*
* Get the list of index OIDs for this relation. (We trust the relcache
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.cindex ad3082c62a..cf47e0960e 100644--- a/src/backend/catalog/toasting.c+++ b/src/backend/catalog/toasting.c@@ -149,7 +149,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
/*
* Is it already toasted?
*/
- if (rel->rd_rel->reltoastrelid != InvalidOid)+ if (OidIsValid(RelationGetToastRelid(rel)))
return false;
/*
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.cindex 78f96789b0..20da5201b6 100644--- a/src/backend/commands/cluster.c+++ b/src/backend/commands/cluster.c@@ -780,7 +780,7 @@ make_new_heap(Oid OIDOldHeap, Oid NewTableSpace, Oid NewAccessMethod,
* Note that NewHeapCreateToastTable ends with CommandCounterIncrement, so
* that the TOAST table will be visible for insertion.
*/
- toastid = OldHeap->rd_rel->reltoastrelid;+ toastid = RelationGetToastRelid(OldHeap);
if (OidIsValid(toastid))
{
/* keep the existing toast table's reloptions, if any */
@@ -870,8 +870,8 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
* We don't need to open the toast relation here, just lock it. The lock
* will be held till end of transaction.
*/
- if (OldHeap->rd_rel->reltoastrelid)- LockRelationOid(OldHeap->rd_rel->reltoastrelid, AccessExclusiveLock);+ if (RelationGetToastRelid(OldHeap))+ LockRelationOid(RelationGetToastRelid(OldHeap), AccessExclusiveLock);
/*
* If both tables have TOAST tables, perform toast swap by content. It is
@@ -880,7 +880,8 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
* swap by links. This is okay because swap by content is only essential
* for system catalogs, and we don't support schema changes for them.
*/
- if (OldHeap->rd_rel->reltoastrelid && NewHeap->rd_rel->reltoastrelid)+ if (OidIsValid(RelationGetToastRelid(OldHeap)) &&+ OidIsValid(RelationGetToastRelid(NewHeap)))
{
*pSwapToastByContent = true;
@@ -902,7 +903,7 @@ copy_table_data(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex, bool verbose,
* work for values copied over from the old toast table, but not for
* any values that we toast which were previously not toasted.)
*/
- NewHeap->rd_toastoid = OldHeap->rd_rel->reltoastrelid;+ NewHeap->rd_toastoid = RelationGetToastRelid(OldHeap);
}
else
*pSwapToastByContent = false;
@@ -1581,19 +1582,19 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
Relation newrel;
newrel = table_open(OIDOldHeap, NoLock);
- if (OidIsValid(newrel->rd_rel->reltoastrelid))+ if (OidIsValid(RelationGetToastRelid(newrel)))
{
Oid toastidx;
char NewToastName[NAMEDATALEN];
/* Get the associated valid index to be renamed */
- toastidx = toast_get_valid_index(newrel->rd_rel->reltoastrelid,+ toastidx = toast_get_valid_index(RelationGetToastRelid(newrel),
NoLock);
/* rename the toast table ... */
snprintf(NewToastName, NAMEDATALEN, "pg_toast_%u",
OIDOldHeap);
- RenameRelationInternal(newrel->rd_rel->reltoastrelid,+ RenameRelationInternal(RelationGetToastRelid(newrel),
NewToastName, true, false);
/* ... and its valid index too. */
@@ -1609,7 +1610,7 @@ finish_heap_swap(Oid OIDOldHeap, Oid OIDNewHeap,
* that is updated as part of RenameRelationInternal.
*/
CommandCounterIncrement();
- ResetRelRewrite(newrel->rd_rel->reltoastrelid);+ ResetRelRewrite(RelationGetToastRelid(newrel));
}
relation_close(newrel, NoLock);
}
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.cindex e33ad81529..b7ddced7f0 100644--- a/src/backend/commands/indexcmds.c+++ b/src/backend/commands/indexcmds.c@@ -3680,9 +3680,9 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
}
/* Also add the toast indexes */
- if (OidIsValid(heapRelation->rd_rel->reltoastrelid))+ if (OidIsValid(RelationGetToastRelid(heapRelation)))
{
- Oid toastOid = heapRelation->rd_rel->reltoastrelid;+ Oid toastOid = RelationGetToastRelid(heapRelation);
Relation toastRelation = table_open(toastOid,
ShareUpdateExclusiveLock);
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.cindex c6f82e93b9..cf076dec5d 100644--- a/src/backend/commands/tablecmds.c+++ b/src/backend/commands/tablecmds.c@@ -2140,7 +2140,7 @@ ExecuteTruncateGuts(List *explicit_rels,
/*
* The same for the toast table, if any.
*/
- toast_relid = rel->rd_rel->reltoastrelid;+ toast_relid = RelationGetToastRelid(rel);
if (OidIsValid(toast_relid))
{
Relation toastrel = relation_open(toast_relid,
@@ -15161,10 +15161,10 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
ReleaseSysCache(tuple);
/* repeat the whole exercise for the toast table, if there's one */
- if (OidIsValid(rel->rd_rel->reltoastrelid))+ if (OidIsValid(RelationGetToastRelid(rel)))
{
Relation toastrel;
- Oid toastid = rel->rd_rel->reltoastrelid;+ Oid toastid = RelationGetToastRelid(rel);
toastrel = table_open(toastid, lockmode);
@@ -15253,7 +15253,7 @@ ATExecSetTableSpace(Oid tableOid, Oid newTableSpace, LOCKMODE lockmode)
return;
}
- reltoastrelid = rel->rd_rel->reltoastrelid;+ reltoastrelid = RelationGetToastRelid(rel);
/* Fetch the list of indexes on toast relation if necessary */
if (OidIsValid(reltoastrelid))
{
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.cindex ac8f5d9c25..08bd489aed 100644--- a/src/backend/commands/vacuum.c+++ b/src/backend/commands/vacuum.c@@ -2187,7 +2187,7 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params,
if ((params->options & VACOPT_PROCESS_TOAST) != 0 &&
((params->options & VACOPT_FULL) == 0 ||
(params->options & VACOPT_PROCESS_MAIN) == 0))
- toast_relid = rel->rd_rel->reltoastrelid;+ toast_relid = RelationGetToastRelid(rel);
else
toast_relid = InvalidOid;
diff --git a/src/backend/replication/logical/reorderbuffer.c b/src/backend/replication/logical/reorderbuffer.cindex 22bcf171ff..c9d9590108 100644--- a/src/backend/replication/logical/reorderbuffer.c+++ b/src/backend/replication/logical/reorderbuffer.c@@ -4822,10 +4822,10 @@ ReorderBufferToastReplace(ReorderBuffer *rb, ReorderBufferTXN *txn,
desc = RelationGetDescr(relation);
- toast_rel = RelationIdGetRelation(relation->rd_rel->reltoastrelid);+ toast_rel = RelationIdGetRelation(RelationGetToastRelid(relation));
if (!RelationIsValid(toast_rel))
elog(ERROR, "could not open toast relation with OID %u (base relation \"%s\")",
- relation->rd_rel->reltoastrelid, RelationGetRelationName(relation));+ RelationGetToastRelid(relation), RelationGetRelationName(relation));
toast_desc = RelationGetDescr(toast_rel);
diff --git a/src/backend/utils/adt/dbsize.c b/src/backend/utils/adt/dbsize.cindex e63e99c141..a3597be9bc 100644--- a/src/backend/utils/adt/dbsize.c+++ b/src/backend/utils/adt/dbsize.c@@ -436,8 +436,8 @@ calculate_table_size(Relation rel)
/*
* Size of toast relation
*/
- if (OidIsValid(rel->rd_rel->reltoastrelid))- size += calculate_toast_table_size(rel->rd_rel->reltoastrelid);+ if (OidIsValid(RelationGetToastRelid(rel)))+ size += calculate_toast_table_size(RelationGetToastRelid(rel));
return size;
}
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.hindex 8700204953..6631b37d97 100644--- a/src/include/utils/rel.h+++ b/src/include/utils/rel.h@@ -504,6 +504,12 @@ typedef struct ViewOptions
*/
#define RelationGetRelid(relation) ((relation)->rd_id)
+/*+ * RelationGetToastRelid+ * Returns the OID of the relation's TOAST table (or InvalidOid if none)+ */+#define RelationGetToastRelid(relation) ((relation)->rd_rel->reltoastrelid)+
/*
* RelationGetNumberOfAttributes
* Returns the total number of attributes in a relation.
--
2.39.5 (Apple Git-154)
v4-0003-Assert-that-we-have-a-valid-snapshot-if-we-might-.patch (text/plain, 2.8K) download
From 187ef6ee2abe56f72ee9b32de6961227c4abff38 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Mon, 14 Oct 2024 14:53:36 -0500
Subject: [PATCH v4 3/3] Assert that we have a valid snapshot if we might need
TOAST access.
Suggested-by: Michael Paquier
Reviewed-by: Michael Paquier
Discussion: https://postgr.es/m/ZvMSUPOqUU-VNADN%40nathan
---
src/backend/access/heap/heapam.c | 29 +++++++++++++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex da5e656a08..d819c58ff3 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -210,6 +210,27 @@ static const int MultiXactStatusLock[MaxMultiXactStatus + 1] =
#define TUPLOCK_from_mxstatus(status) \
(MultiXactStatusLock[(status)])
+/*+ * Check that we have a valid snapshot if we might need TOAST access.+ */+static inline void+AssertHasSnapshotForToast(Relation rel)+{+#ifdef USE_ASSERT_CHECKING++ /* bootstrap mode in particular breaks this rule */+ if (!IsNormalProcessingMode())+ return;++ /* if the relation doesn't have a TOAST table, we are good */+ if (!OidIsValid(RelationGetToastRelid(rel)))+ return;++ Assert(HaveRegisteredOrActiveSnapshot());++#endif /* USE_ASSERT_CHECKING */+}+
/* ----------------------------------------------------------------
* heap support routines
* ----------------------------------------------------------------
@@ -1995,6 +2016,8 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid,
Assert(HeapTupleHeaderGetNatts(tup->t_data) <=
RelationGetNumberOfAttributes(relation));
+ AssertHasSnapshotForToast(relation);+
/*
* Fill in tuple header fields and toast the tuple if necessary.
*
@@ -2272,6 +2295,8 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
/* currently not needed (thus unsupported) for heap_multi_insert() */
Assert(!(options & HEAP_INSERT_NO_LOGICAL));
+ AssertHasSnapshotForToast(relation);+
needwal = RelationNeedsWAL(relation);
saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
HEAP_DEFAULT_FILLFACTOR);
@@ -2694,6 +2719,8 @@ heap_delete(Relation relation, ItemPointer tid,
Assert(ItemPointerIsValid(tid));
+ AssertHasSnapshotForToast(relation);+
/*
* Forbid this during a parallel operation, lest it allocate a combo CID.
* Other workers might need that combo CID for visibility checks, and we
@@ -3189,6 +3216,8 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
Assert(HeapTupleHeaderGetNatts(newtup->t_data) <=
RelationGetNumberOfAttributes(relation));
+ AssertHasSnapshotForToast(relation);+
/*
* Forbid this during a parallel operation, lest it allocate a combo CID.
* Other workers might need that combo CID for visibility checks, and we
--
2.39.5 (Apple Git-154)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, michael@paquier.xyz, exclusion@gmail.com, tgl@sss.pgh.pa.us, jkatz@postgresql.org, pgsql-hackers@lists.postgresql.org
Subject: Re: Large expressions in indexes can't be stored (non-TOASTable)
In-Reply-To: <Zw_hxaDoC3TzQYjj@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox