Re: Event Triggers: adding information

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Ok. Will prepare a non controversial patch for ddl_command_end.
>
> Thanks. I will make a forceful effort to review that in a timely
> fashion when it's posted.

Please find it attached to this email.

COLUMNS=72 git diff --stat
doc/src/sgml/event-trigger.sgml | 93 ++++++-
src/backend/commands/event_trigger.c | 122 +++++++--
src/backend/tcop/utility.c | 273 ++++++++++---------
src/backend/utils/cache/evtcache.c | 2 +
src/include/commands/event_trigger.h | 1 +
src/include/utils/evtcache.h | 3 +-
src/test/regress/expected/event_trigger.out | 6 +-
src/test/regress/sql/event_trigger.sql | 4 +
8 files changed, 359 insertions(+), 145 deletions(-)

What is the next part you want to see separated away?

We have a choice of:

- Context exposing and filtering

My preference for that point is to include it and limit it to event
triggers written in C, leaving the door open to add new events in
the future, and allowing existing consumers of that framework see
through implementation details should they need it.

It's better than just a ProcessUtility_hook because you can see it
in \dy, decide to disable it with ALTER EVENT TRIGGER (e.g. in a
transcation to avoid recursive calling into itself), and it won't
run in single user mode even when in shared_preload_libraries.

Another idea is to protect it with a GUC.

Yet another idea is to add a new property in the CREATE EVENT
TRIGGER command so that user can request explicitely to have that
and know the feature will break at next release and all the ones
after that:

CREATE EVENT TRIGGER foo ON ddl_command_start
WHEN show_internals in ('Yes I know what I''m doing',
'Thanks')
EXECUTE PROCEDURE …();

That particular phrasing and option choice might be revisited before
commit, though, I'm not wedded to it.

- Additional Information to PL/pgSQL

We're talking about :

- operation (CREATE, ALTER, DROP)
- object type (TABLE, FUNCTION, TEXT SEARCH DICTIONARY, …)
- object OID (when there's a single object target)
- object shema name (when there's a single object target)
- object name (when there's a single object target)

- Additional Information to PL/*

It's separated away because you did opt-out from reviewing that
part, so I guess we will continue to make that a separate patch, and
it will still need to happen. I've been said that I shouldn't be on
the hook to provide for that, and I wonder, but anyway I did already
implement it in a previous version so I can do it again as soon as
we know what we expose.

- Command String Normalisation

I removed it already from the main patch because we needed to talk
about it more, and it's kind of a big part in itself. Meanwhile it
seems to me that an agreement has been reached and that I will be
able to follow the consensus and resume working on that part.

- ddl_event_trace

Now is the time to kill it (for 9.3) or allow it in.

It means another cache lookup at DDL Event time (so 2 cache lookups
per DDL in 9.3); and allow users who just want to have the
information not to have to care about when it is available.

- Non DDL related Events

Extra bonus items, a table_rewrite event. Unlikely for 9.3 at best,
unless someone else wants to have at it, maybe?

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