sql_drop Event Trigger

Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:

So please find attached to this email an implementation of the sql_drop
event trigger, that refrains on exposing any new information to the
users.

Already a v1 of that patch, per comments from Álvaro I reuse the
ObjectAddresses facility rather than building my own List of object
addresses.

Note that with that in place it's now easy to also add support for the
DROP OWNED BY command, but that's left for a future patch as I expect
some amount of discussion to go about it.

Also, I removed the code that was doing de-deduplication of the object
addresses we collect, now trusting performMultipleDeletions() not to
screw us up. There's a use case that needs particular attention here,
though:

DROP TABLE foo, foo;

I'm not sure we want to deduplicate foo in the pg_dropped_objects()
output in that case, so I've not done so in this version of the patch.
Also, Álvaro is concerned that the cost of deduplicating might be higher
than what we want to take here.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Wed, Jan 30, 2013 at 12:59 PM, Dimitri Fontaine
<dimitri@2ndquadrant.fr> wrote:

Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:

So please find attached to this email an implementation of the sql_drop
event trigger, that refrains on exposing any new information to the
users.

Already a v1 of that patch, per comments from Álvaro I reuse the
ObjectAddresses facility rather than building my own List of object
addresses.

Note that with that in place it's now easy to also add support for the
DROP OWNED BY command, but that's left for a future patch as I expect
some amount of discussion to go about it.

Also, I removed the code that was doing de-deduplication of the object
addresses we collect, now trusting performMultipleDeletions() not to
screw us up. There's a use case that needs particular attention here,
though:

DROP TABLE foo, foo;

I'm not sure we want to deduplicate foo in the pg_dropped_objects()
output in that case, so I've not done so in this version of the patch.
Also, Álvaro is concerned that the cost of deduplicating might be higher
than what we want to take here.

Taking a first look at this, I think the idea of pg_dropped_objects()
is really pretty clever. I like it. I assure we will end up with
several functions of this type eventually, so it might be good to
adopt some kind of distinguishing naming convention for this type of
function. pg_event_trigger_context_dropped_objects() seems far too
verbose, but that's the idea.

With this approach, there's no real need to introduce a new event
type. We could just make ddl_command_end triggers able to use this,
and we're done. The point of sql_drop was that it would have been
called once *per dropped object*, not once per command. But,
actually, thinking about this, for something like Slony, exposing
pg_dropped_objects() to ddl_command_end triggers should be just as
good, and maybe a whole lot better, than what I was proposing.

Does it work for you to rip out sql_drop and just make
pg_dropped_objects() - perhaps renamed - available in ddl_command_end?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

Taking a first look at this, I think the idea of pg_dropped_objects()
is really pretty clever. I like it. I assure we will end up with
several functions of this type eventually, so it might be good to
adopt some kind of distinguishing naming convention for this type of
function. pg_event_trigger_context_dropped_objects() seems far too
verbose, but that's the idea.

Thanks. Agreed that we will have more of them. In the attached version 3
of the patch, it got renamed to pg_event_trigger_dropped_objects().

With this approach, there's no real need to introduce a new event
type. We could just make ddl_command_end triggers able to use this,
and we're done. The point of sql_drop was that it would have been
called once *per dropped object*, not once per command. But,

Well, from the beginning of the sql_drop discussion, it's been clear
that it's meant to allow for users to easily attach their function to
any drop that might appear, whatever the command at origin of that drop.

So in the attached, I've made the sql_drop an event triggers called once
per object, which means that currently you only know an object is
getting DROPed as part of a DROP TABLE command.

actually, thinking about this, for something like Slony, exposing
pg_dropped_objects() to ddl_command_end triggers should be just as
good, and maybe a whole lot better, than what I was proposing.

It also changes the protocol to use for getting at the information
related to the objects. I think we will have to have the out parameters
of the function to grow to include the next information we're going to
make available to TG_* variables in the next patch of the series.

Does it work for you to rip out sql_drop and just make
pg_dropped_objects() - perhaps renamed - available in ddl_command_end?

I did rename pg_dropped_objects() and it's now available in
ddl_command_end, but I kept the new "sql_drop" event, which is now
called once per object dropped, as intended (but without any useful
information yet).

What do you think?
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

On Mon, Feb 4, 2013 at 11:59 AM, Dimitri Fontaine
<dimitri@2ndquadrant.fr> wrote:

Thanks. Agreed that we will have more of them. In the attached version 3
of the patch, it got renamed to pg_event_trigger_dropped_objects().

Works for me.

With this approach, there's no real need to introduce a new event
type. We could just make ddl_command_end triggers able to use this,
and we're done. The point of sql_drop was that it would have been
called once *per dropped object*, not once per command. But,

