From 7a61938fdeb00b33faaf794312a2518f7abef233 Mon Sep 17 00:00:00 2001 From: Noah Misch 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 --prefix= \ + -Dcassert=true -Ddebug=true -Doptimization=1 \ + -Dtap_tests=enabled -Dinjection_points=true +ninja -C install +meson test -C --suite setup +meson test -C 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