[HACKERS] sql_drop event trigger vs buildfarm

Buildfarm critters friarbird and jaguarundi have been failing for 11 days, apparently due to the sql_drop event for event triggers feature. These are both FBSD machines, but more importantly are both building with -DCLOBBER_CACHE_ALWAYS. Tom complained about it at the time, see

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera writes: > Here's another idea --- have three columns, "type", "schema" (as in the > current patch and as shown above), and a third one for object identity. > > For tables and other objects that have simple names, the identity would > be their names. For columns, it'd be .. For > f

Re: [HACKERS] sql_drop Event Trigger

On Tue, Mar 5, 2013 at 5:37 PM, Alvaro Herrera wrote: > Okay, I added a couple of lines to skip reporting dropped temp schemas, > and to skip any objects belonging to any temp schema (not just my own, > note). Not posting a new version because the change is pretty trivial. > > Now, one last thing

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera escribió: > Now, one last thing that comes up is what about objects that don't have > straight names (such as pg_amop, pg_amproc, pg_default_acl etc etc), the > only thing you get is a catalog OID and an object OID ... but they are > pretty useless because by the time you get to the

Re: [HACKERS] sql_drop Event Trigger

Okay, I added a couple of lines to skip reporting dropped temp schemas, and to skip any objects belonging to any temp schema (not just my own, note). Not posting a new version because the change is pretty trivial. Now, one last thing that comes up is what about objects that don't have straight na

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera escribió: > 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. This seems to work. See attached; I like the result because there's no clutter and it supports all three cases w

Re: [HACKERS] sql_drop Event Trigger

On Mon, Mar 4, 2013 at 4:59 PM, Alvaro Herrera 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

Re: [HACKERS] sql_drop Event Trigger

On Mon, Mar 4, 2013 at 11:13 AM, Alvaro Herrera wrote: > Another question. If I do ALTER TABLE foo DROP COLUMN bar, do we need > to fire an event trigger for the dropped column? Right now we don't, > ISTM we should. And if we want that, then the above set of three > properties doesn't cut it.

Re: [HACKERS] sql_drop Event Trigger

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 an

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Alvaro Herrera writes: > > Do we want some more stuff provided by pg_dropped_objects? We now have > > classId, objectId, objectSubId, object name, schema name. One further > > thing I think we need is the object's type, i.e. a simple untranslated > > string "table",

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Alvaro Herrera writes: > > Another question. If I do ALTER TABLE foo DROP COLUMN bar, do we need > > to fire an event trigger for the dropped column? Right now we don't, > > ISTM we should. And if we want that, then the above set of three > > properties doesn't cut

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera writes: > I'm very unsure about that idea. In any case the proposed name seems > way too general. Maybe we could have a separate datatype for objects > being dropped, which would be specific to > pg_event_trigger_dropped_objects(), and not try to mix it with other > events' info;

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Alvaro Herrera writes: > > Do we want some more stuff provided by pg_dropped_objects? We now have > > classId, objectId, objectSubId, object name, schema name. One further > > thing I think we need is the object's type, i.e. a simple untranslated > > string "table",

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera writes: > Dimitri Fontaine escribió: > >> The good news is that the patch to do that has already been sent on this >> list, and got reviewed in details by Álvaro who did offer incremental >> changes. Version 3 of that patch is to be found in: >> >> http://www.postgresql.org/mess

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > The good news is that the patch to do that has already been sent on this > list, and got reviewed in details by Álvaro who did offer incremental > changes. Version 3 of that patch is to be found in: > > http://www.postgresql.org/message-id/m2fw19n1hr@2ndquadrant

Re: [HACKERS] sql_drop Event Trigger

