> > > > + /* Okay to parallelize inserts, so mark it. */ > > + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS) > > + ((DR_intorel *) dest)->is_parallel = true; > > > > + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS) > > + ((DR_intorel *) dest)->is_parallel = false; > > We need to know exactly what is the command in above place, to dereference > and mark is_parallel to true, because is_parallel is being added to the > respective structures, not to the generic _DestReceiver structure. So, in > future the above code becomes something like below: > > + /* Okay to parallelize inserts, so mark it. */ > + if (ins_cmd == PARALLEL_INSERT_CMD_CREATE_TABLE_AS) > + ((DR_intorel *) dest)->is_parallel = true; > + else if (ins_cmd == PARALLEL_INSERT_CMD_REFRESH_MAT_VIEW) > + ((DR_transientrel *) dest)->is_parallel = true; > + else if (ins_cmd == PARALLEL_INSERT_CMD_COPY_TO) > + ((DR_copy *) dest)->is_parallel = true; > > In the below place, instead of new function, I think we can just have > something like if (fpes->ins_cmd_type != PARALLEL_INSERT_CMD_UNDEF) > > > Or > > > > + if (fpes->ins_cmd_type == PARALLEL_INSERT_CMD_CREATE_TABLE_AS) > > + pg_atomic_add_fetch_u64(&fpes->processed, > > + queryDesc->estate->es_processed); > > > > If you think the above code will extend the ins_cmd type check in the > future, the generic function may make sense. > > We can also change below to fpes->ins_cmd_type != > PARALLEL_INSERT_CMD_UNDEF. > > + if (fpes->ins_cmd_type == PARALLEL_INSERT_CMD_CREATE_TABLE_AS) > + receiver = ExecParallelGetInsReceiver(toc, fpes); > > If okay, I will modify it in the next version of the patch.
Yes, that looks good to me. Best regards, houzj
