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

On Fri, Mar 24, 2017 at 5:57 AM, Rafia Sabih wrote: >> I suspect that code fails to achieve its goals anyway. At the top of >> exec_eval_expr(), you call exec_prepare_plan() and unconditionally >> pass CURSOR_OPT_PARALLEL_OK, so when that function returns,

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

On Thu, Mar 23, 2017 at 11:26 PM, Robert Haas wrote: > > The changes to the plpgsql code don't look so good to me. The change > to exec_stmt_return_query fixes the same bug that I mentioned in the > email linked above, but only half of it -- it corrects the RETURN > QUERY

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

On Thu, Mar 23, 2017 at 12:41 AM, Rafia Sabih wrote: > Agree, done. OK, committed execute-once-v3.patch after some further study. See also https://www.postgresql.org/message-id/CA%2BTgmoZ_ZuH%2BauEeeWnmtorPsgc_SmP%2BXWbDsJ%2BcWvWBSjNwDQ%40mail.gmail.com which

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

On Thu, Mar 23, 2017 at 5:23 AM, Dilip Kumar wrote: > On Wed, Mar 22, 2017 at 10:33 PM, Robert Haas wrote: >> So couldn't we actually make this test !fcache->returnsSet || !es->lazyEval? >> That would let us allow parallel execution for all

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

On Wed, Mar 22, 2017 at 10:33 PM, Robert Haas wrote: > So couldn't we actually make this test !fcache->returnsSet || !es->lazyEval? > That would let us allow parallel execution for all non-set-returning > functions, and also for set-returning functions that end up with >

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

On Wed, Mar 22, 2017 at 3:00 AM, Rafia Sabih wrote: > On Wed, Mar 22, 2017 at 12:55 AM, Robert Haas > wrote: > > On Tue, Mar 21, 2017 at 6:36 AM, Dilip Kumar > wrote: > >> How about taking the decision of execute_once

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

On Wed, Mar 22, 2017 at 12:55 AM, Robert Haas wrote: > On Tue, Mar 21, 2017 at 6:36 AM, Dilip Kumar wrote: >> How about taking the decision of execute_once based on >> fcache->returnsSet instead of based on lazyEval? >> >> change >> +

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

