pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedTrying to break online checksums with LLMs
20+ messages / 8 participants
[nested] [flat]
* Trying to break online checksums with LLMs
@ 2026-09-08 21:38 Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Gustafsson @ 2026-09-08 21:38 UTC (permalink / raw)
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Noah kindly ran Claude- and GPT-automated reviews on the online checksums
feature with the fix from the open item applied. Since he has been very
successful in identifying previously unknown issues in other features I was
interested in seeing what could shake loose. The full reports are attached,
with an executive summary of the identified issues and the fixes below. It's
mostly documentation or corner cases from re-starting operations within tight
windows, none of the issues would lead to incorrect results or false negatives
or positives in page verification. Fixes are in separate patches for easier
review, for a commit I would group the doc ones together.
Claude:
-------
* D1: Cancelling the enable launcher lets a second launcher start; the race
logs a false "data checksums are now enabled" while the cluster is off, and the
enable is silently lost
- This can cause an incorrect log entry, and the user will have to restart once
the exit handler is done which is surprising. The fix is to keep the
launcher_running shmem flag set to true during the exit handler as well. Fixed
on 0005.
* D2: Killing the launcher mid-*disable* leaves the cluster in `inprogress-off`
with nothing to finish the transition (an interrupted *enable* self-heals; a
disable does not)
- This preserves the state at inprogress-off and require a new disable
operation to clear it to off. The fix would be to execute
SetDataChecksumsOff() in the exit handler for this state as well, which I opted
out of doing since if you're terminating due to SetDataChecksumsOff() is
misbehaving, re-running it immediately again in the exit handlar is unlikely
what you want. Leaving it like this allows for using an offline disable in the
(perhaps unlikely) situation that SetDataChecksumsOff is broken.
* D3: pg_upgrade tells the user checksums are "being enabled" when the old
cluster was interrupted mid-*disable*
- Both inprogress-on and inprogress-off used the same pg_fatal error message
which indeed is misleading. Fixed in 0004.
* D4: pg_settings.enumvals for data_checksums is the phantom {""} instead of
the value list or NULL
- This is correct, the enum struct had the visibility flag backwards. Fixed in
0001.
* D5: pg_stat_progress_data_checksums.blocks_total/blocks_done are per-fork
though documented as per-relation, so blocks_done moves backward within one
relation
- Another correct finding, the documentation said "relation" but it should say
"relation fork", fixed in 0003. A future improvement would be to make the
current fork visible in the progress reporting (or switch to per-relation).
* D6: pg_enable/disable_data_checksums ship the delegable {POSTGRES=X} ACL yet
a hardcoded superuser() gate makes a granted EXECUTE silently ineffective; the
docs state no privilege model
- Correct, the ACL implied that access could be granted, but the functions are
strictly superuser only. Fixed, and the documentation updated in 0002.
GPT:
----
* A late temporary table is treated as pre-existing: Online enablement waits
for a session that it need not wait for
- The fact that processing need to wait for temporary tables is documented, and
while doing so it will use a specialized wait event highlighting this in the
progress reporting. It's true that the code can be optimized to not wait for
temporary tables which were created after the state changed to inprogress-on
but that's an improvement for future versions.
* pg_settings.enumvals` is {""}: Monitoring and introspection clients receive
false metadata
- Duplicate of Claude D4, fixed.
* A registered worker can outlive a terminated launcher: Relation I/O and
full-page-image WAL can begin after `SHOW data_checksums` reports `off` and the
launcher has exited
- This requires termination within a very tight window, and cannot lead to
wrong results or corruption. Will look at a fix.
* pg_upgrade misreports inprogress-off: Its fatal diagnostic says checksums are
being enabled while they are being disabled
- Duplicate of Claude D3, fixed.
* Progress block counters are per fork, not per relation as documented:
Monitors see unexplained resets and totals
- Duplicate of Claude D5, fixed.
* The documented function signature omits a valid one-argument call: Users are
not told they can set only cost_delay
- Fixed in 0003
* The function documentation omits the mandatory superuser requirement: Users
cannot tell who is authorized to run the operations
- Partial duplicate of Claude D6, fixed.
--
Daniel Gustafsson
Attachments:
[application/octet-stream] f19c0ec-checksum-online-claude-v0.patch (81.7K, ../../E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se/2-f19c0ec-checksum-online-claude-v0.patch)
download | inline diff:
From fba2cb102ac1e499aa44299891379ee714a6bab6 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Fri, 4 Sep 2026 20:12:30 +0000
Subject: [PATCH] Audit of f19c0ec (online data checksums): report + 6
reproducer tests
An automated, model-driven audit of commit f19c0ec ("Online enabling and
disabling of data checksums") finds six user-visible defects introduced by that
commit and still present at the tip of branch f19c0ec-checksum-online (50bb165),
which already carries four follow-up bug-fix commits. Each defect has a
standalone reproducer TAP test that fails on this branch because the defect is
present; the branch contains no fix.
D1 Cancelling the enable launcher lets a concurrent pg_enable start a SECOND
launcher; the race logs "data checksums are now enabled" while the cluster
is off, and the issued enable is silently lost. (medium)
D2 Killing the launcher mid-disable leaves the cluster in inprogress-off with
nothing to finish the transition; an interrupted enable self-heals, a
disable does not (recoverable by re-issuing disable or a restart). (low-med)
D3 pg_upgrade reports checksums "being enabled" for a cluster interrupted
mid-disable (the ChecksumStateType numeric order is not its semantic
order). (low)
D4 pg_settings.enumvals for data_checksums is the phantom {""} instead of the
value list or NULL. (low)
D5 pg_stat_progress_data_checksums block counters are tracked per fork though
documented per relation, so blocks_done moves backward within one
relation. (low)
D6 pg_enable/disable_data_checksums ship the delegable {POSTGRES=X} ACL yet a
hardcoded superuser() check makes a granted EXECUTE silently ineffective;
the docs state no privilege model. (low)
The tests are deliberately kept out of the meson/make suite: with no fix on the
branch they fail by design, and a correct fix for each defect turns its
# DEFECT: assertion green.
See AUDIT-REPORT-checksum-online.md for the full write-up (mechanism, file:line,
ancestry, one out-of-scope finding attributable to follow-up 05e7b3e, and four
refuted candidates) and PROVENANCE.md for the model, verbatim prompt, method,
the adaptive-usage controller and how it behaved, and the human review that
corrected the automation (a D2 over-claim, D6 promoted from "uncertain", and a
test bug fixed).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WBiRXNEsCL7PzXS998Jgin
---
AUDIT-REPORT-checksum-online.md | 290 ++++++++++++++++++
PROVENANCE.md | 194 ++++++++++++
.../audit-defect-tests/README.md | 42 +++
.../t/101_launcher_cancel_double_launcher.pl | 198 ++++++++++++
.../102_pg_upgrade_inprogress_off_message.pl | 183 +++++++++++
.../t/103_data_checksums_enumvals.pl | 138 +++++++++
.../t/104_progress_blocks_per_fork.pl | 178 +++++++++++
.../t/105_launcher_kill_disable_wedge.pl | 200 ++++++++++++
.../t/106_checksum_func_grant_defeated.pl | 112 +++++++
9 files changed, 1535 insertions(+)
create mode 100644 AUDIT-REPORT-checksum-online.md
create mode 100644 PROVENANCE.md
create mode 100644 src/test/modules/test_checksums/audit-defect-tests/README.md
create mode 100644 src/test/modules/test_checksums/audit-defect-tests/t/101_launcher_cancel_double_launcher.pl
create mode 100644 src/test/modules/test_checksums/audit-defect-tests/t/102_pg_upgrade_inprogress_off_message.pl
create mode 100644 src/test/modules/test_checksums/audit-defect-tests/t/103_data_checksums_enumvals.pl
create mode 100644 src/test/modules/test_checksums/audit-defect-tests/t/104_progress_blocks_per_fork.pl
create mode 100644 src/test/modules/test_checksums/audit-defect-tests/t/105_launcher_kill_disable_wedge.pl
create mode 100644 src/test/modules/test_checksums/audit-defect-tests/t/106_checksum_func_grant_defeated.pl
diff --git a/AUDIT-REPORT-checksum-online.md b/AUDIT-REPORT-checksum-online.md
new file mode 100644
index 0000000..6de8b60
--- /dev/null
+++ b/AUDIT-REPORT-checksum-online.md
@@ -0,0 +1,290 @@
+# Audit of commit f19c0ec — user-visible defects in online data checksums
+
+This branch reports **six user-visible defects** introduced by PostgreSQL commit
+`f19c0ec` ("Online enabling and disabling of data checksums") that are **still
+present** at the tip of branch `f19c0ec-checksum-online` (commit `50bb165`), which
+already carries four follow-up bug-fix commits. Each defect has a standalone
+reproducer TAP test that fails on this branch precisely because the defect is
+present. One further real problem was found but is **out of scope** (a follow-up
+commit, not `f19c0ec`, introduced it); four other candidates were **refuted**.
+
+Everything on this branch — this report, the tests, and `PROVENANCE.md` — was
+produced by an automated, model-driven audit and then verified by the repository
+owner's own review and re-runs. See `PROVENANCE.md` for exactly how, including
+the model, the prompt, and the parts the automation got wrong before correction.
+
+## Scope and method
+
+* **Audited commit:** `f19c0ec` (Daniel Gustafsson, 2026-04-03).
+* **Present-at-tip means:** observable at `50bb165`, i.e. after the four
+ post-`f19c0ec` follow-ups on this branch: `05e7b3e` (replay no longer adopts
+ another node's checksum state), `1d4d52d` (pg_checksums refuses interrupted
+ transitions), `85f7eed` (pg_rewind checks source/target states), `50bb165`
+ (pg_combinebackup refuses a mixed-state chain).
+* **A defect** is behavior contrary to the feature's own binding principles —
+ its documentation, the invariants stated in its commit message and code
+ comments, plain correctness, and consistency with how the rest of PostgreSQL
+ behaves in the equivalent situation — that a user can observe (through SQL, a
+ client tool, a log line, wrong monitoring output, a crash, a hang, or
+ corruption). Defects already fixed by the four follow-ups, or already asserted
+ by the shipped tests `t/001`–`t/024`, are out of scope.
+* **How they were verified:** each defect was reproduced on a live build
+ installed from this branch (meson, `cassert`+`debug`+`tap_tests`+
+ `injection_points`), and each is attributed to `f19c0ec` by `git blame` and by
+ confirming the four follow-ups do not touch the cited code. The reproducer
+ tests were re-run by hand against this branch; all fail at their `# DEFECT:`
+ assertions while their setup/context assertions pass.
+
+## Severity at a glance
+
+| # | Defect | Severity | Test |
+|---|--------|----------|------|
+| D1 | Cancelling the enable launcher lets a second launcher start; the race logs a false "data checksums are now enabled" while the cluster is off, and the enable is silently lost | medium | `t/101` |
+| D2 | Killing the launcher mid-*disable* leaves the cluster in `inprogress-off` with nothing to finish the transition (an interrupted *enable* self-heals; a disable does not) | low–medium | `t/105` |
+| D3 | `pg_upgrade` tells the user checksums are "being enabled" when the old cluster was interrupted mid-*disable* | low | `t/102` |
+| D4 | `pg_settings.enumvals` for `data_checksums` is the phantom `{""}` instead of the value list or NULL | low | `t/103` |
+| D5 | `pg_stat_progress_data_checksums.blocks_total`/`blocks_done` are per-fork though documented as per-relation, so `blocks_done` moves backward within one relation | low | `t/104` |
+| D6 | `pg_enable/disable_data_checksums` ship the delegable `{POSTGRES=X}` ACL yet a hardcoded `superuser()` gate makes a granted `EXECUTE` silently ineffective; the docs state no privilege model | low | `t/106` |
+
+All `file:line` references below are at this branch tip (`50bb165`).
+
+## Confirmed defects
+
+### D1 — a cancelled enable launcher lets a second launcher start, logging a false "now enabled" while checksums are off
+
+**User-visible effect.** After a superuser cancels an in-progress
+`pg_enable_data_checksums()` (for example with `pg_cancel_backend()`) and a
+concurrent `pg_enable_data_checksums()` runs, the server log records
+`data checksums are now enabled` even though `SHOW data_checksums` and
+`pg_controldata` both report **off**. The operator is told, in the log, that
+their data is now checksum-protected when it is not, and the enable they issued
+is silently lost.
+
+**Mechanism.** Cancelling the launcher makes `DataChecksumsWorkerLauncherMain`
+fall through to its `done:` label, which clears
+`DataChecksumState->launcher_running` at `datachecksum_state.c:1398-1399` while
+the cluster state is still `inprogress-on`. The mandatory reset to off is
+deferred to the `launcher_exit` shmem-exit callback
+(`datachecksum_state.c:1155-1156`), which runs the slow `SetDataChecksumsOff()`
+(two procsignal barriers plus two forced `CHECKPOINT_WAIT` checkpoints).
+Because `StartDataChecksumsWorkerLauncher` gates a new launcher solely on
+`launcher_running` (`:664`, `:682`), a concurrent `pg_enable_data_checksums()`
+in that window starts a **second** launcher. The two race; the surviving one
+reaches `SetDataChecksumsOn()` (`xlog.c:4852`) with the state no longer
+`inprogress-on`, so it emits `WARNING: cannot set data checksums to "on",
+current state is not "inprogress-on", disabling` and disables — yet
+`LauncherMain` then logs `data checksums are now enabled` unconditionally
+(`datachecksum_state.c:1351-1354`). This breaks the single-launcher invariant
+the feature was built around.
+
+**Test.** `t/101_launcher_cancel_double_launcher.pl` drives the window
+deterministically with the in-tree `injection_points` module (freezing the
+checkpointer at `create-checkpoint-initial`). The load-bearing assertion — the
+launcher must not log "data checksums are now enabled" while `data_checksums` is
+off — fails on this branch. Corroborating assertions (a second launcher with a
+new PID appears; the state moves backward `inprogress-off` → `inprogress-on`)
+pass only because the defect is present.
+
+### D2 — an interrupted *disable* is not self-healed, unlike an interrupted *enable*
+
+**User-visible effect.** If the datachecksums launcher is terminated
+(`pg_terminate_backend()`) while a `pg_disable_data_checksums()` is in its
+`inprogress-off` phase, the cluster is left in `inprogress-off` with no process
+to complete the transition to off. `SHOW data_checksums` reports
+`inprogress-off` indefinitely. The state is recoverable — re-issuing
+`pg_disable_data_checksums()` or restarting the server both finish it — but it
+does not resolve on its own, in contrast to an interrupted *enable*, which the
+same code path heals back to off.
+
+**Mechanism.** `launcher_exit` (`datachecksum_state.c:1155-1156`) contains a
+branch for an interrupted enable only:
+
+```c
+if (DataChecksumsInProgressOn())
+ SetDataChecksumsOff();
+```
+
+There is no symmetric branch for an interrupted disable (no
+`DataChecksumsInProgressOff()` helper exists), and `bgw_restart_time` is
+`BGW_NEVER_RESTART`, so nothing advances a disable that was interrupted after
+`SetDataChecksumsOff()` had persisted `inprogress-off` but before it reached
+off. This is an asymmetry in the feature's own cleanup logic, not a data-safety
+problem: in `inprogress-off` checksums are written but not verified, so no page
+fails verification.
+
+**Test.** `t/105_launcher_kill_disable_wedge.pl` contrasts the two directions
+with the same kill. The control (kill mid-enable → self-heals to off) passes;
+the defect assertion (kill mid-disable → should also reach off) fails, and the
+trailing assertions confirm the state is recovered by a re-issued
+`pg_disable_data_checksums()`.
+
+### D3 — `pg_upgrade` reports the wrong direction for a cluster interrupted mid-disable
+
+**User-visible effect.** `pg_upgrade` (and `pg_upgrade --check`) on an old
+cluster left in `inprogress-off` fails with `checksums are being enabled in the
+old cluster` — the exact opposite of what happened (the cluster was being
+*dis
+says `inprogress-off`; `pg_controldata` prints version 2). Refusing the upgrade
+is correct; only the wording is wrong.
+
+**Mechanism.** `src/bin/pg_upgrade/controldata.c:661-662`:
+
+```c
+if (oldctrl->data_checksum_version > PG_DATA_CHECKSUM_VERSION)
+ pg_fatal("data checksums are being enabled in the old cluster");
+```
+
+The `ChecksumStateType` enum (`storage/checksum.h`) appends `INPROGRESS_OFF = 2`
+and `INPROGRESS_ON = 3` after `PG_DATA_CHECKSUM_VERSION = 1`, so the `> 1` test
+correctly fires for **both** interrupted directions, but the single hardcoded
+message describes only the enable direction. (Commit `078aac2e` later prepended
+"data " to the string; it did not fix the direction, and is not one of the four
+scoped follow-ups.)
+
+**Test.** `t/102_pg_upgrade_inprogress_off_message.pl` brings a cluster cleanly
+to rest in `inprogress-off` (SIGSTOP a backend so the disable launcher blocks on
+its procsignal barrier, then SIGTERM the launcher) and asserts the `pg_upgrade`
+diagnostic does not claim the enable direction. It fails on this branch.
+
+### D4 — `pg_settings.enumvals` for `data_checksums` is the phantom `{""}`
+
+**User-visible effect.** `SELECT enumvals FROM pg_settings WHERE name =
+'data_checksums'` returns a one-element array whose single element is the empty
+string (`{""}`), in every state. A correct enum GUC reports either NULL (as the
+pre-`f19c0ec` boolean `data_checksums` did) or its list of valid values (as
+`wal_level` and the sibling read-only preset enum `huge_pages_status` do). The
+universal invariant `enumvals IS NULL OR setting = ANY(enumvals)`, which every
+other enum GUC satisfies, is violated: the current setting is never a member of
+its own advertised value list.
+
+**Mechanism.** `f19c0ec` turned `data_checksums` from bool into an enum GUC but
+marks all four entries of `data_checksums_options[]` hidden
+(`guc_tables.c:515-521`, third field `true`). With every entry hidden,
+`config_enum_get_options()` appends no names, so `pg_settings` builds the bogus
+`{""}` instead of NULL or the value list.
+
+**Test.** `t/103_data_checksums_enumvals.pl` checks the invariant against sibling
+enum GUCs (which pass) and against `data_checksums` (which fails), in both the
+off and on states.
+
+### D5 — progress view block counters are per-fork though documented as per-relation
+
+**User-visible effect.** While one relation is being processed,
+`pg_stat_progress_data_checksums.blocks_total` changes several times and
+`blocks_done` moves **backward**. `monitoring.sgml` documents both as "the
+number of blocks in the current relation" (total / processed), which implies a
+per-relation quantity that only grows. A monitoring tool or DBA watching the
+view sees `blocks_done` reset and `blocks_total` jump (for example to the main,
+then fsm, then vm fork sizes of one table).
+
+**Mechanism.** The worker sets `BLOCKS_TOTAL`/`BLOCKS_DONE` per fork in
+`ProcessSingleRelationFork` (`datachecksum_state.c:708-731`, and `:749`/`:785`
+update `BLOCKS_DONE`), so the counters describe the current *fork*, not the
+current *relation* as documented.
+
+**Test.** `t/104_progress_blocks_per_fork.pl` takes per-fork ground truth from
+`pg_relation_size('big','main'|'fsm'|'vm')`, throttles the enable with a per-block
+cost delay so the small forks stay observable, and asserts (a) `blocks_total`
+equals the whole-relation block count while the relation is current, and (b)
+`blocks_done` never decreases while the relation is current. Both fail on this
+branch.
+
+### D6 — a granted `EXECUTE` on the checksum functions is silently defeated by an internal `superuser()` gate
+
+**User-visible effect.** A superuser runs `GRANT EXECUTE ON FUNCTION
+pg_enable_data_checksums(int,int) TO alice`; the grant succeeds and
+`has_function_privilege('alice', 'pg_enable_data_checksums(int,int)', 'execute')`
+returns true. Yet `alice` (a non-superuser) calling the function gets `ERROR:
+must be superuser to change data checksum state`. For the identically-ACL'd
+`pg_reload_conf()` and `pg_switch_wal()`, the same grant lets `alice` run them.
+The recorded privilege is therefore a lie for these functions alone, and the
+documentation (`func-admin.sgml`, "Data Checksum Functions") states no privilege
+model at all, unlike every neighboring restricted admin function.
+
+**Mechanism.** Both functions ship the delegable ACL `proacl => '{POSTGRES=X}'`
+in `pg_proc.dat` — the ACL that, for the ~70 other admin functions carrying it,
+means "may be delegated with GRANT." But `f19c0ec` also hardcodes a
+`superuser()` gate in the bodies (`datachecksum_state.c:574-577` for disable,
+`:596-599` for enable), so a grant that the executor accepts is then rejected
+inside the function. Either the gate is redundant (drop it and honor the grant)
+or the ACL should not be delegable; shipping both is contradictory, and unique
+among `{POSTGRES=X}` functions.
+
+**Test.** `t/106_checksum_func_grant_defeated.pl` grants both functions to a
+non-superuser, confirms `has_function_privilege` is true (as for the passing
+control `pg_reload_conf`), and asserts — fix-agnostically — that a recorded
+`EXECUTE` grant actually permits execution. It fails on this branch because the
+grantee is rejected with "must be superuser". (A fix that instead makes the ACL
+non-delegable would flip `has_function_privilege` to false and satisfy the
+assertion vacuously.)
+
+## Found, but out of scope
+
+**`wal.sgml` "Replication" impact bullet is stale at the tip.** The bullet
+(`doc/src/sgml/wal.sgml`, ~lines 433-443) tells DBAs that a standby receiving an
+online checksum-state change "will issue a restartpoint" to flush the state into
+`pg_control`, that "the restartpoint will block redo," and that reducing
+`max_wal_size` shortens it. At the tip the state is persisted **inline** during
+redo via `UpdateControlFile()` and the only redo stall is a procsignal-barrier
+wait (`EmitAndWaitDataChecksumsBarrier`), which `max_wal_size` does not affect;
+no restartpoint is issued by the transition. So the documented cause and
+mitigation are wrong. **This is excluded** because the paragraph was accurate for
+`f19c0ec` as written; the behavior it describes was changed by follow-up
+`05e7b3e` (which reworked standby replay to persist inline), which left the
+paragraph stale. The defect was therefore introduced by a follow-up, not by
+`f19c0ec`. It is recorded here for completeness and is worth fixing on its own.
+
+## Considered and refuted
+
+* **"`LauncherMain` logs 'now enabled' unconditionally."** True structurally, but
+ in ordinary single-launcher operation `SetDataChecksumsOn()` succeeds, so the
+ log is truthful. The false log is reachable only via the D1 race, where it is
+ reported.
+* **"`pg_controldata` diverges from `pg_control_init()`."** The claim that the
+ SQL/CLI mirror is broken did not hold up: both expose the checksum version
+ consistently.
+* **"Transaction-wait is not surfaced in the progress view."** The `enabling`
+ phase is the documented phase during the initial transaction wait; no separate
+ phase is promised for it.
+* **"Enable waits only for transactions holding an XID, not all open
+ transactions."** Waiting on XID-holding transactions is the correct and
+ sufficient condition; a read-only open transaction cannot have created the
+ unlogged/new relations the wait exists to serialize against.
+
+## Running the tests
+
+The tests are standalone `PostgreSQL::Test` TAP scripts kept **outside** the
+shipped `meson` suite (they are reproducers that fail on this un-fixed branch, so
+wiring them into `meson test` would break CI by design). Run them against a build
+of this branch:
+
+```sh
+# build this branch with TAP + injection points, install to $INST
+meson setup build . -Dtap_tests=enabled -Dinjection_points=true \
+ -Dcassert=true -Ddebug=true --prefix=$INST
+ninja -C build install
+
+# run a reproducer (101, 105, 106 use injection points)
+PATH=$INST/bin:$PATH \
+PG_REGRESS=$PWD/build/src/test/regress/pg_regress \
+PERL5LIB=$PWD/src/test/perl \
+enable_injection_points=yes \
+ prove -v src/test/modules/test_checksums/audit-defect-tests/t/101_launcher_cancel_double_launcher.pl
+```
+
+Each test prints its `# DEFECT:` assertions as `not ok` on this branch, with
+diagnostics naming the observed wrong value; setup/context assertions pass. A
+correct fix for the corresponding defect turns the `not ok` into `ok`.
+
+## Caveats
+
+* The tests demonstrate the defects; they are **not** a fix, and this branch
+ contains no source change. Each test is written so it would pass under a
+ correct fix and fail now.
+* D1, D2, and D3 depend on `injection_points` (D1/D2) or on reaching a genuine
+ `inprogress-off` state (D3); they are deterministic given those tools but do
+ drive process-timing windows. D4, D5, and D6 are straightforwardly
+ deterministic.
+* Severities are the auditor's judgement. None of the six risks data corruption
+ or a false checksum failure; D1 (a misleading "now enabled" log plus a silently
+ lost enable) is the most consequential operationally.
diff --git a/PROVENANCE.md b/PROVENANCE.md
new file mode 100644
index 0000000..9442e90
--- /dev/null
+++ b/PROVENANCE.md
@@ -0,0 +1,194 @@
+# PROVENANCE
+
+Branch `f19c0ec-checksum-defect-tests` is the product of an automated, model-driven
+audit of PostgreSQL commit `f19c0ec` ("Online enabling and disabling of data
+checksums"). The report (`AUDIT-REPORT-checksum-online.md`), the six reproducer
+tests, and this file were written by a language model orchestrating many
+subagents. **No human wrote the report or the tests.** The repository owner
+supplied the prompt, then reviewed and re-ran the results: he re-ran every test,
+verified each defect's `git blame` ancestry himself, corrected one over-claim,
+promoted one finding the automation had left "uncertain", and fixed a bug in one
+test's error-handling. Those human interventions are listed under
+[Human review](#human-review-what-the-automation-got-wrong).
+
+## Tooling and model
+
+* **Tool:** Claude Code (Anthropic's agentic CLI), using its Workflow orchestration
+ feature — one JavaScript script driving many subagents through three phases
+ (find → adversarially verify → write reproducer tests).
+* **Model:** **Opus 4.8**, for the orchestrator and for every subagent (no
+ per-agent model override, so the subagents ran under the session model). The
+ session's default was initially set to Fable 5.1 (`claude-fable-5-1`) with
+ `/model`, but the session was switched to Opus partway through (a "[cyber]"
+ switch-to-Opus notice; the in-session `Co-Authored-By` identity changed to
+ "Claude Opus 4.8"; and the usage was billed as Opus). The workflow run and the
+ assembly therefore ran under Opus 4.8, which is also what the commit's
+ `Co-Authored-By` trailer records. (An earlier draft of this file said Fable 5.1,
+ taken from the assistant's system prompt; that does not reliably reflect the
+ serving model and was corrected once the billing, the switch notice, and the
+ co-author identity were reconciled.)
+* **Run by:** the repository owner (Noah Misch), interactively, from
+ `/home/nm/src/pg/postgresql`.
+* **Workflow run:** `wf_9e53ff76-24e`. 47 subagents, 0 errors, ~136 minutes,
+ ~5.39M subagent tokens / ~2.51M orchestrator-visible output tokens.
+
+## The prompt
+
+Verbatim, as typed by the repository owner:
+
+```
+Make a large workflow, with at most 90 agents, to write test cases covering
+user-visible defects in commit f19c0ec that are still present in branch
+f19c0ec-checksum-online. Use your own worktree; disregard the present dir
+except as repository to which to attach your worktree. The workflow should
+first look for extant user-visible defects. If it finds any, write a test
+case covering some of those defects. If any defects found weren't suitable to
+test, describe them in a report.
+
+The workflow should adaptively manage usage so, when it reaches a session
+limit, it loses 1-2 agents instead of 16 concurrent agents. (Example that you
+can improve on: start at 16 concurrent, but when one exits, estimate the
+burndown curve. If there's a decent chance an agent started now would fail
+due to session limit, don't replace the agent that just exited. Check the
+curve again at the next agent exit.)
+
+Commit the following on a fresh branch:
+- A report describing any defects found, testable or not. Prefix the report
+ with [no defects] if that's so.
+- Any tests written
+- A PROVENANCE.md file containing model, prompt, etc.
+```
+
+## Environment
+
+* **Audited commit:** `f19c0ec` "Online enabling and disabling of data checksums"
+ (Daniel Gustafsson, 2026-04-03).
+* **Branch under audit:** `f19c0ec-checksum-online`, tip `50bb165`. That tip is the
+ repository's `origin/master` (`6885b84`) plus one unrelated commit (`7344937`,
+ parallel-scan atomics) and the four checksum follow-ups `05e7b3e`, `1d4d52d`,
+ `85f7eed`, `50bb165`. So "present at the tip" means present after those four
+ follow-ups.
+* **This branch:** `f19c0ec-checksum-defect-tests`, based on `50bb165`, in a
+ dedicated `git worktree` at `/home/nm/src/pg/csum-audit` attached to the owner's
+ clone at `/home/nm/src/pg/postgresql`. The main working directory was used only
+ as the repository to attach the worktree to, per the prompt.
+* **Build:** meson, `-Dcassert=true -Ddebug=true -Doptimization=1
+ -Dtap_tests=enabled -Dinjection_points=true`, `CC="ccache gcc"`, installed to
+ `/home/nm/src/pg/csum-inst`.
+* **Host:** Debian 13 (trixie), Linux 6.12.94 x86_64.
+* **Harnesses (not part of this branch):** `/home/nm/src/pg/csumrun.sh` (throwaway
+ SQL cluster, `initdb --no-data-checksums`, `psql -X`) and
+ `/home/nm/src/pg/csumtap.sh` (standalone `PostgreSQL::Test` TAP against the
+ install). Intermediate evidence lives outside the branch under
+ `/home/nm/src/pg/csumwork` and the workflow transcript directory.
+
+## Scope rubric given to every agent
+
+A defect is behavior contrary to the feature's own binding principles — its
+documentation, invariants stated in its commit message and code comments, plain
+correctness/data-integrity, and consistency with how the rest of PostgreSQL
+behaves in the equivalent situation — that is **user-visible** (SQL, a client
+tool, a log line, monitoring output, a crash, a hang, corruption), **introduced by
+`f19c0ec`**, and **still present at the tip**. Explicitly out of scope: anything
+already fixed by the four follow-ups, anything already asserted by the shipped
+tests `t/001`–`t/024`, and internal-only cleanliness.
+
+## Method
+
+1. **Find** (18 subagents, one per subsystem lens: the enable/disable state
+ machine, launcher/worker lifecycle, progress view, checksum-failure stats, the
+ `data_checksums` GUC, `pg_control`/SRFs, `pg_upgrade`, logical decoding, base
+ backup, the read/write verify paths, single-node crash/restart, `pg_waldump`
+ desc, wait events, and a docs-vs-behavior sweep). Read-only analysis of the tip,
+ returning structured candidates. → 12 candidates.
+2. **Verify** (2 subagents per candidate = 24): a **reproducer** that builds and
+ runs a probe on the installed build, and a **skeptic** prompted to refute by
+ default (pre-existing? already fixed? already tested? internal-only? documented
+ behavior actually matches?). → 5 confirmed, 3 uncertain, 4 refuted.
+3. **Test** (one subagent per confirmed, testable defect = 5): write a standalone
+ TAP reproducer and iterate until it runs cleanly and fails on this branch for
+ the right reason.
+
+The six committed tests are the five the workflow wrote (`t/101`–`t/105`) plus one
+(`t/106`) the owner added while promoting an "uncertain" finding (see below).
+
+## Adaptive usage control (the prompt's second requirement)
+
+A single controller governed all three fan-out phases (`adaptiveRun` in the
+workflow script). It answers the prompt's requirement — lose 1-2 agents at a
+session limit, not 16 — with two mechanisms:
+
+* **Soft ramp.** A shared token baseline is captured once; as spend approaches an
+ estimated one-window ceiling (`CEILING_DELTA = 3,000,000` output tokens,
+ best-effort — the true session limit is not readable from the script and is
+ shared across concurrent sessions), the per-exit concurrency target shrinks, but
+ **never below one**, so a phase always finishes. Overshooting the true limit
+ while already at concurrency one loses at most that one agent.
+* **Hard-failure backstop.** The first agent that dies (a `null` return, the signal
+ a real limit gives) caps the target at 2; the second caps it at 1; a third drains
+ in-flight work and pauses, to be resumed from cache. This is what bounds the loss
+ to 1-2 even if the soft estimate is wrong.
+
+The target is recomputed at every agent exit — the prompt's "check the curve again
+at the next agent exit."
+
+**How it actually behaved this run** (from the workflow's own progress log): Find
+started at the requested 16. After the first of 16 concurrent finders returned, the
+shared spend already stood at 483k, so the running per-agent average was
+momentarily huge and the controller conservatively dropped the target to 3; as more
+finders completed and the average settled, it recovered to 15-16 and Find finished
+all 18. Verify ran at its cap of 4, Test at 3 (both lowered from 16 on purpose, to
+bound concurrent live clusters on a 2-CPU / ~2 GB-RAM host, independent of the
+session-limit logic). No agent ever failed (`fails 0` throughout), total spend
+(~2.51M) stayed under the ceiling, so the hard-failure backstop and the pause/resume
+path were implemented and exercised in the ramp but never triggered by a real limit.
+The early dip to target 3 is an artifact of measuring average cost from a shared,
+cumulative token counter while many agents are in flight; it is conservative (it
+throttles rather than over-commits) and self-corrected.
+
+## Human review — what the automation got wrong
+
+The owner reviewed the automation's output rather than trusting it, and changed
+three things:
+
+* **D2 over-claim corrected.** The automation titled the interrupted-disable
+ finding "permanently wedges the cluster in `inprogress-off`." Its own test
+ already showed the state is recovered by re-issuing `pg_disable_data_checksums()`
+ (and the docs note a restart resolves it). "Permanently" was removed from the
+ report and from the test's comments; the defect is the *asymmetry* (an
+ interrupted enable self-heals, an interrupted disable does not), not a permanent
+ wedge.
+* **D6 promoted from "uncertain".** The skeptic had refuted the GRANT/`superuser()`
+ finding as defensible design. On review it is a genuine, if low-severity,
+ inconsistency: the `{POSTGRES=X}` ACL is delegable and `has_function_privilege`
+ returns true, yet the grant is silently ineffective — unique among `{POSTGRES=X}`
+ functions. It was promoted to a confirmed defect and given a new, fix-agnostic
+ test (`t/106`), written and verified by the owner during review.
+* **A test bug was fixed.** `t/106`'s first draft used `on_error_stop => 0` when
+ shelling out as the granted role, so a SQL error left `psql`'s exit status at 0
+ and the helper reported success; the test passed spuriously. The helper was
+ changed to detect the error on stderr, after which the test correctly fails at
+ its defect assertions.
+
+The owner also verified independently, by `git blame` and by diffing the four
+follow-ups, that each confirmed defect is attributable to `f19c0ec` and untouched
+by the follow-ups; and re-ran all six tests against this branch, confirming each
+fails only at its `# DEFECT:` assertions.
+
+One further real problem — a stale "Replication" paragraph in `wal.sgml` — was
+found but **excluded**: the paragraph was accurate for `f19c0ec` and became wrong
+only when follow-up `05e7b3e` changed standby replay, so it is a follow-up defect,
+not `f19c0ec`'s. It is described in the report for completeness.
+
+## What is on this branch
+
+```
+AUDIT-REPORT-checksum-online.md the report (6 defects + 1 out-of-scope + 4 refuted)
+PROVENANCE.md this file
+src/test/modules/test_checksums/audit-defect-tests/README.md how to run the tests
+src/test/modules/test_checksums/audit-defect-tests/t/101..106 six reproducer TAP tests
+```
+
+The tests are intentionally not wired into the `meson`/`make` suite: this branch
+carries no fix, so they fail by design, and a correct fix for each defect turns its
+failing assertion green.
diff --git a/src/test/modules/test_checksums/audit-defect-tests/README.md b/src/test/modules/test_checksums/audit-defect-tests/README.md
new file mode 100644
index 0000000..fd2699f
--- /dev/null
+++ b/src/test/modules/test_checksums/audit-defect-tests/README.md
@@ -0,0 +1,42 @@
+# audit-defect-tests — reproducers for f19c0ec defects still present at this tip
+
+These six standalone TAP tests each **demonstrate a distinct user-visible defect**
+introduced by commit `f19c0ec` ("Online enabling and disabling of data checksums")
+and still present at the tip of branch `f19c0ec-checksum-online`. The full write-up
+(mechanism, `file:line`, ancestry, severity) is in `AUDIT-REPORT-checksum-online.md`
+at the repository root; provenance is in `PROVENANCE.md`.
+
+They are deliberately kept **out of the `meson`/`make` test suite**: this branch
+carries no fix, so every test fails on purpose (that failure is the demonstration),
+and wiring them in would break CI. A correct fix for a given defect turns its
+`# DEFECT:` assertion from `not ok` into `ok`.
+
+| Test | Defect | Uses injection points |
+|------|--------|:---:|
+| `t/101_launcher_cancel_double_launcher.pl` | D1: cancelled enable launcher → second launcher → false "now enabled" log while off | yes |
+| `t/102_pg_upgrade_inprogress_off_message.pl` | D3: `pg_upgrade` says "being enabled" for a mid-*disable* cluster | no* |
+| `t/103_data_checksums_enumvals.pl` | D4: `pg_settings.enumvals` is the phantom `{""}` | no |
+| `t/104_progress_blocks_per_fork.pl` | D5: progress block counters are per-fork, documented per-relation | no |
+| `t/105_launcher_kill_disable_wedge.pl` | D2: interrupted *disable* not self-healed (recoverable, not automatic) | yes |
+| `t/106_checksum_func_grant_defeated.pl` | D6: granted `EXECUTE` defeated by internal `superuser()` gate | no |
+
+\* `t/102` reaches a real `inprogress-off` state with SIGSTOP/SIGTERM, not an
+injection point.
+
+## Run
+
+From a build of this branch (configure with `-Dtap_tests=enabled
+-Dinjection_points=true`, installed to `$INST`):
+
+```sh
+PATH=$INST/bin:$PATH \
+PG_REGRESS=$PWD/build/src/test/regress/pg_regress \
+PERL5LIB=$PWD/src/test/perl \
+enable_injection_points=yes \
+ prove -v src/test/modules/test_checksums/audit-defect-tests/t/*.pl
+```
+
+Each script starts its own cluster (checksums off via `--no-data-checksums` where
+it tests online enabling) with small `shared_buffers`; runtimes are a few seconds
+each. On this branch every script ends `Result: FAIL` with its `# DEFECT:`
+assertion(s) as `not ok` and its setup/context assertions as `ok`.
diff --git a/src/test/modules/test_checksums/audit-defect-tests/t/101_launcher_cancel_double_launcher.pl b/src/test/modules/test_checksums/audit-defect-tests/t/101_launcher_cancel_double_launcher.pl
new file mode 100644
index 0000000..bd59f02
--- /dev/null
+++ b/src/test/modules/test_checksums/audit-defect-tests/t/101_launcher_cancel_double_launcher.pl
@@ -0,0 +1,198 @@
+# 101_launcher_cancel_double_launcher.pl
+#
+# Reproducer for a launcher-lifecycle defect introduced by f19c0ec
+# ("Online enabling and disabling of data checksums").
+#
+# THE DEFECT
+# Cancelling (SIGINT / pg_cancel_backend) an in-progress ENABLE launcher
+# makes DataChecksumsWorkerLauncherMain() fall through to its `done:` label,
+# which clears DataChecksumState->launcher_running (datachecksum_state.c:1399)
+# while the cluster state is still "inprogress-on". The mandatory reset to
+# "off" is deferred to the launcher_exit shmem-exit callback (:1155-1156),
+# which runs SetDataChecksumsOff() -- a long, non-atomic operation (two
+# procsignal barriers + two forced CHECKPOINT_WAIT checkpoints).
+# StartDataChecksumsWorkerLauncher() gates a new launcher solely on
+# launcher_running (:664/:682), so during that window a concurrent
+# pg_enable_data_checksums() starts a SECOND launcher. The two launchers
+# then race on the checksum state. The surviving (second) launcher reaches
+# SetDataChecksumsOn() with the state no longer "inprogress-on"; that helper
+# bails with a WARNING ("cannot set data checksums to \"on\" ...") and calls
+# SetDataChecksumsOff(), yet LauncherMain UNCONDITIONALLY logs
+# "data checksums are now enabled" (:1354). Net result: the server log
+# reports a successful enable while data_checksums and pg_controldata both
+# report OFF -- a false success, and the user's enable is silently lost.
+#
+# This breaks the single-launcher invariant the feature was designed around
+# (feature-dev commit bf25e55, which "fixes a bug where the launcher would
+# erroneously revert back to the off state").
+#
+# We drive the window deterministically with the standard in-tree
+# "injection_points" test module (NOT test_checksums): freezing the
+# checkpointer at "create-checkpoint-initial" stalls the exit-time
+# SetDataChecksumsOff() inside its first forced checkpoint, holding
+# launcher_running = false open long enough for the concurrent enable.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Keep any accidental stall bounded rather than hanging on the global default.
+$PostgreSQL::Test::Utils::timeout_default = 120;
+
+my $node = PostgreSQL::Test::Cluster->new('csum_launcher');
+$node->init(no_data_checksums => 1);
+$node->append_conf('postgresql.conf', <<'CONF');
+shared_buffers = 16MB
+max_worker_processes = 16
+checkpoint_timeout = 1h
+max_wal_size = 4GB
+log_min_messages = info
+CONF
+$node->start;
+
+# injection_points is a standard in-tree test module, not test_checksums.
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+
+sub csum_state
+{
+ return $_[0]->safe_psql('postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums'");
+}
+
+sub launcher_pid
+{
+ return $_[0]->safe_psql('postgres',
+ "SELECT coalesce((SELECT pid FROM pg_stat_activity"
+ . " WHERE backend_type = 'datachecksums launcher' LIMIT 1)::text, '')");
+}
+
+# ---- setup (all of this must PASS) --------------------------------------
+is(csum_state($node), 'off', 'setup: cluster starts with checksums off');
+my $logstart = -s $node->logfile;
+
+# A never-ending open transaction so any ENABLE launcher parks in
+# WaitForAllTransactionsToFinish() while the state is "inprogress-on".
+my $holder = $node->background_psql('postgres');
+$holder->query_safe('BEGIN');
+$holder->query_safe('SELECT txid_current()');
+
+# Session A: start ENABLE launcher L1. It reaches "inprogress-on" and parks.
+$node->safe_psql('postgres', 'SELECT pg_enable_data_checksums(0, 100)');
+ok( $node->poll_query_until(
+ 'postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums'",
+ 'inprogress-on'),
+ 'setup: launcher L1 moved the cluster to inprogress-on');
+ok( $node->poll_query_until(
+ 'postgres',
+ "SELECT count(*) > 0 FROM pg_stat_activity"
+ . " WHERE backend_type = 'datachecksums launcher'"
+ . " AND query LIKE 'Waiting for transactions older than%'"),
+ 'setup: launcher L1 parked waiting for the open transaction');
+my $l1 = launcher_pid($node);
+note "L1 launcher pid = $l1";
+
+# Freeze the checkpointer so L1's exit-time SetDataChecksumsOff() blocks inside
+# its first forced checkpoint, holding launcher_running = false open.
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('create-checkpoint-initial', 'wait')");
+
+# ---- open the window: cancel L1 -----------------------------------------
+# pg_cancel_backend -> abort_requested -> ProcessAllDatabases() returns false
+# -> `done:` clears launcher_running (state still inprogress-on) -> proc_exit
+# -> launcher_exit -> SetDataChecksumsOff() -> blocks in RequestCheckpoint.
+$node->safe_psql('postgres', "SELECT pg_cancel_backend($l1)");
+$node->wait_for_event('checkpointer', 'create-checkpoint-initial');
+
+# The exit-time disable has published "inprogress-off" and is now stalled in
+# the checkpoint, with launcher_running already cleared. (Confirms the window
+# is open.)
+ok( $node->poll_query_until(
+ 'postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums'",
+ 'inprogress-off'),
+ 'window open: L1 cleared launcher_running and began the exit-time disable (inprogress-off)');
+
+# ---- a concurrent enable starts a SECOND launcher (corroborating) -------
+# These next checks PASS only because the defect is present; a correct
+# single-launcher implementation would refuse to start a second launcher here.
+$node->safe_psql('postgres', 'SELECT pg_enable_data_checksums(0, 100)');
+ok( $node->poll_query_until(
+ 'postgres',
+ "SELECT count(*) FROM pg_stat_activity"
+ . " WHERE backend_type = 'datachecksums launcher' AND pid <> $l1",
+ '1'),
+ 'corroborating: a SECOND launcher started while L1 was still exiting');
+my $l2 = launcher_pid($node);
+note "L2 launcher pid = $l2";
+ok($l2 ne '' && $l2 ne $l1,
+ "corroborating: launcher pid changed L1=$l1 -> L2=$l2 (single-launcher invariant broken)");
+ok( $node->poll_query_until(
+ 'postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums'",
+ 'inprogress-on'),
+ 'corroborating: checksum state moved BACKWARDS inprogress-off -> inprogress-on under L2');
+
+# ---- let the two launchers race to completion ---------------------------
+# Release the checkpointer: L1 finishes disabling to OFF (clobbering L2's
+# inprogress-on) while L2 stays parked on the open transaction.
+$node->safe_psql('postgres',
+ "SELECT injection_points_wakeup('create-checkpoint-initial')");
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach('create-checkpoint-initial')");
+ok( $node->poll_query_until(
+ 'postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums'",
+ 'off'),
+ 'L1 finished its exit-time disable: state back to off');
+
+# Release the open transaction so L2 proceeds to SetDataChecksumsOn() while the
+# state is 'off'. The guard fires and disables, but the launcher still logs
+# "data checksums are now enabled".
+$holder->query_safe('COMMIT');
+$holder->quit;
+ok( $node->poll_query_until(
+ 'postgres',
+ "SELECT count(*) = 0 FROM pg_stat_activity"
+ . " WHERE backend_type = 'datachecksums launcher'"),
+ 'both launchers have exited');
+
+my $final = csum_state($node);
+note "FINAL data_checksums = $final";
+
+my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $logstart);
+my $said_enabled = ($log =~ /data checksums are now enabled/) ? 1 : 0;
+my $said_warning = ($log =~ /cannot set data checksums to "on"/) ? 1 : 0;
+my ($ctl) = run_command([ 'pg_controldata', '-D', $node->data_dir ]);
+my ($ctlver) = ($ctl =~ /Data page checksum version:\s+(\d+)/);
+$ctlver = defined $ctlver ? $ctlver : '?';
+note
+ "log says 'now enabled' = $said_enabled; guard WARNING = $said_warning; controldata version = $ctlver";
+
+# Corroborating evidence of the race outcome (both hold only while the defect
+# is present):
+ok($said_enabled,
+ 'corroborating: launcher logged "data checksums are now enabled"');
+ok($said_warning,
+ 'corroborating: SetDataChecksumsOn() hit its "current state is not inprogress-on" guard and disabled');
+is($final, 'off',
+ 'corroborating: SHOW data_checksums is off despite the "now enabled" log');
+is($ctlver, '0',
+ 'corroborating: pg_controldata reports checksum version 0 (off)');
+
+# ---- THE DEMONSTRATION --------------------------------------------------
+# DEFECT: The launcher must never report "data checksums are now enabled"
+# DEFECT: unless data checksums actually reached the "on" state. A cancelled
+# DEFECT: enable followed by a concurrent enable must NOT leave the cluster
+# DEFECT: reporting off while the server log claims a successful enable: the
+# DEFECT: enable must either succeed (leaving checksums on) or fail loudly --
+# DEFECT: never log false success. Under a correct single-launcher
+# DEFECT: implementation this assertion PASSES; on this branch it FAILS
+# DEFECT: because the second launcher logs "now enabled" while state is off.
+ok( !($said_enabled && $final eq 'off'),
+ 'DEFECT: launcher must not log "data checksums are now enabled" while checksums are off');
+
+$node->stop;
+done_testing();
diff --git a/src/test/modules/test_checksums/audit-defect-tests/t/102_pg_upgrade_inprogress_off_message.pl b/src/test/modules/test_checksums/audit-defect-tests/t/102_pg_upgrade_inprogress_off_message.pl
new file mode 100644
index 0000000..bd1a6ac
--- /dev/null
+++ b/src/test/modules/test_checksums/audit-defect-tests/t/102_pg_upgrade_inprogress_off_message.pl
@@ -0,0 +1,183 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# DEFECT DEMONSTRATION for commit f19c0ec ("Online enabling and disabling of
+# data checksums").
+#
+# pg_upgrade's control-data guard (src/bin/pg_upgrade/controldata.c) rejects an
+# old cluster whose data-checksum state is any in-progress state:
+#
+# if (oldctrl->data_checksum_version > PG_DATA_CHECKSUM_VERSION)
+# pg_fatal("data checksums are being enabled in the old cluster");
+#
+# Because the ChecksumStateType enum appends INPROGRESS_OFF (= 2) and
+# INPROGRESS_ON (= 3) after PG_DATA_CHECKSUM_VERSION (= 1), the "> 1" test
+# correctly fires for BOTH interrupted directions -- but the single hardcoded
+# message only describes the "being enabled" direction. For a cluster that was
+# interrupted mid-DISABLE (state INPROGRESS_OFF, "Data page checksum version: 2"
+# in the control file) the message states the exact OPPOSITE of what happened,
+# and contradicts the feature's own canonical naming: SHOW data_checksums says
+# 'inprogress-off' and pg_controldata prints version 2.
+#
+# This test builds an old cluster that comes cleanly to rest in INPROGRESS_OFF
+# and inspects pg_upgrade's diagnostic. Refusing the upgrade is CORRECT (the
+# code comment says to "disallow the upgrade"), so the DEFECT assertions target
+# only the wording, not the exit status.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+use Cwd qw(abs_path);
+
+# ------------------------------------------------------------------ helpers
+
+# Run a command; return (stdout, stderr, exitcode).
+sub run_capture
+{
+ my (@cmd) = @_;
+ my ($out, $err) = ('', '');
+ IPC::Run::run(\@cmd, '>' => \$out, '2>' => \$err);
+ my $rc = $? >> 8;
+ return ($out, $err, $rc);
+}
+
+# Read checksum version and cluster state from an offline data dir via the
+# pg_controldata binary.
+sub controldata
+{
+ my ($bindir, $datadir) = @_;
+ my ($out, $err, $rc) =
+ run_capture("$bindir/pg_controldata", '-D' => $datadir);
+ die "pg_controldata failed (rc=$rc): $err" if $rc != 0;
+ my ($ver) = $out =~ /^Data page checksum version:\s*(\d+)/m;
+ my ($state) = $out =~ /^Database cluster state:\s*(.+?)\s*$/m;
+ return ($ver, $state);
+}
+
+# ------------------------------------------------------------ set up old node
+
+# initdb enables data checksums by default in this build, which is exactly the
+# starting point for an online DISABLE.
+my $old = PostgreSQL::Test::Cluster->new('old');
+$old->init;
+$old->append_conf('postgresql.conf',
+ "shared_buffers = '16MB'\nmax_worker_processes = 16\n");
+$old->start;
+
+my $bindir = $old->config_data('--bindir');
+
+is($old->safe_psql('postgres', 'SHOW data_checksums'),
+ 'on', 'old cluster starts with data checksums on (setup)');
+
+# ---- Interrupt an online DISABLE so the cluster is pinned in INPROGRESS_OFF ----
+#
+# Hold one backend where it cannot absorb a procsignal barrier by SIGSTOPping
+# it. pg_disable_data_checksums() launches the "datachecksums launcher", which
+# writes INPROGRESS_OFF to the control file and then blocks in
+# WaitForProcSignalBarrier() waiting for the stopped backend -- so the state is
+# deterministically pinned at INPROGRESS_OFF and cannot advance to full "off".
+
+my $stall = $old->background_psql('postgres');
+my $stall_pid = $stall->query('SELECT pg_backend_pid()');
+$stall_pid =~ s/\D//g;
+like($stall_pid, qr/^\d+$/, 'obtained a backend pid to stall (setup)');
+is(kill('STOP', $stall_pid), 1, 'SIGSTOP the stall backend (setup)');
+
+# The SQL function returns as soon as the launcher has been started.
+$old->safe_psql('postgres', 'SELECT pg_disable_data_checksums()');
+
+# Confirm the disable is pinned at inprogress-off.
+ok( $old->poll_query_until(
+ 'postgres',
+ "SELECT current_setting('data_checksums')",
+ 'inprogress-off'),
+ 'online disable is pinned at inprogress-off (setup)');
+
+# Terminate the launcher directly. launcher_exit() reverts only the
+# INPROGRESS_ON direction, so an interrupted DISABLE keeps INPROGRESS_OFF.
+my $launcher_pid = '';
+foreach my $i (1 .. 100)
+{
+ $launcher_pid = $old->safe_psql('postgres',
+ "SELECT pid FROM pg_stat_activity WHERE backend_type = 'datachecksums launcher'"
+ );
+ last if $launcher_pid ne '';
+ usleep(100_000);
+}
+like($launcher_pid, qr/^\d+$/, 'located the datachecksums launcher (setup)');
+is(kill('TERM', $launcher_pid), 1, 'SIGTERM the launcher (setup)');
+
+# Wait until the launcher is gone, so nothing can complete the transition.
+ok( $old->poll_query_until(
+ 'postgres',
+ "SELECT count(*) FROM pg_stat_activity WHERE backend_type = 'datachecksums launcher'",
+ '0'),
+ 'launcher has exited, state left at inprogress-off (setup)');
+
+# With no launcher, it is now safe to resume the stalled backend.
+is(kill('CONT', $stall_pid), 1, 'SIGCONT the stall backend (setup)');
+$stall->quit;
+
+is($old->safe_psql('postgres', 'SHOW data_checksums'),
+ 'inprogress-off',
+ 'state remains inprogress-off after launcher exit (setup)');
+
+# A clean (fast) shutdown persists the interrupted state in the control file.
+$old->stop('fast');
+
+my ($old_ver, $old_state) = controldata($bindir, $old->data_dir);
+is($old_ver, '2',
+ 'old control file records checksum version 2 (inprogress-off) (setup)');
+like($old_state, qr/shut down/, 'old cluster is cleanly shut down (setup)');
+
+# ------------------------------------------------------------ set up new node
+
+my $new = PostgreSQL::Test::Cluster->new('new');
+$new->init;
+$new->append_conf('postgresql.conf',
+ "shared_buffers = '16MB'\nmax_worker_processes = 16\n");
+# The new cluster need not be started: pg_upgrade reads control data (and fires
+# this diagnostic) before starting any server.
+
+# ------------------------------------------------------------ run pg_upgrade
+
+# Resolve absolute paths (node data_dir/host are relative to the harness cwd)
+# and leave cwd unchanged (it is a writable temp dir where pg_upgrade may create
+# its output directory).
+my $old_datadir = abs_path($old->data_dir);
+my $new_datadir = abs_path($new->data_dir);
+my $sockdir = abs_path($new->host);
+
+my @cmd = (
+ "$bindir/pg_upgrade", '--check', '--no-sync',
+ '--old-datadir' => $old_datadir,
+ '--new-datadir' => $new_datadir,
+ '--old-bindir' => $bindir,
+ '--new-bindir' => $bindir,
+ '--socketdir' => $sockdir,
+ '--username' => 'postgres');
+
+my ($out, $err, $rc) = run_capture(@cmd);
+my $msg = "$out\n$err";
+note "pg_upgrade --check exit=$rc; output:\n$msg";
+
+# Refusing to upgrade a cluster mid-transition is the correct behavior.
+is($rc, 1,
+ 'pg_upgrade --check refuses an in-progress old cluster (correct behavior)');
+
+# DEFECT: the old cluster was interrupted while DISABLING checksums
+# (inprogress-off / control-file version 2). A correct diagnostic must NOT tell
+# the user checksums are "being enabled"; it should name the disable direction
+# or be direction-neutral.
+unlike($msg, qr/being enabled/i,
+ 'pg_upgrade must not report "being enabled" for a cluster interrupted mid-DISABLE');
+
+# DEFECT: the diagnostic should reflect the actual situation -- a disable in
+# progress (inprogress-off) -- consistent with SHOW data_checksums and
+# pg_controldata (version 2).
+like($msg, qr/disabl|in ?progress|processing/i,
+ 'pg_upgrade diagnostic should reflect the disable/in-progress direction');
+
+done_testing();
diff --git a/src/test/modules/test_checksums/audit-defect-tests/t/103_data_checksums_enumvals.pl b/src/test/modules/test_checksums/audit-defect-tests/t/103_data_checksums_enumvals.pl
new file mode 100644
index 0000000..915ad55
--- /dev/null
+++ b/src/test/modules/test_checksums/audit-defect-tests/t/103_data_checksums_enumvals.pl
@@ -0,0 +1,138 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+#
+# Reproducer for a defect introduced by commit f19c0ec ("Online enabling and
+# disabling of data checksums"). That commit turned the "data_checksums" GUC
+# from a bool into an enum GUC, but its option table
+# (data_checksums_options[] in src/backend/utils/misc/guc_tables.c) marks ALL
+# FOUR values -- "on"/"off"/"inprogress-on"/"inprogress-off" -- as hidden.
+#
+# Because every entry is hidden, config_enum_get_options() (guc.c) appends no
+# value names, so when pg_settings builds the "enumvals" column (guc_funcs.c)
+# it produces the bogus one-element array {""} -- an array whose single
+# element is the empty string -- in EVERY checksum state. A correct enum GUC
+# reports either NULL (as the pre-f19c0ec bool data_checksums did) or the list
+# of valid values (as wal_level and the sibling read-only preset enum
+# huge_pages_status do).
+#
+# The universal, machine-checkable invariant that every other enum GUC in the
+# tree satisfies is:
+#
+# enumvals IS NULL OR setting = ANY(enumvals)
+#
+# i.e. the current value is always a member of its own advertised value list
+# (or the list is absent). data_checksums violates it because its setting
+# ('off', 'on', ...) is never a member of {""}. The failing assertions below
+# are written to expect the CORRECT behavior and therefore fail on this
+# branch; that failure is the demonstration.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# Start a cluster with data checksums OFF, so that the defect can be observed
+# both in the initial "off" state and across an online enable to "on".
+my $node = PostgreSQL::Test::Cluster->new('csum_enumvals');
+$node->init(no_data_checksums => 1);
+$node->append_conf(
+ 'postgresql.conf', qq{
+shared_buffers = 16MB
+max_worker_processes = 16
+});
+$node->start;
+
+# Convenience: fetch a single scalar from pg_settings for data_checksums.
+sub dc
+{
+ my ($expr) = @_;
+ return $node->safe_psql('postgres',
+ "SELECT $expr FROM pg_settings WHERE name = 'data_checksums'");
+}
+
+# Emit the full enumvals picture as a diagnostic so failures are self-evident.
+sub diag_enumvals
+{
+ my ($label) = @_;
+ my $row = $node->safe_psql(
+ 'postgres', qq{
+ SELECT format('setting=%s enumvals=%s in_enumvals=%s',
+ setting,
+ coalesce(enumvals::text, 'NULL'),
+ coalesce((setting = ANY(enumvals))::text, 'NULL'))
+ FROM pg_settings WHERE name = 'data_checksums'});
+ diag("data_checksums [$label]: $row");
+}
+
+# ---------------------------------------------------------------------------
+# Setup / context assertions (these PASS): confirm data_checksums is an enum
+# GUC and that the equivalent enum GUCs elsewhere in the tree behave correctly.
+# This is what makes data_checksums' behavior a defect rather than a choice.
+# ---------------------------------------------------------------------------
+
+is(dc('vartype'), 'enum',
+ 'data_checksums is an enum GUC (context for the defect)');
+
+# Every other enum GUC satisfies "current setting is a member of enumvals".
+# huge_pages_status is the closest sibling: a read-only (PGC_INTERNAL) preset
+# enum GUC, exactly like data_checksums.
+for my $g (qw(huge_pages_status wal_level))
+{
+ my $ok = $node->safe_psql('postgres',
+ "SELECT setting = ANY(enumvals) FROM pg_settings WHERE name = '$g'");
+ is($ok, 't',
+ "sibling enum GUC $g: current setting is a member of its enumvals");
+}
+
+diag_enumvals('checksums off');
+
+# ---------------------------------------------------------------------------
+# Defect demonstration in the OFF state (these FAIL on this branch).
+# ---------------------------------------------------------------------------
+
+# DEFECT: pg_settings.enumvals for an enum GUC must be NULL or list the valid
+# values, and the current setting must be one of them -- so
+# "enumvals IS NULL OR setting = ANY(enumvals)" must return 't', exactly as it
+# does for wal_level, huge_pages_status and every other enum GUC. On this
+# branch enumvals is {""} and setting is 'off', so the query returns 'f'.
+is(dc('enumvals IS NULL OR setting = ANY(enumvals)'),
+ 't',
+ 'data_checksums (off): setting is a member of enumvals, or enumvals is NULL'
+);
+
+# DEFECT: enumvals must not be the phantom single-empty-string array {""}. A
+# correct implementation reports NULL (like the pre-f19c0ec bool GUC) or the
+# real values {on,off,inprogress-on,inprogress-off}; both are DISTINCT FROM
+# {""}. On this branch enumvals is exactly {""}, so this returns 'f'.
+# (IS DISTINCT FROM is NULL-safe, so a NULL-enumvals fix still passes.)
+is(dc(q{enumvals IS DISTINCT FROM '{""}'::text[]}),
+ 't',
+ 'data_checksums (off): enumvals is not the phantom empty-string array {""}'
+);
+
+# ---------------------------------------------------------------------------
+# Online-enable the checksums; the enable itself is setup and PASSES. Then
+# confirm the defect persists in the ON state.
+# ---------------------------------------------------------------------------
+
+$node->safe_psql('postgres', 'SELECT pg_enable_data_checksums()');
+$node->poll_query_until('postgres',
+ "SELECT setting = 'on' FROM pg_settings WHERE name = 'data_checksums'")
+ or die "timed out waiting for data checksums to be enabled";
+pass('online enable of data checksums reached the "on" state (setup)');
+
+diag_enumvals('checksums on');
+
+# DEFECT: the same invariant must hold in the 'on' state -- the setting is now
+# 'on', which must be a member of enumvals. On this branch enumvals is still
+# {""}, so the setting is never a member of its own value list in any state,
+# and this returns 'f'.
+is(dc('enumvals IS NULL OR setting = ANY(enumvals)'),
+ 't',
+ 'data_checksums (on): setting is a member of enumvals, or enumvals is NULL'
+);
+
+$node->stop;
+
+done_testing();
diff --git a/src/test/modules/test_checksums/audit-defect-tests/t/104_progress_blocks_per_fork.pl b/src/test/modules/test_checksums/audit-defect-tests/t/104_progress_blocks_per_fork.pl
new file mode 100644
index 0000000..c25fe49
--- /dev/null
+++ b/src/test/modules/test_checksums/audit-defect-tests/t/104_progress_blocks_per_fork.pl
@@ -0,0 +1,178 @@
+# Copyright (c) 2025, PostgreSQL Global Development Group
+#
+# Reproducer for a user-visible defect in the pg_stat_progress_data_checksums
+# view added by commit f19c0ec ("Online enabling and disabling of data
+# checksums").
+#
+# The documentation (doc/src/sgml/monitoring.sgml) states that the view's
+# blocks_total / blocks_done columns count:
+# "The number of blocks in the current RELATION which will be processed"
+# "The number of blocks in the current RELATION which have been processed."
+#
+# But the worker tracks those counters PER FORK, not per relation:
+# ProcessSingleRelationByOid() loops over every fork of a relation
+# (datachecksum_state.c, the "for (ForkNumber fnum ...)" loop), and for each
+# fork ProcessSingleRelationFork() unconditionally sets
+# PROGRESS_DATACHECKSUMS_BLOCKS_TOTAL = <blocks in THIS fork>
+# PROGRESS_DATACHECKSUMS_BLOCKS_DONE = 0
+# and then counts that fork's blocks up from 0. relations_done is only bumped
+# after the WHOLE relation finishes.
+#
+# So for any relation with more than one fork carrying blocks -- e.g. every
+# vacuumed heap, which has main + fsm + vm forks -- while that single relation
+# is "current":
+# * blocks_total takes several different values (one per fork), and never the
+# whole-relation block count the documentation promises; and
+# * blocks_done resets to 0 and thus moves BACKWARD at every fork boundary.
+#
+# This test drives an online enable under a heavy cost-delay throttle and
+# samples the progress view, using the worker's pg_stat_activity string
+# ("processing: public.big (<fork>, <n> blocks)") as ground truth for which
+# relation/fork is current. The two "# DEFECT:" assertions state the correct,
+# per-relation behavior and therefore FAIL on this branch.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+# ------------------------------------------------------------------ setup ---
+# Start a cluster with data checksums OFF so we can enable them online.
+my $node = PostgreSQL::Test::Cluster->new('csum_progress');
+$node->init(no_data_checksums => 1);
+$node->append_conf('postgresql.conf',
+ "shared_buffers = 16MB\n"
+ . "max_worker_processes = 16\n");
+$node->start;
+is($node->safe_psql('postgres', 'SHOW data_checksums'),
+ 'off', 'setup: cluster started with data checksums off');
+
+# A heap large enough to have a multi-block main fork, plus (after VACUUM) the
+# fsm and vm forks, so a single relation spans several forks with blocks.
+$node->safe_psql('postgres', qq{
+ CREATE TABLE big AS
+ SELECT g AS id, repeat('x', 60) AS pad FROM generate_series(1, 10000) g;
+ VACUUM (FREEZE, ANALYZE) big;
+});
+
+# Ground-truth per-fork sizes and the whole-relation total the docs promise.
+my ($main_blk, $fsm_blk, $vm_blk) = split /\|/, $node->safe_psql('postgres', q{
+ SELECT (pg_relation_size('big','main') / current_setting('block_size')::int)
+ ||'|'|| (pg_relation_size('big','fsm') / current_setting('block_size')::int)
+ ||'|'|| (pg_relation_size('big','vm') / current_setting('block_size')::int)
+});
+my $relation_total = $main_blk + $fsm_blk + $vm_blk;
+note("big fork block counts: main=$main_blk fsm=$fsm_blk vm=$vm_blk; "
+ . "whole-relation total = $relation_total");
+cmp_ok($main_blk, '>=', 2, 'setup: big main fork has multiple blocks');
+cmp_ok($fsm_blk + $vm_blk, '>=', 2,
+ 'setup: big has non-main forks (fsm/vm) with blocks');
+
+# A read-only sampler. It must NOT write any table: a writing transaction
+# would acquire an xid that the launcher's WaitForAllTransactionsToFinish()
+# could wait on, deadlocking against this very loop. Reading the stat views +
+# pg_sleep + building arrays acquires no xid, and pg_sleep/statement
+# boundaries absorb the checksum procsignal barrier. pg_stat_clear_snapshot()
+# each iteration defeats the per-transaction caching of pg_stat_activity so we
+# observe live changes.
+$node->safe_psql('postgres', q{
+CREATE FUNCTION sample_progress(target text, relation_total bigint)
+RETURNS text AS $$
+DECLARE
+ q text; bt bigint; bd bigint; fork text;
+ nsamp int := 0; seen boolean := false; post int := 0;
+ forks text[] := '{}'; btots bigint[] := '{}';
+ viol int := 0; backward int := 0; prev_bd bigint := NULL;
+ bd_min bigint := NULL; bd_max bigint := NULL;
+ start_ts timestamptz := clock_timestamp();
+BEGIN
+ LOOP
+ PERFORM pg_stat_clear_snapshot();
+ SELECT a.query, p.blocks_total, p.blocks_done
+ INTO q, bt, bd
+ FROM pg_stat_progress_data_checksums p
+ JOIN pg_stat_activity a ON a.pid = p.pid
+ WHERE p.datname = current_database()
+ AND a.query LIKE 'processing: ' || target || ' (%'
+ LIMIT 1;
+
+ IF q IS NOT NULL THEN
+ seen := true; post := 0; nsamp := nsamp + 1;
+ fork := substring(q from '\(([a-z]+),');
+ IF NOT (fork = ANY (forks)) THEN forks := forks || fork; END IF;
+ IF NOT (bt = ANY (btots)) THEN btots := btots || bt; END IF;
+ IF bt <> relation_total THEN viol := viol + 1; END IF;
+ IF prev_bd IS NOT NULL AND bd < prev_bd THEN
+ backward := backward + 1;
+ END IF;
+ prev_bd := bd;
+ IF bd_min IS NULL OR bd < bd_min THEN bd_min := bd; END IF;
+ IF bd_max IS NULL OR bd > bd_max THEN bd_max := bd; END IF;
+ ELSIF seen THEN
+ -- relation no longer current: it has finished being processed
+ post := post + 1;
+ prev_bd := NULL;
+ END IF;
+
+ EXIT WHEN seen AND post > 50; -- target relation finished
+ EXIT WHEN clock_timestamp() - start_ts > interval '90 seconds'; -- safety
+ PERFORM pg_sleep(0.001);
+ END LOOP;
+
+ RETURN format(
+ 'nsamp=%s;forks=%s;distinct_forks=%s;distinct_btot=%s;viol=%s;backward=%s;bd_min=%s;bd_max=%s',
+ nsamp, array_to_string(forks, ','),
+ coalesce(array_length(forks, 1), 0),
+ array_to_string(btots, ','), viol, backward,
+ coalesce(bd_min::text,'NA'), coalesce(bd_max::text,'NA'));
+END;
+$$ LANGUAGE plpgsql;
+});
+
+# --------------------------------------------------------------- exercise ---
+# Kick off the online enable (returns immediately; work happens in bg workers)
+# under a heavy per-block throttle so the small forks stay observable.
+$node->safe_psql('postgres', 'SELECT pg_enable_data_checksums(2, 1)');
+pass('setup: online enable of data checksums started');
+
+# Sample the progress view while public.big is the current relation.
+my $res = $node->safe_psql('postgres',
+ "SELECT sample_progress('public.big', $relation_total)",
+ timeout => $PostgreSQL::Test::Utils::timeout_default);
+
+my %r = map { split /=/, $_, 2 } split /;/, $res;
+note("sampler result: $res");
+note("distinct blocks_total observed while public.big was current: "
+ . "{$r{distinct_btot}} -- documented value would be the constant "
+ . "whole-relation total $relation_total");
+
+# We must actually have watched the relation being processed, across >1 fork,
+# or the demonstration below is inconclusive. (Ground truth is the worker's
+# own activity string, independent of the buggy counters.)
+cmp_ok($r{nsamp}, '>', 0, 'setup: observed public.big being processed');
+cmp_ok($r{distinct_forks}, '>=', 2,
+ "setup: worker processed public.big across >=2 forks ($r{forks})");
+
+# ---------------------------------------------------------- the defect(s) ---
+
+# DEFECT: while a single relation is the "current relation", blocks_total must
+# equal that relation's whole block count ($relation_total) -- per
+# monitoring.sgml, "The number of blocks in the current relation". On this
+# branch blocks_total is instead the size of whichever fork is being processed,
+# so every sample violates this.
+is($r{viol}, 0,
+ "blocks_total equals the current relation's total block count in every sample "
+ . "(got $r{viol} of $r{nsamp} samples where blocks_total <> $relation_total; "
+ . "observed blocks_total values {$r{distinct_btot}})");
+
+# DEFECT: blocks_done reports progress through the current relation, so while a
+# single relation stays current it must never move backward. On this branch it
+# is reset to 0 at every fork boundary, so it decreases at each boundary.
+is($r{backward}, 0,
+ "blocks_done never moves backward while public.big stays the current relation "
+ . "(observed $r{backward} backward step(s); blocks_done ranged $r{bd_min}..$r{bd_max})");
+
+$node->stop;
+done_testing();
diff --git a/src/test/modules/test_checksums/audit-defect-tests/t/105_launcher_kill_disable_wedge.pl b/src/test/modules/test_checksums/audit-defect-tests/t/105_launcher_kill_disable_wedge.pl
new file mode 100644
index 0000000..be2f906
--- /dev/null
+++ b/src/test/modules/test_checksums/audit-defect-tests/t/105_launcher_kill_disable_wedge.pl
@@ -0,0 +1,200 @@
+# 105_launcher_kill_disable_wedge.pl
+#
+# Commit f19c0ec "Online enabling and disabling of data checksums" installs
+# launcher_exit() (src/backend/postmaster/datachecksum_state.c) as the
+# on_shmem_exit cleanup that is meant to leave a consistent checksum state
+# whenever the "datachecksums launcher" process exits. It only heals an
+# interrupted *enable*:
+#
+# if (DataChecksumsInProgressOn())
+# SetDataChecksumsOff();
+#
+# There is no branch for an interrupted *disable*, and no DataChecksumsInProgressOff()
+# helper exists at all. SetDataChecksumsOff() (xlog.c) disables in two committed
+# steps separated by a forced RequestCheckpoint(CHECKPOINT_WAIT) plus a
+# WaitForProcSignalBarrier: it first persists ControlFile->data_checksum_version =
+# PG_DATA_CHECKSUM_INPROGRESS_OFF, then (only after the checkpoint completes)
+# PG_DATA_CHECKSUM_OFF. If the launcher is terminated (pg_terminate_backend ->
+# SIGTERM -> die -> proc_exit -> launcher_exit) during that window, launcher_exit()
+# runs while the persisted state is 'inprogress-off'; DataChecksumsInProgressOn()
+# is false, so nothing finishes the transition. bgw_restart_time is
+# BGW_NEVER_RESTART, so no launcher is relaunched: the cluster is wedged in
+# 'inprogress-off' with no process left to advance it. (A server restart would
+# self-heal it via StartupXLOG's end-of-recovery handling, but a live-cluster
+# SIGTERM never reaches that path.)
+#
+# This test contrasts the two cases using the SAME kill:
+# * launcher killed mid-ENABLE (state 'inprogress-on') -> self-heals to 'off'
+# * launcher killed mid-DISABLE (state 'inprogress-off') -> should also reach 'off'
+# The first (control) passes on this branch and on a fixed branch; the second is
+# the defect and fails on this branch, where the state stays 'inprogress-off'.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('csum_kill_disable');
+$node->init(no_data_checksums => 1);
+$node->append_conf('postgresql.conf', <<'EOF');
+shared_buffers = 16MB
+max_worker_processes = 16
+checkpoint_timeout = 1h
+log_min_messages = warning
+shared_preload_libraries = 'injection_points'
+EOF
+$node->start;
+
+# Current 'data_checksums' setting (what a client sees via SHOW / pg_settings).
+sub cstate
+{
+ return $node->safe_psql('postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums';");
+}
+
+# pid of the datachecksums launcher, or '' if none is running.
+sub launcher_pid
+{
+ return $node->safe_psql('postgres',
+ "SELECT pid FROM pg_stat_activity "
+ . "WHERE backend_type = 'datachecksums launcher' LIMIT 1;");
+}
+
+# Block until the launcher has fully exited (its on_shmem_exit cleanup, i.e.
+# launcher_exit(), has therefore already run to completion).
+sub wait_no_launcher
+{
+ $node->poll_query_until('postgres',
+ "SELECT count(*) = 0 FROM pg_stat_activity "
+ . "WHERE backend_type = 'datachecksums launcher';")
+ or die "datachecksums launcher never exited";
+}
+
+$node->safe_psql('postgres', 'CREATE EXTENSION injection_points;');
+# A little data so the enable worker has real work to do before it parks.
+$node->safe_psql('postgres',
+ 'CREATE TABLE t AS SELECT generate_series(1, 10000) AS a;');
+
+is(cstate(), 'off', 'setup: cluster initialized with checksums off');
+
+#############################################################################
+# CONTROL -- terminate the launcher mid-ENABLE (state 'inprogress-on').
+#
+# launcher_exit()'s one existing branch (DataChecksumsInProgressOn ->
+# SetDataChecksumsOff) turns this back to 'off'. This passes on this branch
+# and would pass on a fixed branch too, isolating the disable-only regression.
+#############################################################################
+
+# Hold the enable worker inside its temp-table wait so the cluster sits in
+# 'inprogress-on' with the launcher alive and waiting on the worker.
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('datachecksumsworker-fake-temptable-wait','wait');");
+$node->safe_psql('postgres', 'SELECT pg_enable_data_checksums(0, 100);');
+$node->wait_for_event('datachecksums worker',
+ 'datachecksumsworker-fake-temptable-wait');
+is(cstate(), 'inprogress-on',
+ 'control setup: enable is parked in inprogress-on');
+
+my $lp = launcher_pid();
+ok($lp ne '', "control setup: launcher is running (pid $lp)");
+
+# Kill the launcher while state is 'inprogress-on'. launcher_exit() signals the
+# worker on its way out; terminate any worker explicitly too and confirm it is
+# gone (its injection wait frees its slot on death), then drop the injection
+# point so the follow-up enable is not affected by it.
+$node->safe_psql('postgres', "SELECT pg_terminate_backend($lp);");
+$node->safe_psql('postgres',
+ "SELECT pg_terminate_backend(pid) FROM pg_stat_activity "
+ . "WHERE backend_type = 'datachecksums worker';");
+$node->poll_query_until('postgres',
+ "SELECT count(*) = 0 FROM pg_stat_activity "
+ . "WHERE backend_type = 'datachecksums worker';")
+ or die "enable worker never exited";
+wait_no_launcher();
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach('datachecksumsworker-fake-temptable-wait');");
+
+# Control result: the interrupted enable heals back to 'off' on its own.
+my $after_enable_kill = $node->poll_query_until('postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums';", 'off');
+is($after_enable_kill, 1,
+ 'control: launcher killed mid-enable self-heals to off');
+is(cstate(), 'off', 'control: state is off after the mid-enable kill');
+
+#############################################################################
+# DEFECT -- terminate the launcher mid-DISABLE (state 'inprogress-off').
+#############################################################################
+
+# Bring checksums fully ON first.
+$node->safe_psql('postgres', 'SELECT pg_enable_data_checksums(0, 100);');
+$node->poll_query_until('postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums';", 'on')
+ or die "enable never reached 'on'";
+wait_no_launcher();
+is(cstate(), 'on', 'defect setup: checksums fully enabled (on)');
+
+# Block the forced checkpoint that SetDataChecksumsOff() runs *after* it has
+# already persisted state 'inprogress-off'. The checkpointer parks on this
+# injection point, so the launcher blocks in RequestCheckpoint(CHECKPOINT_WAIT)
+# with the persisted state at 'inprogress-off'.
+$node->safe_psql('postgres',
+ "SELECT injection_points_attach('create-checkpoint-run','wait');");
+$node->safe_psql('postgres', 'SELECT pg_disable_data_checksums();');
+
+my $reached = $node->poll_query_until('postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums';",
+ 'inprogress-off');
+is($reached, 1, 'defect setup: disable is parked in inprogress-off');
+
+my $lp2 = launcher_pid();
+ok($lp2 ne '', "defect setup: launcher is running (pid $lp2)");
+
+# Terminate the launcher while state is 'inprogress-off'.
+$node->safe_psql('postgres', "SELECT pg_terminate_backend($lp2);");
+wait_no_launcher();
+
+# Release the orphaned checkpoint. The checkpointer (not the launcher) is the
+# process parked on this injection point, and it is still alive, so wakeup finds
+# it. After this the cluster is otherwise perfectly healthy.
+$node->safe_psql('postgres',
+ "SELECT injection_points_wakeup('create-checkpoint-run');");
+$node->safe_psql('postgres',
+ "SELECT injection_points_detach('create-checkpoint-run');");
+
+# Give any would-be completion every chance: an ordinary checkpoint and a moment.
+$node->safe_psql('postgres', 'CHECKPOINT;');
+$node->safe_psql('postgres', 'SELECT pg_sleep(1);');
+
+my $after_disable_kill = cstate();
+diag("data_checksums after mid-disable launcher kill = '$after_disable_kill'");
+
+# DEFECT: a launcher terminated mid-disable must still finish the transition to
+# 'off', exactly as launcher_exit() finishes an interrupted enable. The correct
+# final state is 'off'. On this branch launcher_exit() has no disable branch, so
+# the cluster is left indefinitely in 'inprogress-off' with no process to advance
+# it (recovered only by re-issuing pg_disable_data_checksums() or a server
+# restart, both exercised below) -- this assertion fails.
+is($after_disable_kill, 'off',
+ 'DEFECT: disable completes to off after launcher killed mid-inprogress-off');
+
+# The remaining checks document, and pass under, the consequence of the defect
+# on this branch: the wedge persists with no process left to advance it, and
+# is only cleared by a manual re-issue of the disable (or a server restart).
+# On a fixed branch the state would already be 'off' here and the re-issued
+# disable would be a harmless no-op, so these still pass there.
+my $n_launcher = $node->safe_psql('postgres',
+ "SELECT count(*) FROM pg_stat_activity "
+ . "WHERE backend_type = 'datachecksums launcher';");
+is($n_launcher, '0', 'no launcher process remains to complete the disable');
+
+$node->safe_psql('postgres', 'SELECT pg_disable_data_checksums();');
+my $recovered = $node->poll_query_until('postgres',
+ "SELECT setting FROM pg_settings WHERE name = 'data_checksums';", 'off');
+wait_no_launcher();
+is($recovered, 1,
+ 'a re-issued pg_disable_data_checksums() clears the wedge back to off');
+is(cstate(), 'off', 'checksums are off again after the manual re-issue');
+
+$node->stop;
+done_testing();
diff --git a/src/test/modules/test_checksums/audit-defect-tests/t/106_checksum_func_grant_defeated.pl b/src/test/modules/test_checksums/audit-defect-tests/t/106_checksum_func_grant_defeated.pl
new file mode 100644
index 0000000..4d1e432
--- /dev/null
+++ b/src/test/modules/test_checksums/audit-defect-tests/t/106_checksum_func_grant_defeated.pl
@@ -0,0 +1,112 @@
+# 106_checksum_func_grant_defeated.pl
+#
+# Reproducer for a privilege-model defect introduced by commit f19c0ec
+# ("Online enabling and disabling of data checksums").
+#
+# THE DEFECT
+# pg_enable_data_checksums() and pg_disable_data_checksums() ship the
+# *delegable* catalog ACL proacl => '{POSTGRES=X}' (pg_proc.dat) -- the exact
+# ACL carried by pg_switch_wal(), pg_reload_conf(), pg_backup_start() and the
+# ~70 other admin functions whose access is, by PostgreSQL convention,
+# "managed through the normal GRANT system": a superuser may GRANT EXECUTE to
+# a non-superuser to delegate them. But f19c0ec ALSO hardcodes a superuser()
+# gate in the function bodies (datachecksum_state.c enable_data_checksums /
+# disable_data_checksums: "must be superuser to change data checksum state").
+#
+# So a GRANT EXECUTE to a non-superuser is accepted and recorded in proacl
+# (has_function_privilege() returns true), yet the grantee is still rejected
+# at run time -- these are the only {POSTGRES=X} functions whose recorded
+# EXECUTE privilege does not actually permit execution. The docs
+# (func-admin.sgml "Data Checksum Functions") compound it by stating no
+# privilege model at all, unlike every neighboring restricted function.
+#
+# The assertion below is deliberately FIX-AGNOSTIC: it only requires that the
+# catalog's recorded privilege AGREE with real executability. A fix that
+# drops the redundant superuser() gate (has_priv stays true, execution works)
+# AND a fix that makes the ACL non-delegable (has_priv becomes false) both
+# satisfy it; only the current inconsistent state fails. pg_reload_conf(),
+# with the identical {POSTGRES=X} ACL, is the passing control.
+
+use strict;
+use warnings FATAL => 'all';
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('csum_grant');
+$node->init(no_data_checksums => 1);
+$node->append_conf('postgresql.conf', "shared_buffers = 16MB\nmax_worker_processes = 16\n");
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE ROLE alice LOGIN NOSUPERUSER');
+
+# has_function_privilege for alice on a function signature.
+sub has_priv
+{
+ my ($sig) = @_;
+ return $node->safe_psql('postgres',
+ "SELECT has_function_privilege('alice', '$sig', 'execute')");
+}
+
+# Does alice actually get to run $call? Returns 'ok' on success, else the
+# SQLSTATE-bearing error message. Uses psql as alice via SET ROLE.
+sub alice_can_run
+{
+ my ($call) = @_;
+ my ($stdout, $stderr) = ('', '');
+ $node->psql('postgres', "SET ROLE alice; $call",
+ stdout => \$stdout, stderr => \$stderr, on_error_stop => 1);
+ # Any server error (permission denied, must be superuser, ...) lands on
+ # stderr; empty stderr means the call actually ran.
+ return ($stderr =~ /\S/) ? $stderr : 'ok';
+}
+
+# ---- setup / context (PASS) --------------------------------------------
+# Before any grant, the ACL layer denies alice (permission denied) -- proves
+# PUBLIC has no execute and alice is a plain non-superuser.
+like(alice_can_run('SELECT pg_enable_data_checksums()'),
+ qr/permission denied for function pg_enable_data_checksums/,
+ 'setup: without a grant, alice is denied by the ACL layer');
+
+# The identically-ACL'd control function: grant it, and the recorded privilege
+# both reads true AND actually lets alice run it.
+$node->safe_psql('postgres', 'GRANT EXECUTE ON FUNCTION pg_reload_conf() TO alice');
+is(has_priv('pg_reload_conf()'), 't',
+ 'control: GRANT recorded for pg_reload_conf (has_function_privilege = t)');
+is(alice_can_run('SELECT pg_reload_conf()'), 'ok',
+ 'control: a {POSTGRES=X} function honors the GRANT -- alice can run pg_reload_conf');
+
+# ---- grant the checksum functions to alice ------------------------------
+$node->safe_psql('postgres',
+ 'GRANT EXECUTE ON FUNCTION pg_enable_data_checksums(int,int) TO alice');
+$node->safe_psql('postgres',
+ 'GRANT EXECUTE ON FUNCTION pg_disable_data_checksums() TO alice');
+
+# The GRANT is accepted and recorded, exactly as for the control (PASS).
+is(has_priv('pg_enable_data_checksums(int,int)'), 't',
+ 'GRANT EXECUTE on pg_enable_data_checksums is recorded (has_function_privilege = t)');
+is(has_priv('pg_disable_data_checksums()'), 't',
+ 'GRANT EXECUTE on pg_disable_data_checksums is recorded (has_function_privilege = t)');
+
+my $enable_err = alice_can_run('SELECT pg_enable_data_checksums()');
+my $disable_err = alice_can_run('SELECT pg_disable_data_checksums()');
+diag("alice pg_enable_data_checksums() -> $enable_err");
+diag("alice pg_disable_data_checksums() -> $disable_err");
+
+# ---- the defect (FAIL on this branch) -----------------------------------
+
+# DEFECT: the catalog's recorded EXECUTE privilege must agree with real
+# executability, as it does for pg_reload_conf and every other {POSTGRES=X}
+# function. Since has_function_privilege('alice', ...) is true (asserted
+# above), alice must be able to execute the function -- i.e. NOT be rejected
+# with "must be superuser". On this branch the internal superuser() gate
+# rejects her, so the recorded privilege is a lie. (Fix-agnostic: a fix that
+# instead makes the ACL non-delegable would flip has_function_privilege to
+# false and this test's has_priv assertions, not this one, would change.)
+unlike($enable_err, qr/must be superuser to change data checksum state/,
+ 'DEFECT: a recorded EXECUTE grant on pg_enable_data_checksums must actually permit execution');
+unlike($disable_err, qr/must be superuser to change data checksum state/,
+ 'DEFECT: a recorded EXECUTE grant on pg_disable_data_checksums must actually permit execution');
+
+$node->stop;
+done_testing();
--
2.49.0
=
[application/octet-stream] f19c0ec-checksum-online-gpt-v0.patch (31.7K, ../../E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se/3-f19c0ec-checksum-online-gpt-v0.patch)
download | inline diff:
From 7a61938fdeb00b33faaf794312a2518f7abef233 Mon Sep 17 00:00:00 2001
From: Noah Misch <noah@leadboat.com>
Date: Fri, 4 Sep 2026 22:20:55 +0000
Subject: [PATCH] Add audit tests for online checksum defects
---
.../PROVENANCE.md | 186 +++++++++++++
.../REPORT.md | 246 ++++++++++++++++++
.../t/001_enumvals.pl | 52 ++++
.../t/002_late_temp_table.pl | 114 ++++++++
4 files changed, 598 insertions(+)
create mode 100644 src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/PROVENANCE.md
create mode 100644 src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/REPORT.md
create mode 100644 src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/001_enumvals.pl
create mode 100644 src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/002_late_temp_table.pl
diff --git a/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/PROVENANCE.md b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/PROVENANCE.md
new file mode 100644
index 0000000..4f2b49f
--- /dev/null
+++ b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/PROVENANCE.md
@@ -0,0 +1,186 @@
+# Provenance
+
+## Request
+
+The user supplied this prompt verbatim on 2026-09-04 (UTC):
+
+> Norms you must never violate:
+> - In the norms below, replace $THIS_CONVERSATION_NAME with the name of this
+> conversation, the one that would be changed by /rename.
+> - Use your own worktree(s); disregard the present dir except as repository to which to attach your worktree.
+> - When you create a branch, prefix its name with: $THIS_CONVERSATION_NAME/
+> - Never modify a branch you did not create.
+> - Never modify a worktree you did not create.
+> - When you create a directory or file in a directory you did not create, it
+> must have a name that starts with: $THIS_CONVERSATION_NAME
+>
+> Make a large workflow, with at most 270 agents, to write test cases covering
+> user-visible defects in commit f19c0ec that are still present in branch
+> f19c0ec-checksum-online. Use your own worktree; disregard the present dir
+> except as repository to which to attach your worktree. The workflow should
+> first look for extant user-visible defects. If it finds any, write a test
+> case covering some of those defects. If any defects found weren't suitable to
+> test, describe them in a report.
+>
+> Commit the following on a fresh branch:
+> - A report describing any defects found, testable or not. Prefix the report
+> with [no defects] if that's so.
+> - Any tests written
+> - A PROVENANCE.md file containing model, prompt, etc.
+
+The supplied environment identified `/home/nm/src/pg/postgresql` as the source
+repository, 2026-09-04 as the date, and UTC as the timezone.
+
+## Model and orchestration
+
+- Coordinator: OpenAI Codex using `gpt-5.6-sol` at `ultra` reasoning effort,
+ as recorded in the session turn context. The Codex CLI version was
+ `0.152.0`.
+- Subagents: 14 distinct Codex subagents (eight direct and six nested),
+ inheriting the coordinator model, were used over more than twenty delegated
+ discovery, reproduction, adversarial-review, history, and test-review
+ assignments. At most three subagents ran beside the coordinator at once,
+ respecting the environment's four-slot concurrency limit and the user's
+ 270-agent ceiling.
+- Conversation name at creation time: `f19c0ec-checksum-online`; renamed by
+ the user to `f19c0ec-checksum-online-gpt` before branch creation.
+- Session/thread identifier: `01a06e31-c5ad-7ac2-a537-3b041125c354`.
+- Human input: the prompt and environment above, followed only by the
+ conversation rename and a request to continue. There was no human defect
+ selection, code contribution, or review.
+- Authorship: Codex generated `REPORT.md`, `PROVENANCE.md`, and both TAP test
+ files. Subagents contributed analysis and review but made no tracked edits;
+ conceptual reuse from prior repository work is disclosed below.
+
+The delegated workflow used these roles:
+
+1. establish commit ancestry, scope, and later-fix history;
+2. inspect checksum state transitions and background-worker lifecycles;
+3. inspect SQL/GUC metadata, privilege behavior, and client tools;
+4. inspect monitoring and documentation contracts;
+5. inspect storage, WAL, temporary-relation, and progress behavior;
+6. inspect replication, backup, restore, and upgrade interactions;
+7. reproduce candidates dynamically and challenge their user visibility and
+ `f19c0ec` attribution;
+8. review new TAP tests independently for determinism, cleanup, portability,
+ and assertion quality.
+
+## Repository and isolation
+
+- Introduction commit audited: `f19c0ec`.
+- Target branch inspected without modification: `f19c0ec-checksum-online`.
+- Target commit tested: `50bb165a05199c2cf93c1c9f42ddbca4162bee7a`.
+- Private worktree created detached for the audit, then attached to the fresh
+ branch `f19c0ec-checksum-online-gpt/defect-tests`:
+ `/home/nm/src/pg/f19c0ec-checksum-online-worktree`.
+- Private build, install, and run roots:
+ `/home/nm/src/pg/f19c0ec-checksum-online-build`,
+ `/home/nm/src/pg/f19c0ec-checksum-online-install`, and
+ `/home/nm/src/pg/f19c0ec-checksum-online-test-run`.
+
+The target branch name occupies Git ref `refs/heads/f19c0ec-checksum-online`.
+Git therefore cannot simultaneously create a branch below
+`refs/heads/f19c0ec-checksum-online/`, which the required conversation-name
+prefix would demand. Discovery and authoring proceeded detached so that the
+existing branch was not renamed, deleted, or modified. The user renamed the
+conversation to `f19c0ec-checksum-online-gpt`, after which the fresh branch
+`f19c0ec-checksum-online-gpt/defect-tests` was created from target commit
+`50bb165a05199c2cf93c1c9f42ddbca4162bee7a`. The object ID of the commit
+containing this file is not embedded in the file itself, because doing so
+would make the commit hash self-referential.
+
+No branch or worktree owned by another workflow was modified. All new tracked
+files are below a newly created directory whose name begins with the
+conversation name.
+
+## Evidence and prior work
+
+Primary evidence came from source and documentation at the target commit,
+`git log`, `git blame`, PostgreSQL's existing TAP tests, and fresh dynamic
+reproductions.
+
+One subagent also consulted these external primary PostgreSQL sources for
+context while evaluating replication and recovery leads:
+
+- [PostgreSQL 19 documentation: Data Checksums](https://www.postgresql.org/docs/19/checksums.html);
+- pgsql-hackers thread "Changing the state of data checksums in a running
+ cluster": [message 1](https://www.postgresql.org/message-id/f1281cf3-89a3-4936-9bc5-2a5a6291229f@vondra.me),
+ [message 2](https://www.postgresql.org/message-id/9e1331e1-93a0-4e27-934a-17b89342be4d@vondra.me),
+ [message 3](https://www.postgresql.org/message-id/FAE6FC0E-AA0B-4CF4-B49B-BA6C2FC55FB8@yesql.se),
+ and [message 4](https://www.postgresql.org/message-id/538e820b-db2a-4f53-ba24-c354c72fc1a9@vondra.me).
+
+Those sources provided design context only and helped reject replication
+candidates. No counted finding, test logic, or copied report prose was
+derived from them; the report's evidence is the checked-out tree, its history,
+and live behavior.
+
+During repository-wide discovery, the workflow found an existing read-only
+branch `f19c0ec-checksum-defect-tests` at commit `fba2cb1`. It was inspected
+as prior art after independent source analysis and supplied several candidate
+leads. It was not checked out for modification or cherry-picked. In
+particular, its `t/103_data_checksums_enumvals.pl` identified the same enum
+metadata defect and used the core `setting = ANY(enumvals)` consistency
+invariant also used here. The new, shorter `t/001_enumvals.pl` retains that
+concept, requires all four states if the setting remains an enum, and permits
+a legitimate fix that changes the read-only setting to a non-enum type.
+
+This audit independently reclassified or rejected several of the prior
+branch's other claims and found the late-temporary-table cutoff defect that
+its tests did not cover. The late-temp test and the revised enum test were
+written in the private worktree rather than copied from the prior branch.
+
+## Build and verification
+
+The exact target was configured with Meson using assertions, debug support,
+TAP tests, and injection points:
+
+```text
+CC='ccache gcc' meson setup <build> <source> --prefix=<install> \
+ -Dcassert=true -Ddebug=true -Doptimization=1 \
+ -Dtap_tests=enabled -Dinjection_points=true
+ninja -C <build> install
+meson test -C <build> --suite setup
+meson test -C <build> test_checksums/001_basic --print-errorlogs
+```
+
+After the fresh branch was created, the final test blobs were run from
+`post-branch-001` and `post-branch-002` directories below the private run
+root. Each `prove` process received exactly this relevant environment:
+
+```text
+PATH=/home/nm/src/pg/f19c0ec-checksum-online-build/tmp_install/home/nm/src/pg/f19c0ec-checksum-online-install/bin:/usr/bin:/bin
+LD_LIBRARY_PATH=/home/nm/src/pg/f19c0ec-checksum-online-build/tmp_install/home/nm/src/pg/f19c0ec-checksum-online-install/lib/x86_64-linux-gnu
+PERL5LIB=/home/nm/src/pg/f19c0ec-checksum-online-worktree/src/test/perl
+top_builddir=/home/nm/src/pg/f19c0ec-checksum-online-build
+PG_REGRESS=/home/nm/src/pg/f19c0ec-checksum-online-build/src/test/regress/pg_regress
+REGRESS_SHLIB=/home/nm/src/pg/f19c0ec-checksum-online-build/src/test/regress/regress.so
+INITDB_TEMPLATE=/home/nm/src/pg/f19c0ec-checksum-online-build/tmp_install/initdb-template
+```
+
+The invocations were:
+
+```text
+prove -v /home/nm/src/pg/f19c0ec-checksum-online-worktree/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/001_enumvals.pl
+prove -v /home/nm/src/pg/f19c0ec-checksum-online-worktree/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/002_late_temp_table.pl
+```
+
+The three setup tests passed, and the existing `test_checksums/001_basic` TAP
+test passed all 27 assertions. With the temporary-install TAP environment:
+
+- `t/001_enumvals.pl` failed only its intended assertion and reported
+ `type=enum setting=off enumvals={""}`;
+- `t/002_late_temp_table.pl` was repeated independently and consistently
+ passed setup assertions 1-4, then failed only its intended final assertion
+ with `blocked-by-late-temp` in about 7-8 seconds;
+- Perl syntax checks and `git diff --check` passed;
+- all test clusters and live-reproduction processes were stopped during
+ cleanup.
+
+The orphan-worker candidate was also reproduced in a fresh private cluster by
+gating worker startup, pausing and resuming the postmaster with
+`SIGSTOP`/`SIGCONT`, and terminating the launcher. Its exact observation
+is recorded in `REPORT.md`; no retained tracked fixture or server data came
+from that experiment.
+
+`pgperltidy` could not be run because the environment lacked its `perltidy`
+dependency. Independent style review found no actionable formatting issue.
diff --git a/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/REPORT.md b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/REPORT.md
new file mode 100644
index 0000000..73a2673
--- /dev/null
+++ b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/REPORT.md
@@ -0,0 +1,246 @@
+# User-visible defects introduced by `f19c0ec`
+
+## Scope and result
+
+This audit examined commit `f19c0ec` ("Online enabling and disabling of data
+checksums") and retested candidate defects at commit
+`50bb165a05199c2cf93c1c9f42ddbca4162bee7a`, the tip of
+`f19c0ec-checksum-online` during the audit. It found seven user-visible
+defects introduced by `f19c0ec` that remain at that tip. Two have standalone
+TAP regression tests in this directory; five are documented without a
+committed test because a functional test would be platform-specific,
+fix-hostile, or unable to distinguish the defective documentation from its
+fix.
+
+| Defect | User impact | Coverage here |
+|---|---|---|
+| A late temporary table is treated as pre-existing | Online enablement waits for a session that it need not wait for | `t/002_late_temp_table.pl` |
+| `pg_settings.enumvals` is `{""}` | Monitoring and introspection clients receive false metadata | `t/001_enumvals.pl` |
+| A registered worker can outlive a terminated launcher | Relation I/O and full-page-image WAL can begin after `SHOW data_checksums` reports `off` and the launcher has exited | Live-reproduced; report only |
+| `pg_upgrade` misreports `inprogress-off` | Its fatal diagnostic says checksums are being enabled while they are being disabled | Report only |
+| Progress block counters are per fork, not per relation as documented | Monitors see unexplained resets and totals | Report only |
+| The documented function signature omits a valid one-argument call | Users are not told they can set only `cost_delay` | Report only |
+| The function documentation omits the mandatory superuser requirement | Users cannot tell who is authorized to run the operations | Report only |
+
+## Tested defects
+
+### 1. Late-created temporary tables can delay enablement
+
+The monitoring documentation says that the worker waits for temporary tables
+that existed "at the time the command was started"
+(`doc/src/sgml/monitoring.sgml:8386-8390`). That cutoff is important: once the
+cluster has entered `inprogress-on`, pages belonging to newly created
+temporary tables receive checksums when written and do not require rewriting.
+
+The implementation instead builds `InitialTempTableList` separately when each
+per-database worker starts
+(`src/backend/postmaster/datachecksum_state.c:1793-1800`) and later waits for
+the OIDs in that list (`:1918-1954`). Since the launcher processes databases
+serially, a temporary table can be created after
+`pg_enable_data_checksums()` has started but before the worker for that
+database is launched. The worker misclassifies that table, whose pages will
+receive checksums when written, as pre-existing and waits until its owning
+session exits. A long-lived session can therefore make the cluster remain in
+`inprogress-on` indefinitely, contrary to the documented cutoff.
+
+`t/002_late_temp_table.pl` creates the table strictly after the command has
+entered `inprogress-on`, but before its database worker can start. On the
+audited tip, setup assertions 1-4 pass and the final assertion fails with:
+
+```text
+online checksum enable outcome with late temp table: blocked-by-late-temp
+```
+
+The test then closes the late session and waits for enablement to finish before
+reporting the failure, so it leaves no worker or server behind.
+
+### 2. `pg_settings` advertises no usable enum values
+
+`f19c0ec` changed `data_checksums` from a boolean GUC to an enum, but marked
+all four enum entries hidden
+(`src/backend/utils/misc/guc_tables.c:515-520`). Consequently the
+`pg_settings` row has `vartype = 'enum'` and a current setting such as `off`,
+but `enumvals = '{""}'`. This contradicts the documented meaning of
+`enumvals` as the allowed values of an enum parameter
+(`doc/src/sgml/system-views.sgml:3826-3833`). The generic GUC code itself
+calls an all-hidden enum a "broken GUC setup"
+(`src/backend/utils/misc/guc.c:2991-2996`).
+
+Monitoring tools and other clients that inspect type metadata through
+`pg_settings` cannot validate or present this setting
+correctly. If `data_checksums` remains an enum, `t/001_enumvals.pl` requires
+the four states introduced by `f19c0ec` to be present and the current setting
+to be one of the advertised values. It also permits a fix that represents
+this read-only status as a non-enum setting with `enumvals` set to `NULL`, as
+long as the displayed status remains one of the four defined states. On the
+audited tip its only assertion fails, with the diagnostic:
+
+```text
+type=enum setting=off enumvals={""}
+```
+
+The containment assertion permits future states and does not prescribe the
+ordering of the four existing states.
+
+## Confirmed defects not covered by a committed test
+
+### 3. A per-database worker can start after launcher termination
+
+The launcher publishes the intended database and invocation, registers a
+dynamic background worker, and waits for it to start
+(`src/backend/postmaster/datachecksum_state.c:974-1020`). It does not publish
+the worker PID until after startup succeeds (`:1071-1080`). If the launcher
+receives `SIGTERM` in that interval, its exit callback sees an invalid
+`worker_pid`, cannot signal the registered worker (`:1133-1148`), and resets
+the cluster from `inprogress-on` to `off` (`:1151-1156`). Background-worker
+notification cleanup clears only the dead launcher's notification PID; it
+does not cancel the registration (`src/backend/postmaster/bgworker.c:413-427,
+540-551`). The stored invocation remains current, so the delayed worker's
+checks do not reject it (`datachecksum_state.c:1776-1806`).
+The register-wait-publish ordering and missing registration cancellation were
+already present in `f19c0ec`; later invocation and PID bookkeeping changes did
+not close the window.
+
+This was reproduced live. The postmaster process was paused with `SIGSTOP`
+while `pg_stat_activity` showed the launcher waiting in `BgWorkerStartup`;
+`pg_terminate_backend()` then terminated the launcher and `SHOW
+data_checksums` reported `off` with no launcher. After the postmaster was
+resumed with `SIGCONT`, the registered worker started. While checksums were
+still `off` and no launcher existed, `pg_stat_activity` showed:
+
+```text
+datachecksums worker | active | IPC | ChecksumEnableTemptableWait
+```
+
+and `pg_stat_progress_data_checksums` contained its progress row. The server
+log first reported that the launcher notification PID was invalid and then
+started the worker. The orphan can read and dirty pages and emit
+full-page-image WAL records for its whole database after `SHOW data_checksums`
+reports `off` and the launcher has exited. It no longer calculates checksums
+once the state is `off`. The remaining relation I/O, WAL, and replication
+traffic are unexpected, though the audit found no corruption path.
+
+A deterministic reproducer requires Unix process control to pause the
+postmaster with `SIGSTOP` in a narrow background-worker startup phase, with
+every monitoring connection established beforehand. An upstream test could
+instead add a purpose-built injection point. The current live reproducer is
+therefore not included as a portable regression test here.
+
+### 4. `pg_upgrade` reports the wrong in-progress direction
+
+The control-file enum assigns distinct values to
+`PG_DATA_CHECKSUM_INPROGRESS_OFF` and `PG_DATA_CHECKSUM_INPROGRESS_ON`
+(`src/include/storage/checksum.h:26-32`). `pg_upgrade` correctly refuses both,
+but classifies every value greater than the enabled version as "data checksums
+are being enabled" (`src/bin/pg_upgrade/controldata.c:656-662`). The
+classification and original "checksums are being enabled" diagnostic came
+from `f19c0ec`; later commit `078aac2` only inserted the word "data". A
+cluster whose state is `inprogress-off` therefore receives a factually
+reversed fatal diagnostic. That can send an administrator toward the wrong
+recovery action.
+
+Producing a naturally generated, cleanly shut-down `inprogress-off` old
+cluster on demand requires holding the disable barrier and externally
+interrupting its launcher. That would couple the diagnostic test to
+contentious signal-state semantics rather than isolate `pg_upgrade`. Directly
+manufacturing the internal control-file value would be brittle, so no test is
+included.
+
+### 5. Block progress counters have a different unit than documented
+
+The progress view documents `blocks_total` and `blocks_done` as blocks in the
+current *relation* (`doc/src/sgml/monitoring.sgml:8317-8339`). In fact,
+`ProcessSingleRelationByOid()` iterates relation forks and
+`ProcessSingleRelationFork()` resets both counters for each fork
+(`src/backend/postmaster/datachecksum_state.c:725-750, 839-879`). For a
+relation with main, FSM, and VM forks, the counters reset or change totals
+while `relations_done` remains unchanged. The view has no fork column, so a
+monitor cannot interpret those transitions from the documented contract.
+The per-fork unit dates to `f19c0ec`; later commit `e981025` made the
+`blocks_done` reset explicit without changing that unit.
+
+Both an implementation fix that aggregates forks and a documentation fix that
+defines the counters as per-fork would resolve the mismatch. A runtime test
+demanding monotonic relation-wide values would reject the valid
+documentation-only resolution and would also depend on observing a brief
+phase. This defect is report-only pending a choice of contract.
+
+### 6. The documented signature hides a supported call form
+
+The catalog gives defaults to both trailing arguments of
+`pg_enable_data_checksums(int, int)`
+(`src/include/catalog/pg_proc.dat:12478-12483`), so zero-, one-, and
+two-argument calls are valid. The documentation encloses both parameters in
+one optional group and discusses only specifying both
+(`doc/src/sgml/func/func-admin.sgml:3155-3176`). It therefore does not show
+the valid `pg_enable_data_checksums(cost_delay)` form, which changes the delay
+while retaining the default cost limit.
+
+A positive SQL test of the one-argument call already passes on the defective
+branch and after a documentation fix. A source-text assertion against SGML
+markup would be brittle, so this documentation defect has no functional test.
+
+### 7. The documentation omits the superuser requirement
+
+The same function section describes both checksum state-changing functions
+without an authorization requirement
+(`doc/src/sgml/func/func-admin.sgml:3126-3202`). Both implementations
+unconditionally reject callers for whom `superuser()` is false
+(`src/backend/postmaster/datachecksum_state.c:574-577, 596-599`). A reader of
+the manual therefore cannot determine who may perform the documented
+operation. If the hard check is intentionally nondelegable, the manual
+should also make clear that granting `EXECUTE` does not suffice.
+
+A runtime denial test would exercise the existing intended implementation and
+would continue to pass after the missing documentation is supplied. A test
+that greps prose would be inappropriate, so this is report-only.
+
+## Leads evaluated but not counted
+
+- Cancel-and-immediately-reenable behavior at the branch tip can launch
+ overlapping work and produce a false success log. In `f19c0ec`, however,
+ `SIGINT` cancellation eventually raised `ERROR`; the exit callback repaired
+ the checksum state while `launcher_running` remained true, so an immediate
+ same-direction request could not start a second launcher. Later commits
+ `8fb8ded`, `bf25e55`, `a4f02ca`, and `ed6775d` supplied the state
+ transitions and early `done:` path that make the overlap reachable. The
+ clear-before-callback ordering was latent in `f19c0ec`, but the user-visible
+ defect was introduced by those follow-ups and is outside this report's
+ stated scope.
+- Killing a disable launcher can leave or briefly expose `inprogress-off`, but
+ the audit found no documented post-`SIGTERM` state contract and the operation
+ is recoverable by reissuing the command or restarting. A test that insists
+ on one cleanup policy would encode an unestablished requirement.
+- A non-superuser remains rejected after `GRANT EXECUTE` on the checksum
+ functions. That does not by itself prove a code defect: function ACLs and
+ authorization checks inside a function can legitimately coexist. Only the
+ objective documentation omission is counted above.
+- The replication-impact text still says a standby issues a restartpoint that
+ blocks redo and recommends reducing `max_wal_size`. Commit `05e7b3e` changed
+ replay to persist the state inline without that restartpoint, leaving the
+ text stale; the defect therefore postdates `f19c0ec`.
+- `50bb165` treats a final incremental backup in `inprogress-off` as enabled
+ when checking a chain whose prior full backup was `on`, and can falsely
+ report that only some backups have checksums enabled. That issue was
+ introduced by the target-tip commit itself and is not covered by its
+ `t/024_combinebackup_mixed.pl` test.
+- Missing visual padding in one `pg_controldata` label and progress values
+ retained while a worker waits on temporary tables do not violate a stated
+ interface and were not classified as defects.
+
+## Running the tests
+
+These tests are intentionally standalone and are not connected to the regular
+`test_checksums` Meson target: both are regression reproducers expected to fail
+at `50bb165`. Run them from an empty scratch directory with the usual
+PostgreSQL TAP environment (`PATH` pointing at the temporary installation,
+`PERL5LIB` at `src/test/perl`, and `PG_REGRESS`, `REGRESS_SHLIB`, and
+`INITDB_TEMPLATE` set as for the in-tree TAP suite):
+
+```sh
+prove -v /path/to/f19c0ec-checksum-online-defect-tests/t/001_enumvals.pl
+prove -v /path/to/f19c0ec-checksum-online-defect-tests/t/002_late_temp_table.pl
+```
+
+The late-temp script stops its cluster before its intended failing assertion;
+the enum-metadata script stops its cluster immediately afterward.
diff --git a/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/001_enumvals.pl b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/001_enumvals.pl
new file mode 100644
index 0000000..93f54a4
--- /dev/null
+++ b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/001_enumvals.pl
@@ -0,0 +1,52 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Verify that pg_settings gives internally consistent metadata for the
+# data_checksums setting. Commit f19c0ec converted this GUC from bool to enum,
+# but marked every enum entry hidden. Consequently enumvals is {""}, even
+# though the setting is one of on, off, inprogress-on, or inprogress-off.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('enumvals');
+$node->init(no_data_checksums => 1);
+$node->start;
+
+my $metadata = $node->safe_psql(
+ 'postgres', q{
+ SELECT format('type=%s setting=%s enumvals=%s',
+ vartype, setting, coalesce(enumvals::text, 'NULL'))
+ FROM pg_settings
+ WHERE name = 'data_checksums'});
+note("data_checksums metadata: $metadata");
+
+# pg_settings documents enumvals as the allowed values for enum parameters.
+# If data_checksums remains an enum, containment is tolerant of future states
+# while requiring the four states introduced by f19c0ec, and membership makes
+# its metadata self-consistent. Converting this read-only status setting to a
+# non-enum type is also a valid fix, in which case enumvals must be NULL and
+# the displayed status must still be one of the four defined states.
+is(
+ $node->safe_psql(
+ 'postgres', q{
+ SELECT CASE WHEN vartype = 'enum' THEN
+ enumvals @> ARRAY[
+ 'on', 'off', 'inprogress-on', 'inprogress-off'
+ ]::text[]
+ AND setting = ANY(enumvals)
+ ELSE enumvals IS NULL
+ AND setting = ANY(ARRAY[
+ 'on', 'off', 'inprogress-on', 'inprogress-off'
+ ]::text[])
+ END
+ FROM pg_settings
+ WHERE name = 'data_checksums'}),
+ 't',
+ 'data_checksums exposes consistent type metadata through pg_settings');
+
+$node->stop;
+done_testing();
diff --git a/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/002_late_temp_table.pl b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/002_late_temp_table.pl
new file mode 100644
index 0000000..891b983
--- /dev/null
+++ b/src/test/modules/test_checksums/f19c0ec-checksum-online-defect-tests/t/002_late_temp_table.pl
@@ -0,0 +1,114 @@
+# Copyright (c) 2026, PostgreSQL Global Development Group
+
+# Verify the documented cutoff for temporary tables during online checksum
+# enablement. A table created after the cluster enters inprogress-on receives
+# checksums and must not delay completion. At present each database worker
+# takes its initial-temp snapshot only when that worker starts, so a late table
+# in a database not yet visited is mistakenly treated as old.
+
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('late_temp');
+$node->init(no_data_checksums => 1);
+$node->append_conf('postgresql.conf',
+ "shared_buffers = '16MB'\nmax_worker_processes = 16\n");
+$node->start;
+
+$node->safe_psql('postgres', 'CREATE DATABASE late_a');
+$node->safe_psql('postgres', 'CREATE DATABASE late_b');
+
+# Put a pre-existing temporary table in every connectable database that might
+# be visited first. Whichever worker reaches one first will wait there, giving
+# us a deterministic window after the command starts but before a later
+# database worker starts.
+my %old_session;
+for my $dbname (qw(postgres template1 late_a late_b))
+{
+ my $session = $node->background_psql($dbname);
+ $session->query_safe('CREATE TEMP TABLE old_temp(i int)');
+ $old_session{$dbname} = $session;
+}
+
+is($node->safe_psql('postgres', 'SHOW data_checksums'), 'off',
+ 'checksums start disabled');
+$node->safe_psql('postgres', 'SELECT pg_enable_data_checksums()');
+
+$node->wait_for_event('datachecksums worker',
+ 'ChecksumEnableTemptableWait');
+
+my $blocked_db = $node->safe_psql(
+ 'postgres', q{
+ SELECT datname
+ FROM pg_stat_activity
+ WHERE backend_type = 'datachecksums worker'
+ AND wait_event = 'ChecksumEnableTemptableWait'});
+ok(exists $old_session{$blocked_db},
+ "an intentionally held database blocked the first worker ($blocked_db)");
+is($node->safe_psql('postgres', 'SHOW data_checksums'), 'inprogress-on',
+ 'enablement has started before the late table is created');
+
+# Pick a database whose worker cannot have run yet: had either candidate run,
+# its still-live old_temp would have blocked that earlier worker instead.
+my $target = $blocked_db eq 'late_a' ? 'late_b' : 'late_a';
+
+# Replace the target's pre-command temp table with one created strictly after
+# the command entered inprogress-on, while an earlier database remains held.
+$old_session{$target}->quit;
+delete $old_session{$target};
+my $late_session = $node->background_psql($target);
+$late_session->query_safe('CREATE TEMP TABLE late_temp(i int)');
+pass("created a temporary table in $target after enablement started");
+
+# Release every genuinely pre-existing temp table. The late table remains.
+for my $session (values %old_session)
+{
+ $session->quit;
+}
+%old_session = ();
+
+# Stop as soon as either the correct outcome occurs or the target worker is
+# observed waiting for the late table. This avoids a long timeout on the
+# defective branch while retaining a bounded fallback for slow machines.
+my $outcome = 'timeout';
+for my $attempt (1 .. 10 * $PostgreSQL::Test::Utils::timeout_default)
+{
+ my $state = $node->safe_psql('postgres', 'SHOW data_checksums');
+ if ($state eq 'on')
+ {
+ $outcome = 'on';
+ last;
+ }
+
+ my $waiting = $node->safe_psql(
+ 'postgres', qq{
+ SELECT count(*) > 0
+ FROM pg_stat_activity
+ WHERE backend_type = 'datachecksums worker'
+ AND datname = '$target'
+ AND wait_event = 'ChecksumEnableTemptableWait'});
+ if ($waiting eq 't')
+ {
+ $outcome = 'blocked-by-late-temp';
+ last;
+ }
+
+ PostgreSQL::Test::Utils::usleep(100_000);
+}
+
+# Drop the late table and allow the defective branch to finish, so teardown is
+# clean and the test does not leave a live checksum worker behind.
+$late_session->quit;
+$node->poll_query_until('postgres', 'SHOW data_checksums', 'on')
+ or die 'checksum enablement did not finish during cleanup';
+$node->stop;
+
+diag("online checksum enable outcome with late temp table: $outcome");
+is($outcome, 'on',
+ 'a temporary table created after enablement starts does not block completion');
+
+done_testing();
--
2.49.0
=
[application/octet-stream] v1-0001-Fix-enum-value-visibility-for-data_checksums.patch (1.6K, ../../E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se/4-v1-0001-Fix-enum-value-visibility-for-data_checksums.patch)
download | inline diff:
From 76a2db073dbb39d48500bd900cc377855e7d4daf Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Sat, 5 Sep 2026 20:20:45 +0200
Subject: [PATCH v1 1/5] Fix enum value visibility for data_checksums
When the data_checksums GUC was changed into an enum the enum values
were all incorrectly marked as hidden, which made pg_settings report
an empty array. Fix by setting all as visible as they should be.
Found by Noah using AI assisted review with GPT. Backpatch to v19
where the GUC was changed in the online checksums feature.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/...
Backpatch-through: 19
---
src/backend/utils/misc/guc_tables.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c6d9b2a6f89..342aaeef59a 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -513,10 +513,10 @@ static const struct config_enum_entry file_extend_method_options[] = {
};
static const struct config_enum_entry data_checksums_options[] = {
- {"on", PG_DATA_CHECKSUM_VERSION, true},
- {"off", PG_DATA_CHECKSUM_OFF, true},
- {"inprogress-on", PG_DATA_CHECKSUM_INPROGRESS_ON, true},
- {"inprogress-off", PG_DATA_CHECKSUM_INPROGRESS_OFF, true},
+ {"on", PG_DATA_CHECKSUM_VERSION, false},
+ {"off", PG_DATA_CHECKSUM_OFF, false},
+ {"inprogress-on", PG_DATA_CHECKSUM_INPROGRESS_ON, false},
+ {"inprogress-off", PG_DATA_CHECKSUM_INPROGRESS_OFF, false},
{NULL, 0, false}
};
--
2.39.3 (Apple Git-146)
=
[application/octet-stream] v1-0002-doc-Add-missing-superuser-note-on-online-checksum.patch (2.9K, ../../E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se/5-v1-0002-doc-Add-missing-superuser-note-on-online-checksum.patch)
download | inline diff:
From 1d2053a7e604cddf0216c93f92e192805a631d3a Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Sat, 5 Sep 2026 20:31:12 +0200
Subject: [PATCH v1 2/5] doc: Add missing superuser note on online checksum
functionn
The documentation for the functions to enable and disable data checksums in
a running cluster failed to mention that they are superuser only. The ACL
in the procedure definition was also promising more than the implementation
allowed so remove to match the strict superuser check in the functions.
Found by Noah using AI assisted review with GPT. Backpatch to v19
where the online checksums feature was introduced.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/...
Backpatch-through: 19
---
doc/src/sgml/func/func-admin.sgml | 9 ++++++++-
src/include/catalog/pg_proc.dat | 2 +-
2 files changed, 9 insertions(+), 2 deletions(-)
diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml
index 54eeb42e5bc..16e62a93acb 100644
--- a/doc/src/sgml/func/func-admin.sgml
+++ b/doc/src/sgml/func/func-admin.sgml
@@ -3128,7 +3128,8 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
<para>
The functions shown in <xref linkend="functions-checksums-table" /> can
- be used to enable or disable data checksums in a running cluster.
+ be used to enable or disable data checksums in a running cluster. Use of
+ these functions is restricted to superusers.
</para>
<para>
Changing data checksums can be done in a cluster with concurrent activity
@@ -3175,6 +3176,9 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
specified, the process is throttled using the same principles as
<link linkend="runtime-config-resource-vacuum-cost">Cost-based Vacuum Delay</link>.
</para>
+ <para>
+ This function is restricted to superusers.
+ </para>
</entry>
</row>
@@ -3193,6 +3197,9 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
stopped validating data checksums, the data checksum state will be
set to <literal>off</literal>.
</para>
+ <para>
+ This function is restricted to superusers.
+ </para>
</entry>
</row>
</tbody>
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 960763ee50b..a66cd1fe3f8 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -12477,7 +12477,7 @@
prorettype => 'void', proargtypes => 'int4 int4',
proallargtypes => '{int4,int4}', proargmodes => '{i,i}',
proargnames => '{cost_delay,cost_limit}', proargdefaults => '{0,100}',
- prosrc => 'enable_data_checksums', proacl => '{POSTGRES=X}' },
+ prosrc => 'enable_data_checksums' },
# collation management functions
{ oid => '3445', descr => 'import collations from operating system',
--
2.39.3 (Apple Git-146)
=
[application/octet-stream] v1-0003-doc-Documentation-fixups-for-online-checksums.patch (3.3K, ../../E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se/6-v1-0003-doc-Documentation-fixups-for-online-checksums.patch)
download | inline diff:
From 5482ed74d2468a15e2183effc796364584986750 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Sat, 5 Sep 2026 20:42:18 +0200
Subject: [PATCH v1 3/5] doc: Documentation fixups for online checksums
The documentation for the checksums progress reporting had accidentally
omitted the work "fork" and for th blocks_* columns. The values are per
fork but the documentation made it seem they were per relation.
Found by Noah using AI assisted review with GPT. Backpatch to v19
where the online checksums feature was introduced.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/...
Backpatch-through: 19
---
doc/src/sgml/func/func-admin.sgml | 4 +++-
doc/src/sgml/monitoring.sgml | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/doc/src/sgml/func/func-admin.sgml b/doc/src/sgml/func/func-admin.sgml
index 16e62a93acb..7cf3a88b03f 100644
--- a/doc/src/sgml/func/func-admin.sgml
+++ b/doc/src/sgml/func/func-admin.sgml
@@ -3159,7 +3159,7 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
<indexterm>
<primary>pg_enable_data_checksums</primary>
</indexterm>
- <function>pg_enable_data_checksums</function> ( <optional><parameter>cost_delay</parameter> <type>int</type>, <parameter>cost_limit</parameter> <type>int</type></optional> )
+ <function>pg_enable_data_checksums</function> ( <optional><parameter>cost_delay</parameter> <type>int</type> <optional>, <parameter>cost_limit</parameter> <type>int</type></optional></optional> )
<returnvalue>void</returnvalue>
</para>
<para>
@@ -3175,6 +3175,8 @@ SELECT convert_from(pg_read_binary_file('file_in_utf8.txt'), 'UTF8');
If <parameter>cost_delay</parameter> and <parameter>cost_limit</parameter> are
specified, the process is throttled using the same principles as
<link linkend="runtime-config-resource-vacuum-cost">Cost-based Vacuum Delay</link>.
+ <parameter>cost_delay</paraneter> defaults to 0,
+ <parameter>cost_limit</parameter> defaults to 100.
</para>
<para>
This function is restricted to superusers.
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index b403fb990a7..4d46327a102 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -8320,7 +8320,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<structfield>blocks_total</structfield> <type>bigint</type>
</para>
<para>
- The number of blocks in the current relation which will be processed,
+ The number of blocks in the current relation fork which will be processed,
or <literal>NULL</literal> if the worker process hasn't
calculated the number of blocks yet. The launcher process has
this set to <literal>NULL</literal>.
@@ -8334,7 +8334,7 @@ FROM pg_stat_get_backend_idset() AS backendid;
<structfield>blocks_done</structfield> <type>bigint</type>
</para>
<para>
- The number of blocks in the current relation which have been processed.
+ The number of blocks in the current relation fork which have been processed.
The launcher process has this set to <literal>NULL</literal>.
</para>
</entry>
--
2.39.3 (Apple Git-146)
=
[application/octet-stream] v1-0005-Only-reset-launcher-state-in-exit-handler.patch (1.3K, ../../E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se/7-v1-0005-Only-reset-launcher-state-in-exit-handler.patch)
download | inline diff:
From a70e4079914ea7d620b35533546b9ee1f3860615 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Tue, 8 Sep 2026 22:42:21 +0200
Subject: [PATCH v1 5/5] Only reset launcher state in exit handler
The launcher_running flag was reset to false during shutdown, which
left a small window where a new launcher could be started when the
exit handler was still running. Fix by only updating the running
flag during the exit handler.
Found by Noah using AI assisted review with Claude. Backpatch to
v19 where the online checksums feature was introduced.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/...
Backpatch-through: 19
---
src/backend/postmaster/datachecksum_state.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/backend/postmaster/datachecksum_state.c b/src/backend/postmaster/datachecksum_state.c
index f69258bc33d..485e5dde1a5 100644
--- a/src/backend/postmaster/datachecksum_state.c
+++ b/src/backend/postmaster/datachecksum_state.c
@@ -1377,8 +1377,6 @@ done:
/* Shut down progress reporting as we are done */
pgstat_progress_end_command();
- launcher_running = false;
- DataChecksumState->launcher_running = false;
LWLockRelease(DataChecksumsWorkerLock);
}
--
2.39.3 (Apple Git-146)
=
[application/octet-stream] v1-0004-Improve-error-message-for-checksum-state-in-pg_up.patch (1.6K, ../../E15AC050-C4B5-488D-BB2D-3C7AC9F89EA8@yesql.se/8-v1-0004-Improve-error-message-for-checksum-state-in-pg_up.patch)
download | inline diff:
From 1bd208979345f830aa053b5e27afac8bff3d2807 Mon Sep 17 00:00:00 2001
From: Daniel Gustafsson <dgustafsson@postgresql.org>
Date: Mon, 7 Sep 2026 12:22:01 +0200
Subject: [PATCH v1 4/5] Improve error message for checksum state in pg_upgrade
When attempting to upgrade a cluster which is an inprogress state, the
same error message was used regardless of which state it was. Fix by
using different error messages for the different inprogress states.
Found by Noah using AI assisted review with Claude. Backpatch to v19
where the online checksums feature was introduced.
Reported-by: Noah Misch <noah@leadboat.com>
Discussion: https://postgr.es/m/...
Backpatch-through: 19
---
src/bin/pg_upgrade/controldata.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/bin/pg_upgrade/controldata.c b/src/bin/pg_upgrade/controldata.c
index 7543a988045..c985e917073 100644
--- a/src/bin/pg_upgrade/controldata.c
+++ b/src/bin/pg_upgrade/controldata.c
@@ -658,8 +658,10 @@ check_control_data(ControlData *oldctrl,
* upgrade. The user should either let the process finish, or turn off
* data checksums, before retrying.
*/
- if (oldctrl->data_checksum_version > PG_DATA_CHECKSUM_VERSION)
+ if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_ON)
pg_fatal("data checksums are being enabled in the old cluster");
+ if (oldctrl->data_checksum_version == PG_DATA_CHECKSUM_INPROGRESS_OFF)
+ pg_fatal("data checksums are being disabled in the old cluster");
/*
* We might eventually allow upgrades from checksum to no-checksum
--
2.39.3 (Apple Git-146)
=
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 20:33 Robert Haas <robertmhaas@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 2 replies; 20+ messages in thread
From: Robert Haas @ 2026-09-10 20:33 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Tue, Sep 8, 2026 at 5:38 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> Noah kindly ran Claude- and GPT-automated reviews on the online checksums
> feature with the fix from the open item applied. Since he has been very
> successful in identifying previously unknown issues in other features I was
> interested in seeing what could shake loose. The full reports are attached,
> with an executive summary of the identified issues and the fixes below. It's
> mostly documentation or corner cases from re-starting operations within tight
> windows, none of the issues would lead to incorrect results or false negatives
> or positives in page verification. Fixes are in separate patches for easier
> review, for a commit I would group the doc ones together.
Nice.
Patch 0003's commit message writes "th" where "the" is intended.
Regarding 0005, Claude is suggesting to me that this might be the
wrong fix. It says (I think correctly) that the real problem is the
code at the end of launcher_exit, which is willing to set
DataChecksumState->launcher_running = false even if our process was
not the one that set that value to true. Unless our local copy of
launcher_running is true, we have no business calling
SetDataChecksumsOff() or setting DataChecksumState->launcher_running =
false. If you fix that, then perhaps it's OK for
DataChecksumsWorkerLauncherMain to stay as it is: the problem there is
that after DataChecksumsWorkerLauncherMain sets launcher_running back
to false, launcher_exit() can still run and decide to mess with shared
state.
Another small problem that Claude found: Within
DataChecksumsWorkerLauncherMain, if an abort is requested, we will set
abort_requested = true and goto done. From there we can goto again.
Now we're starting over, but abort_requested is still true, because
nothing resets it. If the user starts to enable checksums, then
changes their mind and starts to disable them, then changes their mind
again and starts to enable them, the second enable will get confused
and not complete properly. I think this is a one-line fix.
But much more seriously, Claude also noticed this: In general, if
PostgreSQL creates a relation and then crashes before commit, the
relation files are left on disk and not removed, but there's no
catalog entry pointing to them any more. If checksums are enabled
using pg_checksums, such files are updated to have checksums as well.
But this feature enables checksums by working through the catalog, so
it won't update those files. That means that if, at any point in the
history of the cluster, there's ever been a crash that left a relation
file behind on disk, and then you use this feature to enable
checksums, and then after that you take a base backup, checksum
verification will fail, and thus the backup will fail. I don't quite
see how we can actually fix this, and it seems like a serious problem.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 20:37 Tom Lane <tgl@sss.pgh.pa.us>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 2 replies; 20+ messages in thread
From: Tom Lane @ 2026-09-10 20:37 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Robert Haas <robertmhaas@gmail.com> writes:
> But much more seriously, Claude also noticed this: In general, if
> PostgreSQL creates a relation and then crashes before commit, the
> relation files are left on disk and not removed, but there's no
> catalog entry pointing to them any more. If checksums are enabled
> using pg_checksums, such files are updated to have checksums as well.
> But this feature enables checksums by working through the catalog, so
> it won't update those files. That means that if, at any point in the
> history of the cluster, there's ever been a crash that left a relation
> file behind on disk, and then you use this feature to enable
> checksums, and then after that you take a base backup, checksum
> verification will fail, and thus the backup will fail. I don't quite
> see how we can actually fix this, and it seems like a serious problem.
Don't we have logic somewhere to clean up orphaned relation files?
regards, tom lane
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 20:51 Robert Haas <robertmhaas@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 20+ messages in thread
From: Robert Haas @ 2026-09-10 20:51 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 10, 2026 at 4:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Don't we have logic somewhere to clean up orphaned relation files?
Uh ... no?
I mean, temp tables get cleaned up and unlogged tables get reset to
empty, but the goal of being able to remove permanent relation files
instead of permanently leaking them has, to my knowledge, thus far
eluded us. I had hope to address that when working on UNDO, and other
people have tried as well, but unless I've missed something big,
nobody has thus far succeeded.
--
Robert Haas
EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 21:03 Jochen Bandhauer <jb@jbitc.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 20+ messages in thread
From: Jochen Bandhauer @ 2026-09-10 21:03 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; Robert Haas <robertmhaas@gmail.com>; +Cc: Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On 9/10/26 22:37, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> But much more seriously, Claude also noticed this: In general, if
>> PostgreSQL creates a relation and then crashes before commit, the
>> relation files are left on disk and not removed, but there's no
>> catalog entry pointing to them any more. If checksums are enabled
>> using pg_checksums, such files are updated to have checksums as well.
>> But this feature enables checksums by working through the catalog, so
>> it won't update those files. That means that if, at any point in the
>> history of the cluster, there's ever been a crash that left a relation
>> file behind on disk, and then you use this feature to enable
>> checksums, and then after that you take a base backup, checksum
>> verification will fail, and thus the backup will fail. I don't quite
>> see how we can actually fix this, and it seems like a serious problem.
> Don't we have logic somewhere to clean up orphaned relation files?
>
> regards, tom lane
>
There is an item [1] on the TODO list, and a thread [2]:
[1] https://wiki.postgresql.org/wiki/Todo#Administration
[2]
https://www.postgresql.org/message-id/flat/200606081508.k58F85m29270%40candle.pha.pa.us
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 21:04 Greg Burd <greg@burd.me>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 0 replies; 20+ messages in thread
From: Greg Burd @ 2026-09-10 21:04 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 10, 2026, at 4:51 PM, Robert Haas wrote:
> On Thu, Sep 10, 2026 at 4:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Don't we have logic somewhere to clean up orphaned relation files?
>
> Uh ... no?
>
> I mean, temp tables get cleaned up and unlogged tables get reset to
> empty, but the goal of being able to remove permanent relation files
> instead of permanently leaking them has, to my knowledge, thus far
> eluded us. I had hope to address that when working on UNDO, and other
> people have tried as well, but unless I've missed something big,
> nobody has thus far succeeded.
This is incoming with my resurrection of the UNDO work you and other
did and the addition of what I call "FILEOPS".
ETA... a week, target v20.
-greg
> --
> Robert Haas
> EDB: http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 21:08 Daniel Gustafsson <daniel@yesql.se>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Daniel Gustafsson @ 2026-09-10 21:08 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
> On 10 Sep 2026, at 22:33, Robert Haas <robertmhaas@gmail.com> wrote:
Thanks for reviewing!
> Patch 0003's commit message writes "th" where "the" is intended.
Ugh, will fix.
> Regarding 0005, Claude is suggesting to me that this might be the
> wrong fix. It says (I think correctly) that the real problem is the
> code at the end of launcher_exit, which is willing to set
> DataChecksumState->launcher_running = false even if our process was
> not the one that set that value to true. Unless our local copy of
> launcher_running is true, we have no business calling
> SetDataChecksumsOff() or setting DataChecksumState->launcher_running =
> false. If you fix that, then perhaps it's OK for
> DataChecksumsWorkerLauncherMain to stay as it is: the problem there is
> that after DataChecksumsWorkerLauncherMain sets launcher_running back
> to false, launcher_exit() can still run and decide to mess with shared
> state.
Interesting, I will have a look at that tomorrow morning after coffee.
> Another small problem that Claude found: Within
> DataChecksumsWorkerLauncherMain, if an abort is requested, we will set
> abort_requested = true and goto done. From there we can goto again.
> Now we're starting over, but abort_requested is still true, because
> nothing resets it. If the user starts to enable checksums, then
> changes their mind and starts to disable them, then changes their mind
> again and starts to enable them, the second enable will get confused
> and not complete properly. I think this is a one-line fix.
Right, in this case the second enable will not proceed and checksums will end
up disabled. Will have a look.
> But much more seriously, Claude also noticed this: In general, if
> PostgreSQL creates a relation and then crashes before commit, the
> relation files are left on disk and not removed, but there's no
> catalog entry pointing to them any more. If checksums are enabled
> using pg_checksums, such files are updated to have checksums as well.
> But this feature enables checksums by working through the catalog, so
> it won't update those files. That means that if, at any point in the
> history of the cluster, there's ever been a crash that left a relation
> file behind on disk, and then you use this feature to enable
> checksums, and then after that you take a base backup, checksum
> verification will fail, and thus the backup will fail. I don't quite
> see how we can actually fix this, and it seems like a serious problem.
I have a feeling this had been brought up in one of all the postcommit threads,
but I fail to find it now (perhaps it was a basebackup and not checksums
thread?). The gist of that discussion was that there is somewhere between very
little add nothing we can do to protect against orphaned/external files in the
datadir when using the catalog as the main source of truth for what we expect
it to contain. I wonder if the best we can do is to document exactly how to
verify the state of the page when getting a backup page verification failure?
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 21:35 Robert Haas <robertmhaas@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 2 replies; 20+ messages in thread
From: Robert Haas @ 2026-09-10 21:35 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 10, 2026 at 5:08 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> I have a feeling this had been brought up in one of all the postcommit threads,
> but I fail to find it now (perhaps it was a basebackup and not checksums
> thread?). The gist of that discussion was that there is somewhere between very
> little add nothing we can do to protect against orphaned/external files in the
> datadir when using the catalog as the main source of truth for what we expect
> it to contain. I wonder if the best we can do is to document exactly how to
> verify the state of the page when getting a backup page verification failure?
I think that it is fine to disclaim external files -- if you add
random files to the data directory, stuff may break, and that's sad,
but oh well. But I think orphaned files are a completely different
ball game. That's just a normal outcome of running PostgreSQL for a
period of time under real-world conditions, and to my knowledge this
would be the first feature that doesn't work with them. I'd like to
hear what others think, but my first reaction is to think that's
probably not OK. If we don't remove the files automatically, and don't
even provide any tools to remove the files manually, I think it
follows logically that everything we ship has to cope with their
possible existence. Otherwise, it seems like we're shipping a feature
that sometimes doesn't work and for which we can't even provide
reliable steps to fix it when it doesn't.
--
Robert Haas
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 21:39 Tom Lane <tgl@sss.pgh.pa.us>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 0 replies; 20+ messages in thread
From: Tom Lane @ 2026-09-10 21:39 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Sep 10, 2026 at 4:37 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Don't we have logic somewhere to clean up orphaned relation files?
> Uh ... no?
> I mean, temp tables get cleaned up and unlogged tables get reset to
> empty, but the goal of being able to remove permanent relation files
> instead of permanently leaking them has, to my knowledge, thus far
> eluded us.
You're right, I was thinking of temp-table cleanup.
regards, tom lane
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 21:50 Daniel Gustafsson <daniel@yesql.se>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Daniel Gustafsson @ 2026-09-10 21:50 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
> On 10 Sep 2026, at 23:35, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Thu, Sep 10, 2026 at 5:08 PM Daniel Gustafsson <daniel@yesql.se> wrote:
>> I have a feeling this had been brought up in one of all the postcommit threads,
>> but I fail to find it now (perhaps it was a basebackup and not checksums
>> thread?). The gist of that discussion was that there is somewhere between very
>> little add nothing we can do to protect against orphaned/external files in the
>> datadir when using the catalog as the main source of truth for what we expect
>> it to contain. I wonder if the best we can do is to document exactly how to
>> verify the state of the page when getting a backup page verification failure?
>
> I think that it is fine to disclaim external files -- if you add
> random files to the data directory, stuff may break, and that's sad,
> but oh well. But I think orphaned files are a completely different
> ball game. That's just a normal outcome of running PostgreSQL for a
> period of time under real-world conditions, and to my knowledge this
> would be the first feature that doesn't work with them. I'd like to
> hear what others think, but my first reaction is to think that's
> probably not OK. If we don't remove the files automatically, and don't
> even provide any tools to remove the files manually, I think it
> follows logically that everything we ship has to cope with their
> possible existence. Otherwise, it seems like we're shipping a feature
> that sometimes doesn't work and for which we can't even provide
> reliable steps to fix it when it doesn't.
It is too close to midnight for investigation, so I am mostly asking an open
question as a TODO marker for myself in the morning: Are we guaranteed that a
create relation that crash will end up with orphan files containing correct
checksums? That is, if you have a cluster initdb'd with checksums on, and
never use neither online checksums or pg_checksums, can you end with an orphan
file from CREATE TABLE crashing which cause a backup page verification failure?
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-10 22:00 Robert Haas <robertmhaas@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 20+ messages in thread
From: Robert Haas @ 2026-09-10 22:00 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 10, 2026 at 5:50 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> It is too close to midnight for investigation, so I am mostly asking an open
> question as a TODO marker for myself in the morning: Are we guaranteed that a
> create relation that crash will end up with orphan files containing correct
> checksums? That is, if you have a cluster initdb'd with checksums on, and
> never use neither online checksums or pg_checksums, can you end with an orphan
> file from CREATE TABLE crashing which cause a backup page verification failure?
All of the blocks in an orphaned relation file were properly
WAL-logged when they were written, so they might be inconsistent at
the moment of the crash, but crash replay will fix them up so that
they have valid contents. From replay's point of view, those files
aren't orphaned at all, and it treats them just like any other
relation files. The problem is just that at a certain point they stop
being mentioned in the WAL stream any more, because the transaction
that caused them to be created gets terminated by the system going
down, and never has a chance to write additional WAL records asking
for those files to be removed, nor on the other hand to commit so that
the catalog rows that point to those files become visible.
Over time, as activity continues in the rest of the cluster but not in
the orphaned files, the xmin and xmax values in any tuples in those
orphaned files will drift away from what the rest of the cluster is
doing -- the *tuples* won't be readable any more -- but the *pages*
are fine. To base backups and anything else that operates at the
storage layer, they just look like pages we haven't chosen to write
for a long while.
--
Robert Haas
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-11 03:27 Noah Misch <noah@leadboat.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Noah Misch @ 2026-09-11 03:27 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
On Thu, Sep 10, 2026 at 06:00:31PM -0400, Robert Haas wrote:
> On Thu, Sep 10, 2026 at 5:50 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > It is too close to midnight for investigation, so I am mostly asking an open
> > question as a TODO marker for myself in the morning: Are we guaranteed that a
> > create relation that crash will end up with orphan files containing correct
> > checksums? That is, if you have a cluster initdb'd with checksums on, and
> > never use neither online checksums or pg_checksums, can you end with an orphan
> > file from CREATE TABLE crashing which cause a backup page verification failure?
>
> All of the blocks in an orphaned relation file were properly
> WAL-logged when they were written, so they might be inconsistent at
> the moment of the crash, but crash replay will fix them up so that
> they have valid contents. From replay's point of view, those files
> aren't orphaned at all, and it treats them just like any other
> relation files. The problem is just that at a certain point they stop
> being mentioned in the WAL stream any more, because the transaction
> that caused them to be created gets terminated by the system going
> down, and never has a chance to write additional WAL records asking
> for those files to be removed, nor on the other hand to commit so that
> the catalog rows that point to those files become visible.
>
> Over time, as activity continues in the rest of the cluster but not in
> the orphaned files, the xmin and xmax values in any tuples in those
> orphaned files will drift away from what the rest of the cluster is
> doing -- the *tuples* won't be readable any more -- but the *pages*
> are fine. To base backups and anything else that operates at the
> storage layer, they just look like pages we haven't chosen to write
> for a long while.
Offhand, I expect checksum violations in orphan files can arise with
wal_level=minimal. If the transaction never reaches commit, or its commit
takes the smgrDoPendingSyncs() -> smgrdosyncall() path, the new relfilenode
has no WAL.
wal_level=minimal was last the default in v9.6, well before checksums were on
by default. Also, a checksum failure in pg_basebackup would require raising
wal_level after making the orphan. Those prerequisites may explain the lack
of reports.
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-14 13:47 Daniel Gustafsson <daniel@yesql.se>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Daniel Gustafsson @ 2026-09-14 13:47 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>
> On 10 Sep 2026, at 23:35, Robert Haas <robertmhaas@gmail.com> wrote:
> I think that it is fine to disclaim external files -- if you add
> random files to the data directory, stuff may break, and that's sad,
> but oh well. But I think orphaned files are a completely different
> ball game. That's just a normal outcome of running PostgreSQL for a
> period of time under real-world conditions, and to my knowledge this
> would be the first feature that doesn't work with them. I'd like to
> hear what others think, but my first reaction is to think that's
> probably not OK. If we don't remove the files automatically, and don't
> even provide any tools to remove the files manually, I think it
> follows logically that everything we ship has to cope with their
> possible existence. Otherwise, it seems like we're shipping a feature
> that sometimes doesn't work and for which we can't even provide
> reliable steps to fix it when it doesn't.
Thinking about this over the weekend I think there is really only one possible
option. When enabling checksums online the data checksums launcher will have
to scan the datadir for orphan files, and abort processing if any are found
(and log which files it found). While we don't have a tool for clearning, I
don't think online checksums can, or should, handle the orphan files in any way
other than bailing out. This needs adding net new code, which this really
isn't a great time to be doing. CC:ing Heikki from RMT since I know he has
eyes on this already.
I ended up pushing the patchset for the other open item to master in the
meantime (after off-list +1 from RMT), but held off from backpatching to 19 at
the time of pushing till the BF has had a few builds of it. If we decide to
take online checksums out of 19 due to the above it's also less to back out.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-14 19:59 Robert Haas <robertmhaas@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 1 reply; 20+ messages in thread
From: Robert Haas @ 2026-09-14 19:59 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>
On Mon, Sep 14, 2026 at 9:47 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> Thinking about this over the weekend I think there is really only one possible
> option. When enabling checksums online the data checksums launcher will have
> to scan the datadir for orphan files, and abort processing if any are found
> (and log which files it found). While we don't have a tool for clearning, I
> don't think online checksums can, or should, handle the orphan files in any way
> other than bailing out. This needs adding net new code, which this really
> isn't a great time to be doing. CC:ing Heikki from RMT since I know he has
> eyes on this already.
It's for the RMT to decide whether that's good enough, but I'm
skeptical. I think a big part of the problem here is that it's really
hard to make something race-free. But even if we solve that problem or
disclaim it, I feel like it doesn't leave users in a very good place.
A lot of users will experience failures and will have no easy way to
find their way out from under the problem. Against that, there's a
plausible argument that users want this feature enough that they'll
still be happier to have the feature despite that problem. My
counter-argument to that is that if we shipped features with usability
restrictions that severe in every release, I don't think we'd be where
we are today as a project. Limitations are fine, but limitations where
there's not much you can do to avoid the problem or recover from it
are pretty bad, IMHO.
--
Robert Haas
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-14 21:59 Daniel Gustafsson <daniel@yesql.se>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 2 replies; 20+ messages in thread
From: Daniel Gustafsson @ 2026-09-14 21:59 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>; rmt@lists.postgresql.org
> On 14 Sep 2026, at 21:59, Robert Haas <robertmhaas@gmail.com> wrote:
> It's for the RMT to decide whether that's good enough, but I'm
> skeptical. I think a big part of the problem here is that it's really
> hard to make something race-free.
Once the cluster has moved to inprogress-on, any new orphan files will have
checksums written. This means that if the launcher can get a list of orphan
files once the state has changed, there cannot be any more created (without
checksums) during processing. There can still be new orphan files *with*
checksums written, but those would be no different from what we have today with
pg_checksums processing.
This reduction in problem scope likely means we can avoid the race conditions
that such scans are usually subject to. This is back-of-napkin sketching but I
think this invariant of the orphan files problem is less hard than the generic
one.
But at any rate, this is clearly not the time to design that.
If we revert we should keep online checksums in master and only pull it out of
19, to allow for this to be fixed early in 20, and avoid the churn. I held off
from backpatching the open item fix for this very reason; unless I hear
otherwise by Wednesday I'll go ahead with the revert, else the backpatch.
Adding RMT to the thread to keep everyone in the loop.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-14 23:30 Robert Haas <robertmhaas@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
1 sibling, 1 reply; 20+ messages in thread
From: Robert Haas @ 2026-09-14 23:30 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>; rmt@lists.postgresql.org
On Mon, Sep 14, 2026 at 5:59 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> Once the cluster has moved to inprogress-on, any new orphan files will have
> checksums written. This means that if the launcher can get a list of orphan
> files once the state has changed, there cannot be any more created (without
> checksums) during processing. There can still be new orphan files *with*
> checksums written, but those would be no different from what we have today with
> pg_checksums processing.
That's a very good point. One thing that bothers me is: why don't we
just process the files by directory scan to begin with, instead of
trying to get at them via pg_class and then using this mechanism to
mop up the leftovers? I suspect the answer is locking. For an actually
orphaned file, we don't need to lock anything. But it might be hard to
tell the difference between a file that corresponds to nothing we saw
in pg_class because it's orphaned and a file that corresponds to
nothing we saw in pg_class because it was concurrently created and the
pg_class entry isn't yet visible to us. And, in the latter case,
skipping locking would probably be bad for all the same reasons it
would be bad to do that in general. But if you can find some way to
distinguish those cases reliably, then I think this might be a robust
fix.
--
Robert Haas
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-15 04:24 Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Bertrand Drouvot @ 2026-09-15 04:24 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Daniel Gustafsson <daniel@yesql.se>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>; rmt@lists.postgresql.org
Hi,
On Tue, Sep 15, 2026 at 1:31 AM Robert Haas <robertmhaas@gmail.com> wrote:
>
> But it might be hard to
> tell the difference between a file that corresponds to nothing we saw
> in pg_class because it's orphaned and a file that corresponds to
> nothing we saw in pg_class because it was concurrently created and the
> pg_class entry isn't yet visible to us. And, in the latter case,
> skipping locking would probably be bad for all the same reasons it
> would be bad to do that in general. But if you can find some way to
> distinguish those cases reliably, then I think this might be a robust
> fix.
pg_orphaned ([1]) is using a dirty snapshot while looking for the
relfilnode(s) in
pg_class to distinguish those cases. Maybe we could do the same here?
[1]: https://github.com/bdrouvot/pg_orphaned
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-15 11:59 Nathan Bossart <nathandbossart@gmail.com>
parent: Daniel Gustafsson <daniel@yesql.se>
1 sibling, 1 reply; 20+ messages in thread
From: Nathan Bossart @ 2026-09-15 11:59 UTC (permalink / raw)
To: Daniel Gustafsson <daniel@yesql.se>; +Cc: Robert Haas <robertmhaas@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>; rmt@lists.postgresql.org
On Mon, Sep 14, 2026 at 11:59:15PM +0200, Daniel Gustafsson wrote:
> But at any rate, this is clearly not the time to design that.
>
> If we revert we should keep online checksums in master and only pull it out of
> 19, to allow for this to be fixed early in 20, and avoid the churn. I held off
> from backpatching the open item fix for this very reason; unless I hear
> otherwise by Wednesday I'll go ahead with the revert, else the backpatch.
> Adding RMT to the thread to keep everyone in the loop.
Thanks for the heads up. Based on the discussion, it sounds like reverting
is the right call, unfortunately.
--
nathan
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-15 12:18 Daniel Gustafsson <daniel@yesql.se>
parent: Nathan Bossart <nathandbossart@gmail.com>
0 siblings, 1 reply; 20+ messages in thread
From: Daniel Gustafsson @ 2026-09-15 12:18 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>; rmt@lists.postgresql.org
> On 15 Sep 2026, at 13:59, Nathan Bossart <nathandbossart@gmail.com> wrote:
>
> On Mon, Sep 14, 2026 at 11:59:15PM +0200, Daniel Gustafsson wrote:
>> But at any rate, this is clearly not the time to design that.
>>
>> If we revert we should keep online checksums in master and only pull it out of
>> 19, to allow for this to be fixed early in 20, and avoid the churn. I held off
>> from backpatching the open item fix for this very reason; unless I hear
>> otherwise by Wednesday I'll go ahead with the revert, else the backpatch.
>> Adding RMT to the thread to keep everyone in the loop.
>
> Thanks for the heads up. Based on the discussion, it sounds like reverting
> is the right call, unfortunately.
I target doing it early tomorrow my time to allow lots of buildfarm monitoring
time, and to have time to look over the patch tonight.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: Trying to break online checksums with LLMs
@ 2026-09-16 21:38 Daniel Gustafsson <daniel@yesql.se>
parent: Daniel Gustafsson <daniel@yesql.se>
0 siblings, 0 replies; 20+ messages in thread
From: Daniel Gustafsson @ 2026-09-16 21:38 UTC (permalink / raw)
To: Nathan Bossart <nathandbossart@gmail.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>; Heikki Linnakangas <hlinnaka@iki.fi>; rmt@lists.postgresql.org
> On 15 Sep 2026, at 14:18, Daniel Gustafsson <daniel@yesql.se> wrote:
>> On 15 Sep 2026, at 13:59, Nathan Bossart <nathandbossart@gmail.com> wrote:
>> Thanks for the heads up. Based on the discussion, it sounds like reverting
>> is the right call, unfortunately.
>
> I target doing it early tomorrow my time to allow lots of buildfarm monitoring
> time, and to have time to look over the patch tonight.
This was done earlier today.
--
Daniel Gustafsson
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2026-09-16 21:38 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-09-08 21:38 Trying to break online checksums with LLMs Daniel Gustafsson <daniel@yesql.se>
2026-09-10 20:33 ` Robert Haas <robertmhaas@gmail.com>
2026-09-10 20:37 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-09-10 20:51 ` Robert Haas <robertmhaas@gmail.com>
2026-09-10 21:04 ` Greg Burd <greg@burd.me>
2026-09-10 21:39 ` Tom Lane <tgl@sss.pgh.pa.us>
2026-09-10 21:03 ` Jochen Bandhauer <jb@jbitc.de>
2026-09-10 21:08 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-10 21:35 ` Robert Haas <robertmhaas@gmail.com>
2026-09-10 21:50 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-10 22:00 ` Robert Haas <robertmhaas@gmail.com>
2026-09-11 03:27 ` Noah Misch <noah@leadboat.com>
2026-09-14 13:47 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-14 19:59 ` Robert Haas <robertmhaas@gmail.com>
2026-09-14 21:59 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-14 23:30 ` Robert Haas <robertmhaas@gmail.com>
2026-09-15 04:24 ` Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
2026-09-15 11:59 ` Nathan Bossart <nathandbossart@gmail.com>
2026-09-15 12:18 ` Daniel Gustafsson <daniel@yesql.se>
2026-09-16 21:38 ` Daniel Gustafsson <daniel@yesql.se>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox