| From: | Bryan Green <dbryan(dot)green(at)gmail(dot)com> |
|---|---|
| To: | Andrey Borodin <x4mmm(at)yandex-team(dot)ru>, Jacob Brazeal <jacob(dot)brazeal(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: serializable anomaly - duplicate primary keys |
| Date: | 2026-09-10 15:21:07 |
| Message-ID: | 55cd61b8-ab97-4700-964f-686560a0f408@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On 8/7/2026 8:59 AM, Andrey Borodin wrote:
>
>
>> On 28 Jul 2026, at 10:36, Jacob Brazeal <jacob(dot)brazeal(at)gmail(dot)com> wrote:
>>
>> I found a SERIALIZABLE anomaly where a transaction reads a row, successfully
>> inserts another row with the same primary key, observes both rows, and commits.
>
> Hi Jacob,
>
> Thanks for the report. I reproduced it, including with INSERT ... ON
> CONFLICT DO NOTHING.
>
> This is related to the ON CONFLICT issue discussed in [0], but has the
> opposite ordering: here the deletion commits first, and the unique check
> relies on it through SnapshotDirty.
>
> The window seems fairly narrow: a serializable transaction must read a row,
> keep its snapshot across another transaction's deletion, and then reuse the
> same key. It does not corrupt the index or leave a persistent duplicate, but
> the committed transaction violates the SERIALIZABLE guarantee. This seems
> plausible in delete-and-recreate or upsert workflows with long transactions.
>
> PFA invasive fix on top of HEAD. The table AM reports the deleting XID when
> SnapshotDirty skips a tuple still visible to the transaction snapshot. SSI
> then fails only if there is already an rw-conflict to that transaction. It
> also marks the transaction doomed before raising the error, so a savepoint
> cannot hide the failure.
>
> This extends the table AM interface. An ABI-preserving back-branch fix can
> be considered separately.
>
>
> Best regards, Andrey Borodin.
>
> [0] https://postgr.es/m/165342c0-0c75-461e-b334-b997639ad48d%40aphyr.com
Andrey,
I am doing security audits of commitfest items. The nbtree part looks
correct. Several non-security related observations though.
The crosscheck isn't gated on the relation.
s1: BEGIN ISOLATION LEVEL SERIALIZABLE; SELECT * FROM t WHERE k=1;
s2: BEGIN ISOLATION LEVEL SERIALIZABLE; UPDATE t SET ... WHERE k=1;
DROP TABLE foo; COMMIT;
s1: CREATE TABLE foo (...); [errors]
That commits on master. Catalog dependencies don't participate in SSI,
so this rejects valid DDL. It wants the same
PredicateLockingNeededForRelation gate the other conflict-out paths have.
The UNIQUE_CHECK_EXISTING skip drops deferred constraints. With the
delete still in progress at insert time the partial check defers, and
the recheck comes back as existing, which the gate skips:
s1: BEGIN SERIALIZABLE; SELECT * FROM test WHERE k=1;
s2: BEGIN SERIALIZABLE; DELETE FROM test WHERE j=1000000;
s1: INSERT INTO test VALUES (1,2);
s2: COMMIT;
s1: COMMIT; [commits, no error]
Immediate constraints wait on the delete and re-check, so they catch it.
Same function misses again once the deleter's been summarized. The
hash_search comes back NULL after SummarizeOldestCommittedSxact evicts
it, and it returns without failing, so the history that errors above
will commit once enough serializable writes have forced summarization.
Exclusion constraints have the same reuse-a-committed-delete hole, and
this doesn't reach them. check_exclusion_or_unique_constraint runs its
own DirtySnapshot scan and never goes through _bt_check_unique. An
EXCLUDE USING btree (k WITH =) version of perm 1 reproduces. You didn't
break it, fcff8a57519 was nbtree-only too, but the root cause in the
commit message covers it, so worth calling in-scope or follow-up.
One small thing, table_index_fetch_tuple_check calls the new AM callback
with no NULL check, so an out-of-tree AM compiled against the new
tableam.h crashes on its first unique check instead of tripping an
assertion.
--
Bryan Green
EDB: https://www.enterprisedb.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Ayoub Kazar | 2026-09-10 15:30:26 | Re: [PATCH] Rewrite undirected edge patterns in GRAPH_TABLE using UNION ALL |
| Previous Message | Etsuro Fujita | 2026-09-10 15:14:31 | Re: Several issues with postgres_fdw stats import |