Re: [HACKERS] UPDATE of partition key - Mailing list pgsql-hackers

On 11 May 2017 at 17:23, Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Fri, Mar 17, 2017 at 4:07 PM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>> On 4 March 2017 at 12:49, Robert Haas <robertmhaas@gmail.com> wrote:
>>> On Thu, Mar 2, 2017 at 11:53 AM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>>>> I think it does not make sense running after row triggers in case of
>>>> row-movement. There is no update happened on that leaf partition. This
>>>> reasoning can also apply to BR update triggers. But the reasons for
>>>> having a BR trigger and AR triggers are quite different. Generally, a
>>>> user needs to do some modifications to the row before getting the
>>>> final NEW row into the database, and hence [s]he defines a BR trigger
>>>> for that. And we can't just silently skip this step only because the
>>>> final row went into some other partition; in fact the row-movement
>>>> itself might depend on what the BR trigger did with the row. Whereas,
>>>> AR triggers are typically written for doing some other operation once
>>>> it is made sure the row is actually updated. In case of row-movement,
>>>> it is not actually updated.
>>>
>>> How about running the BR update triggers for the old partition and the
>>> AR update triggers for the new partition?  It seems weird to run BR
>>> update triggers but not AR update triggers.  Another option would be
>>> to run BR and AR delete triggers and then BR and AR insert triggers,
>>> emphasizing the choice to treat this update as a delete + insert, but
>>> (as Amit Kh. pointed out to me when we were in a room together this
>>> week) that precludes using the BEFORE trigger to modify the row.
>>>
>
> I also find the current behavior with respect to triggers quite odd.
> The two points that appears odd are (a) Executing both before row
> update and delete triggers on original partition sounds quite odd.
Note that *before* trigger gets fired *before* the update happens. The
actual update may not even happen, depending upon what the trigger
does. And then in our case, the update does not happen; not just that,
it is transformed into delete-insert. So then we should fire
before-delete trigger.

> (b) It seems inconsistent to consider behavior for row and statement
> triggers differently

I am not sure whether we should compare row and statement triggers.
Statement triggers are anyway fired only per-statement, depending upon
whether it is update or insert or delete. It has nothing to do with
how the rows are modified.


>
>>
>> I checked the trigger behaviour in case of UPSERT. Here, when there is
>> conflict found, ExecOnConflictUpdate() is called, and then the
>> function returns immediately, which means AR INSERT trigger will not
>> fire. And ExecOnConflictUpdate() calls ExecUpdate(), which means BR
>> and AR UPDATE triggers will be fired. So in short, when an INSERT
>> becomes an UPDATE, BR INSERT triggers do fire, but then the BR UPDATE
>> and AR UPDATE also get fired. On the same lines, it makes sense in
>> case of UPDATE=>DELETE+INSERT operation to fire a BR UPDATE trigger on
>> the original table, and then the BR and AR DELETE/INSERT triggers on
>> the respective tables.
>>
>
> I am not sure if it is good idea to compare it with "Insert On
> Conflict Do Update", but  even if we want that way, I think Insert On
> Conflict is consistent in statement level triggers which means it will
> fire both Insert and Update statement level triggres (as per below
> note in docs) whereas the documentation in the patch indicates that
> this patch will only fire Update statement level triggers which is
> odd
>
> Note in docs about Insert On Conflict
> "Note that with an INSERT with an ON CONFLICT DO UPDATE clause, both
> INSERT and UPDATE statement level trigger will be fired.

I guess the reason this behaviour is kept for UPSERT, is because the
statement itself suggests : insert/update.


-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company