From: Tom Lane <tgl@sss.pgh.pa.us>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: panso8@gmail.com
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18656: "STABLE" function sometimes does not see changes
Date: Wed, 16 Oct 2024 17:40:23 -0400
Message-ID: <480025.1729114823@sss.pgh.pa.us> (raw)
In-Reply-To: <4082805.1729014675@sss.pgh.pa.us>
References: <18656-cade1780866ef66c@postgresql.org>
<4082805.1729014675@sss.pgh.pa.us>
I wrote:
> After some study I propose that the correct fix is that
> _SPI_execute_plan should act as though we're inside an atomic
> context if IsSubTransaction(). We are inside an atomic context
> so far as _SPI_commit and _SPI_rollback are concerned: they
> will not allow you to COMMIT/ROLLBACK. So it's not very clear
> why _SPI_execute_plan should think differently.
I dug into this further and identified exactly where it's going off
the rails. As things stand, _SPI_execute_plan thinks it can operate
non-atomically, so it doesn't push a snapshot and it passes
PROCESS_UTILITY_QUERY_NONATOMIC to ProcessUtility. But in
standard_ProcessUtility we find
bool isAtomicContext = (!(context == PROCESS_UTILITY_TOPLEVEL || context == PROCESS_UTILITY_QUERY_NONATOMIC) || IsTransactionBlock());
IsTransactionBlock() is always true in a subtransaction, so we pass
down atomic = true to ExecuteCallStmt, which then thinks it doesn't
need to push a snapshot either. End result is that the stable
function runs with the Portal snapshot and sees stale data.
So my patch fixes it by ensuring that _SPI_execute_plan will think
it's atomic in the same cases that standard_ProcessUtility will force
that. It's a little concerning though that standard_ProcessUtility is
arriving at its result through a completely different bit of
reasoning. There's an argument to be made that the above-quoted bit
of logic needs revision.
Nonetheless, it seems correct to me that the different parts of spi.c
share identical rules for what is a nonatomic execution environment.
I'm also fairly leery of touching standard_ProcessUtility's logic in a
back-patch --- that seems way too likely to have unforeseen side
effects on unrelated parts of the system. So I went ahead and pushed
what I had (after a bit more work on the comments). We can think
about whether to revise the standard_ProcessUtility logic at leisure.
regards, tom lane
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-bugs@postgresql.org
Cc: tgl@sss.pgh.pa.us, peter@eisentraut.org, panso8@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #18656: "STABLE" function sometimes does not see changes
In-Reply-To: <480025.1729114823@sss.pgh.pa.us>
* 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