Re: Rename of triggers for partitioned tables - Mailing list pgsql-hackers
| From | Arne Roland |
|---|---|
| Subject | Re: Rename of triggers for partitioned tables |
| Date | |
| Msg-id | 4a0f161e127444e2aea52b325596645b@index.de Whole thread Raw |
| In response to | Re: Rename of triggers for partitioned tables (Arne Roland <A.Roland@index.de>) |
| Responses |
Re: Rename of triggers for partitioned tables
|
| List | pgsql-hackers |
Hi,
attached a patch with some cleanup and a couple of test cases. The lookup is now done with the parentid and the renaming affects detached partitions as well.
The test cases are not perfectly concise, but only add about 0.4 % to the total runtime of regression tests at my machine. So I didn't bother to much. They only consist of basic sql tests.
Further feedback appreciated! Thank you!
Regards
Arne
From: Arne Roland <A.Roland@index.de>
Sent: Thursday, April 1, 2021 4:38:59 PM
To: Alvaro Herrera
Cc: David Steele; Pg Hackers
Subject: Re: Rename of triggers for partitioned tables
Sent: Thursday, April 1, 2021 4:38:59 PM
To: Alvaro Herrera
Cc: David Steele; Pg Hackers
Subject: Re: Rename of triggers for partitioned tables
Alvaro Herrera wrote:
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 trigger 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.
> 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 the following edge case:
> 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!
> 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 [...]
> 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 [...]
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 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 triggers there in a similar manner we detach partitioned indexes. That could give above guarantee. (At least if one would be willing to write a migration for that.) But then we can't tell 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 partitions, 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 the 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 wouldn't the better in that case.
- The trigger is outside of partitioned table. That 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.
> 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
Attachment
pgsql-hackers by date: