Re: UPDATE of partition key

On Mon, Sep 18, 2017 at 11:29 AM, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> On Fri, Sep 15, 2017 at 4:55 PM, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
>> On 12 September 2017 at 12:39, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
>>> On 12 September 2017 at 11:57, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>>>> On Tue, Sep 12, 2017 at 11:15 AM, Amit Khandekar <amitdkhan(dot)pg(at)gmail(dot)com> wrote:
>>>>

>> In the attached patch, we first call ExecARUpdateTriggers(), and while
>> doing that, we first save the info that a NEW row is already captured
>> (mtstate->mt_transition_capture->tcs_update_old_table == true). If it
>> captured, we pass NULL transition_capture pointer to
>> ExecARDeleteTriggers() (and ExecARInsertTriggers) so that it does not
>> again capture an extra row.
>>
>> Modified a testcase in update.sql by including DELETE statement
>> trigger that uses transition tables.
>
> Ok, this fix looks correct to me, I will review the latest patch.

Please find few more comments.

+ * in which they appear in the PartitionDesc. Also, extract the
+ * partition key columns of the root partitioned table. Those of the
+ * child partitions would be collected during recursive expansion.
*/
+ pull_child_partition_columns(&all_part_cols, oldrelation, oldrelation);
expand_partitioned_rtentry(root, rte, rti, oldrelation, oldrc,
lockmode, &root->append_rel_list,
+ &all_part_cols,

pcinfo->all_part_cols is only used in case of update, I think we can
call pull_child_partition_columns
only if rte has updateCols?

@@ -2029,6 +2034,7 @@ typedef struct PartitionedChildRelInfo

Index parent_relid;
List *child_rels;
+ Bitmapset *all_part_cols;
} PartitionedChildRelInfo;

I might be missing something, but do we really need to store
all_part_cols inside the
PartitionedChildRelInfo, can't we call pull_child_partition_columns
directly inside
inheritance_planner whenever we realize that RTE has some updateCols
and we want to
check the overlap?

+extern void partition_walker_init(PartitionWalker *walker, Relation rel);
+extern Relation partition_walker_next(PartitionWalker *walker,
+ Relation *parent);
+

I don't see these functions are used anywhere?

+typedef struct PartitionWalker
+{
+ List *rels_list;
+ ListCell *cur_cell;
+} PartitionWalker;
+

Same as above

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com