On 01/19/2015 06:14 PM, Robert Haas wrote:
> On Fri, Jan 16, 2015 at 10:59 AM, Andres Freund <andres@2ndquadrant.com> wrote:
>> This will give you: The old trigger name. The new table name. The new
>> column name. The new function name.
>
> Ouch. That's clearly no good. I'm struggling to understand whether
> this is a problem with our previous analysis, or a problem with this
> patch:
>
> http://www.postgresql.org/message-id/20141028003356.GA387814@tornado.leadboat.com
>
> pg_get_triggerdef_worker() relies on generate_function_name(), which
> uses the system caches, and on get_rule_expr(), for deparsing the WHEN
> clause. If we allowed only ADDING triggers with a lesser lock and
> never modifying or dropping them with a lesser lock, then changing the
> initial scan of pg_trigger at the top of pg_get_triggerdef_worker() to
> use the transaction snapshot might be OK; if we can see the trigger
> with the transaction snapshot at all, we know it can't have
> subsequently changed. But allowing alterations of any kind isn't
> going to work, so I think our previous analysis on that point was
> incorrect.
>
> I *think* we could fix that if generate_function_name() and
> get_rule_expr() had an option to use the active snapshot instead of a
> fresh snapshot. The former doesn't look too hard to arrange, but the
> latter is a tougher nut to crack.
The function name in the trigger should already be broken in master
since changing the name of a function does not lock the table. Would be
really neat to fix though.
A possible way forward I see here is to undo my changes to the lock
level of ALTER TRIGGER and of pg_get_triggerdef_worker(). This should
work if Noah is correct about his analysis in where he came to the
conclusion: "pg_get_triggerdef() is fine as-is with concurrent CREATE
TRIGGER.". Is this still true with the new information?
--
Andreas Karlsson