Well, from the beginning of the sql_drop discussion, it's been clear
that it's meant to allow for users to easily attach their function to
any drop that might appear, whatever the command at origin of that drop.

What precludes us from doing that in ddl_command_end? ISTM we can
just extend the ddl_command_start/end triggers to a slightly broader
range of commands and be done with it.

actually, thinking about this, for something like Slony, exposing
pg_dropped_objects() to ddl_command_end triggers should be just as
good, and maybe a whole lot better, than what I was proposing.

It also changes the protocol to use for getting at the information
related to the objects. I think we will have to have the out parameters
of the function to grow to include the next information we're going to
make available to TG_* variables in the next patch of the series.

Does it work for you to rip out sql_drop and just make
pg_dropped_objects() - perhaps renamed - available in ddl_command_end?

I did rename pg_dropped_objects() and it's now available in
ddl_command_end, but I kept the new "sql_drop" event, which is now
called once per object dropped, as intended (but without any useful
information yet).

What do you think?

Well, having spent a year or more trying to convince you that we need
sql_drop - mostly because of the complexities of passing an array of
arguments to the trigger function - I now think we don't, because the
pg_event_trigger_dropped_objects() bit solves that problem rather
elegantly. It seems to me with just a little bit of hacking we should
be able to make this work by adding that function and changing nothing
else. I might be wrong, of course.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

Well, having spent a year or more trying to convince you that we need
sql_drop - mostly because of the complexities of passing an array of
arguments to the trigger function - I now think we don't, because the
pg_event_trigger_dropped_objects() bit solves that problem rather
elegantly. It seems to me with just a little bit of hacking we should
be able to make this work by adding that function and changing nothing
else. I might be wrong, of course.

It's not exactly like we don't need to add anything else than just the
function to support the feature, but it's not that much either. Please
see attached.

doc/src/sgml/event-trigger.sgml | 15 +-
doc/src/sgml/func.sgml | 48 ++++++-
src/backend/access/transam/xact.c | 8 +-
src/backend/catalog/dependency.c | 38 +----
src/backend/commands/event_trigger.c | 123 ++++++++++++++++-
src/backend/tcop/utility.c | 28 +++-
src/include/catalog/dependency.h | 31 ++++-
src/include/catalog/pg_proc.h | 4 +-
src/include/commands/event_trigger.h | 21 +++
src/include/utils/builtins.h | 3 +
src/test/regress/expected/event_trigger.out | 67 ++++++++-
src/test/regress/sql/event_trigger.sql | 39 +++++-
12 files changed, 374 insertions(+), 51 deletions(-)

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

And already a v1.

Álvaro did spot a line I did remove by mistake in the docs, and some
extra whitespace changes that pgindent will change anyway and that as
such I shouldn't force you to read and discard.

It's a 3 lines change set from before.

Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:

Robert Haas <robertmhaas@gmail.com> writes:

Well, having spent a year or more trying to convince you that we need
sql_drop - mostly because of the complexities of passing an array of
arguments to the trigger function - I now think we don't, because the
pg_event_trigger_dropped_objects() bit solves that problem rather
elegantly. It seems to me with just a little bit of hacking we should
be able to make this work by adding that function and changing nothing
else. I might be wrong, of course.

It's not exactly like we don't need to add anything else than just the
function to support the feature, but it's not that much either. Please
see attached.

doc/src/sgml/event-trigger.sgml | 15 +-
doc/src/sgml/func.sgml | 48 ++++++-
src/backend/access/transam/xact.c | 8 +-
src/backend/catalog/dependency.c | 38 +----
src/backend/commands/event_trigger.c | 123 ++++++++++++++++-
src/backend/tcop/utility.c | 28 +++-
src/include/catalog/dependency.h | 31 ++++-
src/include/catalog/pg_proc.h | 4 +-
src/include/commands/event_trigger.h | 21 +++
src/include/utils/builtins.h | 3 +
src/test/regress/expected/event_trigger.out | 67 ++++++++-
src/test/regress/sql/event_trigger.sql | 39 +++++-
12 files changed, 374 insertions(+), 51 deletions(-)

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Robert Haas escribió:

On Mon, Feb 4, 2013 at 11:59 AM, Dimitri Fontaine
<dimitri@2ndquadrant.fr> wrote:

Thanks. Agreed that we will have more of them. In the attached version 3
of the patch, it got renamed to pg_event_trigger_dropped_objects().

Works for me.

With this approach, there's no real need to introduce a new event
type. We could just make ddl_command_end triggers able to use this,
and we're done. The point of sql_drop was that it would have been
called once *per dropped object*, not once per command. But,

Well, from the beginning of the sql_drop discussion, it's been clear
that it's meant to allow for users to easily attach their function to
any drop that might appear, whatever the command at origin of that drop.

What precludes us from doing that in ddl_command_end? ISTM we can
just extend the ddl_command_start/end triggers to a slightly broader
range of commands and be done with it.

I thought there was the idea that the list of objects to drop was to be
acquired before actually doing the deletion; so that the trigger
function could, for instance, get the name of the table being dropped.
I don't see that it works if we only provide
pg_event_trigger_dropped_objects to ddl_command_end events. Am I
missing something?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Dimitri Fontaine escribió:

And already a v1.

Álvaro did spot a line I did remove by mistake in the docs, and some
extra whitespace changes that pgindent will change anyway and that as
such I shouldn't force you to read and discard.

The bigger change I mentioned was the stuff in dependency.c -- I wasn't
too happy about exposing the whole ObjectAddresses stuff to the outside
world. The attached version only exposes simple accessors to let an
external user of that to iterate on such arrays. Some more commentary
is probably needed on those new functions. Also, if we're going to
extend things in this way we probably need to get "extra" out alongside
the object array itself.

A larger issue with the patch is handling of subxacts. A quick test
doesn't reveal any obvious misbehavior, but having the list of objects
dropped by a global variable might be problematic. What if, say, the
event trigger function does something funny in an EXCEPTION block and it
fails? Some clever test case is needed here, I think. Also, if we
reset the variable at EOXact, do we also need to do something at
EOSubXact?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

I thought there was the idea that the list of objects to drop was to be
acquired before actually doing the deletion; so that the trigger
function could, for instance, get the name of the table being dropped.
I don't see that it works if we only provide
pg_event_trigger_dropped_objects to ddl_command_end events. Am I
missing something?

Tom and Robert have been rightfully insisting on how delicate it has
been to come up with the right behavior for performMultipleDeletions,
and that's not something we can easily reorganise.

So the only way to get at the information seems to be what Robert
insisted that I do, that is storing the information about the objects
being dropped for later processing.

Of course, later processing means that the objects are already dropped
and that you can't do much. The idea is to provide more than just the
OID of the object, we have yet to decide if adding a catalog cache
lookup within performMultipleDeletions() is ok. If it is, we will extend
the pg_event_trigger_dropped_objects() definition to also return the
object name and its schema name, at a minimum.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

The bigger change I mentioned was the stuff in dependency.c -- I wasn't
too happy about exposing the whole ObjectAddresses stuff to the outside
world. The attached version only exposes simple accessors to let an
external user of that to iterate on such arrays. Some more commentary
is probably needed on those new functions. Also, if we're going to
extend things in this way we probably need to get "extra" out alongside
the object array itself.

Thanks for that.

I added some comments on those new functions, and made it so that we
call get_object_addresses_numelements() only once per loop rather than
at each step in the loop. See attached.

A larger issue with the patch is handling of subxacts. A quick test
doesn't reveal any obvious misbehavior, but having the list of objects
dropped by a global variable might be problematic. What if, say, the
event trigger function does something funny in an EXCEPTION block and it
fails? Some clever test case is needed here, I think. Also, if we
reset the variable at EOXact, do we also need to do something at
EOSubXact?

Now that you mention it, the case I'd be worried about is:

BEGIN;
SAVEPOINT a;
DROP TABLE foo;
ROLLBACK TO SAVEPOINT a;
DROP TABLE bar;
COMMIT;

As we currently have no support for on-commit triggers or the like, the
user function is going to run "as part" of the DROP TABLE foo; command,
and its effects are all going to disappear at the next ROLLBACK TO
SAVEPOINT anyway.

If the event trigger user function fails in an EXCEPTION block, I
suppose that the whole transaction is going to get a ROLLBACK, which
will call AbortTransaction() or CleanupTransaction(), which will reset
the static variable EventTriggerSQLDropInProgress. And the list itself
is gone away with the memory context reset.

I think the only missing detail is to force EventTriggerSQLDropList back
to NULL from within AtEOXact_EventTrigger(), and I've done so in the
attached. As we're only looking at the list when the protecting boolean
is true, I don't think it's offering anything else than clarity, which
makes it worthwile already.

You will find both the patch-on-top-of-your-patch (named .2.b) and the
new whole patch attached (named .3), as it makes things way easier IME.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

Dimitri Fontaine escribió:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

