Re: Parallel Inserts in CREATE TABLE AS

On Thu, Dec 24, 2020 at 10:25 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
> You could change intoclause_len = strlen(intoclausestr) to
> strlen(intoclausestr) + 1 and use intoclause_len in the remaining
> places. We can avoid the +1 in the other places.
> + /* Estimate space for into clause for CTAS. */
> + if (IS_CTAS(intoclause) && OidIsValid(objectid))
> + {
> + intoclausestr = nodeToString(intoclause);
> + intoclause_len = strlen(intoclausestr);
> + shm_toc_estimate_chunk(&pcxt->estimator, intoclause_len + 1);
> + shm_toc_estimate_keys(&pcxt->estimator, 1);
> + }

Done.

> Can we use node->nworkers_launched == 0 in place of
> node->need_to_scan_locally, that way the setting and resetting of
> node->need_to_scan_locally can be removed. Unless need_to_scan_locally
> is needed in any of the functions that gets called.
> + /* Enable leader to insert in case no parallel workers were launched. */
> + if (node->nworkers_launched == 0)
> + node->need_to_scan_locally = true;
> +
> + /*
> + * By now, for parallel workers (if launched any), would have
> started their
> + * work i.e. insertion to target table. In case the leader is chosen to
> + * participate for parallel inserts in CTAS, then finish its
> share before
> + * going to wait for the parallel workers to finish.
> + */
> + if (node->need_to_scan_locally)
> + {

need_to_scan_locally is being set in ExecGather() even if
nworkers_launched > 0 it can still be true, so I think we can not
remove need_to_scan_locally in ExecParallelInsertInCTAS.

Attaching v15 patch set for further review. Note that the change is
only in 0001 patch, other patches remain unchanged from v14.

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