Horton

Re: UPDATE of partition key

Robert Haas <robertmhaas@gmail.com>

View thread View on postgr.es
From: Robert Haas <robertmhaas@gmail.com>
To: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Amit Khandekar <amitdkhan.pg@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-06-21T14:44:19Z
Lists: pgsql-hackers 
On Wed, Jun 21, 2017 at 5:28 AM, Amit Langote
<Langote_Amit_f8@lab.ntt.co.jp> wrote:>> The comment "UPDATE/DELETE
cases are handled above" is referring to
>> the code that initializes the WCOs generated by the planner.  You've
>> modified the comment in your patch, but the associated code: your
>> updated comment says that only "DELETEs and local UPDATES are handled
>> above", but in reality, *all* updates are still handled above.  And
>> then they are handled again here.  Similarly for returning lists.
>> It's certainly not OK for the comment to be inaccurate, but I think
>> it's also bad to redo the work which the planner has already done,
>> even if it makes the patch smaller.
>
> I guess this has to do with the UPDATE turning into DELETE+INSERT.  So, it
> seems like WCOs are being initialized for the leaf partitions
> (ResultRelInfos in the mt_partitions array) that are in turn are
> initialized for the aforementioned INSERT.  That's why the term "...local
> UPDATEs" in the new comment text.
>
> If that's true, I wonder if it makes sense to apply what would be
> WCO_RLS_UPDATE_CHECK to a leaf partition that the tuple will be moved into
> by calling ExecInsert()?

I think we probably should apply the insert policy, just as we're
executing the insert trigger.

>> Also, I feel like it's probably not correct to use the first result
>> relation as the nominal relation for building WCOs and returning lists
>> anyway.  I mean, if the first result relation has a different column
>> order than the parent relation, isn't this just broken?  If it works
>> for some reason, the comments don't explain what that reason is.
>
> Yep, it's more appropriate to use
> ModifyTableState->rootResultRelationInfo->ri_RelationDesc somehow.  That
> is, if answer to the question I raised above is positive.

The questions appear to me to be independent.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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.