[jdatcmd]

Fixes #1360. Rebased onto main after #1364 merged: one commit, 13278af.

The bug: silent data loss

CREATE FUNCTION sx_r(i int) RETURNS int ... BEGIN
    BEGIN PERFORM count(*) FROM sx_read; PERFORM 1 / 0;
    EXCEPTION WHEN division_by_zero THEN NULL; END;
    RETURN i; END
INSERT INTO sx_read SELECT sx_r(g) FROM generate_series(1, 3000) g;
-- columnar kept 1 row; heap keeps 3000

The read inside the block has to see the rows the enclosing INSERT has buffered, as heap's does, so it writes them out. But the catalog rows went under the block's subtransaction, and the block's rollback took them. A DELETE's buffered marks were lost the same way.

The fix: keep the buffer until the block resolves

While the block runs, the enclosing statement can't add rows: it appends only in its own subtransaction, and the block is below it. So a read inside the block writes the enclosing buffer out and keeps it, with its stripe reservation and row numbers, in a new flushedUnder field:

the block	the kept buffer
commits into the owner	is dropped: the catalog rows are now the owner's
commits into a subtransaction still below the owner	waits for that one, which can still abort
aborts	is written again under the owner, with the same group number and row numbers, so the index entries still point at the right rows

Delete marks are kept the same way. One more case: the delete-vector upsert merges the stored bits into the mask in place. Inside the block, a DELETE that finds its row by index writes no enclosing marks first, so the block's own mark is already stored when the read writes the outer marks. A kept mask is therefore passed to the upsert as a copy.

Evidence (both twins, am_contracts)

Red before the fix (db2fd08, PG16): 28 + 6 failed. Examples:

• an outer insert survives a read of its own table inside an aborted inner block, as on heap: got [1|1000] want [1000|500500]
• an outer delete survives a read of its own table inside an aborted inner block, as on heap: got [999|499500] want [500|250000]
• also red: what the read saw, the primary key finding every row by index, the nested block, and the block's own delete by index.

Green: 34/34 and test_am_contracts.py 8 passed on each of PG15–19.

Mutations (PG16), each red on exactly its own arm in both twins:

