Hello!
Bryan Green <[email protected]> writes: > ... > BEGIN; > DO $$ > BEGIN > BEGIN > INSERT INTO t VALUES (1); -- subtransaction acquires an XID > EXCEPTION WHEN OTHERS THEN > NULL; -- swallow the commit-time error > END; > END $$; > COMMIT; -- crashes here > > TRAP: failed Assert("curval == 0 || ... || curval == status"), > File: "clog.c", Line: 702 > TransactionIdSetStatusBit > TransactionIdSetTreeStatus > TransactionIdCommitTree > RecordTransactionCommit > > The fix is small and stays in xact.c. AtSubCommit_childXids() records the > parent's child count before it appends, and AbortSubTransaction() restores > that count when the subtransaction aborts after the transfer. The entries > we added are the tail of the parent's array, so restoring the length drops > exactly them; the grandchildren revert to aborted, which is correct because > the whole subtree is rolling back. > > I confirmed on current master that the reproduction crashes without the > patch and commits cleanly with it, that the aborted row is gone, that a > committed sibling savepoint survives, and that the regression tests pass. Probably this requires to have this test in the patch, reproducing this issue it's not easy. Using a test that use the callback or an injection point, don't know if this it's expects to be backported, if that's not the case probably an injection point plus a regression will be enough. Related to the code, you added this: @@ -250,6 +252,7 @@ static TransactionStateData TopTransactionStateData = { .state = TRANS_DEFAULT, .blockState = TBLOCK_DEFAULT, .topXidLogged = false, + .savedParentNChildXids = -1, }; Why not initialize in StartTransaction() alongside with the other transaction states ? I was able to confirm this error, but it requires a test that targets this specific case, that's why I think that the test should be included here. Regards, -- Jonathan Gonzalez V. EDB https://www.enterprisedb.com
