Re: assessing parallel-safety

On Thu, Mar 19, 2015 at 8:53 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:
>
> On Wed, Mar 18, 2015 at 9:31 PM, Robert Haas <robertmhaas(at)gmail(dot)com>
wrote:
> >
> > On Tue, Mar 17, 2015 at 9:48 AM, Robert Haas <robertmhaas(at)gmail(dot)com>
wrote:
> > > On Tue, Mar 17, 2015 at 2:26 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > >> Neither that rule, nor its variant downthread, would hurt operator
authors too
> > >> much. To make the planner categorically parallel-safe, though,
means limiting
> > >> evaluate_function() to parallel-safe functions. That would
dramatically slow
> > >> selected queries. It's enough for the PL scenario if planning a
parallel-safe
> > >> query is itself parallel-safe. If the planner is parallel-unsafe
when
> > >> planning a parallel-unsafe query, what would suffer?
> > >
> > > Good point. So I guess the rule can be that planning a parallel-safe
> > > query should be parallel-safe. From there, it follows that estimators
> > > for a parallel-safe operator must also be parallel-safe. Which seems
> > > fine.
> >
> > More work is needed here, but for now, here is a rebased patch, per
> > Amit's request.
> >
>
> Apart from this, I have one observation:
> static int
> exec_stmt_execsql(PLpgSQL_execstate *estate,
> PLpgSQL_stmt_execsql
> *stmt)
> {
> ParamListInfo paramLI;
> long tcount;
> int rc;
> PLpgSQL_expr *expr =
> stmt->sqlstmt;
>
> /*
> * On the first call for this statement generate the plan, and detect
> * whether
> the statement is INSERT/UPDATE/DELETE
> */
> if (expr->plan == NULL)
> {
> ListCell *l;
>
> exec_prepare_plan(estate, expr, 0);
>
> Shouldn't we need parallelOk in function exec_stmt_execsql()
> to pass cursoroption in above function as we have done in
> exec_run_select()?
>

Today while integrating parallel_seqscan patch with this patch, I had
another observation which is that even if the function is parallel-unsafe,
it still can treat statements inside that function as parallel-safe and
allow
parallelism on such statements.

I think the code in question is as below:
@@ -496,7 +496,9 @@ init_execution_state(List *queryTree_list,
if (queryTree->commandType == CMD_UTILITY)
stmt = queryTree->utilityStmt;
else
- stmt = (Node *) pg_plan_query(queryTree, 0, NULL);
+ stmt = (Node *) pg_plan_query(queryTree,
+ fcache->readonly_func ? CURSOR_OPT_PARALLEL_OK : 0,
+ NULL);

Basically this is executing a statement inside a function and if the
function is parallel-unsafe, and statement it is trying to execute is
parallel-safe (contains no other parallel-unsafe expressions), then
it will choose a parallel plan for such a statement. Shouldn't we
try to avoid such cases?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com