TEST_LOAD_SOURCE (fresh/update/existing) in test/install

[jnasbyupgrade]

Stacked on #28 (phase 2: schema switching). Adds the update+upgrade (U&U) load-mode axis test/install was always meant to carry, per pgxntool/README.asc's documented pattern.

What changed

• `test/install/load.sql` selects fresh/update/existing via the `count_nulls.test_load_mode` GUC (`TEST_LOAD_SOURCE` make var, same propagation mechanism as `TEST_SCHEMA`):
  • fresh (default): unchanged from phases 1/2.
  • update: `CREATE EXTENSION count_nulls VERSION '0.9.6'` then `ALTER EXTENSION UPDATE` - committed, since `test/install` runs outside any per-test rolled-back transaction (the whole reason this pattern needs `test/install` rather than `test/deps.sql` - a real `ALTER EXTENSION UPDATE` has to actually commit to be tested honestly).
  • existing: asserts count_nulls is already installed and at the current version, touches nothing - for a real `pg_upgrade` run external to this invocation (a later phase adds the CI job that drives this).
• New `extension-update-test` CI job: just `make verify-results TEST_LOAD_SOURCE=update`. No external script needed for this leg - unlike a real `pg_upgrade`, an in-place update is pure SQL, so `test/install` can do the whole fresh-vs-updated comparison inside one `pg_regress` invocation.

Deliberately does NOT add a dependency guard yet (the "prove a non-CASCADE drop is blocked" mechanism) - that only earns its keep protecting against an EXTERNAL step corrupting state test/install can't see into (a real `pg_upgrade`), which doesn't exist until the next phase. Adding it here would also immediately conflict with the main suite's own `test__shutdown__drop_all`. Also not crossing this new job with `TEST_SCHEMA` yet - that's its own phase, once the real pg_upgrade job exists too so both can cross schema together.

Verification

Locally against PG17: fresh, update, and existing modes all pass via `make verify-results` - zero expected-output changes needed for any combination, including `update` × `TEST_SCHEMA=Quoted` crossed together, confirming phase 2's schema-invariant design holds across load modes too (load-bearing for the later phase that crosses U&U with schema in CI). `existing` mode verified manually against a real out-of-band `CREATE EXTENSION` + `--use-existing` run.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 87275d90-1520-4c94-b062-c05e7d0b9b22

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

test-permission-check

[github-actions]

Claude finished @jnasbyupgrade's task in 3m 7s —— View job

---

Claude's Review of PR #29

• Read repo CLAUDE.md / pgxntool README (U&U testing pattern) for context
• Review diff (test/install/load.sql, Makefile, .github/workflows/ci.yml)
• Post review feedback

Note: this PR is already merged, so this is a retrospective review for the record / any follow-up cleanup.

Summary

This implements the "load-mode" (fresh/update/existing) axis exactly as documented in pgxntool/README.asc's U&U Testing section. I verified:

• sql/count_nulls--0.9.6.sql (full install) and the 0.9.6 → 1.0.0 → stable upgrade chain all exist, so ALTER EXTENSION UPDATE in update mode has a real path to the current default_version (stable, per count_nulls.control).
• test/deps.sql was already emptied (from an earlier phase) so the per-test CREATE EXTENSION no longer conflicts with the committed test/install load — consistent with the pattern's prerequisite.
• The :'var'/\if boolean idiom in load.sql (booleans round-tripping through psql as t/f) is the same idiom already used for TEST_SCHEMA, so it's proven safe by phase 2.

Overall this is solid, well-documented work. A few minor, non-blocking observations:

1. .github/workflows/ci.yml:20-55 — the PR description says "New extension-update-test CI job", but the actual change adds Install count_nulls / Update 0.9.6 -> current and run the suite as extra steps inside the existing test job (the in-file comment explains this was a deliberate choice, "a single job rather than a dedicated one"). Just flagging the description/implementation mismatch in case it's confusing for future readers of the PR history — the code itself looks intentional and well-reasoned.

2. test/install/load.sql:114-117 — SET client_min_messages = WARNING; / ...= NOTICE; around ALTER EXTENSION UPDATE hardcodes the "restore" value to NOTICE rather than using RESET client_min_messages;. Low risk in this context (regress sessions default to NOTICE), but RESET would restore whatever was actually in effect rather than assuming a value.

3. test/install/load.sql:58-60 — CREATE SCHEMA IF NOT EXISTS :"schema" runs unconditionally before the mode branch, including in existing mode. Today this is harmless since existing mode is never crossed with a non-empty TEST_SCHEMA yet (per the PR description, that's deferred to a later phase), and IF NOT EXISTS makes it a no-op when the schema is already there from the real out-of-band install. Worth a second look when the schema-crossing phase lands, since existing mode's contract is "touches nothing."

4. .github/workflows/ci.yml:52-55 — the explicit make install step immediately before make verify-results TEST_LOAD_SOURCE=update looks redundant: verify-results already depends on install/installcheck via TEST_DEPS in pgxntool/base.mk. Not incorrect, just an extra step.

None of these are blocking — they're small polish items for a later pass if the author wants them. The core design (mode selection via count_nulls.test_load_mode GUC, missing_ok-free reads, schema-invariant expected output) is sound and matches the documented pattern well.