RE: Parallel Inserts in CREATE TABLE AS

Hi,

I'm very interested in this feature,
and I'm looking at the patch, here are some comments.

1.
+ if (!TupIsNull(outerTupleSlot))
+ {
+ (void) node->ps.dest->receiveSlot(outerTupleSlot, node->ps.dest);
+ node->ps.state->es_processed++;
+ }
+
+ if(TupIsNull(outerTupleSlot))
+ break;
+ }

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.

2.
+
+ if (into != NULL &&
+ IsA(into, IntoClause))
+ {

The check can be replaced by ISCTAS(into).

3.
+ /*
+ * For parallelizing inserts in CTAS i.e. making each
+ * parallel worker inerst it's tuples, we must send
+ * information such as intoclause(for each worker

'inerst' looks like a typo (insert).

4.
+ /* Estimate space for into clause for CTAS. */
+ if (ISCTAS(planstate->intoclause))
+ {
+ intoclausestr = nodeToString(planstate->intoclause);
+ shm_toc_estimate_chunk(&pcxt->estimator, strlen(intoclausestr) + 1);
+ shm_toc_estimate_keys(&pcxt->estimator, 1);
+ }
...
+ if (intoclausestr != NULL)
+ {
+ char *shmptr = (char *)shm_toc_allocate(pcxt->toc,
+ strlen(intoclausestr) + 1);
+ strcpy(shmptr, intoclausestr);
+ shm_toc_insert(pcxt->toc, PARALLEL_KEY_INTO_CLAUSE, shmptr);
+ }

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);

the code in ExecInitParallelPlan

5.
+ if (intoclausestr != NULL)
+ {
+ char *shmptr = (char *)shm_toc_allocate(pcxt->toc,
+ strlen(intoclausestr) + 1);
+ strcpy(shmptr, intoclausestr);
+ shm_toc_insert(pcxt->toc, PARALLEL_KEY_INTO_CLAUSE, shmptr);
+ }
+
/* Set up the tuple queues that the workers will write into. */
- pei->tqueue = ExecParallelSetupTupleQueues(pcxt, false);
+ if (intoclausestr == NULL)
+ pei->tqueue = ExecParallelSetupTupleQueues(pcxt, false);

The two check about intoclausestr seems can be combined like:

if (intoclausestr != NULL)
{
...
}
else
{
...
}

Best regards,
houzj