Thanks for the comments. On Mon, Dec 7, 2020 at 8:56 AM Zhihong Yu <z...@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
v8-0001-Parallel-Inserts-in-CREATE-TABLE-AS.patch
Description: Binary data