On Thu, Sep 3, 2015 at 6:21 AM, Amit Kapila <amit.kapila16@gmail.com> wrote:
> [ new patches ]
More review comments:
ExecParallelEstimate() and ExecParallelInitializeDSM() should use
planstate_tree_walker to iterate over the whole planstate tree.
That's because the parallel node need not be at the top of the tree.
By using the walker, you'll find plan state nodes that need work of
the relevant type even if they are deeply buried. The logic should be
something like:
if (node == NULL) return false;
switch (nodeTag(node)) { ... /* parallel aware node enumerated here */ }
return planstate_tree_walker(node, ExecParallelEstimate, context);
The function signature should be changed to bool
ExecParallelEstimate(PlanState *planstate, parallel_estimate_ctx
*context) where parallel_estimate_ctx is a structure containing
ParallelContext *context and Size *psize. The comment about handling
only a few node types should go away, because by using the
planstate_tree_walker we can iterate over anything.
It looks to me like there would be trouble if an initPlan or subPlan
were kept below a Funnel, or as I guess we're going to call it, a
Gather node. That's because a SubPlan doesn't actually have a pointer
to the node tree for the sub-plan in it. It just has an index into
PlannedStmt.subplans. But create_parallel_worker_plannedstmt sets the
subplans list to NIL. So that's not gonna work. Now maybe there's no
way for an initPlan or a subPlan to creep down under the funnel, but I
don't immediately see what would prevent it.
+ * There should be atleast thousand pages to scan for each worker.
"at least a thousand"
+cost_patialseqscan(Path *path, PlannerInfo *root,
patial->partial.
I also don't see where you are checking that a partial seq scan has
nothing parallel-restricted in its quals.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
