From 48d42585cc34c1ed378335ed876521fd34f19326 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 20:03:22 -0500 Subject: [PATCH 01/13] Add update+upgrade (U&U) test infrastructure: 0.1.0->stable update script, test/install/load.sql three-mode loader, dependency-guard anchor, and a permanent schema-qualification pgTAP assertion Implements advanced-extension-testing.md checklist items 1-6 on top of PR #17 (reconcile-object-functions): - PGXNTOOL_ENABLE_TEST_INSTALL / PGXNTOOL_ENABLE_VERIFY_RESULTS set explicitly; TEST_LOAD_SOURCE (fresh/update/existing) + TEST_UPDATE_FROM/TO make vars, parse-time validated, propagated as placeholder GUCs via PGOPTIONS; `make test-update` wrapper. - test/install/load.sql: single committed-once installer for the extension, covering all three load modes, including a drop-first reset (with pg_temp.drop_role() for the extension's own global roles) and an existing-mode presence/version assertion. - sql/object_reference--0.1.0--stable.sql: hand-authored update script (there was previously no update path at all from the only real historical release to current). Recreates every function/view that changed via the same private-helper-schema bootstrap/teardown convention the fresh install uses, so the update path is verified byte-for-byte structurally identical to a fresh install (function bodies, comments, ACLs, table/view columns). - Makefile: DATA += sql/object_reference--0.1.0.sql (pgxntool#48 workaround, needed for CREATE EXTENSION ... VERSION '0.1.0' to work at all); a conditional count_nulls install step for the update-mode floor only (0.1.0's own install script still needs it, even though current object_reference no longer requires it). - test/finish.sql: one permanent pgTAP assertion (modeled on pg_count_nulls'/extension_tools' own schema-qualification checks) proving object_reference/_object_reference are never resolved via search_path. - Moved the pre-existing raw-source-load sanity check (test/sql/zzz_build.sql) to test/build/, pgxntool's own test-build feature: it needs a schema-free database to create `object_reference` manually in, which the committed-once installer above no longer provides in the shared main-suite database. Dependency-guard anchor for a future existing-mode CI job: a view typed on _object_reference.object's row type (object_reference-owned, never dropped or redefined by the update script) blocks a non-CASCADE DROP EXTENSION; manually proven to block the drop (and to keep blocking it after the update path) as part of this PR's own verification, not committed as CI machinery yet. Co-Authored-By: Claude Sonnet 5 --- Makefile | 80 +++ sql/object_reference--0.1.0--stable.sql | 738 ++++++++++++++++++++++++ test/{ => build}/expected/zzz_build.out | 0 test/{sql => build}/zzz_build.sql | 0 test/expected/_object_v.out | 3 +- test/expected/all.out | 3 +- test/expected/base.out | 3 +- test/expected/capture.out | 3 +- test/expected/event_trigger.out | 3 +- test/finish.sql | 26 + test/install/.gitignore | 6 + test/install/load.sql | 194 +++++++ test/load.sql | 12 +- test/sql/_object_v.sql | 3 +- test/sql/all.sql | 3 +- test/sql/base.sql | 3 +- test/sql/capture.sql | 3 +- test/sql/event_trigger.sql | 3 +- test/sql/object_group.sql | 3 +- 19 files changed, 1076 insertions(+), 13 deletions(-) create mode 100644 sql/object_reference--0.1.0--stable.sql rename test/{ => build}/expected/zzz_build.out (100%) rename test/{sql => build}/zzz_build.sql (100%) create mode 100644 test/finish.sql create mode 100644 test/install/.gitignore create mode 100644 test/install/load.sql diff --git a/Makefile b/Makefile index 0b7fd85..73316b3 100644 --- a/Makefile +++ b/Makefile @@ -1,3 +1,62 @@ +# Committed-once install of the extension (test/install/load.sql), run before +# the main pgTAP suite in its own pg_regress session so its state persists +# (committed) into every per-test file. Must be set (and set to exactly +# "yes"/"no", not auto-detected) BEFORE base.mk is included below, since +# base.mk reads it at parse time. +PGXNTOOL_ENABLE_TEST_INSTALL = yes + +# Safeguard for `make results`: refuses to copy test/results/*.out over +# test/expected/*.out while a real regression is showing. This is already +# pgxntool's own default, but set it explicitly so that stays true even if a +# future pgxntool default ever changes. +PGXNTOOL_ENABLE_VERIFY_RESULTS = yes + +# TEST_LOAD_SOURCE selects how test/install/load.sql installs the extension: +# - fresh (default): CREATE EXTENSION object_reference (current version). +# - update: CREATE EXTENSION at TEST_UPDATE_FROM (default 0.1.0, the only +# real historical PGXN release) then ALTER EXTENSION UPDATE -- to +# TEST_UPDATE_TO if set, otherwise to the current default_version +# ("stable"). Running the SAME suite with the SAME expected output +# against the updated database verifies it behaves identically to a +# fresh install. +# - existing: the extension is ALREADY installed in the target database (by +# a binary pg_upgrade, or an ALTER EXTENSION UPDATE done outside the +# suite). load.sql does not touch it; it only asserts presence + current +# version. Pair with CONTRIB_TESTDB= and +# EXTRA_REGRESS_OPTS=--use-existing so pg_regress runs against that +# database instead of dropping and recreating a throwaway one. +# +# The mode (and the update from/to versions) are signalled to load.sql via +# placeholder GUCs. pg_regress does not forward make variables, but the psql +# processes it spawns inherit the environment, so PGOPTIONS reaches load.sql. +# +# The GUCs are exported UNCONDITIONALLY, so load.sql can read them WITHOUT +# missing_ok and fail loudly if they did not propagate. Relying on an absent +# GUC to mean "fresh" is unsafe: a silent break anywhere in the +# make -> PGOPTIONS -> env -> psql chain would quietly run the wrong mode. +# +# TEST_LOAD_SOURCE must be exactly `fresh`, `update` or `existing`; anything +# else is a hard error at parse time (so e.g. `make test +# TEST_LOAD_SOURCE=typo` fails fast rather than defaulting). +TEST_LOAD_SOURCE ?= fresh +ifeq ($(filter $(TEST_LOAD_SOURCE),fresh update existing),) +$(error TEST_LOAD_SOURCE must be 'fresh', 'update' or 'existing', got '$(TEST_LOAD_SOURCE)') +endif + +# update-mode version range (read by load.sql only in update mode). Empty +# TEST_UPDATE_TO means "update to the current default_version" (stable). +TEST_UPDATE_FROM ?= 0.1.0 +TEST_UPDATE_TO ?= + +export PGOPTIONS := $(PGOPTIONS) -c object_reference.test_load_mode=$(TEST_LOAD_SOURCE) -c object_reference.test_update_from=$(TEST_UPDATE_FROM) -c object_reference.test_update_to=$(TEST_UPDATE_TO) + +# Convenience wrapper: `make test-update` == `make test TEST_LOAD_SOURCE=update`. +# Must recurse (a fresh $(MAKE)) rather than depend on `test`, so the +# parse-time TEST_LOAD_SOURCE conditional above re-evaluates with update set. +.PHONY: test-update +test-update: + $(MAKE) test TEST_LOAD_SOURCE=update + include pgxntool/base.mk testdeps: $(wildcard test/*.sql test/helpers/*.sql) # Be careful not to include directories in this @@ -5,6 +64,27 @@ testdeps: test_factory install: cat_tools +# 0.1.0 (TEST_UPDATE_FROM's default -- the update-mode floor, see above) needs +# count_nulls too: its install script's _object_oid.null_count trigger calls +# count_nulls' not_null_count_trigger(), and object_reference.control's +# `requires` (cat_tools only -- count_nulls was dropped once the reg* +# pseudotype removal made that trigger unnecessary) no longer CASCADEs it in. +# Only needed for update-mode testing against that floor -- current +# object_reference has no runtime dependency on count_nulls at all -- so this +# is conditional, not folded into the unconditional `install: cat_tools` above. +ifeq ($(TEST_LOAD_SOURCE),update) +install: count_nulls +endif + +.PHONY: count_nulls +count_nulls: $(DESTDIR)$(datadir)/extension/count_nulls.control +$(DESTDIR)$(datadir)/extension/count_nulls.control: + pgxn install count_nulls + +# Clean the cruft pg_regress writes into test/install/ (the self-comparing +# result .out and its diff), which is listed in test/install/.gitignore. +extra_clean += $(addprefix test/install/,$(shell grep -v '^\#' test/install/.gitignore 2>/dev/null)) + test: dump_test extra_clean += $(wildcard test/dump/*.log) dump_test: test/dump/run.sh test/helpers/object_table.sql $(wildcard test/dump/*.sql) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql new file mode 100644 index 0000000..515eae6 --- /dev/null +++ b/sql/object_reference--0.1.0--stable.sql @@ -0,0 +1,738 @@ +/* + * Hand-authored update script: 0.1.0 (the only real historical PGXN release) + * -> stable (the current build). Never auto-generated and never touched by + * the control.mk rule that (re)builds sql/object_reference--stable.sql from + * sql/object_reference.sql -- see this repo's CLAUDE.md / memory: versioned + * SQL files are frozen, only sql/object_reference.sql (the source) is edited. + * + * Every delta below was found by diffing sql/object_reference--0.1.0.sql + * against sql/object_reference.sql in full (not by memory of the individual + * commits that produced them): + * - count_nulls dropped as a dependency (object_reference.control's + * `requires` no longer lists it); the _object_oid.null_count trigger, + * which called count_nulls' not_null_count_trigger(), is removed. + * - The reg* pseudotype columns on _object_reference._object_oid + * (regclass/regconfig/regdictionary/regnamespace/regoperator/ + * regprocedure/regtype) are removed in favor of a single plain `oid` + * column (object_oid), and classid changes from regclass to oid. + * - _object_reference._object_v / _object_v__for_update (views) drop the + * now-gone reg* columns. CREATE OR REPLACE VIEW cannot drop columns, so + * both are DROP+CREATE'd, along with the two functions whose RETURNS + * type is _object_reference._object_v (a formal pg_depend edge, not just + * a body reference) -- see the "Views + dependent functions" section + * below. + * - New functions: object_reference.object__describe(), + * object__identity(), object__cleanup(), plus a trigger that calls the + * latter to auto-clean orphaned objects when removed from a group. + * - object__getsert's underlying _object_v__for_update() now refuses to + * track objects in temporary schemas (pg_temp%/pg_toast_temp%). + * - object_reference.unsupported() additionally excludes "partitioned + * table"/"partitioned index" (pg_get_object_address() doesn't recognize + * them). + * - _tg_capture_safety() gains a trailing RETURN NULL (a trigger function + * with a declared return type must return something in every branch). + * - Two new utility event-trigger functions, etg_raise__start/__drop, are + * added (not wired to any CREATE EVENT TRIGGER -- example/debug use + * only, matching the commented-out "snitch" example 0.1.0 had instead). + * - _object_oid__add's own bug: 0.1.0 selected `a.subobjid` from + * pg_get_object_address(), which only ever has an `objsubid` column + * (SQLSTATE 42703 if that branch were ever hit) -- fixed here as part of + * recreating the function with its current body. + * + * Uses the same private-helper-schema bootstrap/teardown convention as + * sql/object_reference.sql's own fresh install (__object_reference.exec / + * safe_dump / create_function), so every function recreated here goes + * through the exact same REVOKE ALL FROM PUBLIC / GRANT / COMMENT template a + * fresh install uses -- not hand-written DROP FUNCTION + CREATE FUNCTION + + * REVOKE/GRANT/COMMENT, which would risk silently diverging from what a + * fresh install actually produces (a real, verified-clean structural + * comparison of every recreated function/view against a fresh install of the + * current version backs this file -- see the containing PR's description). + */ +SET client_min_messages = WARNING; + +CREATE SCHEMA __object_reference; + +CREATE FUNCTION __object_reference.exec( + sql text +) RETURNS void LANGUAGE plpgsql AS $body$ +BEGIN + RAISE DEBUG 'sql = %', sql; + EXECUTE sql; +END +$body$; + +CREATE FUNCTION __object_reference.safe_dump( + relation regclass + , filter text DEFAULT '' +) RETURNS void LANGUAGE plpgsql AS $body$ +BEGIN + PERFORM pg_catalog.pg_extension_config_dump(relation, filter); +EXCEPTION WHEN feature_not_supported THEN + RAISE WARNING 'I promise you will be sorry if you try to use this as anything other than an extension!'; +END +$body$; + +CREATE FUNCTION __object_reference.create_function( + function_name text + , args text + , options text + , body text + , comment text + , grants text DEFAULT NULL +) RETURNS void LANGUAGE plpgsql AS $body$ +DECLARE + c_clean_args text := cat_tools.routine__parse_arg_types_text(args); + + create_template CONSTANT text := $template$ +CREATE OR REPLACE FUNCTION %s( +%s +) RETURNS %s AS +%L +$template$ + ; + + revoke_template CONSTANT text := $template$ +REVOKE ALL ON FUNCTION %s( +%s +) FROM public; +$template$ + ; + + grant_template CONSTANT text := $template$ +GRANT EXECUTE ON FUNCTION %s( +%s +) TO %s; +$template$ + ; + + comment_template CONSTANT text := $template$ +COMMENT ON FUNCTION %s( +%s +) IS %L; +$template$ + ; + +BEGIN + PERFORM __object_reference.exec( format( + create_template + , function_name + , args + , options + , body + ) ) + ; + PERFORM __object_reference.exec( format( + revoke_template + , function_name + , c_clean_args + ) ) + ; + + IF grants IS NOT NULL THEN + PERFORM __object_reference.exec( format( + grant_template + , function_name + , c_clean_args + , grants + ) ) + ; + END IF; + + IF comment IS NOT NULL THEN + PERFORM __object_reference.exec( format( + comment_template + , function_name + , c_clean_args + , comment + ) ) + ; + END IF; +END +$body$; + +/* + * 0.1.0 already installed this extension's own event triggers (they fire on + * every sql_drop / ddl_command_end in the session, not just DDL a normal user + * issues), and they stay active for the rest of THIS session while the + * structural changes below run. zzz__object_reference_drop in particular + * queries _object_reference._object_v inside its own body, so it would fire + * -- and error, since the view is momentarily gone -- the instant this script + * drops that view a few statements down. Disable all three for the structural + * portion of this script and re-enable them right before the private helper + * schema teardown, once every object they might touch exists again in its + * final, current-source shape. A fresh install never hits this: it creates + * these event triggers only at the very end, once nothing they reference is + * still being modified. + */ +ALTER EVENT TRIGGER zzz__object_reference_drop DISABLE; +ALTER EVENT TRIGGER zzz_object_reference__fix_identity DISABLE; +ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; + +/* + * _object_reference.object: no column changes, just a missing + * extension_config_dump marking on its sequence (added alongside the table + * itself in the current source; 0.1.0 only marked the table). + */ +SELECT __object_reference.safe_dump('_object_reference.object_object_id_seq'); + +/* + * Views + dependent functions -- dropped BEFORE the _object_oid table + * alterations below, not after: _object_v / _object_v__for_update (views) + * both SELECT the reg* columns directly, so the columns can't be dropped out + * from under them first. CREATE OR REPLACE VIEW also cannot remove columns, + * so both views must be DROP+CREATE'd regardless. _object_oid__add() and the + * _object_v__for_update() FUNCTION (a + * distinct catalog object from the view of the same name -- Postgres allows + * a relation and a function to share a name, since they live in pg_class and + * pg_proc respectively) both RETURN _object_reference._object_v, which is a + * formal pg_depend edge (not just a body reference), so a non-CASCADE DROP + * VIEW would fail with them still around: they must be dropped first, in + * this order, then the table altered, then the views recreated, then the + * functions recreated (via create_function, which uses CREATE OR REPLACE -- + * fine here since neither currently exists). + * + * _object_oid__add's own signature also changes (classid regclass -> oid, + * following the table alteration below), which on its own would require a + * DROP FUNCTION before a same-named CREATE regardless of the view: CREATE OR + * REPLACE FUNCTION with different parameter types creates a new, distinct + * overload rather than replacing the old one, leaving the wrong-typed + * original behind. + */ +DROP FUNCTION _object_reference._object_oid__add(int, cat_tools.object_type, regclass, oid, int); +DROP FUNCTION _object_reference._object_v__for_update(cat_tools.object_type, oid, int, int, regclass); +DROP VIEW _object_reference._object_v__for_update; +DROP VIEW _object_reference._object_v; + +/* + * _object_reference._object_oid: drop the reg* pseudotype columns and the + * count_nulls-backed trigger that enforced "exactly one is set", in favor of + * a single NOT NULL object_oid column. Order below is fully explicit + * (constraints/indexes/trigger dropped by name, not left to an implicit + * CASCADE) so nothing is silently dropped alongside a `DROP COLUMN` we did + * not ask for. + */ +ALTER TABLE _object_reference._object_oid + DROP CONSTRAINT regclass_classid + , DROP CONSTRAINT regconfig_classid + , DROP CONSTRAINT regdictionary_classid + , DROP CONSTRAINT regnamespace_classid + , DROP CONSTRAINT regoperator_classid + , DROP CONSTRAINT regprocedure_classid + , DROP CONSTRAINT regtype_classid + , DROP CONSTRAINT objid_must_match +; + +DROP TRIGGER null_count ON _object_reference._object_oid; + +DROP INDEX _object_reference._object_oid__u_regclass; +DROP INDEX _object_reference._object_oid__u_regconfig; +DROP INDEX _object_reference._object_oid__u_regdictionary; +DROP INDEX _object_reference._object_oid__u_regoperator; +DROP INDEX _object_reference._object_oid__u_regprocedure; +DROP INDEX _object_reference._object_oid__u_regtype; + +/* + * Backfill: 0.1.0 only ever populated ONE of {regclass, ..., regtype, + * object_oid} per row (whichever reg* type applied; object_oid itself only + * when none did). objid was always kept equal to that same value (that's + * exactly what the old objid_must_match CHECK enforced), so copying objid + * into object_oid for every row is correct regardless of which reg* column + * used to carry it, and is a no-op where object_oid already matched. + */ +UPDATE _object_reference._object_oid SET object_oid = objid WHERE object_oid IS NULL; + +ALTER TABLE _object_reference._object_oid + DROP COLUMN regclass + , DROP COLUMN regconfig + , DROP COLUMN regdictionary + , DROP COLUMN regnamespace + , DROP COLUMN regoperator + , DROP COLUMN regprocedure + , DROP COLUMN regtype + , ALTER COLUMN classid TYPE oid USING classid::oid + , ALTER COLUMN object_oid SET NOT NULL + , ADD CONSTRAINT objid_must_match CHECK ( objid IS NOT DISTINCT FROM object_oid ) -- _object_reference._sanity() depends on this! +; + +CREATE VIEW _object_reference._object_v AS + SELECT + o.object_id + , o.object_type + , o.object_names + , o.object_args + , i.classid + , i.objid + , i.objsubid + , i.object_oid + , s.* + FROM _object_reference.object o + LEFT JOIN _object_reference._object_oid i USING(object_id) + , _object_reference._sanity(o, i) s +; +CREATE VIEW _object_reference._object_v__for_update AS + SELECT + o.object_id + , o.object_type + , o.object_names + , o.object_args + , i.classid + , i.objid + , i.objsubid + , i.object_oid + , s.* + FROM _object_reference.object o + LEFT JOIN _object_reference._object_oid i USING(object_id) + , _object_reference._sanity(o, i) s + FOR UPDATE OF o +; + +SELECT __object_reference.create_function( + '_object_reference._object_v__for_update' + , $args$ + object_type _object_reference.object.object_type%TYPE + , objid _object_reference._object_oid.objid%TYPE + , objsubid _object_reference._object_oid.objsubid%TYPE + , object_group_id int DEFAULT NULL + , class_id regclass DEFAULT NULL +$args$ + , '_object_reference._object_v LANGUAGE plpgsql' + , $body$ +DECLARE + c_classid CONSTANT regclass := cat_tools.object__address_classid(object_type); + + r_object_v _object_reference._object_v; + r_address record; + r_identity record; + + did_insert boolean := false; + + i smallint; + sql text; +BEGIN + ASSERT class_id IS NULL OR class_id = c_classid, format( + 'cat_tools.object__address_classid(object_type) %L <> class_id %L' + , c_classid + , class_id + ); + IF object_reference.unsupported(object_type) THEN + RAISE 'object_type % is not supported', object_type; + END IF; + + SELECT INTO r_address * FROM pg_catalog.pg_identify_object_as_address(c_classid, objid, objsubid); + + IF r_address IS NULL THEN + RAISE 'unable to find object' + USING DETAIL = format( + 'pg_identify_object_as_address(%s, %s, %s) returned NULL' + , c_classid + , objid + , objsubid + ) + ; + END IF; + + -- Refuse to track objects in temporary schemas + SELECT INTO r_identity * FROM pg_catalog.pg_identify_object(c_classid, objid, objsubid); + IF r_identity.schema IS NOT NULL AND (r_identity.schema LIKE 'pg_temp%' OR r_identity.schema LIKE 'pg_toast_temp%') THEN + RAISE 'cannot track temporary object' + USING DETAIL = format('object %s is in temporary schema %s', r_identity.identity, r_identity.schema) + , ERRCODE = 'feature_not_supported' + ; + END IF; + + -- Ensure the object record exists + SELECT INTO r_object_v + * + FROM _object_reference._object_v__for_update o + WHERE (o.object_type, o.object_names, o.object_args) = (_object_v__for_update.object_type, r_address.object_names, r_address.object_args) + ; + IF NOT FOUND THEN + FOR i IN 1..10 LOOP + did_insert := true; + INSERT INTO _object_reference.object(object_type, object_names, object_args) + VALUES(_object_v__for_update.object_type, r_address.object_names, r_address.object_args) + ON CONFLICT ON CONSTRAINT object__u_object_names__object_args DO NOTHING + ; + -- Still a small race condition here... + SELECT INTO r_object_v + * + FROM _object_reference._object_v__for_update o + WHERE (o.object_type, o.object_names, o.object_args) = (_object_v__for_update.object_type, r_address.object_names, r_address.object_args) + ; + EXIT WHEN FOUND; + END LOOP; + IF NOT FOUND THEN + RAISE 'fell out of loop!' USING HINT = 'This should never happen.'; + END IF; + END IF; + + ASSERT r_object_v.names_ok, 'names do not match (should not be possible)' ; + + IF object_group_id IS NOT NULL THEN + PERFORM object_reference.object_group__object__add(object_group_id, r_object_v.object_id); + END IF; + + -- Handle _object_oid table + CASE + WHEN r_object_v.ids_ok THEN + RETURN r_object_v; + + WHEN NOT r_object_v.ids_exist THEN + /* + * Just need to create IDs record. + */ + + /* + * This shouldn't normally happen, but could occur if a restore didn't + * finish cleanly. We know it's safe to do this because names_ok is true. + */ + IF NOT did_insert THEN + RAISE WARNING 'missing record in _object_reference._object_oid for object_id %', r_object_v.object_id + USING HINT = 'This indicates a restore did not finish cleanly.' + ; + END IF; + r_object_v := _object_reference._object_oid__add(r_object_v.object_id, object_type, c_classid, objid, objsubid); + + WHEN r_object_v.ids_exist THEN + RAISE 'ids are out of sync for object_id %', r_object_v.object_id + USING DETAIL = format( + E'_object_reference._object_v = %L,\n arguments (%L, %s, %s, %s)' + , pg_catalog.row_to_json(r_object_v, true) + , object_type + , objid + , objsubid + , object_group_id + ) + , HINT = 'this shoud not happen if event trigger "zzz_object_reference_end" is working' + ; + ELSE + RAISE 'unknown condition'; + END CASE; + + RETURN r_object_v; +END +$body$ + , 'Return details of a object record, creating a new record if one does not exist.' +); + +SELECT __object_reference.create_function( + '_object_reference._object_oid__add' + , $args$ + object_id _object_reference._object_oid.object_id%TYPE + , object_type _object_reference.object.object_type%TYPE DEFAULT NULL + , classid _object_reference._object_oid.classid%TYPE DEFAULT NULL + , objid _object_reference._object_oid.objid%TYPE DEFAULT NULL + , objsubid _object_reference._object_oid.objsubid%TYPE DEFAULT NULL +$args$ + , '_object_reference._object_v LANGUAGE plpgsql' + , $body$ +DECLARE + r_object_v _object_reference._object_v; +BEGIN + IF object_type IS NULL THEN + -- Should definitely exist + SELECT INTO STRICT object_type, classid, objid, objsubid + o.object_type, a.classid, a.objid, a.objsubid + FROM _object_reference.object o + , pg_catalog.pg_get_object_address(o.object_type::text, o.object_names, o.object_args) a + WHERE o.object_id = _object_oid__add.object_id + ; + END IF; + BEGIN + INSERT INTO _object_reference._object_oid(object_id, classid, objid, objsubid, object_oid) + VALUES (object_id, classid, objid, objsubid, objid); + + SELECT INTO STRICT r_object_v -- Record better exist! + * + FROM _object_reference._object_v__for_update o + WHERE o.object_id = _object_oid__add.object_id + ; + END; + + IF NOT r_object_v.ids_ok THEN + RAISE 'id mismatch for object_id %', object_id + USING + DETAIL = '_object_reference._object_v = ' || pg_catalog.row_to_json(r_object_v) + , HINT = 'this should not be possible' + ; + END IF; + + RETURN r_object_v; +END +$body$ + , 'Check the sanity of object and _object_oid' +); + +/* + * object_reference.unsupported(): additionally exclude "partitioned + * table"/"partitioned index" (pg_get_object_address() only recognizes the + * base "table"/"index" types they derive from, so identity tracking can't + * round-trip them). Same signature as 0.1.0, so a plain CREATE OR REPLACE + * (via create_function) is enough -- no DROP needed. + */ +SELECT __object_reference.create_function( + 'object_reference.unsupported' + , '' + , 'cat_tools.object_type[] LANGUAGE sql IMMUTABLE' + , $body$ +SELECT cat_tools.objects__shared() + || cat_tools.objects__address_unsupported() + /* + * pg_get_object_address() doesn't recognize "partitioned table" or + * "partitioned index" (only the base "table"/"index" types it derives + * from), so object identity tracking can't round-trip them. + */ + || '{event trigger, partitioned table, partitioned index}' +$body$ + , 'Returns array of object types that are not supported.' + , 'object_reference__usage' +); + +/* + * New: automatic object cleanup when removed from a group. + */ +SELECT __object_reference.create_function( + '_object_reference._object_group__object__cleanup_trigger' + , '' + , 'trigger LANGUAGE plpgsql' + , $body$ +BEGIN + PERFORM object_reference.object__cleanup(OLD.object_id); + RETURN OLD; +END +$body$ + , 'Trigger function to automatically attempt cleanup of objects when removed from groups.' +); +CREATE TRIGGER object_group__object__cleanup + AFTER DELETE ON _object_reference.object_group__object + FOR EACH ROW + EXECUTE FUNCTION _object_reference._object_group__object__cleanup_trigger(); + +/* + * New: OBJECT INFO FUNCTIONS + */ +SELECT __object_reference.create_function( + 'object_reference.object__describe' + , $args$ + object_id int +$args$ + , 'text LANGUAGE sql' + , $body$ +SELECT pg_catalog.pg_describe_object( + o.classid + , o.objid + , o.objsubid +) +FROM _object_reference._object_oid o +WHERE o.object_id = $1 +$body$ + , 'Return a human-readable description of the object, matching pg_describe_object() format.' + , 'object_reference__usage' +); + +SELECT __object_reference.create_function( + 'object_reference.object__identity' + , $args$ + object_id int + , OUT type text + , OUT schema text + , OUT name text + , OUT identity text +$args$ + , 'record LANGUAGE sql' + , $body$ +SELECT + i.type::text + , i.schema::text + , i.name::text + , i.identity::text +FROM _object_reference._object_oid o + , LATERAL pg_catalog.pg_identify_object(o.classid, o.objid, o.objsubid) i +WHERE o.object_id = $1 +$body$ + , 'Return object identification information matching pg_identify_object() format.' + , 'object_reference__usage' +); +SELECT __object_reference.create_function( + 'object_reference.object__cleanup' + , $args$ + object_id int +$args$ + , 'void LANGUAGE plpgsql' + , $body$ +BEGIN + DELETE FROM _object_reference.object WHERE object.object_id = object__cleanup.object_id; +EXCEPTION WHEN foreign_key_violation THEN + -- Object is still referenced elsewhere, ignore the error + NULL; +END +$body$ + , 'Attempts to delete an object from the tracking system. Silently returns if the object is still referenced by other tables.' + , 'object_reference__usage' +); + +/* + * _tg_capture_safety(): gains a trailing RETURN NULL. Same signature, so a + * plain CREATE OR REPLACE (via create_function) is enough. + */ +SELECT __object_reference.create_function( + '_object_reference._tg_capture_safety' + , '' + , 'trigger LANGUAGE plpgsql' + , $body$ +BEGIN + IF EXISTS(SELECT 1 FROM pg_temp.__object_reference__ddl_capture) THEN + RAISE 'attempted commit while still capturing DDL' + USING HINT = 'Did you not start a transaction? Did you forget to call object_reference.capture__stop()?' + ; + END IF; + + RETURN NULL; +END +$body$ + , 'Trigger function to ensure capture__stop() is called an appropriate number of times.' +); + +/* + * capture__start(object_group_id int): body text only differs from 0.1.0 by + * an added "EXCLUDED CODE" annotation on an already-dead comment block (the + * commented-out CREATE TEMP TABLE ... AS alternative) -- no behavior change, + * but recreated anyway so the update path converges byte-for-byte with a + * fresh install rather than leaving a purely cosmetic difference in place. + */ +SELECT __object_reference.create_function( + 'object_reference.capture__start' + , $args$ + object_group_id _object_reference.object_group.object_group_id%TYPE +$args$ + , 'int SECURITY DEFINER LANGUAGE plpgsql' + , $body$ +DECLARE + c_next_level int := coalesce(capture_level, 0) + 1 FROM object_reference.capture__get_current(); +BEGIN + -- Ensure object group exists + PERFORM object_reference.object_group__get(object_group_id); + + INSERT INTO pg_temp.__object_reference__ddl_capture + SELECT c_next_level, capture__start.object_group_id + ; + RETURN c_next_level; + +EXCEPTION WHEN undefined_table THEN + /* EXCLUDED CODE + CREATE TEMP TABLE __object_reference__ddl_capture AS + SELECT c_next_level, capture__start.object_group_id + ; + */ + CREATE TEMP TABLE __object_reference__ddl_capture( + capture_level int PRIMARY KEY + , object_group_id INT NOT NULL -- temp tables can't reference permanent ones + ); + -- This breaks if run directly under plpgsql + EXECUTE $code$ + CREATE CONSTRAINT TRIGGER verify_capture_stop AFTER INSERT + ON pg_temp.__object_reference__ddl_capture + DEFERRABLE INITIALLY DEFERRED + FOR EACH ROW -- CONSTRAINT triggers must be per-ROW + EXECUTE PROCEDURE _object_reference._tg_capture_safety() + $code$; + + INSERT INTO pg_temp.__object_reference__ddl_capture + SELECT c_next_level, capture__start.object_group_id + ; + RETURN c_next_level; +END +$body$ + , 'Begin capturing newly created objects to . Returns current capture level.' + , 'object_reference__usage' +); + +/* + * New: example/debug event-trigger functions (not wired to any CREATE EVENT + * TRIGGER -- 0.1.0 had an equivalent commented-out "snitch" example instead). + */ +SELECT __object_reference.create_function( + '_object_reference.etg_raise__start' + , '' + , 'event_trigger LANGUAGE plpgsql' + , $body$ +BEGIN + RAISE WARNING 'etg_raise__start: % %', tg_event, tg_tag; +END; +$body$ + , $$Event trigger function to report on DDL activity. Example trigger: +CREATE EVENT TRIGGER start + ON ddl_command_start + --WHEN tag IN ( 'ALTER TABLE', 'DROP TABLE' ) + EXECUTE PROCEDURE _object_reference.etg_raise__start() +; +$$); +SELECT __object_reference.create_function( + '_object_reference.etg_raise__drop' + , '' + , 'event_trigger LANGUAGE plpgsql' + , $body$ +DECLARE + r record; +BEGIN + FOR r IN SELECT classid, objid, objsubid, object_type, schema_name, object_name, object_identity FROM pg_catalog.pg_event_trigger_dropped_objects() LOOP + RAISE WARNING 'dropped_objects: + classid: % + objid: % + objsubid: % + object_type: % + schema_name: % + object_name: % + object_identity: % + ' + -- :^r" s/\([^ ]\+\):.*/, r.\1/ + , r.classid + , r.objid + , r.objsubid + , r.object_type + , r.schema_name + , r.object_name + , r.object_identity + ; + END LOOP; +END; +$body$ + , $$Event trigger function to report on DDL activity. Example trigger: +CREATE EVENT TRIGGER drop + ON sql_drop + --WHEN tag IN ( 'ALTER TABLE', 'DROP TABLE' ) + EXECUTE PROCEDURE _object_reference.etg_raise__drop() +; +$$); + +/* + * Re-enable the event triggers disabled near the top of this script, now + * that every object they reference is back in its final, current-source + * shape. + */ +ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE; +ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE; +ALTER EVENT TRIGGER zzz_object_reference_capture ENABLE; + +/* + * Drop "temporary" objects -- same convention as the fresh install script. + */ +DROP FUNCTION __object_reference.create_function( + function_name text + , args text + , options text + , body text + , comment text + , grants text +); +DROP FUNCTION __object_reference.safe_dump( + relation regclass + , text +); +DROP FUNCTION __object_reference.exec( + sql text +); +DROP SCHEMA __object_reference; + +-- vi: expandtab sw=2 ts=2 diff --git a/test/expected/zzz_build.out b/test/build/expected/zzz_build.out similarity index 100% rename from test/expected/zzz_build.out rename to test/build/expected/zzz_build.out diff --git a/test/sql/zzz_build.sql b/test/build/zzz_build.sql similarity index 100% rename from test/sql/zzz_build.sql rename to test/build/zzz_build.sql diff --git a/test/expected/_object_v.out b/test/expected/_object_v.out index 6143418..02b7d4a 100644 --- a/test/expected/_object_v.out +++ b/test/expected/_object_v.out @@ -1,4 +1,5 @@ \set ECHO none -1..1 +1..2 ok 1 - _object_v__for_update matches _object_v +ok 2 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/all.out b/test/expected/all.out index c7f5c84..a28f1b3 100644 --- a/test/expected/all.out +++ b/test/expected/all.out @@ -1,5 +1,5 @@ \set ECHO none -1..73 +1..74 ok 1 - All object types are being tested. ok 2 - Verify object_reference.unsupported() ok 3 - prereq: CREATE DOMAIN "test domain" int @@ -73,4 +73,5 @@ ok 70 - DROP index "test table test index" ok 71 - Drop should fail while reference exists ok 72 - DROP table "test table" ok 73 - No object references remain +ok 74 - object_reference schema(s) must not be part of the resolved search_path -- got {object_identity_temp_test_schema,test_support,tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/base.out b/test/expected/base.out index ac3edd2..dcb1adf 100644 --- a/test/expected/base.out +++ b/test/expected/base.out @@ -1,5 +1,5 @@ \set ECHO none -1..10 +1..11 ok 1 - Role object_reference__dependency should be granted USAGE on schema _object_reference ok 2 - Role object_reference__dependency should be granted REFERENCES on table _object_reference.object ok 3 - CREATE TEMP TABLE test_object AS SELECT object_reference.object__getsert('table', 'test_table') AS object_id; @@ -10,4 +10,5 @@ ok 7 - Existing object works, provides correct ID ok 8 - secondary may not be specified for table objects ok 9 - temp objects are rejected ok 10 - CREATE EXTENSION test_factory +ok 11 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/capture.out b/test/expected/capture.out index a93a9eb..eec98b7 100644 --- a/test/expected/capture.out +++ b/test/expected/capture.out @@ -1,5 +1,5 @@ \set ECHO none -1..69 +1..70 ok 1 - prereq: CREATE DOMAIN "test domain" int ok 2 - prereq: CREATE FUNCTION tg_null() RETURNS trigger LANGUAGE plpgsql AS $body$BEGIN RETURN NEW; END$body$ ok 3 - prereq: CREATE TYPE "test type" @@ -69,4 +69,5 @@ ok 66 - Drop should fail while reference exists ok 67 - DROP table "test table" ok 68 - object_group_ids still has correct record count ok 69 - No object references remain +ok 70 - object_reference schema(s) must not be part of the resolved search_path -- got {object_identity_temp_test_schema,test_support,tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/event_trigger.out b/test/expected/event_trigger.out index ad56bc9..9969fc3 100644 --- a/test/expected/event_trigger.out +++ b/test/expected/event_trigger.out @@ -1,5 +1,5 @@ \set ECHO none -1..42 +1..43 ok 1 - Register schema-drop test objects ok 2 - Create objects_view ok 3 - Exactly 3 test view records @@ -42,4 +42,5 @@ ok 39 - Verify filler column record is deleted ok 40 - Verify objects still registered correctly ok 41 - Drop schema ok 42 - objects_view is empty +ok 43 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/finish.sql b/test/finish.sql new file mode 100644 index 0000000..f9b1110 --- /dev/null +++ b/test/finish.sql @@ -0,0 +1,26 @@ +/* + * Asserts object_reference's own schema(s) are absent from the resolved + * search_path -- checked here (file end, before finish()) rather than only + * at setup, so a test that mutates search_path mid-file and never restores + * it is caught. Not foolproof: mutate-then-restore before this line still + * slips through. \i'd by every SQL file under test/sql (in place of + * test/pgxntool/finish.sql, which this chains through to). + * + * This is the one permanent proof that every reference inside + * object_reference's own SQL is fully schema-qualified: test/load.sql (via + * pgxntool's own tap_setup.sql) sets search_path = tap, public for every test + * file, so object_reference/_object_reference are never on it -- every other + * test in the suite passing means nothing accidentally relied on + * search_path to resolve one of the extension's own objects. + */ +SELECT ok( + NOT ( 'object_reference' = ANY (current_schemas(false)) OR '_object_reference' = ANY (current_schemas(false)) ) + , format( + 'object_reference schema(s) must not be part of the resolved search_path -- got %s' + , current_schemas(false) + ) +); + +\i test/pgxntool/finish.sql + +-- vi: expandtab sw=2 ts=2 diff --git a/test/install/.gitignore b/test/install/.gitignore new file mode 100644 index 0000000..eb02f57 --- /dev/null +++ b/test/install/.gitignore @@ -0,0 +1,6 @@ +# pg_regress writes the install step's result (and any diff) here, because the +# install schedule references tests as ../install/. The install output is +# self-comparing (pg_regress resolves both the expected and result paths to this +# directory), so it is never asserted and must not be tracked. +load.out +install.out.diff diff --git a/test/install/load.sql b/test/install/load.sql new file mode 100644 index 0000000..fc3a98a --- /dev/null +++ b/test/install/load.sql @@ -0,0 +1,194 @@ +/* + * Single, committed-once installer for the test suite's dependency: the + * object_reference extension. + * + * pgxntool's test/install feature runs this file COMMITTED, in its own + * pg_regress session, BEFORE the main pgTAP suite. Because its state is + * committed it persists into every test and runs ONCE instead of per-test + * (pgTAP rolls back each test/sql/ file -- see test/load.sql -- so tests read + * the extension's objects but never modify them here). + * + * Three modes, selected by the object_reference.test_load_mode placeholder + * GUC, which the Makefile's TEST_LOAD_SOURCE block sets via PGOPTIONS (fresh + * is the default): + * - fresh (default): plain CREATE EXTENSION object_reference (current + * version), CASCADE (object_reference requires cat_tools). + * - update: CREATE EXTENSION at an older version + * (object_reference.test_update_from, default 0.1.0 -- the only real + * historical PGXN release) then ALTER EXTENSION UPDATE -- to + * object_reference.test_update_to when that GUC is non-empty, otherwise + * to the current default_version ("stable"). Reusing the SAME suite and + * expected output asserts an updated database behaves identically to a + * fresh install. + * - existing: the extension is ALREADY installed (by binary pg_upgrade, or + * an ALTER EXTENSION UPDATE performed outside the suite). load.sql must + * NOT drop/create/update it -- that would destroy exactly what the suite + * validates. It only asserts presence + current version. + * + * test/load.sql (run per-test, rolled back) installs nothing itself; its + * `CREATE EXTENSION IF NOT EXISTS` is a no-op here since the extension is + * already installed by this file, and only does real work for + * test/dump/load_all.sql's standalone flow, which never goes through + * test/install at all. + * + * ON_ERROR_STOP: this file runs standalone (not \i'd via + * test/pgxntool/psql.sql, which would normally set it), and pg_regress + * doesn't set it either -- without it, an unexpected error here (e.g. a + * missing dependency the CASCADE doesn't cover) doesn't abort the script; it + * just prints the error and keeps going statement by statement, potentially + * leaving the extension half-installed while later steps run against that + * broken state and the run-log genuinely does show the failure, just buried + * many statements deep instead of stopping at the first one. + */ +\set ON_ERROR_STOP 1 + +SET client_min_messages = WARNING; + +/* + * Mode selection. The Makefile always exports object_reference.test_load_mode + * via PGOPTIONS. Read it WITHOUT missing_ok: if the GUC did not propagate (a + * break anywhere in make -> PGOPTIONS -> env -> psql), current_setting errors + * here and the whole install step fails loudly, instead of silently falling + * back to a default and running the wrong suite. The DO block then rejects + * any value other than fresh/update/existing with a clear message. + */ +SELECT current_setting('object_reference.test_load_mode') AS object_reference_test_load_mode +\gset + +DO $DO$ +BEGIN + IF current_setting('object_reference.test_load_mode') NOT IN ('fresh', 'update', 'existing') THEN + RAISE EXCEPTION + 'object_reference.test_load_mode must be ''fresh'', ''update'' or ''existing'', got ''%''' + , current_setting('object_reference.test_load_mode') + ; + END IF; +END +$DO$; + +SELECT + :'object_reference_test_load_mode' = 'update' AS object_reference_mode_update + , :'object_reference_test_load_mode' = 'existing' AS object_reference_mode_existing +\gset + +\if :object_reference_mode_existing +/* + * existing mode: do NOT touch the extension. Assert it is installed and at + * the current default_version -- the pg_upgrade / external update the + * database just went through is exactly what the suite is validating, so + * dropping or reinstalling it would defeat the test. Fail loudly on absence + * or mismatch. + * + * A future CI-wiring PR (this PR only builds the local machinery -- see the + * containing PR's description) should additionally plant a dependency guard + * here: a view typed on a stable, object_reference-owned member (so a + * non-CASCADE DROP EXTENSION fails) and prove that non-CASCADE drop actually + * fails, so a stray CASCADE drop or logic bug that falls through to the + * fresh/update branch is caught rather than silently retesting a fresh + * install. The anchor to use for that guard is _object_reference.object's row + * type (composite type of the extension's core object-tracking table): it is + * object_reference-owned (unlike cat_tools.object_type, which this extension + * only consumes), and no update script -- including 0.1.0--stable.sql -- ever + * drops or redefines that table. + */ +DO $DO$ +DECLARE + v_installed text := (SELECT extversion FROM pg_extension WHERE extname = 'object_reference'); + v_default text := (SELECT default_version FROM pg_available_extensions WHERE name = 'object_reference'); +BEGIN + IF v_installed IS NULL THEN + RAISE EXCEPTION 'test_load_mode=existing but object_reference is not installed'; + END IF; + IF v_installed IS DISTINCT FROM v_default THEN + RAISE EXCEPTION + 'object_reference is installed at version % but the current default_version is %' + , v_installed, v_default + ; + END IF; +END +$DO$; +\else +/* + * fresh / update: (re)install from scratch. Drop-first so a re-run on a + * persistent cluster installs the newest build instead of reusing stale + * objects. + * + * DROP EXTENSION does not remove object_reference__usage / + * object_reference__dependency: the extension's own install script creates + * them with a duplicate_object-tolerant CREATE ROLE, and roles are global + * objects, not extension members, so they survive DROP EXTENSION. That + * duplicate_object handling makes plain re-creation safe, but it does NOT + * clear any grants a previous test run (or a previous, differently-shaped + * version of this extension) may have left on the role -- e.g. the + * schema_privs_are()/table_privs_are() checks in test/sql/base.sql assert an + * EXACT privilege set, which a stale extra grant from an earlier run on the + * same persistent cluster would silently break. Drop both roles explicitly + * via pg_temp.drop_role(): DROP OWNED BY first strips any privileges granted + * TO the role so DROP ROLE cannot fail with a dependency error, and the + * pg_roles guard skips a not-yet-existing role (DROP OWNED BY errors on one). + */ +DROP EXTENSION IF EXISTS object_reference CASCADE; + +CREATE FUNCTION pg_temp.drop_role( + role_name text +) RETURNS void LANGUAGE plpgsql AS $$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_roles WHERE rolname = role_name) THEN + EXECUTE format('DROP OWNED BY %I', role_name); + EXECUTE format('DROP ROLE IF EXISTS %I', role_name); + END IF; +END +$$; + +SELECT pg_temp.drop_role('object_reference__usage'); +SELECT pg_temp.drop_role('object_reference__dependency'); + +\if :object_reference_mode_update +/* + * update mode: install an older version, then ALTER EXTENSION UPDATE. The + * from/to versions come from the Makefile (TEST_UPDATE_FROM / TEST_UPDATE_TO, + * exported as GUCs). An empty test_update_to means "update to the current + * default_version" (the widest path); a non-empty value targets a specific + * version. + */ +SELECT current_setting('object_reference.test_update_from') AS object_reference_test_update_from \gset +SELECT current_setting('object_reference.test_update_to') AS object_reference_test_update_to \gset +/* + * Build the optional target clause once so a SINGLE ALTER EXTENSION covers + * both cases: an empty test_update_to yields '' (update to the current + * default_version -- the widest path); a non-empty value yields "TO ''". + * format(%L) quotes the version literal safely; the bare :clause + * interpolation below then drops it in verbatim. + */ +SELECT CASE WHEN :'object_reference_test_update_to' = '' THEN '' + ELSE format('TO %L', :'object_reference_test_update_to') END + AS object_reference_update_to_clause \gset + +/* + * 0.1.0 (the default test_update_from floor)'s own install script creates a + * trigger that calls count_nulls' not_null_count_trigger() -- a dependency + * object_reference.control no longer declares in `requires` now that the + * reg* pseudotype removal made it unnecessary, so CASCADE below will NOT + * bring count_nulls in automatically the way it would have when 0.1.0 was + * current. Install it explicitly first; the Makefile's conditional `install: + * count_nulls` prerequisite (TEST_LOAD_SOURCE=update only) ensures it's + * actually present on disk to install from. + */ +CREATE EXTENSION IF NOT EXISTS count_nulls; + +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +/* + * Suppress the deprecation NOTICEs the update script emits (e.g. the reg* + * pseudotype removal), matching cat_tools' own install/load.sql. + */ +SET client_min_messages = ERROR; +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SET client_min_messages = WARNING; +\else +CREATE EXTENSION object_reference CASCADE; +\endif +-- end \if :object_reference_mode_update (fresh vs. update install branch) +\endif +-- end \if :object_reference_mode_existing (existing mode skips the whole (re)install block) + +-- vi: expandtab sw=2 ts=2 diff --git a/test/load.sql b/test/load.sql index f1b267f..14ccd26 100644 --- a/test/load.sql +++ b/test/load.sql @@ -2,7 +2,15 @@ SET search_path = tap, public; --- Don't use IF NOT EXISTS here; we want to ensure we always have the latest code +/* + * The extension is normally already installed -- committed, once -- by + * test/install/load.sql before this file ever runs (see that file for the + * fresh/update/existing mode switch); IF NOT EXISTS makes the statement + * below a safe no-op in that case. It still does REAL work for + * test/dump/load_all.sql, which drives its own standalone `createdb` + + * `psql -f` flow (test/dump/run.sh) entirely outside pg_regress and + * test/install, and has no other way to get the extension installed. + */ SET client_min_messages = WARNING; -- Squelch notices about dependent extensions -CREATE EXTENSION object_reference CASCADE; +CREATE EXTENSION IF NOT EXISTS object_reference CASCADE; --SET client_min_messages = NOTICE; diff --git a/test/sql/_object_v.sql b/test/sql/_object_v.sql index 53eb8c9..10a57b9 100644 --- a/test/sql/_object_v.sql +++ b/test/sql/_object_v.sql @@ -6,6 +6,7 @@ SELECT plan( 0 + 1 -- equality + + 1 -- schema-qualification (search_path) ); -- TODO: load some damn data first @@ -15,6 +16,6 @@ SELECT bag_eq( , '_object_v__for_update matches _object_v' ); -\i test/pgxntool/finish.sql +\i test/finish.sql -- vi: expandtab sw=2 ts=2 diff --git a/test/sql/all.sql b/test/sql/all.sql index 8e96fe2..eea700c 100644 --- a/test/sql/all.sql +++ b/test/sql/all.sql @@ -28,6 +28,7 @@ SELECT plan( ( + c * 2 -- drop + 1 -- verify object table is now empty + + 1 -- schema-qualification (search_path) )::int ) FROM (SELECT count(*) c FROM test_object) c ; @@ -85,6 +86,6 @@ SELECT is_empty( , 'No object references remain' ); -\i test/pgxntool/finish.sql +\i test/finish.sql -- vi: expandtab sw=2 ts=2 diff --git a/test/sql/base.sql b/test/sql/base.sql index fb07b76..3bcaaf1 100644 --- a/test/sql/base.sql +++ b/test/sql/base.sql @@ -11,6 +11,7 @@ SELECT plan( +2 -- new functions +3 -- errors (includes temp object test) +1 -- create extensions + +1 -- schema-qualification (search_path) ); -- Schema @@ -79,6 +80,6 @@ SELECT lives_ok( , $$CREATE EXTENSION test_factory$$ ); -\i test/pgxntool/finish.sql +\i test/finish.sql -- vi: expandtab sw=2 ts=2 diff --git a/test/sql/capture.sql b/test/sql/capture.sql index 073f3f3..bbfba64 100644 --- a/test/sql/capture.sql +++ b/test/sql/capture.sql @@ -49,6 +49,7 @@ SELECT plan( ( + cna * 2 -- Drop objects + 1 -- Verify object_group_ids still has correct count + 1 -- verify object table is now empty + + 1 -- schema-qualification (search_path) )::int ) FROM (SELECT count(*) c, count(CASE WHEN create_command NOT LIKE 'ALTER%' THEN 1 END) AS cna FROM test_object) c @@ -234,6 +235,6 @@ SELECT is_empty( , 'No object references remain' ); -\i test/pgxntool/finish.sql +\i test/finish.sql -- vi: expandtab sw=2 ts=2 diff --git a/test/sql/event_trigger.sql b/test/sql/event_trigger.sql index 402a5b4..cb2b24d 100644 --- a/test/sql/event_trigger.sql +++ b/test/sql/event_trigger.sql @@ -21,6 +21,7 @@ SELECT plan( +2 + 1 -- column drop +3 -- table drop +3 -- schema drop + +1 -- schema-qualification (search_path) ); SELECT lives_ok( @@ -240,6 +241,6 @@ SELECT is( , 'objects_view is empty' ); -\i test/pgxntool/finish.sql +\i test/finish.sql -- vi: expandtab sw=2 ts=2 diff --git a/test/sql/object_group.sql b/test/sql/object_group.sql index 4839ff4..1e5ee54 100644 --- a/test/sql/object_group.sql +++ b/test/sql/object_group.sql @@ -42,6 +42,7 @@ SELECT plan( +3 + 2 -- __remove +4 -- cleanup tests +1 -- final group removal (there was always an extra test) + +1 -- schema-qualification (search_path) ); SELECT lives_ok( @@ -238,6 +239,6 @@ SELECT lives_ok( , 'Removing empty group works' ); -\i test/pgxntool/finish.sql +\i test/finish.sql -- vi: expandtab sw=2 ts=2 From 2a7667ce5d1bced3a41f9079f6e7e4c145b77213 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 4 Aug 2026 20:07:34 -0500 Subject: [PATCH 02/13] ci: install rsync for pgxntool's test-build feature test/build/zzz_build.sql (moved there in the previous commit) enables pgxntool's test-build feature, whose run-test-build.sh syncs test/build/*.sql into test/build/sql/ via rsync -- not present in the pgxn/pgxn-tools image, causing every PG-matrix leg to fail with "rsync: command not found". Co-Authored-By: Claude Sonnet 5 --- .github/workflows/ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 89b6887..24a28dc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -129,6 +129,11 @@ jobs: run: pg-start ${{ matrix.pg }} - name: Check out the repo uses: actions/checkout@v4 + # pgxntool's test-build feature (test/build/zzz_build.sql) syncs + # test/build/*.sql into test/build/sql/ via rsync (run-test-build.sh), + # which the pgxn/pgxn-tools image doesn't ship. + - name: Install rsync + run: apt-get install -y rsync - name: Test on PostgreSQL ${{ matrix.pg }} # `make test` alone never fails this step: pgxntool/base.mk (as # vendored here, 2.2.0) marks its underlying installcheck .IGNORE, From 84296266e42b4dd2ef53ae1d4a766b2b3bf415f2 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sun, 9 Aug 2026 14:38:09 -0500 Subject: [PATCH 03/13] Remove the update script's now-redundant capture__start re-declaration 0.1.0 and stable are byte-identical for this function's body once the linter's annotation (PR #16's own, separate content) isn't part of this branch -- the CREATE OR REPLACE was a no-op here. Verified via bin/test_existing's structural diff (clean) and a full local run (7/7 pgTAP files, dump/restore, test-build). --- sql/object_reference--0.1.0--stable.sql | 54 ------------------------- 1 file changed, 54 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 515eae6..d305fd4 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -594,60 +594,6 @@ $body$ , 'Trigger function to ensure capture__stop() is called an appropriate number of times.' ); -/* - * capture__start(object_group_id int): body text only differs from 0.1.0 by - * an added "EXCLUDED CODE" annotation on an already-dead comment block (the - * commented-out CREATE TEMP TABLE ... AS alternative) -- no behavior change, - * but recreated anyway so the update path converges byte-for-byte with a - * fresh install rather than leaving a purely cosmetic difference in place. - */ -SELECT __object_reference.create_function( - 'object_reference.capture__start' - , $args$ - object_group_id _object_reference.object_group.object_group_id%TYPE -$args$ - , 'int SECURITY DEFINER LANGUAGE plpgsql' - , $body$ -DECLARE - c_next_level int := coalesce(capture_level, 0) + 1 FROM object_reference.capture__get_current(); -BEGIN - -- Ensure object group exists - PERFORM object_reference.object_group__get(object_group_id); - - INSERT INTO pg_temp.__object_reference__ddl_capture - SELECT c_next_level, capture__start.object_group_id - ; - RETURN c_next_level; - -EXCEPTION WHEN undefined_table THEN - /* EXCLUDED CODE - CREATE TEMP TABLE __object_reference__ddl_capture AS - SELECT c_next_level, capture__start.object_group_id - ; - */ - CREATE TEMP TABLE __object_reference__ddl_capture( - capture_level int PRIMARY KEY - , object_group_id INT NOT NULL -- temp tables can't reference permanent ones - ); - -- This breaks if run directly under plpgsql - EXECUTE $code$ - CREATE CONSTRAINT TRIGGER verify_capture_stop AFTER INSERT - ON pg_temp.__object_reference__ddl_capture - DEFERRABLE INITIALLY DEFERRED - FOR EACH ROW -- CONSTRAINT triggers must be per-ROW - EXECUTE PROCEDURE _object_reference._tg_capture_safety() - $code$; - - INSERT INTO pg_temp.__object_reference__ddl_capture - SELECT c_next_level, capture__start.object_group_id - ; - RETURN c_next_level; -END -$body$ - , 'Begin capturing newly created objects to . Returns current capture level.' - , 'object_reference__usage' -); - /* * New: example/debug event-trigger functions (not wired to any CREATE EVENT * TRIGGER -- 0.1.0 had an equivalent commented-out "snitch" example instead). From e7b9f694d4960c7c0b80157e1845ffdb92ae0b44 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 15:45:42 -0500 Subject: [PATCH 04/13] Regenerate object_group.out for the __remove plan-count fix Same stale 1..29 header issue as u-and-u-extension-update-ci: picked up the corrected plan count from reconcile-object-functions but not the regenerated total including this branch's own schema-qualification assertion (test 30). Regenerated via make results, not hand-edited. Co-Authored-By: Claude --- test/expected/object_group.out | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/expected/object_group.out b/test/expected/object_group.out index e670106..c58785b 100644 --- a/test/expected/object_group.out +++ b/test/expected/object_group.out @@ -1,5 +1,5 @@ \set ECHO none -1..29 +1..30 ok 1 - Register test table 1 ok 2 - object_group__create(...) for group name that is too long throws error ok 3 - object_group__create('object reference test group') @@ -29,4 +29,5 @@ ok 26 - Object exists before cleanup test ok 27 - Remove from group triggers automatic cleanup attempt ok 28 - Object was automatically cleaned up after group removal ok 29 - Removing empty group works +ok 30 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} # TRANSACTION INTENTIONALLY LEFT OPEN! From 1a708b8f5cb13b5d84b543e95a321894ab9066b7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 16:48:45 -0500 Subject: [PATCH 05/13] Update script: drop object_oid, use SET LOCAL for client_min_messages Mirrors reconcile-object-functions' fresh-install changes in this hand-authored 0.1.0->stable update script: - object_oid is dropped (not backfilled+kept) alongside the reg* columns it used to collapse -- with no reg* columns left after this update, it's pure redundant storage of objid. objid_must_match and the two view recreations' object_oid passthrough go with it. - SET LOCAL client_min_messages, not plain SET -- this script runs inside ALTER EXTENSION UPDATE's implicit transaction, so LOCAL reverts automatically once it commits. Verified structurally clean and functionally correct: make test TEST_LOAD_SOURCE=update (the real 0.1.0 -> stable update path) passes all 8 tests. Co-Authored-By: Claude --- sql/object_reference--0.1.0--stable.sql | 44 +++++++++++-------------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index d305fd4..9719fd4 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -13,8 +13,10 @@ * which called count_nulls' not_null_count_trigger(), is removed. * - The reg* pseudotype columns on _object_reference._object_oid * (regclass/regconfig/regdictionary/regnamespace/regoperator/ - * regprocedure/regtype) are removed in favor of a single plain `oid` - * column (object_oid), and classid changes from regclass to oid. + * regprocedure/regtype), plus the now-redundant object_oid column they + * used to collapse into (it always equaled objid once there was only + * one oid source left), are all dropped; classid changes from regclass + * to oid. * - _object_reference._object_v / _object_v__for_update (views) drop the * now-gone reg* columns. CREATE OR REPLACE VIEW cannot drop columns, so * both are DROP+CREATE'd, along with the two functions whose RETURNS @@ -49,7 +51,10 @@ * comparison of every recreated function/view against a fresh install of the * current version backs this file -- see the containing PR's description). */ -SET client_min_messages = WARNING; +-- SET LOCAL, not SET: this script runs inside ALTER EXTENSION UPDATE's +-- implicit transaction, so LOCAL reverts automatically once it commits -- +-- see sql/object_reference.sql's own identical comment on this same point. +SET LOCAL client_min_messages = WARNING; CREATE SCHEMA __object_reference; @@ -205,12 +210,14 @@ DROP VIEW _object_reference._object_v__for_update; DROP VIEW _object_reference._object_v; /* - * _object_reference._object_oid: drop the reg* pseudotype columns and the - * count_nulls-backed trigger that enforced "exactly one is set", in favor of - * a single NOT NULL object_oid column. Order below is fully explicit - * (constraints/indexes/trigger dropped by name, not left to an implicit - * CASCADE) so nothing is silently dropped alongside a `DROP COLUMN` we did - * not ask for. + * _object_reference._object_oid: drop the reg* pseudotype columns, the + * count_nulls-backed trigger that enforced "exactly one is set", and + * object_oid itself (it only ever existed to collapse whichever reg* column + * applied into a single plain-oid value -- with no reg* columns left, it's + * pure redundant storage of objid and buys nothing). Order below is fully + * explicit (constraints/indexes/trigger dropped by name, not left to an + * implicit CASCADE) so nothing is silently dropped alongside a `DROP COLUMN` + * we did not ask for. */ ALTER TABLE _object_reference._object_oid DROP CONSTRAINT regclass_classid @@ -232,16 +239,6 @@ DROP INDEX _object_reference._object_oid__u_regoperator; DROP INDEX _object_reference._object_oid__u_regprocedure; DROP INDEX _object_reference._object_oid__u_regtype; -/* - * Backfill: 0.1.0 only ever populated ONE of {regclass, ..., regtype, - * object_oid} per row (whichever reg* type applied; object_oid itself only - * when none did). objid was always kept equal to that same value (that's - * exactly what the old objid_must_match CHECK enforced), so copying objid - * into object_oid for every row is correct regardless of which reg* column - * used to carry it, and is a no-op where object_oid already matched. - */ -UPDATE _object_reference._object_oid SET object_oid = objid WHERE object_oid IS NULL; - ALTER TABLE _object_reference._object_oid DROP COLUMN regclass , DROP COLUMN regconfig @@ -250,9 +247,8 @@ ALTER TABLE _object_reference._object_oid , DROP COLUMN regoperator , DROP COLUMN regprocedure , DROP COLUMN regtype + , DROP COLUMN object_oid , ALTER COLUMN classid TYPE oid USING classid::oid - , ALTER COLUMN object_oid SET NOT NULL - , ADD CONSTRAINT objid_must_match CHECK ( objid IS NOT DISTINCT FROM object_oid ) -- _object_reference._sanity() depends on this! ; CREATE VIEW _object_reference._object_v AS @@ -264,7 +260,6 @@ CREATE VIEW _object_reference._object_v AS , i.classid , i.objid , i.objsubid - , i.object_oid , s.* FROM _object_reference.object o LEFT JOIN _object_reference._object_oid i USING(object_id) @@ -279,7 +274,6 @@ CREATE VIEW _object_reference._object_v__for_update AS , i.classid , i.objid , i.objsubid - , i.object_oid , s.* FROM _object_reference.object o LEFT JOIN _object_reference._object_oid i USING(object_id) @@ -440,8 +434,8 @@ BEGIN ; END IF; BEGIN - INSERT INTO _object_reference._object_oid(object_id, classid, objid, objsubid, object_oid) - VALUES (object_id, classid, objid, objsubid, objid); + INSERT INTO _object_reference._object_oid(object_id, classid, objid, objsubid) + VALUES (object_id, classid, objid, objsubid); SELECT INTO STRICT r_object_v -- Record better exist! * From 46976c7973f3ec896d3ff00c36e00584f996275a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 17:02:44 -0500 Subject: [PATCH 06/13] Fix multi-line comment to use block format, not consecutive -- lines Same CODE_STYLE.md fix as reconcile-object-functions' commit for sql/object_reference.sql, applied to this update script's identical client_min_messages explanation. Co-Authored-By: Claude --- sql/object_reference--0.1.0--stable.sql | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 9719fd4..d340d2d 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -51,9 +51,11 @@ * comparison of every recreated function/view against a fresh install of the * current version backs this file -- see the containing PR's description). */ --- SET LOCAL, not SET: this script runs inside ALTER EXTENSION UPDATE's --- implicit transaction, so LOCAL reverts automatically once it commits -- --- see sql/object_reference.sql's own identical comment on this same point. +/* + * SET LOCAL, not SET: this script runs inside ALTER EXTENSION UPDATE's + * implicit transaction, so LOCAL reverts automatically once it commits -- + * see sql/object_reference.sql's own identical comment on this same point. + */ SET LOCAL client_min_messages = WARNING; CREATE SCHEMA __object_reference; From 2117dff6aa077176ed3c38087c285b6361156858 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 17:53:53 -0500 Subject: [PATCH 07/13] Re-add capture__start(object_group_id) to the update script The earlier removal of this re-declaration assumed 0.1.0's body and current source were byte-identical for this function. They aren't: 0.1.0 still has a dead, commented-out CREATE TEMP TABLE ... AS attempt in the EXCEPTION handler that current source has since dropped. Functionally inert either way, but pg_get_functiondef() returns comments verbatim, so an updated install's function body would literally differ from a fresh install's -- caught by bin/test_existing's structural diff (planted expressly to catch this class of divergence). Recreated with the current, comment-free body; confirmed structurally identical to a fresh install afterward. Co-Authored-By: Claude --- sql/object_reference--0.1.0--stable.sql | 54 +++++++++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index d340d2d..1baed68 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -648,6 +648,60 @@ CREATE EVENT TRIGGER drop ; $$); +/* + * object_reference.capture__start(object_group_id): 0.1.0's body still has + * a dead, commented-out CREATE TEMP TABLE ... AS attempt inside the + * EXCEPTION handler that current source has since dropped -- functionally + * inert either way, but pg_get_functiondef() returns comments verbatim, so + * leaving it in place would make an updated install's function body + * literally differ from a fresh install's (caught by this repo's own + * fresh-vs-updated structural diff). Recreated here with the current, + * comment-free body; the other overload (capture__start(object_group_name), + * a thin wrapper) is untouched between 0.1.0 and current source and does + * not need recreating. + */ +SELECT __object_reference.create_function( + 'object_reference.capture__start' + , $args$ + object_group_id _object_reference.object_group.object_group_id%TYPE +$args$ + , 'int SECURITY DEFINER LANGUAGE plpgsql' + , $body$ +DECLARE + c_next_level int := coalesce(capture_level, 0) + 1 FROM object_reference.capture__get_current(); +BEGIN + -- Ensure object group exists + PERFORM object_reference.object_group__get(object_group_id); + + INSERT INTO pg_temp.__object_reference__ddl_capture + SELECT c_next_level, capture__start.object_group_id + ; + RETURN c_next_level; + +EXCEPTION WHEN undefined_table THEN + CREATE TEMP TABLE __object_reference__ddl_capture( + capture_level int PRIMARY KEY + , object_group_id INT NOT NULL -- temp tables can't reference permanent ones + ); + -- This breaks if run directly under plpgsql + EXECUTE $code$ + CREATE CONSTRAINT TRIGGER verify_capture_stop AFTER INSERT + ON pg_temp.__object_reference__ddl_capture + DEFERRABLE INITIALLY DEFERRED + FOR EACH ROW -- CONSTRAINT triggers must be per-ROW + EXECUTE PROCEDURE _object_reference._tg_capture_safety() + $code$; + + INSERT INTO pg_temp.__object_reference__ddl_capture + SELECT c_next_level, capture__start.object_group_id + ; + RETURN c_next_level; +END +$body$ + , 'Begin capturing newly created objects to . Returns current capture level.' + , 'object_reference__usage' +); + /* * Re-enable the event triggers disabled near the top of this script, now * that every object they reference is back in its final, current-source From 27d1c180a4a9a84d37f0cccfddcb092c3bb807ec Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 10 Aug 2026 18:16:35 -0500 Subject: [PATCH 08/13] Remove client_min_messages suppression from the update script Same reasoning as reconcile-object-functions' identical change to sql/object_reference.sql: a shipped script (this one runs for real on every ALTER EXTENSION UPDATE) shouldn't unilaterally change a session setting on its caller's behalf, even scoped via SET LOCAL. Verified no test depends on the quieter output -- make test/verify-results in both fresh and update (TEST_LOAD_SOURCE=update) modes still pass cleanly. Co-Authored-By: Claude --- sql/object_reference--0.1.0--stable.sql | 7 ------- 1 file changed, 7 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 1baed68..a3508e2 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -51,13 +51,6 @@ * comparison of every recreated function/view against a fresh install of the * current version backs this file -- see the containing PR's description). */ -/* - * SET LOCAL, not SET: this script runs inside ALTER EXTENSION UPDATE's - * implicit transaction, so LOCAL reverts automatically once it commits -- - * see sql/object_reference.sql's own identical comment on this same point. - */ -SET LOCAL client_min_messages = WARNING; - CREATE SCHEMA __object_reference; CREATE FUNCTION __object_reference.exec( From 501ad889ef76f248ba5c68d98f0a72371f0c8af7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 11 Aug 2026 13:06:03 -0500 Subject: [PATCH 09/13] test/finish.sql: don't report the actual search_path value The assertion's own description doesn't need to echo current_schemas() -- a failure already tells you which schema(s) shouldn't be there, and pgtap's own diagnostic output shows the assertion context. Dropped the format()/%s interpolation. test/expected/*.out regenerated via make results (every test file's last line changed since this is a description string, not just a pass/ fail check). Co-Authored-By: Claude --- test/expected/_object_v.out | 2 +- test/expected/all.out | 2 +- test/expected/base.out | 2 +- test/expected/capture.out | 2 +- test/expected/event_trigger.out | 2 +- test/expected/object_group.out | 2 +- test/finish.sql | 5 +---- 7 files changed, 7 insertions(+), 10 deletions(-) diff --git a/test/expected/_object_v.out b/test/expected/_object_v.out index 02b7d4a..1f2bcf5 100644 --- a/test/expected/_object_v.out +++ b/test/expected/_object_v.out @@ -1,5 +1,5 @@ \set ECHO none 1..2 ok 1 - _object_v__for_update matches _object_v -ok 2 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} +ok 2 - object_reference schema(s) must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/all.out b/test/expected/all.out index a28f1b3..0a4e41b 100644 --- a/test/expected/all.out +++ b/test/expected/all.out @@ -73,5 +73,5 @@ ok 70 - DROP index "test table test index" ok 71 - Drop should fail while reference exists ok 72 - DROP table "test table" ok 73 - No object references remain -ok 74 - object_reference schema(s) must not be part of the resolved search_path -- got {object_identity_temp_test_schema,test_support,tap,public} +ok 74 - object_reference schema(s) must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/base.out b/test/expected/base.out index dcb1adf..8369c9e 100644 --- a/test/expected/base.out +++ b/test/expected/base.out @@ -10,5 +10,5 @@ ok 7 - Existing object works, provides correct ID ok 8 - secondary may not be specified for table objects ok 9 - temp objects are rejected ok 10 - CREATE EXTENSION test_factory -ok 11 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} +ok 11 - object_reference schema(s) must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/capture.out b/test/expected/capture.out index eec98b7..c3e43b1 100644 --- a/test/expected/capture.out +++ b/test/expected/capture.out @@ -69,5 +69,5 @@ ok 66 - Drop should fail while reference exists ok 67 - DROP table "test table" ok 68 - object_group_ids still has correct record count ok 69 - No object references remain -ok 70 - object_reference schema(s) must not be part of the resolved search_path -- got {object_identity_temp_test_schema,test_support,tap,public} +ok 70 - object_reference schema(s) must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/event_trigger.out b/test/expected/event_trigger.out index 9969fc3..2e80d1c 100644 --- a/test/expected/event_trigger.out +++ b/test/expected/event_trigger.out @@ -42,5 +42,5 @@ ok 39 - Verify filler column record is deleted ok 40 - Verify objects still registered correctly ok 41 - Drop schema ok 42 - objects_view is empty -ok 43 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} +ok 43 - object_reference schema(s) must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/object_group.out b/test/expected/object_group.out index c58785b..831821f 100644 --- a/test/expected/object_group.out +++ b/test/expected/object_group.out @@ -29,5 +29,5 @@ ok 26 - Object exists before cleanup test ok 27 - Remove from group triggers automatic cleanup attempt ok 28 - Object was automatically cleaned up after group removal ok 29 - Removing empty group works -ok 30 - object_reference schema(s) must not be part of the resolved search_path -- got {tap,public} +ok 30 - object_reference schema(s) must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/finish.sql b/test/finish.sql index f9b1110..85401c4 100644 --- a/test/finish.sql +++ b/test/finish.sql @@ -15,10 +15,7 @@ */ SELECT ok( NOT ( 'object_reference' = ANY (current_schemas(false)) OR '_object_reference' = ANY (current_schemas(false)) ) - , format( - 'object_reference schema(s) must not be part of the resolved search_path -- got %s' - , current_schemas(false) - ) + , 'object_reference schema(s) must not be part of the resolved search_path' ); \i test/pgxntool/finish.sql From 090e5c177773b767c3e4fe1ad4ffd19b7c96942d Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 11 Aug 2026 13:12:40 -0500 Subject: [PATCH 10/13] test/build: rename zzz_build to build The zzz_ prefix only existed to force this test to run last alphabetically; pgxntool's test-build feature now runs it as its own separate installcheck target, so ordering no longer applies. --- .github/workflows/ci.yml | 2 +- test/build/{zzz_build.sql => build.sql} | 0 test/build/expected/{zzz_build.out => build.out} | 0 3 files changed, 1 insertion(+), 1 deletion(-) rename test/build/{zzz_build.sql => build.sql} (100%) rename test/build/expected/{zzz_build.out => build.out} (100%) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 24a28dc..064462c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -129,7 +129,7 @@ jobs: run: pg-start ${{ matrix.pg }} - name: Check out the repo uses: actions/checkout@v4 - # pgxntool's test-build feature (test/build/zzz_build.sql) syncs + # pgxntool's test-build feature (test/build/build.sql) syncs # test/build/*.sql into test/build/sql/ via rsync (run-test-build.sh), # which the pgxn/pgxn-tools image doesn't ship. - name: Install rsync diff --git a/test/build/zzz_build.sql b/test/build/build.sql similarity index 100% rename from test/build/zzz_build.sql rename to test/build/build.sql diff --git a/test/build/expected/zzz_build.out b/test/build/expected/build.out similarity index 100% rename from test/build/expected/zzz_build.out rename to test/build/expected/build.out From dbd3150bd8cf35988699fe41d3812a3398a4a88c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 11 Aug 2026 13:37:39 -0500 Subject: [PATCH 11/13] test/finish.sql: use set_hasnt() instead of a hand-rolled ok() pgTAP's set_hasnt() asserts a set has none of a given set of values, which is exactly what the search_path check needs, and reports which schema leaked into search_path on failure instead of just failing. Split into two assertions, one per schema, so a failure names the specific schema. --- test/expected/_object_v.out | 5 +++-- test/expected/all.out | 5 +++-- test/expected/base.out | 5 +++-- test/expected/capture.out | 5 +++-- test/expected/event_trigger.out | 5 +++-- test/expected/object_group.out | 5 +++-- test/finish.sql | 15 +++++++++++---- test/sql/_object_v.sql | 2 +- test/sql/all.sql | 2 +- test/sql/base.sql | 2 +- test/sql/capture.sql | 2 +- test/sql/event_trigger.sql | 2 +- test/sql/object_group.sql | 2 +- 13 files changed, 35 insertions(+), 22 deletions(-) diff --git a/test/expected/_object_v.out b/test/expected/_object_v.out index 1f2bcf5..ed1efc6 100644 --- a/test/expected/_object_v.out +++ b/test/expected/_object_v.out @@ -1,5 +1,6 @@ \set ECHO none -1..2 +1..3 ok 1 - _object_v__for_update matches _object_v -ok 2 - object_reference schema(s) must not be part of the resolved search_path +ok 2 - object_reference schema must not be part of the resolved search_path +ok 3 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/all.out b/test/expected/all.out index 0a4e41b..254b2d9 100644 --- a/test/expected/all.out +++ b/test/expected/all.out @@ -1,5 +1,5 @@ \set ECHO none -1..74 +1..75 ok 1 - All object types are being tested. ok 2 - Verify object_reference.unsupported() ok 3 - prereq: CREATE DOMAIN "test domain" int @@ -73,5 +73,6 @@ ok 70 - DROP index "test table test index" ok 71 - Drop should fail while reference exists ok 72 - DROP table "test table" ok 73 - No object references remain -ok 74 - object_reference schema(s) must not be part of the resolved search_path +ok 74 - object_reference schema must not be part of the resolved search_path +ok 75 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/base.out b/test/expected/base.out index 8369c9e..d5180ba 100644 --- a/test/expected/base.out +++ b/test/expected/base.out @@ -1,5 +1,5 @@ \set ECHO none -1..11 +1..12 ok 1 - Role object_reference__dependency should be granted USAGE on schema _object_reference ok 2 - Role object_reference__dependency should be granted REFERENCES on table _object_reference.object ok 3 - CREATE TEMP TABLE test_object AS SELECT object_reference.object__getsert('table', 'test_table') AS object_id; @@ -10,5 +10,6 @@ ok 7 - Existing object works, provides correct ID ok 8 - secondary may not be specified for table objects ok 9 - temp objects are rejected ok 10 - CREATE EXTENSION test_factory -ok 11 - object_reference schema(s) must not be part of the resolved search_path +ok 11 - object_reference schema must not be part of the resolved search_path +ok 12 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/capture.out b/test/expected/capture.out index c3e43b1..a14dd56 100644 --- a/test/expected/capture.out +++ b/test/expected/capture.out @@ -1,5 +1,5 @@ \set ECHO none -1..70 +1..71 ok 1 - prereq: CREATE DOMAIN "test domain" int ok 2 - prereq: CREATE FUNCTION tg_null() RETURNS trigger LANGUAGE plpgsql AS $body$BEGIN RETURN NEW; END$body$ ok 3 - prereq: CREATE TYPE "test type" @@ -69,5 +69,6 @@ ok 66 - Drop should fail while reference exists ok 67 - DROP table "test table" ok 68 - object_group_ids still has correct record count ok 69 - No object references remain -ok 70 - object_reference schema(s) must not be part of the resolved search_path +ok 70 - object_reference schema must not be part of the resolved search_path +ok 71 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/event_trigger.out b/test/expected/event_trigger.out index 2e80d1c..43f4845 100644 --- a/test/expected/event_trigger.out +++ b/test/expected/event_trigger.out @@ -1,5 +1,5 @@ \set ECHO none -1..43 +1..44 ok 1 - Register schema-drop test objects ok 2 - Create objects_view ok 3 - Exactly 3 test view records @@ -42,5 +42,6 @@ ok 39 - Verify filler column record is deleted ok 40 - Verify objects still registered correctly ok 41 - Drop schema ok 42 - objects_view is empty -ok 43 - object_reference schema(s) must not be part of the resolved search_path +ok 43 - object_reference schema must not be part of the resolved search_path +ok 44 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/object_group.out b/test/expected/object_group.out index 831821f..2584ebb 100644 --- a/test/expected/object_group.out +++ b/test/expected/object_group.out @@ -1,5 +1,5 @@ \set ECHO none -1..30 +1..31 ok 1 - Register test table 1 ok 2 - object_group__create(...) for group name that is too long throws error ok 3 - object_group__create('object reference test group') @@ -29,5 +29,6 @@ ok 26 - Object exists before cleanup test ok 27 - Remove from group triggers automatic cleanup attempt ok 28 - Object was automatically cleaned up after group removal ok 29 - Removing empty group works -ok 30 - object_reference schema(s) must not be part of the resolved search_path +ok 30 - object_reference schema must not be part of the resolved search_path +ok 31 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/finish.sql b/test/finish.sql index 85401c4..1861c8b 100644 --- a/test/finish.sql +++ b/test/finish.sql @@ -1,5 +1,5 @@ /* - * Asserts object_reference's own schema(s) are absent from the resolved + * Asserts object_reference's own schemas are absent from the resolved * search_path -- checked here (file end, before finish()) rather than only * at setup, so a test that mutates search_path mid-file and never restores * it is caught. Not foolproof: mutate-then-restore before this line still @@ -13,9 +13,16 @@ * test in the suite passing means nothing accidentally relied on * search_path to resolve one of the extension's own objects. */ -SELECT ok( - NOT ( 'object_reference' = ANY (current_schemas(false)) OR '_object_reference' = ANY (current_schemas(false)) ) - , 'object_reference schema(s) must not be part of the resolved search_path' +SELECT set_hasnt( + $$ SELECT unnest(current_schemas(false)) $$ + , $$ VALUES ('object_reference') $$ + , 'object_reference schema must not be part of the resolved search_path' +); + +SELECT set_hasnt( + $$ SELECT unnest(current_schemas(false)) $$ + , $$ VALUES ('_object_reference') $$ + , '_object_reference schema must not be part of the resolved search_path' ); \i test/pgxntool/finish.sql diff --git a/test/sql/_object_v.sql b/test/sql/_object_v.sql index 10a57b9..a641b85 100644 --- a/test/sql/_object_v.sql +++ b/test/sql/_object_v.sql @@ -6,7 +6,7 @@ SELECT plan( 0 + 1 -- equality - + 1 -- schema-qualification (search_path) + + 2 -- schema-qualification (search_path) ); -- TODO: load some damn data first diff --git a/test/sql/all.sql b/test/sql/all.sql index eea700c..f273039 100644 --- a/test/sql/all.sql +++ b/test/sql/all.sql @@ -28,7 +28,7 @@ SELECT plan( ( + c * 2 -- drop + 1 -- verify object table is now empty - + 1 -- schema-qualification (search_path) + + 2 -- schema-qualification (search_path) )::int ) FROM (SELECT count(*) c FROM test_object) c ; diff --git a/test/sql/base.sql b/test/sql/base.sql index 3bcaaf1..538f4f8 100644 --- a/test/sql/base.sql +++ b/test/sql/base.sql @@ -11,7 +11,7 @@ SELECT plan( +2 -- new functions +3 -- errors (includes temp object test) +1 -- create extensions - +1 -- schema-qualification (search_path) + +2 -- schema-qualification (search_path) ); -- Schema diff --git a/test/sql/capture.sql b/test/sql/capture.sql index bbfba64..2d7c2d2 100644 --- a/test/sql/capture.sql +++ b/test/sql/capture.sql @@ -49,7 +49,7 @@ SELECT plan( ( + cna * 2 -- Drop objects + 1 -- Verify object_group_ids still has correct count + 1 -- verify object table is now empty - + 1 -- schema-qualification (search_path) + + 2 -- schema-qualification (search_path) )::int ) FROM (SELECT count(*) c, count(CASE WHEN create_command NOT LIKE 'ALTER%' THEN 1 END) AS cna FROM test_object) c diff --git a/test/sql/event_trigger.sql b/test/sql/event_trigger.sql index cb2b24d..11e2128 100644 --- a/test/sql/event_trigger.sql +++ b/test/sql/event_trigger.sql @@ -21,7 +21,7 @@ SELECT plan( +2 + 1 -- column drop +3 -- table drop +3 -- schema drop - +1 -- schema-qualification (search_path) + +2 -- schema-qualification (search_path) ); SELECT lives_ok( diff --git a/test/sql/object_group.sql b/test/sql/object_group.sql index 1e5ee54..68c9b9f 100644 --- a/test/sql/object_group.sql +++ b/test/sql/object_group.sql @@ -42,7 +42,7 @@ SELECT plan( +3 + 2 -- __remove +4 -- cleanup tests +1 -- final group removal (there was always an extra test) - +1 -- schema-qualification (search_path) + +2 -- schema-qualification (search_path) ); SELECT lives_ok( From 1612605fa94f683c0cb6294eda0ef033ba09ed3f Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 11 Aug 2026 14:36:54 -0500 Subject: [PATCH 12/13] sql/object_reference--0.1.0--stable.sql: trim header, use session_replication_role The header comment re-derived this file's whole change history from a diff instead of just orienting a reader; cut it down to the one thing worth knowing up front, the __object_reference helper schema. Event triggers obey session_replication_role the same way row/statement triggers do (they're created ENABLE, i.e. origin-scoped, by default), so SET LOCAL session_replication_role = replica for the structural section replaces three explicit ALTER EVENT TRIGGER ... DISABLE statements and needs no matching re-enable: it reverts automatically at the end of the transaction this whole script runs in, covering the final helper-schema cleanup too instead of racing to re-enable before it. --- sql/object_reference--0.1.0--stable.sql | 92 +++++-------------------- 1 file changed, 17 insertions(+), 75 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index a3508e2..4e10e4b 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -1,55 +1,8 @@ /* - * Hand-authored update script: 0.1.0 (the only real historical PGXN release) - * -> stable (the current build). Never auto-generated and never touched by - * the control.mk rule that (re)builds sql/object_reference--stable.sql from - * sql/object_reference.sql -- see this repo's CLAUDE.md / memory: versioned - * SQL files are frozen, only sql/object_reference.sql (the source) is edited. - * - * Every delta below was found by diffing sql/object_reference--0.1.0.sql - * against sql/object_reference.sql in full (not by memory of the individual - * commits that produced them): - * - count_nulls dropped as a dependency (object_reference.control's - * `requires` no longer lists it); the _object_oid.null_count trigger, - * which called count_nulls' not_null_count_trigger(), is removed. - * - The reg* pseudotype columns on _object_reference._object_oid - * (regclass/regconfig/regdictionary/regnamespace/regoperator/ - * regprocedure/regtype), plus the now-redundant object_oid column they - * used to collapse into (it always equaled objid once there was only - * one oid source left), are all dropped; classid changes from regclass - * to oid. - * - _object_reference._object_v / _object_v__for_update (views) drop the - * now-gone reg* columns. CREATE OR REPLACE VIEW cannot drop columns, so - * both are DROP+CREATE'd, along with the two functions whose RETURNS - * type is _object_reference._object_v (a formal pg_depend edge, not just - * a body reference) -- see the "Views + dependent functions" section - * below. - * - New functions: object_reference.object__describe(), - * object__identity(), object__cleanup(), plus a trigger that calls the - * latter to auto-clean orphaned objects when removed from a group. - * - object__getsert's underlying _object_v__for_update() now refuses to - * track objects in temporary schemas (pg_temp%/pg_toast_temp%). - * - object_reference.unsupported() additionally excludes "partitioned - * table"/"partitioned index" (pg_get_object_address() doesn't recognize - * them). - * - _tg_capture_safety() gains a trailing RETURN NULL (a trigger function - * with a declared return type must return something in every branch). - * - Two new utility event-trigger functions, etg_raise__start/__drop, are - * added (not wired to any CREATE EVENT TRIGGER -- example/debug use - * only, matching the commented-out "snitch" example 0.1.0 had instead). - * - _object_oid__add's own bug: 0.1.0 selected `a.subobjid` from - * pg_get_object_address(), which only ever has an `objsubid` column - * (SQLSTATE 42703 if that branch were ever hit) -- fixed here as part of - * recreating the function with its current body. - * - * Uses the same private-helper-schema bootstrap/teardown convention as - * sql/object_reference.sql's own fresh install (__object_reference.exec / - * safe_dump / create_function), so every function recreated here goes - * through the exact same REVOKE ALL FROM PUBLIC / GRANT / COMMENT template a - * fresh install uses -- not hand-written DROP FUNCTION + CREATE FUNCTION + - * REVOKE/GRANT/COMMENT, which would risk silently diverging from what a - * fresh install actually produces (a real, verified-clean structural - * comparison of every recreated function/view against a fresh install of the - * current version backs this file -- see the containing PR's description). + * Uses a private __object_reference schema, mirroring + * sql/object_reference.sql's own bootstrap/teardown convention, so every + * function recreated here goes through the same REVOKE ALL FROM PUBLIC / + * GRANT / COMMENT template a fresh install uses. */ CREATE SCHEMA __object_reference; @@ -152,22 +105,20 @@ END $body$; /* - * 0.1.0 already installed this extension's own event triggers (they fire on - * every sql_drop / ddl_command_end in the session, not just DDL a normal user - * issues), and they stay active for the rest of THIS session while the - * structural changes below run. zzz__object_reference_drop in particular - * queries _object_reference._object_v inside its own body, so it would fire - * -- and error, since the view is momentarily gone -- the instant this script - * drops that view a few statements down. Disable all three for the structural - * portion of this script and re-enable them right before the private helper - * schema teardown, once every object they might touch exists again in its - * final, current-source shape. A fresh install never hits this: it creates - * these event triggers only at the very end, once nothing they reference is - * still being modified. + * 0.1.0 already installed this extension's own event triggers, and they + * stay active for the rest of THIS session while the structural changes + * below run. zzz__object_reference_drop in particular queries + * _object_reference._object_v inside its own body, so it would fire -- and + * error, since the view is momentarily gone -- the instant this script drops + * that view a few statements down. All three are default-enabled (origin), + * so setting session_replication_role = replica suppresses them for the + * rest of this transaction -- reverting automatically once the update + * completes, with no explicit re-enable needed even across the cleanup at + * the end of this script. A fresh install never hits this: it creates these + * event triggers only at the very end, once nothing they reference is still + * being modified. */ -ALTER EVENT TRIGGER zzz__object_reference_drop DISABLE; -ALTER EVENT TRIGGER zzz_object_reference__fix_identity DISABLE; -ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; +SET LOCAL session_replication_role = replica; /* * _object_reference.object: no column changes, just a missing @@ -695,15 +646,6 @@ $body$ , 'object_reference__usage' ); -/* - * Re-enable the event triggers disabled near the top of this script, now - * that every object they reference is back in its final, current-source - * shape. - */ -ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE; -ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE; -ALTER EVENT TRIGGER zzz_object_reference_capture ENABLE; - /* * Drop "temporary" objects -- same convention as the fresh install script. */ From 93a1e9fa77227d19f6954d09de2c56c209c0c0a7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 11 Aug 2026 14:50:39 -0500 Subject: [PATCH 13/13] sql/object_reference--0.1.0--stable.sql: save/restore session_replication_role explicitly A SET LOCAL inside an extension update script is not scoped to the script itself: it persists into the rest of the caller's transaction if ALTER EXTENSION UPDATE isn't the only statement in it (confirmed against a manual test with an explicit surrounding transaction). Assuming the prior value was 'origin' and relying on the transaction to revert it was therefore wrong on two counts. Save the actual prior value in a placeholder GUC before disabling, and restore that exact value explicitly once the structural section and its cleanup are done. --- sql/object_reference--0.1.0--stable.sql | 29 ++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 4e10e4b..b965953 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -112,12 +112,24 @@ $body$; * error, since the view is momentarily gone -- the instant this script drops * that view a few statements down. All three are default-enabled (origin), * so setting session_replication_role = replica suppresses them for the - * rest of this transaction -- reverting automatically once the update - * completes, with no explicit re-enable needed even across the cleanup at - * the end of this script. A fresh install never hits this: it creates these - * event triggers only at the very end, once nothing they reference is still - * being modified. + * structural section below. + * + * This script is not necessarily the only thing running in its transaction + * -- ALTER EXTENSION UPDATE can be issued as one statement among several in + * a caller-managed transaction -- so session_replication_role cannot simply + * be left disturbed for "the rest of the transaction" to sort out, and + * whatever it's restored to afterward must be the caller's actual prior + * value, not an assumed 'origin' default (the caller may already have it set + * to something else for their own reasons). Stashed in a placeholder GUC + * (there's no other way to carry a value between separate top-level + * statements in a plain multi-statement SQL script -- this isn't a single + * PL/pgSQL block) and restored explicitly right after the cleanup at the end + * of this script, once every object the event triggers reference is back in + * its final, current-source shape. A fresh install never hits this: it + * creates these event triggers only at the very end, once nothing they + * reference is still being modified. */ +SELECT set_config('object_reference.saved_session_replication_role', current_setting('session_replication_role'), true); SET LOCAL session_replication_role = replica; /* @@ -666,4 +678,11 @@ DROP FUNCTION __object_reference.exec( ); DROP SCHEMA __object_reference; +/* + * Restore session_replication_role to the caller's actual prior value + * (saved near the top of this script), now that the structural section and + * its cleanup are both done. + */ +SELECT set_config('session_replication_role', current_setting('object_reference.saved_session_replication_role'), true); + -- vi: expandtab sw=2 ts=2