On 11 May 2017 at 17:23, Amit Kapila <amit.kapil...@gmail.com> wrote: > On Fri, Mar 17, 2017 at 4:07 PM, Amit Khandekar <amitdkhan...@gmail.com> > wrote: >> On 4 March 2017 at 12:49, Robert Haas <robertmh...@gmail.com> wrote: >>> On Thu, Mar 2, 2017 at 11:53 AM, Amit Khandekar <amitdkhan...@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 -- Sent via pgsql-hackers mailing list (email@example.com) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers