Re: [HACKERS] sql_drop Event Triggerg
Alvaro Herrera alvhe...@2ndquadrant.com writes: Pushed, with some further minor changes. One not-so-minor change I Thanks a lot for all the work you did put into this patch! introduced was that pg_event_trigger_dropped_objects() now only works within a sql_drop event function. The reason I decided to do this was that if we don't have that protection, then it is possible to have a ddl_command_end trigger calling pg_event_trigger_dropped_objects(); and if there is an sql_drop trigger, then it'd return the list of dropped objects, but if there's no sql_drop trigger, it'd raise an error. That seemed surprising enough action-at-a-distance that some protection is warranted. +1 I hope that we can add another such function for ddl_command_start and ddl_command_end function to get at some object information from there, now that the support code is in place. That would allow making any use of the event trigger facility in 9.3 without having to write a C coded extension… which has been the goal for the last two cycles… 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
Re: [HACKERS] sql_drop Event Triggerg
Pushed, with some further minor changes. One not-so-minor change I introduced was that pg_event_trigger_dropped_objects() now only works within a sql_drop event function. The reason I decided to do this was that if we don't have that protection, then it is possible to have a ddl_command_end trigger calling pg_event_trigger_dropped_objects(); and if there is an sql_drop trigger, then it'd return the list of dropped objects, but if there's no sql_drop trigger, it'd raise an error. That seemed surprising enough action-at-a-distance that some protection is warranted. Thanks for all the review. -- Álvaro Herrerahttp://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
Re: [HACKERS] sql_drop Event Triggerg
Robert Haas escribió:
On Wed, Mar 20, 2013 at 5:42 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
Here's a new version of this patch, rebased on top of the new
pg_identify_object() stuff. Note that the regression test doesn't work
yet, because I didn't adjust to the new identity output definition (the
docs need work, too). But that's a simple change to do. I'm leaving
that for later.
I think this is getting there. A few things to think about:
Thanks.
- pg_event_trigger_dropped_objects seems to assume that
currentEventTriggerState will be pointing to the same list on every
call. But is that necessarily true? I'm thinking about a case where
someone opens a cursor in an event trigger and then tries to read from
that cursor later in the transaction. I think you might be able to
crash the server that way.
Well, no, because it uses materialized return mode, so there's no next
time --- the list is constructed completely before
pg_event_trigger_dropped_objects returns. So there's no such hole.
- I am not wild about the idea of propagating PG_TRY/PG_CATCH blocks
into yet more places. On Linux-x86 they are pretty cheap because
Linux doesn't need a system call to change the signal mask and x86 has
few registers that must be saved-and-restored, but elsewhere this can
be a performance problem. Now maybe ProcessUtility is not a
sufficiently-frequently called function for this to matter... but I'm
not sure. The alternative is to teach the error handling pathways
about this in somewhat greater detail, since the point of TRY/CATCH is
to cleanup things that the regular error handling stuff doesn't now
about.
I tried this and it doesn't work. The error pathways you speak about
would be the xact.c entry points to commit and abort transactions;
however, there's a problem with that because some of the commands that
ProcessUtility() deals with have their own transaction management
calls internally; so those would call CommitTransaction() and the
event trigger state would go away, and then when control gets back to
ProcessUtility there would be nothing to clean up. I think we could
ignore the problem, or install smarts in ProcessUtility to avoid calling
event_trigger.c when one of those commands is involved -- but this seems
to me a solution worse than the problem. So all in all I feel like the
macro on top of PG_TRY is the way to go.
Now there *is* one rather big performance problem in this patch, which
is that it turns on collection of object dropped data regardless of
there being event triggers that use the info at all. That's a serious
drawback and we're going to get complaints about it. So we need to do
something to fix that.
One idea that comes to mind is to add some more things to the grammar,
CREATE EVENT TRIGGER name ... WITH ('DROPPED OBJECTS');
or some such, so that when events happen for which any triggers have
that flag enabled, *then* collecting is activated, otherwise not. This
would be stored in a new column in pg_event_trigger (say evtsupport, a
char array much like proargmodes).
The sequence of (ahem) events goes like this:
ProcessUtility()
EventTriggerBeginCompleteQuery()
EventTriggerDDLCommandStart()
EventCacheLookup()
EventTriggerInvoke()
.. run whatever command we've been handed ...
EventTriggerDDLCommandEnd()
EventCacheLookup()
EventTriggerInvoke()
EventTriggerEndCompleteQuery()
So EventTriggerBeginCompleteQuery() will have to peek into the event
trigger cache for any ddl_command_end triggers that might apply, and see
if any of them has the flag for dropped objects. If it's there, then
enable dropped object collection.
--
Álvaro Herrerahttp://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
Re: [HACKERS] sql_drop Event Triggerg
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Now there *is* one rather big performance problem in this patch, which
is that it turns on collection of object dropped data regardless of
there being event triggers that use the info at all. That's a serious
drawback and we're going to get complaints about it. So we need to do
something to fix that.
One idea that comes to mind is to add some more things to the grammar,
CREATE EVENT TRIGGER name ... WITH ('DROPPED OBJECTS');
Uh ... surely we can just notice whether there's a trigger on the
object-drop event? I don't understand why we need *yet more*
mechanism here.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sql_drop Event Triggerg
Tom Lane escribió:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
Now there *is* one rather big performance problem in this patch, which
is that it turns on collection of object dropped data regardless of
there being event triggers that use the info at all. That's a serious
drawback and we're going to get complaints about it. So we need to do
something to fix that.
One idea that comes to mind is to add some more things to the grammar,
CREATE EVENT TRIGGER name ... WITH ('DROPPED OBJECTS');
Uh ... surely we can just notice whether there's a trigger on the
object-drop event? I don't understand why we need *yet more*
mechanism here.
There's no object-drop event, only ddl_command_end. From previous
discussion I understood we didn't want a separate event, so that's what
we've been running with.
However, I think previous discussions have conflated many different
things, and we've been slowly fixing them one by one; so perhaps at this
point it does make sense to have a new object-drop event. Let's discuss
it -- we would define it as taking place just before ddl_command_end,
and firing any time a command (with matching tag?) has called
performDeletion or performMultipleDeletions. Does that sound okay?
--
Álvaro Herrerahttp://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
Re: [HACKERS] sql_drop Event Triggerg
On Tue, Mar 26, 2013 at 3:02 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: I tried this and it doesn't work. The error pathways you speak about would be the xact.c entry points to commit and abort transactions; however, there's a problem with that because some of the commands that ProcessUtility() deals with have their own transaction management calls internally; so those would call CommitTransaction() and the event trigger state would go away, and then when control gets back to ProcessUtility there would be nothing to clean up. I think we could ignore the problem, or install smarts in ProcessUtility to avoid calling event_trigger.c when one of those commands is involved -- but this seems to me a solution worse than the problem. So all in all I feel like the macro on top of PG_TRY is the way to go. I see. :-( Now there *is* one rather big performance problem in this patch, which is that it turns on collection of object dropped data regardless of there being event triggers that use the info at all. That's a serious drawback and we're going to get complaints about it. So we need to do something to fix that. Really? Who is going to care about that? Surely that overhead is quite trivial. -- 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
Re: [HACKERS] sql_drop Event Triggerg
Robert Haas escribió: On Tue, Mar 26, 2013 at 3:02 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Now there *is* one rather big performance problem in this patch, which is that it turns on collection of object dropped data regardless of there being event triggers that use the info at all. That's a serious drawback and we're going to get complaints about it. So we need to do something to fix that. Really? Who is going to care about that? Surely that overhead is quite trivial. I don't think it is, because it involves syscache lookups for each object being dropped, many extra pallocs, etc. Surely that's many times bigger than the PG_TRY overhead you were worried about. -- Álvaro Herrerahttp://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
Re: [HACKERS] sql_drop Event Triggerg
On Wed, Mar 20, 2013 at 5:42 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Here's a new version of this patch, rebased on top of the new pg_identify_object() stuff. Note that the regression test doesn't work yet, because I didn't adjust to the new identity output definition (the docs need work, too). But that's a simple change to do. I'm leaving that for later. I think this is getting there. A few things to think about: - pg_event_trigger_dropped_objects seems to assume that currentEventTriggerState will be pointing to the same list on every call. But is that necessarily true? I'm thinking about a case where someone opens a cursor in an event trigger and then tries to read from that cursor later in the transaction. I think you might be able to crash the server that way. - I am not wild about the idea of propagating PG_TRY/PG_CATCH blocks into yet more places. On Linux-x86 they are pretty cheap because Linux doesn't need a system call to change the signal mask and x86 has few registers that must be saved-and-restored, but elsewhere this can be a performance problem. Now maybe ProcessUtility is not a sufficiently-frequently called function for this to matter... but I'm not sure. The alternative is to teach the error handling pathways about this in somewhat greater detail, since the point of TRY/CATCH is to cleanup things that the regular error handling stuff doesn't now about. -- 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
Re: [HACKERS] sql_drop Event Triggerg
Here's a new version of this patch, rebased on top of the new
pg_identify_object() stuff. Note that the regression test doesn't work
yet, because I didn't adjust to the new identity output definition (the
docs need work, too). But that's a simple change to do. I'm leaving
that for later.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
*** a/doc/src/sgml/event-trigger.sgml
--- b/doc/src/sgml/event-trigger.sgml
***
*** 36,43
The literalddl_command_start/ event occurs just before the
execution of a literalCREATE/, literalALTER/, or literalDROP/
command. As an exception, however, this event does not occur for
! DDL commands targeting shared objects - databases, roles, and tablespaces
! - or for command targeting event triggers themselves. The event trigger
mechanism does not support these object types.
literalddl_command_start/ also occurs just before the execution of a
literalSELECT INTO/literal command, since this is equivalent to
--- 36,43
The literalddl_command_start/ event occurs just before the
execution of a literalCREATE/, literalALTER/, or literalDROP/
command. As an exception, however, this event does not occur for
! DDL commands targeting shared objects mdash; databases, roles, and tablespaces
! mdash; or for command targeting event triggers themselves. The event trigger
mechanism does not support these object types.
literalddl_command_start/ also occurs just before the execution of a
literalSELECT INTO/literal command, since this is equivalent to
***
*** 46,51
--- 46,61
/para
para
+ To list all objects that have been deleted as part of executing a
+ command, use the set returning
+ function literalpg_event_trigger_dropped_objects()/ from
+ your literalddl_command_end/ event trigger code (see
+ xref linkend=functions-event-triggers). Note that
+ the trigger is executed after the objects have been deleted from the
+ system catalogs, so it's not possible to look them up anymore.
+/para
+
+para
Event triggers (like other functions) cannot be executed in an aborted
transaction. Thus, if a DDL command fails with an error, any associated
literalddl_command_end/ triggers will not be executed. Conversely,
***
*** 433,438
--- 443,453
entry align=centerliteralX/literal/entry
/row
row
+ entry align=leftliteralDROP OWNED/literal/entry
+ entry align=centerliteralX/literal/entry
+ entry align=centerliteralX/literal/entry
+/row
+row
entry align=leftliteralDROP RULE/literal/entry
entry align=centerliteralX/literal/entry
entry align=centerliteralX/literal/entry
*** a/doc/src/sgml/func.sgml
--- b/doc/src/sgml/func.sgml
***
*** 15980,15983 FOR EACH ROW EXECUTE PROCEDURE suppress_redundant_updates_trigger();
--- 15980,16033
xref linkend=SQL-CREATETRIGGER.
/para
/sect1
+
+ sect1 id=functions-event-triggers
+titleEvent Trigger Functions/title
+
+indexterm
+ primarypg_event_trigger_dropped_objects/primary
+/indexterm
+
+para
+ Currently productnamePostgreSQL/ provides one built-in event trigger
+ helper function, functionpg_event_trigger_dropped_objects/, which
+ lists all object dropped by the command in whose literalddl_command_end/
+ event it is called. If the function is run in a context other than a
+ literalddl_command_end/ event trigger function, or if it's run in the
+ literalddl_command_end/ event of a command that does not drop objects,
+ it will return the empty set.
+ /para
+
+para
+ The functionpg_event_trigger_dropped_objects/ function can be used
+ in an event trigger like this:
+ programlisting
+ CREATE FUNCTION test_event_trigger_for_drops()
+ RETURNS event_trigger LANGUAGE plpgsql AS $$
+ DECLARE
+ obj record;
+ BEGIN
+ FOR obj IN SELECT * FROM pg_event_trigger_dropped_objects()
+ LOOP
+ RAISE NOTICE '% dropped object: % %.% %',
+ tg_tag,
+ obj.object_type,
+ obj.schema_name,
+ obj.object_name,
+ obj.subobject_name;
+ END LOOP;
+ END
+ $$;
+ CREATE EVENT TRIGGER test_event_trigger_for_drops
+ON ddl_command_end
+EXECUTE PROCEDURE test_event_trigger_for_drops();
+ /programlisting
+ /para
+
+ para
+For more information about event triggers,
+see xref linkend=event-triggers.
+ /para
+ /sect1
+
/chapter
*** a/src/backend/catalog/dependency.c
--- b/src/backend/catalog/dependency.c
***
*** 257,262 performDeletion(const ObjectAddress *object,
--- 257,268
{
ObjectAddress *thisobj =
Re: [HACKERS] sql_drop Event Triggerg
On Tue, Mar 5, 2013 at 12:45 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hmm, maybe I should be considering a pair of macros instead -- UTILITY_START_DROP and UTILITY_END_DROP. I'll give this a try. Other ideas are welcome. That seems like a possibly promising idea. I do wonder how well any of this is going to scale. Presumably people are going to want similar things for CREATE and (hardest) ALTER. Seems like ProcessUtility() could get pretty messy and confusing. But I don't have a better idea, either. :-( -- 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
Re: [HACKERS] sql_drop Event Triggerg
Robert Haas escribió: On Tue, Mar 5, 2013 at 12:45 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hmm, maybe I should be considering a pair of macros instead -- UTILITY_START_DROP and UTILITY_END_DROP. I'll give this a try. Other ideas are welcome. That seems like a possibly promising idea. I do wonder how well any of this is going to scale. I did followup with a patch implementing that; did you see it? Presumably people are going to want similar things for CREATE and (hardest) ALTER. Seems like ProcessUtility() could get pretty messy and confusing. But I don't have a better idea, either. :-( Well, the first thing that we need to settle is the user interface. Normalized command string don't seem to cut it; requiring users to write SQL parsers is rather unfriendly IMHO. The current idea of having a function that returns objects affected by the command seems relatively sensible. For drops, it seems pretty straighforward so far. For CREATE it's probably somewhat more involved, but seems doable in principle (but yes, we're going to have to sprinkle ProcessUtility() with a lot of UTILITY_START/END_CREATE calls). Not sure about ALTER; maybe we will need a completely different idea to attack that. -- Álvaro Herrerahttp://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
Re: [HACKERS] sql_drop Event Triggerg
On Fri, Mar 8, 2013 at 9:18 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote:
Robert Haas escribió:
On Tue, Mar 5, 2013 at 12:45 PM, Alvaro Herrera
alvhe...@2ndquadrant.com wrote:
Hmm, maybe I should be considering a pair of macros instead --
UTILITY_START_DROP and UTILITY_END_DROP. I'll give this a try. Other
ideas are welcome.
That seems like a possibly promising idea. I do wonder how well any
of this is going to scale.
I did followup with a patch implementing that; did you see it?
No, sorry. Which thread is it on?
Presumably people are going to want
similar things for CREATE and (hardest) ALTER. Seems like
ProcessUtility() could get pretty messy and confusing. But I don't
have a better idea, either. :-(
Well, the first thing that we need to settle is the user interface.
Normalized command string don't seem to cut it; requiring users to write
SQL parsers is rather unfriendly IMHO.
I could not agree more. The format we're moving towards for dropped
objects can easily be converted back into SQL if you happen to want to
do that, but it's also much easier to process programatically than a
compared with a normalized command string. So I think we get the best
of both worlds with this design.
The current idea of having a
function that returns objects affected by the command seems relatively
sensible. For drops, it seems pretty straighforward so far. For CREATE
it's probably somewhat more involved, but seems doable in principle (but
yes, we're going to have to sprinkle ProcessUtility() with a lot of
UTILITY_START/END_CREATE calls).
Not sure about ALTER; maybe we will need a completely different idea to
attack that.
I am inclined to think that putting this logic in ProcessUtility isn't
scalable, even for CREATE, and even moreso for ALTER, unless we can
put it around everything in that function, rather than each command
individually. Suppose for example that on entry to that function we
simply did this:
if (isCompleteQuery)
++CompleteQueryNestingLevel;
...and at exit, we did the reverse. This could work a bit like the
GUC nesting level. When an object is dropped, we find the array slot
for the current nesting level, and it's, say, a List **, and we push a
new element onto that list. When somebody asks for a list of dropped
objects, we pull from the list for the current nesting level. When
decrementing the nesting level, we flush the list associated with the
old nesting level, if any.
if (isCompleteQuery)
{
list_free(dropped_objects_list[CompleteQueryNestingLevel];
dropped_objects_list[CompleteQueryNestingLevel] = NIL;
--CompleteQueryNestingLevel;
}
Now, if we want to support CREATE, we can just have a
created_objects_list array as well, and only minimal changes are
required here. If we want to support ALTER we can have an
altered_objects_list, and the only decision is what data to stuff into
the list elements.
I think this is a lot better than decorating each individual command,
which is already unweildly and bound to get worse. It is a bit of a
definitional change, because it implies that the list of dropped
objects is the list of objects dropped by the most-nearly-enclosing
DDL command, rather than the list of objects dropped by the
most-nearly-enclosing DDL command *that is capable of dropping
objects*. However, I'm inclined to think that's a better definition
anyway.
--
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
Re: [HACKERS] sql_drop Event Triggerg
Robert Haas escribió: On Fri, Mar 8, 2013 at 9:18 AM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Robert Haas escribió: On Tue, Mar 5, 2013 at 12:45 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hmm, maybe I should be considering a pair of macros instead -- UTILITY_START_DROP and UTILITY_END_DROP. I'll give this a try. Other ideas are welcome. That seems like a possibly promising idea. I do wonder how well any of this is going to scale. I did followup with a patch implementing that; did you see it? No, sorry. Which thread is it on? http://www.postgresql.org/message-id/20130305214218.gp9...@alvh.no-ip.org I think Gmail's feature of breaking threads when subject changes is an annoyance here. I somehow added a g at the end and later dropped it. I didn't remember that behavior of Gmail's. The current idea of having a function that returns objects affected by the command seems relatively sensible. For drops, it seems pretty straighforward so far. For CREATE it's probably somewhat more involved, but seems doable in principle (but yes, we're going to have to sprinkle ProcessUtility() with a lot of UTILITY_START/END_CREATE calls). Not sure about ALTER; maybe we will need a completely different idea to attack that. I am inclined to think that putting this logic in ProcessUtility isn't scalable, even for CREATE, and even moreso for ALTER, unless we can put it around everything in that function, rather than each command individually. Suppose for example that on entry to that function we simply did this: if (isCompleteQuery) ++CompleteQueryNestingLevel; ...and at exit, we did the reverse. This could work a bit like the GUC nesting level. Hmm, this seems an interesting idea to explore. -- Álvaro Herrerahttp://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
Re: [HACKERS] sql_drop Event Triggerg
Robert Haas escribió: On Mon, Mar 4, 2013 at 4:59 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Alvaro Herrera escribió: I think this is mostly ready to go in. I'll look at your docs, and unless there are more objections will commit later or early tomorrow. Actually it still needs a bit more work: the error messages in pg_event_trigger_dropped_object need to be reworked. It's a bit annoying that the function throws an error if the function is called in a CREATE command, rather than returning an empty set; or is it just me? That seems like a reasonable thing to change. I'm thinking just removing the check altogether, and if the list of objects dropped is empty, then the empty set is returned. That is, if you call the function directly in user-invoked SQL, you get empty; if you call the function in a CREATE event trigger function, you get empty. Since this makes the boolean drop_in_progress useless, it'd be removed as well. I do have a bit of concern about the save-and-restore logic for the dropped-object list -- it necessitates that the processing of every DDL command that can potentially drop objects be bracketed with a PG_TRY/PG_CATCH block. While that's relatively easy to guarantee today (but doesn't ALTER TABLE need similar handling?), it seems to me that it might get broken in the future. How hard would it be to pass this information up and down the call stack instead of doing it this way? This would be rather messy, I think; there are too many layers, and too many different functions. Or at least move the save/restore logic into something inside the deletion machinery itself so that new callers don't have to worry about it? Hmm, maybe this can be made to work, I'll see about it. -- Álvaro Herrerahttp://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
Re: [HACKERS] sql_drop Event Triggerg
Robert Haas escribió:
Or at least move the save/restore logic into something inside the
deletion machinery itself so that new callers don't have to worry
about it?
I don't think that works well; precisely the point of the
initialize/finalize pair of functions is to bracket the drop so that the
objects reported by the deletion machinery are stored in the right list.
I tried this macro:
/*
* Wrap a code fragment that executes a command that may drop database objects,
* so that the event trigger environment is appropriately setup.
*
* Note this macro will call EventTriggerDDLCommandEnd if the object type is
* supported; caller must make sure to call EventTriggerDDLCommandStart by
* itself.
*/
#define ExecuteDropCommand(isCompleteQuery, codeFragment, has_objtype, objtype)
\
do { \
slist_head _save_objlist; \
bool_supported; \
\
_supported = has_objtype ?
EventTriggerSupportsObjectType(objtype) : true; \
\
if (isCompleteQuery) \
{ \
EventTriggerInitializeDrop(_save_objlist); \
} \
PG_TRY(); \
{ \
codeFragment; \
if (isCompleteQuery _supported) \
{ \
EventTriggerDDLCommandEnd(parsetree); \
} \
} \
PG_CATCH(); \
{ \
if (isCompleteQuery _supported) \
{ \
EventTriggerFinalizeDrop(_save_objlist); \
} \
PG_RE_THROW(); \
} \
PG_END_TRY(); \
EventTriggerFinalizeDrop(_save_objlist); \
} while (0)
This looks nice in DropOwned:
case T_DropOwnedStmt:
if (isCompleteQuery)
EventTriggerDDLCommandStart(parsetree);
ExecuteDropCommand(isCompleteQuery,
DropOwnedObjects((DropOwnedStmt *)
parsetree),
false, 0);
break;
And it works for DropStmt too:
ExecuteDropCommand(isCompleteQuery,
switch (stmt-removeType)
{
case OBJECT_INDEX:
case OBJECT_TABLE:
case OBJECT_SEQUENCE:
case OBJECT_VIEW:
case OBJECT_MATVIEW:
case OBJECT_FOREIGN_TABLE:
RemoveRelations((DropStmt *) parsetree);
break;
default:
RemoveObjects((DropStmt *) parsetree);
break;
}, true, stmt-removeType);
but a rather serious problem is that pgindent refuses to work completely
on this (which is understandable, IMV). My editor doesn't like the
braces inside something that looks like a function call, either. We use
this pattern (a codeFragment being called by a macro) as
ProcessMessageList in inval.c, but the code fragments there are much
simpler.
And in AlterTable, using the macro would be much uglier because the code
fragment is more convoluted.
I'm not really sure if it's worth having the above macro if the only
caller is DropOwned.
Hmm, maybe I should be considering a pair of macros instead --
UTILITY_START_DROP and UTILITY_END_DROP. I'll give this a try. Other
ideas are welcome.
FWIW, I noticed that the AlterTable case lacks a call to DDLCommandEnd;
reporting that to Dimitri led to him noticing that DefineStmt also lacks
one. This is a simple bug in the already-committed implementation which
should probably be fixed separately from this patch.
--
Álvaro Herrerahttp://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