I thought there was the idea that the list of objects to drop was to be
acquired before actually doing the deletion; so that the trigger
function could, for instance, get the name of the table being dropped.
I don't see that it works if we only provide
pg_event_trigger_dropped_objects to ddl_command_end events. Am I
missing something?

Tom and Robert have been rightfully insisting on how delicate it has
been to come up with the right behavior for performMultipleDeletions,
and that's not something we can easily reorganise.

Well, I don't necessarily suggest that. But how about something like
this in performMultipleDeletions:

/*
* Fire event triggers for all objects to be dropped
*/
if (EventTriggerSQLDropInProgress)
{
for (i = 0; i < targetObjects->numrefs; i++)
{
ObjectAddress *thisobj;

thisobj = targetObjects->refs + i;

if (EventTriggerSQLDropInProgress &&
EventTriggerSupportsObjectType(getObjectClass(thisobj)))
{
add_exact_object_address(thisobj, EventTriggerSQLDropList);
}
}

/* invoke sql_drop triggers */
EventTriggerSQLDrop();

/* EventTriggerSQLDropList remains set for ddl_command_end triggers */
}

/* and delete them */
for (i = 0; i < targetObjects->numrefs; i++)
{
ObjectAddress *thisobj;

thisobj = targetObjects->refs + i;

deleteOneObject(thisobj, &depRel, flags);
}

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Well, I don't necessarily suggest that. But how about something like
this in performMultipleDeletions:

[edited snippet of code]

/* invoke sql_drop triggers */
EventTriggerSQLDrop();

/* EventTriggerSQLDropList remains set for ddl_command_end triggers */
}

/* and delete them */
for (i = 0; i < targetObjects->numrefs; i++)

...

deleteOneObject(thisobj, &depRel, flags);

My understanding of Tom and Robert comments is that it is very unsafe to
run random user code at this point, so that can not be an Event Trigger
call point.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Dimitri Fontaine escribió:

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Well, I don't necessarily suggest that. But how about something like
this in performMultipleDeletions:

[edited snippet of code]

/* invoke sql_drop triggers */
EventTriggerSQLDrop();

/* EventTriggerSQLDropList remains set for ddl_command_end triggers */
}

/* and delete them */
for (i = 0; i < targetObjects->numrefs; i++)

...

deleteOneObject(thisobj, &depRel, flags);

My understanding of Tom and Robert comments is that it is very unsafe to
run random user code at this point, so that can not be an Event Trigger
call point.

Hmm, quoth
/messages/by-id/23345.1358476518@sss.pgh.pa.us :

I'd really like to get to a point where we can
define things as happening like this:

* collect information needed to interpret the DDL command
(lookup and lock objects, etc)
* fire "before" event triggers, if any (and if so, recheck info)
* do the command
* fire "after" event triggers, if any

Note that in the snippet I posted above objects have already been looked
up and locked (first phase above).

One thing worth considering, of course, is the "if so, recheck info"
parenthical remark; for example, what happens if the called function
decides to, say, add a column to every dropped table? Or, worse, what
happens if the event trigger function adds a column to table "foo_audit"
when table "foo" is dropped, and you happen to drop both in the same
command. Also, what if the function decides to drop table "foo_audit"
when table "foo" is dropped? I think these things are all worth adding
to a regress test for this feature, to ensure that behavior is sane, and
also to verify that system catalogs after this remains consistent (for
example we don't leave dangling pg_attribute rows, etc). Maybe the
answer to all this is to run the lookup algorithm all over again and
ensure that the two lists are equal, and throw an error otherwise, i.e.
all scenarios above should be considered unsupported. That seems
safest.

But I don't think "code structure convenience" is the only reason to do
things this way instead of postponing firing the trigger until the end.
I think complete support for drop event triggers really needs to have
the objects to be dropped still in catalogs, so that they can be looked
up; for instance, user code might want to check the names of the
columns and take particular actions if particular names or particular
types are present. That doesn't seem an easy thing to do if all you get
is pg_dropped_objects(), because how do you know which columns belong to
which tables?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Hmm, quoth
/messages/by-id/23345.1358476518@sss.pgh.pa.us :

I'd really like to get to a point where we can
define things as happening like this:

* collect information needed to interpret the DDL command
(lookup and lock objects, etc)
* fire "before" event triggers, if any (and if so, recheck info)
* do the command
* fire "after" event triggers, if any

Note that in the snippet I posted above objects have already been looked
up and locked (first phase above).

Ok, I like what you did, but what you did is reinstall the "sql_drop"
event and is not complete, as you mention we have some hard problems to
solve there.

But I don't think "code structure convenience" is the only reason to do
things this way instead of postponing firing the trigger until the end.
I think complete support for drop event triggers really needs to have
the objects to be dropped still in catalogs, so that they can be looked
up; for instance, user code might want to check the names of the
columns and take particular actions if particular names or particular
types are present. That doesn't seem an easy thing to do if all you get
is pg_dropped_objects(), because how do you know which columns belong to
which tables?

Agreed.

In terms of Robert's reviewing, though, I think you're talking about
another patch entirely, that will get worked on in the 9.4 cycle.

And in my terms, doing all that work now is useless anyway because we
are not exposing any object specific information that allow the user to
do any actual catalog lookup anyway, yet.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Feb 5, 2013 at 10:42 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:

A larger issue with the patch is handling of subxacts. A quick test
doesn't reveal any obvious misbehavior, but having the list of objects
dropped by a global variable might be problematic. What if, say, the
event trigger function does something funny in an EXCEPTION block and it
fails? Some clever test case is needed here, I think. Also, if we
reset the variable at EOXact, do we also need to do something at
EOSubXact?

This is an awfully good point, although I think the issue has to do
with command boundaries more than subtransactions. Suppose you create
two ddl_command_end event triggers, A and B. A contains a DROP IF
EXISTS command. Someone runs a toplevel DROP command. Now, A is
going to fire first, and that's going to recursively invoke A (which
will do nothing the second time) and then B; on return from B, we'll
finish running the event triggers for the toplevel command, executing
B again. If the list of dropped objects is stored in a global
variable, it seems like there are a number of ways this can go wrong.

I have not tested the actual behavior of the latest patch, but I think
we want to define things so that the
pg_event_trigger_dropped_objects() function returns, specifically, the
list of objects dropped by the command which caused the event trigger
to fire. In other words, in the above example, the first, recursive
invocation of B should see the object removed by A's DROP-IF-EXISTS,
and the second invocation should see the object removed by the
toplevel command.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

variable, it seems like there are a number of ways this can go wrong.

Yeah, I think the current behavior might be surprising.

I have not tested the actual behavior of the latest patch, but I think
we want to define things so that the
pg_event_trigger_dropped_objects() function returns, specifically, the
list of objects dropped by the command which caused the event trigger
to fire. In other words, in the above example, the first, recursive
invocation of B should see the object removed by A's DROP-IF-EXISTS,
and the second invocation should see the object removed by the
toplevel command.

I disagree with that. I don't see why the enclosing event trigger
shouldn't be aware of all the objects dropped by the command that just
ran to completion, *including* the effects of any event trigger fired
recursively or not.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Wed, Feb 6, 2013 at 9:36 AM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

variable, it seems like there are a number of ways this can go wrong.

Yeah, I think the current behavior might be surprising.

I have not tested the actual behavior of the latest patch, but I think
we want to define things so that the
pg_event_trigger_dropped_objects() function returns, specifically, the
list of objects dropped by the command which caused the event trigger
to fire. In other words, in the above example, the first, recursive
invocation of B should see the object removed by A's DROP-IF-EXISTS,
and the second invocation should see the object removed by the
toplevel command.

I disagree with that. I don't see why the enclosing event trigger
shouldn't be aware of all the objects dropped by the command that just
ran to completion, *including* the effects of any event trigger fired
recursively or not.

Well, that could result in some DROP events being reported more than
once, which I assume would be undesirable for someone hoping to use
this for replication.

(Eventually, we'll have to face the same problem for CREATE events, too.)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Dimitri Fontaine escribió:

Robert Haas <robertmhaas@gmail.com> writes:

I have not tested the actual behavior of the latest patch, but I think
we want to define things so that the
pg_event_trigger_dropped_objects() function returns, specifically, the
list of objects dropped by the command which caused the event trigger
to fire. In other words, in the above example, the first, recursive
invocation of B should see the object removed by A's DROP-IF-EXISTS,
and the second invocation should see the object removed by the
toplevel command.

I disagree with that. I don't see why the enclosing event trigger
shouldn't be aware of all the objects dropped by the command that just
ran to completion, *including* the effects of any event trigger fired
recursively or not.

Not sure about that. If the trigger records objects dropped in a table,
aren't they going to show up there twice if you do that?

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

I disagree with that. I don't see why the enclosing event trigger
shouldn't be aware of all the objects dropped by the command that just
ran to completion, *including* the effects of any event trigger fired
recursively or not.

Well, that could result in some DROP events being reported more than
once, which I assume would be undesirable for someone hoping to use
this for replication.

Any command might have an event trigger attached doing a DROP, so that
you don't know where to expect it, and it's well possible that in your
example both the event triggers have been installed by different tools.

--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers