Fire the event trigger for DROP SCHEMA and DROP OWNED

HISTORY.asc

@@ -6,6 +6,27 @@ 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()`.
+
+Dropping an unregistered extension (by any of the three commands) needs only
+USAGE on the extension's schema, to resolve the new `SECURITY DEFINER`
+function `extension_drop__registered()` (executable by `PUBLIC`), which the
+trigger asks which of the dropped extensions are registered; it needs no
+privileges on `extension_drop__commands`. Dropping a registered extension
+additionally needs SELECT and DELETE on that table. A role without the access
+gets an `insufficient_privilege` error that names the table, its owner and
+the extensions being dropped, with the underlying error as DETAIL and a HINT
+giving the grants that fix it, instead of `permission denied for table
+extension_drop__commands` or `type "extension_drop__commands" does not
+exist`. The registered SQL runs as the caller and may need further
+privileges.
+
 == 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

README.md

@@ -25,6 +25,13 @@ arbitrary SQL to run later. Grant them to specific roles as needed.
   Look up the row (`extension_name`, `sql`) registered for `extension_name`.
   Granted to `PUBLIC` by default.
 
+- `extension_drop__registered(extension_names name[]) RETURNS name[]`
+  Returns those of `extension_names` that have registered drop commands, and
+  asserts the table is sane when there are none. `SECURITY DEFINER` (with
+  `search_path` pinned to the extension's schema, then `pg_temp`); used by
+  the event trigger so dropping an unregistered extension needs no table
+  access. Granted to `PUBLIC` by default.
+
 - `extension_drop__sanity_check(ignore name DEFAULT NULL) RETURNS name[]`
   Returns the names of extensions that have registered drop commands but no
   longer exist. Should always return an empty array; pass an extension name
@@ -42,6 +49,19 @@ 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_drop__add`/`extension_drop__update`, and removes the registration
-afterward.
+extension is dropped -- by `DROP EXTENSION`, `DROP SCHEMA ... CASCADE` or
+`DROP OWNED BY` -- removes its registration from `extension_drop__commands`
+and runs the SQL registered for it via
+`extension_drop__add`/`extension_drop__update`. Those commands fire the
+trigger for every drop, but it returns immediately unless an extension is
+among the dropped objects.
+
+Dropping an unregistered extension needs only USAGE on the extension's
+schema (to resolve `extension_drop__registered()`, a `SECURITY DEFINER`
+function executable by `PUBLIC`, which the trigger asks which of the dropped
+extensions are registered); it needs no privileges on `extension_drop__commands`.
+Dropping a registered extension additionally needs SELECT and DELETE on that
+table. A role without the access gets an `insufficient_privilege` error that
+names the table, its owner and the extensions being dropped, with the
+underlying error as DETAIL and a HINT giving the grants that fix it. The
+registered SQL runs as the caller and may need further privileges.

sql/extension_drop--0.1.1--stable.sql

@@ -1,20 +1,117 @@
+/*
+ * The install script's create_function doesn't exist here, so build the helper directly; see
+ * extension_drop.sql for why it is SECURITY DEFINER with pg_temp last on its search_path.
+ */
+DO $do$
+DECLARE
+  sp text;
+BEGIN
+  SELECT pg_catalog.format('%I', n.nspname) INTO sp
+    FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace
+    WHERE e.extname = 'extension_drop';
+  EXECUTE pg_catalog.format($f$
+CREATE OR REPLACE FUNCTION extension_drop__registered(
+  extension_names name[]
+) RETURNS name[] LANGUAGE plpgsql STABLE SECURITY DEFINER SET search_path = %s, pg_temp AS
+$body$
+DECLARE
+  ret name[] := array(
+    SELECT extension_name FROM extension_drop__commands WHERE extension_name = ANY(extension_names) ORDER BY 1
+  );
+BEGIN
+  -- With nothing registered the trigger returns without its own assert, so any stray row is a stale one.
+  IF ret = '{}'::name[] THEN
+    PERFORM extension_drop__sanity_assert();
+  END IF;
+  RETURN ret;
+END
+$body$
+$f$, sp);
+END
+$do$;
+REVOKE ALL ON FUNCTION extension_drop__registered(name[]) FROM public;
+GRANT EXECUTE ON FUNCTION extension_drop__registered(name[]) TO public;
+COMMENT ON FUNCTION extension_drop__registered(name[]) IS
+  $$Returns the given extensions that have drop commands, after asserting the table is sane if there are none.$$;
+
 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;
+  t record;
+  dropping text;
+  dropped name[];
+  registered name[];
+  names text[];
+  cmds text[];
+  i int;
+  msg text;
 BEGIN
   RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag;
