Re: Rename of triggers for partitioned tables

From: Arne Roland <A(dot)Roland(at)index(dot)de>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
Cc: David Steele <david(at)pgmasters(dot)net>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Rename of triggers for partitioned tables
Date: 2021-03-29 14:02:35
Message-ID: 6e04673973bf4922bf30fe22d5b0f067@index.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thank you for the quick reply!

Alvaro Herrera wrote:
> I think this is not what we want to do. What you're doing here as I
> understand is traversing the inheritance hierarchy down using the
> trigger name, and then fail if the trigger with that name has a
> different parent or if no trigger with that name exists in the child.

The basic concept here was to fail in any circumstance where the trigger on the child isn't as expected.
May it be the name or the parent for that matter.

> What we really want to have happen, is to search the list of triggers in
> the child, see which one has tgparentid=<the one we're renaming>, and
>rename that one -- regardless of what name it had originally.

So if we have a trigger named t42 can be renamed to my_trigger by
ALTER TRIGGER sometrigg ON my_table RENAME TO my_trigger;?
Equivalently there is the question whether we just want to silently ignore
if the corresponding child doesn't have a corresponding trigger.
I am not convinced this is really what we want, but I could agree to raise a notice in these cases.

> Also, you added grammar support for the ONLY keyword in the command, but
> it doesn't do anything different when given that flag, does it? At
> least, I see no change or addition to recursion behavior in ATExecCmd.
> I would expect that if I say "ALTER TRIGGER .. ON ONLY tab" then it
> renames the trigger on table "tab" but not on its child tables; only if
> I remove the ONLY from the command it recurses.

The syntax does work via
+ if (stmt->relation->inh && has_subclass(relid))
in renametrigHelper (src/backend/commands/trigger.c line 1543).
I am not sure which sort of addition in ATExecCmd you expected.
Maybe I can make this more obvious one way or the other. You input would be appreciated.

> I think it would be good to have a new test for pg_dump that verifies
> that this stuff is doing the right thing.

Good idea, I will add a case involving pg_dump.

Regards
Arne

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Vik Fearing 2021-03-29 14:17:50 Re: Idea: Avoid JOINs by using path expressions to follow FKs
Previous Message David Steele 2021-03-29 13:52:10 Re: [PATCH v3 1/1] Fix detection of preadv/pwritev support for OSX.