diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa14bd4..ca76cbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -278,6 +278,19 @@ jobs: run: make install - name: Update 0.9.6 -> current and run the suite run: make verify-results TEST_LOAD_SOURCE=update + - name: Structurally compare the updated objects against a fresh install + # A fixed pgTAP suite only proves the specific behaviors it asserts + # still hold; it can't catch an update script that leaves some + # definition/comment/ACL subtly different from what a fresh install + # of the same version produces. bin/compare_fresh_vs_update installs + # both ways itself, in its own scratch databases - each an + # unqualified `CREATE EXTENSION`, so both land in the same default + # schema by construction, which is all the diff needs to isolate + # real update-vs-fresh divergence rather than a schema-name + # difference - and diffs every object the extension owns; any + # nonempty diff fails the step. Not a make target (it's a + # standalone script, not `make test`), so invoked directly here. + run: bin/compare_fresh_vs_update 0.9.6 # Proves count_nulls survives a BINARY pg_upgrade (in-place catalog # migration to a newer PostgreSQL major). Installs 0.9.6 on the oldest @@ -440,6 +453,23 @@ jobs: run: | bin/test_existing run-suite upgrade_oldest_first bin/test_existing run-suite upgrade_current_first + - name: Structurally compare the pg_upgraded databases against a fresh install + # Same rationale as the test job's own update leg's use of this tool + # (see above), but here the "other side" is a REAL database a binary + # pg_upgrade + ALTER EXTENSION UPDATE (in one order or the other) + # just produced, not a scratch database this tool created itself - + # passed as EXISTING_DB so the script queries it in place instead of + # re-deriving it, discovering that database's OWN randomly generated + # schema (each of the twin databases got an independent one from + # bin/test_existing prepare-old) rather than generating a new one, so + # both sides of each comparison still land in the same schema. Once + # per database, since each holds an independent ordering's result. + # Catches a divergence class the fixed pgTAP suite above doesn't: an + # object left subtly different (body, comment, ACL) by surviving a + # real catalog migration, as opposed to only an in-place update. + run: | + bin/compare_fresh_vs_update 0.9.6 upgrade_oldest_first + bin/compare_fresh_vs_update 0.9.6 upgrade_current_first pg-tle-test: needs: [changes] diff --git a/bin/compare_fresh_vs_update b/bin/compare_fresh_vs_update new file mode 100755 index 0000000..543e87e --- /dev/null +++ b/bin/compare_fresh_vs_update @@ -0,0 +1,262 @@ +#!/usr/bin/env bash +# +# compare_fresh_vs_update - structurally compare every object the count_nulls +# extension owns between a FRESH install (CREATE EXTENSION at current) and an +# UPDATED one (CREATE EXTENSION at FROM_VERSION, then ALTER EXTENSION +# UPDATE) - or, with EXISTING_DB, an already-populated database however it +# was produced. A fixed pgTAP expected-output suite (see TEST_LOAD_SOURCE in +# the Makefile) only proves the specific behaviors it happens to assert +# still work - it does not prove an update script left object +# definitions/comments/ACLs BYTE-FOR-BYTE identical to a fresh install of +# the same version. This catches divergence classes a fixed suite doesn't +# already know to test for. +# +# Modeled on the manual technique used in Postgres-Extensions/cat_tools#46, +# which found a real bug this way (a pre-0.2.2 update path left +# `EXECUTE PROCEDURE` hardcoded in a trigger body that fresh installs had +# already updated to `EXECUTE FUNCTION`). Unlike that PR, this is committed, +# reusable tooling rather than a one-off manual diff. +# +# Postgres-Extensions/cat_tools#67 proposes generalizing this exact +# capability - "list every object an extension owns, with each object's +# actual definition" - into a real, user-facing cat_tools feature, not +# test-only plumbing tied to one extension's update path. If that lands, +# this script's object-discovery-and-rendering logic (query() below, and +# the per-kind comparisons) is a natural candidate to +# reimplement on top of it rather than staying a bespoke, +# count_nulls-specific copy. +# +# What's compared, per object the extension owns (discovered live via +# pg_depend - see query(), not a hardcoded object list, so a +# newly added function is automatically covered without editing this +# script): pg_get_functiondef() (full definition: schema, args, body, +# volatility, strictness - everything), its comment (obj_description), and +# its ACL (proacl). count_nulls currently ships only functions/triggers, no +# views - the doc this is modeled on also compares pg_get_viewdef/type +# labels/extension membership for extensions that have those; add a query +# for the relevant catalog (pg_class for views, pg_type for types, ...) the +# same way if count_nulls ever grows one. +# +# USAGE: bin/compare_fresh_vs_update [FROM_VERSION] [EXISTING_DB] +# FROM_VERSION - the update origin (default: 0.9.6, the oldest version +# count_nulls still ships a full install script for). +# Ignored when EXISTING_DB is given - that database's +# history is whatever already produced it. +# EXISTING_DB - compare a fresh install against this ALREADY-POPULATED +# database instead of creating+updating a scratch one +# (default: none - create our own scratch "updated" +# database, as described below). Lets callers that +# produced their update/upgrade result some other way - +# e.g. bin/test_existing's real binary pg_upgrade path via +# the pg-upgrade-test CI job - reuse this same comparison +# without this script re-deriving that database itself. +# The caller owns EXISTING_DB's lifecycle: it is never +# created, updated, or dropped here, only queried. Because +# EXISTING_DB was installed independently (via +# test/helpers/create_test_schema.sql, generating its own +# random schema), the schema used for this run's fresh +# install is DISCOVERED from EXISTING_DB itself instead of +# freshly generated - same lookup +# test/helpers/find_test_schema.sql uses, inlined here for +# the same reason this script shells out to psql per +# statement rather than `\i`-ing psql helper files - so +# both sides of the comparison still land in the SAME +# schema. +# +# Absent EXISTING_DB (the ordinary "scratch-vs-scratch" mode), this script's +# only job is proving fresh-vs-update PARITY - do the two installs produce +# identical object definitions/comments/ACLs - not exercising schema +# qualification or quoting (that's test/helpers/create_test_schema.sql's +# job, used by the rest of the suite). Both installs are therefore a plain, +# unqualified `CREATE EXTENSION count_nulls`, landing wherever the +# session's default search_path resolves (ordinarily `public`) rather than +# a randomly generated schema: both installs still land in the SAME place +# by construction (both default the same way), which is all +# pg_get_functiondef()'s schema-qualified output needs to avoid a spurious +# schema-name difference. No schema is created, and no stale-schema sweep +# runs, in this mode: this script always creates brand-new scratch +# databases via createdb, which fails loudly if a same-named one already +# exists - an already-adequate safety net - and drops them on exit, so +# there's no cross-run schema staleness path the way there is for the main +# suite's persistently-reused test database. +# +# EXISTING_DB mode is different: that database's schema was chosen by +# whatever created it (test/helpers/create_test_schema.sql, via +# bin/test_existing's prepare-old) - this script has no say in that and +# must discover and reuse that exact schema for its own fresh-install side +# of the comparison, or the diff would show a spurious schema-name +# difference instead of isolating real divergence. CREATE SCHEMA + CREATE +# EXTENSION ... WITH SCHEMA is used for that fresh install (never `SET +# search_path` beforehand): mutating search_path ahead of CREATE EXTENSION +# would let an install succeed via a coincidentally arranged search_path, +# masking the install script secretly depending on unqualified name +# resolution during install - see test/helpers/create_test_schema.sql's +# header comment for the fuller version of this reasoning. +# +# Exits nonzero (and prints a real diff) on ANY difference. Scratch +# database(s) this script created itself are dropped on exit regardless of +# outcome; an EXISTING_DB passed in is left untouched. +set -euo pipefail + +cd "$(dirname "$(readlink -f "$0")")/.." + +from_version=${1:-0.9.6} +# Doubling any embedded single quote before it goes into a SQL string +# literal below (VERSION '...') - CI always passes the fixed literal +# 0.9.6, but the script documents FROM_VERSION as a general argument, so a +# future caller-supplied value containing a quote shouldn't be able to +# break out of the literal. +from_version_lit="${from_version//\'/\'\'}" +existing_db=${2:-} + +fresh_db=compare_fresh_vs_update_fresh +update_db=${existing_db:-compare_fresh_vs_update_updated} +fresh_snapshot=$(mktemp) +update_snapshot=$(mktemp) + +cleanup() { + dropdb --if-exists "$fresh_db" + # Only drop update_db if we created it ourselves - an EXISTING_DB belongs + # to the caller (e.g. the real pg_upgraded database bin/test_existing is + # still using) and must survive this script running. + if [ -z "$existing_db" ]; then + dropdb --if-exists "$update_db" + fi + rm -f "$fresh_snapshot" "$update_snapshot" +} +trap cleanup EXIT + +# psql only interpolates :"var"-style variables when reading a script (-f +# or stdin), NOT inside a -c command string - confirmed directly (a bare +# :"schema" inside a -c string reaches the server un-substituted and is a +# syntax error). Every use below is therefore a plain double-quoted SQL +# identifier/literal built directly in bash instead. + +if [ -n "$existing_db" ]; then + # Discover the schema EXISTING_DB's count_nulls actually lives in - same + # validation test/helpers/find_test_schema.sql performs (hard failure, + # not a pgTAP-style assertion, on anything but exactly one match), run + # against EXISTING_DB instead of `\i`'d in a shared session. Two separate + # invocations, not one: a DO block's own "DO" command-completion tag + # prints to stdout even under -tA (confirmed directly - unlike a RAISE + # NOTICE, which goes to stderr), so combining the validation and the + # SELECT in one invocation would capture "DO" as a spurious first line + # of $schema. psql also does NOT interpolate variables inside + # dollar-quoted strings (see test/helpers/create_test_schema.sql's header + # comment for the fuller explanation), so this can't embed $existing_db + # in the RAISE EXCEPTION text below - the caller already knows which + # EXISTING_DB it passed. + psql -d "$existing_db" -v ON_ERROR_STOP=1 -c " +DO \$\$ +DECLARE + v_count int := (SELECT count(*) FROM pg_namespace WHERE nspname LIKE 'count_nulls test schema %'); +BEGIN + IF v_count <> 1 THEN + RAISE EXCEPTION + 'expected exactly one schema matching ''count_nulls test schema %%'', found %' + , v_count; + END IF; +END +\$\$;" >/dev/null + schema=$(psql -d "$existing_db" -tAc "SELECT nspname FROM pg_namespace WHERE nspname LIKE 'count_nulls test schema %'") + + # Every use below is a plain double-quoted SQL identifier built directly + # in bash, doubling any embedded double quote the SQL spec's own + # identifier-quoting rule requires (not expected in the generated name + # itself, which is prefix+hex, but this keeps the quoting correct + # regardless). + schema_ident="\"${schema//\"/\"\"}\"" +fi + +# query(): every object pg_depend records as owned by the +# count_nulls extension (deptype 'e'), restricted to pg_proc for now (see +# the header comment on extending this). Ordered by name/args so the two +# snapshots line up for a textual diff regardless of OID assignment order, +# which differs between a fresh install and an update. +query() { + cat <<'SQL' +SELECT + '-- ' || p.oid::regprocedure::text || E'\n' + || pg_get_functiondef(p.oid) || E'\n' + || '-- comment: ' || coalesce(obj_description(p.oid, 'pg_proc'), '(none)') || E'\n' + || '-- acl: ' || coalesce(p.proacl::text, '(default)') || E'\n' +FROM pg_depend d +JOIN pg_extension x ON d.refobjid = x.oid AND x.extname = 'count_nulls' +JOIN pg_proc p ON d.objid = p.oid AND d.classid = 'pg_proc'::regclass +WHERE d.deptype = 'e' +ORDER BY p.proname, p.oid::regprocedure::text; +SQL +} + +# install_extension_scratch(): scratch-vs-scratch mode only (see the header +# comment above) - a plain, unqualified CREATE EXTENSION, landing wherever +# the session's default search_path resolves. No schema is created and no +# stale-schema sweep runs: this script's own createdb call above already +# guarantees $db is a genuinely fresh, empty database (createdb fails +# loudly if a same-named one already exists), so there's nothing left for +# a schema-level cleanup to guard against here. +install_extension_scratch() { + local db=$1 version_clause=$2 + psql -d "$db" -v ON_ERROR_STOP=1 -c "CREATE EXTENSION count_nulls$version_clause;" +} + +# install_extension_matching_schema(): EXISTING_DB mode only - installs +# count_nulls into $db (always the fresh_db this script created itself, +# never EXISTING_DB) under the schema discovered above from EXISTING_DB, +# after sweeping any stale same-prefixed schema left behind by a prior +# crashed run of this script (a narrower case than +# test/helpers/create_test_schema.sql's own sweep, which guards a +# persistent, reused database: this script's own scratch databases are +# already dropped by the cleanup trap above on every exit path, so a +# leftover schema here can only come from a run that never reached that +# trap at all, e.g. a killed process). Both statements go through a SINGLE +# -c, not one -c each: multiple -c/-f options in one psql invocation only +# became supported in PostgreSQL 10 (psql before that silently ran only the +# LAST one given) - confirmed the hard way against this repo's own oldest +# supported major (9.4), where splitting these into separate -c options +# silently dropped the cleanup DO block, leaving CREATE EXTENSION's WITH +# SCHEMA targeting a schema that was never created ("ERROR: schema ... does +# not exist"). A single -c string, semicolon-separated, has always run +# every statement it contains, on every supported version. +install_extension_matching_schema() { + local db=$1 version_clause=$2 + psql -d "$db" -v ON_ERROR_STOP=1 -c " +DO \$\$ +DECLARE + r record; +BEGIN + FOR r IN SELECT nspname FROM pg_namespace WHERE nspname LIKE 'count_nulls test schema %' LOOP + EXECUTE format('DROP SCHEMA %I CASCADE', r.nspname); + END LOOP; +END +\$\$; +CREATE SCHEMA $schema_ident; +CREATE EXTENSION count_nulls WITH SCHEMA $schema_ident$version_clause; +" +} + +createdb "$fresh_db" +if [ -n "$existing_db" ]; then + install_extension_matching_schema "$fresh_db" "" +else + install_extension_scratch "$fresh_db" "" +fi + +if [ -z "$existing_db" ]; then + createdb "$update_db" + install_extension_scratch "$update_db" " VERSION '$from_version_lit'" + psql -d "$update_db" -v ON_ERROR_STOP=1 -c "SET client_min_messages = WARNING; ALTER EXTENSION count_nulls UPDATE" +fi + +psql -d "$fresh_db" -tA -v ON_ERROR_STOP=1 -c "$(query)" > "$fresh_snapshot" +psql -d "$update_db" -tA -v ON_ERROR_STOP=1 -c "$(query)" > "$update_snapshot" + +source_desc="$from_version->current update" +[ -n "$existing_db" ] && source_desc="'$existing_db'" + +if diff -u "$fresh_snapshot" "$update_snapshot"; then + echo "OK: fresh install and $source_desc produce IDENTICAL object definitions/comments/ACLs" +else + echo "FAIL: $source_desc diverges from a fresh install of the same version - see diff above" >&2 + exit 1 +fi diff --git a/test/README.md b/test/README.md index b3b58e3..64219cc 100644 --- a/test/README.md +++ b/test/README.md @@ -24,6 +24,23 @@ then invoke via `runtests()`. `test__*` functions covering function definitions, immutability/ strictness, and behavior across `anyarray`/`json`/`jsonb` and both trigger functions. +- `../bin/compare_fresh_vs_update` — not part of the pgTAP suite itself: a + standalone script the `test` CI job's update leg runs after + `TEST_LOAD_SOURCE=update`, which installs a fresh copy and a + 0.9.6-then-updated copy of the extension into their own scratch + databases - each an unqualified `CREATE EXTENSION`, so both land in the + same default schema by construction, which is all the diff needs to + isolate real update-vs-fresh divergence rather than a spurious + schema-name difference - and diffs `pg_get_functiondef`/comments/ACLs for + every object the extension owns. Catches an update script leaving some + definition subtly different from a fresh install, even when the fixed + pgTAP suite above still passes (it only asserts the specific behaviors it + happens to check). The `pg-upgrade-test` CI job also reuses it (via its + optional `EXISTING_DB` argument) to compare a fresh install against the + real, already-populated databases a binary `pg_upgrade` just produced, + discovering and matching that database's own randomly generated schema + (from `helpers/create_test_schema.sql`, via `bin/test_existing + prepare-old`) instead of generating a new one. - `sql/extension_tests.sql` — `\i`'s `core/functions.sql`, adds two more `test__*` functions of its own (`test__check_ncs`, asserting count_nulls landed where expected; `test__shutdown__drop_all`, asserting it can be