Add CREATE support to event triggers

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Add CREATE support to event triggers
Date: 2013-11-08 15:33:22
Message-ID: 20131108153322.GU5809@eldon.alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hello,

Attached you can find a very-much-WIP patch to add CREATE info support
for event triggers (normalized commands). This patch builds mainly on
two things:

1. Dimitri's "DDL rewrite" patch he submitted way back, in
http://www.postgresql.org/message-id/m2zk1j9c44.fsf@2ndQuadrant.fr

I borrowed the whole ddl_rewrite.c code, and tweaked it a bit. There
are several things still wrong with it and which will need to be fixed
before a final patch can even be contemplated; but there are some
questions that require a consensus answer before I go and fix it all,
because what it will look like will depend on said answers.

2. The ideas we used to build DROP support. Mainly, the interesting
thing here is the fact that we use a SRF to report, at
ddl_command_end, all the objects that were created during execution
of that command. We do this by collecting them in a list in some raw
form somewhere during ProcessUtility, and then spitting them out if
the SRF is called. I think the general idea is sound, although of
course I admit there might be bugs in the implementation.

Note this patch doesn't try to add any kind of ALTER support. I think
this is fine in principle, because we agreed that we would attack each
kind of command separately (divide to conquer and all that); but there
is a slight problem for some kind of objects that are represented partly
as ALTER state during creation; for example creating a table with a
sequence uses ALTER SEQ/OWNED BY internally at some point. There might
be other cases I'm missing, also. (The REFRESH command is nominally
also supported.)

Now about the questions I mentioned above:

a) It doesn't work to reverse-parse the statement nodes in all cases;
there are several unfixable bugs if we only do that. In order to create
always-correct statements, we need access to the catalogs for the
created objects. But if we are doing catalog access, then it seems to
me that we can do away with the statement parse nodes completely and
just reconstruct the objects from catalog information. Shall we go that
route?

b) What's the best design of the SRF output? This patch proposes two
columns, object identity and create statement. Is there use for
anything else? Class/object OIDs perhaps, schema OIDs for objects types
that have it? I don't see any immediate need to that info, but perhaps
someone does.

c) The current patch stashes all objects in a list, whenever there's an
event trigger function. But perhaps some users want to have event
triggers and not have any use for the CREATE statements. So one idea is
to require users that want the objects reported to call a special
function in a ddl_command_start event trigger which enables collection;
if it's not called, objects are not reported. This eliminates
performance impact for people not using it, but then maybe it will be
surprising for people that call the SRF and find that it always returns
empty.

d) There's a new routine uncookConstraintOrDefault. This takes a raw
expression, runs transformExpr() on it, and then deparses it (possibly
setting up a deparse context based on some relation). This is a
somewhat funny thing to be doing, so if there are other ideas on how to
handle this, I'm all ears.

This patch doesn't include doc changes or regression tests. Those will
be added in a later version. For now, you can see this code in action
by running installing an event trigger like this:

CREATE OR REPLACE FUNCTION snitch() RETURNS event_trigger LANGUAGE plpgsql AS $$
DECLARE
r RECORD;
BEGIN
FOR r IN SELECT * FROM pg_event_trigger_get_normalized_commands()
LOOP
RAISE NOTICE 'object created: id %, statement %',
r.identity, r.command;
END LOOP;
END;
$$;
CREATE EVENT TRIGGER snitch ON ddl_command_end EXECUTE PROCEDURE snitch();

And then running the DDL of your preference. Be warned that there are
many rough edges, so some objects might be incompletely reported (or
bogusly so, in particular with regards to objects in search_path but not
in the first position of it); but if you see any crashes, please let me
know. Also, many commands are not supported yet by the ddl_rewrite.c
code and they return "unsupported FOOBAR". I didn't want to go to the
effort of writing code for them that might end up being ripped out if we
wanted to go the route of using only syscache to build CREATE
statements. That part is pretty mechanical, and not much code anyway.
I just left whatever Dimitri already had in his patch.

I would have continued working some more on this patch before
submitting, except that I'm going on vacations for a few days starting
tomorrow and we have a rule that no complex patches can go in at CF4
without it having been discussed in previous commitfests. My intention
is that the patch that arrives at CF4 is free of design surprises and
ready for a final review before commit, so if there's any disagreement
with the direction this is going, please speak up now.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachment Content-Type Size
event-trigger-create.patch text/x-diff 81.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Mike Blackwell 2013-11-08 15:33:31 Re: stats for network traffic WIP
Previous Message Heikki Linnakangas 2013-11-08 15:19:31 Re: Gin page deletion bug