pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
BUG #15310: pg_upgrade dissociates event triggers from extensions
5+ messages / 4 participants
[nested] [flat]

* BUG #15310: pg_upgrade dissociates event triggers from extensions
@ 2018-08-07 00:13  PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 1 reply; 5+ messages in thread

From: PG Bug reporting form @ 2018-08-07 00:13 UTC (permalink / raw)
  To: pgsql-bugs@lists.postgresql.org; +Cc: nickbarnes01@gmail.com

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



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #15310: pg_upgrade dissociates event triggers from extensions
@ 2018-08-07 09:40  Haribabu Kommi <kommi.haribabu@gmail.com>
  parent: PG Bug reporting form <noreply@postgresql.org>
  0 siblings, 2 replies; 5+ messages in thread

From: Haribabu Kommi @ 2018-08-07 09:40 UTC (permalink / raw)
  To: nickbarnes01@gmail.com; pgsql-bugs@lists.postgresql.org

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

As per the above, a trigger is not depends on an extension, but an event
trigger
does. Do we need to support the same for trigger also?

Regards,
Haribabu Kommi
Fujitsu Australia

Attachments:

  [application/octet-stream] 0001-Dump-Event-trigger-extension-dependency.patch (1.0K, ../../CAJrrPGcNzZ1n-xNsRfpxt=c_vYWnDK7hd_z80p4G8Tr1G15wAA@mail.gmail.com/3-0001-Dump-Event-trigger-extension-dependency.patch)
  download | inline diff:
From 79d8bb20012f4e54e6bcabf0f64556e8a582ed31 Mon Sep 17 00:00:00 2001
From: Hari Babu <kommi.haribabu@gmail.com>
Date: Tue, 7 Aug 2018 19:35:37 +1000
Subject: [PATCH] Dump Event trigger extension dependency

The event trigger that are added through the extension
are getting dropped their dependency on extension when
the PostgreSQL version is upgraded to newer version.
---
 src/bin/pg_dump/pg_dump.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 20e8aedb19..7caf868744 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -17301,6 +17301,9 @@ dumpEventTrigger(Archive *fout, EventTriggerInfo *evtinfo)
 
 	appendPQExpBuffer(delqry, "DROP EVENT TRIGGER %s;\n",
 					  qevtname);
+	if (dopt->binary_upgrade)
+		binary_upgrade_extension_member(query, &evtinfo->dobj,
+										"EVENT TRIGGER", evtinfo->dobj.name, NULL);
 
 	if (evtinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
 		ArchiveEntry(fout, evtinfo->dobj.catId, evtinfo->dobj.dumpId,
-- 
2.18.0.windows.1



^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #15310: pg_upgrade dissociates event triggers from extensions
@ 2018-08-07 11:29  Michael Paquier <michael@paquier.xyz>
  parent: Haribabu Kommi <kommi.haribabu@gmail.com>
  1 sibling, 1 reply; 5+ messages in thread

From: Michael Paquier @ 2018-08-07 11:29 UTC (permalink / raw)
  To: Haribabu Kommi <kommi.haribabu@gmail.com>; +Cc: nickbarnes01@gmail.com; pgsql-bugs@lists.postgresql.org

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

Attachments:

  [application/pgp-signature] signature.asc (832B, ../../20180807112910.GB19883@paquier.xyz/2-signature.asc)
  download

^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #15310: pg_upgrade dissociates event triggers from extensions
@ 2018-08-07 18:40  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Haribabu Kommi <kommi.haribabu@gmail.com>
  1 sibling, 0 replies; 5+ messages in thread

From: Tom Lane @ 2018-08-07 18:40 UTC (permalink / raw)
  To: Haribabu Kommi <kommi.haribabu@gmail.com>; +Cc: nickbarnes01@gmail.com; pgsql-bugs@lists.postgresql.org

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




^ permalink  raw  reply  [nested|flat] 5+ messages in thread

* Re: BUG #15310: pg_upgrade dissociates event triggers from extensions
@ 2018-08-07 20:38  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Michael Paquier <michael@paquier.xyz>
  0 siblings, 0 replies; 5+ messages in thread

From: Tom Lane @ 2018-08-07 20:38 UTC (permalink / raw)
  To: Michael Paquier <michael@paquier.xyz>; +Cc: Haribabu Kommi <kommi.haribabu@gmail.com>; nickbarnes01@gmail.com; pgsql-bugs@lists.postgresql.org

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




^ permalink  raw  reply  [nested|flat] 5+ messages in thread


end of thread, other threads:[~2018-08-07 20:38 UTC | newest]

Thread overview: 5+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2018-08-07 00:13 BUG #15310: pg_upgrade dissociates event triggers from extensions PG Bug reporting form <noreply@postgresql.org>
2018-08-07 09:40 ` Haribabu Kommi <kommi.haribabu@gmail.com>
2018-08-07 11:29   ` Michael Paquier <michael@paquier.xyz>
2018-08-07 20:38     ` Tom Lane <tgl@sss.pgh.pa.us>
2018-08-07 18:40   ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox