[jdatcmd]

Part of #1341, my core-am items, as planned on #1283. A new suite, am_contracts, in both twins, pins contracts the access method owes core DML and transactions. Writing it found four real bugs, two of them silent data loss or a broken unique constraint. Each is fixed here, and each arm was shown failing against the code before its fix.

The bugs

#	bug	red first (before the fix)	fix
1	INSERT ... ON CONFLICT DO NOTHING raised 0A000 on any table with a unique index; docs/limitations.md said it works	"skips the duplicate and inserts the new row: got [0A000|10|v5]"	speculative insertion inserts the row as an ordinary row, and withdraws it (a delete mark) when the arbiter index reports that a concurrent inserter won the key
2	data loss: a plpgsql EXCEPTION block that rolls back took the enclosing statement's buffered rows with it	an INSERT ... SELECT f(g) of 5000 rows kept 1 ("got [1|5000]"); the matching DELETE gave no answer	ExecutorEnd flushes only the current subtransaction's buffers; an enclosing statement's wait for its own end
3	unique violation: an insert racing an uncommitted DELETE (or key-changing UPDATE) of its key didn't wait, so a rollback left two rows with the key	"got [ok|2|a] want [23505|1|a]"	under a dirty snapshot, a row whose delete is still in progress is reported present, as heap does, so _bt_check_unique waits
4	an index built after a DELETE missed the row for a transaction whose snapshot predates the delete	"got [0] want [1]", through an Index Scan on the new index	the build indexes deleted rows as not alive, as heap indexes a recently dead tuple

Plus one item that was a missing test, not a bug: a TRUNCATE that rolls back, at transaction level and to a savepoint, restores every row. With TRUNCATE emptying the old storage in place, all five of its arms fail.

Proofs beyond the arms

• The withdrawal path (1) is reached only by a race. With one session holding a key uncommitted, the pre-check waits and never inserts. So eight sessions race on clock-derived keys: 0 duplicate keys clean, and 227–507 with the withdrawal disabled.
• The buffered-parent arm ("rows the parent buffered before a savepoint survive a TRUNCATE rolled back to it") is green only because each statement flushes at ExecutorEnd. With that flush removed it fails: 7001 rows against 7501.
• The cost of (4) is pinned: a UNIQUE index still builds over a key whose other holder is a deleted row.

Not fixed here

#1360, the same data loss as (2) but through a read of the enclosing statement's table inside the block. Skipping that flush would give wrong answers instead, so it needs the design proposed there.

Evidence

• am_contracts: 27/27 on each of PG 15–19, with the pytest twin's 7 tests passing on each.
• Seven mutations on PG 18, each reddening exactly its arms:
  • the stub restored;
  • the withdrawal disabled;
  • no ExecutorEnd flush;
  • TRUNCATE in place;
  • flush-everything;
  • the old dirty fetch;
  • the build skipping deleted rows.
• Full PG 18 matrix on this head: 283 suites, ALL VERSIONS PASSED, rc=0.
• Concurrency and index suites on PG 17: unique_conc 33/33, update_conc 25/25, concurrency 7/7, native_dml, index_delete_liveness, advisory_lock_class, native_index, index_only, native_ios, index_am_support, catalog_scan_index, native_index_projection, parallel_am_scan and audit, all green.
• Ledger:
  • 27 rows, 18 dated by the seven mutations, 9 never (5 premises, 4 controls);
  • rows 2343 → 2370, census 1796 → 1805;
  • gate rc=0, and the coverage ceiling stays at 246 because the new suite is seeded.
• Counts: cluster_tests 533 → 540, and the database-free set gives 429 passed.

Merge order

Behind #1357, #1359, #1361, #1358 and #1356. I'll rebase and restate when it's its turn.

🤖 Generated with Claude Code

[OffgridwithJD]

Approving 071ddec. The strongest proof I could run: this PR's tests on main's product code. I used a worktree of this head with src/ checked out from main 1933577, on PG17, and then this head itself.

	am_contracts.sh	test_am_contracts.py
main's code	12 failed of 27	6 failed, 1 passed
this head	27/27	7 passed

On main's code each of the four bugs shows exactly as described:

• ON CONFLICT DO NOTHING: "got [0A000|10|v5]".
• The aborted inner block: "got [1|5000] want [5000|12502500]", and the outer delete gave no answer.
• The unique race: "got [ok|2|a] want [23505|1|a]", and the same for the key-changing UPDATE.
• The index build: "got [0] want [1]".
• The DO NOTHING race: 12000 duplicate keys in the shell's run and 87742 in pytest's, against 0 here.

Regression on this head, PG17:

• unique_conc 33, update_conc 25, concurrency 7, phase3 32 (all PASSED);
• native_dml 13/13, index_delete_liveness 9/9, truncate_cleanup 25/25.

CI is 15/15.

What I checked in the code:

• No stale snapshot fields. The new dirty-snapshot branch reads snapshot->xmin/xmax as "a catalog read saw an in-progress change". The fetch resets both at entry (columnar_tableam.c:1971 on this head), so an earlier candidate can't leave a value behind.
• Subtransaction commit is safe. ExecutorEnd now flushes the current subtransaction's buffers only. SUBXACT_EVENT_COMMIT_SUB already hands a released child's write states and delete buffers to the parent (write_state.c:3924, delete_vector.c:496), so nothing is stranded under a dead subid.
• The build matches heap. It passes alive=false for a deleted row, as heap does for a recently-dead tuple, so a unique build doesn't count it.

Two notes, neither blocking:

1. pgcolumnar_tuple_complete_speculative's withdrawal flushes with PgColumnarFlushWriteStateForRelation, which isn't subtransaction-aware. A withdrawn speculative insert inside an EXCEPTION block, into a table the enclosing statement has buffered rows for, would flush those rows under the block. That's #1360's class, reached by a narrower path. Worth naming there so its design covers this call too.
2. The build now indexes every deleted row, however old, where heap skips a tuple dead to all snapshots. That's correct, and you pinned the unique-index cost. The other cost is index size on a table with many old deletes, until a REINDEX. A CHANGELOG line would set the expectation.

[jdatcmd]

@OffgridwithJD, rebased onto f601281. The new head is 192d51b, replacing 071ddec, which you approved. I'm getting this one ready while #1365 and #1369 wait for their rebases. It doesn't need to jump them.

Why it's one commit now: all nine commits touched TESTS.md and expected_tests.txt, so I applied the reviewed diff once with a squash merge. The commit message lists the original nine.

What to check:

• Patch: identical to 071ddec outside the count files (index and hunk lines aside).
• TESTS.md: am_contracts is now §106, because main's §105 is import_export_privilege. Against main, the diff only adds lines.
• cluster_tests: 539 → 546 by collection.
• Ledger: 2460 + 27 = 2487 rows; never 1893 → 1902; dated 585. All of main's rows are kept and the PR's 27 are unchanged. The gate gives rc=0, and the ceiling stays 244: the new suite is registered and seeded.
• Runs on this tree: am_contracts 27/27 and test_am_contracts.py 7 passed on each of PG15–19. The database-free guard set gives 429 passed.

Please re-approve 192d51b once CI is green. I'll check that the approval is on that SHA before merging (see #1368).

[OffgridwithJD]

@jdatcmd I've verified 192d51b:

• The patch is line-for-line identical to 071ddec outside TESTS.md, expected_tests.txt and the budget, ledger rows included.
• TESTS.md only adds lines, and cluster_tests is 546.
• am_contracts is 27/27 and test_am_contracts.py 7 passed on PG15 and PG18 at this head.
• The gate is clean (2487 / 1902 / 585, ceiling 244).

Its only CI red is suites (PG 18) → advisory_lock_class, the same race that has main red (see #1368). It isn't this PR. #1390 fixes it. I'll re-approve 192d51b once its CI is green; a rerun after #1390 lands should do it.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD, I reran the failed suites (PG 18) job, and 192d51b is now 15/15 green. Please re-approve 192d51b when you can. #1360's fix sits on this branch, so it waits on this merge.

🤖 Generated with Claude Code

[OffgridwithJD]

Re-approved at 192d51b. CI is now green (15/15) after main's advisory_lock_class race was fixed by #1390. Earlier I verified this head against the 071ddec I approved: identical outside the count files. am_contracts was 27/27 and test_am_contracts.py 7 passed on PG15 and PG18, and the gate was clean (2487 / 1902 / 585). Note it will need restating against today's main, which has moved since.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD, thanks for approving 192d51b. It no longer merged (based on f601281), so I rebased onto 6f2031c. The new head is db2fd08.

• Patch: identical to 192d51b outside the count files, except the TESTS.md section number: am_contracts is now §109, after test: arrow_import gets its pytest twin, and its error arms assert SQLSTATE under their own names (#1341) #1395's §108. In COMPLETE, am_contracts sits beside fix: the Iceberg wrapper reads no server file while planning for a role the scan would refuse (#1366) #1367's fdw_plan_privilege.
• Counts: cluster_tests 549 → 556 by collection. The ledger is restated on 2549 / 1962 / 587 to give 2576 / 1971 / 605.
• Gate: clean, with 285 registered and the ceiling at 241. The guard set gives 433 passed.
• Re-run on the rebased tree: main now includes fix: a DELETE whose qual deletes another row of the same table no longer fails (#1394) #1396's upsert fix, so I ran it again. am_contracts is 27/27 and test_am_contracts.py 7 passed on each of PG15–19.

Please re-approve db2fd08. #1360's fix builds on this, so I'll open it once this lands.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD, db2fd08 went stale after #1387, #1389 and #1397, so I rebased onto dc11072. The new head is 859d0f0.

• Patch: identical to 192d51b outside the count files, except the TESTS.md number: am_contracts is now §110.
• Counts: cluster_tests 551 → 558 by collection. The ledger is restated on 2656 / 2041 / 615 to give 2683 / 2050 / 633.
• Gate: clean, with the ceiling at 237. The guard set gives 433 passed.
• No re-run needed: nothing under src/, the SQL or the Makefile has changed on main since my PG15–19 run on db2fd08 (27/27 and 7 passed).

Please re-approve 859d0f0. I'll hold every other ledger merge until it's in, then rebase #1403 (#1360) onto it.

🤖 Generated with Claude Code

[OffgridwithJD]

Verified 859d0f0 myself: outside the four count files its patch is line-identical to 192d51b, which I approved earlier. It is based on dc11072, and nothing is removed from check_ledger.tsv or TESTS.md. CI is fully green (SUCCESS=15). Approving.