Обсуждение: [POC]Enable tuple change partition caused by BEFORE TRIGGER

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 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