On Thu, Feb 28, 2013 at 3:20 PM, Tom Lane wrote: > I think it's fairly obvious that > (1) dealing with a DROP only after it's happened is pretty limiting; > (2) allowing user-defined code to run mid-command is dangerous. > What's at issue is the tradeoff we make between these inescapable > facts,

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Tom Lane writes: > > I think it's fairly obvious that > > (1) dealing with a DROP only after it's happened is pretty limiting; > > (2) allowing user-defined code to run mid-command is dangerous. > > What's at issue is the tradeoff we make between these inescapable > >

Re: [HACKERS] sql_drop Event Trigger

Tom Lane writes: > I think it's fairly obvious that > (1) dealing with a DROP only after it's happened is pretty limiting; > (2) allowing user-defined code to run mid-command is dangerous. > What's at issue is the tradeoff we make between these inescapable > facts, and I'm not sure if we can get c

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera writes: > Robert Haas escribió: >> It seems to me that a better way to do this might be to look up the >> names of all the objects being dropped, as we get rid of them, and >> pass that information off to the ddl_command_end trigger via something >> like the pg_dropped_objects() fun

Re: [HACKERS] sql_drop Event Trigger

Robert Haas escribió: > On Thu, Feb 28, 2013 at 2:14 PM, Tom Lane wrote: > > Alvaro Herrera writes: > > Maybe down the road we'll conclude that there's no other way and we're > > willing to put up with an unsafe feature, but I don't want to take that > > step under time pressure to ship somethin

Re: [HACKERS] sql_drop Event Trigger

Tom Lane escribió: > Alvaro Herrera writes: > > Robert Haas escribi�: > >> I venture to guess that this is exactly the sort of thing that made > >> Tom argue upthread that we shouldn't be putting a firing point in the > >> middle of the drop operation. Any slip-ups here will result in > >> corrup

Re: [HACKERS] sql_drop Event Trigger

On Thu, Feb 28, 2013 at 2:14 PM, Tom Lane wrote: > Alvaro Herrera writes: >> Robert Haas escribió: >>> I venture to guess that this is exactly the sort of thing that made >>> Tom argue upthread that we shouldn't be putting a firing point in the >>> middle of the drop operation. Any slip-ups here

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera writes: > Robert Haas escribió: >> I venture to guess that this is exactly the sort of thing that made >> Tom argue upthread that we shouldn't be putting a firing point in the >> middle of the drop operation. Any slip-ups here will result in >> corrupt catalogs, and it's not exactl

Re: [HACKERS] sql_drop Event Trigger

Robert Haas escribió: > On Thu, Feb 21, 2013 at 12:52 PM, Alvaro Herrera > wrote: > > One funny thing I noticed is that if I add a column in a table being > > dropped, the targetObjects list does not change after the trigger has > > run. The reason for this is that the table's attributes are not

Re: [HACKERS] sql_drop Event Trigger

On Thu, Feb 21, 2013 at 12:52 PM, Alvaro Herrera wrote: >> What if the object that gets whacked around is one of the named >> objects rather than some dependency thereof? Suppose for example that >> the event trigger drops the same object that the user tried to drop. >> We need to error out clean

Re: [HACKERS] sql_drop Event Trigger

On Thu, Feb 21, 2013 at 12:47 PM, Alvaro Herrera wrote: > You're misunderstanding. If you do DROP EVENT TRIGGER, the DDL_DROP > event won't fire at all. So no matter how messed up your system is, you > can always fix it by simply dropping the event trigger. > > What I was saying is that if you h

Re: [HACKERS] sql_drop Event Trigger

Robert Haas escribió: > On Thu, Feb 21, 2013 at 12:25 PM, Robert Haas wrote: > >> There's also code to re-obtain the list of objects to drop after the > >> event trigger functions have run; the second list is compared to the > >> first one, and if they differ, an error is raised. > > > > This is d

Re: [HACKERS] sql_drop Event Trigger

Robert Haas escribió: > On Wed, Feb 20, 2013 at 10:48 PM, Alvaro Herrera > wrote: > > I have added some protections so that these do not fire on undesirable > > events (such as dropping an event trigger); also event triggers and > > other object types are filtered out of pg_dropped_objects, in cas

