Re: non-bulk inserts and tuple routing

Amit Langote <langote_amit_f8@lab.ntt.co.jp>

From: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2018-02-13T01:12:40Z
Lists: pgsql-hackers

Attachments

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

Commits

  1. Fix parent node of WCO expressions in partitioned tables.

  2. Update PartitionTupleRouting struct comment

  3. Be lazier about partition tuple routing.