Horton

Re: Large expressions in indexes can't be stored (non-TOASTable)

Michael Paquier <michael@paquier.xyz>

View thread View on postgr.es
From: Michael Paquier <michael@paquier.xyz>
To: Nathan Bossart <nathandbossart@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, "Jonathan S. Katz" <jkatz@postgresql.org>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2024-10-16T00:12:31Z
Lists: pgsql-hackers 
On Mon, Oct 14, 2024 at 03:02:22PM -0500, Nathan Bossart wrote:
> Here is a reorganized patch set.  0001 would be back-patched, but the
> others would only be applied to v18.

Right.

-   if (!ctx->rel->rd_rel->reltoastrelid)
+   if (!OidIsValid(RelationGetToastRelid(ctx->rel)))

This set of diffs in 0002 is a nice cleanup.  I'd wish for relying
less on zero comparitons when assuming that InvalidOid is in use.

+static inline void
+AssertHasSnapshotForToast(Relation rel)
+{
+	/* 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());
+}

Using a separate inlined routine is indeed cleaner as you have
documented the assumptions behind the check.  Wouldn't it be better to
use a USE_ASSERT_CHECKING block?  These two checks for normal
processing and toastrelid are cheap lookups, but we don't need them at
all in non-assert paths, so I'd suggest to ignore them entirely for
the non-USE_ASSERT_CHECKING case.
--
Michael

Commits

  1. Ensure we have a snapshot when updating various system catalogs.

  2. Remove pg_replication_origin's TOAST table.

  3. Restrict password hash length.

  4. Ensure we have a snapshot when updating pg_index entries.

  5. Add TOAST table to pg_index.

  6. Add toast tables to most system catalogs