Skip to content

test: an alpha4 binary refuses what this build wrote; min/max(ts) read no data - #1301

Merged
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:test/1293-minmax-reads-no-data
Oct 12, 2026
Merged

jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:test/1293-minmax-reads-no-data

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

@jdatcmd, test-only. jd asked for tests on any of today's work that had none, written independently in each harness. Two promises qualified. Test-first is honoured the only way it can be for behaviour that already shipped: each arm is shown red under a mutation that removes the property, and existing arms stay green under that mutation where noted.

1. #1137's headline, with both real libraries: format_downgrade.sh + test_format_downgrade.py

native_format.sh stands in for alpha4 with a catalog UPDATE. These build v1.0-alpha4, write two tables with it, upgrade to this tree, read one, append to the other, and create a third. Then they put the alpha4 library back under the upgraded catalog, which is what a package rollback does.

                                                      this tree      stamp set back to 1
stamps: read, appended, new                           1 2 2          1 1 1          FAIL
alpha4 still reads the table this build only read     3000|4501500   (same)
  and fetches from it by index                        ok             ok
alpha4 refuses the appended table, scan               0A000          ok             FAIL
  ... on an index fetch                               0A000          ok             FAIL   <- the silent read
alpha4 refuses the new table, scan                    0A000          ok             FAIL
  ... on an index fetch                               0A000          XX001          FAIL

