Re: Use of systable_beginscan_ordered in event trigger patch

From: Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Use of systable_beginscan_ordered in event trigger patch
Date: 2012-12-14 20:46:19
Message-ID: m27gok4b1g.fsf@2ndQuadrant.fr
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Robert, does that ring a bell to you? I'm going to crawl the archives
>> tomorrow if not…
>
> Yeah, I'm pretty sure you can't set event triggers of any kind on
> event triggers. You proposed to allow some stuff that would affect
> "every command", but I yelled and screamed about that until we got rid
> of it, 'cuz it just seemed way too dangerous.

I meant about the way the documentation is phrased to introduce the
example, which is somewhat wrong because not all commands are concerned,
quite a bunch of them will not be disabled by such a command trigger.

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> Sooner or later there will be one somewhere in the code path involved
> in doing a heap_delete on pg_event_trigger, or one of the subsidiary
> catalogs such as pg_depend, or maybe one that just manages to fire
> someplace during backend startup, or who knows what.

You're right that we need to be careful here, in ways that I didn't
foresee. The first thing I can think of is to disable such low level
events on system catalogs, of course.

> Hence, I want a kill switch for all event triggers that will most
> certainly work, and the just-committed patch provides one.

We absolutely need that, and running Event Triggers in standalone mode
seems counter productive to me anyway. That said maybe we need to be
able to have a per-session "leave me alone" mode of operation. What do
you think of

ALTER EVENT TRIGGER DISABLE ALL; -- just tried, no conflict

I don't think we need the ENABLE ALL variant, and it would not be
symetric anyway as you would want to be able to only enable those event
triggers that were already enabled before, and it seems to me that
"cancelling" a statement is best done with "ROLLBACK;" or "ROLLBACK TO
SAVEPOINT …;".

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 Tom Lane 2012-12-14 20:48:29 Re: Assert for frontend programs?
Previous Message Robert Haas 2012-12-14 20:45:42 Re: PageIsAllVisible()'s trustworthiness in Hot Standby