Re: [HACKERS] Parallel execution and prepared statements

2016-12-08 Thread Robert Haas
On Wed, Dec 7, 2016 at 9:23 PM, Amit Kapila  wrote:
> Attached patch changes the comment based on above suggestions.

Thanks.  Committed and back-patched to 9.6.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-12-07 Thread Amit Kapila
On Wed, Dec 7, 2016 at 12:30 AM, Robert Haas  wrote:
> On Tue, Dec 6, 2016 at 1:07 PM, Tom Lane  wrote:
>> Robert Haas  writes:
>>> Done.
>>
>> The comment seems quite confused now:
>>
>> * If a tuple count was supplied or data is being written to relation, we
>> * must force the plan to run without parallelism, because we might exit
>> * early.
>>
>> Exit early is exactly what we *won't* do if writing to an INTO rel, so
>> I think this will confuse future readers.  I think it should be more like
>>
>> * If a tuple count was supplied, we must force the plan to run without
>> * parallelism, because we might exit early.  Also disable parallelism
>> * when writing into a relation, because [ uh, why exactly? ]
>>
>> Considering that the writing would happen at top level of the executor,
>> and hence in the parent process, it's not actually clear to me why the
>> second restriction is there at all: can't we write tuples to a rel even
>> though they came from a parallel worker?  In any case, the current wording
>> of the comment is a complete fail at explaining this.
>
> Oops.  You're right.  [ uh, why exactly? ] -> no database changes
> whatsoever are allowed while in parallel mode.  (This restriction
> might be lifted someday, but right now we're stuck with it.)
>

Attached patch changes the comment based on above suggestions.

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


fix_comment_parallel_mode_v1.patch
Description: Binary data

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


Re: [HACKERS] Parallel execution and prepared statements

2016-12-06 Thread Robert Haas
On Tue, Dec 6, 2016 at 1:07 PM, Tom Lane  wrote:
> Robert Haas  writes:
>> Done.
>
> The comment seems quite confused now:
>
> * If a tuple count was supplied or data is being written to relation, we
> * must force the plan to run without parallelism, because we might exit
> * early.
>
> Exit early is exactly what we *won't* do if writing to an INTO rel, so
> I think this will confuse future readers.  I think it should be more like
>
> * If a tuple count was supplied, we must force the plan to run without
> * parallelism, because we might exit early.  Also disable parallelism
> * when writing into a relation, because [ uh, why exactly? ]
>
> Considering that the writing would happen at top level of the executor,
> and hence in the parent process, it's not actually clear to me why the
> second restriction is there at all: can't we write tuples to a rel even
> though they came from a parallel worker?  In any case, the current wording
> of the comment is a complete fail at explaining this.

Oops.  You're right.  [ uh, why exactly? ] -> no database changes
whatsoever are allowed while in parallel mode.  (This restriction
might be lifted someday, but right now we're stuck with it.)

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-12-06 Thread Tom Lane
Robert Haas  writes:
> Done.

The comment seems quite confused now:

* If a tuple count was supplied or data is being written to relation, we
* must force the plan to run without parallelism, because we might exit
* early.

Exit early is exactly what we *won't* do if writing to an INTO rel, so
I think this will confuse future readers.  I think it should be more like

* If a tuple count was supplied, we must force the plan to run without
* parallelism, because we might exit early.  Also disable parallelism
* when writing into a relation, because [ uh, why exactly? ]

Considering that the writing would happen at top level of the executor,
and hence in the parent process, it's not actually clear to me why the
second restriction is there at all: can't we write tuples to a rel even
though they came from a parallel worker?  In any case, the current wording
of the comment is a complete fail at explaining this.

regards, tom lane


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


Re: [HACKERS] Parallel execution and prepared statements

2016-12-06 Thread Robert Haas
On Mon, Dec 5, 2016 at 5:18 AM, Albe Laurenz  wrote:
> Tobias Bussmann wrote:
>>> I think if we don't see any impact then we should backpatch this
>>> patch. However, if we decide to go some other way, then I can provide
>>> a separate patch for back branches.  BTW, what is your opinion?
>>
>> I could not find anything on backporting guidelines in the wiki but my 
>> opinion would be to backpatch
>> the patch in total. With a different behaviour between the simple and 
>> extended query protocol it would
>> be hard to debug query performance issue in user applications that uses 
>> PQprepare. If the user tries
>> to replicate a query with a PREPARE in psql and tries to EXPLAIN EXECUTE it, 
>> the results would be
>> different then what happens within the application. That behaviour could be 
>> confusing, like
>> differences between EXPLAIN SELECT and EXPLAIN EXECUTE can be to less 
>> experienced users.
>
> +1

Done.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-12-05 Thread Albe Laurenz
Tobias Bussmann wrote:
>> I think if we don't see any impact then we should backpatch this
>> patch. However, if we decide to go some other way, then I can provide
>> a separate patch for back branches.  BTW, what is your opinion?
> 
> I could not find anything on backporting guidelines in the wiki but my 
> opinion would be to backpatch
> the patch in total. With a different behaviour between the simple and 
> extended query protocol it would
> be hard to debug query performance issue in user applications that uses 
> PQprepare. If the user tries
> to replicate a query with a PREPARE in psql and tries to EXPLAIN EXECUTE it, 
> the results would be
> different then what happens within the application. That behaviour could be 
> confusing, like
> differences between EXPLAIN SELECT and EXPLAIN EXECUTE can be to less 
> experienced users.

+1

Yours,
Laurenz Albe


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


Re: [HACKERS] Parallel execution and prepared statements

2016-12-03 Thread Tobias Bussmann

> I think if we don't see any impact then we should backpatch this
> patch. However, if we decide to go some other way, then I can provide
> a separate patch for back branches.  BTW, what is your opinion?

I could not find anything on backporting guidelines in the wiki but my opinion 
would be to backpatch the patch in total. With a different behaviour between 
the simple and extended query protocol it would be hard to debug query 
performance issue in user applications that uses PQprepare. If the user tries 
to replicate a query with a PREPARE in psql and tries to EXPLAIN EXECUTE it, 
the results would be different then what happens within the application. That 
behaviour could be confusing, like differences between EXPLAIN SELECT and 
EXPLAIN EXECUTE can be to less experienced users.

Best regards
Tobias




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


Re: [HACKERS] Parallel execution and prepared statements

2016-12-02 Thread Amit Kapila
On Fri, Dec 2, 2016 at 3:31 PM, Tobias Bussmann  wrote:
>
>> On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas  wrote:
>>>
>>> OK, then my vote is to do it that way for now.
>
> Thanks for your opinion. That's fine with me.
>
>> Am 02.12.2016 um 07:22 schrieb Amit Kapila :
>> Done that way in attached patch.
>
> Did a quick review:
>

Thanks for the review.

> You should however include a sentence in the documentation on that parallel 
> plan w/o workers corner-case behaviour. Feel free to take that from my patch 
> or phase a better wording.
>

I have taken it from your patch.

> And again my question regarding back patching to 9.6:
> - 9.6 is currently broken as Laurenz showed in [1]
> - 9.6 does not have documented that SQL PREPARE prepared statements cannot 
> not use parallel query
>
> The former could be fixed by back patching the full patch which would void 
> the latter.
>

I think if we don't see any impact then we should backpatch this
patch. However, if we decide to go some other way, then I can provide
a separate patch for back branches.  BTW, what is your opinion?


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


prepared_stmt_parallel_query_v4.patch
Description: Binary data

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


Re: [HACKERS] Parallel execution and prepared statements

2016-12-02 Thread Tobias Bussmann

> On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas  wrote:
>> 
>> OK, then my vote is to do it that way for now.

Thanks for your opinion. That's fine with me.

> Am 02.12.2016 um 07:22 schrieb Amit Kapila :
> Done that way in attached patch.

Did a quick review: The patch applies cleanly against current head. make 
installcheck with force_parallel_mode = regress passes all tests. My manual 
tests show that parallel query is working for prepared statements in SQL with 
PREPARE and EXECUTE. CREATE TABLE AS EXECUTE is working, EXPLAIN on that shows 
a parallel plan, EXPLAIN ANALZE indicates 0 launched workers for that. Looks 
fine so far!

You should however include a sentence in the documentation on that parallel 
plan w/o workers corner-case behaviour. Feel free to take that from my patch or 
phase a better wording.

And again my question regarding back patching to 9.6: 
- 9.6 is currently broken as Laurenz showed in [1]
- 9.6 does not have documented that SQL PREPARE prepared statements cannot not 
use parallel query

The former could be fixed by back patching the full patch which would void the 
latter. Or it could be fixed by disabling generation of parallel plans in 
extended query protocol prepare. Alternatively only the change in execMain.c 
could be back patched. In these cases we would need to have the a separate 
wording for the 9.6 docs.

Best regards,
Tobias

[1] a737b7a37273e048b164557adef4a58b53999...@ntex2010i.host.magwien.gv.at

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


Re: [HACKERS] Parallel execution and prepared statements

2016-12-01 Thread Amit Kapila
On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas  wrote:
> On Thu, Dec 1, 2016 at 7:57 AM, Amit Kapila  wrote:
>>
>> I have tried to restrict all the non-readonly operation modes or modes
>> where parallelism might not make sense like DestTupleStore.  If we
>> want to just prohibit the cases where it can fail now, then I think
>> prohibiting only DestIntoRel should be sufficient because that is a
>> case where the user is allowed to do DDL for an already prepared read
>> only statement like Create Table AS .. EXECUTE.
>
> OK, then my vote is to do it that way for now.
>

Done that way in attached patch.

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


prepared_stmt_parallel_query_v3.patch
Description: Binary data

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


Re: [HACKERS] Parallel execution and prepared statements

2016-12-01 Thread Robert Haas
On Thu, Dec 1, 2016 at 7:57 AM, Amit Kapila  wrote:
> On Wed, Nov 30, 2016 at 11:57 PM, Robert Haas  wrote:
>> On Tue, Nov 29, 2016 at 6:24 AM, Amit Kapila  wrote:
>>>
>>> Robert, do you have any better ideas for this problem?
>>
>> Not really.  I think your prepared_stmt_parallel_query_v2.patch is
>> probably the best approach proposed so far, but I wonder why we need
>> to include DestCopyOut and DestTupleStore.  DestIntoRel and
>> DestTransientRel both write to an actual relation, which is a problem
>> for parallel mode, but I think the others don't.
>>
>
> I have tried to restrict all the non-readonly operation modes or modes
> where parallelism might not make sense like DestTupleStore.  If we
> want to just prohibit the cases where it can fail now, then I think
> prohibiting only DestIntoRel should be sufficient because that is a
> case where the user is allowed to do DDL for an already prepared read
> only statement like Create Table AS .. EXECUTE.

OK, then my vote is to do it that way for now.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-12-01 Thread Amit Kapila
On Wed, Nov 30, 2016 at 11:57 PM, Robert Haas  wrote:
> On Tue, Nov 29, 2016 at 6:24 AM, Amit Kapila  wrote:
>>
>> Robert, do you have any better ideas for this problem?
>
> Not really.  I think your prepared_stmt_parallel_query_v2.patch is
> probably the best approach proposed so far, but I wonder why we need
> to include DestCopyOut and DestTupleStore.  DestIntoRel and
> DestTransientRel both write to an actual relation, which is a problem
> for parallel mode, but I think the others don't.
>

I have tried to restrict all the non-readonly operation modes or modes
where parallelism might not make sense like DestTupleStore.  If we
want to just prohibit the cases where it can fail now, then I think
prohibiting only DestIntoRel should be sufficient because that is a
case where the user is allowed to do DDL for an already prepared read
only statement like Create Table AS .. EXECUTE.

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


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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-30 Thread Robert Haas
On Tue, Nov 29, 2016 at 6:24 AM, Amit Kapila  wrote:
> On Tue, Nov 29, 2016 at 4:40 PM, Albe Laurenz  wrote:
>> Amit Kapila wrote:
 But with Tobias' complete patch "make installcheck-world" succeeds.
>>>
>>> Okay.  I have looked into patch proposed and found that it will
>>> resolve the regression test problem (Create Table .. AS Execute).  I
>>> think it is slightly prone to breakage if tomorrow we implement usage
>>> of EXECUTE with statements other Create Table (like Copy).  Have you
>>> registered this patch in CF to avoid loosing track?
>>>
>>> We have two patches (one proposed in this thread and one proposed by
>>> me earlier [1]) and both solves the regression test failure.  Unless
>>> there is some better approach, I think we can go with one of these.
>>
>> Tobias did that here: https://commitfest.postgresql.org/12/890/
>>
>> He added your patch as well.
>>
>
> Okay, Thanks.
>
> Robert, do you have any better ideas for this problem?

Not really.  I think your prepared_stmt_parallel_query_v2.patch is
probably the best approach proposed so far, but I wonder why we need
to include DestCopyOut and DestTupleStore.  DestIntoRel and
DestTransientRel both write to an actual relation, which is a problem
for parallel mode, but I think the others don't.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-11-29 Thread Amit Kapila
On Tue, Nov 29, 2016 at 4:40 PM, Albe Laurenz  wrote:
> Amit Kapila wrote:
>>> But with Tobias' complete patch "make installcheck-world" succeeds.
>>
>> Okay.  I have looked into patch proposed and found that it will
>> resolve the regression test problem (Create Table .. AS Execute).  I
>> think it is slightly prone to breakage if tomorrow we implement usage
>> of EXECUTE with statements other Create Table (like Copy).  Have you
>> registered this patch in CF to avoid loosing track?
>>
>> We have two patches (one proposed in this thread and one proposed by
>> me earlier [1]) and both solves the regression test failure.  Unless
>> there is some better approach, I think we can go with one of these.
>
> Tobias did that here: https://commitfest.postgresql.org/12/890/
>
> He added your patch as well.
>

Okay, Thanks.

Robert, do you have any better ideas for this problem?

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


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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-29 Thread Albe Laurenz
Amit Kapila wrote:
>> But with Tobias' complete patch "make installcheck-world" succeeds.
> 
> Okay.  I have looked into patch proposed and found that it will
> resolve the regression test problem (Create Table .. AS Execute).  I
> think it is slightly prone to breakage if tomorrow we implement usage
> of EXECUTE with statements other Create Table (like Copy).  Have you
> registered this patch in CF to avoid loosing track?
> 
> We have two patches (one proposed in this thread and one proposed by
> me earlier [1]) and both solves the regression test failure.  Unless
> there is some better approach, I think we can go with one of these.

Tobias did that here: https://commitfest.postgresql.org/12/890/

He added your patch as well.

Yours,
Laurenz Albe

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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-29 Thread Amit Kapila
On Fri, Nov 18, 2016 at 9:01 PM, Albe Laurenz  wrote:
> Amit Kapila wrote:
>> Can you once test by just passing CURSOR_OPT_PARALLEL_OK in
>> PrepareQuery() and run the tests by having forc_parallel_mode=regress?
>> It seems to me that the code in the planner is changed for setting a
>> parallelModeNeeded flag, so it might work as it is.
>
> No, that doesn't work.
>
> But with Tobias' complete patch "make installcheck-world" succeeds.
>

Okay.  I have looked into patch proposed and found that it will
resolve the regression test problem (Create Table .. AS Execute).  I
think it is slightly prone to breakage if tomorrow we implement usage
of EXECUTE with statements other Create Table (like Copy).  Have you
registered this patch in CF to avoid loosing track?

We have two patches (one proposed in this thread and one proposed by
me earlier [1]) and both solves the regression test failure.  Unless
there is some better approach, I think we can go with one of these.

[1] - 
https://www.postgresql.org/message-id/CAA4eK1L%3DtHmmHDK_KW_ja1_dusJxJF%2BSGQHi%3DAPS4MdNPk7HFQ%40mail.gmail.com
-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com


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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-24 Thread Laurenz Albe
There has been a previous discussion about this topic, including an attempted 
fix by Amit Kapila:
http://www.postgresql.org/message-id/flat/CAA4eK1L=tHmmHDK_KW_ja1_dusJxJF+SGQHi=aps4mdnpk7...@mail.gmail.com/
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-11-21 Thread Tobias Bussmann
Am 18.11.2016 um 14:21 schrieb Albe Laurenz :
> But then the change to use CURSOR_OPT_PARALLEL_OK in tcop/postgres.c should
> be reverted as well, because it breaks things just as bad:
> 
>   /* creates a parallel-enabled plan */
>   PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
>   /* blows up with "cannot insert tuples during a parallel operation" */
>   PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");

Great example of mixing a v3 prepare with an simple query execute. I didn't 
think about that while even the docs state clearly: "Named prepared statements 
can also be created and accessed at the SQL command level, using PREPARE and 
EXECUTE." Sticking with either protocol version does not trigger the error. 


> I think we should either apply something like that patch or disable parallel
> execution for prepared statements altogether and document that.

So we likely need to backpatch something more then a doc-fix for 9.6. Given the 
patch proposals around, this would either disable a feature (in extended query 
protocol) or add a new one (in simple query protocol/SQL). Or would it be more 
appropriate to split the patch and use CURSOR_OPT_PARALLEL_OK in prepare.c on 
master only? I'm asking in case there is a necessity to prepare a proposal for 
an documentation patch targeting 9.6 specifically.

Best regards
Tobias




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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-21 Thread Tobias Bussmann

> True, but we also try to avoid it whenever possible, because it's
> likely to lead to poor performance.

This non-readonly case should be way less often hit compared to other uses of 
prepared statements. But sure, it depends on the individual use case and a 
likely performance regession in these edge cases is nothing to decide for 
easily.


> I think it would be a good idea to come up with a way for a query to
> produce both a parallel and a non-parallel plan and pick between them
> at execution time.  However, that's more work than I've been willing
> to undertake.

Wouldn't the precautionary generation of two plans always increase the planning 
overhead, which precisely is what one want to reduce by using prepared 
statements? 

Best regards
Tobias

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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-21 Thread Albe Laurenz
Robert Haas wrote:
>>/* creates a parallel-enabled plan */
>>PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
>>/* blows up with "cannot insert tuples during a parallel operation" */
>>PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");
> 
> Ouch.  I missed that.
> 
>> With Tobias' patch, this does not fail.
>>
>> I think we should either apply something like that patch or disable parallel
>> execution for prepared statements altogether and document that.
> 
> I agree.  I think probably the first option is better, but I might
> have to commit with one hand so I can hold my nose with the other.

Is there a better, more consistent approach for that?

Yours,
Laurenz Albe

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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-18 Thread Robert Haas
On Fri, Nov 18, 2016 at 8:21 AM, Albe Laurenz  wrote:
> I didn't notice the CREATE TABLE ... AS EXECUTE problem.
>
> But then the change to use CURSOR_OPT_PARALLEL_OK in tcop/postgres.c should
> be reverted as well, because it breaks things just as bad:
>
>/* creates a parallel-enabled plan */
>PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
>/* blows up with "cannot insert tuples during a parallel operation" */
>PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");

Ouch.  I missed that.

> With Tobias' patch, this does not fail.
>
> I think we should either apply something like that patch or disable parallel
> execution for prepared statements altogether and document that.

I agree.  I think probably the first option is better, but I might
have to commit with one hand so I can hold my nose with the other.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-11-18 Thread Albe Laurenz
Amit Kapila wrote:
> Can you once test by just passing CURSOR_OPT_PARALLEL_OK in
> PrepareQuery() and run the tests by having forc_parallel_mode=regress?
> It seems to me that the code in the planner is changed for setting a
> parallelModeNeeded flag, so it might work as it is.

No, that doesn't work.

But with Tobias' complete patch "make installcheck-world" succeeds.

Yours,
Laurenz Albe

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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-18 Thread Albe Laurenz
Robert Haas wrote:
> On Tue, Nov 15, 2016 at 10:41 AM, Albe Laurenz  
> wrote:
>> Tobias Bussmann has discovered an oddity with prepared statements.
>>
>> Parallel scan is used with prepared statements, but only if they have
>> been created with protocol V3 "Parse".
>> If a prepared statement has been prepared with the SQL statement PREPARE,
>> it will never use a parallel scan.
>>
>> I guess that is an oversight in commit 57a6a72b, right?
>> PrepareQuery in commands/prepare.c should call CompleteCachedPlan
>> with cursor options CURSOR_OPT_PARALLEL_OK, just like
>> exec_prepare_message in tcop/postgres.c does.
>>
>> The attached patch fixes the problem for me.
> 
> Actually, commit 57a6a72b made this change, and then 7bea19d0 backed
> it out again because it turned out to break things.

I didn't notice the CREATE TABLE ... AS EXECUTE problem.

But then the change to use CURSOR_OPT_PARALLEL_OK in tcop/postgres.c should
be reverted as well, because it breaks things just as bad:

   /* creates a parallel-enabled plan */
   PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
   /* blows up with "cannot insert tuples during a parallel operation" */
   PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");

With Tobias' patch, this does not fail.

I think we should either apply something like that patch or disable parallel
execution for prepared statements altogether and document that.

Yours,
Laurenz Albe


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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-17 Thread Robert Haas
On Thu, Nov 17, 2016 at 10:07 AM, Tobias Bussmann  wrote:
>> Yeah, we could do something like this, perhaps not in exactly this
>> way, but I'm not sure it's a good idea to just execute the parallel
>> plan without workers.
>
> sure, executing parallel plans w/o workers seems a bit of a hack. But:
> - we already do it this way in some other situations

True, but we also try to avoid it whenever possible, because it's
likely to lead to poor performance.

> - the alternative in this special situation would be to _force_ replanning 
> without the CURSOR_OPT_PARALLEL_OK. The decision for replanning is hidden 
> deep within plancache.c and while we could influence it with 
> CURSOR_OPT_CUSTOM_PLAN this wouldn't have an effect if the prepared statement 
> doesn't have any parameters. Additionally, influencing the decision and 
> generating a non-parallel plan would shift the avg cost calculation used to 
> choose custom or generic plans.

I think it would be a good idea to come up with a way for a query to
produce both a parallel and a non-parallel plan and pick between them
at execution time.  However, that's more work than I've been willing
to undertake.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-11-17 Thread Tobias Bussmann
> Yeah, we could do something like this, perhaps not in exactly this
> way, but I'm not sure it's a good idea to just execute the parallel
> plan without workers.

sure, executing parallel plans w/o workers seems a bit of a hack. But:
- we already do it this way in some other situations
- the alternative in this special situation would be to _force_ replanning 
without the CURSOR_OPT_PARALLEL_OK. The decision for replanning is hidden deep 
within plancache.c and while we could influence it with CURSOR_OPT_CUSTOM_PLAN 
this wouldn't have an effect if the prepared statement doesn't have any 
parameters. Additionally, influencing the decision and generating a 
non-parallel plan would shift the avg cost calculation used to choose custom or 
generic plans.

Maybe someone can come up with a better idea for a solution. These three 
approaches are all I see so far.

Best regards,
Tobias Bussmann

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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-16 Thread Robert Haas
On Tue, Nov 15, 2016 at 3:57 PM, Tobias Bussmann  wrote:
> As the patch in [1] targeting the execution of the plan in ExecutePlan 
> depending on the destination was declined, I hacked around a bit to find 
> another way to use parallel mode with SQL prepared statements while disabling 
> the parallel execution in case of an non read-only execution. For this I used 
> the already present test for an existing intoClause in ExecuteQuery to set 
> the parallelModeNeeded flag of the prepared statement. This results in a non 
> parallel execution of the parallel plan, as we see with a non-zero fetch 
> count used with the extended query protocol. Despite this patch seem to work 
> in my tests, I'm by no means confident this being a proper way of handling 
> the situation in question.

Yeah, we could do something like this, perhaps not in exactly this
way, but I'm not sure it's a good idea to just execute the parallel
plan without workers.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-11-16 Thread Tobias Bussmann
> Can you once test by just passing CURSOR_OPT_PARALLEL_OK in
> PrepareQuery() and run the tests by having forc_parallel_mode=regress?
> It seems to me that the code in the planner is changed for setting a
> parallelModeNeeded flag, so it might work as it is.

Do you mean running a "make installcheck" with "force_parallel_mode=regress" in 
postgresql.conf? I did so with just CURSOR_OPT_PARALLEL_OK in PrepareQuery 
(like the original commit 57a6a72b) and still got 3 failed tests, all with 
CREATE TABLE .. AS EXECUTE .. . With my patch applied, all tests were 
successful.



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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-16 Thread Robert Haas
On Tue, Nov 15, 2016 at 10:41 AM, Albe Laurenz  wrote:
> Tobias Bussmann has discovered an oddity with prepared statements.
>
> Parallel scan is used with prepared statements, but only if they have
> been created with protocol V3 "Parse".
> If a prepared statement has been prepared with the SQL statement PREPARE,
> it will never use a parallel scan.
>
> I guess that is an oversight in commit 57a6a72b, right?
> PrepareQuery in commands/prepare.c should call CompleteCachedPlan
> with cursor options CURSOR_OPT_PARALLEL_OK, just like
> exec_prepare_message in tcop/postgres.c does.
>
> The attached patch fixes the problem for me.

Actually, commit 57a6a72b made this change, and then 7bea19d0 backed
it out again because it turned out to break things.

-- 
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:
http://www.postgresql.org/mailpref/pgsql-hackers


Re: [HACKERS] Parallel execution and prepared statements

2016-11-16 Thread Amit Kapila
On Wed, Nov 16, 2016 at 7:10 PM, Amit Kapila  wrote:
> On Wed, Nov 16, 2016 at 2:27 AM, Tobias Bussmann  wrote:
>> As the patch in [1] targeting the execution of the plan in ExecutePlan 
>> depending on the destination was declined, I hacked around a bit to find 
>> another way to use parallel mode with SQL prepared statements while 
>> disabling the parallel execution in case of an non read-only execution. For 
>> this I used the already present test for an existing intoClause in 
>> ExecuteQuery to set the parallelModeNeeded flag of the prepared statement. 
>> This results in a non parallel execution of the parallel plan, as we see 
>> with a non-zero fetch count used with the extended query protocol. Despite 
>> this patch seem to work in my tests, I'm by no means confident this being a 
>> proper way of handling the situation in question.
>>
>
> Can you once test by just passing CURSOR_OPT_PARALLEL_OK in
> PrepareQuery() and run the tests by having forc_parallel_mode=regress?

Typo.
/forc_parallel_mode/force_parallel_mode


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


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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-16 Thread Amit Kapila
On Wed, Nov 16, 2016 at 2:27 AM, Tobias Bussmann  wrote:
> As the patch in [1] targeting the execution of the plan in ExecutePlan 
> depending on the destination was declined, I hacked around a bit to find 
> another way to use parallel mode with SQL prepared statements while disabling 
> the parallel execution in case of an non read-only execution. For this I used 
> the already present test for an existing intoClause in ExecuteQuery to set 
> the parallelModeNeeded flag of the prepared statement. This results in a non 
> parallel execution of the parallel plan, as we see with a non-zero fetch 
> count used with the extended query protocol. Despite this patch seem to work 
> in my tests, I'm by no means confident this being a proper way of handling 
> the situation in question.
>

Can you once test by just passing CURSOR_OPT_PARALLEL_OK in
PrepareQuery() and run the tests by having forc_parallel_mode=regress?
It seems to me that the code in the planner is changed for setting a
parallelModeNeeded flag, so it might work as it is.

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


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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-15 Thread Tobias Bussmann
As the patch in [1] targeting the execution of the plan in ExecutePlan 
depending on the destination was declined, I hacked around a bit to find 
another way to use parallel mode with SQL prepared statements while disabling 
the parallel execution in case of an non read-only execution. For this I used 
the already present test for an existing intoClause in ExecuteQuery to set the 
parallelModeNeeded flag of the prepared statement. This results in a non 
parallel execution of the parallel plan, as we see with a non-zero fetch count 
used with the extended query protocol. Despite this patch seem to work in my 
tests, I'm by no means confident this being a proper way of handling the 
situation in question.

Best 
Tobias 

[1] 
https://www.postgresql.org/message-id/CAA4eK1KxiYm8F9Pe9xvqzoZocK43w%3DTRPUNHZpe_iOjF%3Dr%2B_Vw%40mail.gmail.com



prepared_stmt_execute_parallel_query.patch
Description: Binary data




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


Re: [HACKERS] Parallel execution and prepared statements

2016-11-15 Thread Tobias Bussmann
Thanks Laurenz, for your help with debugging on this topic. I was preparing a 
message to the list myself and found an earlier discussion [1] on the topic. If 
I understand it correctly, the issue is with CREATE TABLE ... AS EXECUTE  
So it seems parallel execution of prepared statements was intentionally 
disabled in 7bea19d. However, what is missing is at least a mention of that 
current limitation in the 9.6 docs at "When Can Parallel Query Be Used?" [2] 

Best regards,
Tobias

[1] 
https://www.postgresql.org/message-id/CA%2BTgmoaxyXVr6WPDvPQduQpFhD9VRWExXU7axhDpJ7jZBvqxfQ%40mail.gmail.com
[2] 
https://www.postgresql.org/docs/current/static/when-can-parallel-query-be-used.html

> Am 15.11.2016 um 16:41 schrieb Albe Laurenz :
> 
> Tobias Bussmann has discovered an oddity with prepared statements.
> 
> Parallel scan is used with prepared statements, but only if they have
> been created with protocol V3 "Parse".
> If a prepared statement has been prepared with the SQL statement PREPARE,
> it will never use a parallel scan.
> 
> I guess that is an oversight in commit 57a6a72b, right?
> PrepareQuery in commands/prepare.c should call CompleteCachedPlan
> with cursor options CURSOR_OPT_PARALLEL_OK, just like
> exec_prepare_message in tcop/postgres.c does.
> 
> The attached patch fixes the problem for me.
> 
> Yours,
> Laurenz Albe
> <0001-Consider-parallel-plans-for-statements-prepared-with.patch>
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers



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


[HACKERS] Parallel execution and prepared statements

2016-11-15 Thread Albe Laurenz
Tobias Bussmann has discovered an oddity with prepared statements.

Parallel scan is used with prepared statements, but only if they have
been created with protocol V3 "Parse".
If a prepared statement has been prepared with the SQL statement PREPARE,
it will never use a parallel scan.

I guess that is an oversight in commit 57a6a72b, right?
PrepareQuery in commands/prepare.c should call CompleteCachedPlan
with cursor options CURSOR_OPT_PARALLEL_OK, just like
exec_prepare_message in tcop/postgres.c does.

The attached patch fixes the problem for me.

Yours,
Laurenz Albe


0001-Consider-parallel-plans-for-statements-prepared-with.patch
Description: 0001-Consider-parallel-plans-for-statements-prepared-with.patch

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