Re: [POC]Enable tuple change partition caused by BEFORE TRIGGER

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: "movead(dot)li(at)highgo(dot)ca" <movead(dot)li(at)highgo(dot)ca>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Amit Langote <amitlangote09(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Ahsan Hadi <ahsan(dot)hadi(at)gmail(dot)com>, Ashutosh Bapat <ashutosh(dot)bapat(at)2ndquadrant(dot)com>
Subject: Re: [POC]Enable tuple change partition caused by BEFORE TRIGGER
Date: 2020-08-21 11:47:31
Message-ID: CAExHW5vnPCHK4JSDvM6raQTxZRpmgKV66J6qCSZSCRi-+6QLtg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Aug 21, 2020 at 1:28 PM movead(dot)li(at)highgo(dot)ca <movead(dot)li(at)highgo(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fujii Masao 2020-08-21 11:52:34 Re: [PG13] Planning (time + buffers) data structure in explain plan (format text)
Previous Message Dilip Kumar 2020-08-21 11:39:46 Re: PATCH: logical_work_mem and logical streaming of large in-progress transactions