From 039241dba8a44920580f2c5c752c18a12c5398fb Mon Sep 17 00:00:00 2001 From: jnasbyupgrade Date: Fri, 30 Oct 2026 01:48:53 -0500 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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[]