From: Alexander Lakhin <exclusion@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
To: pgsql-bugs@lists.postgresql.org, Andres Freund <andres@anarazel.de>
Subject: Re: BUG #17893: Assert failed in heap_update()/_delete() when FK modiified by RI trigger in non-read-committed xact
Date: Tue, 28 Nov 2023 12:00:00 +0300
Message-ID: <8b695b41-a006-f4d2-545f-c99db7595ee2@gmail.com> (raw)
In-Reply-To: <0c6e10cd-9288-4dda-b35e-7a64684367d3@iki.fi>
References: <17893-35847009eec517b5@postgresql.org>
<b77078c0-090c-0f0e-6f13-3bb7daca93ee@gmail.com>
<1236e144-d772-7ff4-6e23-4ce91d0923ad@gmail.com>
<0c6e10cd-9288-4dda-b35e-7a64684367d3@iki.fi>
Hello Heikki,
27.11.2023 22:23, Heikki Linnakangas wrote:
> I propose to attached slight variation, which moves the assertions before the crosscheck snapshot check. The
> assertions are correct as they are, if you don't need to take the crosscheck into account.
>
Thank you for paying attention to this!
I agree with your variation, it's more accurate.
> I'm a little worried if the callers of tuple_update() might also get confused when tuple_update() returns TM_Updated
> but xmax is invalid. As you said:
> ...
> I think you only get that error with REPEATABLE READ or SERIALIZABLE isolation level. I don't quite understand how
> this works. In READ COMMITTED level, ISTM that ExecUpdate() or ExecDelete() will proceed with EvalPlanQualSlot() and
> locking the row with table_tuple_lock(). Or do we never use the cross-check snapshot in READ COMMITTED mode?
>
I think so, as I see the following in ri_PerformCheck():
/*
* In READ COMMITTED mode, we just need to use an up-to-date regular
* snapshot, and we will see all rows that could be interesting. But in
* transaction-snapshot mode, we can't change the transaction snapshot. If
* the caller passes detectNewRows == false then it's okay to do the query
* with the transaction snapshot; otherwise we use a current snapshot, and
* tell the executor to error out if it finds any rows under the current
* snapshot that wouldn't be visible per the transaction snapshot. Note
* that SPI_execute_snapshot will register the snapshots, so we don't need
* to bother here.
*/
if (IsolationUsesXactSnapshot() && detectNewRows)
{
CommandCounterIncrement(); /* be sure all my own work is visible */
test_snapshot = GetLatestSnapshot();
crosscheck_snapshot = GetTransactionSnapshot();
}
IIUC, that's the only place where crosscheck_snapshot (and then
es_crosscheck_snapshot) may get a non-invalid value.
Best regards,
Alexander
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: exclusion@gmail.com, hlinnaka@iki.fi, andres@anarazel.de
Subject: Re: BUG #17893: Assert failed in heap_update()/_delete() when FK modiified by RI trigger in non-read-committed xact
In-Reply-To: <8b695b41-a006-f4d2-545f-c99db7595ee2@gmail.com>
* 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