pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Arne Roland <A.Roland@index.de>
To: David Steele <david@pgmasters.net>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Rename of triggers for partitioned tables
Date: Mon, 29 Mar 2021 07:37:53 +0000
Message-ID: <63c8be480eda4ef28d17a019e79841e2@index.de> (raw)
In-Reply-To: <0e245ca1-bb16-2416-a9cb-7255907227d7@pgmasters.net>
References: <20210115222641.GA26284@alvherre.pgsql>
	<0e245ca1-bb16-2416-a9cb-7255907227d7@pgmasters.net>

David Steele wrote:
> Arne, thoughts on Álvaro's comments?
>
> Marking this patch as Waiting for Author.

Thank you for the reminder. I was to absorbed by other tasks. I should be more present in the upcoming weeks.

> I think you did not register the patch in commitfest, so I did that for
> you: https://commitfest.postgresql.org/32/2943/

Thank you! I should have done that.

> As you say, triggers on children don't necessarily have to have the same
> name as on parent; this already happens when the trigger is renamed in
> the child table but not on parent.  In that situation the search on the
> child will fail, which will cause the whole thing to fail I think.
>
> We now have the column pg_trigger.tgparentid, and I think it would be
> better (more reliable) to search for the trigger in the child by the
> tgparentid column instead, rather than by name.

The last patch already checks tgparentid and errors out if either of those do not match.
The reasoning behind that was, that I don't see a clear reasonable way to handle this scenario.
If the user choose to rename the child trigger, I am at a loss what to do.
I thought to pass it back to the user to solve the situation instead of simply ignoring the users earlier rename.
Silently renaming sounded a bit presumptuous to me, even though I don't care that much either way.

Do you think silently renaming is better than yielding an error?

> Also, I think it would be good to have
>   ALTER TRIGGER .. ON ONLY parent RENAME TO ..
> to avoid recursing to children.  This seems mostly pointless, but since
> we've allowed changing the name of the trigger in children thus far,
> then we've implicitly made it supported to have triggers that are named
> differently.  (And it's not entirely academic, since the trigger name
> determines firing order.)

If it would be entirely academic, I wouldn't have noticed the whole thing in the first place.

The on only variant sounds like a good idea to me. Even though hardly worth any efford, it seems simple enough. I will work on that.

> Alternatively to this last point, we could decide to disallow renaming
> of triggers on children (i.e. if trigger has tgparentid set, then
> renaming is disallowed).  I don't have a problem with that, but it would
> have to be an explicit decision to take.

I rejected that idea, because I was unsure what to do with migrations. If we would be discussing this a few years back, this would have been likely my vote, but I don't see it happening now.

Regards
Arne

view thread (42+ messages)  latest in thread

Message-ID: <63c8be480eda4ef28d17a019e79841e2@index.de>
Permalink:  ../63c8be480eda4ef28d17a019e79841e2@index.de/
Also on:    postgresql.org/message-id/63c8be480eda4ef28d17a019e79841e2@index.de

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: A.Roland@index.de, david@pgmasters.net, alvherre@alvh.no-ip.org, pgsql-hackers@lists.postgresql.org
  Subject: Re: Rename of triggers for partitioned tables
  In-Reply-To: <63c8be480eda4ef28d17a019e79841e2@index.de>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox