Horton

Re: UPDATE of partition key

Thomas Munro <thomas.munro@enterprisedb.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Amit Khandekar <amitdkhan.pg@gmail.com>
Cc: Robert Haas <robertmhaas@gmail.com>, Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-11-09T03:57:31Z
Lists: pgsql-hackers 
On Wed, Nov 8, 2017 at 5:57 PM, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> On 8 November 2017 at 07:55, Thomas Munro <thomas.munro@enterprisedb.com> wrote:
>> On Tue, Nov 7, 2017 at 8:03 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> The changes to trigger.c still make me super-nervous.  Hey THOMAS
>>> MUNRO, any chance you could review that part?

At first, it seemed quite strange to me that row triggers and
statement triggers fire different events for the same modification.
Row triggers see DELETE +  INSERT (necessarily because different
tables are involved), but this fact is hidden from the target table's
statement triggers.

The alternative would be for all triggers to see consistent events and
transitions.  Instead of having your special case code in ExecInsert
and ExecDelete that creates the two halves of a 'synthetic' UPDATE for
the transition tables, you'd just let the existing ExecInsert and
ExecDelete code do its thing, and you'd need a flag to record that you
should also fire INSERT/DELETE after statement triggers if any rows
moved.

After sleeping on this question, I am coming around to the view that
the way you have it is right.  The distinction isn't really between
row triggers and statement triggers, it's between triggers at
different levels in the hierarchy.  It just so happens that we
currently only fire target table statement triggers and leaf table row
triggers.  Future development ideas that seem consistent with your
choice:

1.  If we ever allow row triggers with transition tables on child
tables, then I think *their* transition tables should certainly see
the deletes and inserts, otherwise OLD TABLE and NEW TABLE would be
inconsistent with the OLD and NEW variables in a single trigger
invocation.  (These were prohibited mainly due to lack of time and
(AFAIK) limited usefulness; I think they would need probably need
their own separate tuplestores, or possibly some kind of filtering.)

2.  If we ever allow row triggers on partitioned tables (ie that fire
when its children are modified), then I think their UPDATE trigger
should probably fire when a row moves between any two (grand-)*child
tables, just as you have it for target table statement triggers.  It
doesn't matter that the view from parent tables' triggers is
inconsistent with the view from leaf table triggers: it's a feature
that we 'hide' partitioning from the user to the extent we can so that
you can treat the partitioned table just like a table.

Any other views?

As for the code, I haven't figured out how to break it yet, and I'm
wondering if there is some way to refactor so that ExecInsert and
ExecDelete don't have to record pseudo-UPDATE trigger events.

-- 
Thomas Munro
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.