From c976b86714d3f1b3da518fb42dac385ce70e1820 Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Wed, 1 Jul 2026 13:05:56 +0530 Subject: [PATCH] ci(postgres): teach the parity guide the over-broad-rollback trap Recovering a poisoned Postgres txn with a full frappe.db.rollback() discards rows the handler already created before the failure -- which MariaDB keeps (no statement-abort) -- so it's a silent MariaDB regression. 'Owns the txn' does not make a full rollback safe in a loop handler. Document the safe cases (re-raise / single op / atomic batch) and the per-iteration/per-record savepoint alternative. Co-Authored-By: Claude Opus 4.8 --- .github/POSTGRES_COMPATIBILITY.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/POSTGRES_COMPATIBILITY.md b/.github/POSTGRES_COMPATIBILITY.md index 6f2445d8d44..48d5f6449a6 100644 --- a/.github/POSTGRES_COMPATIBILITY.md +++ b/.github/POSTGRES_COMPATIBILITY.md @@ -177,6 +177,15 @@ These are auto-handled by the framework and are **not** breaks: frappe#40075). Such a handler must wrap the fallible insert in `frappe.db.savepoint(name)` + `rollback(save_point=name)` — unless it re-`throw`s with no DB call before the throw, or the insert uses `ignore_if_duplicate=True` / `autoname="hash"` (→ `ON CONFLICT DO NOTHING`). +- **Recover the txn with a *scoped* savepoint, not a full `frappe.db.rollback()`, if any prior work + must survive.** A full rollback un-poisons the txn but also discards every row the handler committed + *before* the failure — which MariaDB kept (it has no statement-abort), so it's a **silent MariaDB + regression**. **"The background job / whitelist entrypoint owns the txn" does NOT make a full rollback + safe** if it did multiple inserts in a loop first — it drops the partial results MariaDB retained. A + full rollback is safe only when it (a) immediately re-`throw`s/`raise`s (MariaDB rolls back anyway), + (b) has nothing successful before it (a single op), or (c) the batch is genuinely meant to be + **atomic** (a partial result is an invalid state → rollback + mark *Failed* is correct). Otherwise use + a **per-iteration / per-record savepoint**. ---