From b1bc394d2970c39ce5f204f83637e65b00ff2847 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 9 Aug 2026 13:49:11 -0500 Subject: [PATCH 1/2] bin/compare_fresh_vs_update: adapt to the shared random-schema install design Rebuild bin/compare_fresh_vs_update (the structural fresh-vs-update diff tool added on the now-closed phase5/6 branches) directly on master's current random-schema test install mechanism (test/helpers/create_test_schema.sql), replacing its old SCHEMA CLI argument and TEST_SCHEMA-based looping, both now gone from the rest of the codebase. The script always generates ONE randomly named schema up front - same naming convention as create_test_schema.sql: a constant `count_nulls test schema ` prefix (trailing space included, forcing SQL identifier quoting) plus a random suffix - and installs BOTH the fresh and updated copies into that SAME schema, via CREATE SCHEMA + CREATE EXTENSION ... WITH SCHEMA rather than mutating search_path first. A single shared schema is required, not incidental: pg_get_functiondef()'s output is schema-qualified, so two independently named schemas would show a spurious schema-name difference instead of isolating real update-vs-fresh divergence, which is the whole point of the comparison. Each of the two scratch databases also gets a defensive DROP SCHEMA ... CASCADE sweep for any stale `count_nulls test schema %`-prefixed leftover before creating its own, matching create_test_schema.sql's cleanup-before-create convention. ci.yml's `test` job now invokes the script once per run instead of looping it over the old two-value TEST_SCHEMA axis; test/README.md documents it in the suite's file layout. 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 where a fresh install already used `EXECUTE FUNCTION`). Postgres-Extensions/cat_tools#67 proposes generalizing this exact capability into a real, user-facing cat_tools feature rather than test-only plumbing tied to one extension. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 12 +++ bin/compare_fresh_vs_update | 194 ++++++++++++++++++++++++++++++++++++ test/README.md | 11 ++ 3 files changed, 217 insertions(+) create mode 100755 bin/compare_fresh_vs_update diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index aa14bd4..385a2fa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -278,6 +278,18 @@ 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, both landing in + # the SAME randomly generated schema so the diff isolates 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 diff --git a/bin/compare_fresh_vs_update b/bin/compare_fresh_vs_update new file mode 100755 index 0000000..7e272ef --- /dev/null +++ b/bin/compare_fresh_vs_update @@ -0,0 +1,194 @@ +#!/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), in the SAME schema. 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] +# FROM_VERSION - the update origin (default: 0.9.6, the oldest version +# count_nulls still ships a full install script for). +# +# Both installs always target ONE freshly, randomly generated schema (same +# naming convention as test/helpers/create_test_schema.sql: a constant +# `count_nulls test schema ` prefix - trailing space included, so SQL +# identifier quoting is always required - plus a random suffix), generated +# ONCE up front and reused for BOTH scratch databases. That sharing is +# deliberate, not an oversight: the whole point of this script is comparing +# object DEFINITIONS between a fresh install and an updated one, and +# pg_get_functiondef()'s output is schema-qualified, so installing into two +# DIFFERENTLY-named schemas would show a spurious schema-name difference +# instead of isolating real update-vs-fresh divergence. There is therefore +# nothing left for a caller to usefully specify here, unlike +# test/helpers/create_test_schema.sql's per-install independent +# randomization - every comparison this script runs always goes through a +# schema that requires SQL identifier quoting, on every run, rather than +# optionally through an unquoted/default one. create_test_schema.sql itself +# isn't reused to generate it: that file's whole job is producing an +# INDEPENDENT random name per install/session, the opposite of what's +# needed here, so the same short name-generation expression is instead +# computed directly below. +# +# CREATE SCHEMA + CREATE EXTENSION ... WITH SCHEMA is used for both installs +# (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 +# databases are dropped on exit regardless of outcome. +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//\'/\'\'}" + +fresh_db=compare_fresh_vs_update_fresh +update_db=compare_fresh_vs_update_updated +fresh_snapshot=$(mktemp) +update_snapshot=$(mktemp) + +cleanup() { + dropdb --if-exists "$fresh_db" + dropdb --if-exists "$update_db" + rm -f "$fresh_snapshot" "$update_snapshot" +} +trap cleanup EXIT + +# One schema name, shared by both scratch databases - see the header +# comment above on why a single shared name (not two independently +# randomized ones) is required here. Same generation expression as +# test/helpers/create_test_schema.sql, computed once via SQL for +# consistency with it rather than reimplemented in bash. Targets the +# "postgres" maintenance database explicitly (-d), matching every other +# psql call in this script and in bin/test_existing: unlike those calls, +# neither scratch database exists yet at this point, so there's no +# self-created target to connect to - "postgres" is the one database an +# initdb'd cluster is always guaranteed to have. +schema=$(psql -d postgres -tAc "SELECT 'count_nulls test schema ' || substr(md5(random()::text), 1, 12)") + +# 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 built directly in bash instead, 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//\"/\"\"}\"" + +# 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 +} + +# Cleanup-before-create sweep, inlined rather than factored into a shared +# helper file: this script's own two scratch databases are already dropped +# by the cleanup trap above on every exit path, so a leftover schema here +# can only come from a prior run that never reached that trap at all (e.g. +# a killed process) - a narrower case than test/helpers/create_test_schema.sql's, +# which guards a persistent, reused database. Same DO block that file uses +# for the same purpose. +drop_stale_schemas_sql=$(cat <<'SQL' +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 +$$; +SQL +) + +# CREATE EXTENSION count_nulls WITH SCHEMA "$schema"[ VERSION 'x'], into DB, +# after sweeping any stale same-prefixed schema left behind by a prior +# crashed run. All three 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 and CREATE SCHEMA, 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() { + local db=$1 version_clause=$2 + psql -d "$db" -v ON_ERROR_STOP=1 -c " +$drop_stale_schemas_sql +CREATE SCHEMA $schema_ident; +CREATE EXTENSION count_nulls WITH SCHEMA $schema_ident$version_clause; +" +} + +createdb "$fresh_db" +install_extension "$fresh_db" "" + +createdb "$update_db" +install_extension "$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" + +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" + +if diff -u "$fresh_snapshot" "$update_snapshot"; then + echo "OK: fresh install and $from_version->current update produce IDENTICAL object definitions/comments/ACLs" +else + echo "FAIL: update path 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..52b55a2 100644 --- a/test/README.md +++ b/test/README.md @@ -24,6 +24,17 @@ 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 - both landing in the SAME randomly generated schema, so the + diff isolates 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). - `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 From 0365bf9576008ab936afaadd850717077630ba1b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 9 Aug 2026 16:19:36 -0500 Subject: [PATCH 2/2] bin/compare_fresh_vs_update: EXISTING_DB support, wired into pg-upgrade-test Extends bin/compare_fresh_vs_update with an EXISTING_DB argument so it can structurally diff an already-populated database against a fresh install, instead of only ever creating+updating its own scratch database. This lets pg-upgrade-test's twin databases (upgrade_oldest_first/upgrade_current_first, migrated by a real binary pg_upgrade) reuse the same comparison tool the test job's update leg already uses, rather than duplicating that logic. Since EXISTING_DB was installed independently (its own call to test/helpers/create_test_schema.sql, generating its own random schema), the fresh install this script creates for the comparison discovers and reuses that same schema instead of generating a new one - both sides still need to land in the same schema for the comparison to isolate real divergence rather than a spurious schema-name difference. The scratch-vs-scratch mode (no EXISTING_DB) drops schema handling entirely instead of gaining a second randomly-named schema to match: this script's job in that mode is proving fresh-vs-update parity, not exercising schema-qualification or quoting (test/helpers/create_test_schema.sql's job, used by the rest of the suite), so a plain unqualified `CREATE EXTENSION count_nulls` - landing in whatever the session's default resolves to, ordinarily `public` - is exactly as good for comparing definitions, since both installs still land in the SAME place by construction. The stale-schema cleanup sweep goes with it for that mode too: this script's scratch databases are always brand-new, uniquely-named ones created via createdb, which already fails loudly if a stale one exists, an adequate safety net without a schema-level sweep on top of it. install_extension() is therefore now two functions - install_extension_scratch() (no schema at all) and install_extension_matching_schema() (EXISTING_DB mode's own discover-and-match logic, unchanged, just carved into its own function). pg-upgrade-test's CI job now runs this comparison against both twin databases right after run-suite confirms they're at the current version; test/README.md and the test job's own step comment are updated for the new default-schema scratch behavior. Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 30 ++++- bin/compare_fresh_vs_update | 242 +++++++++++++++++++++++------------- test/README.md | 12 +- 3 files changed, 188 insertions(+), 96 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 385a2fa..ca76cbf 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -283,12 +283,13 @@ jobs: # 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, both landing in - # the SAME randomly generated schema so the diff isolates 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. + # 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 @@ -452,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 index 7e272ef..543e87e 100755 --- a/bin/compare_fresh_vs_update +++ b/bin/compare_fresh_vs_update @@ -3,12 +3,13 @@ # 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), in the SAME schema. 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. +# 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 @@ -36,40 +37,65 @@ # 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] +# 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. # -# Both installs always target ONE freshly, randomly generated schema (same -# naming convention as test/helpers/create_test_schema.sql: a constant -# `count_nulls test schema ` prefix - trailing space included, so SQL -# identifier quoting is always required - plus a random suffix), generated -# ONCE up front and reused for BOTH scratch databases. That sharing is -# deliberate, not an oversight: the whole point of this script is comparing -# object DEFINITIONS between a fresh install and an updated one, and -# pg_get_functiondef()'s output is schema-qualified, so installing into two -# DIFFERENTLY-named schemas would show a spurious schema-name difference -# instead of isolating real update-vs-fresh divergence. There is therefore -# nothing left for a caller to usefully specify here, unlike -# test/helpers/create_test_schema.sql's per-install independent -# randomization - every comparison this script runs always goes through a -# schema that requires SQL identifier quoting, on every run, rather than -# optionally through an unquoted/default one. create_test_schema.sql itself -# isn't reused to generate it: that file's whole job is producing an -# INDEPENDENT random name per install/session, the opposite of what's -# needed here, so the same short name-generation expression is instead -# computed directly below. +# 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. # -# CREATE SCHEMA + CREATE EXTENSION ... WITH SCHEMA is used for both installs -# (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. +# 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 -# databases are dropped on exit regardless of outcome. +# 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")")/.." @@ -81,40 +107,66 @@ from_version=${1:-0.9.6} # 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=compare_fresh_vs_update_updated +update_db=${existing_db:-compare_fresh_vs_update_updated} fresh_snapshot=$(mktemp) update_snapshot=$(mktemp) cleanup() { dropdb --if-exists "$fresh_db" - dropdb --if-exists "$update_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 -# One schema name, shared by both scratch databases - see the header -# comment above on why a single shared name (not two independently -# randomized ones) is required here. Same generation expression as -# test/helpers/create_test_schema.sql, computed once via SQL for -# consistency with it rather than reimplemented in bash. Targets the -# "postgres" maintenance database explicitly (-d), matching every other -# psql call in this script and in bin/test_existing: unlike those calls, -# neither scratch database exists yet at this point, so there's no -# self-created target to connect to - "postgres" is the one database an -# initdb'd cluster is always guaranteed to have. -schema=$(psql -d postgres -tAc "SELECT 'count_nulls test schema ' || substr(md5(random()::text), 1, 12)") - # 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 built directly in bash instead, 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//\"/\"\"}\"" +# 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 @@ -136,15 +188,40 @@ ORDER BY p.proname, p.oid::regprocedure::text; SQL } -# Cleanup-before-create sweep, inlined rather than factored into a shared -# helper file: this script's own two scratch databases are already dropped -# by the cleanup trap above on every exit path, so a leftover schema here -# can only come from a prior run that never reached that trap at all (e.g. -# a killed process) - a narrower case than test/helpers/create_test_schema.sql's, -# which guards a persistent, reused database. Same DO block that file uses -# for the same purpose. -drop_stale_schemas_sql=$(cat <<'SQL' -DO $$ +# 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 @@ -152,43 +229,34 @@ BEGIN EXECUTE format('DROP SCHEMA %I CASCADE', r.nspname); END LOOP; END -$$; -SQL -) - -# CREATE EXTENSION count_nulls WITH SCHEMA "$schema"[ VERSION 'x'], into DB, -# after sweeping any stale same-prefixed schema left behind by a prior -# crashed run. All three 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 and CREATE SCHEMA, 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() { - local db=$1 version_clause=$2 - psql -d "$db" -v ON_ERROR_STOP=1 -c " -$drop_stale_schemas_sql +\$\$; CREATE SCHEMA $schema_ident; CREATE EXTENSION count_nulls WITH SCHEMA $schema_ident$version_clause; " } createdb "$fresh_db" -install_extension "$fresh_db" "" +if [ -n "$existing_db" ]; then + install_extension_matching_schema "$fresh_db" "" +else + install_extension_scratch "$fresh_db" "" +fi -createdb "$update_db" -install_extension "$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" +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 $from_version->current update produce IDENTICAL object definitions/comments/ACLs" + echo "OK: fresh install and $source_desc produce IDENTICAL object definitions/comments/ACLs" else - echo "FAIL: update path diverges from a fresh install of the same version - see diff above" >&2 + 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 52b55a2..64219cc 100644 --- a/test/README.md +++ b/test/README.md @@ -28,13 +28,19 @@ then invoke via `runtests()`. 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 - both landing in the SAME randomly generated schema, so the - diff isolates real update-vs-fresh divergence rather than a spurious + 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). + 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