Re: Parallel Inserts in CREATE TABLE AS

On Tue, Nov 24, 2020 at 4:43 PM Hou, Zhijie <houzj(dot)fnst(at)cn(dot)fujitsu(dot)com> wrote:
>
> I'm very interested in this feature,
> and I'm looking at the patch, here are some comments.
>

Thanks for the review.

>
> How about the following style:
>
> if(TupIsNull(outerTupleSlot))
> Break;
>
> (void) node->ps.dest->receiveSlot(outerTupleSlot, node->ps.dest);
> node->ps.state->es_processed++;
>
> Which looks cleaner.
>

Done.

>
> The check can be replaced by ISCTAS(into).
>

Done.

>
> 'inerst' looks like a typo (insert).
>

Corrected.

>
> The code here call strlen(intoclausestr) for two times,
> After checking the existing code in ExecInitParallelPlan,
> It used to store the strlen in a variable.
>
> So how about the following style:
>
> intoclause_len = strlen(intoclausestr);
> ...
> /* Store serialized intoclause. */
> intoclause_space = shm_toc_allocate(pcxt->toc, intoclause_len + 1);
> memcpy(shmptr, intoclausestr, intoclause_len + 1);
> shm_toc_insert(pcxt->toc, PARALLEL_KEY_INTO_CLAUSE, intoclause_space);
>

Done.

>
> The two check about intoclausestr seems can be combined like:
>
> if (intoclausestr != NULL)
> {
> ...
> }
> else
> {
> ...
> }
>

Done.

Attaching v5 patch. Please consider it for further review.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com