On Thu, Sep 3, 2015 at 6:21 AM, Amit Kapila <amit.kapil...@gmail.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:

-> 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 =
+                               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
+                       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

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to