Re: Parallel Seq Scan

On Thu, Sep 3, 2015 at 6:21 AM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> [ new patches ]

+ pscan = shm_toc_lookup(node->ss.ps.toc, PARALLEL_KEY_SCAN);

This is total nonsense. You can't hard-code the key that's used for
the scan, because we need to be able to support more than one parallel
operator beneath the same funnel. For example:

Append
-> Partial Seq Scan
-> Partial Seq Scan

Each partial sequential scan needs to have a *separate* key, which
will need to be stored in either the Plan or the PlanState or both
(not sure exactly). Each partial seq scan needs to get assigned a
unique key there in the master, probably starting from 0 or 100 or
something and counting up, and then this code needs to extract that
value and use it to look up the correct data for that scan.

+ case T_ResultState:
+ {
+ PlanState *planstate =
((ResultState*)node)->ps.lefttree;
+
+ return
planstate_tree_walker((Node*)planstate, pcxt,
+
ExecParallelInitializeDSM, pscan_size);
+ }

This looks like another instance of using the walker incorrectly.
Nodes where you just want to let the walk continue shouldn't need to
be enumerated; dispatching like this should be the default case.

+ case T_Result:
+ fix_opfuncids((Node*) (((Result
*)node)->resconstantqual));
+ break;

Seems similarly wrong.

+ * cost_patialseqscan

Typo. The actual function name has the same typo.

+ num_parallel_workers = parallel_seqscan_degree;
+ if (parallel_seqscan_degree <= estimated_parallel_workers)
+ num_parallel_workers = parallel_seqscan_degree;
+ else
+ num_parallel_workers = estimated_parallel_workers;

Use Min?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company