diff --git a/sql/object_reference--0.1.0--stable.sql b/sql/object_reference--0.1.0--stable.sql index b965953..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 @@ -105,32 +128,379 @@ 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. + * 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_fix_identity/_etg_capture: gain a self-recognition guard so they skip + * work while this extension's own event_trigger__disable() call is in + * effect for this session (checked via to_regclass() on the temp table + * event_trigger__disable() creates below) -- i.e. this extension's own + * install/update script is doing delicate internal restructuring right + * now. Installed here, ahead of the structural section below, specifically + * so the guard is already active by the time event_trigger__disable() is + * first called a few statements down: recreating them any later would + * leave the OLD (0.1.0), unguarded bodies live for that whole window -- + * which mattered in practice for _etg_capture, which would otherwise try + * to call _object_reference._object_v__for_update() (the FUNCTION) for any + * CREATE-tagged command in this script if a capture happened to be active, + * including a moment where that function has been dropped and not yet + * recreated, which would fail outright. Same signatures as 0.1.0, so a + * plain CREATE OR REPLACE (via create_function) is enough -- no DROP + * needed. + */ +SELECT __object_reference.create_function( + '_object_reference._etg_capture' + , '' + , 'event_trigger SECURITY DEFINER LANGUAGE plpgsql' + , $body$ +DECLARE + c_group_id CONSTANT int := object_group_id FROM object_reference.capture__get_current(); + r record; +BEGIN + /* + * Self-recognition: skip while this extension's own event_trigger__disable() + * is in effect (see below) -- i.e. this extension's own install/update + * script is doing delicate internal restructuring right now. Checked via + * to_regclass() rather than a catalog lookup that would error if the temp + * table doesn't exist, which is the common case. + */ + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RETURN; + END IF; + + IF c_group_id IS NOT NULL THEN -- Would be NULL if table is empty + RAISE DEBUG E'\n\n*** START ***'; + BEGIN + FOR r IN + SELECT classid, objid, objsubid, command_tag, object_type, schema_name, object_identity, in_extension + -- Have to manually exclude command field :/ + FROM pg_catalog.pg_event_trigger_ddl_commands() + LOOP + RAISE DEBUG 'ddl: %', row_to_json(r); + END LOOP; + END; + + FOR r IN SELECT + _object_reference._object_v__for_update( + object_type::cat_tools.object_type + , objid, objsubid + , c_group_id + , classid + ) + , classid, objid, objsubid, command_tag, object_type, schema_name, object_identity, in_extension + FROM pg_catalog.pg_event_trigger_ddl_commands() + WHERE command_tag ~ '^CREATE' --'^(ALTER|CREATE)' + AND NOT object_reference.unsupported(object_type::cat_tools.object_type) + AND (schema_name IS NULL + OR schema_name NOT LIKE 'pg_temp%' -- pg_my_temp_schema() doesn't seem worth it... + ) + /* + * __object_reference is this extension's own scratch install/update + * schema (created and dropped within a single script, never an + * extension member) -- self-recognition via the temp table above + * can't cover the handful of bootstrap statements that run before + * that table exists, so exclude it here too (object_identity + * carries the name for the CREATE SCHEMA statement itself, where + * schema_name is null). + * + * object_reference/_object_reference are excluded outright rather + * than relying on _object_v__for_update()'s own _is_own_object() + * guard: a brand-new object created by this extension's own + * update/install script isn't yet recorded as an 'e' member in + * pg_depend at the point its CREATE fires ddl_command_end (that + * happens once the surrounding CREATE/ALTER EXTENSION completes), + * so _is_own_object() can't see it as self-owned yet either -- + * confirmed by running into it: an active capture group during + * ALTER EXTENSION UPDATE otherwise ends up with this extension's + * own new functions as members. + * + * The schema-creation statement itself (CREATE SCHEMA + * __object_reference/etc.) has schema_name = NULL, with the name + * only available via object_identity -- checked separately, and + * restricted to object_type = 'schema', so an unrelated object of + * some other type whose identity happens to match one of these + * three exact strings (e.g. a same-named extension) isn't caught + * by this fallback. + */ + AND coalesce(schema_name, '') NOT IN ('__object_reference', 'object_reference', '_object_reference') + AND NOT (object_type = 'schema' AND object_identity IN ('__object_reference', 'object_reference', '_object_reference')) + LOOP + RAISE DEBUG 'registered %', row_to_json(r); + END LOOP; + RAISE DEBUG E'*** END ***\n\n'; + END IF; +END +$body$ + , 'Event trigger function to capture newly created objects in an object group.' +); +SELECT __object_reference.create_function( + '_object_reference._etg_fix_identity' + , '' + , 'event_trigger SECURITY DEFINER LANGUAGE plpgsql' + , $body$ +DECLARE + r_ddl record; + r record; +BEGIN + /* + * Self-recognition: skip while this extension's own event_trigger__disable() + * is in effect (see below) -- i.e. this extension's own install/update + * script is doing delicate internal restructuring right now. Checked via + * to_regclass() rather than a catalog lookup that would error if the temp + * table doesn't exist, which is the common case. + */ + IF to_regclass('pg_temp.__object_reference__event_trigger_state') IS NOT NULL THEN + RETURN; + END IF; + + /* + * It's tempting to use pg_event_trigger_ddl_commands() to find exactly what + * items have changed and worry about only those. That won't work because an + * object_names array can depend on multiple names (ie: a column depends on + * the name of it's table, as well as the name of the schema the table is in. + * You might think we could simply recurse through pg_depend to handle this, + * but not every name dependency gets enumerated that way. For example, + * columns are not marked as dependent on their table. + * + * Rather than trying to be cute about this, we just do a brute-force check + * for any names that have changed. + */ + + /* + * Presumably there's no way for an objects type/classid to change, but be + * safe and attempt the update to object_type. If it actually does change the + * constraint on the table should catch it. + */ + FOR r IN + UPDATE _object_reference.object + SET object_type = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).type::cat_tools.object_type + , object_names = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_names + , object_args = (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)).object_args + FROM _object_reference._object_oid oo + WHERE + oo.object_id = object.object_id + AND (object_type::text, object_names, object_args) IS DISTINCT FROM + (pg_catalog.pg_identify_object_as_address(classid, objid, objsubid)) + RETURNING * + LOOP + RAISE DEBUG 'modified_objects(): %', r; + END LOOP; +END +$body$ + , 'Event trigger function to update any records with object names or args that have changed.' +); + +/* + * Restore each trigger's actual prior state (captured above) now that the + * guarded bodies above are live -- self-recognition (checking for + * event_trigger__disable()'s temp table, created below) takes over from + * here for the rest of this script. + */ +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. * - * 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. + * 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 set_config('object_reference.saved_session_replication_role', current_setting('session_replication_role'), true); -SET LOCAL session_replication_role = replica; +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 @@ -293,6 +663,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 +743,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( @@ -679,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 cbed4f5..a43627b 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( @@ -1384,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 ***'; @@ -1397,7 +1452,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 +1466,36 @@ BEGIN AND (schema_name IS NULL OR schema_name NOT LIKE 'pg_temp%' -- pg_my_temp_schema() doesn't seem worth it... ) + /* + * __object_reference is this extension's own scratch install/update + * schema (created and dropped within a single script, never an + * extension member) -- self-recognition via the temp table above + * can't cover the handful of bootstrap statements that run before + * that table exists, so exclude it here too (object_identity + * carries the name for the CREATE SCHEMA statement itself, where + * schema_name is null). + * + * object_reference/_object_reference are excluded outright rather + * than relying on _object_v__for_update()'s own _is_own_object() + * guard: a brand-new object created by this extension's own + * update/install script isn't yet recorded as an 'e' member in + * pg_depend at the point its CREATE fires ddl_command_end (that + * happens once the surrounding CREATE/ALTER EXTENSION completes), + * so _is_own_object() can't see it as self-owned yet either -- + * confirmed by running into it: an active capture group during + * ALTER EXTENSION UPDATE otherwise ends up with this extension's + * own new functions as members. + * + * The schema-creation statement itself (CREATE SCHEMA + * __object_reference/etc.) has schema_name = NULL, with the name + * only available via object_identity -- checked separately, and + * restricted to object_type = 'schema', so an unrelated object of + * some other type whose identity happens to match one of these + * three exact strings (e.g. a same-named extension) isn't caught + * by this fallback. + */ + AND coalesce(schema_name, '') NOT IN ('__object_reference', 'object_reference', '_object_reference') + AND NOT (object_type = 'schema' AND object_identity IN ('__object_reference', 'object_reference', '_object_reference')) LOOP RAISE DEBUG 'registered %', row_to_json(r); END LOOP; @@ -1431,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 @@ -1516,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 fadaba6..24f8e8f 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,12 @@ 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/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/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$$ 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