Horton

Re: UPDATE of partition key

Amit Khandekar <amitdkhan.pg@gmail.com>

View thread View on postgr.es
From: Amit Khandekar <amitdkhan.pg@gmail.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>, Amit Kapila <amit.kapila16@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>
Date: 2017-06-21T17:38:28Z
Lists: pgsql-hackers 
On 21 June 2017 at 20:14, Robert Haas <robertmhaas@gmail.com> wrote:
> 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.

Yes, the RLS quals should execute during tuple routing according to
whether it is a update or whether it has been converted to insert. I
think the tests don't quite test the insert part. Will check.

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

From what I had checked earlier when coding that part,
rootResultRelInfo is NULL in case of inserts, unless something has
changed in later commits. That's the reason I decided to use the first
resultRelInfo.


Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database 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.