Thread: BUG #15310: pg_upgrade dissociates event triggers from extensions
The following bug has been logged on the website: Bug reference: 15310 Logged by: Nick Barnes Email address: nickbarnes01@gmail.com PostgreSQL version: 10.4 Operating system: CentOS 7 Description: Hi, I have an extension which contains an event trigger. As expected, CREATE EXTENSION adds a pg_depend entry between the trigger and the extension. But after running pg_upgrade, this pg_depend entry is gone (and the extension's CREATE EVENT TRIGGER statement now shows up in the pg_dump output, causing the restore to fail with an "event trigger already exists" error). Reproduced for 9.5->9.6 and 9.6->10 upgrades with the attached script. Did I miss something, or is this a bug? If so, is an ALTER EXTENSION ... ADD EVENT TRIGGER command on the upgraded database enough to work around it? It solves my issue, but I'm not sure if there are other symptoms. Thanks, Nick Barnes export PGDATAOLD=~/9.5/data export PGDATANEW=~/10/data export PGBINOLD=/usr/pgsql-9.5/bin export PGBINNEW=/usr/pgsql-10/bin # Create extension files cat >$PGBINOLD/../share/extension/event_trigger_test.control <<EOF default_version = '1.0' EOF cat >$PGBINOLD/../share/extension/event_trigger_test--1.0.sql <<EOF CREATE FUNCTION t() RETURNS event_trigger LANGUAGE plpgsql AS 'BEGIN END'; CREATE EVENT TRIGGER t ON ddl_command_end EXECUTE PROCEDURE t(); EOF cp $PGBINOLD/../share/extension/event_trigger_test* $PGBINNEW/../share/extension/ # Set up old server $PGBINOLD/initdb -D $PGDATAOLD $PGBINOLD/pg_ctl start -w -D $PGDATAOLD $PGBINOLD/createdb test $PGBINOLD/psql test -c "CREATE EXTENSION event_trigger_test" # Upgrade $PGBINOLD/pg_ctl stop -D $PGDATAOLD $PGBINNEW/initdb -D $PGDATANEW $PGBINNEW/pg_upgrade $PGBINNEW/pg_ctl start -w -D $PGDATANEW # Dump/restore $PGBINNEW/createdb test2 $PGBINNEW/pg_dump test | $PGBINNEW/psql test2 # ERROR: event trigger "t" already exists
On Tue, Aug 7, 2018 at 10:14 AM PG Bug reporting form <noreply@postgresql.org> wrote:
Hi,
I have an extension which contains an event trigger. As expected, CREATE
EXTENSION adds a pg_depend entry between the trigger and the extension. But
after running pg_upgrade, this pg_depend entry is gone (and the extension's
CREATE EVENT TRIGGER statement now shows up in the pg_dump output, causing
the restore to fail with an "event trigger already exists" error).
Reproduced for 9.5->9.6 and 9.6->10 upgrades with the attached script.
Did I miss something, or is this a bug?
Yes, I feel it is a bug. During the dump of the event trigger during upgrade, it lost
the dependency on extension.
If so, is an ALTER EXTENSION ... ADD
EVENT TRIGGER command on the upgraded database enough to work around it? It
solves my issue, but I'm not sure if there are other symptoms.
Yes, the above command can solve your problem.
While checking this issue, why it is missed to dump the extension dependency,
I observed that there is call to the function binary_upgrade_extension_member()
to add extension dependency for particular types in the pg_dump.
DO_EVENT_TRIGGER
The above type as per the bug is losing the dependency, by adding a
function call in dumpEventTrigger() function the dependency issue is resolved.
Patch attached.
DO_SHELL_TYPE
I am not able to generate a scenario for the above type where it can loss the
extension dependency.
The other types that don't create extension dependency are as follows,
DO_INDEX, DO_STATSEXT, DO_RULE, DO_TRIGGER,
DO_POLICY, DO_PUBLICATION, DO_SUBSCRIPTION
does. Do we need to support the same for trigger also?
Regards,
Haribabu Kommi
Fujitsu Australia
Attachment
On Tue, Aug 07, 2018 at 07:40:24PM +1000, Haribabu Kommi wrote: > Yes, I feel it is a bug. During the dump of the event trigger during > upgrade, it lost > the dependency on extension. That sounds like a bug to me. Could you add more tests to src/test/modules/test_pg_dump with an event trigger part of an extension with a binary upgrade test? It would be good to check as well if the other object types you have mentioned correctly work or not. -- Michael
Attachment
Haribabu Kommi <kommi.haribabu@gmail.com> writes: > On Tue, Aug 7, 2018 at 10:14 AM PG Bug reporting form < > noreply@postgresql.org> wrote: >> I have an extension which contains an event trigger. As expected, CREATE >> EXTENSION adds a pg_depend entry between the trigger and the extension. But >> after running pg_upgrade, this pg_depend entry is gone (and the extension's >> CREATE EVENT TRIGGER statement now shows up in the pg_dump output, causing >> the restore to fail with an "event trigger already exists" error). >> Did I miss something, or is this a bug? > Yes, I feel it is a bug. Undoubtedly. I haven't checked your fix in detail yet but it looks plausible. I poked around to see if there were any other missing moving parts in extension membership support. Mostly we seem to be OK ... but I found out that CreateUserMapping contains a recordDependencyOnCurrentExtension call, even though there is no support in the grammar for ALTER EXTENSION ADD/DROP USER MAPPING, nor does pg_dump check for extension membership when dumping a user mapping. (So if someone did do a CREATE USER MAPPING in an extension, the membership would be recorded but then silently lost during pg_upgrade, just as with this bug.) I'm inclined to think that not supporting that is the correct thing: if we don't allow roles to be extension members, then user mappings for roles shouldn't be either. Unless someone can come up with a convincing use-case for that, I think we should remove the recordDependencyOnCurrentExtension call from CreateUserMapping. > Do we need to support the same for trigger also? No. Whole tables can be extension members, not properties of tables. regards, tom lane
Michael Paquier <michael@paquier.xyz> writes: > That sounds like a bug to me. Could you add more tests to > src/test/modules/test_pg_dump with an event trigger part of an extension > with a binary upgrade test? It doesn't seem especially useful to me to lock this particular barn door behind the horse. What would be great is a way of cross-checking that the grammar, pg_dump, and the various callers of recordDependencyOnCurrentExtension are all on the same page about which objects can be extension members. I see no way to automate that unfortunately :-( regards, tom lane