Date: Mon, 29 Mar 2021 10:25:36 -0300
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Arne Roland <A.Roland@index.de>
Cc: David Steele <david@pgmasters.net>,
	Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Rename of triggers for partitioned tables
Message-ID: <20210329132536.GA9798@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <e93f8494c6314b58ad52b553b0e50779@index.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
Archived-At: 
 <https://www.postgresql.org/message-id/20210329132536.GA9798%40alvherre.pgsql>
Precedence: bulk

On 2021-Mar-29, Arne Roland wrote:

> @@ -1475,7 +1467,12 @@ renametrig(RenameStmt *stmt)
>  		tuple = heap_copytuple(tuple);	/* need a modifiable copy */
>  		trigform = (Form_pg_trigger) GETSTRUCT(tuple);
>  		tgoid = trigform->oid;
> -
> +		if (tgparent != 0 && tgparent != trigform->tgparentid) {
> +			ereport(ERROR,
> +					(errcode(ERRCODE_UNDEFINED_OBJECT),
> +					 errmsg("trigger \"%s\" for table \"%s\" is not dependent on the trigger on \"%s\"",
> +							stmt->subname, RelationGetRelationName(targetrel), stmt->relation->relname)));
> +		}

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.

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.

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.

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

-- 
Álvaro Herrera                            39°49'30"S 73°17'W
"Siempre hay que alimentar a los dioses, aunque la tierra esté seca" (Orual)