On Tue, Mar 21, 2017 at 6:36 AM, Dilip Kumar wrote: > On Tue, Mar 21, 2017 at 3:36 PM, Rafia Sabih > wrote: >> On Wed, Mar 15, 2017 at 8:55 PM, Robert Haas wrote: >>> Note this: >>> >>> if (completed ||

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

On Tue, Mar 21, 2017 at 3:36 PM, Rafia Sabih wrote: > On Wed, Mar 15, 2017 at 8:55 PM, Robert Haas wrote: >> Note this: >> >> if (completed || !fcache->returnsSet) >> postquel_end(es); >> >> When the SQL function doesn't

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

On Wed, Mar 15, 2017 at 8:55 PM, Robert Haas wrote: > Note this: > > if (completed || !fcache->returnsSet) > postquel_end(es); > > When the SQL function doesn't return a set, then we can allow > parallelism even when lazyEval is set, because we'll only

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

On Fri, Mar 10, 2017 at 7:08 AM, Rafia Sabih 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

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

On Wed, Mar 15, 2017 at 10:52 AM, Dilip Kumar wrote: > 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,

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

On Mon, Mar 13, 2017 at 5:48 PM, Rafia Sabih 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 >

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

On Wed, Mar 8, 2017 at 1:54 AM, Robert Haas wrote: > There are two places where we currently set CURSOR_OPT_PARALLEL_OK in > PL/pgsql: exec_stmt_return_query() sets it when calling > exec_dynquery_with_params(), and exec_run_select() calls it when > calling

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

On Wed, Mar 8, 2017 at 1:54 AM, Robert Haas wrote: > > Here's a draft patch showing the sort of thing I have in mind. I > think it needs more work, but it gives you the idea, I hope. This is > loosely based on your pl_parallel_exec_support_v1.patch, but what I've > done

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

On Tue, Mar 7, 2017 at 9:07 AM, Rafia Sabih wrote: > I have split the patch into two, one is to allow optimiser to select a > parallel plan for queries in PL functions > (pl_parallel_opt_support_v1.patch), wherein CURSOR_OPT_PARALLEL_OK is passed > at required

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

On Sun, Feb 26, 2017 at 7:09 PM, Robert Haas wrote: > > I think I see the problem that you're trying to solve, but I agree > that this doesn't seem all that elegant. The reason why we have that > numberTuples check is because we're afraid that we might be in a > context

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

On Thu, Mar 2, 2017 at 3:54 PM, Amit Kapila wrote: > On Thu, Mar 2, 2017 at 3:50 PM, Robert Haas wrote: >> On Tue, Feb 28, 2017 at 5:25 PM, Amit Kapila wrote: >>> When such a function (that contains statements which have

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

On Thu, Mar 2, 2017 at 3:50 PM, Robert Haas wrote: > On Tue, Feb 28, 2017 at 5:25 PM, Amit Kapila wrote: >> When such a function (that contains statements which have parallel >> plans) is being executed as part of another parallel plan, it can >>

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

On Tue, Feb 28, 2017 at 5:25 PM, Amit Kapila wrote: > When such a function (that contains statements which have parallel > plans) is being executed as part of another parallel plan, it can > allow spawning workers unboundedly. Assume a query like select * > from t1

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

On Mon, Feb 27, 2017 at 12:21 PM, Robert Haas wrote: > On Mon, Feb 27, 2017 at 8:33 AM, Amit Kapila wrote: >> Is there any easy way to find out which way is less expensive? > > No. But that's a separate problem. I'm just saying we shouldn't >

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

On Mon, Feb 27, 2017 at 8:33 AM, Amit Kapila wrote: > Is there any easy way to find out which way is less expensive? No. But that's a separate problem. I'm just saying we shouldn't arbitrarily prohibit parallelism for parallel-safe functions. > Even > if we find some

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

On Sun, Feb 26, 2017 at 4:14 PM, Robert Haas wrote: > On Sun, Feb 26, 2017 at 6:34 AM, Amit Kapila wrote: >> On Sat, Feb 25, 2017 at 9:47 PM, Dilip Kumar wrote: >>> On Sat, Feb 25, 2017 at 5:12 PM, Amit Kapila

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

On Wed, Feb 22, 2017 at 10:25 AM, Rafia Sabih wrote: > 1. Allow parallelism for queries in PL functions by passing > CURSOR_OPT_PARALLEL_OK instead of 0 to exec_prepare_plan called from > exec_stmt_execsql or exec_stmt_dynexecute. Similarly, pass >

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

On Sun, Feb 26, 2017 at 6:34 AM, Amit Kapila wrote: > On Sat, Feb 25, 2017 at 9:47 PM, Dilip Kumar wrote: >> On Sat, Feb 25, 2017 at 5:12 PM, Amit Kapila wrote: >>> Sure, but that should only happen if the function is

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

On Sat, Feb 25, 2017 at 9:47 PM, Dilip Kumar wrote: > On Sat, Feb 25, 2017 at 5:12 PM, Amit Kapila wrote: >> Sure, but that should only happen if the function is *not* declared as >> parallel safe (aka in parallel safe functions, we should not

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

On Sat, Feb 25, 2017 at 5:12 PM, Amit Kapila wrote: > Sure, but that should only happen if the function is *not* declared as > parallel safe (aka in parallel safe functions, we should not generate > parallel plans). So basically we want to put a restriction that

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

On Fri, Feb 24, 2017 at 11:30 AM, Dilip Kumar wrote: > On Fri, Feb 24, 2017 at 10:06 AM, Amit Kapila wrote: >> We have a below check in standard_planner() (!IsParallelWorker()) >> which should prohibit generating parallel plan inside worker, if

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

On Fri, Feb 24, 2017 at 10:06 AM, Amit Kapila wrote: > We have a below check in standard_planner() (!IsParallelWorker()) > which should prohibit generating parallel plan inside worker, if that > is what you are seeing, then we might need a similar check at other > places.

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

On Thu, Feb 23, 2017 at 9:20 PM, Dilip Kumar wrote: > On Thu, Feb 23, 2017 at 8:58 PM, Dilip Kumar wrote: >> Few more comments. >> 1.I don't see any check in the code which will prevent the parallel >> execution of the query inside a function if its

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

On Thu, Feb 23, 2017 at 8:58 PM, Dilip Kumar wrote: > On Thu, Feb 23, 2017 at 12:11 PM, Rafia Sabih > wrote: > > 2. How are you protecting, if the outer select is running in parallel, > then the function called from there should not run

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

On Thu, Feb 23, 2017 at 8:58 PM, Dilip Kumar wrote: > Few more comments. > 1.I don't see any check in the code which will prevent the parallel > execution of the query inside a function if its called from a DML > statement. > e.g. If we use a function in the update

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

On Thu, Feb 23, 2017 at 12:11 PM, Rafia Sabih wrote: > Yes, it can be simplified to > if (dest->mydest == DestIntoRel || (numberTuples && (dest->mydest != > DestSPI && dest->mydest ! DestSQLFunction))) > Thanks. Okay, this looks cleaner. Few more comments. 1.I

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

On Wed, Feb 22, 2017 at 10:22 PM, Dilip Kumar wrote: > > Some initial comments. > > -- > if (numberTuples || dest->mydest == DestIntoRel) > use_parallel_mode = false; > > if (use_parallel_mode) > EnterParallelMode(); > + else if

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

On Wed, Feb 22, 2017 at 10:25 AM, Rafia Sabih wrote: > Hello everybody, > > In the current version, queries in SQL or PL functions can not > leverage parallelism. To relax this restriction I prepared a patch, > the approach used in the patch is explained next, Some

[HACKERS] Enabling parallelism for queries coming from SQL or other PL functions

Hello everybody, In the current version, queries in SQL or PL functions can not leverage parallelism. To relax this restriction I prepared a patch, the approach used in the patch is explained next, Approach: 1. Allow parallelism for queries in PL functions by passing CURSOR_OPT_PARALLEL_OK