Date: Fri, 20 Sep 2024 11:51:50 -0500
From: Nathan Bossart <nathandbossart@gmail.com>
To: Alexander Lakhin <exclusion@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, "Jonathan S. Katz" <jkatz@postgresql.org>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
	Michael Paquier <michael@paquier.xyz>
Subject: Re: Large expressions in indexes can't be stored (non-TOASTable)
Message-ID: <Zu2oJtMI2avvjmtl@nathan>
References: <b611015f-b423-458c-aa2d-be0e655cc1b4@postgresql.org>
 <ZtipsVMZAGjrO1p_@nathan>
 <1679756.1725476893@sss.pgh.pa.us>
 <c500a379-7a20-4ab4-bcc9-5ea9aabe017a@postgresql.org>
 <ZuroEfWjxd4Zj6Yq@nathan>
 <1812111.1726671296@sss.pgh.pa.us>
 <ZusvlZBrPKm913LA@nathan>
 <a97d7401-e7c9-f771-6a00-037379f0a8bb@gmail.com>
 <ZuxvNDVfqpahqZNb@nathan>
 <adb7421f-eaf4-878a-6742-576fc6bf0d8f@gmail.com>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="ZoeJq66r+ghpMz3m"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <adb7421f-eaf4-878a-6742-576fc6bf0d8f@gmail.com>
Archived-At: <https://www.postgresql.org/message-id/Zu2oJtMI2avvjmtl%40nathan>
Precedence: bulk


--ZoeJq66r+ghpMz3m
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit

On Fri, Sep 20, 2024 at 07:00:00AM +0300, Alexander Lakhin wrote:
> I've found another two paths to reach that condition:
> CREATE INDEX CONCURRENTLY ON def (vec_quantizer(id, :'b'));
> ERROR:  cannot fetch toast data without an active snapshot
> 
> REINDEX INDEX CONCURRENTLY def_vec_quantizer_idx;
> (or REINDEX TABLE CONCURRENTLY def;)
> TRAP: failed Assert("HaveRegisteredOrActiveSnapshot()"), File: "toast_internals.c", Line: 668, PID: 2934502

Here's a (probably naive) attempt at fixing these, too.  I'll give each
path a closer look once it feels like we've identified all the bugs.

> Perhaps it would make sense to check all CatalogTupleUpdate(pg_index, ...)
> calls (I've found 10 such instances, but haven't checked them yet).

Indeed.

-- 
nathan

--ZoeJq66r+ghpMz3m
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename="v2-0001-fix-failed-assertions-due-to-pg_index-s-TOAST-tab.patch"

From b7432c42c0cea9c1aadba7c72f9ce2ba6e6407d2 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 20 Sep 2024 11:48:52 -0500
Subject: [PATCH v2 1/1] fix failed assertions due to pg_index's TOAST table

---
 src/backend/catalog/index.c      | 5 +++++
 src/backend/commands/indexcmds.c | 9 +++++++++
 2 files changed, 14 insertions(+)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b2b3ecb524..2e378ef4ef 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2255,6 +2255,7 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
 		PopActiveSnapshot();
 		CommitTransactionCommand();
 		StartTransactionCommand();
+		PushActiveSnapshot(GetTransactionSnapshot());
 
 		/*
 		 * Now we must wait until no running transaction could be using the
@@ -2283,8 +2284,10 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
 		 * Again, commit the transaction to make the pg_index update visible
 		 * to other sessions.
 		 */
+		PopActiveSnapshot();
 		CommitTransactionCommand();
 		StartTransactionCommand();
+		PushActiveSnapshot(GetTransactionSnapshot());
 
 		/*
 		 * Wait till every transaction that saw the old index state has
@@ -2387,6 +2390,8 @@ index_drop(Oid indexId, bool concurrent, bool concurrent_lock_mode)
 	{
 		UnlockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock);
 		UnlockRelationIdForSession(&indexrelid, ShareUpdateExclusiveLock);
+
+		PopActiveSnapshot();
 	}
 }
 
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f99c2d2dee..36318c81ea 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1798,11 +1798,15 @@ DefineIndex(Oid tableId,
 								 PROGRESS_CREATEIDX_PHASE_WAIT_3);
 	WaitForOlderSnapshots(limitXmin, true);
 
+	PushActiveSnapshot(GetTransactionSnapshot());
+
 	/*
 	 * Index can now be marked valid -- update its pg_index entry
 	 */
 	index_set_state_flags(indexRelationId, INDEX_CREATE_SET_VALID);
 
+	PopActiveSnapshot();
+
 	/*
 	 * The pg_index update will cause backends (including this one) to update
 	 * relcache entries for the index itself, but we should also send a
@@ -4236,6 +4240,8 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 	 */
 	set_indexsafe_procflags();
 
+	PushActiveSnapshot(GetTransactionSnapshot());
+
 	forboth(lc, indexIds, lc2, newIndexIds)
 	{
 		ReindexIndexInfo *oldidx = lfirst(lc);
@@ -4280,8 +4286,10 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 	}
 
 	/* Commit this transaction and make index swaps visible */
+	PopActiveSnapshot();
 	CommitTransactionCommand();
 	StartTransactionCommand();
+	PushActiveSnapshot(GetTransactionSnapshot());
 
 	/*
 	 * While we could set PROC_IN_SAFE_IC if all indexes qualified, there's no
@@ -4316,6 +4324,7 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
 	}
 
 	/* Commit this transaction to make the updates visible. */
+	PopActiveSnapshot();
 	CommitTransactionCommand();
 	StartTransactionCommand();
 
-- 
2.39.5 (Apple Git-154)


--ZoeJq66r+ghpMz3m--