Re: Command Triggers patch v18

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Dimitri Fontaine <dimitri(at)2ndquadrant(dot)fr>, Andres Freund <andres(at)2ndquadrant(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Command Triggers patch v18
Date: 2012-03-28 19:41:04
Message-ID: m2ehsc8qen.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Further thought: I think maybe we shouldn't use keywords at all for
> this, and instead use descriptive strings like post-parse or
> pre-execution or command-start, because I bet in the end we're going
> to end up with a bunch of them (create-schema-subcommand-start?
> alter-table-subcommand-start?), and it would be nice not to hassle
> with the grammar every time we want to add a new one. To make this
> work with the grammar, we could either (1) enforce that each is
> exactly one word or (2) require them to be quoted or (3) require those
> that are not a single word to be quoted. I tend think #2 might be the
> best option in this case, but...

What about :

create command trigger foo before prepare alter table …
create command trigger foo before start of alter table …
create command trigger foo before execute alter table …
create command trigger foo before end of alter table …

create command trigger foo when prepare alter table …
create command trigger foo when start alter table …
create command trigger foo when execute of alter table …
create command trigger foo when end of alter table …

create command trigger foo preceding alter table …
create command trigger foo preceding alter table … deferred
create command trigger foo preceding alter table … immediate

create command trigger foo following parser of alter table …
create command trigger foo preceding execute of alter table …

create command trigger foo following alter table …

create command trigger foo before alter table nowait …

I'm not sure how many hooks we can really export here, but I guess we
have enough existing keywords to describe when they get to run (parser,
mapping, lock, check, begin, analyze, access, disable, not exists, do,
end, escape, extract, fetch, following, search…)

> I've also had another general thought about safety: we need to make
> sure that we're only firing triggers from places where it's safe for
> user code to perform arbitrary actions. In particular, we have to
> assume that any triggers we invoke will AcceptInvalidationMessages()
> and CommandCounterIncrement(); and we probably can't do it at all (or
> at least not without some additional safeguard) in places where the
> code does CheckTableNotInUse() up through the point where it's once
> again safe to access the table, since the trigger might try. We also

I've been trying to do that already.

> need to think about re-entrancy: that is, in theory, the trigger might
> execute any other DDL command - e.g. it might drop the table that
> we've been asked to rename. I suspect that some of the current BEFORE

That's why I implemented ALTER COMMAND TRIGGER ... SET DISABLE in the
first place, so that you could run the same command from the trigger
itself without infinite recursion.

> trigger stuff might fall down on that last one, since the existing
> code not-unreasonably expects that once it's locked the table, the
> catalog entries can't go away. Maybe it all happens to work out OK,
> but I don't think we can count on that.

I didn't think of DROP TABLE in a command table running BEFORE ALTER
TABLE, say. That looks evil. It could be documented as yet another way
to shoot yourself in the foot though?

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Marko Kreen 2012-03-28 20:48:58 Re: Standbys, txid_current_snapshot, wraparound
Previous Message Andrew Dunstan 2012-03-28 19:17:23 Re: patch for parallel pg_dump