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
