Fire the event trigger for DROP SCHEMA and DROP OWNED

[jnasbyupgrade]

extension_drop's event trigger now fires for DROP SCHEMA and DROP OWNED as well as DROP EXTENSION, so an extension dropped by DROP SCHEMA ... CASCADE or DROP OWNED BY runs its registered SQL and has its registration removed instead of leaving a stale row in extension_drop__commands (which makes every later DROP EXTENSION raise XD001).

These three are the only commands that can drop an extension. An extension has ordinary pg_depend rows only on its schema and on any extension it requires, so it goes away via DROP EXTENSION (including cascading to dependents), DROP SCHEMA ... CASCADE, or DROP OWNED, which walks the owner dependencies in shdepend. This doesn't depend on the PostgreSQL version. Corroboration on PG12 and PG17 with an unfiltered sql_drop trigger: only those three tags dropped an extension, and DROP ROLE, DROP TYPE/DROP FUNCTION ... CASCADE on a member, and DROP SCHEMA without CASCADE all error out, while REASSIGN OWNED drops nothing.

Since the trigger now runs for every DROP SCHEMA and DROP OWNED, extension_drop__event_trigger() returns immediately unless an extension is among the dropped objects. Unrelated drops therefore don't read extension_drop__commands or run the sanity assert, so they work for non-superusers and are unaffected by a stale row. The row variable is a record so the function compiles for a caller without access to the extension's schema.

Behavior change: a plain role that drops a registered extension with DROP SCHEMA ... CASCADE now fails with permission denied for table extension_drop__commands, as DROP EXTENSION by such a role already does.

ALTER EVENT TRIGGER can't change WHEN, so extension_drop--0.1.1--stable.sql drops and recreates the trigger and replaces the function. Function properties (proacl, proconfig, source hash) and pg_event_trigger.evttags are identical after a fresh install and after updating from 0.1.1, on PG12 and PG17.

Tests: drop_schema_cascade and drop_owned fail without the wider WHEN list. unrelated_drops guards the widening: it fails if the trigger fires for these commands without the early return.

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 099add2f-98cf-430e-a23f-64cafdc90cad

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Claude finished @jnasbyupgrade's task in 30s —— View job

---

Review of PR #55: fire the event trigger for DROP SCHEMA and DROP OWNED

I read the diff and the trigger and sanity-check code. I did not build the extension or run the test suite. The code-review skill failed to run, so this is a manual review.

The change is small and consistent. The trigger WHEN list, the 0.1.1→stable update script, the README and HISTORY entries, and the new tests all agree with each other.

Concern: the trigger now fires for ordinary users (not verified by running it)

DROP EXTENSION is normally run by superusers. DROP SCHEMA and DROP OWNED BY are routine for non-superusers on their own objects. The trigger function is not SECURITY DEFINER. It reads extension_drop__commands and calls extension_drop__sanity_assert(). The install script creates these with '' grants, which revokes all access (sql/extension_drop.sql:113 and :255).

Event triggers run with the privileges of the user who issued the command. So a non-superuser running DROP SCHEMA myschema or DROP OWNED BY me may now fail with permission denied for function extension_drop__event_trigger. That would break DDL that has nothing to do with extensions.

There is a second, related effect. Any stale row in extension_drop__commands now makes every DROP SCHEMA and DROP OWNED raise XD001, not only DROP EXTENSION. The blast radius of a bad state is larger.

Suggestions:

• Add a test that runs DROP SCHEMA and DROP OWNED BY as a non-superuser and confirms they still succeed. If they fail, make the trigger function SECURITY DEFINER with a pinned search_path (it already sets search_path FROM CURRENT).
• Alternatively, skip the work when pg_event_trigger_dropped_objects() contains no object_type = 'extension' row. Do this before sanity_assert, so unrelated drops never hit the assertion. The PR says the trigger function is unchanged, so this would be a deliberate change from that.

Minor

• The tests hide the drop cascades to notice with client_min_messages = WARNING in drop_schema_cascade.sql. drop_owned.sql has no such notice, so this is fine.
• drop_owned.sql creates a SUPERUSER role. That is fine only because the test transaction is left open and rolled back, so make sure that stays true.
• I found no bugs in the update script. Dropping and recreating the trigger is the right approach, since ALTER EVENT TRIGGER can't change WHEN.

[jnasbyupgrade]

Claude here. Addressing the review's non-superuser concern (confirmed by running it: a plain role's DROP SCHEMA and DROP OWNED BY failed with a permission error, and a stale row made unrelated drops raise XD001): extension_drop__event_trigger() now returns immediately unless pg_event_trigger_dropped_objects() contains an extension, before touching extension_drop__commands or the sanity assert. It also uses a record variable so it compiles for a role with no access to the extension's schema. New test unrelated_drops covers a non-superuser DROP SCHEMA ... CASCADE and DROP OWNED BY, and a superuser DROP SCHEMA while a stale row exists; it fails on the previous commit and passes now. SECURITY DEFINER was not used. The SUPERUSER role in drop_owned.sql is created inside the test transaction, which is rolled back.

[github-actions]

Claude finished @jnasbyupgrade's task in 21s —— View job

---

Review of PR #55: fire the event trigger for DROP SCHEMA and DROP OWNED

I read the diff, including the trigger SQL, the update script and test/sql/unrelated_drops.sql. I did not build the extension or run the tests.

I found no bugs. The earlier review's two concerns are both handled in the current head:

• Non-superuser drops. The trigger function now returns early unless pg_event_trigger_dropped_objects() contains an extension row (sql/extension_drop.sql:270). Ordinary DROP SCHEMA and DROP OWNED BY calls therefore never read extension_drop__commands, which has no grants. unrelated_drops.sql:36-55 tests both commands as a non-superuser.
• Stale rows. Because of the early return, a stale registration no longer makes unrelated drops raise XD001. unrelated_drops.sql:57-69 covers this by dropping a schema with a stale row present.

The fresh-install and 0.1.1→stable paths are consistent:

• Both create the same function body and the same WHEN tag IN ('DROP EXTENSION', 'DROP SCHEMA', 'DROP OWNED').
• The update script drops and recreates the trigger, which is needed because ALTER EVENT TRIGGER can't change WHEN.
• The README and HISTORY text match the behavior.

Minor notes (no action required)

• The PR description says "The trigger function is unchanged", but the diff adds the early return and changes r from a table-row type to record. The description is out of date, and the code is fine.
• unrelated_drops.sql leaves the role extension_drop_test_plain and the GRANT CREATE ON DATABASE in place. This is fine only if test/finish.sql rolls back the transaction, so keep it that way. I didn't check that.
• drop_owned.sql creates a SUPERUSER role. This has the same rollback caveat.

[jnasbyupgrade]

Claude here. Decisions needed from the maintainer:

1. Document known limits that predate this PR? A stale row or skipped cleanup can still occur when the event trigger is disabled (ALTER EVENT TRIGGER ... DISABLE), under session_replication_role = replica, and when the extension or schema holding extension_drop itself is dropped (other extensions' registered cleanup is silently skipped).
2. Warn about existing stale rows on update? ALTER EXTENSION ... UPDATE does not repair rows left by earlier DROP SCHEMA / DROP OWNED; HISTORY.asc and the XD001 hint point to extension_drop__repair(). Options: leave as is, or RAISE WARNING in the update script when extension_drop__sanity_check() is non-empty. Auto-repair is not recommended: it could discard cleanup SQL that never ran.
3. Structure for the follow-up PR. A follow-up (non-superuser DROP EXTENSION fix using a SECURITY DEFINER helper) depends on this PR. Per ai/PR.md, a dependent PR needs a decision on structure: (a) push when-tag and the follow-up branch to this repo and open a real stack, (b) open the follow-up as a draft against master with this PR's commits in its diff, or (c) wait for this PR to merge.

[github-actions]

Claude finished @jnasbyupgrade's task in 16s —— View job

---

Review of PR #55: fire the event trigger for DROP SCHEMA and DROP OWNED

I read the full diff: the install and update SQL, README/HISTORY, and the three new tests plus the helper. I did not build the extension or run the tests. The code-review skill failed to run, so this is a manual review.

I found no bugs.

• Install and update paths match. Both use the same function body and the same WHEN tag IN ('DROP EXTENSION', 'DROP SCHEMA', 'DROP OWNED'). The update script drops and recreates the trigger, which is needed because ALTER EVENT TRIGGER can't change WHEN.
• Early return. The function returns before it reads extension_drop__commands or runs the sanity assert, unless pg_event_trigger_dropped_objects() contains an extension row. Non-superuser and stale-row cases are covered by unrelated_drops.sql.
• record variable. It keeps the function from needing the table type before the early return.
• Docs. extension_drop__repair() in HISTORY exists (sql/extension_drop.sql:159). The README and HISTORY text match the behavior.

Minor notes (no action required)

• The PR description is accurate now. The earlier "function unchanged" wording is gone.
• The tests create a SUPERUSER role and a plain role, and they grant CREATE ON DATABASE. These are safe only if the test transaction is rolled back, so I'd keep it that way. I didn't check test/finish.sql.
• DROP OWNED BY for a role that owns other objects still relies on the registered SQL running while those objects exist. That is the same ordering as DROP EXTENSION, so it is fine.