Skip to content

Commit 5e27c8a

Browse files
jnasbyupgradeclaude
andcommitted
Phase 6a: structural fresh-vs-update comparison (bin/compare_fresh_vs_update)
Adds the one checklist item genuinely missing everywhere (cat_tools' own PR #46 only did this manually, uncommitted, per the design doc this whole series is based on): a script that installs fresh and 0.9.6-then-updated copies of count_nulls in their own scratch databases and diffs pg_get_functiondef/comments/ACLs for every object the extension owns (discovered live via pg_depend, not a hardcoded object list). Wired into extension-update-test as an automatic step, crossed with TEST_SCHEMA same as the rest of that job. Also scopes out extending pg-tle-test to the update path (documented inline, and filed as Postgres-Extensions/pgxntool#90): pgxntool 2.3.0's own fix for installcheck's ordering bug (#83) made installcheck unconditionally depend on install, which writes a real .control file to disk - defeating the entire point of proving a pg_tle deployment never touches the filesystem. There's currently no way to invoke the real pgTAP suite without a filesystem install happening first. The fresh-install pg_tle smoke test (already on master via #16) is unaffected, since it never calls installcheck. Verified locally against PG17: fresh/update x empty/Quoted all pass via make verify-results; bin/compare_fresh_vs_update reports identical definitions for both schema legs. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent f3da5c0 commit 5e27c8a

3 files changed

Lines changed: 134 additions & 0 deletions

File tree

.github/workflows/ci.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -258,6 +258,20 @@ jobs:
258258
run: make install
259259
- name: Update 0.9.6 -> current and run the suite, across every TEST_SCHEMA value
260260
run: make test-update-schema-all
261+
- name: Structurally compare the updated objects against a fresh install, across every TEST_SCHEMA value
262+
# A fixed pgTAP suite only proves the specific behaviors it asserts
263+
# still hold; it can't catch an update script that leaves some
264+
# definition/comment/ACL subtly different from what a fresh install
265+
# of the same version produces. bin/compare_fresh_vs_update installs
266+
# both ways itself (in its own scratch databases) and diffs every
267+
# object the extension owns - any nonempty diff fails the step. Not
268+
# a make target (it's a standalone script, not `make test`), so
269+
# looped directly here rather than via test-schema-all.
270+
run: |
271+
for schema in "" Quoted; do
272+
echo "=== schema=$schema ==="
273+
bin/compare_fresh_vs_update "$schema" 0.9.6 || exit 1
274+
done
261275
262276
# Proves count_nulls survives a BINARY pg_upgrade (in-place catalog
263277
# migration to a newer PostgreSQL major). Installs 0.9.6 on an old
@@ -388,6 +402,15 @@ jobs:
388402
bin/test_existing run-suite count_nulls_upgrade_none ""
389403
bin/test_existing run-suite count_nulls_upgrade_quoted Quoted
390404
405+
# Fresh-install smoke test only, deliberately - NOT extended to the
406+
# update path. pgxntool 2.3.0's fix for installcheck's ordering bug
407+
# (Postgres-Extensions/pgxntool#83) made `installcheck` (and so `make
408+
# test`) unconditionally depend on `install`, which writes a real
409+
# .control file to disk - defeating the entire point of proving a pg_tle
410+
# deployment never touches the filesystem. There's currently no way to
411+
# invoke the real pgTAP suite without that happening first; filed as
412+
# Postgres-Extensions/pgxntool#90. Revisit extending this job to the
413+
# update path once that's resolved.
391414
pg-tle-test:
392415
needs: [changes]
393416
# Skipped outright (not just matrix-reduced like `test` above) on a

bin/compare_fresh_vs_update

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
#!/usr/bin/env bash
2+
#
3+
# compare_fresh_vs_update - structurally compare every object the count_nulls
4+
# extension owns between a FRESH install (CREATE EXTENSION at current) and an
5+
# UPDATED one (CREATE EXTENSION at FROM_VERSION, then ALTER EXTENSION
6+
# UPDATE), in the same schema. A fixed pgTAP expected-output suite (see
7+
# TEST_LOAD_SOURCE in the Makefile) only proves the specific behaviors it
8+
# happens to assert still work - it does not prove an update script left
9+
# object definitions/comments/ACLs BYTE-FOR-BYTE identical to a fresh
10+
# install of the same version. This catches divergence classes a fixed
11+
# suite doesn't already know to test for.
12+
#
13+
# Modeled on the manual technique used in Postgres-Extensions/cat_tools#46,
14+
# which found a real bug this way (a pre-0.2.2 update path left
15+
# `EXECUTE PROCEDURE` hardcoded in a trigger body that fresh installs had
16+
# already updated to `EXECUTE FUNCTION`). Unlike that PR, this is committed,
17+
# reusable tooling rather than a one-off manual diff.
18+
#
19+
# What's compared, per object the extension owns (discovered live via
20+
# pg_depend - see extension_members(), not a hardcoded object list, so a
21+
# newly added function is automatically covered without editing this
22+
# script): pg_get_functiondef() (full definition: schema, args, body,
23+
# volatility, strictness - everything), its comment (obj_description), and
24+
# its ACL (proacl). count_nulls currently ships only functions/triggers, no
25+
# views - the doc this is modeled on also compares pg_get_viewdef/type
26+
# labels/extension membership for extensions that have those; add a query
27+
# for the relevant catalog (pg_class for views, pg_type for types, ...) the
28+
# same way if count_nulls ever grows one.
29+
#
30+
# USAGE: bin/compare_fresh_vs_update [SCHEMA] [FROM_VERSION]
31+
# SCHEMA - schema both installs target (default: unqualified, same
32+
# as TEST_SCHEMA empty - see the Makefile). Both installs
33+
# use the SAME schema, since the point is comparing object
34+
# definitions, not exercising schema-qualification (that's
35+
# TEST_SCHEMA's job in the regular suite).
36+
# FROM_VERSION - the update origin (default: 0.9.6, the oldest version
37+
# count_nulls still ships a full install script for).
38+
#
39+
# Exits nonzero (and prints a real diff) on ANY difference. Scratch
40+
# databases are dropped on exit regardless of outcome.
41+
set -euo pipefail
42+
43+
cd "$(dirname "$(readlink -f "$0")")/.."
44+
45+
schema=${1:-}
46+
from_version=${2:-0.9.6}
47+
48+
fresh_db=compare_fresh_vs_update_fresh
49+
update_db=compare_fresh_vs_update_updated
50+
fresh_snapshot=$(mktemp)
51+
update_snapshot=$(mktemp)
52+
53+
cleanup() {
54+
dropdb --if-exists "$fresh_db"
55+
dropdb --if-exists "$update_db"
56+
rm -f "$fresh_snapshot" "$update_snapshot"
57+
}
58+
trap cleanup EXIT
59+
60+
# extension_members(): every object pg_depend records as owned by the
61+
# count_nulls extension (deptype 'e'), restricted to pg_proc for now (see
62+
# the header comment on extending this). Ordered by name/args so the two
63+
# snapshots line up for a textual diff regardless of OID assignment order,
64+
# which differs between a fresh install and an update.
65+
query() {
66+
cat <<'SQL'
67+
SELECT
68+
'-- ' || p.oid::regprocedure::text || E'\n'
69+
|| pg_get_functiondef(p.oid) || E'\n'
70+
|| '-- comment: ' || coalesce(obj_description(p.oid, 'pg_proc'), '(none)') || E'\n'
71+
|| '-- acl: ' || coalesce(p.proacl::text, '(default)') || E'\n'
72+
FROM pg_depend d
73+
JOIN pg_extension x ON d.refobjid = x.oid AND x.extname = 'count_nulls'
74+
JOIN pg_proc p ON d.objid = p.oid AND d.classid = 'pg_proc'::regclass
75+
WHERE d.deptype = 'e'
76+
ORDER BY p.proname, p.oid::regprocedure::text;
77+
SQL
78+
}
79+
80+
install_in_schema() {
81+
local sql=""
82+
if [ -n "$schema" ]; then
83+
sql="CREATE SCHEMA IF NOT EXISTS \"$schema\"; SET search_path = \"$schema\"; "
84+
fi
85+
echo "$sql"
86+
}
87+
88+
createdb "$fresh_db"
89+
psql -d "$fresh_db" -v ON_ERROR_STOP=1 -c "$(install_in_schema)CREATE EXTENSION count_nulls"
90+
91+
createdb "$update_db"
92+
psql -d "$update_db" -v ON_ERROR_STOP=1 -c "$(install_in_schema)CREATE EXTENSION count_nulls VERSION '$from_version'"
93+
psql -d "$update_db" -v ON_ERROR_STOP=1 -c "SET client_min_messages = WARNING; ALTER EXTENSION count_nulls UPDATE"
94+
95+
psql -d "$fresh_db" -tA -v ON_ERROR_STOP=1 -c "$(query)" > "$fresh_snapshot"
96+
psql -d "$update_db" -tA -v ON_ERROR_STOP=1 -c "$(query)" > "$update_snapshot"
97+
98+
if diff -u "$fresh_snapshot" "$update_snapshot"; then
99+
echo "OK: fresh install and $from_version->current update produce IDENTICAL object definitions/comments/ACLs"
100+
else
101+
echo "FAIL: update path diverges from a fresh install of the same version - see diff above" >&2
102+
exit 1
103+
fi

test/README.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ then invoke via `runtests()`.
2424
`test__*` functions covering function definitions, immutability/
2525
strictness, and behavior across `anyarray`/`json`/`jsonb` and both
2626
trigger functions.
27+
- `../bin/compare_fresh_vs_update` — not part of the pgTAP suite itself: a
28+
standalone script the `extension-update-test` CI job runs after
29+
`TEST_LOAD_SOURCE=update`, which installs fresh and 0.9.6-then-updated
30+
copies of the extension in their own scratch databases and diffs
31+
`pg_get_functiondef`/comments/ACLs for every object the extension owns.
32+
Catches an update script leaving some definition subtly different from a
33+
fresh install, even when the fixed pgTAP suite above still passes (it
34+
only asserts the specific behaviors it happens to check).
2735
- `sql/extension_tests.sql``\i`'s `core/functions.sql`, adds two more
2836
`test__*` functions of its own (`test__check_ncs`, asserting count_nulls
2937
landed where expected; `test__shutdown__drop_all`, asserting it can be

0 commit comments

Comments
 (0)