Re: [HACKERS] sql_drop Event Trigger

On Thu, Feb 21, 2013 at 12:25 PM, Robert Haas wrote: >> There's also code to re-obtain the list of objects to drop after the >> event trigger functions have run; the second list is compared to the >> first one, and if they differ, an error is raised. > > This is definitely an improvement. I am no

Re: [HACKERS] sql_drop Event Trigger

On Wed, Feb 20, 2013 at 10:48 PM, Alvaro Herrera wrote: > I have added some protections so that these do not fire on undesirable > events (such as dropping an event trigger); also event triggers and > other object types are filtered out of pg_dropped_objects, in case > something like DROP OWNED ha

Re: [HACKERS] sql_drop Event Trigger

On Sun, Feb 17, 2013 at 4:12 PM, Dimitri Fontaine wrote: >>> Well, a list of object OIDs is of exactly zero use once the command >>> has been carried out. So I don't think that that represents a useful >>> or even very testable feature on its own, if there's no provision to >>> fire user code whi

Re: [HACKERS] sql_drop Event Trigger

Robert Haas writes: > Well, there's this, upon which we surely have not achieved consensus: > > http://www.postgresql.org/message-id/ca+tgmobq6ngsxguihwqcygf0q+7y9zhnerepo3s1vswkknw...@mail.gmail.com Sub-Transaction Handling. I fail to come up with a regression test showing any problem here, and

Re: [HACKERS] sql_drop Event Trigger

On Thu, Feb 14, 2013 at 3:39 PM, Dimitri Fontaine wrote: > Robert Haas writes: >> Wait, I'm confused. I had a note to myself to come back and review >> this, but now that I look at it, I didn't think that patch was pending >> review. Alvaro, Tom, and I all made comments that seems to impinge >>

Re: [HACKERS] sql_drop Event Trigger

Robert Haas writes: > Wait, I'm confused. I had a note to myself to come back and review > this, but now that I look at it, I didn't think that patch was pending > review. Alvaro, Tom, and I all made comments that seems to impinge > upon that design rather heavily. No? The current design follo

Re: [HACKERS] sql_drop Event Trigger

On Mon, Feb 11, 2013 at 9:53 AM, Dimitri Fontaine wrote: > Robert Haas writes: >>> Robert, you specifically opposed to "sql_drop" and I just removed it >>> from the patch. What do you think now? Also, should that be a follow-up >>> patch to the current one for your reviewing purposes? >> >> Well,

Re: [HACKERS] sql_drop Event Trigger

Robert Haas writes: >> Robert, you specifically opposed to "sql_drop" and I just removed it >> from the patch. What do you think now? Also, should that be a follow-up >> patch to the current one for your reviewing purposes? > > Well, if it has a different firing point than ddl_command_end, then >

Re: [HACKERS] sql_drop Event Trigger

On Wed, Feb 6, 2013 at 12:28 PM, Christopher Browne wrote: > On Wed, Feb 6, 2013 at 12:05 PM, Robert Haas wrote: >> On Wed, Feb 6, 2013 at 9:44 AM, Dimitri Fontaine >> wrote: >>> Robert Haas writes: > I disagree with that. I don't see why the enclosing event trigger > shouldn't be awar

Re: [HACKERS] sql_drop Event Trigger

On Wed, Feb 6, 2013 at 11:30 AM, Dimitri Fontaine wrote: > Alvaro Herrera writes: >> Dimitri Fontaine escribió: >>> Tom Lane writes: >>> > I might be forgetting something, but doesn't dependency.c work by first >>> > constructing a list of all the objects it's going to drop, and only then >>> >

Re: [HACKERS] sql_drop Event Trigger

On Wed, Feb 6, 2013 at 12:05 PM, Robert Haas wrote: > On Wed, Feb 6, 2013 at 9:44 AM, Dimitri Fontaine > wrote: >> Robert Haas 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 c

