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

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