Re: Event Triggers: adding information

Robert Haas <robertmhaas@gmail.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