Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-13 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Tom Lane > The originally reported bug is fixed. Not making any claims about other > bugs ... I'm sorry I couldn't reply to you. I've recently been in a situation where I can't use my time for de

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-08 Thread Tom Lane
Catalin Iacob writes: > When reading this I also realized that the backend does send responses for > every individual query in a multi-query request, it's only libpq's PQexec > that throws away the intermediate results and only provides access to the > last one. If you want to see them all, you c

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-08 Thread Catalin Iacob
On Thu, Sep 7, 2017 at 8:07 PM, Tom Lane wrote: > I've pushed up an attempt at this: > > https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b976499480bdbab6d69a11e47991febe53865adc > > Feel free to suggest improvements. Thank you, this helps a lot. Especially since some of the beh

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Tom Lane
Simon Riggs writes: > On 7 September 2017 at 11:31, Tom Lane wrote: >> Haas' idea of some kind of syntactic extension, like "LET guc1 = x, >> guc2 = y FOR statement" seems more feasible to me. I'm not necessarily >> wedded to that particular syntax, but I think it has to look like >> a single-st

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Tom Lane
Simon Riggs writes: > On 7 September 2017 at 11:24, Tom Lane wrote: >> Not hearing anything, I already pushed my patch an hour or three ago. > Yes, I saw. Are you saying that doc commit is all we need? ISTM we > still had an actual bug. The originally reported bug is fixed. Not making any clai

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Simon Riggs
On 7 September 2017 at 11:31, Tom Lane wrote: > Simon Riggs writes: >> I would like to relax the restriction to allow this specific use case... >> SET work_mem = X; SET max_parallel_workers = 4; SELECT ... >> so we still have only one command (the last select), yet we have >> multiple GUC setti

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Simon Riggs
On 7 September 2017 at 11:24, Tom Lane wrote: > Simon Riggs writes: >> On 5 September 2017 at 10:22, Tom Lane wrote: >>> Does anyone want to do further review on this patch? If so, I'll >>> set the CF entry back to "Needs Review". > >> OK, I'll review Michael's patch (and confirm my patch is de

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Tom Lane
Simon Riggs writes: > I would like to relax the restriction to allow this specific use case... > SET work_mem = X; SET max_parallel_workers = 4; SELECT ... > so we still have only one command (the last select), yet we have > multiple GUC settings beforehand. On what basis do you claim that's on

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Tom Lane
Simon Riggs writes: > On 5 September 2017 at 10:22, Tom Lane wrote: >> Does anyone want to do further review on this patch? If so, I'll >> set the CF entry back to "Needs Review". > OK, I'll review Michael's patch (and confirm my patch is dead) Not hearing anything, I already pushed my patch a

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Simon Riggs
On 7 September 2017 at 11:07, Tom Lane wrote: > I wrote: >> Yeah, it seems like we have now made this behavior official enough that >> it's time to document it better. My thought is to create a new subsection >> in the FE/BE Protocol chapter that explains how multi-statement Query >> messages are

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Simon Riggs
On 5 September 2017 at 10:22, Tom Lane wrote: > Michael Paquier writes: >> On Mon, Sep 4, 2017 at 11:15 PM, Tom Lane wrote: >>> I don't want to go there, and was thinking we should expand the new >>> comment in DefineSavepoint to explain why not. > >> Okay. > > Does anyone want to do further rev

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Tom Lane
I wrote: > Yeah, it seems like we have now made this behavior official enough that > it's time to document it better. My thought is to create a new subsection > in the FE/BE Protocol chapter that explains how multi-statement Query > messages are handled, and then to link to that from appropriate p

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-07 Thread Tom Lane
Catalin Iacob writes: > On Mon, Sep 4, 2017 at 4:15 PM, Tom Lane wrote: >> Also, the main thing that we need xact.c's involvement for in the first >> place is the fact that implicit transaction blocks, unlike regular ones, >> auto-cancel on an error, leaving you outside a block not inside a faile

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-05 Thread Catalin Iacob
On Mon, Sep 4, 2017 at 4:15 PM, Tom Lane wrote: > Also, the main thing that we need xact.c's involvement for in the first > place is the fact that implicit transaction blocks, unlike regular ones, > auto-cancel on an error, leaving you outside a block not inside a failed > one. So I don't exactly

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-05 Thread Tom Lane
Michael Paquier writes: > On Mon, Sep 4, 2017 at 11:15 PM, Tom Lane wrote: >> I don't want to go there, and was thinking we should expand the new >> comment in DefineSavepoint to explain why not. > Okay. Does anyone want to do further review on this patch? If so, I'll set the CF entry back to

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-04 Thread Michael Paquier
On Mon, Sep 4, 2017 at 11:15 PM, Tom Lane wrote: > I don't want to go there, and was thinking we should expand the new > comment in DefineSavepoint to explain why not. Okay. > It's certainly not that > much additional work to allow a savepoint so far as xact.c is concerned, > as your patch shows

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-04 Thread Tom Lane
Michael Paquier writes: > Hmm. While this patch looks to me in a better shape than what Simon's > is proposing, thinking about > cah2-v61vxnentfj2v-zd+ma-g6kqmjgd5svxou3jbvdzqh0...@mail.gmail.com > which involved a migration Oracle->Postgres, I have been wondering if > it is possible to still allo

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-03 Thread Michael Paquier
On Mon, Sep 4, 2017 at 7:20 AM, Tom Lane wrote: > I wrote: > On further consideration, I think the control logic I added in > exec_simple_query() is a shade bogus. I set it up to only force > an implicit transaction block when there are at least two statements > remaining to execute. However, th

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-03 Thread Tom Lane
I wrote: > ... PFA a patch > that invents a notion of an "implicit" transaction block. On further consideration, I think the control logic I added in exec_simple_query() is a shade bogus. I set it up to only force an implicit transaction block when there are at least two statements remaining to e

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-02 Thread Tom Lane
I wrote: > My thought is that what we need to do is find a way for isTopLevel > to be false if we're processing a multi-command string. Nah, that's backwards, the problem is exactly that isTopLevel is false if we're processing a multi-command string. That allows DefineSavepoint to think that it's

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-01 Thread Tom Lane
Simon Riggs writes: > On 1 September 2017 at 15:19, Tom Lane wrote: >> This patch makes me itch. Why is it correct for these three checks, >> and only these three checks out of the couple dozen uses of isTopLevel >> in standard_ProcessUtility, to instead do something else? > No problem, it was

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-01 Thread Simon Riggs
On 1 September 2017 at 15:19, Tom Lane wrote: > Simon Riggs writes: >> I've added tests to the recent patch to show it works. > > I don't think those test cases prove anything (ie, they work fine > on an unpatched server). With a backslash maybe they would. > >> Any objection to me backpatching

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-01 Thread Tom Lane
Simon Riggs writes: > I've added tests to the recent patch to show it works. I don't think those test cases prove anything (ie, they work fine on an unpatched server). With a backslash maybe they would. > Any objection to me backpatching this, please say. This patch makes me itch. Why is it c

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-01 Thread Simon Riggs
On 1 September 2017 at 08:09, Michael Paquier wrote: > On Fri, Sep 1, 2017 at 3:05 PM, Simon Riggs wrote: >> I'm not sure I see the use case for anyone using SAVEPOINTs in this >> context, so simply throwing a good error message is enough. >> >> Clearly nobody is using this, so lets just lock the

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-09-01 Thread Michael Paquier
On Fri, Sep 1, 2017 at 3:05 PM, Simon Riggs wrote: > I'm not sure I see the use case for anyone using SAVEPOINTs in this > context, so simply throwing a good error message is enough. > > Clearly nobody is using this, so lets just lock the door. I don't > think fiddling with the transaction block s

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-08-31 Thread Simon Riggs
On 17 May 2017 at 08:38, Tsunakawa, Takayuki wrote: > From: Michael Paquier [mailto:michael.paqu...@gmail.com] >> On Fri, Mar 31, 2017 at 9:58 PM, Ashutosh Bapat >> wrote: >> > Then the question is why not to allow savepoints as well? For that we >> > have to fix transaction block state machine.

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-05-17 Thread Tsunakawa, Takayuki
From: Michael Paquier [mailto:michael.paqu...@gmail.com] > On Fri, Mar 31, 2017 at 9:58 PM, Ashutosh Bapat > wrote: > > Then the question is why not to allow savepoints as well? For that we > > have to fix transaction block state machine. > > I agree with this argument. I have been looking at the

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-05-17 Thread Michael Paquier
On Fri, Mar 31, 2017 at 9:58 PM, Ashutosh Bapat wrote: > Then the question is why not to allow savepoints as well? For that we > have to fix transaction block state machine. I agree with this argument. I have been looking at the patch, and what it does is definitely incorrect. Any query string in

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-04-02 Thread Tsunakawa, Takayuki
From: pgsql-hackers-ow...@postgresql.org > [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Alvaro Herrera > Ashutosh Bapat wrote: > > Please add this to the next commitfest. > > If this cannot be reproduced in 9.6, then it must be added to the Open Items > wiki page instead. I added this

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-03-31 Thread Michael Paquier
On Sat, Apr 1, 2017 at 1:06 AM, Alvaro Herrera wrote: > Ashutosh Bapat wrote: >> Please add this to the next commitfest. > > If this cannot be reproduced in 9.6, then it must be added to the > Open Items wiki page instead. The behavior reported can be reproduced further down (just tried on 9.3, g

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-03-31 Thread Alvaro Herrera
Ashutosh Bapat wrote: > Please add this to the next commitfest. If this cannot be reproduced in 9.6, then it must be added to the Open Items wiki page instead. -- Álvaro Herrerahttps://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services --

Re: [HACKERS] [bug fix] Savepoint-related statements terminates connection

2017-03-31 Thread Ashutosh Bapat
Please add this to the next commitfest. I think there's some misunderstanding between exec_simple_query() and the way we manage transaction block state machine. In exec_simple_query() 952 * We'll tell PortalRun it's a top-level command iff there's exactly one 953 * raw parsetree. If