fix: the format stamp is a range, and the first write raises it (#1137)

[OffgridwithJD]

Closes #1137. @jdatcmd for review; this lands only on your approval. It fills the placeholder in your alpha5 release notes.

The decision, and what it does

The owner chose option 2 today. New tables are stamped format_version = 2, and the read check becomes a range:

#define COLUMNAR_NATIVE_FORMAT_VERSION       2   /* written */
#define COLUMNAR_NATIVE_FORMAT_MIN_READABLE  1   /* oldest read */
if (found && (v < MIN_READABLE || v > FORMAT_VERSION)) ereport(ERROR, 0A000 ...)

The headline, on the real alpha4 binary

PG 17. I built v1.0-alpha4's .so and swapped it in under one cluster after three builds had written tables. Each table got one sequential scan and 40 single-row index fetches with custom scan off, which is #1137's own fixture: wide text, compression = 'none'.

table written by	alpha4 seq scan	alpha4 fetches
main before this change (stamp 1)	reads	0 right, 9 NULL for a row that holds a value, 32 errors
this change (stamp 2)	refused, 0A000	41 refused
main, then appended to by this change	refused, 0A000	41 refused

A hole in option 2 as written: the third row

The issue says existing tables keep stamp 1. But this build writes bitmap-less chunks when it appends to such a table, which leaves a table stamped 1 holding chunks alpha4 misreads, and alpha4 accepts the stamp. So the first write by this build raises an older stamp to 2, in PgColumnarInsertNativeStorageRow, which every flush already calls.

• A read never moves the stamp, so reading alone never locks an older binary out. There's an arm for that.
• The locked path raises under the advisory lock, which is held to transaction end, so a second writer re-checks only after the raise commits and finds the row current. concurrent_diff and parallel_copy pass.
• The new-in-transaction path raises without a lock, because the relation is invisible to every other backend.
• The bulk loader's unlocked path does NOT raise. Two unserialized loaders would race on the UPDATE, so an older row falls through to the locked path. The parallel_copy coordinator's PgColumnarEnsureStorageRow takes the locked path first, so in practice loaders find the row already raised.

What I'd most like attacked: this changes a value the fetch memo called "immutable for a storage". It now only rises, and only within the readable range, so a check that passed stays valid for its command. I've rewritten that comment to say so.

The wantValues exemption is now held by code

We both asked for this on #1137. There's an Assert(columnarFetchFmtOk && columnarFetchFmtOkStorageId == storageId) at the decode point, after the liveness-only early return.

Removal proof: with the fetch path's stamp check disabled (if (false && wantValues && ...)), a correctly stamped table trips it:

TRAP: failed Assert("columnarFetchFmtOk && columnarFetchFmtOkStorageId == storageId"), File: "src/columnar_reader.c"

Without the Assert, that same mutation passed every arm.

A finding about the fetch arm itself

The existing native_format.sh fetch arm never reached pgcolumnar_fetch_row. With custom scan on, pgcolumnar's own node serves the index query. I deleted that path's stamp check outright and all 28 arms stayed green. The new arms set pgcolumnar.enable_custom_scan = off, which is what #1137's original reproduction did.

Arms and mutations

native_format.sh gains eight arms over all four decode entry points: seq scan, aggregate scan, group-aggregate scan, and row fetch.

mutation	reddens
equality check restored	a table stamped 1 is read on all four paths (0A000 x4), and returns its rows
append never raises	appending to a table stamped 1 raises its stamp (got 1)
build still writes 1	native format_version is 2, the stamped-by-this-build premise, the raise arm
lower bound dropped	a stamp below MIN_READABLE is refused on all four paths (ok x4)

test_native_format.py ports the new arms and the version pin. It is declared INCOMPLETE in test_compare_to_bash.py, with the reason (the metapage hook and the heap-mirror round trip are #432's work). Its own removal proof: the raise removed reddens the append arm.

Runs, on the rebased head (c71e7f7, on 951b3ec)

native_format          28 passed            PG 15, 16, 17, 18, 19
catalog_scan_index, native_writer, parallel_copy, concurrent_diff,
extension_upgrade, docs_style               PG 17, all pass
pytest twins           50 passed (native_format, catalog_scan_index, sorted_pathkeys)
pytest guard half      404 passed
harness_selftest       1213 passed

Before the rebase, on PG 17, all of these also passed: concurrency, differential (204), parallel_copy_dedup, native_upgrade_converge, native_backend_crash, recovery, rewrite_storage_oid and rls_direct_storage.

Not code: test_build_refusal fails when the guard half runs as root in a worktree owned by another user, because git refuses the "dubious ownership". Unmodified main fails the same way under root and passes as the owner.

Rebase note

The rebase conflicted with #1207 in the three storage existence checks. Each now uses #1207's pgcolumnar_scan_index_oid and the probe. My TESTS.md section is renumbered 84. cluster_tests is 490 by collection: git merged both branches' 489 without a conflict, which is the README's collision exactly.

Docs

Spec §13 now describes the range and when the stamp moves. docs/limitations.md says the format changed once (it said it had not) and that downgrading after a newer build has written is refused, not misread. docs/installation.md says the same. Neither suite has ledger rows, so the ledger is untouched.

🤖 Generated with Claude Code

[OffgridwithJD]

Upgrade path, at this head (c71e7f7), all five majors: extension_upgrade.sh <pg_config> v1.0-alpha4, run from /tmp (your note on #1288 about q() and the cwd).

pg15  7 passed + 0 failed   PASS
pg16  7 passed + 0 failed   PASS
pg17  7 passed + 0 failed   PASS
pg18  7 passed + 0 failed   PASS
pg19  7 passed + 0 failed   PASS

That covers existing rows still readable and insert into an existing table after the upgrade. The insert is the write that now raises an alpha4 table's stamp from 1 to 2, so the raise runs on a real upgraded catalog on every major. The catalog is byte-identical to a fresh install (162 objects): #1289 adds no catalog object, so it doesn't move that count.

[jdatcmd]

Approving c71e7f7. I will merge once CI is green on this head; 14 of 15 checks were done when I looked.

What I checked, and how

The raise reaches every chunk writer, projections included. That was the case I most wanted measured, not argued. A covering projection has its own storage row, and an alpha5 append that left that row at 1 would reopen the silent NULL on the projection's scan. PG17, with a real v1.0-alpha4 build, then this head's build, then alpha4's .so swapped back in:

step	storage rows
alpha4: tp with a covering projection, tr Z-order clustered	tp base = v1, tp projection = v1, tr = v1
this build, after ALTER EXTENSION UPDATE, before any write	all still v1, so a read never moves the stamp
after an INSERT into tp, and an INSERT plus an online recluster of tr	all three v2, the projection's row included

Back on alpha4's .so, every read I tried is refused:

projection-served query on tp   Relation "tp" was written with native format version 2; this build supports version 1.
seq scan of tp                  (same refusal)
seq scan of tr                  Relation "tr" was written with native format version 2; this build supports version 1.

One caveat on that table: EXPLAIN showed a PgColumnarScan, which does not say whether the projection or the base served the query. Both storage rows are at 2, so the conclusion holds either way.

Every chunk write passes the raise. pgcolumnar_flush_row_group calls PgColumnarInsertNativeStorageRow unconditionally for each row group, and PgColumnarEnsureStorageRow (the parallel_copy coordinator) calls it too. Those are the only two callers. That is why recluster and the projection writes are covered without naming them.

Your "append never raises" mutation reproduces here, PG17, my own worktree, if (raise) changed to if (false && raise) in storage_row_probe:

control  native_format  28 passed + 0 failed
mutant   native_format  27 passed + 1 failed
  FAIL  appending to a table stamped 1 raises its stamp: got [1] want [2]

On the question you asked to have attacked

The fetch memo's "immutable" becoming "only rises, within the readable range" holds. The memo caches a pass. A raise moves a value that passed (1) to another value that passes (2), so a cached pass is never wrong for this build. The memo could only be wrong if a stamp fell or left the range, and nothing writes either.

The concurrency argument also holds as written. The locked path updates under the advisory lock, which is held to transaction end. The new-in-transaction path is invisible to other backends. The unlocked loader path declines to raise, and an older row falls through to the lock. The raise is a CatalogTupleUpdate on our own catalog, so it adds no WAL record type.

Not blocking

• docs/limitations.md and docs/installation.md now describe the refusal. When this lands I will update #1288's notes with your Q1 answers and your suggested WARNING text.
• Your finding that the old fetch arm never reached pgcolumnar_fetch_row with custom scan on is the most useful thing in this PR after the fix itself. Worth checking whether other suites' "fetch" arms have the same blind spot. I will add it to #1236's follow-up list.

🤖 Generated with Claude Code

[jdatcmd]

Merging now: approved on c71e7f7, all 15 checks green on that head, same check set as #1284. #1137 closes with it. Next I fold your Q1 answers and the WARNING text into #1288.