Thread: event trigger API documentation?
I'm having trouble finding documentation about how to write event triggers. The chapter in the documentation http://www.postgresql.org/docs/devel/static/event-triggers.html says they can be written in C or supported PLs, but does not explain it any further. Is there any documentation for it?
On Mon, Apr 15, 2013 at 11:57 PM, Peter Eisentraut <peter_e@gmx.net> wrote: > I'm having trouble finding documentation about how to write event > triggers. The chapter in the documentation > > http://www.postgresql.org/docs/devel/static/event-triggers.html > > says they can be written in C or supported PLs, but does not explain it > any further. Is there any documentation for it? That chapter has two subsections that may be useful, and there's a bit more here: http://www.postgresql.org/docs/devel/static/plpgsql-trigger.html#PLPGSQL-EVENT-TRIGGER Now that you mention it, though, it looks a little sparse. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 4/16/13 7:49 AM, Robert Haas wrote: > On Mon, Apr 15, 2013 at 11:57 PM, Peter Eisentraut <peter_e@gmx.net> wrote: >> I'm having trouble finding documentation about how to write event >> triggers. The chapter in the documentation >> >> http://www.postgresql.org/docs/devel/static/event-triggers.html >> >> says they can be written in C or supported PLs, but does not explain it >> any further. Is there any documentation for it? > > That chapter has two subsections that may be useful, and there's a bit > more here: > > http://www.postgresql.org/docs/devel/static/plpgsql-trigger.html#PLPGSQL-EVENT-TRIGGER > > Now that you mention it, though, it looks a little sparse. I'm specifically looking for C API documentation, along the lines of http://www.postgresql.org/docs/devel/static/trigger-interface.html. The current chapter on event triggers might as well be ripped out and folded into the CREATE EVENT TRIGGER reference page, because it explains nothing about programming those triggers.
Peter Eisentraut <peter_e@gmx.net> writes: > I'm specifically looking for C API documentation, along the lines of > http://www.postgresql.org/docs/devel/static/trigger-interface.html. > > The current chapter on event triggers might as well be ripped out and > folded into the CREATE EVENT TRIGGER reference page, because it explains > nothing about programming those triggers. I'm not sure about ripping it out, it does not sound like a good idea to me. It needs some addition and C level examples yes. The plan was to build a contrib module as an example, that would cancel any (supported) command you try to run by means of ereport(ERROR, …);. Then add that in pieces in the docs with details about what's going on. While the commit fest was still running didn't look like the right time to work on that. Beta looks like when to be working on that. What do you think about the proposal here? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes:
> I'm not sure about ripping it out, it does not sound like a good idea to
> me. It needs some addition and C level examples yes. The plan was to
> build a contrib module as an example, that would cancel any (supported)
> command you try to run by means of ereport(ERROR, …);. Then add that in
> pieces in the docs with details about what's going on.
> While the commit fest was still running didn't look like the right time
> to work on that. Beta looks like when to be working on that.
> What do you think about the proposal here?
We're not adding new contrib modules during beta. Expanding the
documentation seems like a fine beta-period activity, though.
regards, tom lane
On 4/17/13 5:41 AM, Dimitri Fontaine wrote: > I'm not sure about ripping it out, it does not sound like a good idea to > me. It needs some addition and C level examples yes. The plan was to > build a contrib module as an example, that would cancel any (supported) > command you try to run by means of ereport(ERROR, …);. Then add that in > pieces in the docs with details about what's going on. > > While the commit fest was still running didn't look like the right time > to work on that. Beta looks like when to be working on that. Well, if documentation had been available well before beta, other procedural languages might have gained support for event triggers. If it's not being documented, it might not happen very soon. It would have been good to have at least one untrusted language with event trigger support, so that you can hook in external auditing or logging systems. With the existing PL/pgSQL support, the possible actions are a bit limited.
Peter Eisentraut <peter_e@gmx.net> writes: > Well, if documentation had been available well before beta, other > procedural languages might have gained support for event triggers. If > it's not being documented, it might not happen very soon. It's been a moving target for the last two years, and until very recently what to document was not clear enough to spend any time on actually writing the docs. Please also note that the first series of patches did include the support code for all the core PL, but Robert didn't feel like commiting that and no other commiter did step up. I'm struggling to understand how to properly solve the problem here from an organisation perspective. Before beta was not the good time for the people involved, and was not the good time for other people to get involved. Beta is not the good time to fix what couldn't be done before. When are we supposed to work on the rough edges left when a patch went through 8 commit fests and so many discussions that it's quite hard indeed to step back and understand what's in and what's missing to make it sensible for the release? Maybe the right answer is to remove the documentation about event triggers completely for 9.3 and tell the users about them later when we have something else than just internal infrastructure. Now, if it's ok to add support to others PL, I can cook a patch from the bits I had done last year, the only work should be removing variables. > It would have been good to have at least one untrusted language with > event trigger support, so that you can hook in external auditing or > logging systems. With the existing PL/pgSQL support, the possible > actions are a bit limited. Well, you do realise that the only information you get passed down to the event trigger code explicitely are the event name and the command tag, and nothing else, right? If you have a use case that requires any other information, then documenting the event triggers will do nothing to help you implement it, you will need to code in C and go look at the backend sources. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 4/17/13 3:20 PM, Dimitri Fontaine wrote: >> It would have been good to have at least one untrusted language with >> > event trigger support, so that you can hook in external auditing or >> > logging systems. With the existing PL/pgSQL support, the possible >> > actions are a bit limited. > Well, you do realise that the only information you get passed down to > the event trigger code explicitely are the event name and the command > tag, and nothing else, right? Offhand, that seems about enough, but I'm just beginning to explore. Chances are, event triggers will end up somewhere near the top of the release announcements, so we should have a consistent message about what to do with them and how to use them. If for now, we say, we only support writing them in PL/pgSQL, and here is how to do that, and here are some examples, that's fine. But currently, it's not quite clear. Surely you had some use cases in mind when you set out to implement this. What were they, and where are we now in relation to them?
Peter Eisentraut <peter_e@gmx.net> writes: > Offhand, that seems about enough, but I'm just beginning to explore. I'm interested into hearing about any such use case… > Chances are, event triggers will end up somewhere near the top of the > release announcements, so we should have a consistent message about what > to do with them and how to use them. If for now, we say, we only > support writing them in PL/pgSQL, and here is how to do that, and here > are some examples, that's fine. But currently, it's not quite clear. I would prefer that we're silent about them for another (couple of?) release, because we didn't reach yet the feature set that I did consider the bare minimum. In my view 9.3 only has code infrastructure to prepare for the ability to implement Event Triggers later. That this infrastructure already allows you to do some things with it is like a proof of concept. > Surely you had some use cases in mind when you set out to implement > this. What were they, and where are we now in relation to them? I have mainly 4 use cases for them, and none of them are possible to implement in 9.3: - audit (separate log or audit tables for commited only actions) - ddl support for replications (trigger based, logicalrep.) - ddl extensibility - apt-get for extensions without dynamically loaded module The extension items is an example of the more general "ddl extensibility" item. Don't worry about ever seeing a patch to core for implementing it, the whole Event Trigger and Extension Templates exercise is meant to allow for coding that kind of crazy ideas out of core. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine escribió: > Peter Eisentraut <peter_e@gmx.net> writes: > > Offhand, that seems about enough, but I'm just beginning to explore. > > I'm interested into hearing about any such use case… > > > Chances are, event triggers will end up somewhere near the top of the > > release announcements, so we should have a consistent message about what > > to do with them and how to use them. If for now, we say, we only > > support writing them in PL/pgSQL, and here is how to do that, and here > > are some examples, that's fine. But currently, it's not quite clear. > > I would prefer that we're silent about them for another (couple of?) > release, You can be as much silent as you want in marketing materials (though maybe Berkus will disagree with you about being silent there), but it is not admissible to be silent in the documentation or pretend the feature is not there. Whatever got committed, however small, needs to be properly documented. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 4/18/13 5:05 AM, Dimitri Fontaine wrote: > Peter Eisentraut <peter_e@gmx.net> writes: >> > Offhand, that seems about enough, but I'm just beginning to explore. > I'm interested into hearing about any such use case… Without going into too many details (because I don't have them yet), I was thinking about triggering an external test suite whenever there is a schema change in the database.
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > You can be as much silent as you want in marketing materials (though > maybe Berkus will disagree with you about being silent there), but it is > not admissible to be silent in the documentation or pretend the feature > is not there. Whatever got committed, however small, needs to be > properly documented. Definitely, yes. The only questions in this thread are: - only docs or docs + contrib example? Tom said it's too late for the contrib example. - what about support for PLs other than C and PLpgSQL? It used to be part of the patch, and I don't understand well enough the development calendar to guess if I'm supposedto extract that from earlier patch or if that's too late for 9.3. I'm not sure what Peter's idea are wrt to thecalendar here. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Peter Eisentraut <peter_e@gmx.net> writes: > Without going into too many details (because I don't have them yet), I > was thinking about triggering an external test suite whenever there is a > schema change in the database. So if all you want to know about is that something did change in the schema to trigger your action, yes you can do it. I would go as far as to propose that you consider registering an event in a PGQ queue at the time when the ddl event occurs, so that you can have your test suite run be triggers from the outside of the database at its leisure. If you want to stay within PostgreSQL offering proper, a NOTIFY would do, and you can do that in PLpgSQL too. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Dimitri Fontaine escribió: > Alvaro Herrera <alvherre@2ndquadrant.com> writes: > > You can be as much silent as you want in marketing materials (though > > maybe Berkus will disagree with you about being silent there), but it is > > not admissible to be silent in the documentation or pretend the feature > > is not there. Whatever got committed, however small, needs to be > > properly documented. > > Definitely, yes. > > The only questions in this thread are: > > - only docs or docs + contrib example? > > Tom said it's too late for the contrib example. So there's already an answer to this question, isn't there. > - what about support for PLs other than C and PLpgSQL? > > It used to be part of the patch, and I don't understand well enough > the development calendar to guess if I'm supposed to extract that > from earlier patch or if that's too late for 9.3. I'm not sure > what Peter's idea are wrt to the calendar here. It seems far too late for more code at this stage. IMO anyway. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 4/18/13 11:31 AM, Dimitri Fontaine wrote: > The only questions in this thread are: > > - only docs or docs + contrib example? At this point, all that is appropriate is some documentation of the C API. If the contrib example you have in mind is short enough, it might as well become part of the example in the documentation.
On Wed, Apr 17, 2013 at 3:20 PM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Please also note that the first series of patches did include the > support code for all the core PL, but Robert didn't feel like commiting > that and no other commiter did step up. Of course, the chances of that would have been a lot higher had you actually resubmitted the patches for other PLs as separate patches as I suggested. It's asking a bit much to suppose that some other committer was going to go root back through the old patches, extract the portion that applied to the PL they know something about, revise it to match the other changes that were made subsequent to the last patch version that incorporated those changes, and then commit it, without so much as a CommitFest entry to point them in the right direction. > I'm struggling to understand how to properly solve the problem here from > an organisation perspective. Before beta was not the good time for the > people involved, and was not the good time for other people to get > involved. Beta is not the good time to fix what couldn't be done before. Event triggers are suffering from the same problem as materialized views and CRCs: stuff that gets committed in the last CommitFest tends to be big and buggy. We could fix that problem by having one more CommitFest where we only accepted small patches, but then people would just try to force large patches into it after all, on the theory that they were almost done in the previous CommitFest or weren't, really, all that big (attachment: 30kB patch file). No matter what we do, there's always going to be some pain around the end of the development cycle. It's painful to see work that feels nearly done get its release date bumped by a year. And yet it's also painful to squeeze it in and find that there are still loose ends that you just don't quite have time to fix. You can phrase that problem in a way that makes it sound like it's about project policy or committers being jerks, but I think it's mostly just that deadlines suck. I have yet to work in a development organization - EnterpriseDB included - that suffered any less agita around release deliverables than the PostgreSQL community does. There are always - always! - people who want to slip the schedule to fit more into the release, people who want to slip more into the release WITHOUT slipping the schedule (thus upping the defect count), and people who want to stick to the schedule at any cost (and even if it means dumping the feature personally requested by God Himself). And, all the intermediate positions, too. All of those camps are as well-represented on pgsql-hackers as anywhere else. It is not as if any patches submitted now are going to go away. We will presumably have a CommitFest sometime in the next couple of months during which whatever didn't make it into 9.3 can go into 9.4 (or maybe, as I suspect, 10.0). Peter's complaint is legitimate, but it's not a stop-ship issue for 9.3, and the next train will be along shortly. I know that you're disappointed in how much got done with this feature for 9.3, but I think it will have more use cases than you realize, and the next version can and will be even better. Sure, in some ideal world, we could have done more, but you, I, and Alvaro all put a hell of a lot of work into this feature just to get it where it is, and I think we should take some significant pride in making as much progress as we did. If you'd asked me 2 years ago when PostgreSQL would have a feature of this type, my money would have been on NEVER. ...Robert
On Thu, 2013-04-18 at 17:31 +0200, Dimitri Fontaine wrote: > - what about support for PLs other than C and PLpgSQL? > > It used to be part of the patch, and I don't understand well > enough > the development calendar to guess if I'm supposed to extract that > from earlier patch or if that's too late for 9.3. I'm not sure > what Peter's idea are wrt to the calendar here. I added event trigger support to PL/sh, just for some additional validation: https://github.com/petere/plsh/tree/event-triggers It seems pretty straightforward and useful, so I'm not sure where your hesitation is coming from. Based in this, I could add some documentation in the coming weeks. I don't think we have time to add support for this to the in-tree PLs. But I was thinking we should at least check all PLs out there that they don't crash if they are presented with an event trigger function, because if you code a PL like this: if (CALLED_AS_TRIGGER(fcinfo) { // it's a trigger } else { // it's a normal call } there might be some trouble.
Peter Eisentraut <peter_e@gmx.net> writes:
> It seems pretty straightforward and useful, so I'm not sure where your
> hesitation is coming from.
If you're talking about my hesitation to consider what we have now as
marketing material worthy, it comes from the fact that I don't have a
use case where I don't need specific information about the SQL objects
involved in the command, or the Normalized Command String.
> Based in this, I could add some documentation in the coming weeks.
I intend to be working on this $soon too.
> I don't think we have time to add support for this to the in-tree PLs.
> But I was thinking we should at least check all PLs out there that they
> don't crash if they are presented with an event trigger function,
> because if you code a PL like this:
>
> if (CALLED_AS_TRIGGER(fcinfo) {
> // it's a trigger
> }
> else {
> // it's a normal call
> }
>
> there might be some trouble.
As it happens I forgot about that part. Yes the API did change in a way
that's not visible at compile time, and IMV that's a bug we need to be
fixing.
The simple way to have at it seems to involve adding a test branch for
event triggers and reporting an ERROR ERRCODE_FEATURE_NOT_SUPPORTED, the
other way to fix that is to actually inclue the support for event
trigger.
As you did the work for PLsh you know how much (little) is involved. If
you want to consider a patch against one of those for adding even
trigger support (in order to have more data to decice), let me know,
I'll prepare that.
Regards,
--
Dimitri Fontaine 06 63 07 10 78
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Peter Eisentraut <peter_e@gmx.net> writes: > At this point, all that is appropriate is some documentation of the C > API. If the contrib example you have in mind is short enough, it might > as well become part of the example in the documentation. Please find attached a patch against the documentation, containing a full code example of what I had in mind. The contrib would only be useful to include if we want to ship something usable. As you might want to tinker with the code in the docs patch and easily check that it still runs, I include another patch with the new contrib module. I don't expect that to get commited, of course, but I had to do it to check the code so I'd better just share it, right? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
On Mon, 2013-05-06 at 17:17 +0200, Dimitri Fontaine wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > At this point, all that is appropriate is some documentation of the C > > API. If the contrib example you have in mind is short enough, it might > > as well become part of the example in the documentation. > > Please find attached a patch against the documentation, containing a > full code example of what I had in mind. The contrib would only be > useful to include if we want to ship something usable. > > As you might want to tinker with the code in the docs patch and easily > check that it still runs, I include another patch with the new contrib > module. I don't expect that to get commited, of course, but I had to do > it to check the code so I'd better just share it, right? Looks pretty good, but the description of the parsetree field was obviously copied from somewhere else. I would fix it myself, but I don't know what kind of assurances we want to offer about what's in that field.
Peter Eisentraut <peter_e@gmx.net> writes: > Looks pretty good, but the description of the parsetree field was > obviously copied from somewhere else. I would fix it myself, but I > don't know what kind of assurances we want to offer about what's in that > field. Oh, oops. I think we should direct the reader to the source code for more information (and propose both where the Node structure is defined and some places where it's used, ProcessUtility comes to mind), and warn that the parse tree exact content for any given command will change in between major releases, and could change in between minor releases. That said, now that it's exposed in the FDW code and the Event Trigger code, we might want to have some ABI compat' in place for minor versions. I'm only raising the question, my knowledge on how to do that and the impact on the code maintaince is so sore that I have no opinion about what the good answer is. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Random observation in this general area: Regular triggers provide the field Trigger *tg_trigger in the trigger data, which allows you to get the trigger name, OID, and such. Event triggers don't expose anything comparable. That should perhaps be added. Also, as I'm maybe about the fourth person ever to write an actual event trigger, I have a usability report of sorts. I found it confusing that the trigger timing is encoded into the event name. So instead of ON ddl_command_start, I was expecting something more like BEFORE ddl_command. There might be a reason for this design choice, but I found it a confusing departure from known trigger concepts.
Peter Eisentraut <peter_e@gmx.net> writes: > Random observation in this general area: Regular triggers provide the > field Trigger *tg_trigger in the trigger data, which allows you to get > the trigger name, OID, and such. Event triggers don't expose anything > comparable. That should perhaps be added. Agreed. Basically we ran out of time to add in any sort of useful information, so that's all 9.4 material. > Also, as I'm maybe about the fourth person ever to write an actual event > trigger, I have a usability report of sorts. I found it confusing that > the trigger timing is encoded into the event name. So instead of ON > ddl_command_start, I was expecting something more like BEFORE > ddl_command. There might be a reason for this design choice, but I > found it a confusing departure from known trigger concepts. Your proposal here matches what I did propose in the 9.2 cycle. I even wanted to go as far as having BEFORE, AFTER and INSTEAD OF command triggers. The problem was to find the right place in the code for each different command, and while I did work on that and proposed command specific integration points, Robert had the idea of having something more flexible and not tied too much with commands, hence events. The idea is to be able to provide events such as "table_rewrite" or "insert_into_unknown_table" etc. How we got from that decision to prefer "ddl_command_start" to BEFORE "ddl_command" still is unclear to me. We could have kept the grammar and turned it internally into the "before_ddl_command" event. But then certainly you want to be able to say BEFORE CREATE TABLE, ALTER TABLE or BEFORE ANY EVENT, things like that. In the patches I sent containing that kind of implementation, it was said to be too much grammar to maintain, as the patch I had needed to list here all supported commands, and each time to add support for a new one you needed to edit that grammar list… Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Mon, 2013-05-06 at 17:17 +0200, Dimitri Fontaine wrote: > Please find attached a patch against the documentation, containing a > full code example of what I had in mind. Applied with some tweaks.
Peter Eisentraut <peter_e@gmx.net> writes: > Applied with some tweaks. Thanks! There's a typo I made that I see only now: + <varlistentry> + <term><structfield>tg_event</></term> I think that should be "event", not "tg_event", because we're listing the fields for the EventTriggerData structure and not the magic variable names found in the PLs. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Thu, 2013-07-04 at 10:15 +0200, Dimitri Fontaine wrote: > There's a typo I made that I see only now: > > + <varlistentry> > + <term><structfield>tg_event</></term> Fixed.