diff --git a/HISTORY.asc b/HISTORY.asc index 7e6369e..54478b8 100644 --- a/HISTORY.asc +++ b/HISTORY.asc @@ -6,6 +6,14 @@ extension_drop depends on cat_tools, which already requires PostgreSQL too (a cat_tools function it calls needs PG11+ internally) -- this just makes the declared floor honest. +== Run drop commands when `DROP SCHEMA ... CASCADE` or `DROP OWNED BY` drops an extension +The event trigger now fires for `DROP SCHEMA` and `DROP OWNED` as well as +`DROP EXTENSION`, and returns immediately unless an extension is among the +dropped objects, so other drops are unaffected. Previously the other two +commands skipped the registered SQL and left a stale registration that made +every later `DROP EXTENSION` fail with `XD001`; remove any existing stale +registration with `extension_drop__repair()`. + == Remove redundant client_min_messages handling from install script `CREATE EXTENSION` already raises `client_min_messages` to `WARNING` for the install script (only-raising, so a stricter caller is respected) and diff --git a/README.md b/README.md index c38c877..08effaa 100644 --- a/README.md +++ b/README.md @@ -42,6 +42,8 @@ arbitrary SQL to run later. Grant them to specific roles as needed. ## How it works An event trigger (`extension_drop`, on `sql_drop`) fires whenever an -extension is dropped, runs any SQL registered for it via +extension is dropped -- by `DROP EXTENSION`, `DROP SCHEMA ... CASCADE` or +`DROP OWNED BY` -- runs any SQL registered for it via `extension_drop__add`/`extension_drop__update`, and removes the registration -afterward. +afterward. Those commands fire the trigger for every drop, but it returns +immediately unless an extension is among the dropped objects. diff --git a/sql/extension_drop--0.1.1--stable.sql b/sql/extension_drop--0.1.1--stable.sql index b011c62..940420e 100644 --- a/sql/extension_drop--0.1.1--stable.sql +++ b/sql/extension_drop--0.1.1--stable.sql @@ -2,9 +2,17 @@ CREATE OR REPLACE FUNCTION extension_drop__event_trigger( ) RETURNS event_trigger LANGUAGE plpgsql SET search_path FROM CURRENT AS $body$ DECLARE - r extension_drop__commands; + r record; -- Not extension_drop__commands: the type must not resolve before the early return BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; + + -- Most drops involve no extension; don't touch our table (or assert) for them. + IF NOT EXISTS( + SELECT 1 FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension' + ) THEN + RETURN; + END IF; + FOR r IN SELECT c.* FROM extension_drop__commands c @@ -25,4 +33,12 @@ BEGIN END $body$; +-- ALTER EVENT TRIGGER can't change WHEN +DROP EVENT TRIGGER extension_drop; +CREATE EVENT TRIGGER extension_drop + ON sql_drop + WHEN tag IN( 'DROP EXTENSION', 'DROP SCHEMA', 'DROP OWNED' ) -- NOTE! This MUST be IN + EXECUTE PROCEDURE extension_drop__event_trigger() +; + -- vim: sw=2 ts=2 expandtab diff --git a/sql/extension_drop.sql b/sql/extension_drop.sql index 75319be..2e66442 100644 --- a/sql/extension_drop.sql +++ b/sql/extension_drop.sql @@ -264,9 +264,17 @@ SELECT __extension_drop.create_function( , 'event_trigger LANGUAGE plpgsql' , $body$ DECLARE - r extension_drop__commands; + r record; -- Not extension_drop__commands: the type must not resolve before the early return BEGIN RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag; + + -- Most drops involve no extension; don't touch our table (or assert) for them. + IF NOT EXISTS( + SELECT 1 FROM pg_catalog.pg_event_trigger_dropped_objects() WHERE object_type = 'extension' + ) THEN + RETURN; + END IF; + FOR r IN SELECT c.* FROM extension_drop__commands c @@ -291,7 +299,8 @@ $body$ CREATE EVENT TRIGGER extension_drop ON sql_drop - WHEN tag IN( 'DROP EXTENSION' ) -- NOTE! This MUST be IN + -- DROP SCHEMA CASCADE and DROP OWNED BY also drop extensions. + WHEN tag IN( 'DROP EXTENSION', 'DROP SCHEMA', 'DROP OWNED' ) -- NOTE! This MUST be IN EXECUTE PROCEDURE extension_drop__event_trigger() ; diff --git a/test/expected/drop_owned.out b/test/expected/drop_owned.out new file mode 100644 index 0000000..36d377c --- /dev/null +++ b/test/expected/drop_owned.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create role +ok 3 - Create test extension as role +ok 4 - Verify extension_drop__get() +ok 5 - Drop owned +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 - extension_drop's schema should not be reachable via search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_owned_1.out b/test/expected/drop_owned_1.out new file mode 100644 index 0000000..3801831 --- /dev/null +++ b/test/expected/drop_owned_1.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create role +ok 3 - Create test extension as role +ok 4 - Verify extension_drop__get() +ok 5 - Drop owned +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 # SKIP existing mode: extension_drop's placement is outside this suite's control +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_schema_cascade.out b/test/expected/drop_schema_cascade.out new file mode 100644 index 0000000..e280978 --- /dev/null +++ b/test/expected/drop_schema_cascade.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create schema +ok 3 - Create test extension in schema +ok 4 - Verify extension_drop__get() +ok 5 - Drop schema cascade +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 - extension_drop's schema should not be reachable via search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/drop_schema_cascade_1.out b/test/expected/drop_schema_cascade_1.out new file mode 100644 index 0000000..a634d57 --- /dev/null +++ b/test/expected/drop_schema_cascade_1.out @@ -0,0 +1,16 @@ +\set ECHO none +1..13 +ok 1 - Insert into extension_drop_test_table +ok 2 - Create schema +ok 3 - Create test extension in schema +ok 4 - Verify extension_drop__get() +ok 5 - Drop schema cascade +ok 6 - Test extension does not exist +ok 7 - extension_drop_test_table is empty +ok 8 - No stale registration +ok 9 - Create test extension +ok 10 - Test extension exists +ok 11 - Drop test extension +ok 12 - Test extension does not exist +ok 13 # SKIP existing mode: extension_drop's placement is outside this suite's control +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/unrelated_drops.out b/test/expected/unrelated_drops.out new file mode 100644 index 0000000..d1d9177 --- /dev/null +++ b/test/expected/unrelated_drops.out @@ -0,0 +1,11 @@ +\set ECHO none +1..8 +ok 1 - Create non-superuser role +ok 2 - Grant CREATE on database +ok 3 - Non-superuser DROP SCHEMA CASCADE +ok 4 - Non-superuser DROP OWNED BY +ok 5 - Insert stale registration +ok 6 - Stale registration is present +ok 7 - Superuser DROP SCHEMA with stale registration +ok 8 - extension_drop's schema should not be reachable via search_path +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/expected/unrelated_drops_1.out b/test/expected/unrelated_drops_1.out new file mode 100644 index 0000000..6860796 --- /dev/null +++ b/test/expected/unrelated_drops_1.out @@ -0,0 +1,11 @@ +\set ECHO none +1..8 +ok 1 - Create non-superuser role +ok 2 - Grant CREATE on database +ok 3 - Non-superuser DROP SCHEMA CASCADE +ok 4 - Non-superuser DROP OWNED BY +ok 5 - Insert stale registration +ok 6 - Stale registration is present +ok 7 - Superuser DROP SCHEMA with stale registration +ok 8 # SKIP existing mode: extension_drop's placement is outside this suite's control +# TRANSACTION INTENTIONALLY LEFT OPEN! diff --git a/test/helpers/test_ext__verify_dropped.sql b/test/helpers/test_ext__verify_dropped.sql new file mode 100644 index 0000000..a86f134 --- /dev/null +++ b/test/helpers/test_ext__verify_dropped.sql @@ -0,0 +1,11 @@ +SELECT hasnt_extension('extension_drop_test', 'Test extension does not exist'); +SELECT is_empty( + $$SELECT * FROM $$ || :'TT' + , :'TT' || ' is empty' +); +SELECT is_empty( + format($$SELECT unnest(%I.extension_drop__sanity_check())$$, :'extension_drop_schema') + , 'No stale registration' +); + +-- vi: expandtab sw=2 ts=2 diff --git a/test/sql/drop_owned.sql b/test/sql/drop_owned.sql new file mode 100644 index 0000000..dcc439c --- /dev/null +++ b/test/sql/drop_owned.sql @@ -0,0 +1,56 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * DROP OWNED BY can drop an extension; its registered command must run and + * the registration must go away, or the stale row makes every later DROP + * EXTENSION fail. + */ +SELECT plan( + 0 + + + 1 -- Insert into test table + + + 1 -- Create role + + 1 -- Create test extension as role + + 1 -- Verify extension_drop__get() + + 1 -- Drop owned + + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) + + + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) + + + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) +); + +\i test/helpers/test_ext__insert.sql + +SELECT lives_ok( + 'CREATE ROLE extension_drop_test_owner SUPERUSER' + , 'Create role' +); +-- ALTER EXTENSION can't change the owner, so create it as that role +SELECT lives_ok( + $q$DO $$ BEGIN + SET LOCAL ROLE extension_drop_test_owner; + CREATE EXTENSION extension_drop_test; + RESET ROLE; + END $$$q$ + , 'Create test extension as role' +); +SELECT bag_eq( + format($$SELECT * FROM %I.extension_drop__get('extension_drop_test')$$, :'extension_drop_schema') + , $$SELECT 'extension_drop_test'::name, 'DELETE FROM extension_drop_test_table'::text$$ + , 'Verify extension_drop__get()' +); + +SELECT lives_ok( + 'DROP OWNED BY extension_drop_test_owner' + , 'Drop owned' +); +\i test/helpers/test_ext__verify_dropped.sql + +\i test/helpers/test_ext__create_drop.sql + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2 diff --git a/test/sql/drop_schema_cascade.sql b/test/sql/drop_schema_cascade.sql new file mode 100644 index 0000000..a6b46c3 --- /dev/null +++ b/test/sql/drop_schema_cascade.sql @@ -0,0 +1,54 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * DROP SCHEMA ... CASCADE can drop an extension; its registered command must + * run and the registration must go away, or the stale row makes every later + * DROP EXTENSION fail. + */ +SELECT plan( + 0 + + + 1 -- Insert into test table + + + 1 -- Create schema + + 1 -- Create test extension in it + + 1 -- Verify extension_drop__get() + + 1 -- Drop schema cascade + + 3 -- Verify extension is gone, table is empty, no stale registration (test/helpers/test_ext__verify_dropped.sql) + + + 4 -- Create and drop test extension again (test/helpers/test_ext__create_drop.sql) + + + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) +); + +-- Hide the version-specific "drop cascades to" NOTICE +SET client_min_messages = WARNING; + +\i test/helpers/test_ext__insert.sql + +SELECT lives_ok( + 'CREATE SCHEMA extension_drop_test_schema' + , 'Create schema' +); +SELECT lives_ok( + 'CREATE EXTENSION extension_drop_test SCHEMA extension_drop_test_schema' + , 'Create test extension in schema' +); +SELECT bag_eq( + format($$SELECT * FROM %I.extension_drop__get('extension_drop_test')$$, :'extension_drop_schema') + , $$SELECT 'extension_drop_test'::name, 'DELETE FROM extension_drop_test_table'::text$$ + , 'Verify extension_drop__get()' +); + +SELECT lives_ok( + 'DROP SCHEMA extension_drop_test_schema CASCADE' + , 'Drop schema cascade' +); +\i test/helpers/test_ext__verify_dropped.sql + +\i test/helpers/test_ext__create_drop.sql + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2 diff --git a/test/sql/unrelated_drops.sql b/test/sql/unrelated_drops.sql new file mode 100644 index 0000000..1bdc4a5 --- /dev/null +++ b/test/sql/unrelated_drops.sql @@ -0,0 +1,73 @@ +\set ECHO none +\i test/pgxntool/setup.sql + +/* + * DROP SCHEMA and DROP OWNED fire extension_drop's event trigger, so drops + * that involve no extension must not depend on the caller's privileges or on + * the state of extension_drop__commands. + */ +SELECT plan( + 0 + + + 1 -- Create non-superuser role + + 1 -- Grant it CREATE on the database + + 1 -- Non-superuser DROP SCHEMA CASCADE + + 1 -- Non-superuser DROP OWNED BY + + + 1 -- Insert stale registration + + 1 -- Stale registration is present + + 1 -- Superuser DROP SCHEMA with stale registration + + + 1 -- extension_drop's schema should not be on search_path (test/finish.sql) +); + +-- Hide version- and database-specific NOTICE/WARNING text from the drops +SET client_min_messages = ERROR; + +SELECT lives_ok( + 'CREATE ROLE extension_drop_test_plain' + , 'Create non-superuser role' +); +SELECT lives_ok( + format('GRANT CREATE ON DATABASE %I TO extension_drop_test_plain', current_database()) + , 'Grant CREATE on database' +); + +SELECT lives_ok( + $q$DO $$ BEGIN + SET LOCAL ROLE extension_drop_test_plain; + CREATE SCHEMA extension_drop_test_plain_schema; + CREATE TABLE extension_drop_test_plain_schema.t(i int); + DROP SCHEMA extension_drop_test_plain_schema CASCADE; + RESET ROLE; + END $$$q$ + , 'Non-superuser DROP SCHEMA CASCADE' +); +SELECT lives_ok( + $q$DO $$ BEGIN + SET LOCAL ROLE extension_drop_test_plain; + CREATE SCHEMA extension_drop_test_plain_schema; + CREATE TABLE extension_drop_test_plain_schema.t(i int); + DROP OWNED BY extension_drop_test_plain; + RESET ROLE; + END $$$q$ + , 'Non-superuser DROP OWNED BY' +); + +SELECT lives_ok( + format($$INSERT INTO %I.extension_drop__commands VALUES('extension_drop_test_stale', 'SELECT 1')$$, :'extension_drop_schema') + , 'Insert stale registration' +); +SELECT results_eq( + format($$SELECT %I.extension_drop__sanity_check()$$, :'extension_drop_schema') + , $$SELECT '{extension_drop_test_stale}'::name[]$$ + , 'Stale registration is present' +); +SELECT lives_ok( + $$CREATE SCHEMA extension_drop_test_su_schema; DROP SCHEMA extension_drop_test_su_schema$$ + , 'Superuser DROP SCHEMA with stale registration' +); + +\i test/finish.sql + +-- vi: expandtab sw=2 ts=2