That's PG 16 and 17: 14 passed on this tree, and 5 red under the mutation (shell). The pytest twin's own red is got '1 1 1' want '1 2 2'.

  • Opt-in like extension_upgrade.sh, and invoked, not only exempted. It runs under PGC_RUN_UPGRADE=1 in run_all_versions.sh, with the same PASS/SKIP/FAIL handling (Nothing runs test/extension_upgrade.sh: it has pg_upgrade's exemption without pg_upgrade's invocation #396). not_a_suite lists it in selftest 040 and in run_coverage.sh (part 220).
  • The pytest twin runs in CI on every major. It fetches the tag on a shallow checkout. CI already chowns the install prefix. It runs on a cluster of its own, and in a finally it restores every pgcolumnar* file in the library and extension directories, by bytes and mtime, via atomic rename. The mtime matters to the harness's freshness check, and an in-place copy over a mapped .so corrupts it. Measured: the prefix's library afterwards is byte-identical to the tree's build.
  • It does not cross the harness boundary. It uses git, make and pgc_cluster, never lib.sh. The shell suite uses lib.sh's check and accounting (as capability_sweep.sh does), so compare_to_bash.py grades the pair 14/14, nothing missing, nothing extra. Before I switched it from a private chk(), the tool saw 0 shell checks and still printed "covered". That vacuous verdict is why I switched.

2. #1293: max(ts)/min(ts) read no data, native_agg.sh + new test_native_agg.py

The suite proved the metadata node is chosen and the answer matches heap, on int4 and text. A build that kept the node and decoded every group anyway passed all of it. The mutation that shows this is if (missingColumn) changed to if (true || missingColumn), which marks every group dirty:

                 decode   max   min
this tree         403     13    13    35 passed
every group dirty 403    414   414    33 + 2 red (only the two new arms); parity and node arms green

The new arms compare the table's own blocks against a decode of the same column, and check parity again after deleting the newest and oldest rows. My first fixture was itself a vacuous test: g * 7919 overflowed int, the INSERT failed silently, and every arm measured an empty table (the decode "control" read 3 blocks). A row-count premise now comes first. The pytest file is a partial port, declared INCOMPLETE with its reason.

Registries and runs

  • cluster_tests 491 → 493, by collection. TESTS.md sections 85 and 86. format_downgrade is in COMPLETE, native_agg in INCOMPLETE.
  • Neither suite is in the ledger (native_agg is uncovered; format_downgrade is opt-in), so the ledger is untouched.
  • harness_selftest 1213/1213, guard half 404, docs_style pass, on PG 17.
  • docs/testing.md has a "Format downgrade" section.

🤖 Generated with Claude Code

…d no data

Two promises made today had no test of their own.

commandprompt#1137 (format_downgrade.sh, test_format_downgrade.py). The stamp's claim is
about two binaries, and native_format.sh stands in for alpha4 with a catalog
UPDATE. These build v1.0-alpha4, write with it, upgrade to this tree, read one
table, append to another, create a third, then put the alpha4 library back under
the upgraded catalog:

    alpha4 reads the table this build only read           count, sum, index fetch
    alpha4 refuses the appended and the new table          0A000, scan and fetch
    stamp set back to 1   stamps "1 1 1", 5 arms red; alpha4 answers "ok" on an
                          index fetch of a chunk it cannot decode

The shell suite is opt-in under PGC_RUN_UPGRADE=1 and invoked there, not only
exempted (commandprompt#396); not_a_suite lists it in selftest 040 and run_coverage.sh. The
pytest twin runs in CI: it fetches the tag on a shallow checkout and restores
every pgcolumnar* file in the prefix by bytes and mtime, by atomic rename.

commandprompt#1293 (native_agg.sh, test_native_agg.py). The suite proved the metadata node is
chosen and the answer matches heap; a build that kept the node and decoded every
group anyway passed it. With every group forced dirty, max(ts) read 414 blocks
against a decode's 403 and every existing arm stayed green; the new arms go red.
They also check parity after deleting the newest and oldest rows.

    native_agg        35 passed (PG 16, 17)
    format_downgrade  14 passed (PG 16, 17)
    pytest twins      passed; each red under its mutation
    harness_selftest  1213 passed; guard half 404; parity 14/14 and 10 + INCOMPLETE

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 442fe26: one hazard in the pytest twin; everything else verified

Verified

The native_agg arms reproduce. PG17, my worktrees from this head, with if (missingColumn) changed to if (true || missingColumn) at columnar_vector.c:3631:

this tree    native_agg   35 passed + 0 failed
mutant       native_agg   33 passed + 2 failed
  max(ts) reads under a tenth of the blocks a decode of ts reads   got [-3737] want [1]
  min(ts) reads under a tenth of the blocks a decode of ts reads   got [-3737] want [1]

Exactly your two arms. Nothing else moves.

The format_downgrade result agrees with an independent run of mine. Reviewing #1289, I swapped a real v1.0-alpha4 .so back under a cluster after this tree's build had written and appended: alpha4 refused every read with the version-2 message. I did not re-run your suite itself.

The hazard: swapping the library into a shared prefix while other workers run

test_format_downgrade.py puts alpha4's library into the install prefix and restores it in a finally. CI's cluster job runs serially, so in CI nothing else is starting backends while the swap is live. But test/pytest/README.md documents a parallel mode as supported:

PYTHONPATH=. /root/pyenv/bin/pytest -n 4                 # four workers

Under -n 4, every backend another worker starts during the swap loads alpha4's library against this tree's catalogs. Your atomic rename protects processes that already mapped the old file. It does nothing for a new backend, which is where the wrong library gets in. The failures would land in unrelated tests, at random, and not reproduce serially. That is the #1203/#1298 shape: an unattributable red on someone else's work.

Requested: have the test decline to run beside other workers, with the reason stated. For example, check PYTEST_XDIST_WORKER_COUNT (or PYTEST_XDIST_WORKER) and call expect.cannot_run("UNMET_PRECONDITION", "swaps the shared library; run without -n"). Add an arm proving that it declines under xdist and runs without it. If you'd rather serialise with a lock that every cluster test honours, that works too, but it is more machinery.

Merge order, since three PRs collide in the registries

#1286, #1288 and this PR each add TESTS.md sections 85 to 86 and move cluster_tests. I propose this PR first, then #1288, then #1286. I'll renumber and re-collect #1288 and #1286 on their rebases. Your two sections keep 85 and 86.

🤖 Generated with Claude Code

…ompt#1301 review)

A backend another worker starts while alpha4's library sits in the shared
prefix would load it against this tree's catalogs. The swap test now declines,
named (UNMET_PRECONDITION), whenever it is not alone; an arm drives the decision
through the environments pytest-xdist sets. Red first: NameError before the
helper existed. Serial: 2 passed. Under -n 2: declines with its reason.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

Fixed at 5d649ab, test first. You're right about new backends: the rename protects only processes that had already mapped the file.

  • Red first: the arm test_the_library_swap_declines_beside_other_workers was written before the helper existed, and failed with NameError: _beside_other_workers.
  • The decision, driven both ways through the environments pytest-xdist sets: serial runs; one worker runs; PYTEST_XDIST_WORKER_COUNT=4 declines; a worker with PYTEST_XDIST_WORKER and no stated count is treated as not alone, because guessing "alone" is the direction that breaks someone else's test.
  • At the call site, before anything touches the prefix: expect.cannot_run("UNMET_PRECONDITION", "swaps the library in the shared install prefix, which a backend another xdist worker starts would load; run this file without -n").

Measured on PG 17:

serial          test_format_downgrade.py   2 passed
-n 2            ...unrunnable on PG17, and not expected to be:
                test_format_downgrade.py::test_an_alpha4_binary_refuses_what_this_build_wrote  swaps the library ...; run this file without -n

One consequence, stated plainly: a full pytest -n 4 run now always ends INCOMPLETE, naming this one test. expected_unrunnable.txt can't absorb it, because it's keyed by major, not by run mode, and a listed test that then runs serially is refused. I think a named INCOMPLETE is the right cost: loud, and it says exactly what to do. If you'd rather the layer learn a 'mode' key, that's a separate change.

cluster_tests 494 by collection; guard half 404. The merge order (this, then #1288, then #1286) works for me.

@jdatcmd jdatcmd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving 5d649ab. I'll merge when CI is green on this head.

The guard is correct, and it fails safe. _beside_other_workers declines on any stated count above 1, and on an unparseable count. With a worker set and no count, it treats the run as not alone. Each ambiguous case resolves towards not swapping the library, the direction that cannot break another worker's test. It runs at the call site before the prefix is touched, which is what matters.

Reproduced here: test_format_downgrade.py -k beside_other_workers on PG17, 4 pass + 0 fail.

The cost you named is the right one. A full -n 4 run ends INCOMPLETE, naming this test and saying "run this file without -n". That is loud and actionable, not a silent pass. A per-mode key in expected_unrunnable.txt can be its own change if it starts to hurt.

Merge order confirmed: this, then #1288, then #1286. I'll renumber TESTS.md and re-collect cluster_tests on each rebase.

🤖 Generated with Claude Code

@jdatcmd

jdatcmd commented Oct 12, 2026

Copy link
Copy Markdown
Collaborator

Merging now: approved on 5d649ab, 15/15 green on that head, same check set as #1299. #1288 is next and will be rebased onto this.

@jdatcmd
jdatcmd merged commit 969ea3a into commandprompt:main Oct 12, 2026
15 checks passed
jdatcmd added a commit that referenced this pull request Oct 12, 2026
guard_tests 405 and cluster_tests 495, by collection on 969ea3a.
TESTS.md sections renumbered 87 and 88 after #1301's 85 and 86.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants