Re: [HACKERS] UPDATE of partition key

On 27 November 2017 at 13:58, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> On 24 November 2017 at 10:52, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> On 2017/11/23 21:57, Amit Khandekar wrote:
>>> If we collect the partition keys in expand_partitioned_rtentry(), we
>>> need to pass the root relation also, so that we can convert the
>>> partition key attributes to root rel descriptor. And the other thing
>>> is, may be, we can check beforehand (in expand_inherited_rtentry)
>>> whether the rootrte's updatedCols is empty, which I think implies that
>>> it's not an UPDATE operation. If yes, we can just skip collecting the
>>> partition keys.
>>
>> Yeah, it seems like a good idea after all to check in
>> expand_inherited_rtentry() whether the root RTE's updatedCols is non-empty
>> and if so check if any of the updatedCols are partition keys.  If we find
>> some, then it will suffice to just set a simple flag in the
>> PartitionedChildRelInfo that will be created for the root table.  That
>> should be done *after* we have visited all the tables in the partition
>> tree including some that might be partitioned and hence will provide their
>> partition keys.  The following block in expand_inherited_rtentry() looks
>> like a good spot:
>>
>>         if (rte->inh && partitioned_child_rels != NIL)
>>         {
>>             PartitionedChildRelInfo *pcinfo;
>>
>>             pcinfo = makeNode(PartitionedChildRelInfo);
>
> Yes, I am thinking about something like that. Thanks.

In expand_partitioned_rtentry(), rather than collecting partition key
attributes of all partitioned tables, I am now checking if
parentrte->updatedCols has any partition key attributes. If an earlier
parentrte's updatedCols was already found to have partition-keys,
don't continue to check more.

Also, rather than converting all the partition key attriubtes to be
compatible with root's tuple descriptor, we better compare with each
of the partitioned table's updatedCols when we have their handle
handy. Each of the parentrte's updatedCols has exactly the same
attributes as the root's, just with the ordering possibly changed. So
it is safe to compare using the updatedCols of intermediate partition
rels rather than those of the root rel. And, the advantage is : we now
got rid of the conversion mapping from each of the partitioned table
to root that was earlier done in pull_child_partition_columns() in the
previous patches.

PartitionedChildRelInfo now has is_partition_key_update field. This is
updated using get_partitioned_child_rels().

> I am also working on your suggestion of moving the
> convert-to-root-descriptor logic from ExecInsert() to ExecUpdate().

Done.

>
> So, in the upcoming patch version, I am intending to include the above
> two, and if possible, Robert's idea of re-using is_partition_attr()
> for pull_child_partition_columns().

Done. Now, is_partition_attr() is renamed to has_partition_attrs().
This function now accepts a bitmapset of attnums instead of a single
attnum. Moved this function from tablecmds.c to partition.c. This is
now re-used, and the earlier pull_child_partition_columns() is
removed.

Attached v26, that has all of the above points covered. Also, this
patch contains the incremental changes that were attached in the patch
encapsulate_partinfo.patch attached in [1]. In the next version, I
will extract them out again and keep them as a separate preparatory
patch.

[1] https://www.postgresql.org/message-id/CAJ3gD9f86H64e4OCjFFszWW7f4EeyriSaFL8SvJs2yOUbc8VEw%40mail.gmail.com

-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company