Horton

Re: UPDATE of partition key

Amit Kapila <amit.kapila16@gmail.com>

View thread View on postgr.es
From: Amit Kapila <amit.kapila16@gmail.com>
To: Amit Khandekar <amitdkhan.pg@gmail.com>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-05-12T09:26:02Z
Lists: pgsql-hackers 
On Fri, May 12, 2017 at 10:49 AM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> On 12 May 2017 at 08:30, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Thu, May 11, 2017 at 5:41 PM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
>
>> If we try to compare it with the non-partitioned update,
>> there also it is internally a delete and insert operation, but we
>> don't fire triggers for those.
>
> For a non-partitioned table, the delete+insert is internal, whereas
> for partitioned table, it is completely visible to the user.
>

If the user has executed an update on root table, then it is
transparent.  I think we can consider it user visible only in case if
there is some user visible syntax like "Update ... Move Row If
Constraint Not Satisfied"

>>
>>>> (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.
>>>
>>
>> Okay.  The broader point I was trying to convey was that the way this
>> patch defines the behavior of triggers doesn't sound good to me.  It
>> appears to me that in this thread multiple people have raised points
>> around trigger behavior and you should try to consider those.
>
> I understand that there is no single solution which will provide
> completely intuitive trigger behaviour. Skipping BR delete trigger
> should be fine. But then for consistency, we should skip BR insert
> trigger as well, the theory being that the delete+insert are not fired
> by the user so we should not fire them. But I feel both should be
> fired to avoid any consequences unexpected to the user who has
> installed those triggers.
>
> The only specific concern of yours is that of firing *both* update as
> well as insert triggers on the same table, right ? My explanation for
> this was : we have done this before for UPSERT, and we had documented
> the same. We can do it here also.
>
>>  Apart from the options, Robert has suggested, another option could be that
>> we allow firing BR-AR update triggers for original partition and BR-AR
>> insert triggers for the new partition.  In this case, one can argue
>> that we have not actually updated the row in the original partition,
>> so there is no need to fire AR update triggers,
>
> Yes that's what I think. If there is no update happened, then AR
> update trigger should not be executed. AR triggers are only for
> scenarios where it is guaranteed that the DML operation has happened
> when the trigger is being executed.
>
>> but I feel that is what we do for non-partitioned table update and it should be okay here
>> as well.
>
> I don't think so. For e.g. if a BR trigger returns NULL, the update
> does not happen, and then the AR trigger does not fire as well. Do you
> see any other scenarios for non-partitioned tables, where AR triggers
> do fire when the update does not happen ?
>

No, but here also it can be considered as an update for original partition.

>
> Overall, I am also open to skipping both insert+delete BR trigger,
>

I think it might be better to summarize all the options discussed
including what the patch has and see what most people consider as
sensible.


-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Commits

  1. Avoid referencing off the end of subplan_partition_offsets.

  2. Allow UPDATE to move rows between partitions.

  3. Remove useless lookup of root partitioned rel in ExecInitModifyTable().

  4. Factor error generation out of ExecPartitionCheck.

  5. Minor preparatory refactoring for UPDATE row movement.

  6. Simplify and encapsulate tuple routing support code.

  7. Avoid coercing a whole-row variable that is already coerced.

  8. Use ResultRelInfo ** rather than ResultRelInfo * for tuple routing.

  9. Make RelationGetPartitionDispatchInfo expand depth-first.

  10. Expand partitioned tables in PartDesc order.

  11. Use a real RT index when setting up partition tuple routing.

  12. Fix transition tables for partition/inheritance.

  13. Fix confusion about number of subplans in partitioned INSERT setup.

  14. Prevent BEFORE triggers from violating partitioning constraints.

  15. Fire per-statement triggers on partitioned tables.

  16. Fix reporting of violations in ExecConstraints, again.

  17. Don't scan partitioned tables.

  18. Allow FDWs to push down quals without breaking EvalPlanQual rechecks.