-  FOR r IN
-    SELECT c.*
-      FROM extension_drop__commands c
-        JOIN pg_event_trigger_dropped_objects() d
-          ON c.extension_name = d.object_name
-            AND d.object_type = 'extension'
-  LOOP
-    RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', r.extension_name, r.sql;
-    EXECUTE r.sql;
-    DELETE FROM extension_drop__commands WHERE extension_name = r.extension_name;
+
+  -- Most drops involve no extension; don't touch our table (or assert) for them.
+  SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name)
+      , pg_catalog.array_agg(object_name::name ORDER BY object_name)
+    INTO dropping, dropped
+    FROM pg_catalog.pg_event_trigger_dropped_objects()
+    WHERE object_type = 'extension';
+  IF dropping IS NULL THEN
+    RETURN;
+  END IF;
+
+  /*
+   * Callers without table access can still drop unregistered extensions. Without USAGE on our schema the
+   * helper isn't found; fall through to the DELETE below for its clearer error.
+   */
+  BEGIN
+    registered := extension_drop__registered(dropped);
+  EXCEPTION WHEN undefined_function OR insufficient_privilege THEN
+    registered := NULL;
+  END;
+  IF registered = '{}'::name[] THEN
+    RETURN;
+  END IF;
+
+  /*
+   * One statement touches the table, so a privilege error here is ours; registered SQL runs after the
+   * handler so its own errors pass through untouched.
+   */
+  BEGIN
+    WITH del AS (
+      DELETE FROM extension_drop__commands c
+        USING pg_catalog.pg_event_trigger_dropped_objects() d
+        WHERE c.extension_name = d.object_name AND d.object_type = 'extension'
+        RETURNING c.extension_name, c.sql
+    )
+    SELECT pg_catalog.array_agg(extension_name), pg_catalog.array_agg(sql) INTO names, cmds FROM del;
+  EXCEPTION WHEN insufficient_privilege OR undefined_table THEN
+    -- undefined_table: name lookup skips a schema the caller lacks USAGE on, so the table "doesn't exist".
+    GET STACKED DIAGNOSTICS msg = MESSAGE_TEXT;
+
+    -- Look up the table by OID path, not by name, for the same reason.
+    SELECT n.nspname, c.relname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner
+      INTO t
+      FROM pg_catalog.pg_extension e
+        JOIN pg_catalog.pg_class c
+          ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands'
+        JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
+      WHERE e.extname = 'extension_drop';
+    IF NOT FOUND THEN
+      RAISE;
+    END IF;
+
+    RAISE EXCEPTION 'permission denied for table "%"."%" (owned by role "%"), which extension_drop needs to drop extension(s) %'
+        , t.nspname, t.relname, t.owner, dropping
+      USING ERRCODE = 'insufficient_privilege'
+        , DETAIL = msg
+        , HINT = pg_catalog.format(
+            'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s to the current role.'
+            , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname), t.qname);
+  END;
+
+  FOR i IN 1 .. COALESCE(pg_catalog.array_length(cmds, 1), 0) LOOP
+    RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', names[i], cmds[i];
+    EXECUTE cmds[i];
   END LOOP;
 
   /*
@@ -25,4 +122,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

sql/extension_drop.sql

@@ -37,10 +37,24 @@ CREATE FUNCTION __extension_drop.create_function(
 DECLARE
   c_clean_args text := cat_tools.routine__parse_arg_types_text(args);
 
+  -- A definer function must not search pg_temp ahead of its own schema, so pin the path explicitly.
+  search_path_clause text := CASE
+    WHEN options ~* 'security\s+definer' THEN COALESCE(
+      (
+        SELECT pg_catalog.format('= %I, pg_temp', n.nspname)
+          FROM pg_catalog.pg_extension e JOIN pg_catalog.pg_namespace n ON n.oid = e.extnamespace
+          WHERE e.extname = 'extension_drop'
+      )
+      -- Not running as an extension script (e.g. the build smoke test)
+      , pg_catalog.format('= %I, pg_temp', pg_catalog.current_schema())
+    )
+    ELSE 'FROM CURRENT'
+  END;
+
   create_template CONSTANT text := $template$
 CREATE OR REPLACE FUNCTION %s(
 %s
-) RETURNS %s SET search_path FROM CURRENT AS
+) RETURNS %s SET search_path %s AS
 %L
 $template$
   ;
@@ -71,7 +85,8 @@ BEGIN
       create_template
       , function_name
       , args
-      , options -- TODO: Force search_path if options ~* 'definer'
+      , options
+      , search_path_clause
       , body
     ) )
   ;
@@ -255,6 +270,37 @@ $body$
   , '' -- Just revoke all access
 );
 
+/*
+ * REGISTERED
+ */
+/*
+ * SECURITY DEFINER so the trigger can tell whether a dropped extension is registered without the caller
+ * needing table access; reads only, never runs registered SQL. The path is pinned with pg_temp last
+ * (see create_function) so a caller's temp table can't shadow ours.
+ */
+SELECT __extension_drop.create_function(
+  'extension_drop__registered'
+  , $$
+  extension_names name[]
+$$
+  , 'name[] LANGUAGE plpgsql STABLE SECURITY DEFINER'
+  , $body$
+DECLARE
+  ret name[] := array(
+    SELECT extension_name FROM extension_drop__commands WHERE extension_name = ANY(extension_names) ORDER BY 1
+  );
+BEGIN
+  -- With nothing registered the trigger returns without its own assert, so any stray row is a stale one.
+  IF ret = '{}'::name[] THEN
+    PERFORM extension_drop__sanity_assert();
+  END IF;
+  RETURN ret;
+END
+$body$
+  , $$Returns the given extensions that have drop commands, after asserting the table is sane if there are none.$$
+  , 'public'
+);
+
 /*
  * TRIGGER FUNCTION
  */
@@ -264,19 +310,80 @@ SELECT __extension_drop.create_function(
   , 'event_trigger LANGUAGE plpgsql'
   , $body$
 DECLARE
-  r extension_drop__commands;
+  t record;
+  dropping text;
+  dropped name[];
+  registered name[];
+  names text[];
+  cmds text[];
+  i int;
+  msg text;
 BEGIN
   RAISE DEBUG 'extension_drop event trigger entry: tg_event %, tg_tag %', tg_event, tg_tag;
-  FOR r IN
-    SELECT c.*
-      FROM extension_drop__commands c
-        JOIN pg_event_trigger_dropped_objects() d
-          ON c.extension_name = d.object_name
-            AND d.object_type = 'extension'
-  LOOP
-    RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', r.extension_name, r.sql;
-    EXECUTE r.sql;
-    DELETE FROM extension_drop__commands WHERE extension_name = r.extension_name;
+
+  -- Most drops involve no extension; don't touch our table (or assert) for them.
+  SELECT pg_catalog.string_agg(pg_catalog.quote_ident(object_name), ', ' ORDER BY object_name)
+      , pg_catalog.array_agg(object_name::name ORDER BY object_name)
+    INTO dropping, dropped
+    FROM pg_catalog.pg_event_trigger_dropped_objects()
+    WHERE object_type = 'extension';
+  IF dropping IS NULL THEN
+    RETURN;
+  END IF;
+
+  /*
+   * Callers without table access can still drop unregistered extensions. Without USAGE on our schema the
+   * helper isn't found; fall through to the DELETE below for its clearer error.
+   */
+  BEGIN
+    registered := extension_drop__registered(dropped);
+  EXCEPTION WHEN undefined_function OR insufficient_privilege THEN
+    registered := NULL;
+  END;
+  IF registered = '{}'::name[] THEN
+    RETURN;
+  END IF;
+
+  /*
+   * One statement touches the table, so a privilege error here is ours; registered SQL runs after the
+   * handler so its own errors pass through untouched.
+   */
+  BEGIN
+    WITH del AS (
+      DELETE FROM extension_drop__commands c
+        USING pg_catalog.pg_event_trigger_dropped_objects() d
+        WHERE c.extension_name = d.object_name AND d.object_type = 'extension'
+        RETURNING c.extension_name, c.sql
+    )
+    SELECT pg_catalog.array_agg(extension_name), pg_catalog.array_agg(sql) INTO names, cmds FROM del;
+  EXCEPTION WHEN insufficient_privilege OR undefined_table THEN
+    -- undefined_table: name lookup skips a schema the caller lacks USAGE on, so the table "doesn't exist".
+    GET STACKED DIAGNOSTICS msg = MESSAGE_TEXT;
+
+    -- Look up the table by OID path, not by name, for the same reason.
+    SELECT n.nspname, c.relname, pg_catalog.format('%I.%I', n.nspname, c.relname) AS qname, pg_catalog.pg_get_userbyid(c.relowner) AS owner
+      INTO t
+      FROM pg_catalog.pg_extension e
+        JOIN pg_catalog.pg_class c
+          ON c.relnamespace = e.extnamespace AND c.relname = 'extension_drop__commands'
+        JOIN pg_catalog.pg_namespace n ON n.oid = c.relnamespace
+      WHERE e.extname = 'extension_drop';
+    IF NOT FOUND THEN
+      RAISE;
+    END IF;
+
+    RAISE EXCEPTION 'permission denied for table "%"."%" (owned by role "%"), which extension_drop needs to drop extension(s) %'
+        , t.nspname, t.relname, t.owner, dropping
+      USING ERRCODE = 'insufficient_privilege'
+        , DETAIL = msg
+        , HINT = pg_catalog.format(
+            'Run the drop as role %s or a superuser, or grant USAGE on schema %s and SELECT, DELETE on %s to the current role.'
+            , pg_catalog.quote_ident(t.owner), pg_catalog.quote_ident(t.nspname), t.qname);
+  END;
+
+  FOR i IN 1 .. COALESCE(pg_catalog.array_length(cmds, 1), 0) LOOP
+    RAISE DEBUG E'extension "%" is being dropped; executing SQL:\n%', names[i], cmds[i];
+    EXECUTE cmds[i];
   END LOOP;
 
   /*
@@ -291,7 +398,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()
 ;
 

test/build/expected/build.out

@@ -19,4 +19,6 @@ INSTALL
 
 
 
+
+
 # TRANSACTION INTENTIONALLY LEFT OPEN