Re: 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(at)gmx(dot)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(at)gmx(dot)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(at)gmx(dot)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(at)2ndQuadrant(dot)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(at)gmx(dot)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(at)gmx(dot)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, logical rep.)
- 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(at)gmx(dot)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(at)gmx(dot)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(at)2ndquadrant(dot)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 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.

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

Peter Eisentraut <peter_e(at)gmx(dot)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(at)2ndquadrant(dot)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(at)2ndquadrant(dot)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(at)gmx(dot)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(at)gmx(dot)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

On Mon, 2013-05-06 at 17:17 +0200, Dimitri Fontaine wrote:
> Peter Eisentraut <peter_e(at)gmx(dot)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(at)gmx(dot)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(at)gmx(dot)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(at)gmx(dot)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.