Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lRyTW-0006Fv-38 for pgsql-hackers@arkaria.postgresql.org; Thu, 01 Apr 2021 14:39:11 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lRyTV-0002zg-1Z for pgsql-hackers@arkaria.postgresql.org; Thu, 01 Apr 2021 14:39:09 +0000 Received: from makus.postgresql.org ([2001:4800:3e1:1::229]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lRyTU-0002xS-Jz for pgsql-hackers@lists.postgresql.org; Thu, 01 Apr 2021 14:39:08 +0000 Received: from mxout1.routing.net ([2a03:2900:1:a::a]) by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lRyTQ-0002do-KY for pgsql-hackers@lists.postgresql.org; Thu, 01 Apr 2021 14:39:07 +0000 Received: from mxbox4.masterlogin.de (unknown [192.168.10.79]) by mxout1.routing.net (Postfix) with ESMTP id 5FD113FFED; Thu, 1 Apr 2021 14:39:00 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mailerdienst.de; s=20200217; t=1617287940; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=5Br/grzXAfnMZsiaHfXA5OXPXo31r9cuLlVllDGQMhs=; b=Hm991ygy4Jh+NTC5TLdT6D4WjvN3hSdEZY10XSE+U4Cuu1/ohpy6TNIJipU3L0y4HcTHML yb9DAkPuI3V7BQPrObhRXj6F5NiE0EMgpejsUaG5a8MUf+lCQr3bZE4rY3lVmK42ZLmxu4 XDJFSaaNfew/6Zgt2scmgYE2JxuBjlc= Received: from remote.index.de (business-90-187-114-221.pool2.vodafone-ip.de [90.187.114.221]) by mxbox4.masterlogin.de (Postfix) with ESMTPSA id C9E2180887; Thu, 1 Apr 2021 14:38:59 +0000 (UTC) Received: from acedia.index-gruppe.local (192.168.3.121) by acedia.index-gruppe.local (192.168.3.121) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2176.2; Thu, 1 Apr 2021 16:38:59 +0200 Received: from acedia.index-gruppe.local ([fe80::ecd1:5d0a:a834:dd56]) by acedia.index-gruppe.local ([fe80::ecd1:5d0a:a834:dd56%5]) with mapi id 15.01.2176.009; Thu, 1 Apr 2021 16:38:59 +0200 From: Arne Roland To: Alvaro Herrera CC: David Steele , Pg Hackers Subject: Re: Rename of triggers for partitioned tables Thread-Topic: Rename of triggers for partitioned tables Thread-Index: AQHWxEk1+rora/wutkWbcRQhC03q0KnbGQ2sgAAECBOATmSTgIBhAeKAgBDG6bOAACCNT4AAKOcAgAAj1kP///tHgIAEsSJn Date: Thu, 1 Apr 2021 14:38:59 +0000 Message-ID: <14f5c62614554ac7b730fc1ee26db83d@index.de> References: <6e04673973bf4922bf30fe22d5b0f067@index.de>,<20210329151657.GA423@alvherre.pgsql> In-Reply-To: <20210329151657.GA423@alvherre.pgsql> Accept-Language: en-US, de-DE Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.255.1.33] Content-Type: multipart/alternative; boundary="_000_14f5c62614554ac7b730fc1ee26db83dindexde_" MIME-Version: 1.0 X-Mail-ID: 9b131de4-90c1-485b-9e91-f66bd787e8b5 List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk --_000_14f5c62614554ac7b730fc1ee26db83dindexde_ Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Alvaro Herrera wrote: > I think the child cannot not have a corresponding trigger. If you try > to drop the trigger on child, it should say that it cannot be dropped > because the trigger on parent requires it. So I don't think there's any > case where altering name of trigger in parent would raise an user-facing > error. If you can't find the trigger in child, that's a case of catalog > corruption [...] Ah, I didn't know that. That changes my reasoning about that. I only knew, = that the alter table ... detach partition ... doesn't detach the child trig= ger from the partitioned trigger, but the the attach partition seem to add = one. Maybe we should be able to make sure that there is a one to one corres= pondence for child triggers and child partitions. Currently upon detaching we keep the trigger within the partitioned trigger= of the parent relation. (So the trigger remains in place and can only be d= ropped with the parent one.) Only we try to attach the partition again any = partitioned triggers are simply removed. One idea would be to detach partitioned triggers there in a similar manner = we detach partitioned indexes. That could give above guarantee. (At least i= f one would be willing to write a migration for that.) But then we can't te= ll anymore where the trigger stems from. That hence would break our attach/= detach workflow. I was annoyed by the inability to drop partitioned triggers from detached p= artitions, but it seems I just got a workaround. Ugh. This is a bit of a sidetrack discussion, but it is related. > Consider this. You have table p and partition p1. Add trigger t to p, > and do > ALTER TRIGGER t ON p1 RENAME TO q; > What I'm saying is that if you later do > ALTER TRIGGER t ON ONLY p RENAME TO r; > then the trigger on parent is changed, and the trigger on child stays q. > If you later do > ALTER TRIGGER r ON p RENAME TO s; > then triggers on both tables end up with name s. If we get into the mindset of expecting one trigger on the child, we have t= he following edge case: - The trigger is renamed. You seem to favor the silent rename in that case = over raising a notice (or even an error). I am not convinced a notice would= n't the better in that case. - The trigger is outside of partitioned table. That still means that the tr= igger might still be in the inheritance tree, but likely isn't. Should we r= ename it anyways, or skip that? Should be raise a notice about what we are = doing here? I think that would be helpful to the end user. > Hmm, ok, maybe this is sufficient, I didn't actually test it. I think > you do need to add a few test cases to ensure everything is sane. Make > sure to verify what happens if you have multi-level partitioning > (grandparent, parent, child) and you ALTER the one in the middle. Also > things like if parent has two children and one child has multiple > children. The test I had somewhere upwards this thread, already tested that. I am not sure how to add a test for pg_dump though. Can you point me to the= location in pg_regress for pg_dump? > Also, please make sure that it works correctly with INHERITS (legacy > inheritance). We probably don't want to cause recursion in that case. That works, but I will add a test to make sure it stays that way. Thank you= for your input! Regards Arne --_000_14f5c62614554ac7b730fc1ee26db83dindexde_ Content-Type: text/html; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable
Alvaro Herrera wrote:
> I think the child cannot not have a corresponding trigger.  = If you try
> to drop the trigger on child, it should say that it canno= t be dropped
> because the trigger on parent requires it.  So I don= 't think there's any
> case where altering name of trigger in parent would raise= an user-facing
> error.  If you can't find the trigger in child, that= 's a case of catalog
> corruption [...]

Ah, I didn't know that. That changes my reasoning about that. I only knew, = that the alter table ... detach partition ... doesn't detach the child trig= ger from the partitioned trigger, but the the attach partition seem to add = one. Maybe we should be able to make sure that there is a one to one correspondence for child triggers and= child partitions.

Currently upon detaching we keep the trigger withi= n the partitioned trigger of the parent relation. (So the trigger remains i= n place and can only be dropped with the parent one.) Only we try to attach= the partition again any partitioned triggers are simply removed.

One idea would be to detach partitioned trig= gers there in a similar manner we detach partitioned indexes. That co= uld give above guarantee. (At least if one would be willing to write a migr= ation for that.) But then we can't tell anymore where the trigger stems from. That hen= ce would break our attach/detach workflow.

I was annoyed by the inability t= o drop partitioned triggers from detached partitions, but it seems I just g= ot a workaround. Ugh.

This is a bit of a sidetrack dis= cussion, but it is related.

> Consider this.  You have table p and partition p1.  Add trig= ger t to p,
> and do
> ALTER TRIGGER t ON p1 RENAME TO q;

> What I'm saying is that if you later do
> ALTER TRIGGER t ON ONLY p RENAME TO r;
> then the trigger on parent is changed, and the trigger on child stays = q.
> If you later do
> ALTER TRIGGER r ON p RENAME TO s;
> then triggers on both tables end up with name s.

If we get into the mindset of expecting one trigger on the child, we have t= he following edge case:

- The trigger is renamed. You seem to favor the si= lent rename in that case over raising a notice (or even an error). I am not= convinced a notice wouldn't the better in that case.
- The trigger is outside of partitioned table. Tha= t still means that the trigger might still be in the inheritance tree, but = likely isn't. Should we rename it anyways, or skip that? Should be raise a = notice about what we are doing here? I think that would be helpful to the end user.

> Hmm, ok, maybe this is sufficient, I didn't actually test it.  I = think
> you do need to add a few test cases to ensure everything is sane. = ; Make
> sure to verify what happens if you have multi-level partitioning
> (grandparent, parent, child) and you ALTER the one in the middle. = ; Also
> things like if parent has two children and one child has multiple
> children.

The test I had somewhere upwards this thread, alre= ady tested that.

I am not sure how to add a test for pg_dump though= . Can you point me to the location in pg_regress for pg_dump?

> Also, please make sure that it works correctly with INHERITS (legacy > inheritance).  We probably don't want to cause recursion in that = case.

That works, but I will add a test to make sure it stays that way. Thank you= for your input!

Regards
Arne
--_000_14f5c62614554ac7b730fc1ee26db83dindexde_--