Thread: [POC]Enable tuple change partition caused by BEFORE TRIGGER
Hello hackers,
Currently, if BEFORE TRIGGER causes a partition change, it reports an error 'moving row
to another partition during a BEFORE FOR EACH ROW trigger is not supported' and fails
to execute. I want to try to address this limitation and have made an initial patch to get
feedback from other hackers.
The implemented approach is similar to when a change partition caused by an UPDATE
statement. If it's a BEFORE INSERT TRIGGER then we just need to insert the row produced
by a trigger to the new partition, and if it's a BEFORE UPDATE TRIGGER we need to delete
the old tuple and insert the row produced by the trigger to the new partition.
In current BEFORE TRIGGER implementation, it reports an error once a trigger result out
of current partition, but I think it should check it after finish all triggers call, and you can
see the discussion in [1][2]. In the attached patch I have changed this rule, I check the
partition constraint only once after all BEFORE TRIGGERS have been called. If you do not
agree with this way, I can change the implementation.
And another point is that when inserting to new partition caused by BEFORE TRIGGER,
then it will not trigger the BEFORE TRIGGER on a new partition. I think it's the right way,
what's more, I think the UPDATE approach cause partition change should not trigger the
BEFORE TRIGGERS too, you can see discussed on [1].
Regards,
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
URL : www.highgo.ca
EMAIL: mailto:movead(dot)li(at)highgo(dot)ca
Attachment
On Fri, Aug 21, 2020 at 1:28 PM movead.li@highgo.ca <movead.li@highgo.ca> wrote: > > Hello hackers, > > Currently, if BEFORE TRIGGER causes a partition change, it reports an error 'moving row > to another partition during a BEFORE FOR EACH ROW trigger is not supported' and fails > to execute. I want to try to address this limitation and have made an initial patch to get > feedback from other hackers. I am not opposed to removing that limitation, it would be good to know the usecase we will solve. Trying to change a partition key in a before trigger on a partition looks dubious to me. If at all it should be done by a partitioned table level trigger and not a partition level trigger. > > > The implemented approach is similar to when a change partition caused by an UPDATE > > statement. If it's a BEFORE INSERT TRIGGER then we just need to insert the row produced > > by a trigger to the new partition, and if it's a BEFORE UPDATE TRIGGER we need to delete > > the old tuple and insert the row produced by the trigger to the new partition. If the triggers are not written carefully, this could have ping-pong effect, where the row keeps on bouncing from one partition to the other. Obviously it will be user who must be blamed for this but with thousands of partitions it's not exactly easy to keep track of the trigger's effects. If we prohibited the row movement because of before trigger, users don't need to worry about it at all. > > > In current BEFORE TRIGGER implementation, it reports an error once a trigger result out > > of current partition, but I think it should check it after finish all triggers call, and you can > > see the discussion in [1][2]. In the attached patch I have changed this rule, I check the > > partition constraint only once after all BEFORE TRIGGERS have been called. If you do not > > agree with this way, I can change the implementation. I think this change may be good irrespective of the row movement change. > > > And another point is that when inserting to new partition caused by BEFORE TRIGGER, > > then it will not trigger the BEFORE TRIGGER on a new partition. I think it's the right way, > > what's more, I think the UPDATE approach cause partition change should not trigger the > > BEFORE TRIGGERS too, you can see discussed on [1]. > That looks dubious to me. Before row triggers may be used in several different ways, for auditing, for verification of inserted data or to change some other data based on this change and so on. If we don't execute before row trigger on the partition where the row gets moved, all this expected work won't happen. This also needs some background about the usecase which requires this change. -- Best Wishes, Ashutosh Bapat
On 2020-Aug-21, Ashutosh Bapat wrote: > On Fri, Aug 21, 2020 at 1:28 PM movead.li@highgo.ca <movead.li@highgo.ca> wrote: > > In current BEFORE TRIGGER implementation, it reports an error once a > > trigger result out of current partition, but I think it should check > > it after finish all triggers call, and you can see the discussion in > > [1][2]. In the attached patch I have changed this rule, I check the > > partition constraint only once after all BEFORE TRIGGERS have been > > called. If you do not agree with this way, I can change the > > implementation. > > I think this change may be good irrespective of the row movement change. Yeah, it makes sense to delay the complaint about partition movement until all triggers have been executed ... although that makes it harder to report *which* trigger caused the problem. (It seems pretty bad that the error message that you're changing is not covered in regression tests -- mea culpa.) > > And another point is that when inserting to new partition caused by > > BEFORE TRIGGER, then it will not trigger the BEFORE TRIGGER on a new > > partition. I think it's the right way, what's more, I think the > > UPDATE approach cause partition change should not trigger the BEFORE > > TRIGGERS too, you can see discussed on [1]. > > That looks dubious to me. Yeah ... > Before row triggers may be used in several different ways, for > auditing, for verification of inserted data or to change some other > data based on this change and so on. Admittedly, these things should be done by AFTER triggers, not BEFORE triggers, precisely because you want to do them with the final form of each row -- not a form of the row that could still be changed by some hypothetical BEFORE trigger that will fire next. What this is saying to me is that we'd need to make sure to run the final target partition's AFTER triggers, not the original target partition. But I'm not 100% about running the BEFORE triggers. Maybe one way to address this is to check whether the BEFORE triggers in the new target partition are clones; if so then they would have run in the original target partition and so must not be run, but otherwise they have to. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, 26 Aug 2020 at 22:47, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
What this is saying to me is that we'd need to make sure to run the
final target partition's AFTER triggers, not the original target
partition.
Agreed.
But I'm not 100% about running the BEFORE triggers. Maybe
one way to address this is to check whether the BEFORE triggers in the
new target partition are clones; if so then they would have run in the
original target partition and so must not be run, but otherwise they
have to.
This will work as long as the two BEFORE ROW triggers have the same effect. Consider two situations resulting in inserting identical rows 1. row that the before row trigger has redirected to a new partition, say part2 2. a row inserted directly into the part2 - if both these rows are identical before the BEFORE ROW triggers have been applied, they should remain identical while inserting into part2. Any divergence might be problematic for the application.
Best Wishes,
Ashutosh
On 2020-Aug-27, Ashutosh Bapat wrote: > On Wed, 26 Aug 2020 at 22:47, Alvaro Herrera <alvherre@2ndquadrant.com> > wrote: > > But I'm not 100% about running the BEFORE triggers. Maybe > > one way to address this is to check whether the BEFORE triggers in the > > new target partition are clones; if so then they would have run in the > > original target partition and so must not be run, but otherwise they > > have to. > > This will work as long as the two BEFORE ROW triggers have the same effect. > Consider two situations resulting in inserting identical rows 1. row that > the before row trigger has redirected to a new partition, say part2 2. a > row inserted directly into the part2 - if both these rows are identical > before the BEFORE ROW triggers have been applied, they should remain > identical while inserting into part2. Any divergence might be problematic > for the application. Well, that's why I talk about the trigger being "clones" -- with that term, I mean that their definitions have been inherited from a definition in some ancestor partitioned table, and so they must be identical in the partitions. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services