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, 27 Nov 2024 09:59:18 -0600
Message-ID: <Z0dB1ld2iPcS6nC9@nathan> (raw)
In-Reply-To: <Z0a6IwjW36af71J7@paquier.xyz>
References: <Zw14zoBCg-zGu1mi@nathan>
<Zw8E73_nCk0DbO18@paquier.xyz>
<Zw8U0YU_fu2pcXs6@nathan>
<Zw8V0ClvZbUi2QQl@paquier.xyz>
<Zw_hxaDoC3TzQYjj@nathan>
<ZxGZB2d-RttHwmlt@paquier.xyz>
<ZyKdCEaUB2lB1rG8@nathan>
<ZyLFC24UpcIJJmwT@paquier.xyz>
<Z0TQG6869v8t-hV5@nathan>
<Z0a6IwjW36af71J7@paquier.xyz>
On Wed, Nov 27, 2024 at 03:20:19PM +0900, Michael Paquier wrote:
> On Mon, Nov 25, 2024 at 01:29:31PM -0600, Nathan Bossart wrote:
>> Or we could just enforce that you have an active snapshot whenever you
>> modify a catalog with a TOAST table. That's simpler, but it requires extra
>> work in some paths (and probably comments to point out that we're only
>> pushing an active snapshot to satisfy an assertion).
>
> I may be wrong, but I suspect that enforcing the check without being
> column-based is the right way to go and that this is going to catch
> more errors in the long-term than being a maintenance burden. So I
> would keep the snapshot check even if it's a bit aggressive, still
> it's useful. And we are not talking about that may code paths that
> need to be switched to require a snapshot, as well. Most of the ones
> you have mentioned on this thread are really particular in the ways
> they do transaction handling. I suspect that it may also catch
> out-of-core issues with extensions doing direct catalog manipulations.
That is useful feedback, thanks.
One other thing that caught my eye is that replorigin_create() uses
SnapshotDirty, so I'm unsure if
PushActiveSnapshot(GetTransactionSnapshot()) is correct there. The only
other example I found is RelationFindReplTupleByIndex(), which uses
GetLatestSnapshot(). But I do see that CreateSubscription() calls
replorigin_create(), and it seems to rely on the transaction snapshot, so
maybe it's okay, at least for the purpose of TOAST table access... I'm
finding myself wishing there was a bit more commentary about the proper
usage of these snapshot functions. Maybe I can try to add some as a
follow-up exercise.
--
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: <Z0dB1ld2iPcS6nC9@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