Thanks for the comments.
On Mon, Dec 7, 2020 at 8:56 AM Zhihong Yu <zyu@yugabyte.com> wrote:
>
>
> + (void) SetCurrentCommandIdUsedForWorker();
> + myState->output_cid = GetCurrentCommandId(false);
>
> SetCurrentCommandIdUsedForWorker already has void as return type. The '(void)' is not needed.
>
Removed.
>
> + * rd_createSubid is marked invalid, otherwise, the table is
> + * not allowed to extend by the workers.
>
> nit: to extend by the workers -> to be extended by the workers
>
Changed.
>
> For IsParallelInsertInCTASAllowed, logic is inside 'if (IS_CTAS(into))' block.
> You can return false when (!IS_CTAS(into)) - this would save some indentation for the body.
>
Done.
>
> + if (rel && rel->relpersistence != RELPERSISTENCE_TEMP)
> + allowed = true;
>
> Similarly, when the above condition doesn't hold, you can return false directly - reducing the next if condition to
'if(queryDesc)'.
>
Done.
>
> The composite condition is negated. Maybe you can write without negation:
>
Done.
>
> + * Write out the number of tuples this worker has inserted. Leader will use
> + * it to inform to the end client.
>
> 'inform to the end client' -> 'inform the end client' (without to)
>
Changed.
Attaching v8 patch. Consider this for further review.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com