Structural fresh-vs-update comparison + convergence/divergence report

[jnasbyupgrade]

⚠️ SUPERSEDED — this PR is meant to be replaced, not merged as-is. It's stacked on #32, whose premise (crossing pg-upgrade-test/extension-update-test with a fixed ["", Quoted] TEST_SCHEMA matrix) is being replaced by the always-randomize-the-install-schema design in #55. This PR's own "genuinely novel" claim (crossing U&U with TEST_SCHEMA) and its bin/compare_fresh_vs_update wiring are built directly on top of that now-obsolete axis. Left open for reference only.

---

Stacked on #32 (phase 5: crossing U&U with schema). Last PR in this series - adds the one remaining checklist item that had no precedent anywhere in the org, and closes out with the convergence/divergence writeup this whole exercise was actually meant to produce.

What changed

• bin/compare_fresh_vs_update: installs fresh and 0.9.6-then-updated copies of count_nulls in scratch databases and diffs pg_get_functiondef/comments/ACLs for every object the extension owns (via pg_depend, not a hardcoded list). Wired into extension-update-test automatically, crossed with TEST_SCHEMA same as the rest of that job.
• Scoped OUT extending pg-tle-test to the update path, and said so explicitly rather than silently skipping it: pgxntool 2.3.0's own fix for installcheck's ordering bug (Fix installcheck running before install; make test now fails on regressions pgxntool#83, landed in Update pgxntool to 2.3.0 #25) made installcheck unconditionally depend on install, which writes a real .control file to disk - defeating the entire point of a pg_tle-deployment test. Filed as installcheck's new install prerequisite (fix for #79) closes off testing a filesystem-install-free deployment (e.g. pg_tle) pgxntool#90.

Convergence/divergence report

This whole series (#25-#32) started from a design doc built around porting cat_tools PR #16's U&U testing pattern more or less directly. Partway through, the actual work diverged significantly - worth recording explicitly, since that's the real signal for what's generic (belongs in pgxntool) vs. specific to one repo's history.

Taken essentially verbatim, from pgxntool/README.asc's own documented pattern (not cat_tools directly - see below): the TEST_LOAD_SOURCE/TEST_SCHEMA GUC-propagation mechanism (PGOPTIONS -> current_setting(), read without missing_ok), the dependency-guard technique (plant + actively prove a blocked non-CASCADE drop, re-prove after every step), the changes job's docs-only gate and single-source PG-major derivation, and the test/install self-comparing-output convention (don't track load.out - correctness comes from the SQL failing loudly, not a textual diff).

What actually diverged from the original plan, and why:

• test/install, not per-test-file deps.sql GUCs. The first draft of this work (PR Add full update+upgrade test harness (TEST_LOAD_SOURCE/TEST_SCHEMA, bin/test_existing, pg-upgrade-test/extension-update-test) #23, now closed) ported cat_tools' bin/test_existing-heavy pattern almost directly, threading TEST_LOAD_SOURCE/TEST_SCHEMA through test/deps.sql - which runs inside a per-test rolled-back transaction, so an in-place ALTER EXTENSION UPDATE there never actually commits (silently untested against the one thing that matters most: real production behavior). pgxntool/README.asc's own "Update & Upgrade (U&U) Testing" section (which the earlier draft hadn't read closely enough) says this outright, and - strikingly - cites this very repo's own old code as the example of what NOT to copy (a plpgsql-function-assertion style that "adds complexity of its own"). Restarted from scratch on test/install instead.
• Schema-invariant assertion descriptions (design (A)), not per-schema alternate expected-output files. The original PR Add full update+upgrade test harness (TEST_LOAD_SOURCE/TEST_SCHEMA, bin/test_existing, pg-upgrade-test/extension-update-test) #23 needed THREE numbered alternate expected-output files to cross TEST_SCHEMA with the rest of the suite. Rebuilding core/functions.sql's pgTAP call descriptions to never embed the literal schema name (while still using it in the actual executed SQL, via ncs()) reduced that to ONE alternate, for a genuinely different reason (whether there's a schema to clean up, not an artifact of how the extension landed there).
• bin/test_existing shrunk to two subcommands. Once test/install could handle fresh AND update entirely inside one committed pg_regress session, extension-update-test needed no external script at all - just make test-update. bin/test_existing now exists ONLY for the one thing that genuinely can't live inside pg_regress: a real binary pg_upgrade run.
• Bumped pgxntool to 2.3.0 first, as its own phase (Update pgxntool to 2.3.0 #25) - not because the doc called for it, but because 2.3.0 (which the user was about to release) directly fixed two things this exact work was about to reinvent workarounds for (the DATA wildcard gap, PGTLE_VERSION collision) and changed a documented make test exit-code assumption the whole design leaned on.

Genuinely novel, no precedent anywhere in the org (checked directly - see #32): crossing U&U (extension-update-test, pg-upgrade-test) with TEST_SCHEMA. Neither cat_tools nor extension_tools does this. Made possible - not just easier - by the schema-invariant-description design above: without it, this would have meant a combinatorial expected-file explosion instead of zero new files.

Skipped, deliberately: the pg_tle update-path extension (see above, blocked upstream). The every-major pg-upgrade-stepwise climb (count_nulls has no catalog-touching views/functions - nothing version-sensitive to break at a specific PG-major boundary, so the two big-jump pg-upgrade-test legs already cover the real risk).

Filed upstream along the way, as real findings from doing this work, not tangential: Postgres-Extensions/pgxntool#86 (agents have no lightweight way to discover README.asc's relevant sections without reading the whole thing), Postgres-Extensions/pgxntool#90 (the installcheck/install regression above).

Verification

Locally against PG17: all four combinations (fresh/update × empty/Quoted schema) pass via make verify-results; bin/compare_fresh_vs_update reports identical object definitions for both schema legs.

[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: 8c27ec0b-3f1a-4c08-a3c2-5f672387a493

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]

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

[github-actions]

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

---

Code review: PR #33 (Structural fresh-vs-update comparison + convergence/divergence report)

Task list

• Gather context (PR description, changed files)
• Read bin/compare_fresh_vs_update, the ci.yml diff, and test/README.md diff in full
• Cross-check against Makefile's TEST_SCHEMA/TEST_LOAD_SOURCE conventions and the sibling bin/test_existing for consistency
• Post review findings

---

Summary

Reviewed the diff against origin/phase5-cross-schema...HEAD (3 files, +143/-0). No bugs found. The script is well-scoped and its extensive header comments accurately describe its own behavior — I verified the claims rather than just trusting them.

What I checked specifically

• bin/compare_fresh_vs_update correctness:

  • extension_members()'s discovery query walks pg_depend (deptype = 'e') joined to pg_extension/pg_proc rather than a hardcoded object list — confirmed against sql/count_nulls*.sql that the extension only ever creates functions (including trigger functions), never a standalone CREATE TRIGGER, so restricting the comparison to pg_proc is complete, not an oversight.
  • Both the fresh and updated installs are made to target the same schema for a given invocation, so pg_get_functiondef/regprocedure rendering (which is search_path-sensitive) stays consistent between the two snapshots being diffed — schema-qualification differences won't produce false-positive diffs.
  • set -euo pipefail + trap cleanup EXIT correctly drops both scratch DBs and temp files on any exit path; the if diff -u ...; then ... else ...; fi correctly avoids tripping errexit on a real (expected) diff.
  • The ci.yml step (test job, lines 261-274) runs this only after the make install step, which is required since the script's CREATE EXTENSION calls depend on the extension already being installed on the runner — ordering is correct.
  • Confirmed .github/workflows/ci.yml and the embedded shell block parse fine.

• Minor, non-blocking observation: install_in_schema() (bin/compare_fresh_vs_update:~90) interpolates $schema directly into a double-quoted SQL identifier (CREATE SCHEMA IF NOT EXISTS \"$schema\") with no escaping. Not exploitable today — the script is only ever invoked with the two fixed literals ""/"Quoted" from ci.yml — but if this script is ever called with a less-trusted value down the line, an embedded " would break out of the identifier. Not worth blocking on given current call sites; flagging only for awareness. Fix this →

• Docs/CI wiring: test/README.md's new bullet and the ci.yml step/job comments accurately describe the actual behavior added (verified line-by-line against the script and Makefile, not just read for prose quality). The pg-tle-test scope-out comment and its rationale (pgxntool installcheck/install ordering regression) matches the linked upstream issue description in the PR body.

• PR body's own convergence/divergence writeup: matches what's actually in the diff — no discrepancy between the narrative and the code.

No CLAUDE.md violations found (checked pgxntool/CLAUDE.md's guidance, which is scoped to pgxntool development and defers to the host project's own instructions — no host-project CLAUDE.md exists in this repo).

Verdict

No changes requested. The one item above is optional/informational, not a blocker.