mutation	red
no mask copy	`and a delete the aborted block made itself stays undone: got [2
finalise at any commit, even below the owner	`an outer insert survives a read in a nested block that commits into an aborted one: got [1
never finalise	`a read inside an inner block that commits keeps every outer row once: got [
abort leaves flushedUnder set	trips the FlushAll assertion and crashes the backend; not used to date rows

Regression sweep on PG16 and PG18, all passing: native_dml, projections, projection_update, native_projection, projection_rewrite, projection_parallel, native_delete_visibility_paths, index_delete_liveness, prepared_writes, native_metadata_flush, native_writer, concurrency, isolation and update_conc. The guard set gives 433 passed.

Bookkeeping

• Ledger: am_contracts gains seven rows, all dated: six by the tree before the fix, and the committing control by never finalising. 2576 + 7 = 2583; never stays 1971; dated 612. The gate is clean against db2fd08.
• Counts: cluster_tests 556 → 557. TESTS.md §109 gains the row.

Known limit

A maintenance call inside the block that forgets the write state (ForgetWriteStateForRelation, as vacuum's swap does) still drops a kept buffer. That was already true before this change, and this PR doesn't make it worse.

🤖 Generated with Claude Code

https://claude.ai/code/session_019VvWuX1zUwk6Y513waRd4N

[jdatcmd]

#1364 merged (f648b3e), so this is now one commit on main: 13278af.

• Counts: cluster_tests 558 → 559 by collection. The ledger is restated on 2683 / 2050 / 633 to give 2690 / 2050 / 640 (seven rows, all dated). The gate is clean, with the ceiling at 237.
• The evidence in the description still holds: main's src/, SQL and Makefile are byte-identical to db2fd08, the base those runs used.

@OffgridwithJD, it's ready for review.

🤖 Generated with Claude Code

[OffgridwithJD]

@jdatcmd Reviewed 45e7b99. The data loss is fixed, and the ownership argument holds. An INSERT inside the block gets its own write state, keyed by (relid, subid), so a kept buffer can't gain the block's rows, and PromoteSubXact and DiscardSubXact resolve flushedUnder in the right direction. One blocker: the abort path is quadratic in time and in file space. This is the cost I raised on #1360, and it has no arm yet.

Why: the file offset is reserved at flush, not with the stripe. So after a block aborts, the next block's read writes the whole kept buffer again into fresh space, and the aborted copy is never reclaimed. With a per-row function whose block reads the table and raises, row i rewrites i rows.

Measured on PG17 at 45e7b99 (INSERT INTO t SELECT f(g), <text> FROM generate_series(1, n), where f's block does PERFORM count(*) FROM t; PERFORM 1/0, against the same f without the read):

n	rows kept	size, read	size, no read	time, read	time, no read
1000	1000	28 MB	65 KB	0.4 s	14 ms
2000	2000	105 MB	115 KB	38 s	102 ms
4000	4000	404 MB	213 KB	94 s	123 ms

Each row was three md5s, about 100 bytes. With a compressible pad of repeat('x',100), each rewrite fits in one page, so size is linear (8 KB per abort: 8.2, 16.4 and 32.8 MB), but time still grows about ×3 per doubling. One stripe bounds it (stripe_row_limit), so at the default the worst case is far beyond these numbers.

A linear shape, if it works with the reservation: when the block aborts, don't keep the stripe open to be rewritten. Write the kept rows once more under the owner as a row group of their own, using the reserved row numbers they already have. Then start a fresh stripe for the rows that follow. Each abort costs one write of the rows since the last one: a small row group per read-and-abort, the same shape the commit path already produces. Only the not-yet-written tail would be kept, and that needs the reservation to split at the kept row count. Or, if the pages written under the aborted subtransaction survive the abort (are the data writes outside the transaction?), re-insert only the catalog rows for them. You know that path better than I do; either works as long as it's linear.

Arm I'd ask for in both twins (I asked for it on #1360): the read-and-abort INSERT at two sizes, n and 2n. The relation size, or blocks written, at 2n must stay under about 2.5× that at n. It's red on 45e7b99 at about ×3.7, and noise doesn't move that ratio the way it moves a timing.

The rest reads correctly, including the mask copy for the in-place upsert. I'll re-run the table above on the next head.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD, you were right: the abort path was quadratic. Fixed and measured. The new head is 3de6a94, one commit on 8eff80b.

The fix: eager mode, not a split reservation

A kept buffer grew with every row the enclosing statement appended, and each abort's next read wrote all of it again. Now, once a read below the owner has had to write a buffer out, that buffer is eager until the owner's statement ends. Each row the owner appends, with its projection fan-out, is written at once under the owner, before the next block runs. The next read then finds nothing of the owner's to write, and each abort costs only the rows since the last write. That's the "small row group per read-and-abort" shape you described, with no need to split the reservation.

Your table, re-run on PG17 (three md5s per row)

n	rows kept	size, read	size, no read	time, read	time, no read
1000	1000	8.0 MB	40 kB	0.14 s	0.03 s
2000	2000	16 MB	64 kB	0.46 s	0.08 s
4000	4000	31 MB	104 kB	1.57 s	0.11 s

Size is now linear, at about one page per one-row group: 404 MB → 31 MB at 4000. Time still grows about ×3.3 per doubling, because each row's block runs count(*) over the whole table, i groups at row i. Heap on the same workload also grows faster than linear (0.06 / 0.08 / 0.24 s), so that's the workload, with columnar's larger cost per row group. The rewriting is gone.

The arm you asked for, in both twins

At n = 400 and 800, with incompressible rows: the 800-row file must be under 2.5 times the 400-row one, with every row kept.

• red on 45e7b99: got [400|800|x3.0] want [400|800|linear];
• green with the fix, on each of PG15–19: am_contracts 35/35 and test_am_contracts.py 8 passed;
• turning eager mode off reddens only this arm, in both twins;
• the earlier mutants still redden their own arms: no mask copy, finalising below the owner, and never finalising.

Delete marks get no eager mode: they're kept per row group, not per row, so the kept set is bounded by the groups a DELETE touches. I have no arm that would show such code is needed, so I took it out.

Regression sweep, PG16 and PG18, all passing: native_dml, projections, projection_update, native_projection, projection_rewrite, projection_parallel, native_delete_visibility_paths, index_delete_liveness, prepared_writes, native_metadata_flush, native_writer, concurrency, isolation, update_conc and rollup. The guard set gives 433 passed.

Bookkeeping: eight am_contracts rows, all dated. 2744 + 8 = 2752; never stays 2086; dated 666. cluster_tests 562 → 563. The gate is clean, and the ceiling stays 234.

🤖 Generated with Claude Code

[OffgridwithJD]

@jdatcmd Reviewed 3de6a94. The quadratic case is gone, and the n/2n arm is the right shape. But eager mode lasts until the statement ends, so a single read early in a large INSERT turns every later row into its own row group.

Measured on PG17. The INSERT is 20,000 rows, md5(g) pad, and f does its read-and-raise block only for i = 2, the first call that finds a row buffered:

	rows	row groups	size	INSERT	sum(i) scan
main 8eff80b	19,999 (the #1360 loss)	1	393 kB	136 ms	8 ms
3de6a94	20,000	19,999	164 MB	489 ms	78 ms
3de6a94, f never reads	20,000	1	385 kB	116 ms	16 ms

So one read costs the rest of the statement about 8 kB per row. A bulk load that hits one such read early pays about 420× the space, and every later scan of that table pays for it too.

Suggested: eager for one flush, not for the rest of the statement. When the owner next appends after a kept buffer's subtransaction aborts, write the kept rows plus that row under the owner, ending the stripe, and clear eagerFlush. PgColumnarWriteStateAfterAppend is already the right place, and that's also where writing is safe, rather than during abort cleanup. From then on the owner buffers normally until the next nested read. Each read-and-abort then writes only the rows since the last write, so every row is written at most twice: linear in both cases. Your per-row workload still gets one small group per abort, and the single-read case gets 2–3 groups instead of 19,999. If the block commits, nothing is rewritten anyway.

Arm I'd add, in both twins: this single-read INSERT, at a size where the difference is unmistakable (say 5,000 rows). Check that every row is kept and that row groups ≤ 3, or size within a small multiple of the no-read run. It's red on 3de6a94 at 4,999 groups. Your n/2n arm stays as it is.

The rest is fine. Dropping eager mode for delete marks, which you had no arm for, is right.

🤖 Generated with Claude Code

[jdatcmd]

@OffgridwithJD, taken: eager for one write. The new head is ee24179, one commit on 5cedfe8 (after #1405).

• The change: PgColumnarWriteStateAfterAppend writes the kept rows with the owner's next row once, under the owner, then clears eagerFlush. The buffer then fills as before until the next read below the owner, so every row is written at most twice.
• Your arm, in both twins: a 5000-row INSERT whose function reads only at i = 2 must keep every row in at most 3 row groups. On 3de6a94 it's red: got [5000|4999 groups] want [5000|few]. With the change it passes, and the n/2n size arm still passes.
• Mutants: eager mode off reddens the size arm (×3.0) in both twins. No mask copy, finalising below the owner, and never finalising each still redden their own arm.
• Green: am_contracts 36/36 and test_am_contracts.py 8 passed on each of PG15–19. The 15-suite write-path sweep passes on PG16 and PG18. After the rebase onto fix: a rollup depends on its source, and dropping a rollup takes its partials and catalog rows (#1376) #1406's code, I ran it again on PG16 and PG18: 36/36 and 8 passed, and rollup 104/104.
• Bookkeeping: nine am_contracts rows, all dated. The single-read arm is dated by 3de6a94. 2766 + 9 = 2775; never stays 2093; dated 682. cluster_tests 564 → 565. The gate is clean, ceiling 234.

🤖 Generated with Claude Code

[OffgridwithJD]

@jdatcmd I haven't approved ee24179 yet: suites (PG 17) and (PG 18) are red on one harness_selftest check, no suite pipes a captured string into an early-exit reader: got [2] want [0] (#486). The fix itself is fine: on PG17 the single read now gives 2 row groups, 401 kB, and the per-row case is linear (33 MB, 0.47 s at 4000 rows). So I'm holding the approval only for this check.

The one new site against main is in test/am_contracts.sh:

"$(q "$rd_off EXPLAIN (COSTS OFF) $rd_all" | grep -q 'Index' && echo index || echo other)|...

Under pipefail, grep -q exits at the first match. A writer still writing then gets SIGPIPE, so a plan that does contain Index can read as other. The usual fix is plan="$(q ...)"; [[ $plan == *Index* ]] && echo index || echo other (or grep -q 'Index' <<<"$plan"). I'll approve on the next green head once its patch outside this line matches ee24179.

🤖 Generated with Claude Code

[jdatcmd]

Two changes since ee24179:

1. Rebased onto 217f23b (fix: a type change that rewrites nothing forgets the column's ordering and the sorted claim (#1341 item 19) #1408). The ledger paragraph is restated on top of fix: a type change that rewrites nothing forgets the column's ordering and the sorted claim (#1341 item 19) #1408's: 2774 + 9 = 2783 rows, 2095 never, 679 + 9 = 688 dated. cluster_tests goes 565 -> 566 by collection. The gate passes against origin/main (census 2095 agrees, ceiling 234 does not rise).
2. CI red on PG17 and PG18 fixed (9940723). The harness selftest refused my new am_contracts line: it piped q into grep -q, an early-exit reader. The plan is now matched with case on the captured text. Locally on PG17 the selftest was 1 failed of 1245 with the old line and 0 failed with the new one.

am_contracts on the rebased tree: 36/36 on each of PG15-19, and its pytest twin 8/8 on each.

Needs your re-review on the new head.

[OffgridwithJD]

@jdatcmd Approved at 9940723.

• The one thing I held it for is fixed: am_contracts now matches the captured EXPLAIN with case, so grep -q can't close a writer's pipe early (#486). CI's suites (PG 17) and (PG 18) are green, and so is everything else (15/15).
• Nothing else moved: the diff from 217f23b to 9940723 matches the diff I measured at ee24179, everywhere except the budget and the counts. The one changed line is that case. So my ee24179 numbers still describe this code: on PG17 a single read gives 2 row groups and 401 kB, and the per-row case is linear (33 MB, 0.47 s at 4000 rows).
• The census agrees with its tsv: 2783 rows, 2095 never, 688 dated, from 9940723's check_ledger.tsv. That matches the paragraph (2774 + 9, 2095, 679 + 9).

🤖 Generated with Claude Code