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: Tue, 15 Oct 2024 20:20:17 -0500
Message-ID: <Zw8U0YU_fu2pcXs6@nathan> (raw)
In-Reply-To: <Zw8E73_nCk0DbO18@paquier.xyz>
References: <adb7421f-eaf4-878a-6742-576fc6bf0d8f@gmail.com>
<Zu2oJtMI2avvjmtl@nathan>
<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>
On Wed, Oct 16, 2024 at 09:12:31AM +0900, Michael Paquier wrote:
> - 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.
I'm wondering if there's any concern about this one causing back-patching
pain. If so, I can just add the macro for use in new code.
> +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.
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.
--
nathan
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: <Zw8U0YU_fu2pcXs6@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