Re: Enabling parallelism for queries coming from SQL or other PL functions

On Mon, Mar 13, 2017 at 5:48 PM, Rafia Sabih
<rafia(dot)sabih(at)enterprisedb(dot)com> wrote:
> Fixed. The attached patch is over execute_once.patch [1].
> I haven't addressed the issue regarding the confusion I raised upthread
> about incorrect value of !es->lazyeval that is restricting parallelism for
> simple queries coming from sql functions, as I am not sure if it is the
> concern of this patch or not.

I have reviewed the patch, I have some questions.

@@ -3031,7 +3031,7 @@ exec_stmt_return_query(PLpgSQL_execstate *estate,
Assert(stmt->dynquery != NULL);
portal = exec_dynquery_with_params(estate, stmt->dynquery,
stmt->params, NULL,
- CURSOR_OPT_PARALLEL_OK);
+ 0);

After this patch, I have noticed that In exec_stmt_return_query, we
allow parallel query if it's a static query
but not for the dynamic query. Any specific reason for different
handling for these 2 cases?

@ -314,7 +314,7 @@ SPI_execute(const char *src, bool read_only, long tcount)

memset(&plan, 0, sizeof(_SPI_plan));
plan.magic = _SPI_PLAN_MAGIC;
- plan.cursor_options = 0;
+ plan.cursor_options = CURSOR_OPT_PARALLEL_OK;

In SPI_Execute, we are setting cursor_options to
CURSOR_OPT_PARALLEL_OK. I have analysed call to this function from PL
and seems fine to me. But, I have a question have you analyzed all the
calls to this functions?
e.g. build_tuplestore_recursively, get_crosstab_tuplestore.

On Fri, Mar 10, 2017 at 5:38 PM, Rafia Sabih
<rafia(dot)sabih(at)enterprisedb(dot)com> wrote:
> I wanted to clarify a few things here, I noticed that call of ExecutorRun in
> postquel_getnext() uses !es->lazyEval as execute_once, this is confusing, as
> it is true even in cases when a simple query like "select count(*) from t"
> is used in a sql function. Hence, restricting parallelism for cases when it
> shouldn't. It seems to me that es->lazyEval is not set properly or it should
> not be true for simple select statements. I found that in the definition of
> execution_state
> bool lazyEval; /* true if should fetch one row at a time

Hmm, It seems that es->lazyEval is not set properly, Ideally, it
should be set true only if it's lazy evaluation but in this case, it's
used for identifying the tupcount as well. IMHO, it should be fixed.

Any other opinion on this?

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com