Fujita-san,
Thanks for the review.
On 2018/02/09 21:20, Etsuro Fujita wrote:
>>> * Please add a brief decsription about partition_oids to the comments for
>>> this struct.
>>>
>>> @@ -91,6 +91,7 @@ typedef struct PartitionTupleRouting
>>> {
>>> PartitionDispatch *partition_dispatch_info;
>>> int num_dispatch;
>>> + Oid *partition_oids;
>>
>> Done.
>
> Thanks, but one thing I'm wondering is: do we really need this array? I
> think we could store into PartitionTupleRouting the list of oids returned
> by RelationGetPartitionDispatchInfo in ExecSetupPartitionTupleRouting,
> instead. Sorry, I should have commented this in a previous email, but
> what do you think about that?
ExecInitPartitionInfo() will have to iterate the list to get the OID of
the partition to be initialized. Isn't it much cheaper with the array?
> Here are other comments:
>
> o On changes to ExecSetupPartitionTupleRouting:
>
> * This is nitpicking, but it would be better to define partrel and
> part_tupdesc within the if (update_rri_index < num_update_rri &&
> RelationGetRelid(update_rri[update_rri_index].ri_RelationDesc) ==
> leaf_oid) block.
>
> - ResultRelInfo *leaf_part_rri;
> + ResultRelInfo *leaf_part_rri = NULL;
> Relation partrel = NULL;
> TupleDesc part_tupdesc;
> Oid leaf_oid = lfirst_oid(cell);
Sure, done.
> * Do we need this? For a leaf partition that is already present in the
> subplan resultrels, the partition's indices (if any) would have already
> been opened.
>
> + /*
> + * Open partition indices. We wouldn't
> need speculative
> + * insertions though.
> + */
> + if
> (leaf_part_rri->ri_RelationDesc->rd_rel->relhasindex &&
> + leaf_part_rri->ri_IndexRelationDescs == NULL)
> + ExecOpenIndices(leaf_part_rri,
> false);
You're right. Removed the call.
Updated patch is attached.
Thanks,
Amit