Re: [HACKERS] FATAL: ReleaseSavepoint: unexpected state STARTED

2011-08-19 Thread Robert Haas
On Thu, Aug 18, 2011 at 3:57 AM, Marcin Mańk marcin.m...@gmail.com wrote:
 On Wed, Aug 17, 2011 at 11:30 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 =?UTF-8?B?TWFyY2luIE1hxYRr?= marcin.m...@gmail.com writes:
  psql -c 'release q; prepare q(int) as select 1'
 FATAL:  ReleaseSavepoint: unexpected state STARTED

 Can't get terribly excited about that, seeing that the statement is
 surely going to draw an error and abort processing the rest of the
 command string in any case ...

 Oh, I thought FATAL was disconnectiong other sessions. Yeah, that was
 not bad enough.

 Here is a better one:

 psql postgres -c 'savepoint q; select 1'
 FATAL:  DefineSavepoint: unexpected state STARTED
 server closed the connection unexpectedly
        This probably means the server terminated abnormally
        before or while processing the request.

I spent some time looking at this afternoon and it appears that the
root of this problem is that we're a bit schizophrenic about whether a
multi-query command string constitutes a transaction or not.For
example, DECLARE CURSOR works fine in that context, and you can fetch
from the cursor.  Since that command normally only works from within a
transaction, you might reasonably conclude hey, we're in a
transaction.   Furthermore, commands that can't be run from with a
transaction context are fenced out here, too - e.g. VACUUM.  The error
message that you get there shows that someone thought about this
specific case:

[rhaas pgsql]$ psql -c 'SELECT 1;VACUUM'
ERROR:  VACUUM cannot be executed from a function or multi-command string

The transaction control commands are not on the same page.  BEGIN
thinks we're not in a transaction, so you can use BEGIN to start one.
If you do that then things are pretty normal.  Phew.  But let's say
you don't use BEGIN.  ROLLBACK claims that you aren't in a transaction
and therefore you can't roll back:

[rhaas pgsql]$ psql -c 'create table xyz(); rollback'
NOTICE:  there is no transaction in progress
ROLLBACK

But if there really were no transaction in progress at the point where
ROLLBACK was issued, then the rollback wouldn't do anything.  In fact,
it does do something: it prevents xyz from getting created.  Crazily
enough, it does this without aborting the execution of the remaining
commands:

[rhaas pgsql]$ psql -c 'create table xyz(); rollback; select 1'
NOTICE:  there is no transaction in progress
 ?column?
--
1
(1 row)

So, from ROLLBACK's perspective, you are sort of in a transaction, and
sort of not in a transaction.

SAVEPOINT and ROLLBACK TO SAVEPOINT are normally called after
RequireTransactionChain(), so that they can only be called from within
a transaction block.  But exec_simple_query is passing down isTopLevel
= true, so RequireTransactionChain() eventually gets that value in the
mail and says oh, good.  we're in a transaction.  these commands can
be allowed to work!  But then when the functions actually
implementing those commands are invoked, they view TBLOCK_STARTED as
unacceptable, and a FATAL error results.

If backward compatibility were no object, I'd be tempted to deal with
this by turning a multi-query command string into a full-fledged
transaction.  But that would break things like psql -c 'BEGIN; ...do
stuff...; COMMIT; BEGIN; ...do other stuff...; COMMIT', which is
probably common enough that we shouldn't indiscriminately break it.
So I'm inclined to think that the problem - at least as far as
SAVEPOINT and ROLLBACK TO SAVEPOINT are concerned - is that
RequireTransactionChain() really can't just be the mirror image of
PreventTransactionChain().  When you're in a transaction,
PreventTransactionChain() should be unhappy; when you're outside one,
RequireTransactionChain() should be unhappy; and when you're in this
intermediate state inside of a multi-command string, BOTH of them
should be unhappy.  I'm not sure whether we need to go through and
replace isTopLevel with a three-valued flag, or whether we can just
delete the isTopLevel test from RequireTransactionChain() altogether.
The latter seems like it might do the trick, but I haven't puzzled
through all the logic yet.

As for ROLLBACK, I think it should chuck an error instead of doing
this funny 
emit-a-warning-and-silently-arrange-for-the-transaction-to-be-aborted-later
thing.

-- 
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] FATAL: ReleaseSavepoint: unexpected state STARTED

2011-08-19 Thread Tom Lane
Robert Haas robertmh...@gmail.com writes:
 I spent some time looking at this afternoon and it appears that the
 root of this problem is that we're a bit schizophrenic about whether a
 multi-query command string constitutes a transaction or not.

Yeah.  The current behavior sort of automatically adds a BEGIN and a
COMMIT around the string, but it's evidently not tied into the real
BEGIN and COMMIT commands well enough.

 As for ROLLBACK, I think it should chuck an error instead of doing
 this funny 
 emit-a-warning-and-silently-arrange-for-the-transaction-to-be-aborted-later
 thing.

I'm pretty unexcited about changing the behavior of established
mainstream cases just so that we can throw slightly-more-meaningful
errors in the psql -c case.  ROLLBACK when not in a transaction is not
an error, only a NOTICE, and it should stay that way.  If you change
that there are going to be a lot of application and driver authors on
your doorstep with the usual equipment.

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] FATAL: ReleaseSavepoint: unexpected state STARTED

2011-08-19 Thread Robert Haas
On Fri, Aug 19, 2011 at 3:31 PM, Tom Lane t...@sss.pgh.pa.us wrote:
 As for ROLLBACK, I think it should chuck an error instead of doing
 this funny 
 emit-a-warning-and-silently-arrange-for-the-transaction-to-be-aborted-later
 thing.

 I'm pretty unexcited about changing the behavior of established
 mainstream cases just so that we can throw slightly-more-meaningful
 errors in the psql -c case.  ROLLBACK when not in a transaction is not
 an error, only a NOTICE, and it should stay that way.  If you change
 that there are going to be a lot of application and driver authors on
 your doorstep with the usual equipment.

I guess.  It's totally inconsistent, though.  COMMIT emits a WARNING,
ROLLBACK emits a NOTICE, and SAVEPOINT and ROLLBACK TO SAVEPOINT emit
FATAL.   Maybe we should add some commands that throw PANIC and DEBUG1
just for good measure.

The thing that really bothers me is that the way ROLLBACK rolls the
transaction back, but only half-way.  It would be reasonable for it to
JUST emit a notice.  And it would be reasonable for it to error out
and abort the execution of the command string.  But allowing the
execution of the command string to continue while arranging for it to
abort at the end is really pretty strange.

-- 
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


[HACKERS] FATAL: ReleaseSavepoint: unexpected state STARTED

2011-08-17 Thread Marcin Mańk
Hello
I tried reporting the following bug via web form, it somerhow got lost
(it is not in pgsql-bugs archives, it was #6157 I believe). Anyway,
here it is:


 psql -c 'release q; prepare q(int) as select 1'
FATAL:  ReleaseSavepoint: unexpected state STARTED
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
connection to server was lost

The message is from 8.4.2, but the bug is in 9.0.4 too .

Greetings
Marcin Mańk

-- 
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] FATAL: ReleaseSavepoint: unexpected state STARTED

2011-08-17 Thread Tom Lane
=?UTF-8?B?TWFyY2luIE1hxYRr?= marcin.m...@gmail.com writes:
  psql -c 'release q; prepare q(int) as select 1'
 FATAL:  ReleaseSavepoint: unexpected state STARTED

Can't get terribly excited about that, seeing that the statement is
surely going to draw an error and abort processing the rest of the
command string in any case ...

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