From 039241dba8a44920580f2c5c752c18a12c5398fb Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 01:48:53 -0500 Subject: [PATCH 01/35] Refuse to track objects that are members of object_reference itself `object__getsert()` (via `_object_v__for_update()`) now raises feature_not_supported for any object belonging to the object_reference extension. Letting the tracking machinery observe its own members means the extension's own DDL -- including its update scripts restructuring the extension -- becomes something that machinery reacts to. `_object_reference._is_own_object()` checks `pg_depend` membership (`deptype = 'e'`), plus the two objects that are never 'e' members: the extension's declared schema and its own `pg_extension` row. `_etg_capture` also skips objects in the object_reference, _object_reference and __object_reference schemas: an object created by the extension's own install/update script isn't recorded as an 'e' member until the surrounding CREATE/ALTER EXTENSION finishes, so `_is_own_object()` can't see it yet when ddl_command_end fires. The 0.1.0 -> stable update script creates `_is_own_object()` and the updated `_etg_capture` inside its existing session_replication_role section. Part of #40 --- sql/object_reference--0.1.0--stable.sql | 130 +++++++++++++++++++++++- sql/object_reference.sql | 76 +++++++++++++- test/build/expected/build.out | 11 +- test/expected/base.out | 13 ++- test/sql/base.sql | 43 +++++++- 5 files changed, 260 insertions(+), 13 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index b965953..1923bd5 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -132,6 +132,126 @@ $body$; SELECT set_config('object_reference.saved_session_replication_role', current_setting('session_replication_role'), true); SET LOCAL session_replication_role = replica; +/* + * New: refuse to track objects that are themselves members of the + * object_reference extension (see the guard added to + * _object_v__for_update() below). + */ +SELECT __object_reference.create_function( + '_object_reference._is_own_object' + , $args$ + classid oid + , objid oid +$args$ + , 'boolean LANGUAGE sql STABLE' + , $body$ +SELECT + EXISTS( + SELECT 1 + FROM pg_catalog.pg_depend d + WHERE d.classid = _is_own_object.classid + AND d.objid = _is_own_object.objid + AND d.deptype = 'e' + AND d.refclassid = 'pg_catalog.pg_extension'::regclass + AND d.refobjid = e.oid + ) + /* + * The extension's own declared schema (object_reference) is a special + * case: CREATE EXTENSION records the EXTENSION as depending on it (a + * plain DEPENDENCY_NORMAL row, extension -> schema), not the schema as + * an 'e' member of the extension the way every other object it creates + * is -- so it never matches the pg_depend check above. + */ + OR (_is_own_object.classid = 'pg_catalog.pg_namespace'::regclass AND _is_own_object.objid = e.extnamespace) + /* + * The extension's own pg_extension row is also its own special case: it + * isn't a member of itself (no 'e' row with itself as both member and + * owner), so treat it as one explicitly. + */ + OR (_is_own_object.classid = 'pg_catalog.pg_extension'::regclass AND _is_own_object.objid = e.oid) +FROM (SELECT oid, extnamespace FROM pg_catalog.pg_extension WHERE extname = 'object_reference') e +$body$ + , 'Is the object a member of the object_reference extension itself? (pg_depend deptype = e membership, not just co-installation.)' +); + +/* + * _etg_capture: never capture objects in this extension's own schemas + * (see the comment in its body). Same signature as 0.1.0, so a plain + * CREATE OR REPLACE (via create_function) is enough. + */ +SELECT __object_reference.create_function( + '_object_reference._etg_capture' + , '' + , 'event_trigger SECURITY DEFINER LANGUAGE plpgsql' + , $body$ +DECLARE + c_group_id CONSTANT int := object_group_id FROM object_reference.capture__get_current(); + r record; +BEGIN + + IF c_group_id IS NOT NULL THEN -- Would be NULL if table is empty + RAISE DEBUG E'\n\n*** START ***'; + BEGIN + FOR r IN + SELECT classid, objid, objsubid, command_tag, object_type, schema_name, object_identity, in_extension + -- Have to manually exclude command field :/ + FROM pg_catalog.pg_event_trigger_ddl_commands() + LOOP + RAISE DEBUG 'ddl: %', row_to_json(r); + END LOOP; + END; + + FOR r IN SELECT + _object_reference._object_v__for_update( + object_type::cat_tools.object_type + , objid, objsubid + , c_group_id + , classid + ) + , classid, objid, objsubid, command_tag, object_type, schema_name, object_identity, in_extension + FROM pg_catalog.pg_event_trigger_ddl_commands() + WHERE command_tag ~ '^CREATE' --'^(ALTER|CREATE)' + AND NOT object_reference.unsupported(object_type::cat_tools.object_type) + AND (schema_name IS NULL + OR schema_name NOT LIKE 'pg_temp%' -- pg_my_temp_schema() doesn't seem worth it... + ) + /* + * __object_reference is this extension's own scratch install/update + * schema (created and dropped within a single script, never an + * extension member), so _is_own_object() never recognizes it -- + * exclude it here instead. + * + * object_reference/_object_reference are excluded outright rather + * than relying on _object_v__for_update()'s own _is_own_object() + * guard: a brand-new object created by this extension's own + * update/install script isn't yet recorded as an 'e' member in + * pg_depend at the point its CREATE fires ddl_command_end (that + * happens once the surrounding CREATE/ALTER EXTENSION completes), + * so _is_own_object() can't see it as self-owned yet either -- + * confirmed by running into it: an active capture group during + * ALTER EXTENSION UPDATE otherwise ends up with this extension's + * own new functions as members. + * + * The schema-creation statement itself (CREATE SCHEMA + * __object_reference/etc.) has schema_name = NULL, with the name + * only available via object_identity -- checked separately, and + * restricted to object_type = 'schema', so an unrelated object of + * some other type whose identity happens to match one of these + * three exact strings (e.g. a same-named extension) isn't caught + * by this fallback. + */ + AND coalesce(schema_name, '') NOT IN ('__object_reference', 'object_reference', '_object_reference') + AND NOT (object_type = 'schema' AND object_identity IN ('__object_reference', 'object_reference', '_object_reference')) + LOOP + RAISE DEBUG 'registered %', row_to_json(r); + END LOOP; + RAISE DEBUG E'*** END ***\n\n'; + END IF; +END +$body$ + , 'Event trigger function to capture newly created objects in an object group.' +); + /* * _object_reference.object: no column changes, just a missing * extension_config_dump marking on its sequence (added alongside the table @@ -293,6 +413,14 @@ BEGIN ; END IF; + -- Refuse to track objects that are themselves members of this extension + IF _object_reference._is_own_object(c_classid, objid) THEN + RAISE 'cannot track an object that is a member of the object_reference extension itself' + USING DETAIL = format('object %s belongs to the object_reference extension', r_identity.identity) + , ERRCODE = 'feature_not_supported' + ; + END IF; + -- Ensure the object record exists SELECT INTO r_object_v * @@ -365,7 +493,7 @@ BEGIN RETURN r_object_v; END $body$ - , 'Return details of a object record, creating a new record if one does not exist.' + , 'Return details of a object record, creating a new record if one does not exist. Heavy-weight compared to a plain read of _object_reference._object_v -- use that instead when an existing record is all that''s needed.' ); SELECT __object_reference.create_function( diff --git a/sql/object_reference.sql b/sql/object_reference.sql index cbed4f5..e7e00e9 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -161,6 +161,43 @@ $body$ , 'Execute arbitrary SQL with logging.' ); +SELECT __object_reference.create_function( + '_object_reference._is_own_object' + , $args$ + classid oid + , objid oid +$args$ + , 'boolean LANGUAGE sql STABLE' + , $body$ +SELECT + EXISTS( + SELECT 1 + FROM pg_catalog.pg_depend d + WHERE d.classid = _is_own_object.classid + AND d.objid = _is_own_object.objid + AND d.deptype = 'e' + AND d.refclassid = 'pg_catalog.pg_extension'::regclass + AND d.refobjid = e.oid + ) + /* + * The extension's own declared schema (object_reference) is a special + * case: CREATE EXTENSION records the EXTENSION as depending on it (a + * plain DEPENDENCY_NORMAL row, extension -> schema), not the schema as + * an 'e' member of the extension the way every other object it creates + * is -- so it never matches the pg_depend check above. + */ + OR (_is_own_object.classid = 'pg_catalog.pg_namespace'::regclass AND _is_own_object.objid = e.extnamespace) + /* + * The extension's own pg_extension row is also its own special case: it + * isn't a member of itself (no 'e' row with itself as both member and + * owner), so treat it as one explicitly. + */ + OR (_is_own_object.classid = 'pg_catalog.pg_extension'::regclass AND _is_own_object.objid = e.oid) +FROM (SELECT oid, extnamespace FROM pg_catalog.pg_extension WHERE extname = 'object_reference') e +$body$ + , 'Is the object a member of the object_reference extension itself? (pg_depend deptype = e membership, not just co-installation.)' +); + CREATE TABLE _object_reference.object( object_id serial PRIMARY KEY , object_type cat_tools.object_type NOT NULL @@ -886,6 +923,14 @@ BEGIN ; END IF; + -- Refuse to track objects that are themselves members of this extension + IF _object_reference._is_own_object(c_classid, objid) THEN + RAISE 'cannot track an object that is a member of the object_reference extension itself' + USING DETAIL = format('object %s belongs to the object_reference extension', r_identity.identity) + , ERRCODE = 'feature_not_supported' + ; + END IF; + -- Ensure the object record exists SELECT INTO r_object_v * @@ -958,7 +1003,7 @@ BEGIN RETURN r_object_v; END $body$ - , 'Return details of a object record, creating a new record if one does not exist.' + , 'Return details of a object record, creating a new record if one does not exist. Heavy-weight compared to a plain read of _object_reference._object_v -- use that instead when an existing record is all that''s needed.' ); SELECT __object_reference.create_function( @@ -1397,7 +1442,7 @@ BEGIN END LOOP; END; - FOR r IN SELECT + FOR r IN SELECT _object_reference._object_v__for_update( object_type::cat_tools.object_type , objid, objsubid @@ -1411,6 +1456,33 @@ BEGIN AND (schema_name IS NULL OR schema_name NOT LIKE 'pg_temp%' -- pg_my_temp_schema() doesn't seem worth it... ) + /* + * __object_reference is this extension's own scratch install/update + * schema (created and dropped within a single script, never an + * extension member), so _is_own_object() never recognizes it -- + * exclude it here instead. + * + * object_reference/_object_reference are excluded outright rather + * than relying on _object_v__for_update()'s own _is_own_object() + * guard: a brand-new object created by this extension's own + * update/install script isn't yet recorded as an 'e' member in + * pg_depend at the point its CREATE fires ddl_command_end (that + * happens once the surrounding CREATE/ALTER EXTENSION completes), + * so _is_own_object() can't see it as self-owned yet either -- + * confirmed by running into it: an active capture group during + * ALTER EXTENSION UPDATE otherwise ends up with this extension's + * own new functions as members. + * + * The schema-creation statement itself (CREATE SCHEMA + * __object_reference/etc.) has schema_name = NULL, with the name + * only available via object_identity -- checked separately, and + * restricted to object_type = 'schema', so an unrelated object of + * some other type whose identity happens to match one of these + * three exact strings (e.g. a same-named extension) isn't caught + * by this fallback. + */ + AND coalesce(schema_name, '') NOT IN ('__object_reference', 'object_reference', '_object_reference') + AND NOT (object_type = 'schema' AND object_identity IN ('__object_reference', 'object_reference', '_object_reference')) LOOP RAISE DEBUG 'registered %', row_to_json(r); END LOOP; diff --git a/test/build/expected/build.out b/test/build/expected/build.out index fadaba6..21ac942 100644 --- a/test/build/expected/build.out +++ b/test/build/expected/build.out @@ -2,17 +2,17 @@ This extension must be loaded via CREATE EXTENSION object_reference; You really, REALLY do NOT want to try and load this via psql!!! -psql:test/temp_load.not_sql:176: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! -psql:test/temp_load.not_sql:177: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! +psql:test/temp_load.not_sql:213: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! +psql:test/temp_load.not_sql:214: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! -psql:test/temp_load.not_sql:425: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! +psql:test/temp_load.not_sql:462: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! @@ -21,9 +21,10 @@ psql:test/temp_load.not_sql:425: WARNING: I promise you will be sorry if you tr -psql:test/temp_load.not_sql:537: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! -psql:test/temp_load.not_sql:544: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! +psql:test/temp_load.not_sql:574: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! + +psql:test/temp_load.not_sql:581: WARNING: I promise you will be sorry if you try to use this as anything other than an extension! diff --git a/test/expected/base.out b/test/expected/base.out index d5180ba..b2054ef 100644 --- a/test/expected/base.out +++ b/test/expected/base.out @@ -1,5 +1,5 @@ \set ECHO none -1..12 +1..17 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; @@ -9,7 +9,12 @@ ok 6 - object__identity returns same result as pg_identify_object 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 must not be part of the resolved search_path -ok 12 - _object_reference schema must not be part of the resolved search_path +ok 10 - own tracking table is rejected +ok 11 - own event trigger function is rejected +ok 12 - own declared schema is rejected (extension depends on it, not the other way around) +ok 13 - own private schema is rejected (an ordinary 'e' pg_depend member, unlike the declared schema above) +ok 14 - _is_own_object() recognizes its own pg_extension row +ok 15 - CREATE EXTENSION test_factory +ok 16 - object_reference schema must not be part of the resolved search_path +ok 17 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/base.sql b/test/sql/base.sql index 538f4f8..2f5bce1 100644 --- a/test/sql/base.sql +++ b/test/sql/base.sql @@ -9,7 +9,7 @@ SELECT plan( +1 -- schema +3 -- initial +2 -- new functions - +3 -- errors (includes temp object test) + +8 -- errors (includes temp object + self-tracking rejection tests) +1 -- create extensions +2 -- schema-qualification (search_path) ); @@ -74,6 +74,47 @@ SELECT throws_ok( , 'temp objects are rejected' ); +-- Test rejection of object_reference's own extension-member objects +SELECT throws_ok( + $$SELECT object_reference.object__getsert('table', '_object_reference.object')$$ + , '0A000' -- feature_not_supported + , 'cannot track an object that is a member of the object_reference extension itself' + , 'own tracking table is rejected' +); +SELECT throws_ok( + $$SELECT object_reference.object__getsert('function', '_object_reference._etg_drop', '')$$ + , '0A000' -- feature_not_supported + , 'cannot track an object that is a member of the object_reference extension itself' + , 'own event trigger function is rejected' +); +SELECT throws_ok( + $$SELECT object_reference.object__getsert('schema', 'object_reference')$$ + , '0A000' -- feature_not_supported + , 'cannot track an object that is a member of the object_reference extension itself' + , 'own declared schema is rejected (extension depends on it, not the other way around)' +); +SELECT throws_ok( + $$SELECT object_reference.object__getsert('schema', '_object_reference')$$ + , '0A000' -- feature_not_supported + , 'cannot track an object that is a member of the object_reference extension itself' + , 'own private schema is rejected (an ordinary ''e'' pg_depend member, unlike the declared schema above)' +); +/* + * Exercised directly against _is_own_object() rather than through + * object__getsert('extension', ...): the latter's generic by-name OID + * lookup for object types with no reg-type cast (extension included) + * derives the wrong catalog column name and fails before ever reaching + * this check -- a pre-existing, unrelated bug (see + * object__getsert_w_group_id's v_name_field derivation, predating this PR). + */ +SELECT ok( + _object_reference._is_own_object( + 'pg_catalog.pg_extension'::regclass + , (SELECT oid FROM pg_catalog.pg_extension WHERE extname = 'object_reference') + ) + , '_is_own_object() recognizes its own pg_extension row' +); + -- Create extensions SELECT lives_ok( $$CREATE EXTENSION test_factory$$ From ad53bec60369a05360377e76f5a46d74c045e04b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 01:49:16 -0500 Subject: [PATCH 02/35] Replace session_replication_role with event_trigger__disable()/__enable() `_object_reference.event_trigger__disable(name[])` disables the given event triggers and records each one's prior `evtenabled` value in a temp table; `event_trigger__enable()` restores exactly that state. ALTER EVENT TRIGGER is transactional, so a rolled-back caller undoes the DISABLE without separate cleanup. `_etg_capture` and `_etg_fix_identity` return early while that temp table exists in the current session, so they never need disabling. `_etg_drop` can't recognize the extension's own update script that way, so the 0.1.0 -> stable update script disables it with `event_trigger__disable()` around its structural section, replacing the `session_replication_role = replica` bracket. 0.1.0's `_etg_capture`/`_etg_fix_identity` have no such guard, so the update script disables both with raw ALTER EVENT TRIGGER for the bootstrap statements that run before their guarded replacements are installed, then restores each one's saved prior state. Closes #40 --- sql/object_reference--0.1.0--stable.sql | 324 +++++++++++++++++++++--- sql/object_reference.sql | 173 ++++++++++++- test/build/expected/build.out | 2 + test/expected/event_trigger_disable.out | 23 ++ test/sql/event_trigger_disable.sql | 142 +++++++++++ 5 files changed, 625 insertions(+), 39 deletions(-) create mode 100644 test/expected/event_trigger_disable.out create mode 100644 test/sql/event_trigger_disable.sql diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 1923bd5..40b9951 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -1,3 +1,26 @@ +/* + * Immediately disable zzz_object_reference_capture and + * zzz_object_reference__fix_identity (raw ALTER EVENT TRIGGER -- nothing + * else, not even the __object_reference bootstrap schema below, exists yet + * to route through) for the handful of bootstrap statements that follow: + * their OLD (0.1.0) bodies have no way to recognize "this is our own + * update script" until their guarded replacements are installed a few + * statements down, and CREATE SCHEMA __object_reference is exactly the + * kind of CREATE-tagged statement _etg_capture would otherwise try (and + * fail) to register into any capture group active during this update. + * Their actual prior state is captured into a temp table (rather than + * assumed 'O') and restored below, so a DBA's own prior DISABLE of either + * trigger survives this update instead of being silently overwritten. + */ +CREATE TEMP TABLE __object_reference__bootstrap_event_trigger_state AS + SELECT evtname, evtenabled + FROM pg_catalog.pg_event_trigger + WHERE evtname IN ('zzz_object_reference_capture', 'zzz_object_reference__fix_identity') +; + +ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; +ALTER EVENT TRIGGER zzz_object_reference__fix_identity DISABLE; + /* * Uses a private __object_reference schema, mirroring * sql/object_reference.sql's own bootstrap/teardown convention, so every @@ -104,34 +127,6 @@ BEGIN END $body$; -/* - * 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 - * 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; - /* * New: refuse to track objects that are themselves members of the * object_reference extension (see the guard added to @@ -175,9 +170,22 @@ $body$ ); /* - * _etg_capture: never capture objects in this extension's own schemas - * (see the comment in its body). Same signature as 0.1.0, so a plain - * CREATE OR REPLACE (via create_function) is enough. + * _etg_fix_identity/_etg_capture: gain a self-recognition guard so they skip + * work while this extension's own event_trigger__disable() call is in + * effect for this session (checked via to_regclass() on the temp table + * event_trigger__disable() creates below) -- i.e. this extension's own + * install/update script is doing delicate internal restructuring right + * now. Installed here, ahead of the structural section below, specifically + * so the guard is already active by the time event_trigger__disable() is + * first called a few statements down: recreating them any later would + * leave the OLD (0.1.0), unguarded bodies live for that whole window -- + * which mattered in practice for _etg_capture, which would otherwise try + * to call _object_reference._object_v__for_update() (the FUNCTION) for any + * CREATE-tagged command in this script if a capture happened to be active, + * including a moment where that function has been dropped and not yet + * recreated, which would fail outright. Same signatures 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._etg_capture' @@ -188,6 +196,16 @@ DECLARE c_group_id CONSTANT int := object_group_id FROM object_reference.capture__get_current(); r record; BEGIN + /* + * Self-recognition: skip while this extension's own event_trigger__disable() + * is in effect (see below) -- i.e. this extension's own install/update + * script is doing delicate internal restructuring right now. Checked via + * to_regclass() rather than a catalog lookup that would error if the temp + * table doesn't exist, which is the common case. + */ + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RETURN; + END IF; IF c_group_id IS NOT NULL THEN -- Would be NULL if table is empty RAISE DEBUG E'\n\n*** START ***'; @@ -218,8 +236,11 @@ BEGIN /* * __object_reference is this extension's own scratch install/update * schema (created and dropped within a single script, never an - * extension member), so _is_own_object() never recognizes it -- - * exclude it here instead. + * extension member) -- self-recognition via the temp table above + * can't cover the handful of bootstrap statements that run before + * that table exists, so exclude it here too (object_identity + * carries the name for the CREATE SCHEMA statement itself, where + * schema_name is null). * * object_reference/_object_reference are excluded outright rather * than relying on _object_v__for_update()'s own _is_own_object() @@ -251,6 +272,235 @@ END $body$ , 'Event trigger function to capture newly created objects in an object group.' ); +SELECT __object_reference.create_function( + '_object_reference._etg_fix_identity' + , '' + , 'event_trigger SECURITY DEFINER LANGUAGE plpgsql' + , $body$ +DECLARE + r_ddl record; + r record; +BEGIN + /* + * Self-recognition: skip while this extension's own event_trigger__disable() + * is in effect (see below) -- i.e. this extension's own install/update + * script is doing delicate internal restructuring right now. Checked via + * to_regclass() rather than a catalog lookup that would error if the temp + * table doesn't exist, which is the common case. + */ + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RETURN; + END IF; + + /* + * It's tempting to use pg_event_trigger_ddl_commands() to find exactly what + * items have changed and worry about only those. That won't work because an + * object_names array can depend on multiple names (ie: a column depends on + * the name of it's table, as well as the name of the schema the table is in. + * You might think we could simply recurse through pg_depend to handle this, + * but not every name dependency gets enumerated that way. For example, + * columns are not marked as dependent on their table. + * + * Rather than trying to be cute about this, we just do a brute-force check + * for any names that have changed. + */ + + /* + * Presumably there's no way for an objects type/classid to change, but be + * safe and attempt the update to object_type. If it actually does change the + * constraint on the table should catch it. + */ + FOR r IN + UPDATE _object_reference.object + SET object_type = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).type::cat_tools.object_type + , object_names = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_names + , object_args = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_args + FROM _object_reference._object_oid oo + WHERE + oo.object_id = object.object_id + AND (object_type::text, object_names, object_args) IS DISTINCT FROM + (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)) + RETURNING * + LOOP + RAISE DEBUG 'modified_objects(): %', r; + END LOOP; +END +$body$ + , 'Event trigger function to update any records with object names or args that have changed.' +); + +/* + * Restore each trigger's actual prior state (captured above) now that the + * guarded bodies above are live -- self-recognition (checking for + * event_trigger__disable()'s temp table, created below) takes over from + * here for the rest of this script. + */ +DO $$ +DECLARE + r record; +BEGIN + FOR r IN SELECT evtname, evtenabled FROM pg_temp.__object_reference__bootstrap_event_trigger_state LOOP + EXECUTE format( + 'ALTER EVENT TRIGGER %I %s' + , r.evtname + , CASE r.evtenabled + WHEN 'O' THEN 'ENABLE' + WHEN 'R' THEN 'ENABLE REPLICA' + WHEN 'A' THEN 'ENABLE ALWAYS' + WHEN 'D' THEN 'DISABLE' + END + ); + END LOOP; +END +$$; +DROP TABLE pg_temp.__object_reference__bootstrap_event_trigger_state; + +/* + * WARNING: avoid disabling event triggers at all where any other option + * exists. ALTER EVENT TRIGGER is ordinary transactional DDL -- like any + * other catalog write, it's invisible to other sessions until commit (no + * special database-wide/immediate effect: verified empirically that a + * concurrent session's DDL neither blocks on, nor is otherwise affected by, + * another session's still-uncommitted DISABLE) and it takes no lock at all + * on the event trigger itself. The real risk is TWO SESSIONS both trying to + * alter the SAME event trigger concurrently: a second writer blocks on the + * first the way any two concurrent writes to the same catalog row would, + * and without care, the one that unblocks second can record and later + * restore a "prior state" that was never actually the trigger's state + * immediately before it acted (see the FOR UPDATE lock in + * event_trigger__disable()'s body below, which exists specifically to close + * that gap). Prefer a self-recognition check (a session-local flag, checked + * from inside the trigger's own body, as _etg_fix_identity/_etg_capture + * above do) over calling this at all; reach for it only when nothing else + * can make the trigger stay quiet, as is currently true for + * zzz__object_reference_drop. + * + * General-purpose event-trigger disable/enable mechanism, replacing the + * session_replication_role trick 0.1.0 had no equivalent of. 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. It can't self-recognize the way _etg_fix_identity/ + * _etg_capture above do without also touching that same view from inside + * its own body, so it must be truly disabled for the duration of this + * script's structural section. + * + * These are created now, ahead of the structural section, specifically so + * this script itself can call event_trigger__disable() below -- a fresh + * install only ever needs these for FUTURE update scripts, or anywhere + * else a future need to safely quiet an event trigger comes up (hence the + * mechanism-focused name, not one tied to "being mid-update"). + */ +SELECT __object_reference.create_function( + '_object_reference.event_trigger__disable' + , $args$ + event_trigger_names name[] +$args$ + , 'void LANGUAGE plpgsql' + , $body$ +DECLARE + v_name name; + v_enabled "char"; +BEGIN + /* + * WARNING: avoid disabling event triggers at all where any other option + * exists -- this is a database-wide change with real race-condition risk + * against concurrent sessions' DDL. See the warning above this function. + */ + BEGIN + -- Save old trigger state + CREATE TEMP TABLE __object_reference__event_trigger_state AS + SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false + ; + ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); + EXCEPTION WHEN duplicate_table THEN + RAISE 'event_trigger__disable() called while a previous call is still in effect' + USING HINT = 'A previous event_trigger__enable() call may have been skipped.' + ; + END; + + IF array_length(event_trigger_names, 1) <> (SELECT count(DISTINCT x) FROM unnest(event_trigger_names) x) THEN + RAISE 'event_trigger_names contains a duplicate name' USING DETAIL = event_trigger_names::text; + END IF; + + FOREACH v_name IN ARRAY event_trigger_names LOOP + /* + * FOR UPDATE locks the row before we read it, so no other session's own + * ALTER EVENT TRIGGER on the same trigger can land between our read and + * our DISABLE below -- without it, a concurrent change there would + * leave us recording (and later restoring) a state that was never + * actually the trigger's state immediately before we disabled it. + */ + SELECT evtenabled INTO v_enabled + FROM pg_catalog.pg_event_trigger + WHERE evtname = v_name + FOR UPDATE + ; + + IF NOT FOUND THEN + RAISE 'event trigger "%" does not exist', v_name; + END IF; + + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) + VALUES (v_name, v_enabled); + + PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); + END LOOP; +END +$body$ + , 'Disable the given event triggers, remembering their exact prior state; pair with event_trigger__enable().' +); +SELECT __object_reference.create_function( + '_object_reference.event_trigger__enable' + , '' + , 'void LANGUAGE plpgsql' + , $body$ +DECLARE + v_names name[]; + v_states "char"[]; + i int; +BEGIN + BEGIN + SELECT array_agg(evtname), array_agg(evtenabled) + INTO v_names, v_states + FROM pg_temp.__object_reference__event_trigger_state + ; + EXCEPTION WHEN undefined_table THEN + RAISE 'event_trigger__enable() called without a matching event_trigger__disable()'; + END; + + /* + * Drop our own bookkeeping table BEFORE re-enabling anything below: + * dropping it is itself DDL, and if zzz__object_reference_drop is one of + * the triggers being restored here, re-enabling it first would make this + * DROP immediately fire it -- reacting to our own internal cleanup, + * exactly the hazard this whole mechanism exists to avoid. (Confirmed by + * running into it: with the table dropped after, an active capture group + * elsewhere left a stale tracked row that this DROP's cascade into + * post_restore() then found and errored on.) + */ + DROP TABLE pg_temp.__object_reference__event_trigger_state; + + FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP + PERFORM _object_reference.exec(format( + 'ALTER EVENT TRIGGER %I %s' + , v_names[i] + , CASE v_states[i] + WHEN 'O' THEN 'ENABLE' + WHEN 'R' THEN 'ENABLE REPLICA' + WHEN 'A' THEN 'ENABLE ALWAYS' + WHEN 'D' THEN 'DISABLE' + END + )); + END LOOP; +END +$body$ + , 'Restore event triggers disabled by event_trigger__disable() to their exact prior state.' +); + +SELECT _object_reference.event_trigger__disable('{zzz__object_reference_drop}'); /* * _object_reference.object: no column changes, just a missing @@ -807,10 +1057,10 @@ 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. + * Re-enable zzz__object_reference_drop (to its actual prior state, saved by + * event_trigger__disable() 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); +SELECT _object_reference.event_trigger__enable(); -- vi: expandtab sw=2 ts=2 diff --git a/sql/object_reference.sql b/sql/object_reference.sql index e7e00e9..a43627b 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1429,6 +1429,16 @@ DECLARE c_group_id CONSTANT int := object_group_id FROM object_reference.capture__get_current(); r record; BEGIN + /* + * Self-recognition: skip while this extension's own event_trigger__disable() + * is in effect (see below) -- i.e. this extension's own install/update + * script is doing delicate internal restructuring right now. Checked via + * to_regclass() rather than a catalog lookup that would error if the temp + * table doesn't exist, which is the common case. + */ + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RETURN; + END IF; IF c_group_id IS NOT NULL THEN -- Would be NULL if table is empty RAISE DEBUG E'\n\n*** START ***'; @@ -1459,8 +1469,11 @@ BEGIN /* * __object_reference is this extension's own scratch install/update * schema (created and dropped within a single script, never an - * extension member), so _is_own_object() never recognizes it -- - * exclude it here instead. + * extension member) -- self-recognition via the temp table above + * can't cover the handful of bootstrap statements that run before + * that table exists, so exclude it here too (object_identity + * carries the name for the CREATE SCHEMA statement itself, where + * schema_name is null). * * object_reference/_object_reference are excluded outright rather * than relying on _object_v__for_update()'s own _is_own_object() @@ -1503,6 +1516,17 @@ DECLARE r_ddl record; r record; BEGIN + /* + * Self-recognition: skip while this extension's own event_trigger__disable() + * is in effect (see below) -- i.e. this extension's own install/update + * script is doing delicate internal restructuring right now. Checked via + * to_regclass() rather than a catalog lookup that would error if the temp + * table doesn't exist, which is the common case. + */ + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RETURN; + END IF; + /* * It's tempting to use pg_event_trigger_ddl_commands() to find exactly what * items have changed and worry about only those. That won't work because an @@ -1588,6 +1612,151 @@ $body$ , 'Event trigger function to drop object records when objects are removed.' ); +/* + * WARNING: avoid disabling event triggers at all where any other option + * exists. ALTER EVENT TRIGGER is ordinary transactional DDL -- like any + * other catalog write, it's invisible to other sessions until commit (no + * special database-wide/immediate effect: verified empirically that a + * concurrent session's DDL neither blocks on, nor is otherwise affected by, + * another session's still-uncommitted DISABLE) and it takes no lock at all + * on the event trigger itself. The real risk is TWO SESSIONS both trying to + * alter the SAME event trigger concurrently: a second writer blocks on the + * first the way any two concurrent writes to the same catalog row would, + * and without care, the one that unblocks second can record and later + * restore a "prior state" that was never actually the trigger's state + * immediately before it acted (see the FOR UPDATE lock in + * event_trigger__disable()'s body below, which exists specifically to close + * that gap). Prefer a self-recognition check (a session-local flag, checked + * from inside the trigger's own body) over calling this at all; reach for + * it only when nothing else can make the trigger stay quiet, as is + * currently true for zzz__object_reference_drop. + * + * General-purpose event-trigger disable/enable mechanism, for use by this + * extension's OWN install/update scripts only (not part of the public API). + * Not tied to "being mid-update" specifically -- it's a plain disable-with- + * restore primitive for any event trigger that can't self-recognize that it + * should stay quiet. + * + * zzz_object_reference__fix_identity and zzz_object_reference_capture check + * whether this call is currently in effect for their OWN session (via + * to_regclass() on the temp table below) and skip if so, so they never need + * to be disabled this way. zzz__object_reference_drop cannot self-recognize + * the same way without also touching _object_reference._object_v -- a view + * an update script may itself be dropping and recreating -- from inside its + * own body, so it must be truly disabled for the duration of such a + * script's structural section. + * + * ALTER EVENT TRIGGER is ordinary transactional DDL, so if the calling + * script's transaction rolls back, the DISABLE (and any ENABLE already run) + * rolls back with it -- no separate cleanup-on-error logic is needed here. + */ +SELECT __object_reference.create_function( + '_object_reference.event_trigger__disable' + , $args$ + event_trigger_names name[] +$args$ + , 'void LANGUAGE plpgsql' + , $body$ +DECLARE + v_name name; + v_enabled "char"; +BEGIN + /* + * WARNING: avoid disabling event triggers at all where any other option + * exists -- this is a database-wide change with real race-condition risk + * against concurrent sessions' DDL. See the warning above this function. + */ + BEGIN + -- Save old trigger state + CREATE TEMP TABLE __object_reference__event_trigger_state AS + SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false + ; + ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); + EXCEPTION WHEN duplicate_table THEN + RAISE 'event_trigger__disable() called while a previous call is still in effect' + USING HINT = 'A previous event_trigger__enable() call may have been skipped.' + ; + END; + + IF array_length(event_trigger_names, 1) <> (SELECT count(DISTINCT x) FROM unnest(event_trigger_names) x) THEN + RAISE 'event_trigger_names contains a duplicate name' USING DETAIL = event_trigger_names::text; + END IF; + + FOREACH v_name IN ARRAY event_trigger_names LOOP + /* + * FOR UPDATE locks the row before we read it, so no other session's own + * ALTER EVENT TRIGGER on the same trigger can land between our read and + * our DISABLE below -- without it, a concurrent change there would + * leave us recording (and later restoring) a state that was never + * actually the trigger's state immediately before we disabled it. + */ + SELECT evtenabled INTO v_enabled + FROM pg_catalog.pg_event_trigger + WHERE evtname = v_name + FOR UPDATE + ; + + IF NOT FOUND THEN + RAISE 'event trigger "%" does not exist', v_name; + END IF; + + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) + VALUES (v_name, v_enabled); + + PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); + END LOOP; +END +$body$ + , 'Disable the given event triggers, remembering their exact prior state; pair with event_trigger__enable().' +); +SELECT __object_reference.create_function( + '_object_reference.event_trigger__enable' + , '' + , 'void LANGUAGE plpgsql' + , $body$ +DECLARE + v_names name[]; + v_states "char"[]; + i int; +BEGIN + BEGIN + SELECT array_agg(evtname), array_agg(evtenabled) + INTO v_names, v_states + FROM pg_temp.__object_reference__event_trigger_state + ; + EXCEPTION WHEN undefined_table THEN + RAISE 'event_trigger__enable() called without a matching event_trigger__disable()'; + END; + + /* + * Drop our own bookkeeping table BEFORE re-enabling anything below: + * dropping it is itself DDL, and if zzz__object_reference_drop is one of + * the triggers being restored here, re-enabling it first would make this + * DROP immediately fire it -- reacting to our own internal cleanup, + * exactly the hazard this whole mechanism exists to avoid. (Confirmed by + * running into it: with the table dropped after, an active capture group + * elsewhere left a stale tracked row that this DROP's cascade into + * post_restore() then found and errored on.) + */ + DROP TABLE pg_temp.__object_reference__event_trigger_state; + + FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP + PERFORM _object_reference.exec(format( + 'ALTER EVENT TRIGGER %I %s' + , v_names[i] + , CASE v_states[i] + WHEN 'O' THEN 'ENABLE' + WHEN 'R' THEN 'ENABLE REPLICA' + WHEN 'A' THEN 'ENABLE ALWAYS' + WHEN 'D' THEN 'DISABLE' + END + )); + END LOOP; +END +$body$ + , 'Restore event triggers disabled by event_trigger__disable() to their exact prior state.' +); + SELECT __object_reference.create_function( '_object_reference.etg_raise__start' , '' diff --git a/test/build/expected/build.out b/test/build/expected/build.out index 21ac942..24f8e8f 100644 --- a/test/build/expected/build.out +++ b/test/build/expected/build.out @@ -52,6 +52,8 @@ psql:test/temp_load.not_sql:581: WARNING: I promise you will be sorry if you tr + + diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out new file mode 100644 index 0000000..2e0e5f6 --- /dev/null +++ b/test/expected/event_trigger_disable.out @@ -0,0 +1,23 @@ +\set ECHO none +1..20 +ok 1 - manually disable test trigger b ahead of time +ok 2 - disable() both test triggers +ok 3 - test trigger a is disabled while a call is in effect +ok 4 - test trigger b is (still) disabled while a call is in effect +ok 5 - enable() restores both +ok 6 - test trigger a is back to its original (origin) state +ok 7 - test trigger b is still disabled -- its prior state was preserved, not assumed enabled +ok 8 - disable() the first time +ok 9 - a second disable() without enable() in between is rejected +ok 10 - enable() cleans up so later tests are unaffected +ok 11 - enable() without disable() is rejected +ok 12 - disable() rejects an unknown event trigger name +ok 13 - disable() rejects a duplicate name in its own argument list +ok 14 - start a capture group +ok 15 - disable() (any trigger) also signals self-recognizing triggers to stand down +ok 16 - enable() ends that window +ok 17 - the table created while disable() was in effect was NOT captured +ok 18 - stop the capture group +ok 19 - object_reference schema must not be part of the resolved search_path +ok 20 - _object_reference schema must not be part of the resolved search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql new file mode 100644 index 0000000..8730f32 --- /dev/null +++ b/test/sql/event_trigger_disable.sql @@ -0,0 +1,142 @@ +\set ECHO none + +\i test/load.sql + +/* + * event_trigger__disable()/__enable() are ALTER EVENT TRIGGER under the + * hood -- ordinary transactional DDL, invisible to other sessions until + * commit, same as any other catalog change (verified empirically: a + * concurrent session neither blocks on, nor otherwise sees, another + * session's still-uncommitted DISABLE). The real risk is two sessions both + * altering the SAME event trigger concurrently, so this test exercises the + * mechanism against its OWN dummy event triggers rather than the real zzz_* + * ones, to keep it independent of whatever else happens to run concurrently + * in this same parallel test batch. + */ +CREATE FUNCTION event_trigger_disable_test__noop() RETURNS event_trigger LANGUAGE plpgsql AS $$ +BEGIN +END +$$; +CREATE EVENT TRIGGER event_trigger_disable_test__a ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); +CREATE EVENT TRIGGER event_trigger_disable_test__b ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); + +SELECT plan( + 0 + +7 -- multi-trigger disable/enable preserves each one's own prior state + +3 -- nested disable() without an intervening enable() is rejected + +1 -- enable() without a matching disable() is rejected + +1 -- disable() rejects an unknown event trigger name + +1 -- disable() rejects a duplicate name in its own argument list + +5 -- zzz_object_reference_capture self-recognizes and stands down while a disable() is in effect + +2 -- schema-qualification (search_path) +); + +-- Multi-trigger disable/enable, preserving each trigger's own prior state +SELECT lives_ok( + $$ALTER EVENT TRIGGER event_trigger_disable_test__b DISABLE$$ + , 'manually disable test trigger b ahead of time' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,event_trigger_disable_test__b}')$$ + , 'disable() both test triggers' +); +SELECT is( + (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__a') + , 'D' + , 'test trigger a is disabled while a call is in effect' +); +SELECT is( + (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__b') + , 'D' + , 'test trigger b is (still) disabled while a call is in effect' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() restores both' +); +SELECT is( + (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__a') + , 'O' + , 'test trigger a is back to its original (origin) state' +); +SELECT is( + (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__b') + , 'D' + , 'test trigger b is still disabled -- its prior state was preserved, not assumed enabled' +); + +-- Nested disable() without an intervening enable() +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() the first time' +); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , NULL + , 'event_trigger__disable() called while a previous call is still in effect' + , 'a second disable() without enable() in between is rejected' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() cleans up so later tests are unaffected' +); + +-- enable() without a matching disable() +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , NULL + , 'event_trigger__enable() called without a matching event_trigger__disable()' + , 'enable() without disable() is rejected' +); + +-- Unknown event trigger name +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{no_such_event_trigger}')$$ + , NULL + , 'event trigger "no_such_event_trigger" does not exist' + , 'disable() rejects an unknown event trigger name' +); + +-- Duplicate name in the same call +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,event_trigger_disable_test__a}')$$ + , NULL + , 'event_trigger_names contains a duplicate name' + , 'disable() rejects a duplicate name in its own argument list' +); + +/* + * zzz_object_reference_capture / zzz_object_reference__fix_identity check + * for an event_trigger__disable() call currently in effect for THIS + * session (regardless of which trigger names it names) and stand down -- + * verify that here for capture, since it's directly observable. + */ +SELECT lives_ok( + $$SELECT object_reference.capture__start(object_reference.object_group__create('event_trigger_disable_test_group'))$$ + , 'start a capture group' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() (any trigger) also signals self-recognizing triggers to stand down' +); +CREATE TABLE event_trigger_disable_test_table(); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() ends that window' +); +SELECT is_empty( + $$ + SELECT 1 + FROM _object_reference.object_group__object + WHERE object_group_id = (object_reference.object_group__get('event_trigger_disable_test_group')).object_group_id + $$ + , 'the table created while disable() was in effect was NOT captured' +); +SELECT lives_ok( + $$SELECT object_reference.capture__stop('event_trigger_disable_test_group')$$ + , 'stop the capture group' +); + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2 From 8429c9d4f007556d23d3eab475ed04b0ee44112c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 12:40:04 -0500 Subject: [PATCH 03/35] Test that the 0.1.0 -> stable update restores event trigger state In update mode, test/install/load.sql now starts `zzz_object_reference_capture`, `zzz_object_reference__fix_identity` and `zzz__object_reference_drop` in distinct non-default states (DISABLE, ENABLE REPLICA, ENABLE ALWAYS), runs the update, and raises unless each one is back in exactly that state. That covers both restore paths: the bootstrap block for the first two and `event_trigger__enable()` for the third. load.sql's own output is never compared against anything, so the update and the check share a transaction: a failed check also undoes the update, and the suite then fails against the old version. All three triggers are reset to ENABLE afterwards so the rest of the suite runs unchanged. --- test/install/load.sql | 44 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/test/install/load.sql b/test/install/load.sql index fc3a98a..71130be 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -177,6 +177,20 @@ SELECT CASE WHEN :'object_reference_test_update_to' = '' THEN '' CREATE EXTENSION IF NOT EXISTS count_nulls; CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; + +/* + * Fail unless the update restores each of the extension's event triggers to + * the exact state it had before, starting each from a distinct non-default + * state. The update and the check share a transaction, so a failed check + * also undoes the update and the suite then fails against the old version: + * this file's own output is never compared against anything. All three are + * reset to ENABLE afterwards, so the rest of the suite runs as usual. + */ +BEGIN; +ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; +ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE REPLICA; +ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE ALWAYS; + /* * Suppress the deprecation NOTICEs the update script emits (e.g. the reg* * pseudotype removal), matching cat_tools' own install/load.sql. @@ -184,6 +198,36 @@ CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' C SET client_min_messages = ERROR; ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; SET client_min_messages = WARNING; + +DO $DO$ +DECLARE + v_mismatches text; +BEGIN + SELECT string_agg( + format('%I is %L, expected %L.', e.evtname, t.evtenabled, e.evtenabled) + , ' ' ORDER BY e.evtname + ) + INTO v_mismatches + FROM (VALUES + ('zzz_object_reference_capture'::name, 'D'::"char") + , ('zzz_object_reference__fix_identity', 'R') + , ('zzz__object_reference_drop', 'A') + ) e(evtname, evtenabled) + LEFT JOIN pg_catalog.pg_event_trigger t USING (evtname) + WHERE t.evtenabled IS DISTINCT FROM e.evtenabled + ; + IF v_mismatches IS NOT NULL THEN + RAISE EXCEPTION 'ALTER EXTENSION UPDATE did not restore event trigger state' + USING DETAIL = v_mismatches + ; + END IF; +END +$DO$; + +ALTER EVENT TRIGGER zzz_object_reference_capture ENABLE; +ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE; +ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE; +COMMIT; \else CREATE EXTENSION object_reference CASCADE; \endif From b73a656bc91512d966f21d35c3fa29a4429ed22c Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 16:53:43 -0500 Subject: [PATCH 04/35] Test the 0.1.0 -> stable update with a capture group active In update mode, test/install/load.sql now runs three update scenarios before the committed update, each installing 0.1.0 in its own transaction and rolling back: 1. All three event triggers at ENABLE: the update leaves the capture group empty, restores every trigger, leaves no state table behind, and a table created afterwards is captured. 2. zzz__object_reference_drop already disabled: it stays disabled. 3. A table captured before the update and renamed after it shows its new name in the capture group. Each scenario asserts the trigger state it starts the update from. Also restore the trailing space after WHERE in the update script's copy of _etg_fix_identity, so its body matches sql/object_reference.sql, and correct load.sql's description of how a failed check fails the run. --- sql/object_reference--0.1.0--stable.sql | 2 +- test/install/load.sql | 133 +++++++++++++++++++++++- 2 files changed, 130 insertions(+), 5 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index c0c2d8a..9eaef3a 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -314,7 +314,7 @@ BEGIN , object_names = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_names , object_args = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_args FROM _object_reference._object_oid oo - WHERE + WHERE oo.object_id = object.object_id AND (object_type::text, object_names, object_args) IS DISTINCT FROM (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)) diff --git a/test/install/load.sql b/test/install/load.sql index 71130be..486ddb5 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -176,15 +176,140 @@ SELECT CASE WHEN :'object_reference_test_update_to' = '' THEN '' */ CREATE EXTENSION IF NOT EXISTS count_nulls; +/* + * Update scenarios. Each installs the old version inside its own + * transaction and rolls back, so nothing leaks into the committed install + * below; that includes sequence values, which a rollback alone would not + * reset on an install committed beforehand. Each one starts a capture group, + * asserts the event trigger state it starts the update from, updates, and + * checks the result: + * 1. All three triggers at their default ENABLE; a table created after the + * update is captured. + * 2. zzz__object_reference_drop already disabled, and still disabled after. + * 3. A table captured before the update is renamed after it; the capture + * group shows the new name. + */ +CREATE FUNCTION pg_temp.assert_event_trigger_state( + step text + , capture_state "char" + , fix_identity_state "char" + , drop_state "char" +) RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_mismatches text; +BEGIN + SELECT string_agg( + format('%I is %L, expected %L.', e.evtname, t.evtenabled, e.evtenabled) + , ' ' ORDER BY e.evtname + ) + INTO v_mismatches + FROM (VALUES + ('zzz_object_reference_capture'::name, capture_state) + , ('zzz_object_reference__fix_identity', fix_identity_state) + , ('zzz__object_reference_drop', drop_state) + ) e(evtname, evtenabled) + LEFT JOIN pg_catalog.pg_event_trigger t USING (evtname) + WHERE t.evtenabled IS DISTINCT FROM e.evtenabled + ; + IF v_mismatches IS NOT NULL THEN + RAISE EXCEPTION 'unexpected event trigger state %', step + USING DETAIL = v_mismatches + ; + END IF; +END +$$; + +CREATE FUNCTION pg_temp.assert_update_finished( + step text +) RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_installed text := (SELECT extversion FROM pg_catalog.pg_extension WHERE extname = 'object_reference'); + v_target text := coalesce( + nullif(current_setting('object_reference.test_update_to'), '') + , (SELECT default_version FROM pg_catalog.pg_available_extensions WHERE name = 'object_reference') + ); +BEGIN + IF v_installed IS DISTINCT FROM v_target THEN + RAISE EXCEPTION 'object_reference is at version % %, expected %', v_installed, step, v_target; + END IF; + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL + OR to_regclass('pg_temp.__object_reference__bootstrap_event_trigger_state') IS NOT NULL + THEN + RAISE EXCEPTION 'event trigger state table left behind %', step; + END IF; +END +$$; + +CREATE FUNCTION pg_temp.assert_capture_group( + step text + , expected text[] +) RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_members text[]; +BEGIN + SELECT coalesce(array_agg(array_to_string(o.object_names, '.') ORDER BY o.object_names), '{}') + INTO v_members + FROM _object_reference.object_group__object m + JOIN _object_reference.object o USING (object_id) + WHERE m.object_group_id = (object_reference.object_group__get('load_update_scenario')).object_group_id + ; + IF v_members IS DISTINCT FROM expected THEN + RAISE EXCEPTION 'unexpected capture group members %', step + USING DETAIL = format('Members are %L, expected %L.', v_members, expected) + ; + END IF; +END +$$; + +SET client_min_messages = ERROR; + +BEGIN; +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); +SELECT pg_temp.assert_event_trigger_state('before the update in scenario 1', 'O', 'O', 'O'); +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SELECT pg_temp.assert_update_finished('after the update in scenario 1'); +SELECT pg_temp.assert_event_trigger_state('after the update in scenario 1', 'O', 'O', 'O'); +SELECT pg_temp.assert_capture_group('after the update in scenario 1', '{}'); +CREATE TABLE load_update_scenario__after(); +SELECT pg_temp.assert_capture_group('after CREATE TABLE in scenario 1', '{public.load_update_scenario__after}'); +ROLLBACK; + +BEGIN; +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +ALTER EVENT TRIGGER zzz__object_reference_drop DISABLE; +SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); +SELECT pg_temp.assert_event_trigger_state('before the update in scenario 2', 'O', 'O', 'D'); +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SELECT pg_temp.assert_update_finished('after the update in scenario 2'); +SELECT pg_temp.assert_event_trigger_state('after the update in scenario 2', 'O', 'O', 'D'); +SELECT pg_temp.assert_capture_group('after the update in scenario 2', '{}'); +ROLLBACK; + +BEGIN; +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); +CREATE TABLE load_update_scenario__before(); +SELECT pg_temp.assert_capture_group('before the update in scenario 3', '{public.load_update_scenario__before}'); +SELECT pg_temp.assert_event_trigger_state('before the update in scenario 3', 'O', 'O', 'O'); +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SELECT pg_temp.assert_update_finished('after the update in scenario 3'); +SELECT pg_temp.assert_event_trigger_state('after the update in scenario 3', 'O', 'O', 'O'); +ALTER TABLE load_update_scenario__before RENAME TO load_update_scenario__renamed; +SELECT pg_temp.assert_capture_group('after the rename in scenario 3', '{public.load_update_scenario__renamed}'); +ROLLBACK; + +SET client_min_messages = WARNING; + CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; /* * Fail unless the update restores each of the extension's event triggers to * the exact state it had before, starting each from a distinct non-default - * state. The update and the check share a transaction, so a failed check - * also undoes the update and the suite then fails against the old version: - * this file's own output is never compared against anything. All three are - * reset to ENABLE afterwards, so the rest of the suite runs as usual. + * state. A failed check fails the run through psql's ON_ERROR_STOP exit + * status; this file's own output is never compared against anything. All + * three are reset to ENABLE afterwards, so the rest of the suite runs as + * usual. */ BEGIN; ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; From f2deadb392bb887bf913c1e0784797d051db0664 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:02:53 -0500 Subject: [PATCH 05/35] Correct load.sql's note on which pg_regress versions fail on a failed check --- test/install/load.sql | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/install/load.sql b/test/install/load.sql index 486ddb5..eac5ad0 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -306,10 +306,10 @@ CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' C /* * Fail unless the update restores each of the extension's event triggers to * the exact state it had before, starting each from a distinct non-default - * state. A failed check fails the run through psql's ON_ERROR_STOP exit - * status; this file's own output is never compared against anything. All - * three are reset to ENABLE afterwards, so the rest of the suite runs as - * usual. + * state. A failed check exits psql nonzero (ON_ERROR_STOP), which fails the + * run only under pg_regress 16+; PG12's pg_regress reports it as ok. This + * file's own output is never compared against anything. All three are reset + * to ENABLE afterwards, so the rest of the suite runs as usual. */ BEGIN; ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; From 7a44f99557094a8044188c5a1eb56fd6f961fdf7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:12:30 -0500 Subject: [PATCH 06/35] Validate event_trigger__disable()/__enable() arguments before side effects disable() now rejects a NULL/empty array, NULL elements, duplicates, and unknown names, and a call still in effect, before creating its state table or locking anything. enable() checks for a matching disable() up front. Names are processed in "C" collation order (and restored in the same order) so concurrent callers lock in a consistent order. Per-trigger locking moves to _event_trigger__lock(), which falls back to a transaction-scoped advisory lock when the role cannot row-lock pg_event_trigger (insufficient_privilege). Co-Authored-By: Claude Sonnet 5.5 --- sql/object_reference--0.1.0--stable.sql | 117 +++++++++++++------ sql/object_reference.sql | 117 +++++++++++++------ test/build/expected/build.out | 1 + test/expected/event_trigger_disable.out | 39 +++++-- test/sql/event_trigger_disable.sql | 144 +++++++++++++++++++++++- 5 files changed, 342 insertions(+), 76 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 9eaef3a..42d4b2f 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -365,9 +365,8 @@ DROP TABLE pg_temp.__object_reference__bootstrap_event_trigger_state; * first the way any two concurrent writes to the same catalog row would, * and without care, the one that unblocks second can record and later * restore a "prior state" that was never actually the trigger's state - * immediately before it acted (see the FOR UPDATE lock in - * event_trigger__disable()'s body below, which exists specifically to close - * that gap). Prefer a self-recognition check (a session-local flag, checked + * immediately before it acted (see _event_trigger__lock() + * below, which exists specifically to close that gap). Prefer a self-recognition check (a session-local flag, checked * from inside the trigger's own body, as _etg_fix_identity/_etg_capture * above do) over calling this at all; reach for it only when nothing else * can make the trigger stay quiet, as is currently true for @@ -391,6 +390,41 @@ DROP TABLE pg_temp.__object_reference__bootstrap_event_trigger_state; * else a future need to safely quiet an event trigger comes up (hence the * mechanism-focused name, not one tied to "being mid-update"). */ +SELECT __object_reference.create_function( + '_object_reference._event_trigger__lock' + , $args$ + event_trigger_name name +$args$ + , '"char" LANGUAGE plpgsql' + , $body$ +DECLARE + v_oid oid; + v_enabled "char"; +BEGIN + BEGIN + SELECT evtenabled INTO v_enabled + FROM pg_catalog.pg_event_trigger + WHERE evtname = event_trigger_name + FOR UPDATE + ; + EXCEPTION WHEN insufficient_privilege THEN + -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. + SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + IF FOUND THEN + PERFORM pg_catalog.pg_advisory_xact_lock( + 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int + , v_oid::int + ); + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + END IF; + END; + + -- NULL if the trigger does not exist + RETURN v_enabled; +END +$body$ + , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' +); SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -407,37 +441,55 @@ BEGIN * exists -- this is a database-wide change with real race-condition risk * against concurrent sessions' DDL. See the warning above this function. */ - BEGIN - -- Save old trigger state - CREATE TEMP TABLE __object_reference__event_trigger_state AS - SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false - ; - ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); - EXCEPTION WHEN duplicate_table THEN - RAISE 'event_trigger__disable() called while a previous call is still in effect' - USING HINT = 'A previous event_trigger__enable() call may have been skipped.' - ; - END; + + -- Validate everything before any side effect. + IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN + RAISE 'event_trigger_names must be a non-empty array'; + END IF; + + IF array_position(event_trigger_names, NULL) IS NOT NULL THEN + RAISE 'event_trigger_names contains a NULL name' USING DETAIL = event_trigger_names::text; + END IF; IF array_length(event_trigger_names, 1) <> (SELECT count(DISTINCT x) FROM unnest(event_trigger_names) x) THEN RAISE 'event_trigger_names contains a duplicate name' USING DETAIL = event_trigger_names::text; END IF; - FOREACH v_name IN ARRAY event_trigger_names LOOP + SELECT n INTO v_name + FROM unnest(event_trigger_names) n + WHERE NOT EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = n) + ORDER BY n COLLATE "C" + LIMIT 1 + ; + IF FOUND THEN + RAISE 'event trigger "%" does not exist', v_name; + END IF; + + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RAISE 'event_trigger__disable() called while a previous call is still in effect' + USING HINT = 'A previous event_trigger__enable() call may have been skipped.' + ; + END IF; + + -- Save old trigger state + CREATE TEMP TABLE __object_reference__event_trigger_state AS + SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false + ; + ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); + + -- Fixed lock order across all callers, to avoid deadlock. + FOR v_name IN SELECT n FROM unnest(event_trigger_names) n ORDER BY n COLLATE "C" LOOP /* - * FOR UPDATE locks the row before we read it, so no other session's own + * The lock must be held before we read evtenabled, so no other session's * ALTER EVENT TRIGGER on the same trigger can land between our read and - * our DISABLE below -- without it, a concurrent change there would - * leave us recording (and later restoring) a state that was never - * actually the trigger's state immediately before we disabled it. + * our DISABLE below -- otherwise we could record (and later restore) a + * state that was never the trigger's state immediately before we + * disabled it. */ - SELECT evtenabled INTO v_enabled - FROM pg_catalog.pg_event_trigger - WHERE evtname = v_name - FOR UPDATE - ; + v_enabled := _object_reference._event_trigger__lock(v_name); - IF NOT FOUND THEN + -- Dropped by another session since the check above + IF v_enabled IS NULL THEN RAISE 'event trigger "%" does not exist', v_name; END IF; @@ -460,14 +512,15 @@ DECLARE v_states "char"[]; i int; BEGIN - BEGIN - SELECT array_agg(evtname), array_agg(evtenabled) - INTO v_names, v_states - FROM pg_temp.__object_reference__event_trigger_state - ; - EXCEPTION WHEN undefined_table THEN + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NULL THEN RAISE 'event_trigger__enable() called without a matching event_trigger__disable()'; - END; + END IF; + + -- Same order disable() locked in + SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") + INTO v_names, v_states + FROM pg_temp.__object_reference__event_trigger_state + ; /* * Drop our own bookkeeping table BEFORE re-enabling anything below: diff --git a/sql/object_reference.sql b/sql/object_reference.sql index b77b7e0..ed0c6be 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1623,9 +1623,8 @@ $body$ * first the way any two concurrent writes to the same catalog row would, * and without care, the one that unblocks second can record and later * restore a "prior state" that was never actually the trigger's state - * immediately before it acted (see the FOR UPDATE lock in - * event_trigger__disable()'s body below, which exists specifically to close - * that gap). Prefer a self-recognition check (a session-local flag, checked + * immediately before it acted (see _event_trigger__lock() + * below, which exists specifically to close that gap). Prefer a self-recognition check (a session-local flag, checked * from inside the trigger's own body) over calling this at all; reach for * it only when nothing else can make the trigger stay quiet, as is * currently true for zzz__object_reference_drop. @@ -1649,6 +1648,41 @@ $body$ * script's transaction rolls back, the DISABLE (and any ENABLE already run) * rolls back with it -- no separate cleanup-on-error logic is needed here. */ +SELECT __object_reference.create_function( + '_object_reference._event_trigger__lock' + , $args$ + event_trigger_name name +$args$ + , '"char" LANGUAGE plpgsql' + , $body$ +DECLARE + v_oid oid; + v_enabled "char"; +BEGIN + BEGIN + SELECT evtenabled INTO v_enabled + FROM pg_catalog.pg_event_trigger + WHERE evtname = event_trigger_name + FOR UPDATE + ; + EXCEPTION WHEN insufficient_privilege THEN + -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. + SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + IF FOUND THEN + PERFORM pg_catalog.pg_advisory_xact_lock( + 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int + , v_oid::int + ); + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + END IF; + END; + + -- NULL if the trigger does not exist + RETURN v_enabled; +END +$body$ + , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' +); SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -1665,37 +1699,55 @@ BEGIN * exists -- this is a database-wide change with real race-condition risk * against concurrent sessions' DDL. See the warning above this function. */ - BEGIN - -- Save old trigger state - CREATE TEMP TABLE __object_reference__event_trigger_state AS - SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false - ; - ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); - EXCEPTION WHEN duplicate_table THEN - RAISE 'event_trigger__disable() called while a previous call is still in effect' - USING HINT = 'A previous event_trigger__enable() call may have been skipped.' - ; - END; + + -- Validate everything before any side effect. + IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN + RAISE 'event_trigger_names must be a non-empty array'; + END IF; + + IF array_position(event_trigger_names, NULL) IS NOT NULL THEN + RAISE 'event_trigger_names contains a NULL name' USING DETAIL = event_trigger_names::text; + END IF; IF array_length(event_trigger_names, 1) <> (SELECT count(DISTINCT x) FROM unnest(event_trigger_names) x) THEN RAISE 'event_trigger_names contains a duplicate name' USING DETAIL = event_trigger_names::text; END IF; - FOREACH v_name IN ARRAY event_trigger_names LOOP + SELECT n INTO v_name + FROM unnest(event_trigger_names) n + WHERE NOT EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = n) + ORDER BY n COLLATE "C" + LIMIT 1 + ; + IF FOUND THEN + RAISE 'event trigger "%" does not exist', v_name; + END IF; + + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RAISE 'event_trigger__disable() called while a previous call is still in effect' + USING HINT = 'A previous event_trigger__enable() call may have been skipped.' + ; + END IF; + + -- Save old trigger state + CREATE TEMP TABLE __object_reference__event_trigger_state AS + SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false + ; + ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); + + -- Fixed lock order across all callers, to avoid deadlock. + FOR v_name IN SELECT n FROM unnest(event_trigger_names) n ORDER BY n COLLATE "C" LOOP /* - * FOR UPDATE locks the row before we read it, so no other session's own + * The lock must be held before we read evtenabled, so no other session's * ALTER EVENT TRIGGER on the same trigger can land between our read and - * our DISABLE below -- without it, a concurrent change there would - * leave us recording (and later restoring) a state that was never - * actually the trigger's state immediately before we disabled it. + * our DISABLE below -- otherwise we could record (and later restore) a + * state that was never the trigger's state immediately before we + * disabled it. */ - SELECT evtenabled INTO v_enabled - FROM pg_catalog.pg_event_trigger - WHERE evtname = v_name - FOR UPDATE - ; + v_enabled := _object_reference._event_trigger__lock(v_name); - IF NOT FOUND THEN + -- Dropped by another session since the check above + IF v_enabled IS NULL THEN RAISE 'event trigger "%" does not exist', v_name; END IF; @@ -1718,14 +1770,15 @@ DECLARE v_states "char"[]; i int; BEGIN - BEGIN - SELECT array_agg(evtname), array_agg(evtenabled) - INTO v_names, v_states - FROM pg_temp.__object_reference__event_trigger_state - ; - EXCEPTION WHEN undefined_table THEN + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NULL THEN RAISE 'event_trigger__enable() called without a matching event_trigger__disable()'; - END; + END IF; + + -- Same order disable() locked in + SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") + INTO v_names, v_states + FROM pg_temp.__object_reference__event_trigger_state + ; /* * Drop our own bookkeeping table BEFORE re-enabling anything below: diff --git a/test/build/expected/build.out b/test/build/expected/build.out index e9371eb..3e6e155 100644 --- a/test/build/expected/build.out +++ b/test/build/expected/build.out @@ -55,6 +55,7 @@ psql:test/temp_load.not_sql:580: WARNING: I promise you will be sorry if you tr + Loaded OK! diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 2e0e5f6..74623c3 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..20 +1..41 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -12,12 +12,33 @@ ok 9 - a second disable() without enable() in between is rejected ok 10 - enable() cleans up so later tests are unaffected ok 11 - enable() without disable() is rejected ok 12 - disable() rejects an unknown event trigger name -ok 13 - disable() rejects a duplicate name in its own argument list -ok 14 - start a capture group -ok 15 - disable() (any trigger) also signals self-recognizing triggers to stand down -ok 16 - enable() ends that window -ok 17 - the table created while disable() was in effect was NOT captured -ok 18 - stop the capture group -ok 19 - object_reference schema must not be part of the resolved search_path -ok 20 - _object_reference schema must not be part of the resolved search_path +ok 13 - unknown name: nothing was changed +ok 14 - disable() rejects a duplicate name in its own argument list +ok 15 - duplicate name: nothing was changed +ok 16 - disable() rejects a NULL array +ok 17 - NULL array: nothing was changed +ok 18 - disable() rejects an empty array +ok 19 - empty array: nothing was changed +ok 20 - disable() rejects a NULL element +ok 21 - NULL element: nothing was changed +ok 22 - disable() so a call is in effect +ok 23 - duplicate name is reported before the in-effect error +ok 24 - unknown name is reported before the in-effect error +ok 25 - NULL array is reported before the in-effect error +ok 26 - the call still in effect kept its own state +ok 27 - enable() ends that call +ok 28 - disable() with unsorted names +ok 29 - names were processed in "C" collation order, not input order +ok 30 - enable() after unsorted disable() +ok 31 - all three restored +ok 32 - helper does not fail for a role that cannot row-lock the catalog +ok 33 - helper still returns the trigger's evtenabled +ok 34 - helper took the advisory lock keyed on catalog and trigger oid +ok 35 - start a capture group +ok 36 - disable() (any trigger) also signals self-recognizing triggers to stand down +ok 37 - enable() ends that window +ok 38 - the table created while disable() was in effect was NOT captured +ok 39 - stop the capture group +ok 40 - object_reference schema must not be part of the resolved search_path +ok 41 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 8730f32..f0fbf5c 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -19,14 +19,31 @@ END $$; CREATE EVENT TRIGGER event_trigger_disable_test__a ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); CREATE EVENT TRIGGER event_trigger_disable_test__b ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); +-- Names whose "C" order (B, a, z) differs from a typical locale order (a, B, z) +CREATE EVENT TRIGGER "event_trigger_disable_test__B" ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); +CREATE EVENT TRIGGER event_trigger_disable_test__z ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); + +-- True iff no disable() call is in effect and test triggers a and b are in their initial states +CREATE FUNCTION event_trigger_disable_test__pristine() RETURNS boolean LANGUAGE sql AS $$ + SELECT to_regclass('pg_temp.__object_reference__event_trigger_state') IS NULL + AND (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__a') = 'O' + AND (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__b') = 'D' +$$; +CREATE ROLE event_trigger_disable_test__role; +GRANT USAGE ON SCHEMA _object_reference TO event_trigger_disable_test__role; +GRANT EXECUTE ON FUNCTION _object_reference._event_trigger__lock(name) TO event_trigger_disable_test__role; SELECT plan( 0 +7 -- multi-trigger disable/enable preserves each one's own prior state +3 -- nested disable() without an intervening enable() is rejected +1 -- enable() without a matching disable() is rejected - +1 -- disable() rejects an unknown event trigger name - +1 -- disable() rejects a duplicate name in its own argument list + +2 -- disable() rejects an unknown event trigger name, leaving state untouched + +2 -- disable() rejects a duplicate name in its own argument list, leaving state untouched + +6 -- disable() rejects NULL/empty/NULL-element arguments, leaving state untouched + +6 -- argument validation takes precedence over the "previous call in effect" check + +4 -- names passed unsorted are processed in "C" order + +3 -- _event_trigger__lock() falls back to an advisory lock for a role that cannot row-lock the catalog +5 -- zzz_object_reference_capture self-recognizes and stands down while a disable() is in effect +2 -- schema-qualification (search_path) ); @@ -91,11 +108,12 @@ SELECT throws_ok( -- Unknown event trigger name SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable('{no_such_event_trigger}')$$ + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,no_such_event_trigger}')$$ , NULL , 'event trigger "no_such_event_trigger" does not exist' , 'disable() rejects an unknown event trigger name' ); +SELECT ok(event_trigger_disable_test__pristine(), 'unknown name: nothing was changed'); -- Duplicate name in the same call SELECT throws_ok( @@ -104,6 +122,126 @@ SELECT throws_ok( , 'event_trigger_names contains a duplicate name' , 'disable() rejects a duplicate name in its own argument list' ); +SELECT ok(event_trigger_disable_test__pristine(), 'duplicate name: nothing was changed'); + +/* + * Argument validation runs before any side effect. (A raised error rolls + * the call back anyway, so "nothing was changed" can only be observed after + * the fact; the precedence checks below show the argument errors win over + * the "previous call in effect" error that was the first check to run once.) + */ +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable(NULL)$$ + , NULL + , 'event_trigger_names must be a non-empty array' + , 'disable() rejects a NULL array' +); +SELECT ok(event_trigger_disable_test__pristine(), 'NULL array: nothing was changed'); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{}')$$ + , NULL + , 'event_trigger_names must be a non-empty array' + , 'disable() rejects an empty array' +); +SELECT ok(event_trigger_disable_test__pristine(), 'empty array: nothing was changed'); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,NULL}')$$ + , NULL + , 'event_trigger_names contains a NULL name' + , 'disable() rejects a NULL element' +); +SELECT ok(event_trigger_disable_test__pristine(), 'NULL element: nothing was changed'); + +-- Argument errors are reported even while a previous call is in effect +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() so a call is in effect' +); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__b,event_trigger_disable_test__b}')$$ + , NULL + , 'event_trigger_names contains a duplicate name' + , 'duplicate name is reported before the in-effect error' +); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{no_such_event_trigger}')$$ + , NULL + , 'event trigger "no_such_event_trigger" does not exist' + , 'unknown name is reported before the in-effect error' +); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable(NULL)$$ + , NULL + , 'event_trigger_names must be a non-empty array' + , 'NULL array is reported before the in-effect error' +); +SELECT is( + (SELECT array_agg(evtname) FROM pg_temp.__object_reference__event_trigger_state) + , '{event_trigger_disable_test__a}'::name[] + , 'the call still in effect kept its own state' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() ends that call' +); + +-- Unsorted input is processed in "C" order +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__z,event_trigger_disable_test__a,"event_trigger_disable_test__B"}')$$ + , 'disable() with unsorted names' +); +SELECT is( + (SELECT array_agg(evtname ORDER BY ctid) FROM pg_temp.__object_reference__event_trigger_state) + , '{event_trigger_disable_test__B,event_trigger_disable_test__a,event_trigger_disable_test__z}'::name[] + , 'names were processed in "C" collation order, not input order' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() after unsorted disable()' +); +SELECT is( + (SELECT array_agg(evtenabled ORDER BY evtname COLLATE "C") FROM pg_catalog.pg_event_trigger WHERE evtname LIKE 'event\_trigger\_disable\_test\_\_%' AND evtname IN ('event_trigger_disable_test__B', 'event_trigger_disable_test__a', 'event_trigger_disable_test__z')) + , '{O,O,O}'::"char"[] + , 'all three restored' +); + +/* + * A role that cannot row-lock pg_event_trigger (as on managed services + * without true superuser) gets an advisory lock instead of an error. The DO + * block runs as that role, and first proves the role really cannot FOR UPDATE + * the catalog. + */ +SELECT lives_ok( + $t$DO $do$ + BEGIN + SET LOCAL ROLE event_trigger_disable_test__role; + BEGIN + PERFORM 1 FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__a' FOR UPDATE; + RAISE 'test role unexpectedly able to row-lock pg_event_trigger'; + EXCEPTION WHEN insufficient_privilege THEN NULL; + END; + PERFORM set_config('event_trigger_disable_test.enabled' + , _object_reference._event_trigger__lock('event_trigger_disable_test__a')::text, true); + RESET ROLE; + END + $do$$t$ + , 'helper does not fail for a role that cannot row-lock the catalog' +); +SELECT is( + current_setting('event_trigger_disable_test.enabled') + , 'O' + , 'helper still returns the trigger''s evtenabled' +); +SELECT is( + (SELECT count(*)::int FROM pg_locks + WHERE locktype = 'advisory' + AND pid = pg_backend_pid() + AND classid::int = 'pg_catalog.pg_event_trigger'::regclass::oid::int + AND objid::int = (SELECT oid::int FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__a') + AND objsubid = 2) + , 1 + , 'helper took the advisory lock keyed on catalog and trigger oid' +); /* * zzz_object_reference_capture / zzz_object_reference__fix_identity check From 1c69fe8358729d350f4320600c01b2ef79691394 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 17:33:30 -0500 Subject: [PATCH 07/35] Address review: comment placement, honest test names, precedence coverage - Move _event_trigger__lock() ahead of the WARNING block so the block documents event_trigger__disable()/__enable() again; state the advisory fallback's actual scope (other callers of disable() only). - Use pg_event_trigger column %TYPE declarations in the functions this PR adds. - Drop the "nothing was changed" assertions (a rolled-back call can't fail them) and add error-precedence tests for an empty array and a NULL element. - Correct the load.sql comment: a failed check does not fail the make run on any PostgreSQL version. --- sql/object_reference--0.1.0--stable.sql | 91 +++++++++++++------------ sql/object_reference.sql | 91 +++++++++++++------------ test/expected/event_trigger_disable.out | 57 ++++++++-------- test/install/load.sql | 5 +- test/sql/event_trigger_disable.sql | 39 +++++------ 5 files changed, 148 insertions(+), 135 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 42d4b2f..312cf69 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -353,6 +353,43 @@ END $$; DROP TABLE pg_temp.__object_reference__bootstrap_event_trigger_state; +-- Helper for event_trigger__disable(); see the WARNING below. +SELECT __object_reference.create_function( + '_object_reference._event_trigger__lock' + , $args$ + event_trigger_name pg_catalog.pg_event_trigger.evtname%TYPE +$args$ + , 'pg_catalog.pg_event_trigger.evtenabled%TYPE LANGUAGE plpgsql' + , $body$ +DECLARE + v_oid pg_catalog.pg_event_trigger.oid%TYPE; + v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; +BEGIN + BEGIN + SELECT evtenabled INTO v_enabled + FROM pg_catalog.pg_event_trigger + WHERE evtname = event_trigger_name + FOR UPDATE + ; + EXCEPTION WHEN insufficient_privilege THEN + -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. + SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + IF FOUND THEN + PERFORM pg_catalog.pg_advisory_xact_lock( + 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int + , v_oid::int + ); + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + END IF; + END; + + -- NULL if the trigger does not exist + RETURN v_enabled; +END +$body$ + , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' +); + /* * WARNING: avoid disabling event triggers at all where any other option * exists. ALTER EVENT TRIGGER is ordinary transactional DDL -- like any @@ -365,11 +402,16 @@ DROP TABLE pg_temp.__object_reference__bootstrap_event_trigger_state; * first the way any two concurrent writes to the same catalog row would, * and without care, the one that unblocks second can record and later * restore a "prior state" that was never actually the trigger's state - * immediately before it acted (see _event_trigger__lock() - * below, which exists specifically to close that gap). Prefer a self-recognition check (a session-local flag, checked - * from inside the trigger's own body, as _etg_fix_identity/_etg_capture - * above do) over calling this at all; reach for it only when nothing else - * can make the trigger stay quiet, as is currently true for + * immediately before it acted. disable() calls _event_trigger__lock() to + * close that gap: with catalog UPDATE privilege it locks the trigger's + * pg_event_trigger row FOR UPDATE, which also blocks a raw ALTER EVENT + * TRIGGER by another session. Without that privilege (e.g. managed + * services) it falls back to an advisory lock, which serializes only other + * callers of disable(), not a raw ALTER EVENT TRIGGER elsewhere. Prefer a + * self-recognition check (a session-local flag, checked from inside the + * trigger's own body, as _etg_fix_identity/_etg_capture above do) over + * calling this at all; reach for it only when nothing else can make the + * trigger stay quiet, as is currently true for * zzz__object_reference_drop. * * General-purpose event-trigger disable/enable mechanism, replacing the @@ -390,41 +432,6 @@ DROP TABLE pg_temp.__object_reference__bootstrap_event_trigger_state; * else a future need to safely quiet an event trigger comes up (hence the * mechanism-focused name, not one tied to "being mid-update"). */ -SELECT __object_reference.create_function( - '_object_reference._event_trigger__lock' - , $args$ - event_trigger_name name -$args$ - , '"char" LANGUAGE plpgsql' - , $body$ -DECLARE - v_oid oid; - v_enabled "char"; -BEGIN - BEGIN - SELECT evtenabled INTO v_enabled - FROM pg_catalog.pg_event_trigger - WHERE evtname = event_trigger_name - FOR UPDATE - ; - EXCEPTION WHEN insufficient_privilege THEN - -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. - SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - IF FOUND THEN - PERFORM pg_catalog.pg_advisory_xact_lock( - 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int - , v_oid::int - ); - SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - END IF; - END; - - -- NULL if the trigger does not exist - RETURN v_enabled; -END -$body$ - , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' -); SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -433,8 +440,8 @@ $args$ , 'void LANGUAGE plpgsql' , $body$ DECLARE - v_name name; - v_enabled "char"; + v_name pg_catalog.pg_event_trigger.evtname%TYPE; + v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN /* * WARNING: avoid disabling event triggers at all where any other option diff --git a/sql/object_reference.sql b/sql/object_reference.sql index ed0c6be..a7c035f 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1611,6 +1611,43 @@ $body$ , 'Event trigger function to drop object records when objects are removed.' ); +-- Helper for event_trigger__disable(); see the WARNING below. +SELECT __object_reference.create_function( + '_object_reference._event_trigger__lock' + , $args$ + event_trigger_name pg_catalog.pg_event_trigger.evtname%TYPE +$args$ + , 'pg_catalog.pg_event_trigger.evtenabled%TYPE LANGUAGE plpgsql' + , $body$ +DECLARE + v_oid pg_catalog.pg_event_trigger.oid%TYPE; + v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; +BEGIN + BEGIN + SELECT evtenabled INTO v_enabled + FROM pg_catalog.pg_event_trigger + WHERE evtname = event_trigger_name + FOR UPDATE + ; + EXCEPTION WHEN insufficient_privilege THEN + -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. + SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + IF FOUND THEN + PERFORM pg_catalog.pg_advisory_xact_lock( + 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int + , v_oid::int + ); + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; + END IF; + END; + + -- NULL if the trigger does not exist + RETURN v_enabled; +END +$body$ + , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' +); + /* * WARNING: avoid disabling event triggers at all where any other option * exists. ALTER EVENT TRIGGER is ordinary transactional DDL -- like any @@ -1623,11 +1660,16 @@ $body$ * first the way any two concurrent writes to the same catalog row would, * and without care, the one that unblocks second can record and later * restore a "prior state" that was never actually the trigger's state - * immediately before it acted (see _event_trigger__lock() - * below, which exists specifically to close that gap). Prefer a self-recognition check (a session-local flag, checked - * from inside the trigger's own body) over calling this at all; reach for - * it only when nothing else can make the trigger stay quiet, as is - * currently true for zzz__object_reference_drop. + * immediately before it acted. disable() calls _event_trigger__lock() to + * close that gap: with catalog UPDATE privilege it locks the trigger's + * pg_event_trigger row FOR UPDATE, which also blocks a raw ALTER EVENT + * TRIGGER by another session. Without that privilege (e.g. managed + * services) it falls back to an advisory lock, which serializes only other + * callers of disable(), not a raw ALTER EVENT TRIGGER elsewhere. Prefer a + * self-recognition check (a session-local flag, checked from inside the + * trigger's own body) over calling this at all; reach for it only when + * nothing else can make the trigger stay quiet, as is currently true for + * zzz__object_reference_drop. * * General-purpose event-trigger disable/enable mechanism, for use by this * extension's OWN install/update scripts only (not part of the public API). @@ -1648,41 +1690,6 @@ $body$ * script's transaction rolls back, the DISABLE (and any ENABLE already run) * rolls back with it -- no separate cleanup-on-error logic is needed here. */ -SELECT __object_reference.create_function( - '_object_reference._event_trigger__lock' - , $args$ - event_trigger_name name -$args$ - , '"char" LANGUAGE plpgsql' - , $body$ -DECLARE - v_oid oid; - v_enabled "char"; -BEGIN - BEGIN - SELECT evtenabled INTO v_enabled - FROM pg_catalog.pg_event_trigger - WHERE evtname = event_trigger_name - FOR UPDATE - ; - EXCEPTION WHEN insufficient_privilege THEN - -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. - SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - IF FOUND THEN - PERFORM pg_catalog.pg_advisory_xact_lock( - 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int - , v_oid::int - ); - SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - END IF; - END; - - -- NULL if the trigger does not exist - RETURN v_enabled; -END -$body$ - , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' -); SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -1691,8 +1698,8 @@ $args$ , 'void LANGUAGE plpgsql' , $body$ DECLARE - v_name name; - v_enabled "char"; + v_name pg_catalog.pg_event_trigger.evtname%TYPE; + v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN /* * WARNING: avoid disabling event triggers at all where any other option diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 74623c3..9e38888 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..41 +1..38 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -12,33 +12,30 @@ ok 9 - a second disable() without enable() in between is rejected ok 10 - enable() cleans up so later tests are unaffected ok 11 - enable() without disable() is rejected ok 12 - disable() rejects an unknown event trigger name -ok 13 - unknown name: nothing was changed -ok 14 - disable() rejects a duplicate name in its own argument list -ok 15 - duplicate name: nothing was changed -ok 16 - disable() rejects a NULL array -ok 17 - NULL array: nothing was changed -ok 18 - disable() rejects an empty array -ok 19 - empty array: nothing was changed -ok 20 - disable() rejects a NULL element -ok 21 - NULL element: nothing was changed -ok 22 - disable() so a call is in effect -ok 23 - duplicate name is reported before the in-effect error -ok 24 - unknown name is reported before the in-effect error -ok 25 - NULL array is reported before the in-effect error -ok 26 - the call still in effect kept its own state -ok 27 - enable() ends that call -ok 28 - disable() with unsorted names -ok 29 - names were processed in "C" collation order, not input order -ok 30 - enable() after unsorted disable() -ok 31 - all three restored -ok 32 - helper does not fail for a role that cannot row-lock the catalog -ok 33 - helper still returns the trigger's evtenabled -ok 34 - helper took the advisory lock keyed on catalog and trigger oid -ok 35 - start a capture group -ok 36 - disable() (any trigger) also signals self-recognizing triggers to stand down -ok 37 - enable() ends that window -ok 38 - the table created while disable() was in effect was NOT captured -ok 39 - stop the capture group -ok 40 - object_reference schema must not be part of the resolved search_path -ok 41 - _object_reference schema must not be part of the resolved search_path +ok 13 - disable() rejects a duplicate name in its own argument list +ok 14 - disable() rejects a NULL array +ok 15 - disable() rejects an empty array +ok 16 - disable() rejects a NULL element +ok 17 - disable() so a call is in effect +ok 18 - duplicate name is reported before the in-effect error +ok 19 - unknown name is reported before the in-effect error +ok 20 - NULL array is reported before the in-effect error +ok 21 - empty array is reported before the in-effect error +ok 22 - NULL element is reported before the in-effect error +ok 23 - the call still in effect kept its own state +ok 24 - enable() ends that call +ok 25 - disable() with unsorted names +ok 26 - names were processed in "C" collation order, not input order +ok 27 - enable() after unsorted disable() +ok 28 - all three restored +ok 29 - helper does not fail for a role that cannot row-lock the catalog +ok 30 - helper still returns the trigger's evtenabled +ok 31 - helper took the advisory lock keyed on catalog and trigger oid +ok 32 - start a capture group +ok 33 - disable() (any trigger) also signals self-recognizing triggers to stand down +ok 34 - enable() ends that window +ok 35 - the table created while disable() was in effect was NOT captured +ok 36 - stop the capture group +ok 37 - object_reference schema must not be part of the resolved search_path +ok 38 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/install/load.sql b/test/install/load.sql index eac5ad0..b552ddd 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -306,8 +306,9 @@ CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' C /* * Fail unless the update restores each of the extension's event triggers to * the exact state it had before, starting each from a distinct non-default - * state. A failed check exits psql nonzero (ON_ERROR_STOP), which fails the - * run only under pg_regress 16+; PG12's pg_regress reports it as ok. This + * state. A failed check exits psql nonzero (ON_ERROR_STOP), but that does not + * fail the make run on any PostgreSQL version: pg_regress 16+ prints + * "not ok" and PG12's prints "ok", and make exits 0 either way. This * file's own output is never compared against anything. All three are reset * to ENABLE afterwards, so the rest of the suite runs as usual. */ diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index f0fbf5c..3c0368b 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -23,12 +23,6 @@ CREATE EVENT TRIGGER event_trigger_disable_test__b ON ddl_command_start EXECUTE CREATE EVENT TRIGGER "event_trigger_disable_test__B" ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); CREATE EVENT TRIGGER event_trigger_disable_test__z ON ddl_command_start EXECUTE FUNCTION event_trigger_disable_test__noop(); --- True iff no disable() call is in effect and test triggers a and b are in their initial states -CREATE FUNCTION event_trigger_disable_test__pristine() RETURNS boolean LANGUAGE sql AS $$ - SELECT to_regclass('pg_temp.__object_reference__event_trigger_state') IS NULL - AND (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__a') = 'O' - AND (SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__b') = 'D' -$$; CREATE ROLE event_trigger_disable_test__role; GRANT USAGE ON SCHEMA _object_reference TO event_trigger_disable_test__role; GRANT EXECUTE ON FUNCTION _object_reference._event_trigger__lock(name) TO event_trigger_disable_test__role; @@ -38,10 +32,10 @@ SELECT plan( +7 -- multi-trigger disable/enable preserves each one's own prior state +3 -- nested disable() without an intervening enable() is rejected +1 -- enable() without a matching disable() is rejected - +2 -- disable() rejects an unknown event trigger name, leaving state untouched - +2 -- disable() rejects a duplicate name in its own argument list, leaving state untouched - +6 -- disable() rejects NULL/empty/NULL-element arguments, leaving state untouched - +6 -- argument validation takes precedence over the "previous call in effect" check + +1 -- disable() rejects an unknown event trigger name + +1 -- disable() rejects a duplicate name in its own argument list + +3 -- disable() rejects a NULL array, an empty array and a NULL element + +8 -- argument errors are reported in preference to the "previous call in effect" error +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() falls back to an advisory lock for a role that cannot row-lock the catalog +5 -- zzz_object_reference_capture self-recognizes and stands down while a disable() is in effect @@ -113,7 +107,6 @@ SELECT throws_ok( , 'event trigger "no_such_event_trigger" does not exist' , 'disable() rejects an unknown event trigger name' ); -SELECT ok(event_trigger_disable_test__pristine(), 'unknown name: nothing was changed'); -- Duplicate name in the same call SELECT throws_ok( @@ -122,13 +115,12 @@ SELECT throws_ok( , 'event_trigger_names contains a duplicate name' , 'disable() rejects a duplicate name in its own argument list' ); -SELECT ok(event_trigger_disable_test__pristine(), 'duplicate name: nothing was changed'); /* - * Argument validation runs before any side effect. (A raised error rolls - * the call back anyway, so "nothing was changed" can only be observed after - * the fact; the precedence checks below show the argument errors win over - * the "previous call in effect" error that was the first check to run once.) + * Argument errors are reported by disable() itself; any state change made + * before the error would be rolled back with the call, so these tests cannot + * observe whether validation ran before a side effect. What they can observe + * is which error wins when several apply (see the precedence tests below). */ SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable(NULL)$$ @@ -136,21 +128,18 @@ SELECT throws_ok( , 'event_trigger_names must be a non-empty array' , 'disable() rejects a NULL array' ); -SELECT ok(event_trigger_disable_test__pristine(), 'NULL array: nothing was changed'); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{}')$$ , NULL , 'event_trigger_names must be a non-empty array' , 'disable() rejects an empty array' ); -SELECT ok(event_trigger_disable_test__pristine(), 'empty array: nothing was changed'); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,NULL}')$$ , NULL , 'event_trigger_names contains a NULL name' , 'disable() rejects a NULL element' ); -SELECT ok(event_trigger_disable_test__pristine(), 'NULL element: nothing was changed'); -- Argument errors are reported even while a previous call is in effect SELECT lives_ok( @@ -175,6 +164,18 @@ SELECT throws_ok( , 'event_trigger_names must be a non-empty array' , 'NULL array is reported before the in-effect error' ); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{}')$$ + , NULL + , 'event_trigger_names must be a non-empty array' + , 'empty array is reported before the in-effect error' +); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__b,NULL}')$$ + , NULL + , 'event_trigger_names contains a NULL name' + , 'NULL element is reported before the in-effect error' +); SELECT is( (SELECT array_agg(evtname) FROM pg_temp.__object_reference__event_trigger_state) , '{event_trigger_disable_test__a}'::name[] From 266688aaedfa69016b4d9271bd01d96c64655b4a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 31 Oct 2026 18:42:01 -0500 Subject: [PATCH 08/35] Disable all three event triggers in updates, with a sentinel event_trigger__disable() now creates a no-op ddl_command_end event trigger on a pg_temp function before disabling anything, and event_trigger__enable() drops it after restoring every state. While every sql_drop/table_rewrite/ddl_command_end trigger is disabled, Postgres collects the commands nested in an extension script onto the enclosing ALTER EXTENSION and replays them to its ddl_command_end, where pg_event_trigger_ddl_commands() fails on dropped objects before PG14 and hands capture the update's own objects on PG14+. The 0.1.0 -> stable update script disables all three event triggers at its start (by hand, since 0.1.0 has no disable()) and restores them with one enable() at the end. _etg_capture and _etg_fix_identity no longer recognize the update script themselves, so their 0.1.0 bodies are kept unchanged. _event_trigger__lock() no longer falls back to an advisory lock: on insufficient_privilege it reads evtenabled without a lock. disable() and enable() raise undefined_object, invalid_parameter_value or object_not_in_prerequisite_state as appropriate, and the tests check the SQLSTATE. --- sql/object_reference--0.1.0--stable.sql | 386 ++++++------------------ sql/object_reference.sql | 199 ++++-------- test/expected/event_trigger_disable.out | 74 ++--- test/install/load.sql | 22 +- test/sql/event_trigger_disable.sql | 109 +++---- 5 files changed, 261 insertions(+), 529 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 312cf69..fd015f0 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -1,25 +1,33 @@ -/* - * Immediately disable zzz_object_reference_capture and - * zzz_object_reference__fix_identity (raw ALTER EVENT TRIGGER -- nothing - * else, not even the __object_reference bootstrap schema below, exists yet - * to route through) for the handful of bootstrap statements that follow: - * their OLD (0.1.0) bodies have no way to recognize "this is our own - * update script" until their guarded replacements are installed a few - * statements down, and CREATE SCHEMA __object_reference is exactly the - * kind of CREATE-tagged statement _etg_capture would otherwise try (and - * fail) to register into any capture group active during this update. - * Their actual prior state is captured into a temp table (rather than - * assumed 'O') and restored below, so a DBA's own prior DISABLE of either - * trigger survives this update instead of being silently overwritten. - */ -CREATE TEMP TABLE __object_reference__bootstrap_event_trigger_state AS - SELECT evtname, evtenabled - FROM pg_catalog.pg_event_trigger - WHERE evtname IN ('zzz_object_reference_capture', 'zzz_object_reference__fix_identity') +-- event_trigger__disable() of all three event triggers, by hand only because 0.1.0 has no disable(); the event_trigger__enable() at the end undoes it. +CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; +CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel + ON ddl_command_end + EXECUTE PROCEDURE pg_temp.__object_reference__event_trigger_sentinel() ; - -ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; -ALTER EVENT TRIGGER zzz_object_reference__fix_identity DISABLE; +CREATE TEMP TABLE __object_reference__event_trigger_state AS + SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false +; +ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); +DO $$ +DECLARE + v_name pg_catalog.pg_event_trigger.evtname%TYPE; + v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; +BEGIN + FOR v_name IN + SELECT n + FROM unnest('{zzz__object_reference_drop,zzz_object_reference__fix_identity,zzz_object_reference_capture}'::name[]) n + ORDER BY n COLLATE "C" + LOOP + BEGIN + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = v_name FOR UPDATE; + EXCEPTION WHEN insufficient_privilege THEN + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = v_name; + END; + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) VALUES (v_name, v_enabled); + EXECUTE format('ALTER EVENT TRIGGER %I DISABLE', v_name); + END LOOP; +END +$$; /* * Uses a private __object_reference schema, mirroring @@ -168,192 +176,28 @@ $body$ ); /* - * _etg_fix_identity/_etg_capture: gain a self-recognition guard so they skip - * work while this extension's own event_trigger__disable() call is in - * effect for this session (checked via to_regclass() on the temp table - * event_trigger__disable() creates below) -- i.e. this extension's own - * install/update script is doing delicate internal restructuring right - * now. Installed here, ahead of the structural section below, specifically - * so the guard is already active by the time event_trigger__disable() is - * first called a few statements down: recreating them any later would - * leave the OLD (0.1.0), unguarded bodies live for that whole window -- - * which mattered in practice for _etg_capture, which would otherwise try - * to call _object_reference._object_v__for_update() (the FUNCTION) for any - * CREATE-tagged command in this script if a capture happened to be active, - * including a moment where that function has been dropped and not yet - * recreated, which would fail outright. Same signatures 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._etg_capture' - , '' - , 'event_trigger SECURITY DEFINER LANGUAGE plpgsql' - , $body$ -DECLARE - c_group_id CONSTANT int := object_group_id FROM object_reference.capture__get_current(); - r record; -BEGIN - /* - * Self-recognition: skip while this extension's own event_trigger__disable() - * is in effect (see below) -- i.e. this extension's own install/update - * script is doing delicate internal restructuring right now. Checked via - * to_regclass() rather than a catalog lookup that would error if the temp - * table doesn't exist, which is the common case. - */ - IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN - RETURN; - END IF; - - IF c_group_id IS NOT NULL THEN -- Would be NULL if table is empty - RAISE DEBUG E'\n\n*** START ***'; - BEGIN - FOR r IN - SELECT classid, objid, objsubid, command_tag, object_type, schema_name, object_identity, in_extension - -- Have to manually exclude command field :/ - FROM pg_catalog.pg_event_trigger_ddl_commands() - LOOP - RAISE DEBUG 'ddl: %', row_to_json(r); - END LOOP; - END; - - FOR r IN SELECT - _object_reference._object_v__for_update( - object_type::cat_tools.object_type - , objid, objsubid - , c_group_id - , classid - ) - , classid, objid, objsubid, command_tag, object_type, schema_name, object_identity, in_extension - FROM pg_catalog.pg_event_trigger_ddl_commands() - WHERE command_tag ~ '^CREATE' --'^(ALTER|CREATE)' - AND NOT object_reference.unsupported(object_type::cat_tools.object_type) - AND (schema_name IS NULL - OR schema_name NOT LIKE 'pg_temp%' -- pg_my_temp_schema() doesn't seem worth it... - ) - /* - * __object_reference is this extension's own scratch install/update - * schema (created and dropped within a single script, never an - * extension member) -- self-recognition via the temp table above - * can't cover the handful of bootstrap statements that run before - * that table exists, so exclude it here too (object_identity - * carries the name for the CREATE SCHEMA statement itself, where - * schema_name is null). - * - * object_reference/_object_reference are excluded outright rather - * than relying on _object_v__for_update()'s own _is_own_object() - * guard: a brand-new object created by this extension's own - * update/install script isn't yet recorded as an 'e' member in - * pg_depend at the point its CREATE fires ddl_command_end (that - * happens once the surrounding CREATE/ALTER EXTENSION completes), - * so _is_own_object() can't see it as self-owned yet either -- - * confirmed by running into it: an active capture group during - * ALTER EXTENSION UPDATE otherwise ends up with this extension's - * own new functions as members. - * - * The schema-creation statement itself (CREATE SCHEMA - * __object_reference/etc.) has schema_name = NULL, with the name - * only available via object_identity -- checked separately, and - * restricted to object_type = 'schema', so an unrelated object of - * some other type whose identity happens to match one of these - * three exact strings (e.g. a same-named extension) isn't caught - * by this fallback. - */ - AND coalesce(schema_name, '') NOT IN ('__object_reference', 'object_reference', '_object_reference') - AND NOT (object_type = 'schema' AND object_identity IN ('__object_reference', 'object_reference', '_object_reference')) - LOOP - RAISE DEBUG 'registered %', row_to_json(r); - END LOOP; - RAISE DEBUG E'*** END ***\n\n'; - END IF; -END -$body$ - , 'Event trigger function to capture newly created objects in an object group.' -); -SELECT __object_reference.create_function( - '_object_reference._etg_fix_identity' - , '' - , 'event_trigger SECURITY DEFINER LANGUAGE plpgsql' - , $body$ -DECLARE - r_ddl record; - r record; -BEGIN - /* - * Self-recognition: skip while this extension's own event_trigger__disable() - * is in effect (see below) -- i.e. this extension's own install/update - * script is doing delicate internal restructuring right now. Checked via - * to_regclass() rather than a catalog lookup that would error if the temp - * table doesn't exist, which is the common case. - */ - IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN - RETURN; - END IF; - - /* - * It's tempting to use pg_event_trigger_ddl_commands() to find exactly what - * items have changed and worry about only those. That won't work because an - * object_names array can depend on multiple names (ie: a column depends on - * the name of it's table, as well as the name of the schema the table is in. - * You might think we could simply recurse through pg_depend to handle this, - * but not every name dependency gets enumerated that way. For example, - * columns are not marked as dependent on their table. - * - * Rather than trying to be cute about this, we just do a brute-force check - * for any names that have changed. - */ - - /* - * Presumably there's no way for an objects type/classid to change, but be - * safe and attempt the update to object_type. If it actually does change the - * constraint on the table should catch it. - */ - FOR r IN - UPDATE _object_reference.object - SET object_type = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).type::cat_tools.object_type - , object_names = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_names - , object_args = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_args - FROM _object_reference._object_oid oo - WHERE - oo.object_id = object.object_id - AND (object_type::text, object_names, object_args) IS DISTINCT FROM - (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)) - RETURNING * - LOOP - RAISE DEBUG 'modified_objects(): %', r; - END LOOP; -END -$body$ - , 'Event trigger function to update any records with object names or args that have changed.' -); - -/* - * Restore each trigger's actual prior state (captured above) now that the - * guarded bodies above are live -- self-recognition (checking for - * event_trigger__disable()'s temp table, created below) takes over from - * here for the rest of this script. + * event_trigger__disable(names) disables the given event triggers, and + * event_trigger__enable() restores each one's exact prior state. Both are + * internal: for this extension's own update scripts, not the public API. + * + * Avoid disabling event triggers wherever another option exists. The change + * is transactional: other sessions don't see it until commit, and it rolls + * back with the caller's transaction, so no cleanup-on-error logic is needed. + * disable() row-locks each trigger's catalog row before reading its state + * (see _event_trigger__lock() below), which blocks a concurrent raw ALTER + * EVENT TRIGGER of the same trigger until commit. + * + * disable() also creates a no-op ddl_command_end event trigger, the sentinel, + * which enable() drops after restoring every state. While every sql_drop, + * table_rewrite and ddl_command_end event trigger is disabled, Postgres gives + * the commands nested in an extension script no event trigger state of their + * own: it collects them onto the enclosing CREATE/ALTER EXTENSION and replays + * them to that command's ddl_command_end once the triggers are back. There, + * pg_event_trigger_ddl_commands() fails on objects dropped since (before + * PostgreSQL 14) or hands capture the script's own objects. One enabled event + * trigger prevents the collection. The sentinel's function is in pg_temp, + * which capture skips. */ -DO $$ -DECLARE - r record; -BEGIN - FOR r IN SELECT evtname, evtenabled FROM pg_temp.__object_reference__bootstrap_event_trigger_state LOOP - EXECUTE format( - 'ALTER EVENT TRIGGER %I %s' - , r.evtname - , CASE r.evtenabled - WHEN 'O' THEN 'ENABLE' - WHEN 'R' THEN 'ENABLE REPLICA' - WHEN 'A' THEN 'ENABLE ALWAYS' - WHEN 'D' THEN 'DISABLE' - END - ); - END LOOP; -END -$$; -DROP TABLE pg_temp.__object_reference__bootstrap_event_trigger_state; - --- Helper for event_trigger__disable(); see the WARNING below. SELECT __object_reference.create_function( '_object_reference._event_trigger__lock' , $args$ @@ -362,7 +206,6 @@ $args$ , 'pg_catalog.pg_event_trigger.evtenabled%TYPE LANGUAGE plpgsql' , $body$ DECLARE - v_oid pg_catalog.pg_event_trigger.oid%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN BEGIN @@ -372,66 +215,18 @@ BEGIN FOR UPDATE ; EXCEPTION WHEN insufficient_privilege THEN - -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. - SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - IF FOUND THEN - PERFORM pg_catalog.pg_advisory_xact_lock( - 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int - , v_oid::int - ); - SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - END IF; + -- Without catalog UPDATE rights the row can't be locked; the remaining race needs a concurrent raw ALTER of the same trigger during an administrator's own update. + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; -- NULL if the trigger does not exist RETURN v_enabled; END $body$ - , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' + , 'Row-lock one event trigger where privileges allow, and return its current evtenabled (NULL if it does not exist).' ); -/* - * WARNING: avoid disabling event triggers at all where any other option - * exists. ALTER EVENT TRIGGER is ordinary transactional DDL -- like any - * other catalog write, it's invisible to other sessions until commit (no - * special database-wide/immediate effect: verified empirically that a - * concurrent session's DDL neither blocks on, nor is otherwise affected by, - * another session's still-uncommitted DISABLE) and it takes no lock at all - * on the event trigger itself. The real risk is TWO SESSIONS both trying to - * alter the SAME event trigger concurrently: a second writer blocks on the - * first the way any two concurrent writes to the same catalog row would, - * and without care, the one that unblocks second can record and later - * restore a "prior state" that was never actually the trigger's state - * immediately before it acted. disable() calls _event_trigger__lock() to - * close that gap: with catalog UPDATE privilege it locks the trigger's - * pg_event_trigger row FOR UPDATE, which also blocks a raw ALTER EVENT - * TRIGGER by another session. Without that privilege (e.g. managed - * services) it falls back to an advisory lock, which serializes only other - * callers of disable(), not a raw ALTER EVENT TRIGGER elsewhere. Prefer a - * self-recognition check (a session-local flag, checked from inside the - * trigger's own body, as _etg_fix_identity/_etg_capture above do) over - * calling this at all; reach for it only when nothing else can make the - * trigger stay quiet, as is currently true for - * zzz__object_reference_drop. - * - * General-purpose event-trigger disable/enable mechanism, replacing the - * session_replication_role trick 0.1.0 had no equivalent of. 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. It can't self-recognize the way _etg_fix_identity/ - * _etg_capture above do without also touching that same view from inside - * its own body, so it must be truly disabled for the duration of this - * script's structural section. - * - * These are created now, ahead of the structural section, specifically so - * this script itself can call event_trigger__disable() below -- a fresh - * install only ever needs these for FUTURE update scripts, or anywhere - * else a future need to safely quiet an event trigger comes up (hence the - * mechanism-focused name, not one tied to "being mid-update"). - */ +-- name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -443,23 +238,25 @@ DECLARE v_name pg_catalog.pg_event_trigger.evtname%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN - /* - * WARNING: avoid disabling event triggers at all where any other option - * exists -- this is a database-wide change with real race-condition risk - * against concurrent sessions' DDL. See the warning above this function. - */ - -- Validate everything before any side effect. IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN - RAISE 'event_trigger_names must be a non-empty array'; + RAISE 'event_trigger_names must be a non-empty array' + USING ERRCODE = 'invalid_parameter_value' + ; END IF; IF array_position(event_trigger_names, NULL) IS NOT NULL THEN - RAISE 'event_trigger_names contains a NULL name' USING DETAIL = event_trigger_names::text; + RAISE 'event_trigger_names contains a NULL name' + USING ERRCODE = 'invalid_parameter_value' + , DETAIL = event_trigger_names::text + ; END IF; IF array_length(event_trigger_names, 1) <> (SELECT count(DISTINCT x) FROM unnest(event_trigger_names) x) THEN - RAISE 'event_trigger_names contains a duplicate name' USING DETAIL = event_trigger_names::text; + RAISE 'event_trigger_names contains a duplicate name' + USING ERRCODE = 'invalid_parameter_value' + , DETAIL = event_trigger_names::text + ; END IF; SELECT n INTO v_name @@ -469,16 +266,25 @@ BEGIN LIMIT 1 ; IF FOUND THEN - RAISE 'event trigger "%" does not exist', v_name; + RAISE 'event trigger "%" does not exist', v_name + USING ERRCODE = 'undefined_object' + ; END IF; IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN RAISE 'event_trigger__disable() called while a previous call is still in effect' - USING HINT = 'A previous event_trigger__enable() call may have been skipped.' + USING ERRCODE = 'object_not_in_prerequisite_state' + , HINT = 'A previous event_trigger__enable() call may have been skipped.' ; END IF; - -- Save old trigger state + -- The sentinel (see above), before anything is disabled + CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; + CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel + ON ddl_command_end + EXECUTE PROCEDURE pg_temp.__object_reference__event_trigger_sentinel() + ; + CREATE TEMP TABLE __object_reference__event_trigger_state AS SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false ; @@ -486,18 +292,13 @@ BEGIN -- Fixed lock order across all callers, to avoid deadlock. FOR v_name IN SELECT n FROM unnest(event_trigger_names) n ORDER BY n COLLATE "C" LOOP - /* - * The lock must be held before we read evtenabled, so no other session's - * ALTER EVENT TRIGGER on the same trigger can land between our read and - * our DISABLE below -- otherwise we could record (and later restore) a - * state that was never the trigger's state immediately before we - * disabled it. - */ v_enabled := _object_reference._event_trigger__lock(v_name); -- Dropped by another session since the check above IF v_enabled IS NULL THEN - RAISE 'event trigger "%" does not exist', v_name; + RAISE 'event trigger "%" does not exist', v_name + USING ERRCODE = 'undefined_object' + ; END IF; INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) @@ -520,7 +321,9 @@ DECLARE i int; BEGIN IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NULL THEN - RAISE 'event_trigger__enable() called without a matching event_trigger__disable()'; + RAISE 'event_trigger__enable() called without a matching event_trigger__disable()' + USING ERRCODE = 'object_not_in_prerequisite_state' + ; END IF; -- Same order disable() locked in @@ -529,16 +332,7 @@ BEGIN FROM pg_temp.__object_reference__event_trigger_state ; - /* - * Drop our own bookkeeping table BEFORE re-enabling anything below: - * dropping it is itself DDL, and if zzz__object_reference_drop is one of - * the triggers being restored here, re-enabling it first would make this - * DROP immediately fire it -- reacting to our own internal cleanup, - * exactly the hazard this whole mechanism exists to avoid. (Confirmed by - * running into it: with the table dropped after, an active capture group - * elsewhere left a stale tracked row that this DROP's cascade into - * post_restore() then found and errored on.) - */ + -- Dropped before anything is restored, so this DROP fires none of the restored triggers. DROP TABLE pg_temp.__object_reference__event_trigger_state; FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP @@ -553,13 +347,15 @@ BEGIN END )); END LOOP; + + -- Only now that every state is restored (see the sentinel note above) + DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; + DROP FUNCTION pg_temp.__object_reference__event_trigger_sentinel(); END $body$ , 'Restore event triggers disabled by event_trigger__disable() to their exact prior state.' ); -SELECT _object_reference.event_trigger__disable('{zzz__object_reference_drop}'); - /* * _object_reference.object: no column changes, just a missing * extension_config_dump marking on its sequence (added alongside the table @@ -1114,11 +910,7 @@ DROP FUNCTION __object_reference.exec( ); DROP SCHEMA __object_reference; -/* - * Re-enable zzz__object_reference_drop (to its actual prior state, saved by - * event_trigger__disable() near the top of this script), now that the - * structural section and its cleanup are both done. - */ +-- Restores the event triggers disabled at the top of this script. SELECT _object_reference.event_trigger__enable(); -- vi: expandtab sw=2 ts=2 diff --git a/sql/object_reference.sql b/sql/object_reference.sql index a7c035f..60f4de3 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1428,16 +1428,6 @@ DECLARE c_group_id CONSTANT int := object_group_id FROM object_reference.capture__get_current(); r record; BEGIN - /* - * Self-recognition: skip while this extension's own event_trigger__disable() - * is in effect (see below) -- i.e. this extension's own install/update - * script is doing delicate internal restructuring right now. Checked via - * to_regclass() rather than a catalog lookup that would error if the temp - * table doesn't exist, which is the common case. - */ - IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN - RETURN; - END IF; IF c_group_id IS NOT NULL THEN -- Would be NULL if table is empty RAISE DEBUG E'\n\n*** START ***'; @@ -1451,7 +1441,7 @@ BEGIN END LOOP; END; - FOR r IN SELECT + FOR r IN SELECT _object_reference._object_v__for_update( object_type::cat_tools.object_type , objid, objsubid @@ -1465,36 +1455,6 @@ BEGIN AND (schema_name IS NULL OR schema_name NOT LIKE 'pg_temp%' -- pg_my_temp_schema() doesn't seem worth it... ) - /* - * __object_reference is this extension's own scratch install/update - * schema (created and dropped within a single script, never an - * extension member) -- self-recognition via the temp table above - * can't cover the handful of bootstrap statements that run before - * that table exists, so exclude it here too (object_identity - * carries the name for the CREATE SCHEMA statement itself, where - * schema_name is null). - * - * object_reference/_object_reference are excluded outright rather - * than relying on _object_v__for_update()'s own _is_own_object() - * guard: a brand-new object created by this extension's own - * update/install script isn't yet recorded as an 'e' member in - * pg_depend at the point its CREATE fires ddl_command_end (that - * happens once the surrounding CREATE/ALTER EXTENSION completes), - * so _is_own_object() can't see it as self-owned yet either -- - * confirmed by running into it: an active capture group during - * ALTER EXTENSION UPDATE otherwise ends up with this extension's - * own new functions as members. - * - * The schema-creation statement itself (CREATE SCHEMA - * __object_reference/etc.) has schema_name = NULL, with the name - * only available via object_identity -- checked separately, and - * restricted to object_type = 'schema', so an unrelated object of - * some other type whose identity happens to match one of these - * three exact strings (e.g. a same-named extension) isn't caught - * by this fallback. - */ - AND coalesce(schema_name, '') NOT IN ('__object_reference', 'object_reference', '_object_reference') - AND NOT (object_type = 'schema' AND object_identity IN ('__object_reference', 'object_reference', '_object_reference')) LOOP RAISE DEBUG 'registered %', row_to_json(r); END LOOP; @@ -1515,17 +1475,6 @@ DECLARE r_ddl record; r record; BEGIN - /* - * Self-recognition: skip while this extension's own event_trigger__disable() - * is in effect (see below) -- i.e. this extension's own install/update - * script is doing delicate internal restructuring right now. Checked via - * to_regclass() rather than a catalog lookup that would error if the temp - * table doesn't exist, which is the common case. - */ - IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN - RETURN; - END IF; - /* * It's tempting to use pg_event_trigger_ddl_commands() to find exactly what * items have changed and worry about only those. That won't work because an @@ -1611,7 +1560,29 @@ $body$ , 'Event trigger function to drop object records when objects are removed.' ); --- Helper for event_trigger__disable(); see the WARNING below. +/* + * event_trigger__disable(names) disables the given event triggers, and + * event_trigger__enable() restores each one's exact prior state. Both are + * internal: for this extension's own update scripts, not the public API. + * + * Avoid disabling event triggers wherever another option exists. The change + * is transactional: other sessions don't see it until commit, and it rolls + * back with the caller's transaction, so no cleanup-on-error logic is needed. + * disable() row-locks each trigger's catalog row before reading its state + * (see _event_trigger__lock() below), which blocks a concurrent raw ALTER + * EVENT TRIGGER of the same trigger until commit. + * + * disable() also creates a no-op ddl_command_end event trigger, the sentinel, + * which enable() drops after restoring every state. While every sql_drop, + * table_rewrite and ddl_command_end event trigger is disabled, Postgres gives + * the commands nested in an extension script no event trigger state of their + * own: it collects them onto the enclosing CREATE/ALTER EXTENSION and replays + * them to that command's ddl_command_end once the triggers are back. There, + * pg_event_trigger_ddl_commands() fails on objects dropped since (before + * PostgreSQL 14) or hands capture the script's own objects. One enabled event + * trigger prevents the collection. The sentinel's function is in pg_temp, + * which capture skips. + */ SELECT __object_reference.create_function( '_object_reference._event_trigger__lock' , $args$ @@ -1620,7 +1591,6 @@ $args$ , 'pg_catalog.pg_event_trigger.evtenabled%TYPE LANGUAGE plpgsql' , $body$ DECLARE - v_oid pg_catalog.pg_event_trigger.oid%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN BEGIN @@ -1630,66 +1600,18 @@ BEGIN FOR UPDATE ; EXCEPTION WHEN insufficient_privilege THEN - -- No catalog UPDATE (e.g. managed services): an advisory lock only serializes other callers of this function. - SELECT oid INTO v_oid FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - IF FOUND THEN - PERFORM pg_catalog.pg_advisory_xact_lock( - 'pg_catalog.pg_event_trigger'::pg_catalog.regclass::pg_catalog.oid::int - , v_oid::int - ); - SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; - END IF; + -- Without catalog UPDATE rights the row can't be locked; the remaining race needs a concurrent raw ALTER of the same trigger during an administrator's own update. + SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; -- NULL if the trigger does not exist RETURN v_enabled; END $body$ - , 'Lock one event trigger against concurrent callers and return its current evtenabled (NULL if it does not exist).' + , 'Row-lock one event trigger where privileges allow, and return its current evtenabled (NULL if it does not exist).' ); -/* - * WARNING: avoid disabling event triggers at all where any other option - * exists. ALTER EVENT TRIGGER is ordinary transactional DDL -- like any - * other catalog write, it's invisible to other sessions until commit (no - * special database-wide/immediate effect: verified empirically that a - * concurrent session's DDL neither blocks on, nor is otherwise affected by, - * another session's still-uncommitted DISABLE) and it takes no lock at all - * on the event trigger itself. The real risk is TWO SESSIONS both trying to - * alter the SAME event trigger concurrently: a second writer blocks on the - * first the way any two concurrent writes to the same catalog row would, - * and without care, the one that unblocks second can record and later - * restore a "prior state" that was never actually the trigger's state - * immediately before it acted. disable() calls _event_trigger__lock() to - * close that gap: with catalog UPDATE privilege it locks the trigger's - * pg_event_trigger row FOR UPDATE, which also blocks a raw ALTER EVENT - * TRIGGER by another session. Without that privilege (e.g. managed - * services) it falls back to an advisory lock, which serializes only other - * callers of disable(), not a raw ALTER EVENT TRIGGER elsewhere. Prefer a - * self-recognition check (a session-local flag, checked from inside the - * trigger's own body) over calling this at all; reach for it only when - * nothing else can make the trigger stay quiet, as is currently true for - * zzz__object_reference_drop. - * - * General-purpose event-trigger disable/enable mechanism, for use by this - * extension's OWN install/update scripts only (not part of the public API). - * Not tied to "being mid-update" specifically -- it's a plain disable-with- - * restore primitive for any event trigger that can't self-recognize that it - * should stay quiet. - * - * zzz_object_reference__fix_identity and zzz_object_reference_capture check - * whether this call is currently in effect for their OWN session (via - * to_regclass() on the temp table below) and skip if so, so they never need - * to be disabled this way. zzz__object_reference_drop cannot self-recognize - * the same way without also touching _object_reference._object_v -- a view - * an update script may itself be dropping and recreating -- from inside its - * own body, so it must be truly disabled for the duration of such a - * script's structural section. - * - * ALTER EVENT TRIGGER is ordinary transactional DDL, so if the calling - * script's transaction rolls back, the DISABLE (and any ENABLE already run) - * rolls back with it -- no separate cleanup-on-error logic is needed here. - */ +-- name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -1701,23 +1623,25 @@ DECLARE v_name pg_catalog.pg_event_trigger.evtname%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN - /* - * WARNING: avoid disabling event triggers at all where any other option - * exists -- this is a database-wide change with real race-condition risk - * against concurrent sessions' DDL. See the warning above this function. - */ - -- Validate everything before any side effect. IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN - RAISE 'event_trigger_names must be a non-empty array'; + RAISE 'event_trigger_names must be a non-empty array' + USING ERRCODE = 'invalid_parameter_value' + ; END IF; IF array_position(event_trigger_names, NULL) IS NOT NULL THEN - RAISE 'event_trigger_names contains a NULL name' USING DETAIL = event_trigger_names::text; + RAISE 'event_trigger_names contains a NULL name' + USING ERRCODE = 'invalid_parameter_value' + , DETAIL = event_trigger_names::text + ; END IF; IF array_length(event_trigger_names, 1) <> (SELECT count(DISTINCT x) FROM unnest(event_trigger_names) x) THEN - RAISE 'event_trigger_names contains a duplicate name' USING DETAIL = event_trigger_names::text; + RAISE 'event_trigger_names contains a duplicate name' + USING ERRCODE = 'invalid_parameter_value' + , DETAIL = event_trigger_names::text + ; END IF; SELECT n INTO v_name @@ -1727,16 +1651,25 @@ BEGIN LIMIT 1 ; IF FOUND THEN - RAISE 'event trigger "%" does not exist', v_name; + RAISE 'event trigger "%" does not exist', v_name + USING ERRCODE = 'undefined_object' + ; END IF; IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN RAISE 'event_trigger__disable() called while a previous call is still in effect' - USING HINT = 'A previous event_trigger__enable() call may have been skipped.' + USING ERRCODE = 'object_not_in_prerequisite_state' + , HINT = 'A previous event_trigger__enable() call may have been skipped.' ; END IF; - -- Save old trigger state + -- The sentinel (see above), before anything is disabled + CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; + CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel + ON ddl_command_end + EXECUTE PROCEDURE pg_temp.__object_reference__event_trigger_sentinel() + ; + CREATE TEMP TABLE __object_reference__event_trigger_state AS SELECT evtname, evtenabled FROM pg_catalog.pg_event_trigger WHERE false ; @@ -1744,18 +1677,13 @@ BEGIN -- Fixed lock order across all callers, to avoid deadlock. FOR v_name IN SELECT n FROM unnest(event_trigger_names) n ORDER BY n COLLATE "C" LOOP - /* - * The lock must be held before we read evtenabled, so no other session's - * ALTER EVENT TRIGGER on the same trigger can land between our read and - * our DISABLE below -- otherwise we could record (and later restore) a - * state that was never the trigger's state immediately before we - * disabled it. - */ v_enabled := _object_reference._event_trigger__lock(v_name); -- Dropped by another session since the check above IF v_enabled IS NULL THEN - RAISE 'event trigger "%" does not exist', v_name; + RAISE 'event trigger "%" does not exist', v_name + USING ERRCODE = 'undefined_object' + ; END IF; INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) @@ -1778,7 +1706,9 @@ DECLARE i int; BEGIN IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NULL THEN - RAISE 'event_trigger__enable() called without a matching event_trigger__disable()'; + RAISE 'event_trigger__enable() called without a matching event_trigger__disable()' + USING ERRCODE = 'object_not_in_prerequisite_state' + ; END IF; -- Same order disable() locked in @@ -1787,16 +1717,7 @@ BEGIN FROM pg_temp.__object_reference__event_trigger_state ; - /* - * Drop our own bookkeeping table BEFORE re-enabling anything below: - * dropping it is itself DDL, and if zzz__object_reference_drop is one of - * the triggers being restored here, re-enabling it first would make this - * DROP immediately fire it -- reacting to our own internal cleanup, - * exactly the hazard this whole mechanism exists to avoid. (Confirmed by - * running into it: with the table dropped after, an active capture group - * elsewhere left a stale tracked row that this DROP's cascade into - * post_restore() then found and errored on.) - */ + -- Dropped before anything is restored, so this DROP fires none of the restored triggers. DROP TABLE pg_temp.__object_reference__event_trigger_state; FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP @@ -1811,6 +1732,10 @@ BEGIN END )); END LOOP; + + -- Only now that every state is restored (see the sentinel note above) + DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; + DROP FUNCTION pg_temp.__object_reference__event_trigger_sentinel(); END $body$ , 'Restore event triggers disabled by event_trigger__disable() to their exact prior state.' diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 9e38888..7b2d011 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,41 +1,45 @@ \set ECHO none -1..38 +1..42 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect ok 4 - test trigger b is (still) disabled while a call is in effect -ok 5 - enable() restores both -ok 6 - test trigger a is back to its original (origin) state -ok 7 - test trigger b is still disabled -- its prior state was preserved, not assumed enabled -ok 8 - disable() the first time -ok 9 - a second disable() without enable() in between is rejected -ok 10 - enable() cleans up so later tests are unaffected -ok 11 - enable() without disable() is rejected -ok 12 - disable() rejects an unknown event trigger name -ok 13 - disable() rejects a duplicate name in its own argument list -ok 14 - disable() rejects a NULL array -ok 15 - disable() rejects an empty array -ok 16 - disable() rejects a NULL element -ok 17 - disable() so a call is in effect -ok 18 - duplicate name is reported before the in-effect error -ok 19 - unknown name is reported before the in-effect error -ok 20 - NULL array is reported before the in-effect error -ok 21 - empty array is reported before the in-effect error -ok 22 - NULL element is reported before the in-effect error -ok 23 - the call still in effect kept its own state -ok 24 - enable() ends that call -ok 25 - disable() with unsorted names -ok 26 - names were processed in "C" collation order, not input order -ok 27 - enable() after unsorted disable() -ok 28 - all three restored -ok 29 - helper does not fail for a role that cannot row-lock the catalog -ok 30 - helper still returns the trigger's evtenabled -ok 31 - helper took the advisory lock keyed on catalog and trigger oid -ok 32 - start a capture group -ok 33 - disable() (any trigger) also signals self-recognizing triggers to stand down -ok 34 - enable() ends that window -ok 35 - the table created while disable() was in effect was NOT captured -ok 36 - stop the capture group -ok 37 - object_reference schema must not be part of the resolved search_path -ok 38 - _object_reference schema must not be part of the resolved search_path +ok 5 - the sentinel event trigger is enabled on ddl_command_end while a call is in effect +ok 6 - the sentinel function exists while a call is in effect +ok 7 - enable() restores both +ok 8 - test trigger a is back to its original (origin) state +ok 9 - test trigger b is still disabled -- its prior state was preserved, not assumed enabled +ok 10 - enable() dropped the sentinel event trigger +ok 11 - enable() dropped the sentinel function +ok 12 - disable() the first time +ok 13 - a second disable() without enable() in between is rejected +ok 14 - enable() cleans up so later tests are unaffected +ok 15 - enable() without disable() is rejected +ok 16 - disable() rejects an unknown event trigger name +ok 17 - disable() rejects a duplicate name in its own argument list +ok 18 - disable() rejects a NULL array +ok 19 - disable() rejects an empty array +ok 20 - disable() rejects a NULL element +ok 21 - disable() so a call is in effect +ok 22 - duplicate name is reported before the in-effect error +ok 23 - unknown name is reported before the in-effect error +ok 24 - NULL array is reported before the in-effect error +ok 25 - empty array is reported before the in-effect error +ok 26 - NULL element is reported before the in-effect error +ok 27 - the call still in effect kept its own state +ok 28 - enable() ends that call +ok 29 - disable() with unsorted names +ok 30 - names were processed in "C" collation order, not input order +ok 31 - enable() after unsorted disable() +ok 32 - all three restored +ok 33 - helper does not fail for a role that cannot row-lock the catalog +ok 34 - helper still returns the trigger's evtenabled +ok 35 - helper took no advisory lock +ok 36 - start a capture group +ok 37 - disable() zzz_object_reference_capture +ok 38 - enable() restores zzz_object_reference_capture +ok 39 - only the table created after enable() was captured +ok 40 - stop the capture group +ok 41 - object_reference schema must not be part of the resolved search_path +ok 42 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/install/load.sql b/test/install/load.sql index b552ddd..32918eb 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -232,11 +232,14 @@ BEGIN IF v_installed IS DISTINCT FROM v_target THEN RAISE EXCEPTION 'object_reference is at version % %, expected %', v_installed, step, v_target; END IF; - IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL - OR to_regclass('pg_temp.__object_reference__bootstrap_event_trigger_state') IS NOT NULL - THEN + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN RAISE EXCEPTION 'event trigger state table left behind %', step; END IF; + IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') + OR to_regprocedure('pg_temp.__object_reference__event_trigger_sentinel()') IS NOT NULL + THEN + RAISE EXCEPTION 'sentinel event trigger or its function left behind %', step; + END IF; END $$; @@ -306,11 +309,14 @@ CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' C /* * Fail unless the update restores each of the extension's event triggers to * the exact state it had before, starting each from a distinct non-default - * state. A failed check exits psql nonzero (ON_ERROR_STOP), but that does not - * fail the make run on any PostgreSQL version: pg_regress 16+ prints - * "not ok" and PG12's prints "ok", and make exits 0 either way. This - * file's own output is never compared against anything. All three are reset - * to ENABLE afterwards, so the rest of the suite runs as usual. + * state. All three are reset to ENABLE afterwards, so the rest of the suite + * runs as usual. + * + * A failed check in this file never fails make by itself: pgxntool ignores + * installcheck's exit status and fails `make test` only when regression.diffs + * exists, and this file's output is never compared. pg_regress 16+ prints + * "not ok" for it and PG12's prints "ok"; make fails only if later tests also + * fail. */ BEGIN; ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 3c0368b..97ba121 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -3,15 +3,11 @@ \i test/load.sql /* - * event_trigger__disable()/__enable() are ALTER EVENT TRIGGER under the - * hood -- ordinary transactional DDL, invisible to other sessions until - * commit, same as any other catalog change (verified empirically: a - * concurrent session neither blocks on, nor otherwise sees, another - * session's still-uncommitted DISABLE). The real risk is two sessions both - * altering the SAME event trigger concurrently, so this test exercises the - * mechanism against its OWN dummy event triggers rather than the real zzz_* - * ones, to keep it independent of whatever else happens to run concurrently - * in this same parallel test batch. + * This file runs in one transaction that is never committed (test/load.sql + * opens it, test/finish.sql leaves it open), and pg_regress runs the test files + * one at a time, so no other session ever sees an event trigger disabled here. + * Most tests use their own no-op event triggers, whose names and prior states + * they control; the capture test disables the real zzz_object_reference_capture. */ CREATE FUNCTION event_trigger_disable_test__noop() RETURNS event_trigger LANGUAGE plpgsql AS $$ BEGIN @@ -30,6 +26,7 @@ GRANT EXECUTE ON FUNCTION _object_reference._event_trigger__lock(name) TO event_ SELECT plan( 0 +7 -- multi-trigger disable/enable preserves each one's own prior state + +4 -- the sentinel exists while a call is in effect and is gone, function included, after enable() +3 -- nested disable() without an intervening enable() is rejected +1 -- enable() without a matching disable() is rejected +1 -- disable() rejects an unknown event trigger name @@ -37,8 +34,8 @@ SELECT plan( +3 -- disable() rejects a NULL array, an empty array and a NULL element +8 -- argument errors are reported in preference to the "previous call in effect" error +4 -- names passed unsorted are processed in "C" order - +3 -- _event_trigger__lock() falls back to an advisory lock for a role that cannot row-lock the catalog - +5 -- zzz_object_reference_capture self-recognizes and stands down while a disable() is in effect + +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog + +5 -- disabling zzz_object_reference_capture stops capture until enable() +2 -- schema-qualification (search_path) ); @@ -61,6 +58,15 @@ SELECT is( , 'D' , 'test trigger b is (still) disabled while a call is in effect' ); +SELECT is( + (SELECT evtevent || ' ' || evtenabled::text FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') + , 'ddl_command_end O' + , 'the sentinel event trigger is enabled on ddl_command_end while a call is in effect' +); +SELECT ok( + to_regprocedure('pg_temp.__object_reference__event_trigger_sentinel()') IS NOT NULL + , 'the sentinel function exists while a call is in effect' +); SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ , 'enable() restores both' @@ -75,6 +81,14 @@ SELECT is( , 'D' , 'test trigger b is still disabled -- its prior state was preserved, not assumed enabled' ); +SELECT ok( + NOT EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') + , 'enable() dropped the sentinel event trigger' +); +SELECT ok( + to_regprocedure('pg_temp.__object_reference__event_trigger_sentinel()') IS NULL + , 'enable() dropped the sentinel function' +); -- Nested disable() without an intervening enable() SELECT lives_ok( @@ -83,7 +97,7 @@ SELECT lives_ok( ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , NULL + , '55000' , 'event_trigger__disable() called while a previous call is still in effect' , 'a second disable() without enable() in between is rejected' ); @@ -95,7 +109,7 @@ SELECT lives_ok( -- enable() without a matching disable() SELECT throws_ok( $$SELECT _object_reference.event_trigger__enable()$$ - , NULL + , '55000' , 'event_trigger__enable() called without a matching event_trigger__disable()' , 'enable() without disable() is rejected' ); @@ -103,7 +117,7 @@ SELECT throws_ok( -- Unknown event trigger name SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,no_such_event_trigger}')$$ - , NULL + , '42704' , 'event trigger "no_such_event_trigger" does not exist' , 'disable() rejects an unknown event trigger name' ); @@ -111,7 +125,7 @@ SELECT throws_ok( -- Duplicate name in the same call SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,event_trigger_disable_test__a}')$$ - , NULL + , '22023' , 'event_trigger_names contains a duplicate name' , 'disable() rejects a duplicate name in its own argument list' ); @@ -124,19 +138,19 @@ SELECT throws_ok( */ SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable(NULL)$$ - , NULL + , '22023' , 'event_trigger_names must be a non-empty array' , 'disable() rejects a NULL array' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{}')$$ - , NULL + , '22023' , 'event_trigger_names must be a non-empty array' , 'disable() rejects an empty array' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a,NULL}')$$ - , NULL + , '22023' , 'event_trigger_names contains a NULL name' , 'disable() rejects a NULL element' ); @@ -148,31 +162,31 @@ SELECT lives_ok( ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__b,event_trigger_disable_test__b}')$$ - , NULL + , '22023' , 'event_trigger_names contains a duplicate name' , 'duplicate name is reported before the in-effect error' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{no_such_event_trigger}')$$ - , NULL + , '42704' , 'event trigger "no_such_event_trigger" does not exist' , 'unknown name is reported before the in-effect error' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable(NULL)$$ - , NULL + , '22023' , 'event_trigger_names must be a non-empty array' , 'NULL array is reported before the in-effect error' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{}')$$ - , NULL + , '22023' , 'event_trigger_names must be a non-empty array' , 'empty array is reported before the in-effect error' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__b,NULL}')$$ - , NULL + , '22023' , 'event_trigger_names contains a NULL name' , 'NULL element is reported before the in-effect error' ); @@ -208,9 +222,9 @@ SELECT is( /* * A role that cannot row-lock pg_event_trigger (as on managed services - * without true superuser) gets an advisory lock instead of an error. The DO - * block runs as that role, and first proves the role really cannot FOR UPDATE - * the catalog. + * without true superuser) gets the trigger's state read unlocked instead of an + * error. The DO block runs as that role, and first proves the role really + * cannot FOR UPDATE the catalog. */ SELECT lives_ok( $t$DO $do$ @@ -234,42 +248,33 @@ SELECT is( , 'helper still returns the trigger''s evtenabled' ); SELECT is( - (SELECT count(*)::int FROM pg_locks - WHERE locktype = 'advisory' - AND pid = pg_backend_pid() - AND classid::int = 'pg_catalog.pg_event_trigger'::regclass::oid::int - AND objid::int = (SELECT oid::int FROM pg_catalog.pg_event_trigger WHERE evtname = 'event_trigger_disable_test__a') - AND objsubid = 2) - , 1 - , 'helper took the advisory lock keyed on catalog and trigger oid' + (SELECT count(*)::int FROM pg_catalog.pg_locks WHERE locktype = 'advisory' AND pid = pg_backend_pid()) + , 0 + , 'helper took no advisory lock' ); -/* - * zzz_object_reference_capture / zzz_object_reference__fix_identity check - * for an event_trigger__disable() call currently in effect for THIS - * session (regardless of which trigger names it names) and stand down -- - * verify that here for capture, since it's directly observable. - */ +-- Disabling zzz_object_reference_capture stops capture until enable() SELECT lives_ok( $$SELECT object_reference.capture__start(object_reference.object_group__create('event_trigger_disable_test_group'))$$ , 'start a capture group' ); SELECT lives_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() (any trigger) also signals self-recognizing triggers to stand down' + $$SELECT _object_reference.event_trigger__disable('{zzz_object_reference_capture}')$$ + , 'disable() zzz_object_reference_capture' ); -CREATE TABLE event_trigger_disable_test_table(); +CREATE TABLE public.event_trigger_disable_test__while_disabled(); SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() ends that window' -); -SELECT is_empty( - $$ - SELECT 1 - FROM _object_reference.object_group__object - WHERE object_group_id = (object_reference.object_group__get('event_trigger_disable_test_group')).object_group_id - $$ - , 'the table created while disable() was in effect was NOT captured' + , 'enable() restores zzz_object_reference_capture' +); +CREATE TABLE public.event_trigger_disable_test__after_enable(); +SELECT is( + (SELECT array_agg(array_to_string(o.object_names, '.') ORDER BY o.object_names) + FROM _object_reference.object_group__object m + JOIN _object_reference.object o USING (object_id) + WHERE m.object_group_id = (object_reference.object_group__get('event_trigger_disable_test_group')).object_group_id) + , '{public.event_trigger_disable_test__after_enable}'::text[] + , 'only the table created after enable() was captured' ); SELECT lives_ok( $$SELECT object_reference.capture__stop('event_trigger_disable_test_group')$$ From 4ce16610af1c10a4950346ec40dd4a96feeefefa Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Sat, 31 Oct 2026 19:21:22 -0500 Subject: [PATCH 09/35] Give the post-lock event trigger re-check its own error message event_trigger__disable() raised the same 'does not exist' text for a trigger missing at validation and for one dropped between validation and the lock. The latter now says it was dropped concurrently (same ERRCODE). Co-Authored-By: Claude Sonnet 5.5 --- sql/object_reference--0.1.0--stable.sql | 2 +- sql/object_reference.sql | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index fd015f0..c299b12 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -296,7 +296,7 @@ BEGIN -- Dropped by another session since the check above IF v_enabled IS NULL THEN - RAISE 'event trigger "%" does not exist', v_name + RAISE 'event trigger "%" was dropped concurrently', v_name USING ERRCODE = 'undefined_object' ; END IF; diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 60f4de3..c39b6b3 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1681,7 +1681,7 @@ BEGIN -- Dropped by another session since the check above IF v_enabled IS NULL THEN - RAISE 'event trigger "%" does not exist', v_name + RAISE 'event trigger "%" was dropped concurrently', v_name USING ERRCODE = 'undefined_object' ; END IF; From 5d768a29b8534d29add5c47e7ee6ea713b5be932 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 2 Nov 2026 16:51:59 -0600 Subject: [PATCH 10/35] event_trigger__disable(): raise a clear error when another session's sentinel exists Co-Authored-By: Claude Sonnet 5.5 --- sql/object_reference--0.1.0--stable.sql | 8 ++++ sql/object_reference.sql | 8 ++++ test/expected/event_trigger_disable.out | 62 +++++++++++++------------ test/sql/event_trigger_disable.sql | 24 ++++++++++ 4 files changed, 73 insertions(+), 29 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index c299b12..85b15db 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -278,6 +278,14 @@ BEGIN ; END IF; + -- Another session's sentinel (or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". + IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') THEN + RAISE 'event_trigger__disable() from another session is still in effect' + USING ERRCODE = 'object_not_in_prerequisite_state' + , HINT = 'Wait for that session to finish, or drop event trigger __object_reference__event_trigger_sentinel if it was committed without event_trigger__enable().' + ; + END IF; + -- The sentinel (see above), before anything is disabled CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel diff --git a/sql/object_reference.sql b/sql/object_reference.sql index c39b6b3..6b79428 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1663,6 +1663,14 @@ BEGIN ; END IF; + -- Another session's sentinel (or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". + IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') THEN + RAISE 'event_trigger__disable() from another session is still in effect' + USING ERRCODE = 'object_not_in_prerequisite_state' + , HINT = 'Wait for that session to finish, or drop event trigger __object_reference__event_trigger_sentinel if it was committed without event_trigger__enable().' + ; + END IF; + -- The sentinel (see above), before anything is disabled CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 7b2d011..5f6ebac 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..42 +1..46 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -14,32 +14,36 @@ ok 11 - enable() dropped the sentinel function ok 12 - disable() the first time ok 13 - a second disable() without enable() in between is rejected ok 14 - enable() cleans up so later tests are unaffected -ok 15 - enable() without disable() is rejected -ok 16 - disable() rejects an unknown event trigger name -ok 17 - disable() rejects a duplicate name in its own argument list -ok 18 - disable() rejects a NULL array -ok 19 - disable() rejects an empty array -ok 20 - disable() rejects a NULL element -ok 21 - disable() so a call is in effect -ok 22 - duplicate name is reported before the in-effect error -ok 23 - unknown name is reported before the in-effect error -ok 24 - NULL array is reported before the in-effect error -ok 25 - empty array is reported before the in-effect error -ok 26 - NULL element is reported before the in-effect error -ok 27 - the call still in effect kept its own state -ok 28 - enable() ends that call -ok 29 - disable() with unsorted names -ok 30 - names were processed in "C" collation order, not input order -ok 31 - enable() after unsorted disable() -ok 32 - all three restored -ok 33 - helper does not fail for a role that cannot row-lock the catalog -ok 34 - helper still returns the trigger's evtenabled -ok 35 - helper took no advisory lock -ok 36 - start a capture group -ok 37 - disable() zzz_object_reference_capture -ok 38 - enable() restores zzz_object_reference_capture -ok 39 - only the table created after enable() was captured -ok 40 - stop the capture group -ok 41 - object_reference schema must not be part of the resolved search_path -ok 42 - _object_reference schema must not be part of the resolved search_path +ok 15 - disable() rejects an existing sentinel event trigger +ok 16 - the rejected disable() created no state table +ok 17 - disable() succeeds once the stray sentinel is gone +ok 18 - enable() cleans up so later tests are unaffected +ok 19 - enable() without disable() is rejected +ok 20 - disable() rejects an unknown event trigger name +ok 21 - disable() rejects a duplicate name in its own argument list +ok 22 - disable() rejects a NULL array +ok 23 - disable() rejects an empty array +ok 24 - disable() rejects a NULL element +ok 25 - disable() so a call is in effect +ok 26 - duplicate name is reported before the in-effect error +ok 27 - unknown name is reported before the in-effect error +ok 28 - NULL array is reported before the in-effect error +ok 29 - empty array is reported before the in-effect error +ok 30 - NULL element is reported before the in-effect error +ok 31 - the call still in effect kept its own state +ok 32 - enable() ends that call +ok 33 - disable() with unsorted names +ok 34 - names were processed in "C" collation order, not input order +ok 35 - enable() after unsorted disable() +ok 36 - all three restored +ok 37 - helper does not fail for a role that cannot row-lock the catalog +ok 38 - helper still returns the trigger's evtenabled +ok 39 - helper took no advisory lock +ok 40 - start a capture group +ok 41 - disable() zzz_object_reference_capture +ok 42 - enable() restores zzz_object_reference_capture +ok 43 - only the table created after enable() was captured +ok 44 - stop the capture group +ok 45 - object_reference schema must not be part of the resolved search_path +ok 46 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 97ba121..2e53b93 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -28,6 +28,7 @@ SELECT plan( +7 -- multi-trigger disable/enable preserves each one's own prior state +4 -- the sentinel exists while a call is in effect and is gone, function included, after enable() +3 -- nested disable() without an intervening enable() is rejected + +4 -- disable() rejects a sentinel event trigger left by another session, before changing anything +1 -- enable() without a matching disable() is rejected +1 -- disable() rejects an unknown event trigger name +1 -- disable() rejects a duplicate name in its own argument list @@ -106,6 +107,29 @@ SELECT lives_ok( , 'enable() cleans up so later tests are unaffected' ); +-- A sentinel event trigger that this session did not create +CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end EXECUTE FUNCTION event_trigger_disable_test__noop(); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , '55000' + , 'event_trigger__disable() from another session is still in effect' + , 'disable() rejects an existing sentinel event trigger' +); +SELECT is( + to_regclass('pg_temp.__object_reference__event_trigger_state') + , NULL + , 'the rejected disable() created no state table' +); +DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() succeeds once the stray sentinel is gone' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() cleans up so later tests are unaffected' +); + -- enable() without a matching disable() SELECT throws_ok( $$SELECT _object_reference.event_trigger__enable()$$ From b7e54c446b415af8d85bcda758a93899634d87f1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 2 Nov 2026 17:07:37 -0600 Subject: [PATCH 11/35] Tidy comments and reuse the trigger-state assertion helper in load.sql --- sql/object_reference--0.1.0--stable.sql | 7 ++++- sql/object_reference.sql | 1 + test/install/load.sql | 42 ++++++------------------- 3 files changed, 16 insertions(+), 34 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 85b15db..a544ba1 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -1,4 +1,8 @@ --- event_trigger__disable() of all three event triggers, by hand only because 0.1.0 has no disable(); the event_trigger__enable() at the end undoes it. +/* + * Disables all three event triggers for this script (a hand-written copy of + * event_trigger__disable(), which 0.1.0 lacks); event_trigger__enable() at the + * end restores them. See the sentinel note on event_trigger__disable() below. + */ CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end @@ -324,6 +328,7 @@ SELECT __object_reference.create_function( , 'void LANGUAGE plpgsql' , $body$ DECLARE + -- name[] and "char"[] stand in for %TYPE[], as in disable() above. v_names name[]; v_states "char"[]; i int; diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 6b79428..6633e9f 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1709,6 +1709,7 @@ SELECT __object_reference.create_function( , 'void LANGUAGE plpgsql' , $body$ DECLARE + -- name[] and "char"[] stand in for %TYPE[], as in disable() above. v_names name[]; v_states "char"[]; i int; diff --git a/test/install/load.sql b/test/install/load.sql index 32918eb..77c67d4 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -307,16 +307,15 @@ SET client_min_messages = WARNING; CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; /* - * Fail unless the update restores each of the extension's event triggers to - * the exact state it had before, starting each from a distinct non-default - * state. All three are reset to ENABLE afterwards, so the rest of the suite - * runs as usual. + * This check never fails `make test` by itself: pgxntool ignores installcheck's + * exit status and fails only when regression.diffs exists, and this file's + * output is never compared (pg_regress 16+ prints "not ok" for it, PG12's + * prints "ok"); make fails only if later tests also fail. * - * A failed check in this file never fails make by itself: pgxntool ignores - * installcheck's exit status and fails `make test` only when regression.diffs - * exists, and this file's output is never compared. pg_regress 16+ prints - * "not ok" for it and PG12's prints "ok"; make fails only if later tests also - * fail. + * It fails the file unless the update restores each of the extension's event + * triggers to the exact state it had before, starting each from a distinct + * non-default state. All three are reset to ENABLE afterwards, so the rest of + * the suite runs as usual. */ BEGIN; ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; @@ -331,30 +330,7 @@ SET client_min_messages = ERROR; ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; SET client_min_messages = WARNING; -DO $DO$ -DECLARE - v_mismatches text; -BEGIN - SELECT string_agg( - format('%I is %L, expected %L.', e.evtname, t.evtenabled, e.evtenabled) - , ' ' ORDER BY e.evtname - ) - INTO v_mismatches - FROM (VALUES - ('zzz_object_reference_capture'::name, 'D'::"char") - , ('zzz_object_reference__fix_identity', 'R') - , ('zzz__object_reference_drop', 'A') - ) e(evtname, evtenabled) - LEFT JOIN pg_catalog.pg_event_trigger t USING (evtname) - WHERE t.evtenabled IS DISTINCT FROM e.evtenabled - ; - IF v_mismatches IS NOT NULL THEN - RAISE EXCEPTION 'ALTER EXTENSION UPDATE did not restore event trigger state' - USING DETAIL = v_mismatches - ; - END IF; -END -$DO$; +SELECT pg_temp.assert_event_trigger_state('after the committed update', 'D', 'R', 'A'); ALTER EVENT TRIGGER zzz_object_reference_capture ENABLE; ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE; From 8630c69eb89c3ffa08f2f2cc0cc2d42a68210aaa Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 2 Nov 2026 17:25:13 -0600 Subject: [PATCH 12/35] Make a failed update-mode load.sql fail make test-update test/load.sql now raises when the test mode is update or existing and test/install/load.sql did not leave object_reference installed. Without that, every later test file ran CREATE EXTENSION IF NOT EXISTS, installed the current version fresh and passed, so make exited 0. test/dump/load_all.sql installs the extension itself and sets the mode to fresh before including test/load.sql. Reword the load.sql comments accordingly, and narrow the sentinel-check comment in disable() to a committed sentinel. The update script's bootstrap notes that, unlike disable(), it does not check for an existing sentinel. --- sql/object_reference--0.1.0--stable.sql | 3 ++- sql/object_reference.sql | 2 +- test/dump/load_all.sql | 4 ++++ test/install/load.sql | 24 ++++++++++++------------ test/load.sql | 16 ++++++++++++++++ 5 files changed, 35 insertions(+), 14 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index a544ba1..2f80625 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -3,6 +3,7 @@ * event_trigger__disable(), which 0.1.0 lacks); event_trigger__enable() at the * end restores them. See the sentinel note on event_trigger__disable() below. */ +-- Unlike disable(), this does not check for an existing sentinel: a committed one fails with the plain "already exists" error. CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end @@ -282,7 +283,7 @@ BEGIN ; END IF; - -- Another session's sentinel (or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". + -- A committed sentinel (another session's, or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') THEN RAISE 'event_trigger__disable() from another session is still in effect' USING ERRCODE = 'object_not_in_prerequisite_state' diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 6633e9f..c2dc328 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1663,7 +1663,7 @@ BEGIN ; END IF; - -- Another session's sentinel (or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". + -- A committed sentinel (another session's, or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') THEN RAISE 'event_trigger__disable() from another session is still in effect' USING ERRCODE = 'object_not_in_prerequisite_state' diff --git a/test/dump/load_all.sql b/test/dump/load_all.sql index 0d9dd26..062c6cd 100644 --- a/test/dump/load_all.sql +++ b/test/dump/load_all.sql @@ -1,5 +1,9 @@ \set ECHO none +-- This flow installs the extension itself, whatever TEST_LOAD_SOURCE says. +\set QUIET on +SET object_reference.test_load_mode = 'fresh'; +\set QUIET off \i test/load.sql /* diff --git a/test/install/load.sql b/test/install/load.sql index 77c67d4..87c1990 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -179,10 +179,9 @@ CREATE EXTENSION IF NOT EXISTS count_nulls; /* * Update scenarios. Each installs the old version inside its own * transaction and rolls back, so nothing leaks into the committed install - * below; that includes sequence values, which a rollback alone would not - * reset on an install committed beforehand. Each one starts a capture group, - * asserts the event trigger state it starts the update from, updates, and - * checks the result: + * below (the install is inside the rolled-back transaction, so its sequences + * roll back too). Each one starts a capture group, asserts the event + * trigger state it starts the update from, updates, and checks the result: * 1. All three triggers at their default ENABLE; a table created after the * update is captured. * 2. zzz__object_reference_drop already disabled, and still disabled after. @@ -307,15 +306,16 @@ SET client_min_messages = WARNING; CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; /* - * This check never fails `make test` by itself: pgxntool ignores installcheck's - * exit status and fails only when regression.diffs exists, and this file's - * output is never compared (pg_regress 16+ prints "not ok" for it, PG12's - * prints "ok"); make fails only if later tests also fail. + * This check fails only this load.sql run: pgxntool ignores installcheck's exit + * status and fails `make test` only when regression.diffs exists, and this + * file's output is never compared (pg_regress 16+ prints "not ok" for it, + * PG12's prints "ok"). make fails because test/load.sql then raises in every + * later test file, as the extension was not installed. * - * It fails the file unless the update restores each of the extension's event - * triggers to the exact state it had before, starting each from a distinct - * non-default state. All three are reset to ENABLE afterwards, so the rest of - * the suite runs as usual. + * It fails this load.sql run unless the update restores each of the + * extension's event triggers to the exact state it had before, starting each + * from a distinct non-default state. All three are reset to ENABLE afterwards, + * so the rest of the suite runs as usual. */ BEGIN; ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; diff --git a/test/load.sql b/test/load.sql index 14ccd26..a8bf105 100644 --- a/test/load.sql +++ b/test/load.sql @@ -11,6 +11,22 @@ SET search_path = tap, public; * `psql -f` flow (test/dump/run.sh) entirely outside pg_regress and * test/install, and has no other way to get the extension installed. */ +/* + * In update and existing modes test/install/load.sql must already have + * installed the extension; raising here fails every test file when it didn't, + * instead of letting the CREATE EXTENSION below install the current version + * fresh and pass. + */ +DO $$ +BEGIN + IF current_setting('object_reference.test_load_mode', true) IN ('update', 'existing') + AND NOT EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'object_reference') + THEN + RAISE EXCEPTION 'test/install/load.sql did not install object_reference in % mode', current_setting('object_reference.test_load_mode'); + END IF; +END +$$; + SET client_min_messages = WARNING; -- Squelch notices about dependent extensions CREATE EXTENSION IF NOT EXISTS object_reference CASCADE; --SET client_min_messages = NOTICE; From 5a5d2ef22bc0cc783842bf48cfe6f927f72b2ba0 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 2 Nov 2026 17:45:33 -0600 Subject: [PATCH 13/35] Warn when event trigger row lock falls back to unlocked read; make load guard version-aware The 42501 fallback in the event trigger lock helper and in the update script's bootstrap now raises a WARNING naming the trigger, then reads evtenabled without the lock. A test runs the fallback as an unprivileged role and checks the WARNING in the expected output. test/load.sql now checks that update and existing modes left the extension at the version under test (test_update_to if set, else default_version), not only that it is installed. Comments in test/load.sql and test/install/load.sql describe this accurately. --- sql/object_reference--0.1.0--stable.sql | 4 ++- sql/object_reference.sql | 3 +- test/expected/event_trigger_disable.out | 1 + test/install/load.sql | 9 +++--- test/load.sql | 43 ++++++++++++++++--------- test/sql/event_trigger_disable.sql | 11 +++++-- 6 files changed, 46 insertions(+), 25 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 2f80625..6d005e3 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -26,6 +26,7 @@ BEGIN BEGIN SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = v_name FOR UPDATE; EXCEPTION WHEN insufficient_privilege THEN + RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', v_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = v_name; END; INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) VALUES (v_name, v_enabled); @@ -220,7 +221,8 @@ BEGIN FOR UPDATE ; EXCEPTION WHEN insufficient_privilege THEN - -- Without catalog UPDATE rights the row can't be locked; the remaining race needs a concurrent raw ALTER of the same trigger during an administrator's own update. + -- Without catalog UPDATE rights the row can't be locked; the race that leaves open needs a concurrent raw ALTER of the same trigger during an administrator's own update. + RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', event_trigger_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; diff --git a/sql/object_reference.sql b/sql/object_reference.sql index c2dc328..c9cb7e0 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1600,7 +1600,8 @@ BEGIN FOR UPDATE ; EXCEPTION WHEN insufficient_privilege THEN - -- Without catalog UPDATE rights the row can't be locked; the remaining race needs a concurrent raw ALTER of the same trigger during an administrator's own update. + -- Without catalog UPDATE rights the row can't be locked; the race that leaves open needs a concurrent raw ALTER of the same trigger during an administrator's own update. + RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', event_trigger_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 5f6ebac..701e73f 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -36,6 +36,7 @@ ok 33 - disable() with unsorted names ok 34 - names were processed in "C" collation order, not input order ok 35 - enable() after unsorted disable() ok 36 - all three restored +WARNING: could not lock the pg_event_trigger row of event trigger "event_trigger_disable_test__a"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked ok 37 - helper does not fail for a role that cannot row-lock the catalog ok 38 - helper still returns the trigger's evtenabled ok 39 - helper took no advisory lock diff --git a/test/install/load.sql b/test/install/load.sql index 87c1990..a8bb05b 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -179,8 +179,8 @@ CREATE EXTENSION IF NOT EXISTS count_nulls; /* * Update scenarios. Each installs the old version inside its own * transaction and rolls back, so nothing leaks into the committed install - * below (the install is inside the rolled-back transaction, so its sequences - * roll back too). Each one starts a capture group, asserts the event + * below (the extension and its sequences don't exist after the rollback). + * Each one starts a capture group, asserts the event * trigger state it starts the update from, updates, and checks the result: * 1. All three triggers at their default ENABLE; a table created after the * update is captured. @@ -309,8 +309,9 @@ CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' C * This check fails only this load.sql run: pgxntool ignores installcheck's exit * status and fails `make test` only when regression.diffs exists, and this * file's output is never compared (pg_regress 16+ prints "not ok" for it, - * PG12's prints "ok"). make fails because test/load.sql then raises in every - * later test file, as the extension was not installed. + * PG12's prints "ok"). make fails because the failed transaction leaves the + * extension at test_update_from, and test/load.sql then raises in every later + * test file, as the installed version is not the one under test. * * It fails this load.sql run unless the update restores each of the * extension's event triggers to the exact state it had before, starting each diff --git a/test/load.sql b/test/load.sql index a8bf105..adc0cbe 100644 --- a/test/load.sql +++ b/test/load.sql @@ -2,6 +2,33 @@ SET search_path = tap, public; +/* + * In update and existing modes, test/install/load.sql must have left the + * extension installed at the version under test (test_update_to if set, else + * default_version). Raising here fails every test file when it did not, which + * is what makes `make` fail for a load.sql check that stopped short: the + * CREATE EXTENSION below would otherwise install the current version fresh + * and pass, and a version left behind would go unnoticed. + */ +DO $$ +DECLARE + v_mode text := current_setting('object_reference.test_load_mode', true); + v_installed text := (SELECT extversion FROM pg_catalog.pg_extension WHERE extname = 'object_reference'); + v_expected text; +BEGIN + IF v_mode IN ('update', 'existing') THEN + v_expected := coalesce( + CASE WHEN v_mode = 'update' THEN nullif(current_setting('object_reference.test_update_to', true), '') END + , (SELECT default_version FROM pg_catalog.pg_available_extensions WHERE name = 'object_reference') + ); + IF v_installed IS DISTINCT FROM v_expected THEN + RAISE EXCEPTION 'test/install/load.sql left object_reference at version % in % mode, expected %' + , coalesce(v_installed, '(not installed)'), v_mode, v_expected; + END IF; + END IF; +END +$$; + /* * The extension is normally already installed -- committed, once -- by * test/install/load.sql before this file ever runs (see that file for the @@ -11,22 +38,6 @@ SET search_path = tap, public; * `psql -f` flow (test/dump/run.sh) entirely outside pg_regress and * test/install, and has no other way to get the extension installed. */ -/* - * In update and existing modes test/install/load.sql must already have - * installed the extension; raising here fails every test file when it didn't, - * instead of letting the CREATE EXTENSION below install the current version - * fresh and pass. - */ -DO $$ -BEGIN - IF current_setting('object_reference.test_load_mode', true) IN ('update', 'existing') - AND NOT EXISTS (SELECT 1 FROM pg_catalog.pg_extension WHERE extname = 'object_reference') - THEN - RAISE EXCEPTION 'test/install/load.sql did not install object_reference in % mode', current_setting('object_reference.test_load_mode'); - END IF; -END -$$; - SET client_min_messages = WARNING; -- Squelch notices about dependent extensions CREATE EXTENSION IF NOT EXISTS object_reference CASCADE; --SET client_min_messages = NOTICE; diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 2e53b93..a921005 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -246,10 +246,14 @@ SELECT is( /* * A role that cannot row-lock pg_event_trigger (as on managed services - * without true superuser) gets the trigger's state read unlocked instead of an - * error. The DO block runs as that role, and first proves the role really - * cannot FOR UPDATE the catalog. + * without true superuser) gets the trigger's state read unlocked, with a + * WARNING naming the trigger, instead of an error. The DO block runs as that + * role, and first proves the role really cannot FOR UPDATE the catalog. The + * WARNING is checked by the expected output: it appears just before the next + * assertion's line. Terse verbosity keeps the PostgreSQL source location out + * of that output. */ +\set VERBOSITY terse SELECT lives_ok( $t$DO $do$ BEGIN @@ -266,6 +270,7 @@ SELECT lives_ok( $do$$t$ , 'helper does not fail for a role that cannot row-lock the catalog' ); +\set VERBOSITY verbose SELECT is( current_setting('event_trigger_disable_test.enabled') , 'O' From 373e2635dbe939200e3486e0440312969bebe6cc Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Mon, 2 Nov 2026 17:52:44 -0600 Subject: [PATCH 14/35] Tighten comments and wording around the lock fallback and load guard Comment and message wording only; no behavior change. The load guard's error message is now accurate in update and existing modes alike. --- sql/object_reference--0.1.0--stable.sql | 2 +- sql/object_reference.sql | 2 +- test/install/load.sql | 4 ++-- test/load.sql | 11 ++++------- test/sql/event_trigger_disable.sql | 13 ++++++------- 5 files changed, 14 insertions(+), 18 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 6d005e3..eecb13e 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -221,7 +221,7 @@ BEGIN FOR UPDATE ; EXCEPTION WHEN insufficient_privilege THEN - -- Without catalog UPDATE rights the row can't be locked; the race that leaves open needs a concurrent raw ALTER of the same trigger during an administrator's own update. + -- No catalog UPDATE rights: the row can't be locked. RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', event_trigger_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; diff --git a/sql/object_reference.sql b/sql/object_reference.sql index c9cb7e0..97e0f13 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1600,7 +1600,7 @@ BEGIN FOR UPDATE ; EXCEPTION WHEN insufficient_privilege THEN - -- Without catalog UPDATE rights the row can't be locked; the race that leaves open needs a concurrent raw ALTER of the same trigger during an administrator's own update. + -- No catalog UPDATE rights: the row can't be locked. RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', event_trigger_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; diff --git a/test/install/load.sql b/test/install/load.sql index a8bb05b..07798f5 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -180,8 +180,8 @@ CREATE EXTENSION IF NOT EXISTS count_nulls; * Update scenarios. Each installs the old version inside its own * transaction and rolls back, so nothing leaks into the committed install * below (the extension and its sequences don't exist after the rollback). - * Each one starts a capture group, asserts the event - * trigger state it starts the update from, updates, and checks the result: + * Each one starts a capture group, asserts the event trigger state it starts + * the update from, updates, and checks the result: * 1. All three triggers at their default ENABLE; a table created after the * update is captured. * 2. zzz__object_reference_drop already disabled, and still disabled after. diff --git a/test/load.sql b/test/load.sql index adc0cbe..d203c02 100644 --- a/test/load.sql +++ b/test/load.sql @@ -3,12 +3,9 @@ SET search_path = tap, public; /* - * In update and existing modes, test/install/load.sql must have left the - * extension installed at the version under test (test_update_to if set, else - * default_version). Raising here fails every test file when it did not, which - * is what makes `make` fail for a load.sql check that stopped short: the - * CREATE EXTENSION below would otherwise install the current version fresh - * and pass, and a version left behind would go unnoticed. + * In update and existing modes the extension must be installed at the version + * under test; raising here fails every test file otherwise, so `make` fails + * when test/install/load.sql stopped short. */ DO $$ DECLARE @@ -22,7 +19,7 @@ BEGIN , (SELECT default_version FROM pg_catalog.pg_available_extensions WHERE name = 'object_reference') ); IF v_installed IS DISTINCT FROM v_expected THEN - RAISE EXCEPTION 'test/install/load.sql left object_reference at version % in % mode, expected %' + RAISE EXCEPTION 'object_reference is at version % in % mode, expected %' , coalesce(v_installed, '(not installed)'), v_mode, v_expected; END IF; END IF; diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index a921005..d6bb554 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -245,13 +245,12 @@ SELECT is( ); /* - * A role that cannot row-lock pg_event_trigger (as on managed services - * without true superuser) gets the trigger's state read unlocked, with a - * WARNING naming the trigger, instead of an error. The DO block runs as that - * role, and first proves the role really cannot FOR UPDATE the catalog. The - * WARNING is checked by the expected output: it appears just before the next - * assertion's line. Terse verbosity keeps the PostgreSQL source location out - * of that output. + * A role without UPDATE on the catalog gets the trigger's state read + * unlocked, with a WARNING naming the trigger, instead of an error. The DO + * block runs as that role, and first proves the role really cannot FOR UPDATE + * the catalog. The WARNING is checked via the expected output, where it + * precedes this assertion's `ok` line. Terse verbosity keeps the source + * location out of it. */ \set VERBOSITY terse SELECT lives_ok( From 12c5b07db798002236e14807c57da736abccacd7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 11:21:28 -0600 Subject: [PATCH 15/35] Address review: clarify lock-fallback warning, lock-order and caller-names comments, drop redundant ORDER BY, quiet test warning Reword the unlocked-read warning, move the sorted-order comment to the lock loop, drop the existence-check ORDER BY, disable before recording state, suppress the environment-dependent warning in the test, and warn on a manual run that the transaction is left open. --- sql/object_reference--0.1.0--stable.sql | 17 +++++++++-------- sql/object_reference.sql | 15 ++++++++------- test/expected/event_trigger_disable.out | 2 +- test/sql/event_trigger_disable.sql | 15 ++++++++------- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index eecb13e..df03935 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -26,7 +26,7 @@ BEGIN BEGIN SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = v_name FOR UPDATE; EXCEPTION WHEN insufficient_privilege THEN - RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', v_name; + RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so races with ALTER EVENT TRIGGER are possible', v_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = v_name; END; INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) VALUES (v_name, v_enabled); @@ -222,7 +222,7 @@ BEGIN ; EXCEPTION WHEN insufficient_privilege THEN -- No catalog UPDATE rights: the row can't be locked. - RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', event_trigger_name; + RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so races with ALTER EVENT TRIGGER are possible', event_trigger_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; @@ -233,7 +233,10 @@ $body$ , 'Row-lock one event trigger where privileges allow, and return its current evtenabled (NULL if it does not exist).' ); --- name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. +/* + * The caller names the triggers to disable, rather than this disabling all of them, so that no trigger is disabled (or locked) beyond what the caller needs. + * name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. + */ SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -245,7 +248,6 @@ DECLARE v_name pg_catalog.pg_event_trigger.evtname%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN - -- Validate everything before any side effect. IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN RAISE 'event_trigger_names must be a non-empty array' USING ERRCODE = 'invalid_parameter_value' @@ -269,7 +271,6 @@ BEGIN SELECT n INTO v_name FROM unnest(event_trigger_names) n WHERE NOT EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = n) - ORDER BY n COLLATE "C" LIMIT 1 ; IF FOUND THEN @@ -305,7 +306,7 @@ BEGIN ; ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); - -- Fixed lock order across all callers, to avoid deadlock. + -- Sorting in "C" collation gives every caller the same lock order, so two callers (ourselves included) can't deadlock on each other's triggers. FOR v_name IN SELECT n FROM unnest(event_trigger_names) n ORDER BY n COLLATE "C" LOOP v_enabled := _object_reference._event_trigger__lock(v_name); @@ -316,10 +317,10 @@ BEGIN ; END IF; + PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) VALUES (v_name, v_enabled); - - PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); END LOOP; END $body$ diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 97e0f13..2a15433 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1601,7 +1601,7 @@ BEGIN ; EXCEPTION WHEN insufficient_privilege THEN -- No catalog UPDATE rights: the row can't be locked. - RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked', event_trigger_name; + RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so races with ALTER EVENT TRIGGER are possible', event_trigger_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = event_trigger_name; END; @@ -1612,7 +1612,10 @@ $body$ , 'Row-lock one event trigger where privileges allow, and return its current evtenabled (NULL if it does not exist).' ); --- name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. +/* + * The caller names the triggers to disable, rather than this disabling all of them, so that no trigger is disabled (or locked) beyond what the caller needs. + * name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. + */ SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' , $args$ @@ -1624,7 +1627,6 @@ DECLARE v_name pg_catalog.pg_event_trigger.evtname%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; BEGIN - -- Validate everything before any side effect. IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN RAISE 'event_trigger_names must be a non-empty array' USING ERRCODE = 'invalid_parameter_value' @@ -1648,7 +1650,6 @@ BEGIN SELECT n INTO v_name FROM unnest(event_trigger_names) n WHERE NOT EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = n) - ORDER BY n COLLATE "C" LIMIT 1 ; IF FOUND THEN @@ -1684,7 +1685,7 @@ BEGIN ; ALTER TABLE pg_temp.__object_reference__event_trigger_state ADD PRIMARY KEY (evtname); - -- Fixed lock order across all callers, to avoid deadlock. + -- Sorting in "C" collation gives every caller the same lock order, so two callers (ourselves included) can't deadlock on each other's triggers. FOR v_name IN SELECT n FROM unnest(event_trigger_names) n ORDER BY n COLLATE "C" LOOP v_enabled := _object_reference._event_trigger__lock(v_name); @@ -1695,10 +1696,10 @@ BEGIN ; END IF; + PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) VALUES (v_name, v_enabled); - - PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); END LOOP; END $body$ diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 701e73f..1c6cf73 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -36,7 +36,6 @@ ok 33 - disable() with unsorted names ok 34 - names were processed in "C" collation order, not input order ok 35 - enable() after unsorted disable() ok 36 - all three restored -WARNING: could not lock the pg_event_trigger row of event trigger "event_trigger_disable_test__a"; reading its state without a lock, so a concurrent ALTER EVENT TRIGGER of it is not blocked ok 37 - helper does not fail for a role that cannot row-lock the catalog ok 38 - helper still returns the trigger's evtenabled ok 39 - helper took no advisory lock @@ -48,3 +47,4 @@ ok 44 - stop the capture group ok 45 - object_reference schema must not be part of the resolved search_path ok 46 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! +# WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index d6bb554..8b39150 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -3,8 +3,8 @@ \i test/load.sql /* - * This file runs in one transaction that is never committed (test/load.sql - * opens it, test/finish.sql leaves it open), and pg_regress runs the test files + * This file assumes its transaction is never committed (test/load.sql opens + * it, test/finish.sql leaves it open) and that pg_regress runs the test files * one at a time, so no other session ever sees an event trigger disabled here. * Most tests use their own no-op event triggers, whose names and prior states * they control; the capture test disables the real zzz_object_reference_capture. @@ -248,11 +248,11 @@ SELECT is( * A role without UPDATE on the catalog gets the trigger's state read * unlocked, with a WARNING naming the trigger, instead of an error. The DO * block runs as that role, and first proves the role really cannot FOR UPDATE - * the catalog. The WARNING is checked via the expected output, where it - * precedes this assertion's `ok` line. Terse verbosity keeps the source - * location out of it. + * the catalog. The WARNING is suppressed because whether it is raised depends + * on the environment's catalog privileges. */ -\set VERBOSITY terse +SELECT current_setting('client_min_messages') AS saved_client_min_messages \gset +SET client_min_messages = error; SELECT lives_ok( $t$DO $do$ BEGIN @@ -269,7 +269,7 @@ SELECT lives_ok( $do$$t$ , 'helper does not fail for a role that cannot row-lock the catalog' ); -\set VERBOSITY verbose +SET client_min_messages = :'saved_client_min_messages'; SELECT is( current_setting('event_trigger_disable_test.enabled') , 'O' @@ -310,5 +310,6 @@ SELECT lives_ok( ); \i test/finish.sql +\echo # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. -- vi: expandtab sw=2 ts=2 From 8f48f5f04697df92cd09ff90f86d21c1e05411a6 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 11:33:27 -0600 Subject: [PATCH 16/35] Restructure event trigger comments; move update scenarios to test/build Place the disable/enable overview, sentinel and locking notes directly above event_trigger__disable(), give _event_trigger__lock() its own short comment, and say why enable() restores in sorted order. The update script's bootstrap now runs ALTER EVENT TRIGGER before recording the state, like disable(). The rolled-back update scenarios move from test/install/load.sql to test/build/update.sql, whose output is compared, so a failed assertion fails make. load.sql keeps only the committed update; the shared state assertion lives in test/helpers/event_trigger_state.sql. --- sql/object_reference--0.1.0--stable.sql | 66 ++++++----- sql/object_reference.sql | 64 +++++----- test/build/expected/update.out | 1 + test/build/update.sql | 148 ++++++++++++++++++++++++ test/helpers/event_trigger_state.sql | 33 ++++++ test/install/load.sql | 125 +------------------- 6 files changed, 258 insertions(+), 179 deletions(-) create mode 100644 test/build/expected/update.out create mode 100644 test/build/update.sql create mode 100644 test/helpers/event_trigger_state.sql diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index df03935..b5609a9 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -29,8 +29,8 @@ BEGIN RAISE WARNING 'could not lock the pg_event_trigger row of event trigger "%"; reading its state without a lock, so races with ALTER EVENT TRIGGER are possible', v_name; SELECT evtenabled INTO v_enabled FROM pg_catalog.pg_event_trigger WHERE evtname = v_name; END; - INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) VALUES (v_name, v_enabled); EXECUTE format('ALTER EVENT TRIGGER %I DISABLE', v_name); + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) VALUES (v_name, v_enabled); END LOOP; END $$; @@ -182,27 +182,8 @@ $body$ ); /* - * event_trigger__disable(names) disables the given event triggers, and - * event_trigger__enable() restores each one's exact prior state. Both are - * internal: for this extension's own update scripts, not the public API. - * - * Avoid disabling event triggers wherever another option exists. The change - * is transactional: other sessions don't see it until commit, and it rolls - * back with the caller's transaction, so no cleanup-on-error logic is needed. - * disable() row-locks each trigger's catalog row before reading its state - * (see _event_trigger__lock() below), which blocks a concurrent raw ALTER - * EVENT TRIGGER of the same trigger until commit. - * - * disable() also creates a no-op ddl_command_end event trigger, the sentinel, - * which enable() drops after restoring every state. While every sql_drop, - * table_rewrite and ddl_command_end event trigger is disabled, Postgres gives - * the commands nested in an extension script no event trigger state of their - * own: it collects them onto the enclosing CREATE/ALTER EXTENSION and replays - * them to that command's ddl_command_end once the triggers are back. There, - * pg_event_trigger_ddl_commands() fails on objects dropped since (before - * PostgreSQL 14) or hands capture the script's own objects. One enabled event - * trigger prevents the collection. The sentinel's function is in pg_temp, - * which capture skips. + * Takes the row lock event_trigger__disable() relies on to block a concurrent raw + * ALTER EVENT TRIGGER of the same trigger until commit. */ SELECT __object_reference.create_function( '_object_reference._event_trigger__lock' @@ -234,8 +215,33 @@ $body$ ); /* - * The caller names the triggers to disable, rather than this disabling all of them, so that no trigger is disabled (or locked) beyond what the caller needs. - * name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. + * event_trigger__disable(names) disables the named event triggers, and + * event_trigger__enable() restores each one's exact prior state. Both are + * internal: for this extension's own update scripts, not the public API. + * Avoid disabling event triggers wherever another option exists. + * + * The caller names the triggers, rather than this disabling all of them, so + * that no trigger is disabled (or locked) beyond what the caller needs. + * + * The change is transactional: other sessions don't see it until commit, and + * it rolls back with the caller's transaction, so no cleanup-on-error logic is + * needed. disable() locks each trigger's catalog row before reading its state + * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER + * of the same trigger until commit. + * + * disable() first creates a no-op ddl_command_end event trigger, the sentinel, + * which enable() drops after restoring every state. While every sql_drop, + * table_rewrite and ddl_command_end event trigger is disabled, Postgres gives + * the commands nested in an extension script no event trigger state of their + * own: it collects them onto the enclosing CREATE/ALTER EXTENSION and replays + * them to that command's ddl_command_end once the triggers are back. There, + * pg_event_trigger_ddl_commands() fails on objects dropped since (before + * PostgreSQL 14) or hands capture the script's own objects. One enabled event + * trigger prevents the collection. The sentinel's function is in pg_temp, + * which capture skips. + * + * name[] and "char"[] stand in for %TYPE[], which signatures never accept and + * PL/pgSQL DECLARE accepts only from PostgreSQL 17. */ SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' @@ -294,7 +300,7 @@ BEGIN ; END IF; - -- The sentinel (see above), before anything is disabled + -- The sentinel, before anything is disabled CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end @@ -332,7 +338,7 @@ SELECT __object_reference.create_function( , 'void LANGUAGE plpgsql' , $body$ DECLARE - -- name[] and "char"[] stand in for %TYPE[], as in disable() above. + -- name[] and "char"[] stand in for %TYPE[], as in disable(). v_names name[]; v_states "char"[]; i int; @@ -343,7 +349,11 @@ BEGIN ; END IF; - -- Same order disable() locked in + /* + * Same sorted order disable() locks in: run in a later transaction, these + * ALTERs take the row locks again, and one order for every caller prevents + * deadlock. + */ SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") INTO v_names, v_states FROM pg_temp.__object_reference__event_trigger_state @@ -365,7 +375,7 @@ BEGIN )); END LOOP; - -- Only now that every state is restored (see the sentinel note above) + -- Only now that every state is restored (see the sentinel note above event_trigger__disable()) DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; DROP FUNCTION pg_temp.__object_reference__event_trigger_sentinel(); END diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 2a15433..4f05cc2 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1561,27 +1561,8 @@ $body$ ); /* - * event_trigger__disable(names) disables the given event triggers, and - * event_trigger__enable() restores each one's exact prior state. Both are - * internal: for this extension's own update scripts, not the public API. - * - * Avoid disabling event triggers wherever another option exists. The change - * is transactional: other sessions don't see it until commit, and it rolls - * back with the caller's transaction, so no cleanup-on-error logic is needed. - * disable() row-locks each trigger's catalog row before reading its state - * (see _event_trigger__lock() below), which blocks a concurrent raw ALTER - * EVENT TRIGGER of the same trigger until commit. - * - * disable() also creates a no-op ddl_command_end event trigger, the sentinel, - * which enable() drops after restoring every state. While every sql_drop, - * table_rewrite and ddl_command_end event trigger is disabled, Postgres gives - * the commands nested in an extension script no event trigger state of their - * own: it collects them onto the enclosing CREATE/ALTER EXTENSION and replays - * them to that command's ddl_command_end once the triggers are back. There, - * pg_event_trigger_ddl_commands() fails on objects dropped since (before - * PostgreSQL 14) or hands capture the script's own objects. One enabled event - * trigger prevents the collection. The sentinel's function is in pg_temp, - * which capture skips. + * Takes the row lock event_trigger__disable() relies on to block a concurrent raw + * ALTER EVENT TRIGGER of the same trigger until commit. */ SELECT __object_reference.create_function( '_object_reference._event_trigger__lock' @@ -1613,8 +1594,33 @@ $body$ ); /* - * The caller names the triggers to disable, rather than this disabling all of them, so that no trigger is disabled (or locked) beyond what the caller needs. - * name[] and "char"[] stand in for %TYPE[], which signatures never accept and PL/pgSQL DECLARE accepts only from PostgreSQL 17. + * event_trigger__disable(names) disables the named event triggers, and + * event_trigger__enable() restores each one's exact prior state. Both are + * internal: for this extension's own update scripts, not the public API. + * Avoid disabling event triggers wherever another option exists. + * + * The caller names the triggers, rather than this disabling all of them, so + * that no trigger is disabled (or locked) beyond what the caller needs. + * + * The change is transactional: other sessions don't see it until commit, and + * it rolls back with the caller's transaction, so no cleanup-on-error logic is + * needed. disable() locks each trigger's catalog row before reading its state + * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER + * of the same trigger until commit. + * + * disable() first creates a no-op ddl_command_end event trigger, the sentinel, + * which enable() drops after restoring every state. While every sql_drop, + * table_rewrite and ddl_command_end event trigger is disabled, Postgres gives + * the commands nested in an extension script no event trigger state of their + * own: it collects them onto the enclosing CREATE/ALTER EXTENSION and replays + * them to that command's ddl_command_end once the triggers are back. There, + * pg_event_trigger_ddl_commands() fails on objects dropped since (before + * PostgreSQL 14) or hands capture the script's own objects. One enabled event + * trigger prevents the collection. The sentinel's function is in pg_temp, + * which capture skips. + * + * name[] and "char"[] stand in for %TYPE[], which signatures never accept and + * PL/pgSQL DECLARE accepts only from PostgreSQL 17. */ SELECT __object_reference.create_function( '_object_reference.event_trigger__disable' @@ -1673,7 +1679,7 @@ BEGIN ; END IF; - -- The sentinel (see above), before anything is disabled + -- The sentinel, before anything is disabled CREATE FUNCTION pg_temp.__object_reference__event_trigger_sentinel() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end @@ -1711,7 +1717,7 @@ SELECT __object_reference.create_function( , 'void LANGUAGE plpgsql' , $body$ DECLARE - -- name[] and "char"[] stand in for %TYPE[], as in disable() above. + -- name[] and "char"[] stand in for %TYPE[], as in disable(). v_names name[]; v_states "char"[]; i int; @@ -1722,7 +1728,11 @@ BEGIN ; END IF; - -- Same order disable() locked in + /* + * Same sorted order disable() locks in: run in a later transaction, these + * ALTERs take the row locks again, and one order for every caller prevents + * deadlock. + */ SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") INTO v_names, v_states FROM pg_temp.__object_reference__event_trigger_state @@ -1744,7 +1754,7 @@ BEGIN )); END LOOP; - -- Only now that every state is restored (see the sentinel note above) + -- Only now that every state is restored (see the sentinel note above event_trigger__disable()) DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; DROP FUNCTION pg_temp.__object_reference__event_trigger_sentinel(); END diff --git a/test/build/expected/update.out b/test/build/expected/update.out new file mode 100644 index 0000000..25fdbb1 --- /dev/null +++ b/test/build/expected/update.out @@ -0,0 +1 @@ +\set ECHO none diff --git a/test/build/update.sql b/test/build/update.sql new file mode 100644 index 0000000..a9b22e4 --- /dev/null +++ b/test/build/update.sql @@ -0,0 +1,148 @@ +\set ECHO none +/* + * Update-mode scenarios (make test-update): install the old version, then + * ALTER EXTENSION UPDATE, in rolled-back transactions, so nothing persists. + * Each starts a capture group, asserts the event trigger state it starts the + * update from, updates, and checks the result: + * 1. All three triggers at their default ENABLE; a table created after the + * update is captured. + * 2. zzz__object_reference_drop already disabled, and still disabled after. + * 3. A table captured before the update is renamed after it; the capture + * group shows the new name. + * + * Runs only when object_reference.test_load_mode is 'update' (the Makefile's + * TEST_LOAD_SOURCE); the output is empty in every mode, so a failed assertion + * shows up as a diff against the expected file, which fails test-build. + * count_nulls is created per scenario (the 0.1.0 install script needs it) and + * rolled back with it. + */ +\set ON_ERROR_STOP 1 + +SELECT current_setting('object_reference.test_load_mode') = 'update' AS object_reference_mode_update +\gset + +\if :object_reference_mode_update +\o /dev/null + +SELECT current_setting('object_reference.test_update_from') AS object_reference_test_update_from \gset +SELECT CASE WHEN current_setting('object_reference.test_update_to') = '' THEN '' + ELSE format('TO %L', current_setting('object_reference.test_update_to')) END + AS object_reference_update_to_clause \gset + +SET client_min_messages = ERROR; + +\i test/helpers/event_trigger_state.sql + +/* + * 0.1.0's install script sets client_min_messages = WARNING itself and calls a + * cat_tools function that has since been deprecated, so the session setting + * can't hide its WARNINGs and they would vary with the cat_tools version. A + * per-function setting can: create cat_tools (rolled back with the scenario) + * and silence every function in it. + */ +CREATE FUNCTION pg_temp.quiet_cat_tools() RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_function regprocedure; +BEGIN + CREATE EXTENSION IF NOT EXISTS cat_tools; + FOR v_function IN + SELECT p.oid::regprocedure + FROM pg_catalog.pg_proc p + WHERE p.pronamespace = 'cat_tools'::regnamespace AND p.prokind = 'f' + LOOP + EXECUTE format('ALTER FUNCTION %s SET client_min_messages = error', v_function); + END LOOP; +END +$$; + +CREATE FUNCTION pg_temp.assert_update_finished( + step text +) RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_installed text := (SELECT extversion FROM pg_catalog.pg_extension WHERE extname = 'object_reference'); + v_target text := coalesce( + nullif(current_setting('object_reference.test_update_to'), '') + , (SELECT default_version FROM pg_catalog.pg_available_extensions WHERE name = 'object_reference') + ); +BEGIN + IF v_installed IS DISTINCT FROM v_target THEN + RAISE EXCEPTION 'object_reference is at version % %, expected %', v_installed, step, v_target; + END IF; + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RAISE EXCEPTION 'event trigger state table left behind %', step; + END IF; + IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') + OR to_regprocedure('pg_temp.__object_reference__event_trigger_sentinel()') IS NOT NULL + THEN + RAISE EXCEPTION 'sentinel event trigger or its function left behind %', step; + END IF; +END +$$; + +CREATE FUNCTION pg_temp.assert_capture_group( + step text + , expected text[] +) RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_members text[]; +BEGIN + SELECT coalesce(array_agg(array_to_string(o.object_names, '.') ORDER BY o.object_names), '{}') + INTO v_members + FROM _object_reference.object_group__object m + JOIN _object_reference.object o USING (object_id) + WHERE m.object_group_id = (object_reference.object_group__get('load_update_scenario')).object_group_id + ; + IF v_members IS DISTINCT FROM expected THEN + RAISE EXCEPTION 'unexpected capture group members %', step + USING DETAIL = format('Members are %L, expected %L.', v_members, expected) + ; + END IF; +END +$$; + +BEGIN; +SELECT pg_temp.quiet_cat_tools(); +CREATE EXTENSION IF NOT EXISTS count_nulls; +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); +SELECT pg_temp.assert_event_trigger_state('before the update in scenario 1', 'O', 'O', 'O'); +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SELECT pg_temp.assert_update_finished('after the update in scenario 1'); +SELECT pg_temp.assert_event_trigger_state('after the update in scenario 1', 'O', 'O', 'O'); +SELECT pg_temp.assert_capture_group('after the update in scenario 1', '{}'); +CREATE TABLE load_update_scenario__after(); +SELECT pg_temp.assert_capture_group('after CREATE TABLE in scenario 1', '{public.load_update_scenario__after}'); +ROLLBACK; + +BEGIN; +SELECT pg_temp.quiet_cat_tools(); +CREATE EXTENSION IF NOT EXISTS count_nulls; +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +ALTER EVENT TRIGGER zzz__object_reference_drop DISABLE; +SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); +SELECT pg_temp.assert_event_trigger_state('before the update in scenario 2', 'O', 'O', 'D'); +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SELECT pg_temp.assert_update_finished('after the update in scenario 2'); +SELECT pg_temp.assert_event_trigger_state('after the update in scenario 2', 'O', 'O', 'D'); +SELECT pg_temp.assert_capture_group('after the update in scenario 2', '{}'); +ROLLBACK; + +BEGIN; +SELECT pg_temp.quiet_cat_tools(); +CREATE EXTENSION IF NOT EXISTS count_nulls; +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); +CREATE TABLE load_update_scenario__before(); +SELECT pg_temp.assert_capture_group('before the update in scenario 3', '{public.load_update_scenario__before}'); +SELECT pg_temp.assert_event_trigger_state('before the update in scenario 3', 'O', 'O', 'O'); +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SELECT pg_temp.assert_update_finished('after the update in scenario 3'); +SELECT pg_temp.assert_event_trigger_state('after the update in scenario 3', 'O', 'O', 'O'); +ALTER TABLE load_update_scenario__before RENAME TO load_update_scenario__renamed; +SELECT pg_temp.assert_capture_group('after the rename in scenario 3', '{public.load_update_scenario__renamed}'); +ROLLBACK; + +\o +\endif + +-- vi: expandtab sw=2 ts=2 diff --git a/test/helpers/event_trigger_state.sql b/test/helpers/event_trigger_state.sql new file mode 100644 index 0000000..af0cd5e --- /dev/null +++ b/test/helpers/event_trigger_state.sql @@ -0,0 +1,33 @@ +/* + * Asserts the enabled state of the extension's three event triggers (used by + * the update-mode checks in test/install/load.sql and test/build/update.sql). + */ +CREATE FUNCTION pg_temp.assert_event_trigger_state( + step text + , capture_state "char" + , fix_identity_state "char" + , drop_state "char" +) RETURNS void LANGUAGE plpgsql AS $$ +DECLARE + v_mismatches text; +BEGIN + SELECT string_agg( + format('%I is %L, expected %L.', e.evtname, t.evtenabled, e.evtenabled) + , ' ' ORDER BY e.evtname + ) + INTO v_mismatches + FROM (VALUES + ('zzz_object_reference_capture'::name, capture_state) + , ('zzz_object_reference__fix_identity', fix_identity_state) + , ('zzz__object_reference_drop', drop_state) + ) e(evtname, evtenabled) + LEFT JOIN pg_catalog.pg_event_trigger t USING (evtname) + WHERE t.evtenabled IS DISTINCT FROM e.evtenabled + ; + IF v_mismatches IS NOT NULL THEN + RAISE EXCEPTION 'unexpected event trigger state %', step + USING DETAIL = v_mismatches + ; + END IF; +END +$$; diff --git a/test/install/load.sql b/test/install/load.sql index 07798f5..0a09938 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -176,130 +176,7 @@ SELECT CASE WHEN :'object_reference_test_update_to' = '' THEN '' */ CREATE EXTENSION IF NOT EXISTS count_nulls; -/* - * Update scenarios. Each installs the old version inside its own - * transaction and rolls back, so nothing leaks into the committed install - * below (the extension and its sequences don't exist after the rollback). - * Each one starts a capture group, asserts the event trigger state it starts - * the update from, updates, and checks the result: - * 1. All three triggers at their default ENABLE; a table created after the - * update is captured. - * 2. zzz__object_reference_drop already disabled, and still disabled after. - * 3. A table captured before the update is renamed after it; the capture - * group shows the new name. - */ -CREATE FUNCTION pg_temp.assert_event_trigger_state( - step text - , capture_state "char" - , fix_identity_state "char" - , drop_state "char" -) RETURNS void LANGUAGE plpgsql AS $$ -DECLARE - v_mismatches text; -BEGIN - SELECT string_agg( - format('%I is %L, expected %L.', e.evtname, t.evtenabled, e.evtenabled) - , ' ' ORDER BY e.evtname - ) - INTO v_mismatches - FROM (VALUES - ('zzz_object_reference_capture'::name, capture_state) - , ('zzz_object_reference__fix_identity', fix_identity_state) - , ('zzz__object_reference_drop', drop_state) - ) e(evtname, evtenabled) - LEFT JOIN pg_catalog.pg_event_trigger t USING (evtname) - WHERE t.evtenabled IS DISTINCT FROM e.evtenabled - ; - IF v_mismatches IS NOT NULL THEN - RAISE EXCEPTION 'unexpected event trigger state %', step - USING DETAIL = v_mismatches - ; - END IF; -END -$$; - -CREATE FUNCTION pg_temp.assert_update_finished( - step text -) RETURNS void LANGUAGE plpgsql AS $$ -DECLARE - v_installed text := (SELECT extversion FROM pg_catalog.pg_extension WHERE extname = 'object_reference'); - v_target text := coalesce( - nullif(current_setting('object_reference.test_update_to'), '') - , (SELECT default_version FROM pg_catalog.pg_available_extensions WHERE name = 'object_reference') - ); -BEGIN - IF v_installed IS DISTINCT FROM v_target THEN - RAISE EXCEPTION 'object_reference is at version % %, expected %', v_installed, step, v_target; - END IF; - IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN - RAISE EXCEPTION 'event trigger state table left behind %', step; - END IF; - IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') - OR to_regprocedure('pg_temp.__object_reference__event_trigger_sentinel()') IS NOT NULL - THEN - RAISE EXCEPTION 'sentinel event trigger or its function left behind %', step; - END IF; -END -$$; - -CREATE FUNCTION pg_temp.assert_capture_group( - step text - , expected text[] -) RETURNS void LANGUAGE plpgsql AS $$ -DECLARE - v_members text[]; -BEGIN - SELECT coalesce(array_agg(array_to_string(o.object_names, '.') ORDER BY o.object_names), '{}') - INTO v_members - FROM _object_reference.object_group__object m - JOIN _object_reference.object o USING (object_id) - WHERE m.object_group_id = (object_reference.object_group__get('load_update_scenario')).object_group_id - ; - IF v_members IS DISTINCT FROM expected THEN - RAISE EXCEPTION 'unexpected capture group members %', step - USING DETAIL = format('Members are %L, expected %L.', v_members, expected) - ; - END IF; -END -$$; - -SET client_min_messages = ERROR; - -BEGIN; -CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; -SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); -SELECT pg_temp.assert_event_trigger_state('before the update in scenario 1', 'O', 'O', 'O'); -ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; -SELECT pg_temp.assert_update_finished('after the update in scenario 1'); -SELECT pg_temp.assert_event_trigger_state('after the update in scenario 1', 'O', 'O', 'O'); -SELECT pg_temp.assert_capture_group('after the update in scenario 1', '{}'); -CREATE TABLE load_update_scenario__after(); -SELECT pg_temp.assert_capture_group('after CREATE TABLE in scenario 1', '{public.load_update_scenario__after}'); -ROLLBACK; - -BEGIN; -CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; -ALTER EVENT TRIGGER zzz__object_reference_drop DISABLE; -SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); -SELECT pg_temp.assert_event_trigger_state('before the update in scenario 2', 'O', 'O', 'D'); -ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; -SELECT pg_temp.assert_update_finished('after the update in scenario 2'); -SELECT pg_temp.assert_event_trigger_state('after the update in scenario 2', 'O', 'O', 'D'); -SELECT pg_temp.assert_capture_group('after the update in scenario 2', '{}'); -ROLLBACK; - -BEGIN; -CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; -SELECT object_reference.capture__start(object_reference.object_group__create('load_update_scenario')); -CREATE TABLE load_update_scenario__before(); -SELECT pg_temp.assert_capture_group('before the update in scenario 3', '{public.load_update_scenario__before}'); -SELECT pg_temp.assert_event_trigger_state('before the update in scenario 3', 'O', 'O', 'O'); -ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; -SELECT pg_temp.assert_update_finished('after the update in scenario 3'); -SELECT pg_temp.assert_event_trigger_state('after the update in scenario 3', 'O', 'O', 'O'); -ALTER TABLE load_update_scenario__before RENAME TO load_update_scenario__renamed; -SELECT pg_temp.assert_capture_group('after the rename in scenario 3', '{public.load_update_scenario__renamed}'); -ROLLBACK; +\i test/helpers/event_trigger_state.sql SET client_min_messages = WARNING; From 54c95e7550dcfc102ee551316b3da8c5e930dfe0 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 11:40:12 -0600 Subject: [PATCH 17/35] Clarify the enable() lock-order comment --- sql/object_reference--0.1.0--stable.sql | 5 ++--- sql/object_reference.sql | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index b5609a9..495916f 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -350,9 +350,8 @@ BEGIN END IF; /* - * Same sorted order disable() locks in: run in a later transaction, these - * ALTERs take the row locks again, and one order for every caller prevents - * deadlock. + * Same sorted order as disable(): if run in a later transaction, these ALTERs + * re-take the row locks, and one order for every caller prevents deadlock. */ SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") INTO v_names, v_states diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 4f05cc2..9fecbbb 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1729,9 +1729,8 @@ BEGIN END IF; /* - * Same sorted order disable() locks in: run in a later transaction, these - * ALTERs take the row locks again, and one order for every caller prevents - * deadlock. + * Same sorted order as disable(): if run in a later transaction, these ALTERs + * re-take the row locks, and one order for every caller prevents deadlock. */ SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") INTO v_names, v_states From 13fb35dd60dfccb4f756645df120004ee7a567d1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 11:43:59 -0600 Subject: [PATCH 18/35] Move the D/R/A restore check into test/build/update.sql; load.sql only updates --- test/build/update.sql | 15 +++++++++++++++ test/helpers/event_trigger_state.sql | 2 +- test/install/load.sql | 27 --------------------------- 3 files changed, 16 insertions(+), 28 deletions(-) diff --git a/test/build/update.sql b/test/build/update.sql index a9b22e4..d9eef76 100644 --- a/test/build/update.sql +++ b/test/build/update.sql @@ -9,6 +9,8 @@ * 2. zzz__object_reference_drop already disabled, and still disabled after. * 3. A table captured before the update is renamed after it; the capture * group shows the new name. + * 4. The three triggers start as DISABLE, ENABLE REPLICA and ENABLE ALWAYS + * and must be restored to exactly those states. * * Runs only when object_reference.test_load_mode is 'update' (the Makefile's * TEST_LOAD_SOURCE); the output is empty in every mode, so a failed assertion @@ -142,6 +144,19 @@ ALTER TABLE load_update_scenario__before RENAME TO load_update_scenario__renamed SELECT pg_temp.assert_capture_group('after the rename in scenario 3', '{public.load_update_scenario__renamed}'); ROLLBACK; +BEGIN; +SELECT pg_temp.quiet_cat_tools(); +CREATE EXTENSION IF NOT EXISTS count_nulls; +CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; +ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; +ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE REPLICA; +ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE ALWAYS; +SELECT pg_temp.assert_event_trigger_state('before the update in scenario 4', 'D', 'R', 'A'); +ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; +SELECT pg_temp.assert_update_finished('after the update in scenario 4'); +SELECT pg_temp.assert_event_trigger_state('after the update in scenario 4', 'D', 'R', 'A'); +ROLLBACK; + \o \endif diff --git a/test/helpers/event_trigger_state.sql b/test/helpers/event_trigger_state.sql index af0cd5e..b447189 100644 --- a/test/helpers/event_trigger_state.sql +++ b/test/helpers/event_trigger_state.sql @@ -1,6 +1,6 @@ /* * Asserts the enabled state of the extension's three event triggers (used by - * the update-mode checks in test/install/load.sql and test/build/update.sql). + * test/build/update.sql). */ CREATE FUNCTION pg_temp.assert_event_trigger_state( step text diff --git a/test/install/load.sql b/test/install/load.sql index 0a09938..60aaa89 100644 --- a/test/install/load.sql +++ b/test/install/load.sql @@ -176,30 +176,10 @@ SELECT CASE WHEN :'object_reference_test_update_to' = '' THEN '' */ CREATE EXTENSION IF NOT EXISTS count_nulls; -\i test/helpers/event_trigger_state.sql - SET client_min_messages = WARNING; CREATE EXTENSION object_reference VERSION :'object_reference_test_update_from' CASCADE; -/* - * This check fails only this load.sql run: pgxntool ignores installcheck's exit - * status and fails `make test` only when regression.diffs exists, and this - * file's output is never compared (pg_regress 16+ prints "not ok" for it, - * PG12's prints "ok"). make fails because the failed transaction leaves the - * extension at test_update_from, and test/load.sql then raises in every later - * test file, as the installed version is not the one under test. - * - * It fails this load.sql run unless the update restores each of the - * extension's event triggers to the exact state it had before, starting each - * from a distinct non-default state. All three are reset to ENABLE afterwards, - * so the rest of the suite runs as usual. - */ -BEGIN; -ALTER EVENT TRIGGER zzz_object_reference_capture DISABLE; -ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE REPLICA; -ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE ALWAYS; - /* * Suppress the deprecation NOTICEs the update script emits (e.g. the reg* * pseudotype removal), matching cat_tools' own install/load.sql. @@ -207,13 +187,6 @@ ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE ALWAYS; SET client_min_messages = ERROR; ALTER EXTENSION object_reference UPDATE :object_reference_update_to_clause; SET client_min_messages = WARNING; - -SELECT pg_temp.assert_event_trigger_state('after the committed update', 'D', 'R', 'A'); - -ALTER EVENT TRIGGER zzz_object_reference_capture ENABLE; -ALTER EVENT TRIGGER zzz_object_reference__fix_identity ENABLE; -ALTER EVENT TRIGGER zzz__object_reference_drop ENABLE; -COMMIT; \else CREATE EXTENSION object_reference CASCADE; \endif From b2af0d00d87eb7fedc88c0291e2422dba9806dbe Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 11:51:50 -0600 Subject: [PATCH 19/35] Fix update.sql header: only scenarios 1-3 start a capture group --- test/build/update.sql | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/build/update.sql b/test/build/update.sql index d9eef76..9bac00a 100644 --- a/test/build/update.sql +++ b/test/build/update.sql @@ -2,8 +2,8 @@ /* * Update-mode scenarios (make test-update): install the old version, then * ALTER EXTENSION UPDATE, in rolled-back transactions, so nothing persists. - * Each starts a capture group, asserts the event trigger state it starts the - * update from, updates, and checks the result: + * Scenarios 1-3 start a capture group; each asserts the event trigger state it + * starts the update from, updates, and checks the result: * 1. All three triggers at their default ENABLE; a table created after the * update is captured. * 2. zzz__object_reference_drop already disabled, and still disabled after. From ddfe844385251b874670d0d7681fc2ed5854511a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 12:27:08 -0600 Subject: [PATCH 20/35] Drop error-precedence tests for event_trigger__disable() --- test/expected/event_trigger_disable.out | 38 ++++++++----------- test/sql/event_trigger_disable.sql | 49 +------------------------ 2 files changed, 16 insertions(+), 71 deletions(-) diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 1c6cf73..9726695 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..46 +1..38 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -24,27 +24,19 @@ ok 21 - disable() rejects a duplicate name in its own argument list ok 22 - disable() rejects a NULL array ok 23 - disable() rejects an empty array ok 24 - disable() rejects a NULL element -ok 25 - disable() so a call is in effect -ok 26 - duplicate name is reported before the in-effect error -ok 27 - unknown name is reported before the in-effect error -ok 28 - NULL array is reported before the in-effect error -ok 29 - empty array is reported before the in-effect error -ok 30 - NULL element is reported before the in-effect error -ok 31 - the call still in effect kept its own state -ok 32 - enable() ends that call -ok 33 - disable() with unsorted names -ok 34 - names were processed in "C" collation order, not input order -ok 35 - enable() after unsorted disable() -ok 36 - all three restored -ok 37 - helper does not fail for a role that cannot row-lock the catalog -ok 38 - helper still returns the trigger's evtenabled -ok 39 - helper took no advisory lock -ok 40 - start a capture group -ok 41 - disable() zzz_object_reference_capture -ok 42 - enable() restores zzz_object_reference_capture -ok 43 - only the table created after enable() was captured -ok 44 - stop the capture group -ok 45 - object_reference schema must not be part of the resolved search_path -ok 46 - _object_reference schema must not be part of the resolved search_path +ok 25 - disable() with unsorted names +ok 26 - names were processed in "C" collation order, not input order +ok 27 - enable() after unsorted disable() +ok 28 - all three restored +ok 29 - helper does not fail for a role that cannot row-lock the catalog +ok 30 - helper still returns the trigger's evtenabled +ok 31 - helper took no advisory lock +ok 32 - start a capture group +ok 33 - disable() zzz_object_reference_capture +ok 34 - enable() restores zzz_object_reference_capture +ok 35 - only the table created after enable() was captured +ok 36 - stop the capture group +ok 37 - object_reference schema must not be part of the resolved search_path +ok 38 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 8b39150..cab69cc 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -33,7 +33,6 @@ SELECT plan( +1 -- disable() rejects an unknown event trigger name +1 -- disable() rejects a duplicate name in its own argument list +3 -- disable() rejects a NULL array, an empty array and a NULL element - +8 -- argument errors are reported in preference to the "previous call in effect" error +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() @@ -157,8 +156,7 @@ SELECT throws_ok( /* * Argument errors are reported by disable() itself; any state change made * before the error would be rolled back with the call, so these tests cannot - * observe whether validation ran before a side effect. What they can observe - * is which error wins when several apply (see the precedence tests below). + * observe whether validation ran before a side effect. */ SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable(NULL)$$ @@ -179,51 +177,6 @@ SELECT throws_ok( , 'disable() rejects a NULL element' ); --- Argument errors are reported even while a previous call is in effect -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() so a call is in effect' -); -SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__b,event_trigger_disable_test__b}')$$ - , '22023' - , 'event_trigger_names contains a duplicate name' - , 'duplicate name is reported before the in-effect error' -); -SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable('{no_such_event_trigger}')$$ - , '42704' - , 'event trigger "no_such_event_trigger" does not exist' - , 'unknown name is reported before the in-effect error' -); -SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable(NULL)$$ - , '22023' - , 'event_trigger_names must be a non-empty array' - , 'NULL array is reported before the in-effect error' -); -SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable('{}')$$ - , '22023' - , 'event_trigger_names must be a non-empty array' - , 'empty array is reported before the in-effect error' -); -SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__b,NULL}')$$ - , '22023' - , 'event_trigger_names contains a NULL name' - , 'NULL element is reported before the in-effect error' -); -SELECT is( - (SELECT array_agg(evtname) FROM pg_temp.__object_reference__event_trigger_state) - , '{event_trigger_disable_test__a}'::name[] - , 'the call still in effect kept its own state' -); -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() ends that call' -); - -- Unsorted input is processed in "C" order SELECT lives_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__z,event_trigger_disable_test__a,"event_trigger_disable_test__B"}')$$ From e9aa9d6965421c434e3a01e11d1db7ebdc1d89ed Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 12:36:14 -0600 Subject: [PATCH 21/35] Move stray-sentinel and nested-disable tests to the end of event_trigger_disable.sql Both sections ended in a cleanup enable(); as one closing sequence that leaves a disable() in effect (the file's transaction is never committed) they need none. --- test/expected/event_trigger_disable.out | 53 +++++++++---------- test/sql/event_trigger_disable.sql | 67 ++++++++++--------------- 2 files changed, 51 insertions(+), 69 deletions(-) diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 9726695..734111f 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..38 +1..35 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -11,32 +11,29 @@ ok 8 - test trigger a is back to its original (origin) state ok 9 - test trigger b is still disabled -- its prior state was preserved, not assumed enabled ok 10 - enable() dropped the sentinel event trigger ok 11 - enable() dropped the sentinel function -ok 12 - disable() the first time -ok 13 - a second disable() without enable() in between is rejected -ok 14 - enable() cleans up so later tests are unaffected -ok 15 - disable() rejects an existing sentinel event trigger -ok 16 - the rejected disable() created no state table -ok 17 - disable() succeeds once the stray sentinel is gone -ok 18 - enable() cleans up so later tests are unaffected -ok 19 - enable() without disable() is rejected -ok 20 - disable() rejects an unknown event trigger name -ok 21 - disable() rejects a duplicate name in its own argument list -ok 22 - disable() rejects a NULL array -ok 23 - disable() rejects an empty array -ok 24 - disable() rejects a NULL element -ok 25 - disable() with unsorted names -ok 26 - names were processed in "C" collation order, not input order -ok 27 - enable() after unsorted disable() -ok 28 - all three restored -ok 29 - helper does not fail for a role that cannot row-lock the catalog -ok 30 - helper still returns the trigger's evtenabled -ok 31 - helper took no advisory lock -ok 32 - start a capture group -ok 33 - disable() zzz_object_reference_capture -ok 34 - enable() restores zzz_object_reference_capture -ok 35 - only the table created after enable() was captured -ok 36 - stop the capture group -ok 37 - object_reference schema must not be part of the resolved search_path -ok 38 - _object_reference schema must not be part of the resolved search_path +ok 12 - enable() without disable() is rejected +ok 13 - disable() rejects an unknown event trigger name +ok 14 - disable() rejects a duplicate name in its own argument list +ok 15 - disable() rejects a NULL array +ok 16 - disable() rejects an empty array +ok 17 - disable() rejects a NULL element +ok 18 - disable() with unsorted names +ok 19 - names were processed in "C" collation order, not input order +ok 20 - enable() after unsorted disable() +ok 21 - all three restored +ok 22 - helper does not fail for a role that cannot row-lock the catalog +ok 23 - helper still returns the trigger's evtenabled +ok 24 - helper took no advisory lock +ok 25 - start a capture group +ok 26 - disable() zzz_object_reference_capture +ok 27 - enable() restores zzz_object_reference_capture +ok 28 - only the table created after enable() was captured +ok 29 - stop the capture group +ok 30 - disable() rejects an existing sentinel event trigger +ok 31 - the rejected disable() created no state table +ok 32 - disable() succeeds once the stray sentinel is gone +ok 33 - a second disable() without enable() in between is rejected +ok 34 - object_reference schema must not be part of the resolved search_path +ok 35 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index cab69cc..6e72598 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -27,8 +27,6 @@ SELECT plan( 0 +7 -- multi-trigger disable/enable preserves each one's own prior state +4 -- the sentinel exists while a call is in effect and is gone, function included, after enable() - +3 -- nested disable() without an intervening enable() is rejected - +4 -- disable() rejects a sentinel event trigger left by another session, before changing anything +1 -- enable() without a matching disable() is rejected +1 -- disable() rejects an unknown event trigger name +1 -- disable() rejects a duplicate name in its own argument list @@ -36,6 +34,7 @@ SELECT plan( +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() + +4 -- closing sequence: a stray sentinel event trigger is rejected, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); @@ -90,45 +89,6 @@ SELECT ok( , 'enable() dropped the sentinel function' ); --- Nested disable() without an intervening enable() -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() the first time' -); -SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , '55000' - , 'event_trigger__disable() called while a previous call is still in effect' - , 'a second disable() without enable() in between is rejected' -); -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() cleans up so later tests are unaffected' -); - --- A sentinel event trigger that this session did not create -CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end EXECUTE FUNCTION event_trigger_disable_test__noop(); -SELECT throws_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , '55000' - , 'event_trigger__disable() from another session is still in effect' - , 'disable() rejects an existing sentinel event trigger' -); -SELECT is( - to_regclass('pg_temp.__object_reference__event_trigger_state') - , NULL - , 'the rejected disable() created no state table' -); -DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() succeeds once the stray sentinel is gone' -); -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() cleans up so later tests are unaffected' -); - -- enable() without a matching disable() SELECT throws_ok( $$SELECT _object_reference.event_trigger__enable()$$ @@ -262,6 +222,31 @@ SELECT lives_ok( , 'stop the capture group' ); +-- Must stay last: this sequence leaves a disable() call in effect. +CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end EXECUTE FUNCTION event_trigger_disable_test__noop(); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , '55000' + , 'event_trigger__disable() from another session is still in effect' + , 'disable() rejects an existing sentinel event trigger' +); +SELECT is( + to_regclass('pg_temp.__object_reference__event_trigger_state') + , NULL + , 'the rejected disable() created no state table' +); +DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() succeeds once the stray sentinel is gone' +); +SELECT throws_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , '55000' + , 'event_trigger__disable() called while a previous call is still in effect' + , 'a second disable() without enable() in between is rejected' +); + \i test/finish.sql \echo # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. From 5e3959d9fddada6700f5b9eac4b3bf168c6f1ab1 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 12:48:31 -0600 Subject: [PATCH 22/35] Name all four closing-sequence assertions in the event_trigger_disable plan comment The plan comment for the closing block listed two of its four assertions. It now names each one: the stray sentinel is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected. --- test/sql/event_trigger_disable.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 6e72598..31ae564 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -34,7 +34,7 @@ SELECT plan( +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() - +4 -- closing sequence: a stray sentinel event trigger is rejected, then a nested disable() is rejected + +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); From a05bc78d52104cc1255911b172cb00a039b8f506 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 14:19:01 -0600 Subject: [PATCH 23/35] Clarify event_trigger__disable() transactional behavior in its header comment --- sql/object_reference--0.1.0--stable.sql | 9 +++++---- sql/object_reference.sql | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 495916f..378e723 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -224,10 +224,11 @@ $body$ * that no trigger is disabled (or locked) beyond what the caller needs. * * The change is transactional: other sessions don't see it until commit, and - * it rolls back with the caller's transaction, so no cleanup-on-error logic is - * needed. disable() locks each trigger's catalog row before reading its state - * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER - * of the same trigger until commit. + * it commits or rolls back with the caller's transaction. An error therefore + * needs no cleanup logic, but a caller that commits without calling enable() + * leaves the triggers disabled. disable() locks each trigger's catalog row + * before reading its state (_event_trigger__lock()), which blocks a concurrent + * raw ALTER EVENT TRIGGER of the same trigger until commit. * * disable() first creates a no-op ddl_command_end event trigger, the sentinel, * which enable() drops after restoring every state. While every sql_drop, diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 9fecbbb..4561c84 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1603,10 +1603,11 @@ $body$ * that no trigger is disabled (or locked) beyond what the caller needs. * * The change is transactional: other sessions don't see it until commit, and - * it rolls back with the caller's transaction, so no cleanup-on-error logic is - * needed. disable() locks each trigger's catalog row before reading its state - * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER - * of the same trigger until commit. + * it commits or rolls back with the caller's transaction. An error therefore + * needs no cleanup logic, but a caller that commits without calling enable() + * leaves the triggers disabled. disable() locks each trigger's catalog row + * before reading its state (_event_trigger__lock()), which blocks a concurrent + * raw ALTER EVENT TRIGGER of the same trigger until commit. * * disable() first creates a no-op ddl_command_end event trigger, the sentinel, * which enable() drops after restoring every state. While every sql_drop, From 9d77972ef89f1feac504dd8d32ba183c26960703 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 14:39:37 -0600 Subject: [PATCH 24/35] Clarify event_trigger__disable() error and commit-without-enable() cases --- sql/object_reference--0.1.0--stable.sql | 11 ++++++----- sql/object_reference.sql | 11 ++++++----- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 378e723..5b61c7a 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -224,11 +224,12 @@ $body$ * that no trigger is disabled (or locked) beyond what the caller needs. * * The change is transactional: other sessions don't see it until commit, and - * it commits or rolls back with the caller's transaction. An error therefore - * needs no cleanup logic, but a caller that commits without calling enable() - * leaves the triggers disabled. disable() locks each trigger's catalog row - * before reading its state (_event_trigger__lock()), which blocks a concurrent - * raw ALTER EVENT TRIGGER of the same trigger until commit. + * it commits or rolls back with the caller's transaction. An error in the same + * transaction needs no cleanup logic; a commit without enable() leaves the + * triggers disabled, and the saved states are lost when the session ends. + * disable() locks each trigger's catalog row before reading its state + * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER + * of the same trigger until commit. * * disable() first creates a no-op ddl_command_end event trigger, the sentinel, * which enable() drops after restoring every state. While every sql_drop, diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 4561c84..845c8ae 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1603,11 +1603,12 @@ $body$ * that no trigger is disabled (or locked) beyond what the caller needs. * * The change is transactional: other sessions don't see it until commit, and - * it commits or rolls back with the caller's transaction. An error therefore - * needs no cleanup logic, but a caller that commits without calling enable() - * leaves the triggers disabled. disable() locks each trigger's catalog row - * before reading its state (_event_trigger__lock()), which blocks a concurrent - * raw ALTER EVENT TRIGGER of the same trigger until commit. + * it commits or rolls back with the caller's transaction. An error in the same + * transaction needs no cleanup logic; a commit without enable() leaves the + * triggers disabled, and the saved states are lost when the session ends. + * disable() locks each trigger's catalog row before reading its state + * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER + * of the same trigger until commit. * * disable() first creates a no-op ddl_command_end event trigger, the sentinel, * which enable() drops after restoring every state. While every sql_drop, From b36dd9abdf73efc8ebaf7d6d7c87732a404ab794 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 14:48:40 -0600 Subject: [PATCH 25/35] Fail the commit when event_trigger__disable() is not followed by enable() A deferred constraint trigger on the session's event-trigger state table raises at commit while any state row remains. enable() empties the table and fires the pending check before dropping it, so a later disable() still works. --- sql/object_reference--0.1.0--stable.sql | 62 ++++++++++++++++++++++--- sql/object_reference.sql | 62 ++++++++++++++++++++++--- test/build/expected/build.out | 1 + test/expected/event_trigger_disable.out | 22 ++++++--- test/sql/event_trigger_disable.sql | 45 +++++++++++++++++- 5 files changed, 170 insertions(+), 22 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index 5b61c7a..e154ca0 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -214,6 +214,24 @@ $body$ , 'Row-lock one event trigger where privileges allow, and return its current evtenabled (NULL if it does not exist).' ); +SELECT __object_reference.create_function( + '_object_reference._event_trigger__state_check' + , '' + , 'trigger LANGUAGE plpgsql' + , $body$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_temp.__object_reference__event_trigger_state) THEN + RAISE 'transaction committed without event_trigger__enable() after event_trigger__disable()' + USING ERRCODE = 'object_not_in_prerequisite_state' + , HINT = 'Call _object_reference.event_trigger__enable() before committing.' + ; + END IF; + RETURN NULL; +END +$body$ + , 'Deferred constraint trigger function: fail the commit if event_trigger__enable() has not run.' +); + /* * event_trigger__disable(names) disables the named event triggers, and * event_trigger__enable() restores each one's exact prior state. Both are @@ -225,8 +243,10 @@ $body$ * * The change is transactional: other sessions don't see it until commit, and * it commits or rolls back with the caller's transaction. An error in the same - * transaction needs no cleanup logic; a commit without enable() leaves the - * triggers disabled, and the saved states are lost when the session ends. + * transaction needs no cleanup logic. Committing without calling enable() + * raises an error (a deferred constraint trigger on the state table, + * _event_trigger__state_check()), so the commit rolls back and the triggers + * stay as they were. A hand-run ALTER EVENT TRIGGER is not covered. * disable() locks each trigger's catalog row before reading its state * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER * of the same trigger until commit. @@ -255,6 +275,9 @@ $args$ DECLARE v_name pg_catalog.pg_event_trigger.evtname%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; + -- name[] and "char"[] stand in for %TYPE[], as in the header comment. + v_names name[] := '{}'; + v_states "char"[] := '{}'; BEGIN IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN RAISE 'event_trigger_names must be a non-empty array' @@ -294,11 +317,11 @@ BEGIN ; END IF; - -- A committed sentinel (another session's, or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". + -- A committed sentinel (another session's, or one left by a commit that bypassed the state check) would make CREATE EVENT TRIGGER below fail with a plain "already exists". IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') THEN RAISE 'event_trigger__disable() from another session is still in effect' USING ERRCODE = 'object_not_in_prerequisite_state' - , HINT = 'Wait for that session to finish, or drop event trigger __object_reference__event_trigger_sentinel if it was committed without event_trigger__enable().' + , HINT = 'Wait for that session to finish, or drop event trigger __object_reference__event_trigger_sentinel if it was left behind without event_trigger__enable().' ; END IF; @@ -327,9 +350,29 @@ BEGIN PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); - INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) - VALUES (v_name, v_enabled); + v_names := v_names || v_name; + v_states := v_states || v_enabled; END LOOP; + + /* + * Created only now, with the state rows inserted after it: this CREATE fires any + * still-enabled capture trigger, which the caller is expected to have named above. + * Fails the commit while any state row remains, i.e. enable() has not run. + * ENABLE ALWAYS so session_replication_role can't skip it. + */ + CREATE CONSTRAINT TRIGGER __object_reference__event_trigger_state_check + AFTER INSERT ON pg_temp.__object_reference__event_trigger_state + DEFERRABLE INITIALLY DEFERRED + FOR EACH ROW EXECUTE FUNCTION _object_reference._event_trigger__state_check() + ; + ALTER TABLE pg_temp.__object_reference__event_trigger_state + ENABLE ALWAYS TRIGGER __object_reference__event_trigger_state_check + ; + -- A caller's earlier SET CONSTRAINTS ALL IMMEDIATE would otherwise apply to this new trigger too. + SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check DEFERRED; + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) + SELECT * FROM unnest(v_names, v_states) + ; END $body$ , 'Disable the given event triggers, remembering their exact prior state; pair with event_trigger__enable().' @@ -360,7 +403,12 @@ BEGIN FROM pg_temp.__object_reference__event_trigger_state ; - -- Dropped before anything is restored, so this DROP fires none of the restored triggers. + /* + * Dropped before anything is restored, so this DROP fires none of the restored triggers. + * The commit check's pending events make DROP TABLE fail, so empty the table and fire them (harmlessly) first. + */ + DELETE FROM pg_temp.__object_reference__event_trigger_state; + SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE; DROP TABLE pg_temp.__object_reference__event_trigger_state; FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 845c8ae..c7f1a6e 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1593,6 +1593,24 @@ $body$ , 'Row-lock one event trigger where privileges allow, and return its current evtenabled (NULL if it does not exist).' ); +SELECT __object_reference.create_function( + '_object_reference._event_trigger__state_check' + , '' + , 'trigger LANGUAGE plpgsql' + , $body$ +BEGIN + IF EXISTS (SELECT 1 FROM pg_temp.__object_reference__event_trigger_state) THEN + RAISE 'transaction committed without event_trigger__enable() after event_trigger__disable()' + USING ERRCODE = 'object_not_in_prerequisite_state' + , HINT = 'Call _object_reference.event_trigger__enable() before committing.' + ; + END IF; + RETURN NULL; +END +$body$ + , 'Deferred constraint trigger function: fail the commit if event_trigger__enable() has not run.' +); + /* * event_trigger__disable(names) disables the named event triggers, and * event_trigger__enable() restores each one's exact prior state. Both are @@ -1604,8 +1622,10 @@ $body$ * * The change is transactional: other sessions don't see it until commit, and * it commits or rolls back with the caller's transaction. An error in the same - * transaction needs no cleanup logic; a commit without enable() leaves the - * triggers disabled, and the saved states are lost when the session ends. + * transaction needs no cleanup logic. Committing without calling enable() + * raises an error (a deferred constraint trigger on the state table, + * _event_trigger__state_check()), so the commit rolls back and the triggers + * stay as they were. A hand-run ALTER EVENT TRIGGER is not covered. * disable() locks each trigger's catalog row before reading its state * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER * of the same trigger until commit. @@ -1634,6 +1654,9 @@ $args$ DECLARE v_name pg_catalog.pg_event_trigger.evtname%TYPE; v_enabled pg_catalog.pg_event_trigger.evtenabled%TYPE; + -- name[] and "char"[] stand in for %TYPE[], as in the header comment. + v_names name[] := '{}'; + v_states "char"[] := '{}'; BEGIN IF event_trigger_names IS NULL OR coalesce(array_length(event_trigger_names, 1), 0) = 0 THEN RAISE 'event_trigger_names must be a non-empty array' @@ -1673,11 +1696,11 @@ BEGIN ; END IF; - -- A committed sentinel (another session's, or one committed without enable()) would make CREATE EVENT TRIGGER below fail with a plain "already exists". + -- A committed sentinel (another session's, or one left by a commit that bypassed the state check) would make CREATE EVENT TRIGGER below fail with a plain "already exists". IF EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') THEN RAISE 'event_trigger__disable() from another session is still in effect' USING ERRCODE = 'object_not_in_prerequisite_state' - , HINT = 'Wait for that session to finish, or drop event trigger __object_reference__event_trigger_sentinel if it was committed without event_trigger__enable().' + , HINT = 'Wait for that session to finish, or drop event trigger __object_reference__event_trigger_sentinel if it was left behind without event_trigger__enable().' ; END IF; @@ -1706,9 +1729,29 @@ BEGIN PERFORM _object_reference.exec(format('ALTER EVENT TRIGGER %I DISABLE', v_name)); - INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) - VALUES (v_name, v_enabled); + v_names := v_names || v_name; + v_states := v_states || v_enabled; END LOOP; + + /* + * Created only now, with the state rows inserted after it: this CREATE fires any + * still-enabled capture trigger, which the caller is expected to have named above. + * Fails the commit while any state row remains, i.e. enable() has not run. + * ENABLE ALWAYS so session_replication_role can't skip it. + */ + CREATE CONSTRAINT TRIGGER __object_reference__event_trigger_state_check + AFTER INSERT ON pg_temp.__object_reference__event_trigger_state + DEFERRABLE INITIALLY DEFERRED + FOR EACH ROW EXECUTE FUNCTION _object_reference._event_trigger__state_check() + ; + ALTER TABLE pg_temp.__object_reference__event_trigger_state + ENABLE ALWAYS TRIGGER __object_reference__event_trigger_state_check + ; + -- A caller's earlier SET CONSTRAINTS ALL IMMEDIATE would otherwise apply to this new trigger too. + SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check DEFERRED; + INSERT INTO pg_temp.__object_reference__event_trigger_state(evtname, evtenabled) + SELECT * FROM unnest(v_names, v_states) + ; END $body$ , 'Disable the given event triggers, remembering their exact prior state; pair with event_trigger__enable().' @@ -1739,7 +1782,12 @@ BEGIN FROM pg_temp.__object_reference__event_trigger_state ; - -- Dropped before anything is restored, so this DROP fires none of the restored triggers. + /* + * Dropped before anything is restored, so this DROP fires none of the restored triggers. + * The commit check's pending events make DROP TABLE fail, so empty the table and fire them (harmlessly) first. + */ + DELETE FROM pg_temp.__object_reference__event_trigger_state; + SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE; DROP TABLE pg_temp.__object_reference__event_trigger_state; FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP diff --git a/test/build/expected/build.out b/test/build/expected/build.out index 3e6e155..553450f 100644 --- a/test/build/expected/build.out +++ b/test/build/expected/build.out @@ -56,6 +56,7 @@ psql:test/temp_load.not_sql:580: WARNING: I promise you will be sorry if you tr + Loaded OK! diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 734111f..a9c3105 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..35 +1..43 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -29,11 +29,19 @@ ok 26 - disable() zzz_object_reference_capture ok 27 - enable() restores zzz_object_reference_capture ok 28 - only the table created after enable() was captured ok 29 - stop the capture group -ok 30 - disable() rejects an existing sentinel event trigger -ok 31 - the rejected disable() created no state table -ok 32 - disable() succeeds once the stray sentinel is gone -ok 33 - a second disable() without enable() in between is rejected -ok 34 - object_reference schema must not be part of the resolved search_path -ok 35 - _object_reference schema must not be part of the resolved search_path +ok 30 - disable() for the commit check +ok 31 - the commit check fails while enable() has not run +ok 32 - the commit check also fails under session_replication_role = replica +ok 33 - enable() after the failed check +ok 34 - the commit check passes after enable() (and leaves ALL IMMEDIATE in force, which the next disable() must tolerate) +ok 35 - enable() dropped the state table +ok 36 - disable() again in the same transaction +ok 37 - enable() again in the same transaction +ok 38 - disable() rejects an existing sentinel event trigger +ok 39 - the rejected disable() created no state table +ok 40 - disable() succeeds once the stray sentinel is gone +ok 41 - a second disable() without enable() in between is rejected +ok 42 - object_reference schema must not be part of the resolved search_path +ok 43 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 31ae564..91d9cab 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -4,7 +4,8 @@ /* * This file assumes its transaction is never committed (test/load.sql opens - * it, test/finish.sql leaves it open) and that pg_regress runs the test files + * it, test/finish.sql leaves it open; committing would fail anyway, as the + * last test leaves a disable() call without its enable()) and that pg_regress runs the test files * one at a time, so no other session ever sees an event trigger disabled here. * Most tests use their own no-op event triggers, whose names and prior states * they control; the capture test disables the real zzz_object_reference_capture. @@ -34,6 +35,7 @@ SELECT plan( +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() + +8 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and a later disable()/enable() cycle works +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); @@ -222,6 +224,47 @@ SELECT lives_ok( , 'stop the capture group' ); +-- Commit check: SET CONSTRAINTS ... IMMEDIATE runs the same check COMMIT would +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() for the commit check' +); +SELECT throws_ok( + $$SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE$$ + , '55000' + , 'transaction committed without event_trigger__enable() after event_trigger__disable()' + , 'the commit check fails while enable() has not run' +); +SET LOCAL session_replication_role = replica; +SELECT throws_ok( + $$SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE$$ + , '55000' + , 'transaction committed without event_trigger__enable() after event_trigger__disable()' + , 'the commit check also fails under session_replication_role = replica' +); +SET LOCAL session_replication_role = origin; +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() after the failed check' +); +SELECT lives_ok( + $$SET CONSTRAINTS ALL IMMEDIATE$$ + , 'the commit check passes after enable() (and leaves ALL IMMEDIATE in force, which the next disable() must tolerate)' +); +SELECT is( + to_regclass('pg_temp.__object_reference__event_trigger_state') + , NULL + , 'enable() dropped the state table' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() again in the same transaction' +); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() again in the same transaction' +); + -- Must stay last: this sequence leaves a disable() call in effect. CREATE EVENT TRIGGER __object_reference__event_trigger_sentinel ON ddl_command_end EXECUTE FUNCTION event_trigger_disable_test__noop(); SELECT throws_ok( From 27c26ef791aa3ba72e7dbb3b704d85cdea98d298 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 15:00:57 -0600 Subject: [PATCH 26/35] Fix enable() for the update path; make test ordering exercise ENABLE ALWAYS event_trigger__enable() now drops the commit-check trigger's state only when the trigger exists: the update script's hand-written disable creates the state table without it, so enable() failed there. The replica-role test now sets session_replication_role before disable(), so ENABLE ALWAYS on the check trigger is what makes it fail. Document that an active capture group must be named in disable()'s arguments, and correct stale comments. --- sql/object_reference--0.1.0--stable.sql | 38 ++++++++++++++++--------- sql/object_reference.sql | 38 ++++++++++++++++--------- test/expected/event_trigger_disable.out | 28 +++++++++--------- test/sql/event_trigger_disable.sql | 17 ++++++++--- 4 files changed, 76 insertions(+), 45 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index e154ca0..f4a6ee2 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -241,15 +241,18 @@ $body$ * The caller names the triggers, rather than this disabling all of them, so * that no trigger is disabled (or locked) beyond what the caller needs. * - * The change is transactional: other sessions don't see it until commit, and - * it commits or rolls back with the caller's transaction. An error in the same - * transaction needs no cleanup logic. Committing without calling enable() + * Call enable() in the same transaction as disable(): committing without it * raises an error (a deferred constraint trigger on the state table, * _event_trigger__state_check()), so the commit rolls back and the triggers - * stay as they were. A hand-run ALTER EVENT TRIGGER is not covered. - * disable() locks each trigger's catalog row before reading its state - * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER - * of the same trigger until commit. + * stay as they were. The disable therefore never becomes visible to other + * sessions, and an error needs no cleanup logic. A hand-run ALTER EVENT TRIGGER + * is not covered. disable() locks each trigger's catalog row before reading + * its state (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT + * TRIGGER of the same trigger until the transaction ends. + * + * If a capture group is active, include zzz_object_reference_capture in the + * names: disable() creates the check trigger (a CREATE, which capture records + * unless disabled), and capture does not skip it. * * disable() first creates a no-op ddl_command_end event trigger, the sentinel, * which enable() drops after restoring every state. While every sql_drop, @@ -355,10 +358,10 @@ BEGIN END LOOP; /* - * Created only now, with the state rows inserted after it: this CREATE fires any - * still-enabled capture trigger, which the caller is expected to have named above. * Fails the commit while any state row remains, i.e. enable() has not run. - * ENABLE ALWAYS so session_replication_role can't skip it. + * ENABLE ALWAYS so session_replication_role can't skip it. Created after the + * disable loop, and the state rows go in after it: this CREATE fires any + * still-enabled capture trigger, which the caller is expected to have named above. */ CREATE CONSTRAINT TRIGGER __object_reference__event_trigger_state_check AFTER INSERT ON pg_temp.__object_reference__event_trigger_state @@ -395,8 +398,8 @@ BEGIN END IF; /* - * Same sorted order as disable(): if run in a later transaction, these ALTERs - * re-take the row locks, and one order for every caller prevents deadlock. + * Same sorted order as disable(): these ALTERs take the row locks again, and + * one order for every caller prevents deadlock. */ SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") INTO v_names, v_states @@ -406,9 +409,16 @@ BEGIN /* * Dropped before anything is restored, so this DROP fires none of the restored triggers. * The commit check's pending events make DROP TABLE fail, so empty the table and fire them (harmlessly) first. + * The update script's hand-written disable creates the table without the check trigger. */ - DELETE FROM pg_temp.__object_reference__event_trigger_state; - SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE; + IF EXISTS ( + SELECT 1 FROM pg_catalog.pg_trigger + WHERE tgrelid = 'pg_temp.__object_reference__event_trigger_state'::regclass + AND tgname = '__object_reference__event_trigger_state_check' + ) THEN + DELETE FROM pg_temp.__object_reference__event_trigger_state; + SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE; + END IF; DROP TABLE pg_temp.__object_reference__event_trigger_state; FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP diff --git a/sql/object_reference.sql b/sql/object_reference.sql index c7f1a6e..30da83c 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1620,15 +1620,18 @@ $body$ * The caller names the triggers, rather than this disabling all of them, so * that no trigger is disabled (or locked) beyond what the caller needs. * - * The change is transactional: other sessions don't see it until commit, and - * it commits or rolls back with the caller's transaction. An error in the same - * transaction needs no cleanup logic. Committing without calling enable() + * Call enable() in the same transaction as disable(): committing without it * raises an error (a deferred constraint trigger on the state table, * _event_trigger__state_check()), so the commit rolls back and the triggers - * stay as they were. A hand-run ALTER EVENT TRIGGER is not covered. - * disable() locks each trigger's catalog row before reading its state - * (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT TRIGGER - * of the same trigger until commit. + * stay as they were. The disable therefore never becomes visible to other + * sessions, and an error needs no cleanup logic. A hand-run ALTER EVENT TRIGGER + * is not covered. disable() locks each trigger's catalog row before reading + * its state (_event_trigger__lock()), which blocks a concurrent raw ALTER EVENT + * TRIGGER of the same trigger until the transaction ends. + * + * If a capture group is active, include zzz_object_reference_capture in the + * names: disable() creates the check trigger (a CREATE, which capture records + * unless disabled), and capture does not skip it. * * disable() first creates a no-op ddl_command_end event trigger, the sentinel, * which enable() drops after restoring every state. While every sql_drop, @@ -1734,10 +1737,10 @@ BEGIN END LOOP; /* - * Created only now, with the state rows inserted after it: this CREATE fires any - * still-enabled capture trigger, which the caller is expected to have named above. * Fails the commit while any state row remains, i.e. enable() has not run. - * ENABLE ALWAYS so session_replication_role can't skip it. + * ENABLE ALWAYS so session_replication_role can't skip it. Created after the + * disable loop, and the state rows go in after it: this CREATE fires any + * still-enabled capture trigger, which the caller is expected to have named above. */ CREATE CONSTRAINT TRIGGER __object_reference__event_trigger_state_check AFTER INSERT ON pg_temp.__object_reference__event_trigger_state @@ -1774,8 +1777,8 @@ BEGIN END IF; /* - * Same sorted order as disable(): if run in a later transaction, these ALTERs - * re-take the row locks, and one order for every caller prevents deadlock. + * Same sorted order as disable(): these ALTERs take the row locks again, and + * one order for every caller prevents deadlock. */ SELECT array_agg(evtname ORDER BY evtname COLLATE "C"), array_agg(evtenabled ORDER BY evtname COLLATE "C") INTO v_names, v_states @@ -1785,9 +1788,16 @@ BEGIN /* * Dropped before anything is restored, so this DROP fires none of the restored triggers. * The commit check's pending events make DROP TABLE fail, so empty the table and fire them (harmlessly) first. + * The update script's hand-written disable creates the table without the check trigger. */ - DELETE FROM pg_temp.__object_reference__event_trigger_state; - SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE; + IF EXISTS ( + SELECT 1 FROM pg_catalog.pg_trigger + WHERE tgrelid = 'pg_temp.__object_reference__event_trigger_state'::regclass + AND tgname = '__object_reference__event_trigger_state_check' + ) THEN + DELETE FROM pg_temp.__object_reference__event_trigger_state; + SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE; + END IF; DROP TABLE pg_temp.__object_reference__event_trigger_state; FOR i IN 1..coalesce(array_length(v_names, 1), 0) LOOP diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index a9c3105..92b94a5 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..43 +1..45 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -31,17 +31,19 @@ ok 28 - only the table created after enable() was captured ok 29 - stop the capture group ok 30 - disable() for the commit check ok 31 - the commit check fails while enable() has not run -ok 32 - the commit check also fails under session_replication_role = replica -ok 33 - enable() after the failed check -ok 34 - the commit check passes after enable() (and leaves ALL IMMEDIATE in force, which the next disable() must tolerate) -ok 35 - enable() dropped the state table -ok 36 - disable() again in the same transaction -ok 37 - enable() again in the same transaction -ok 38 - disable() rejects an existing sentinel event trigger -ok 39 - the rejected disable() created no state table -ok 40 - disable() succeeds once the stray sentinel is gone -ok 41 - a second disable() without enable() in between is rejected -ok 42 - object_reference schema must not be part of the resolved search_path -ok 43 - _object_reference schema must not be part of the resolved search_path +ok 32 - enable() after the failed check +ok 33 - disable() under session_replication_role = replica +ok 34 - the commit check also fails under session_replication_role = replica +ok 35 - enable() after the replica check +ok 36 - the commit check passes after enable() (and leaves ALL IMMEDIATE in force, which the next disable() must tolerate) +ok 37 - enable() dropped the state table +ok 38 - disable() again in the same transaction +ok 39 - enable() again in the same transaction +ok 40 - disable() rejects an existing sentinel event trigger +ok 41 - the rejected disable() created no state table +ok 42 - disable() succeeds once the stray sentinel is gone +ok 43 - a second disable() without enable() in between is rejected +ok 44 - object_reference schema must not be part of the resolved search_path +ok 45 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 91d9cab..063963c 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -4,8 +4,7 @@ /* * This file assumes its transaction is never committed (test/load.sql opens - * it, test/finish.sql leaves it open; committing would fail anyway, as the - * last test leaves a disable() call without its enable()) and that pg_regress runs the test files + * it, test/finish.sql leaves it open) and that pg_regress runs the test files * one at a time, so no other session ever sees an event trigger disabled here. * Most tests use their own no-op event triggers, whose names and prior states * they control; the capture test disables the real zzz_object_reference_capture. @@ -35,7 +34,7 @@ SELECT plan( +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() - +8 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and a later disable()/enable() cycle works + +10 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and a later disable()/enable() cycle works +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); @@ -235,7 +234,17 @@ SELECT throws_ok( , 'transaction committed without event_trigger__enable() after event_trigger__disable()' , 'the commit check fails while enable() has not run' ); +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__enable()$$ + , 'enable() after the failed check' +); + +-- replica is set before disable(): only ENABLE ALWAYS on the check trigger lets its queued event fire SET LOCAL session_replication_role = replica; +SELECT lives_ok( + $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ + , 'disable() under session_replication_role = replica' +); SELECT throws_ok( $$SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE$$ , '55000' @@ -245,7 +254,7 @@ SELECT throws_ok( SET LOCAL session_replication_role = origin; SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() after the failed check' + , 'enable() after the replica check' ); SELECT lives_ok( $$SET CONSTRAINTS ALL IMMEDIATE$$ From 57aa966aa49a9c113e917f5bf6acba2b26daa2fb Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 15:05:49 -0600 Subject: [PATCH 27/35] Tighten check-trigger comment; split and extend commit-check tests Test 36 now claims only that the check passes after enable(); the tolerance of a preceding SET CONSTRAINTS ALL IMMEDIATE moves to the description of the next disable(). A new test records that SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises. --- sql/object_reference--0.1.0--stable.sql | 4 +--- sql/object_reference.sql | 4 +--- test/expected/event_trigger_disable.out | 21 +++++++++++---------- test/sql/event_trigger_disable.sql | 12 +++++++++--- 4 files changed, 22 insertions(+), 19 deletions(-) diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index f4a6ee2..d17a275 100644 --- a/sql/object_reference--0.1.0--stable.sql +++ b/sql/object_reference--0.1.0--stable.sql @@ -359,9 +359,7 @@ BEGIN /* * Fails the commit while any state row remains, i.e. enable() has not run. - * ENABLE ALWAYS so session_replication_role can't skip it. Created after the - * disable loop, and the state rows go in after it: this CREATE fires any - * still-enabled capture trigger, which the caller is expected to have named above. + * ENABLE ALWAYS so session_replication_role can't skip it. */ CREATE CONSTRAINT TRIGGER __object_reference__event_trigger_state_check AFTER INSERT ON pg_temp.__object_reference__event_trigger_state diff --git a/sql/object_reference.sql b/sql/object_reference.sql index 30da83c..3131a55 100644 --- a/sql/object_reference.sql +++ b/sql/object_reference.sql @@ -1738,9 +1738,7 @@ BEGIN /* * Fails the commit while any state row remains, i.e. enable() has not run. - * ENABLE ALWAYS so session_replication_role can't skip it. Created after the - * disable loop, and the state rows go in after it: this CREATE fires any - * still-enabled capture trigger, which the caller is expected to have named above. + * ENABLE ALWAYS so session_replication_role can't skip it. */ CREATE CONSTRAINT TRIGGER __object_reference__event_trigger_state_check AFTER INSERT ON pg_temp.__object_reference__event_trigger_state diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 92b94a5..1cb8193 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..45 +1..46 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -35,15 +35,16 @@ ok 32 - enable() after the failed check ok 33 - disable() under session_replication_role = replica ok 34 - the commit check also fails under session_replication_role = replica ok 35 - enable() after the replica check -ok 36 - the commit check passes after enable() (and leaves ALL IMMEDIATE in force, which the next disable() must tolerate) +ok 36 - the commit check passes after enable() ok 37 - enable() dropped the state table -ok 38 - disable() again in the same transaction -ok 39 - enable() again in the same transaction -ok 40 - disable() rejects an existing sentinel event trigger -ok 41 - the rejected disable() created no state table -ok 42 - disable() succeeds once the stray sentinel is gone -ok 43 - a second disable() without enable() in between is rejected -ok 44 - object_reference schema must not be part of the resolved search_path -ok 45 - _object_reference schema must not be part of the resolved search_path +ok 38 - disable() again after SET CONSTRAINTS ALL IMMEDIATE (the check trigger must start out deferred) +ok 39 - SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises +ok 40 - enable() again in the same transaction +ok 41 - disable() rejects an existing sentinel event trigger +ok 42 - the rejected disable() created no state table +ok 43 - disable() succeeds once the stray sentinel is gone +ok 44 - a second disable() without enable() in between is rejected +ok 45 - object_reference schema must not be part of the resolved search_path +ok 46 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 063963c..d343e57 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -34,7 +34,7 @@ SELECT plan( +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() - +10 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and a later disable()/enable() cycle works + +11 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and a later disable()/enable() cycle works +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); @@ -258,7 +258,7 @@ SELECT lives_ok( ); SELECT lives_ok( $$SET CONSTRAINTS ALL IMMEDIATE$$ - , 'the commit check passes after enable() (and leaves ALL IMMEDIATE in force, which the next disable() must tolerate)' + , 'the commit check passes after enable()' ); SELECT is( to_regclass('pg_temp.__object_reference__event_trigger_state') @@ -267,7 +267,13 @@ SELECT is( ); SELECT lives_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() again in the same transaction' + , 'disable() again after SET CONSTRAINTS ALL IMMEDIATE (the check trigger must start out deferred)' +); +SELECT throws_ok( + $$SET CONSTRAINTS ALL IMMEDIATE$$ + , '55000' + , 'transaction committed without event_trigger__enable() after event_trigger__disable()' + , 'SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises' ); SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ From b37123eda6d9c5d00b990bd2a0c9e16e9132f2da Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 17:16:34 -0600 Subject: [PATCH 28/35] Use pgTAP assertions for catalog lookups; reorder commit-check tests to reuse state Replace to_regprocedure/to_regclass checks with has_function/hasnt_function/ hasnt_table, array_agg comparisons with results_eq, and run the SET CONSTRAINTS ALL IMMEDIATE failure case before enable() instead of re-running disable(). --- test/expected/event_trigger_disable.out | 24 ++++----- test/sql/event_trigger_disable.sql | 71 +++++++++++-------------- 2 files changed, 43 insertions(+), 52 deletions(-) diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 1cb8193..dea5956 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..46 +1..44 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -34,17 +34,15 @@ ok 31 - the commit check fails while enable() has not run ok 32 - enable() after the failed check ok 33 - disable() under session_replication_role = replica ok 34 - the commit check also fails under session_replication_role = replica -ok 35 - enable() after the replica check -ok 36 - the commit check passes after enable() -ok 37 - enable() dropped the state table -ok 38 - disable() again after SET CONSTRAINTS ALL IMMEDIATE (the check trigger must start out deferred) -ok 39 - SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises -ok 40 - enable() again in the same transaction -ok 41 - disable() rejects an existing sentinel event trigger -ok 42 - the rejected disable() created no state table -ok 43 - disable() succeeds once the stray sentinel is gone -ok 44 - a second disable() without enable() in between is rejected -ok 45 - object_reference schema must not be part of the resolved search_path -ok 46 - _object_reference schema must not be part of the resolved search_path +ok 35 - SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises +ok 36 - enable() after the failed checks +ok 37 - the commit check passes after enable() +ok 38 - enable() dropped the state table +ok 39 - disable() rejects an existing sentinel event trigger +ok 40 - the rejected disable() created no state table +ok 41 - disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred) +ok 42 - a second disable() without enable() in between is rejected +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! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index d343e57..369477c 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -34,7 +34,7 @@ SELECT plan( +4 -- names passed unsorted are processed in "C" order +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() - +11 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and a later disable()/enable() cycle works + +9 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and SET CONSTRAINTS ALL IMMEDIATE fails between disable() and enable() and passes after +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); @@ -63,8 +63,10 @@ SELECT is( , 'ddl_command_end O' , 'the sentinel event trigger is enabled on ddl_command_end while a call is in effect' ); -SELECT ok( - to_regprocedure('pg_temp.__object_reference__event_trigger_sentinel()') IS NOT NULL +SELECT has_function( + (SELECT nspname FROM pg_catalog.pg_namespace WHERE oid = pg_catalog.pg_my_temp_schema()) + , '__object_reference__event_trigger_sentinel' + , '{}'::name[] , 'the sentinel function exists while a call is in effect' ); SELECT lives_ok( @@ -85,8 +87,10 @@ SELECT ok( NOT EXISTS (SELECT 1 FROM pg_catalog.pg_event_trigger WHERE evtname = '__object_reference__event_trigger_sentinel') , 'enable() dropped the sentinel event trigger' ); -SELECT ok( - to_regprocedure('pg_temp.__object_reference__event_trigger_sentinel()') IS NULL +SELECT hasnt_function( + (SELECT nspname FROM pg_catalog.pg_namespace WHERE oid = pg_catalog.pg_my_temp_schema()) + , '__object_reference__event_trigger_sentinel' + , '{}'::name[] , 'enable() dropped the sentinel function' ); @@ -143,18 +147,18 @@ SELECT lives_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__z,event_trigger_disable_test__a,"event_trigger_disable_test__B"}')$$ , 'disable() with unsorted names' ); -SELECT is( - (SELECT array_agg(evtname ORDER BY ctid) FROM pg_temp.__object_reference__event_trigger_state) - , '{event_trigger_disable_test__B,event_trigger_disable_test__a,event_trigger_disable_test__z}'::name[] +SELECT results_eq( + $$SELECT evtname FROM pg_temp.__object_reference__event_trigger_state ORDER BY ctid$$ + , $$VALUES ('event_trigger_disable_test__B'::name), ('event_trigger_disable_test__a'), ('event_trigger_disable_test__z')$$ , 'names were processed in "C" collation order, not input order' ); SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ , 'enable() after unsorted disable()' ); -SELECT is( - (SELECT array_agg(evtenabled ORDER BY evtname COLLATE "C") FROM pg_catalog.pg_event_trigger WHERE evtname LIKE 'event\_trigger\_disable\_test\_\_%' AND evtname IN ('event_trigger_disable_test__B', 'event_trigger_disable_test__a', 'event_trigger_disable_test__z')) - , '{O,O,O}'::"char"[] +SELECT results_eq( + $$SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname IN ('event_trigger_disable_test__B', 'event_trigger_disable_test__a', 'event_trigger_disable_test__z')$$ + , $$VALUES ('O'::"char"), ('O'), ('O')$$ , 'all three restored' ); @@ -162,8 +166,7 @@ SELECT is( * A role without UPDATE on the catalog gets the trigger's state read * unlocked, with a WARNING naming the trigger, instead of an error. The DO * block runs as that role, and first proves the role really cannot FOR UPDATE - * the catalog. The WARNING is suppressed because whether it is raised depends - * on the environment's catalog privileges. + * the catalog. The WARNING is suppressed to keep it out of the expected output. */ SELECT current_setting('client_min_messages') AS saved_client_min_messages \gset SET client_min_messages = error; @@ -210,12 +213,12 @@ SELECT lives_ok( , 'enable() restores zzz_object_reference_capture' ); CREATE TABLE public.event_trigger_disable_test__after_enable(); -SELECT is( - (SELECT array_agg(array_to_string(o.object_names, '.') ORDER BY o.object_names) +SELECT results_eq( + $$SELECT array_to_string(o.object_names, '.') FROM _object_reference.object_group__object m JOIN _object_reference.object o USING (object_id) - WHERE m.object_group_id = (object_reference.object_group__get('event_trigger_disable_test_group')).object_group_id) - , '{public.event_trigger_disable_test__after_enable}'::text[] + WHERE m.object_group_id = (object_reference.object_group__get('event_trigger_disable_test_group')).object_group_id$$ + , $$VALUES ('public.event_trigger_disable_test__after_enable')$$ , 'only the table created after enable() was captured' ); SELECT lives_ok( @@ -252,23 +255,6 @@ SELECT throws_ok( , 'the commit check also fails under session_replication_role = replica' ); SET LOCAL session_replication_role = origin; -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() after the replica check' -); -SELECT lives_ok( - $$SET CONSTRAINTS ALL IMMEDIATE$$ - , 'the commit check passes after enable()' -); -SELECT is( - to_regclass('pg_temp.__object_reference__event_trigger_state') - , NULL - , 'enable() dropped the state table' -); -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() again after SET CONSTRAINTS ALL IMMEDIATE (the check trigger must start out deferred)' -); SELECT throws_ok( $$SET CONSTRAINTS ALL IMMEDIATE$$ , '55000' @@ -277,7 +263,15 @@ SELECT throws_ok( ); SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() again in the same transaction' + , 'enable() after the failed checks' +); +SELECT lives_ok( + $$SET CONSTRAINTS ALL IMMEDIATE$$ + , 'the commit check passes after enable()' +); +SELECT hasnt_table( + '__object_reference__event_trigger_state' + , 'enable() dropped the state table' ); -- Must stay last: this sequence leaves a disable() call in effect. @@ -288,15 +282,14 @@ SELECT throws_ok( , 'event_trigger__disable() from another session is still in effect' , 'disable() rejects an existing sentinel event trigger' ); -SELECT is( - to_regclass('pg_temp.__object_reference__event_trigger_state') - , NULL +SELECT hasnt_table( + '__object_reference__event_trigger_state' , 'the rejected disable() created no state table' ); DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; SELECT lives_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() succeeds once the stray sentinel is gone' + , 'disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred)' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ From 8668c0add655cbdbce979c382999a37090cf191a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 17:26:21 -0600 Subject: [PATCH 29/35] Remove the no-advisory-lock test from event_trigger_disable _event_trigger__lock() no longer takes an advisory lock, so the test only pinned the absence of removed behavior. --- test/expected/event_trigger_disable.out | 43 ++++++++++++------------- test/sql/event_trigger_disable.sql | 7 +--- 2 files changed, 22 insertions(+), 28 deletions(-) diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index dea5956..3f4dc04 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..44 +1..43 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -23,26 +23,25 @@ ok 20 - enable() after unsorted disable() ok 21 - all three restored ok 22 - helper does not fail for a role that cannot row-lock the catalog ok 23 - helper still returns the trigger's evtenabled -ok 24 - helper took no advisory lock -ok 25 - start a capture group -ok 26 - disable() zzz_object_reference_capture -ok 27 - enable() restores zzz_object_reference_capture -ok 28 - only the table created after enable() was captured -ok 29 - stop the capture group -ok 30 - disable() for the commit check -ok 31 - the commit check fails while enable() has not run -ok 32 - enable() after the failed check -ok 33 - disable() under session_replication_role = replica -ok 34 - the commit check also fails under session_replication_role = replica -ok 35 - SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises -ok 36 - enable() after the failed checks -ok 37 - the commit check passes after enable() -ok 38 - enable() dropped the state table -ok 39 - disable() rejects an existing sentinel event trigger -ok 40 - the rejected disable() created no state table -ok 41 - disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred) -ok 42 - a second disable() without enable() in between is rejected -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 +ok 24 - start a capture group +ok 25 - disable() zzz_object_reference_capture +ok 26 - enable() restores zzz_object_reference_capture +ok 27 - only the table created after enable() was captured +ok 28 - stop the capture group +ok 29 - disable() for the commit check +ok 30 - the commit check fails while enable() has not run +ok 31 - enable() after the failed check +ok 32 - disable() under session_replication_role = replica +ok 33 - the commit check also fails under session_replication_role = replica +ok 34 - SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises +ok 35 - enable() after the failed checks +ok 36 - the commit check passes after enable() +ok 37 - enable() dropped the state table +ok 38 - disable() rejects an existing sentinel event trigger +ok 39 - the rejected disable() created no state table +ok 40 - disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred) +ok 41 - a second disable() without enable() in between is rejected +ok 42 - object_reference schema must not be part of the resolved search_path +ok 43 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 369477c..52e87e1 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -32,7 +32,7 @@ SELECT plan( +1 -- disable() rejects a duplicate name in its own argument list +3 -- disable() rejects a NULL array, an empty array and a NULL element +4 -- names passed unsorted are processed in "C" order - +3 -- _event_trigger__lock() reads the state unlocked, with no advisory lock, for a role that cannot row-lock the catalog + +2 -- _event_trigger__lock() reads the state unlocked for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() +9 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and SET CONSTRAINTS ALL IMMEDIATE fails between disable() and enable() and passes after +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected @@ -192,11 +192,6 @@ SELECT is( , 'O' , 'helper still returns the trigger''s evtenabled' ); -SELECT is( - (SELECT count(*)::int FROM pg_catalog.pg_locks WHERE locktype = 'advisory' AND pid = pg_backend_pid()) - , 0 - , 'helper took no advisory lock' -); -- Disabling zzz_object_reference_capture stops capture until enable() SELECT lives_ok( From 3356fa97fd603bb008029c6f06eda7f74a5186b9 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 17:29:01 -0600 Subject: [PATCH 30/35] Reuse existing disable()/enable() pairs for the commit-check tests The origin-role commit-check assertions now run inside the unsorted-names pair instead of their own, leaving one dedicated pair for the session_replication_role = replica case (the check trigger's event is queued when disable() inserts, so replica needs its own disable()). Compare the restored states with bag_eq against a VALUES list. --- test/expected/event_trigger_disable.out | 48 ++++++++++++------------- test/sql/event_trigger_disable.sql | 46 ++++++++++-------------- 2 files changed, 42 insertions(+), 52 deletions(-) diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index 3f4dc04..ed04603 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..43 +1..41 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -19,29 +19,27 @@ ok 16 - disable() rejects an empty array ok 17 - disable() rejects a NULL element ok 18 - disable() with unsorted names ok 19 - names were processed in "C" collation order, not input order -ok 20 - enable() after unsorted disable() -ok 21 - all three restored -ok 22 - helper does not fail for a role that cannot row-lock the catalog -ok 23 - helper still returns the trigger's evtenabled -ok 24 - start a capture group -ok 25 - disable() zzz_object_reference_capture -ok 26 - enable() restores zzz_object_reference_capture -ok 27 - only the table created after enable() was captured -ok 28 - stop the capture group -ok 29 - disable() for the commit check -ok 30 - the commit check fails while enable() has not run -ok 31 - enable() after the failed check -ok 32 - disable() under session_replication_role = replica -ok 33 - the commit check also fails under session_replication_role = replica -ok 34 - SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises -ok 35 - enable() after the failed checks -ok 36 - the commit check passes after enable() -ok 37 - enable() dropped the state table -ok 38 - disable() rejects an existing sentinel event trigger -ok 39 - the rejected disable() created no state table -ok 40 - disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred) -ok 41 - a second disable() without enable() in between is rejected -ok 42 - object_reference schema must not be part of the resolved search_path -ok 43 - _object_reference schema must not be part of the resolved search_path +ok 20 - the commit check fails while enable() has not run +ok 21 - SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises +ok 22 - enable() after unsorted disable() +ok 23 - all three restored +ok 24 - helper does not fail for a role that cannot row-lock the catalog +ok 25 - helper still returns the trigger's evtenabled +ok 26 - start a capture group +ok 27 - disable() zzz_object_reference_capture +ok 28 - enable() restores zzz_object_reference_capture +ok 29 - only the table created after enable() was captured +ok 30 - stop the capture group +ok 31 - disable() under session_replication_role = replica +ok 32 - the commit check also fails under session_replication_role = replica +ok 33 - enable() after the replica-role disable() +ok 34 - the commit check passes after enable() +ok 35 - enable() dropped the state table +ok 36 - disable() rejects an existing sentinel event trigger +ok 37 - the rejected disable() created no state table +ok 38 - disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred) +ok 39 - a second disable() without enable() in between is rejected +ok 40 - object_reference schema must not be part of the resolved search_path +ok 41 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 52e87e1..795e625 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -32,9 +32,10 @@ SELECT plan( +1 -- disable() rejects a duplicate name in its own argument list +3 -- disable() rejects a NULL array, an empty array and a NULL element +4 -- names passed unsorted are processed in "C" order + +2 -- commit check, origin role: the check fails while enable() has not run, also via SET CONSTRAINTS ALL IMMEDIATE +2 -- _event_trigger__lock() reads the state unlocked for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() - +9 -- commit check: a disable() without enable() fails at commit (also under replica), a normal pair does not, and SET CONSTRAINTS ALL IMMEDIATE fails between disable() and enable() and passes after + +5 -- commit check, replica role: the check fails while enable() has not run; after enable() it passes and the state table is gone +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); @@ -142,7 +143,7 @@ SELECT throws_ok( , 'disable() rejects a NULL element' ); --- Unsorted input is processed in "C" order +-- Unsorted input is processed in "C" order; the commit check (origin) piggybacks on this disable()/enable() pair SELECT lives_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__z,event_trigger_disable_test__a,"event_trigger_disable_test__B"}')$$ , 'disable() with unsorted names' @@ -152,11 +153,24 @@ SELECT results_eq( , $$VALUES ('event_trigger_disable_test__B'::name), ('event_trigger_disable_test__a'), ('event_trigger_disable_test__z')$$ , 'names were processed in "C" collation order, not input order' ); +-- SET CONSTRAINTS ... IMMEDIATE runs the same check COMMIT would +SELECT throws_ok( + $$SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE$$ + , '55000' + , 'transaction committed without event_trigger__enable() after event_trigger__disable()' + , 'the commit check fails while enable() has not run' +); +SELECT throws_ok( + $$SET CONSTRAINTS ALL IMMEDIATE$$ + , '55000' + , 'transaction committed without event_trigger__enable() after event_trigger__disable()' + , 'SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises' +); SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ , 'enable() after unsorted disable()' ); -SELECT results_eq( +SELECT bag_eq( $$SELECT evtenabled FROM pg_catalog.pg_event_trigger WHERE evtname IN ('event_trigger_disable_test__B', 'event_trigger_disable_test__a', 'event_trigger_disable_test__z')$$ , $$VALUES ('O'::"char"), ('O'), ('O')$$ , 'all three restored' @@ -221,23 +235,7 @@ SELECT lives_ok( , 'stop the capture group' ); --- Commit check: SET CONSTRAINTS ... IMMEDIATE runs the same check COMMIT would -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() for the commit check' -); -SELECT throws_ok( - $$SET CONSTRAINTS pg_temp.__object_reference__event_trigger_state_check IMMEDIATE$$ - , '55000' - , 'transaction committed without event_trigger__enable() after event_trigger__disable()' - , 'the commit check fails while enable() has not run' -); -SELECT lives_ok( - $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() after the failed check' -); - --- replica is set before disable(): only ENABLE ALWAYS on the check trigger lets its queued event fire +-- Commit check, continued: replica is set before disable(), so only ENABLE ALWAYS on the check trigger lets its event be queued SET LOCAL session_replication_role = replica; SELECT lives_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ @@ -250,15 +248,9 @@ SELECT throws_ok( , 'the commit check also fails under session_replication_role = replica' ); SET LOCAL session_replication_role = origin; -SELECT throws_ok( - $$SET CONSTRAINTS ALL IMMEDIATE$$ - , '55000' - , 'transaction committed without event_trigger__enable() after event_trigger__disable()' - , 'SET CONSTRAINTS ALL IMMEDIATE between disable() and enable() raises' -); SELECT lives_ok( $$SELECT _object_reference.event_trigger__enable()$$ - , 'enable() after the failed checks' + , 'enable() after the replica-role disable()' ); SELECT lives_ok( $$SET CONSTRAINTS ALL IMMEDIATE$$ From 49b85cc57bf13eb8b91cb550d7a388857d267fd7 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 17:29:51 -0600 Subject: [PATCH 31/35] Drop the vacuous rejected-disable() state-table test; one claim per description throws_ok rolls the failed disable() back in a subtransaction, so a state table created before the sentinel check would vanish before hasnt_table looked; the assertion could not fail. --- test/expected/event_trigger_disable.out | 11 +++++------ test/sql/event_trigger_disable.sql | 9 +++------ 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/test/expected/event_trigger_disable.out b/test/expected/event_trigger_disable.out index ed04603..00953d5 100644 --- a/test/expected/event_trigger_disable.out +++ b/test/expected/event_trigger_disable.out @@ -1,5 +1,5 @@ \set ECHO none -1..41 +1..40 ok 1 - manually disable test trigger b ahead of time ok 2 - disable() both test triggers ok 3 - test trigger a is disabled while a call is in effect @@ -36,10 +36,9 @@ ok 33 - enable() after the replica-role disable() ok 34 - the commit check passes after enable() ok 35 - enable() dropped the state table ok 36 - disable() rejects an existing sentinel event trigger -ok 37 - the rejected disable() created no state table -ok 38 - disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred) -ok 39 - a second disable() without enable() in between is rejected -ok 40 - object_reference schema must not be part of the resolved search_path -ok 41 - _object_reference schema must not be part of the resolved search_path +ok 37 - disable() succeeds once the stray sentinel is gone +ok 38 - a second disable() without enable() in between is rejected +ok 39 - object_reference schema must not be part of the resolved search_path +ok 40 - _object_reference schema must not be part of the resolved search_path # TRANSACTION INTENTIONALLY LEFT OPEN! # WARNING: this test assumes it runs in a transaction that is never committed; if you ran it by hand in psql, ROLLBACK now. diff --git a/test/sql/event_trigger_disable.sql b/test/sql/event_trigger_disable.sql index 795e625..7ba206b 100644 --- a/test/sql/event_trigger_disable.sql +++ b/test/sql/event_trigger_disable.sql @@ -36,7 +36,7 @@ SELECT plan( +2 -- _event_trigger__lock() reads the state unlocked for a role that cannot row-lock the catalog +5 -- disabling zzz_object_reference_capture stops capture until enable() +5 -- commit check, replica role: the check fails while enable() has not run; after enable() it passes and the state table is gone - +4 -- closing sequence: a stray sentinel event trigger is rejected, no state table is created, disable() succeeds once the stray is dropped, then a nested disable() is rejected + +3 -- closing sequence: a stray sentinel event trigger is rejected, disable() succeeds once the stray is dropped, then a nested disable() is rejected +2 -- schema-qualification (search_path) ); @@ -269,14 +269,11 @@ SELECT throws_ok( , 'event_trigger__disable() from another session is still in effect' , 'disable() rejects an existing sentinel event trigger' ); -SELECT hasnt_table( - '__object_reference__event_trigger_state' - , 'the rejected disable() created no state table' -); DROP EVENT TRIGGER __object_reference__event_trigger_sentinel; +-- SET CONSTRAINTS ALL IMMEDIATE has already run in this transaction: the new check trigger must still start deferred. SELECT lives_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ - , 'disable() succeeds once the stray sentinel is gone (also after SET CONSTRAINTS ALL IMMEDIATE: the check trigger starts out deferred)' + , 'disable() succeeds once the stray sentinel is gone' ); SELECT throws_ok( $$SELECT _object_reference.event_trigger__disable('{event_trigger_disable_test__a}')$$ From 093dba1d4209e549d48e62cb319b33aa9c522b66 Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 17:53:20 -0600 Subject: [PATCH 32/35] CI: run the update path via make verify-results TEST_LOAD_SOURCE=update Add a CI step that runs the suite against a database installed at TEST_UPDATE_FROM and then ALTER EXTENSION UPDATEd. Describe test-update as a human shorthand for the TEST_LOAD_SOURCE form. --- .github/workflows/ci.yml | 5 +++++ Makefile | 3 ++- test/build/update.sql | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 064462c..ba919b3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -143,6 +143,11 @@ jobs: # to yes, PGXNTOOL_VERIFY_RESULTS_MODE to pgtap, matching this # suite's pgTAP-based tests) and exits non-zero on failure. run: make verify-results + - name: Test the update path on PostgreSQL ${{ matrix.pg }} + # Runs the same suite against a database installed at TEST_UPDATE_FROM + # and then ALTER EXTENSION UPDATEd. Uses verify-results rather than + # `make test-update`: that wraps `make test`, which never fails the step. + run: make verify-results TEST_LOAD_SOURCE=update # A single stable check name for use as a required status check in branch # protection rules. Matrix jobs produce check names like "🐘 PostgreSQL 14" diff --git a/Makefile b/Makefile index 73316b3..628b514 100644 --- a/Makefile +++ b/Makefile @@ -50,7 +50,8 @@ 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`. +# Human shorthand for `make test TEST_LOAD_SOURCE=update`; CI and scripts use +# the TEST_LOAD_SOURCE form directly. # 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 diff --git a/test/build/update.sql b/test/build/update.sql index 9bac00a..30735b4 100644 --- a/test/build/update.sql +++ b/test/build/update.sql @@ -1,6 +1,6 @@ \set ECHO none /* - * Update-mode scenarios (make test-update): install the old version, then + * Update-mode scenarios (make test TEST_LOAD_SOURCE=update): install the old version, then * ALTER EXTENSION UPDATE, in rolled-back transactions, so nothing persists. * Scenarios 1-3 start a capture group; each asserts the event trigger state it * starts the update from, updates, and checks the result: From afa2fc61e63bc2beaa8b0598c6092211252cc39b Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 17:58:42 -0600 Subject: [PATCH 33/35] Make test--update the preferred update-path target; keep test-update as a synonym --- .github/workflows/ci.yml | 4 ++-- Makefile | 9 +++++---- test/build/update.sql | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ba919b3..0657476 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -145,8 +145,8 @@ jobs: run: make verify-results - name: Test the update path on PostgreSQL ${{ matrix.pg }} # Runs the same suite against a database installed at TEST_UPDATE_FROM - # and then ALTER EXTENSION UPDATEd. Uses verify-results rather than - # `make test-update`: that wraps `make test`, which never fails the step. + # and then ALTER EXTENSION UPDATEd. + # Not `make test--update`: that wraps `make test`, which never fails the step. run: make verify-results TEST_LOAD_SOURCE=update # A single stable check name for use as a required status check in branch diff --git a/Makefile b/Makefile index 628b514..89d2f85 100644 --- a/Makefile +++ b/Makefile @@ -50,12 +50,13 @@ 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) -# Human shorthand for `make test TEST_LOAD_SOURCE=update`; CI and scripts use -# the TEST_LOAD_SOURCE form directly. +# `test--update` is the preferred name (`--` names a flavor of +# that target); `test-update` is a synonym for anyone who types a single dash. +# CI and scripts may use the underlying TEST_LOAD_SOURCE variable directly. # 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: +.PHONY: test--update test-update +test--update test-update: $(MAKE) test TEST_LOAD_SOURCE=update include pgxntool/base.mk diff --git a/test/build/update.sql b/test/build/update.sql index 30735b4..8b0bbcb 100644 --- a/test/build/update.sql +++ b/test/build/update.sql @@ -1,6 +1,6 @@ \set ECHO none /* - * Update-mode scenarios (make test TEST_LOAD_SOURCE=update): install the old version, then + * Update-mode scenarios (make test--update): install the old version, then * ALTER EXTENSION UPDATE, in rolled-back transactions, so nothing persists. * Scenarios 1-3 start a capture group; each asserts the event trigger state it * starts the update from, updates, and checks the result: From bd195e89c6a93c431bdd50197645f7a594415bbc Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 18:05:24 -0600 Subject: [PATCH 34/35] CI: run the update path via make test--update make test exits non-zero on a regression with the vendored pgxntool, so the update-path step needs no CI-specific form. Drop the comments claiming otherwise. --- .github/workflows/ci.yml | 12 +++--------- Makefile | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0657476..d9b3be0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,19 +135,13 @@ jobs: - 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, - # so a regression exits 0 and only prints the diff. verify-results is - # the real gate -- it runs the same TEST_DEPS chain, then actually - # inspects the pgTAP results (PGXNTOOL_ENABLE_VERIFY_RESULTS defaults - # to yes, PGXNTOOL_VERIFY_RESULTS_MODE to pgtap, matching this - # suite's pgTAP-based tests) and exits non-zero on failure. + # verify-results is the stricter documented check: it runs the same + # TEST_DEPS chain, then parses the pgTAP results. run: make verify-results - name: Test the update path on PostgreSQL ${{ matrix.pg }} # Runs the same suite against a database installed at TEST_UPDATE_FROM # and then ALTER EXTENSION UPDATEd. - # Not `make test--update`: that wraps `make test`, which never fails the step. - run: make verify-results TEST_LOAD_SOURCE=update + run: make test--update # A single stable check name for use as a required status check in branch # protection rules. Matrix jobs produce check names like "🐘 PostgreSQL 14" diff --git a/Makefile b/Makefile index 89d2f85..e6b0aa2 100644 --- a/Makefile +++ b/Makefile @@ -52,7 +52,7 @@ export PGOPTIONS := $(PGOPTIONS) -c object_reference.test_load_mode=$(TEST_LOAD_ # `test--update` is the preferred name (`--` names a flavor of # that target); `test-update` is a synonym for anyone who types a single dash. -# CI and scripts may use the underlying TEST_LOAD_SOURCE variable directly. +# Scripts may use the underlying TEST_LOAD_SOURCE variable directly. # 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 From 11680f6b9cb538c01a1a8ba19b699261dd33ad9a Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Tue, 3 Nov 2026 18:18:00 -0600 Subject: [PATCH 35/35] CI: reword the verify-results step comment --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9b3be0..88b5a9c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -135,8 +135,8 @@ jobs: - name: Install rsync run: apt-get install -y rsync - name: Test on PostgreSQL ${{ matrix.pg }} - # verify-results is the stricter documented check: it runs the same - # TEST_DEPS chain, then parses the pgTAP results. + # Fresh-mode run: verify-results runs the test chain, then parses the + # pgTAP results, which `make test` alone does not. run: make verify-results - name: Test the update path on PostgreSQL ${{ matrix.pg }} # Runs the same suite against a database installed at TEST_UPDATE_FROM