Re: [HACKERS] sql_drop Event Trigger

On Wed, Feb 6, 2013 at 11:04 AM, Tom Lane wrote: > Dimitri Fontaine writes: >> Alvaro Herrera 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

Re: [HACKERS] sql_drop Event Trigger

On Wed, Feb 6, 2013 at 9:44 AM, Dimitri Fontaine wrote: > Robert Haas 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

Re: [HACKERS] sql_drop Event Trigger

Tom Lane writes: > Well, a list of object OIDs is of exactly zero use once the command > has been carried out. So I don't think that that represents a useful > or even very testable feature on its own, if there's no provision to > fire user code while the OIDs are still in the catalogs. For the

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine writes: > Tom Lane writes: >> I might be forgetting something, but doesn't dependency.c work by first >> constructing a list of all the objects it's going to drop, and only then >> dropping them? Could we inject a "pre deletion" event trigger call at >> the point where the list

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera writes: > Dimitri Fontaine escribió: >> Tom Lane writes: >> > I might be forgetting something, but doesn't dependency.c work by first >> > constructing a list of all the objects it's going to drop, and only then >> > dropping them? Could we inject a "pre deletion" event trigger ca

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Tom Lane writes: > > I might be forgetting something, but doesn't dependency.c work by first > > constructing a list of all the objects it's going to drop, and only then > > dropping them? Could we inject a "pre deletion" event trigger call at > > the point where the

Re: [HACKERS] sql_drop Event Trigger

Tom Lane writes: > I might be forgetting something, but doesn't dependency.c work by first > constructing a list of all the objects it's going to drop, and only then > dropping them? Could we inject a "pre deletion" event trigger call at > the point where the list is completed? What happens if t

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine writes: > Alvaro Herrera 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. > Tom and Robert have been ri

Re: [HACKERS] sql_drop Event Trigger

Robert Haas 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 DRO

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Robert Haas 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 t

Re: [HACKERS] sql_drop Event Trigger

On Wed, Feb 6, 2013 at 9:36 AM, Dimitri Fontaine wrote: > Robert Haas 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

Re: [HACKERS] sql_drop Event Trigger

Robert Haas 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()

Re: [HACKERS] sql_drop Event Trigger

On Tue, Feb 5, 2013 at 10:42 PM, Alvaro Herrera 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 do

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera writes: > Hmm, quoth > http://www.postgresql.org/message-id/23345.1358476...@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 loc

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Alvaro Herrera 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(); > > > >

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera 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 s

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine escribió: > Alvaro Herrera 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 wo

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera 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

Re: [HACKERS] sql_drop Event Trigger

Alvaro Herrera 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_tri

Re: [HACKERS] sql_drop Event Trigger

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 dependen

Re: [HACKERS] sql_drop Event Trigger

Robert Haas escribió: > On Mon, Feb 4, 2013 at 11:59 AM, Dimitri Fontaine > 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 nee

Re: [HACKERS] sql_drop Event Trigger

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 writes: > Robert Haas writes: >>

Re: [HACKERS] sql_drop Event Trigger

Robert Haas 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 ra

Re: [HACKERS] sql_drop Event Trigger

On Mon, Feb 4, 2013 at 11:59 AM, Dimitri Fontaine 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. W

Re: [HACKERS] sql_drop Event Trigger

Robert Haas 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 o

Re: [HACKERS] sql_drop Event Trigger

On Wed, Jan 30, 2013 at 12:59 PM, Dimitri Fontaine wrote: > Dimitri Fontaine 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

Re: [HACKERS] sql_drop Event Trigger

Dimitri Fontaine 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 L

[HACKERS] sql_drop Event Trigger

Hi, I took the liberty to create a new thread for $subject, because the main comments I've been receiving about Event Triggers at this point is how difficult it is to try and follow our discussions about them. In order for everybody interested to be able to easily get the important bits of inform