From: Tom Lane <tgl@sss.pgh.pa.us>
To: Haribabu Kommi <kommi.haribabu@gmail.com>
cc: nickbarnes01@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15310: pg_upgrade dissociates event triggers from extensions
In-reply-to: 
 <CAJrrPGcNzZ1n-xNsRfpxt=c_vYWnDK7hd_z80p4G8Tr1G15wAA@mail.gmail.com>
References: <153360083872.1395.4593932457718151600@wrigleys.postgresql.org>
 <CAJrrPGcNzZ1n-xNsRfpxt=c_vYWnDK7hd_z80p4G8Tr1G15wAA@mail.gmail.com>
Comments: In-reply-to Haribabu Kommi <kommi.haribabu@gmail.com>
	message dated "Tue, 07 Aug 2018 19:40:24 +1000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <27951.1533667213.1@sss.pgh.pa.us>
Date: Tue, 07 Aug 2018 14:40:13 -0400
Message-ID: <27952.1533667213@sss.pgh.pa.us>
Precedence: bulk

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