Обсуждение: TODO: Add a GUC to control whether BEGIN inside a transcation should abort the transaction.

Hello,

Is this really a TODO or is this someone being overzealous with the TODO
list?

Joshua D. Drake

-- 
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive  PostgreSQL solutions since 1997            http://www.commandprompt.com/

Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate

Joshua D. Drake wrote:
> Hello,
> 
> Is this really a TODO or is this someone being overzealous with the TODO
> list?

>  Add a GUC to control whether BEGIN inside a transcation should abort
> the transaction.

Well, right now, BEGIN inside a transaction just issues a warning:
test=> BEGIN;BEGINtest=> BEGIN;WARNING:  there is already a transaction in progressBEGINtest=> SELECT 1;
?column?----------       1(1 row)test=> COMMIT;COMMIT
 

I think you can make the case that this should be an error, or at least
that's how it got on the TODO list.  I can always remove it if people
don't want the item completed.

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

>             1
>     (1 row)
>     
>     test=> COMMIT;
>     COMMIT
> 
> I think you can make the case that this should be an error, or at least
> that's how it got on the TODO list.  I can always remove it if people
> don't want the item completed.

Well I can tell you that my customers who are postgresql users ;) would
howl in fury if we did that. They are already significantly irritated
that certain errors are so strict. E.g.,

postgres=# BEGIN;
BEGIN
postgres=# ALTER TABLE baz ADD COLUMN bar text;
ERROR:  relation "baz" does not exist
postgres=# SELECT * FROM foo;
ERROR:  current transaction is aborted, commands ignored until end of
transaction block

You do not need to argue with me about the purpose :), I understand why
it is just really frustrating for many users.

I would say that a GUC variable for such behavior as listed in the TODO
is overzealous. We should either enforce it, or not. As we do not now,
there is no reason imo to change it.

Sincerely,

Joshua D. Drake

-- 
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive  PostgreSQL solutions since 1997            http://www.commandprompt.com/

Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate

"Joshua D. Drake" <jd@commandprompt.com> writes:
> I would say that a GUC variable for such behavior as listed in the TODO
> is overzealous. We should either enforce it, or not. As we do not now,
> there is no reason imo to change it.

Not only is it overzealous, but the proposal to have one reflects a
failure to learn from history.  GUC variables that change
transaction-boundary semantics are a bad idea, period: see autocommit.
        regards, tom lane

On Thu, 2006-12-28 at 13:52 -0500, Tom Lane wrote:
> "Joshua D. Drake" <jd@commandprompt.com> writes:
> > I would say that a GUC variable for such behavior as listed in the TODO
> > is overzealous. We should either enforce it, or not. As we do not now,
> > there is no reason imo to change it.
> 
> Not only is it overzealous, but the proposal to have one reflects a
> failure to learn from history.  GUC variables that change
> transaction-boundary semantics are a bad idea, period: see autocommit.

Nod. Let's get this TODO removed.

Sincerely,

Joshua D. Drake


> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 4: Have you searched our list archives?
> 
>                http://archives.postgresql.org
> 
-- 
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive  PostgreSQL solutions since 1997            http://www.commandprompt.com/

Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate

Joshua D. Drake wrote:
> On Thu, 2006-12-28 at 13:52 -0500, Tom Lane wrote:
> > "Joshua D. Drake" <jd@commandprompt.com> writes:
> > > I would say that a GUC variable for such behavior as listed in the TODO
> > > is overzealous. We should either enforce it, or not. As we do not now,
> > > there is no reason imo to change it.
> > 
> > Not only is it overzealous, but the proposal to have one reflects a
> > failure to learn from history.  GUC variables that change
> > transaction-boundary semantics are a bad idea, period: see autocommit.
> 
> Nod. Let's get this TODO removed.

OK, removed.

--  Bruce Momjian   bruce@momjian.us EnterpriseDB    http://www.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

On Thursday 28 December 2006 15:44, Bruce Momjian wrote:
> Joshua D. Drake wrote:
> > On Thu, 2006-12-28 at 13:52 -0500, Tom Lane wrote:
> > > "Joshua D. Drake" <jd@commandprompt.com> writes:
> > > > I would say that a GUC variable for such behavior as listed in the
> > > > TODO is overzealous. We should either enforce it, or not. As we do
> > > > not now, there is no reason imo to change it.
> > >
> > > Not only is it overzealous, but the proposal to have one reflects a
> > > failure to learn from history.  GUC variables that change
> > > transaction-boundary semantics are a bad idea, period: see autocommit.
> >
> > Nod. Let's get this TODO removed.
>
> OK, removed.

I thought this was needed for spec compliance?  If we have no plans to even 
attempt to support it, istm that ought to be noted someplace. 

-- 
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Robert Treat <xzilla@users.sourceforge.net> writes:
> I thought this was needed for spec compliance?

BEGIN isn't in the spec at all ...

Now you could point to the spec for START TRANSACTION, which saith
        1) If a <start transaction statement> statement is executed when an           SQL-transaction is currently
active,then an exception condition           is raised: invalid transaction state - active SQL-transaction.
 

However, seeing that the spec doesn't think that an exception necessarily
aborts the transaction, I think the case for saying that ERROR is more
spec-compliant here than WARNING is mighty thin.

And if you want to hold our feet to the fire about whether errors abort
transactions or not, this particular point is not one thousandth part
of what would need to change.
        regards, tom lane

Am Donnerstag, 28. Dezember 2006 19:52 schrieb Tom Lane:
> Not only is it overzealous, but the proposal to have one reflects a
> failure to learn from history.  GUC variables that change
> transaction-boundary semantics are a bad idea, period: see autocommit.

But this option would not, in fact, change the transaction-boundary semantics.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

Am Donnerstag, 28. Dezember 2006 18:57 schrieb Bruce Momjian:
> I think you can make the case that this should be an error, or at least
> that's how it got on the TODO list.  I can always remove it if people
> don't want the item completed.

The reason this was added is that modular applications expect that a locally 
issued BEGIN ... COMMIT executes a transaction, whereas now you don't know 
what you're getting.  I think this change would have merit.

-- 
Peter Eisentraut
http://developer.postgresql.org/~petere/

Peter Eisentraut <peter_e@gmx.net> writes:
> Am Donnerstag, 28. Dezember 2006 18:57 schrieb Bruce Momjian:
>> I think you can make the case that this should be an error, or at least
>> that's how it got on the TODO list.  I can always remove it if people
>> don't want the item completed.

> The reason this was added is that modular applications expect that a locally 
> issued BEGIN ... COMMIT executes a transaction, whereas now you don't know 
> what you're getting.  I think this change would have merit.

But how is making BEGIN an error going to improve life for such an
application?  It'll be just as broken.  In fact, if the app depends on
getting an error from BEGIN in any critical way, it'll be *more* broken
if it's ever run under the default warning-only setting.
        regards, tom lane

Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> Am Donnerstag, 28. Dezember 2006 18:57 schrieb Bruce Momjian:
>>> I think you can make the case that this should be an error, or at least
>>> that's how it got on the TODO list.  I can always remove it if people
>>> don't want the item completed.
> 
>> The reason this was added is that modular applications expect that a locally 
>> issued BEGIN ... COMMIT executes a transaction, whereas now you don't know 
>> what you're getting.  I think this change would have merit.
> 
> But how is making BEGIN an error going to improve life for such an
> application?  It'll be just as broken.  In fact, if the app depends on
> getting an error from BEGIN in any critical way, it'll be *more* broken
> if it's ever run under the default warning-only setting.

While we are on the topic, I have implemented a poor mans nested 
transaction feature into my database access layer. essentially 
subsequent calls to begin a transaction after the initial begin simply 
increase an internal counter and set a savepoint. as you commit the 
transactions the counter is decreased and the savepoints are released. 
maybe this could be implemented inside postgresql to make the life of 
modular programmers easier?

regards,
Lukas

news.postgresql.org wrote:

> While we are on the topic, I have implemented a poor mans nested 
> transaction feature into my database access layer. essentially 
> subsequent calls to begin a transaction after the initial begin simply 
> increase an internal counter and set a savepoint. as you commit the 
> transactions the counter is decreased and the savepoints are released. 
> maybe this could be implemented inside postgresql to make the life of 
> modular programmers easier?

Yeah, it's called "SAVEPOINT foo" and "RELEASE foo".

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:
> news.postgresql.org wrote:
> 
>> While we are on the topic, I have implemented a poor mans nested 
>> transaction feature into my database access layer. essentially 
>> subsequent calls to begin a transaction after the initial begin simply 
>> increase an internal counter and set a savepoint. as you commit the 
>> transactions the counter is decreased and the savepoints are released. 
>> maybe this could be implemented inside postgresql to make the life of 
>> modular programmers easier?
> 
> Yeah, it's called "SAVEPOINT foo" and "RELEASE foo".

Err, I think you misunderstood what I said. My implementation uses 
SAVEPOINTs already. The point is having some API where you do not have 
to care of you are already in a transaction or not. Depending on if you 
are it will either open a new transaction or simply place a savepoint.

with the following invented commands:
MBEGIN FOO1 // open transaction; set counter to 1
MBEGIN FOO2 // set savepoint FOO2; set counter to 2
MBEGIN FOO3 // set savepoint FOO3; set counter to 3
MROLLBACK FOO3 // rollback to FOO3; set counter to 2
MCOMMIT FOO2 // release FOO2; set counter to 1
MCOMMIT FOO1 // commit

regards,
Lukas

regards,
Lukas

On Tue, 2007-01-02 at 11:53 +0100, Peter Eisentraut wrote:
> Am Donnerstag, 28. Dezember 2006 18:57 schrieb Bruce Momjian:
> > I think you can make the case that this should be an error, or at least
> > that's how it got on the TODO list.  I can always remove it if people
> > don't want the item completed.
> 
> The reason this was added is that modular applications expect that a locally 
> issued BEGIN ... COMMIT executes a transaction, whereas now you don't know 
> what you're getting.  I think this change would have merit.

Interesting point. My only comment is, "Do it one way or the other,
don't give me a user or a distribution packager a foot gun."

E.g., no GUC parameter. Just change the behavior or don't.

Joshua D. Drake


> 
-- 
     === The PostgreSQL Company: Command Prompt, Inc. ===
Sales/Support: +1.503.667.4564 || 24x7/Emergency: +1.800.492.2240
Providing the most comprehensive  PostgreSQL solutions since 1997            http://www.commandprompt.com/

Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate

Lukas Kahwe Smith <smith@pooteeweet.org> writes:
> Err, I think you misunderstood what I said. My implementation uses 
> SAVEPOINTs already. The point is having some API where you do not have 
> to care of you are already in a transaction or not.

It's not that hard, is it?
if (PQtransactionStatus(conn) == PQTRANS_IDLE)    PQexec(conn, "BEGIN");else    PQexec(conn, "SAVEPOINT foo");
        regards, tom lane

On 1/2/07, Joshua D. Drake <jd@commandprompt.com> wrote:
Please refer the conversation beginning at:
http://archives.postgresql.org/pgsql-hackers/2006-05/msg00249.php

That is where this TODO item came from. In the conversation, it was understood that such a change would break many applications, hence one of the option was to introduce a GUC var and keep it on by default, and a couple of releases later, remove the GUC and make the behaviour default ( http://archives.postgresql.org/pgsql-hackers/2006-05/msg00273.php).

This would help the programmers can realize that they are doing something wrong, and they can make appropriate changes to their code.

The usability of the GUC comes in a production environment, where it is not possible to change the application. The DBA can buy some time by turning the GUC var off.

I submitted a patch, which was incorrect and incomplete as I was _very_ new to PGSQL. I could not follow up on it as I was switching jobs at the time.

Tom objected to the default=ON setting for the GUC.

The TODO has been declared "misconceived", but I guess there's still interest out here. Would like to finish it once we reach a consensus.

Best regards,

--
gurjeet[.singh]@EnterpriseDB.com
singh.gurjeet@{ gmail | hotmail | yahoo }.com

Tom Lane wrote:
> Lukas Kahwe Smith <smith@pooteeweet.org> writes:
>> Err, I think you misunderstood what I said. My implementation uses 
>> SAVEPOINTs already. The point is having some API where you do not have 
>> to care of you are already in a transaction or not.
> 
> It's not that hard, is it?
> 
>     if (PQtransactionStatus(conn) == PQTRANS_IDLE)
>         PQexec(conn, "BEGIN");
>     else
>         PQexec(conn, "SAVEPOINT foo");

Its not exactly convenient either, especially in the case of modular 
code that may be developed by different people. Anyways, like I said I 
have a solution in my framework to make life of module developers 
easier. Obviously proper nested transactions would be the ideal, but so 
it goes. I was just throwing this out here when I saw Peter's comment.

regards,
Lukas

Tom Lane wrote:
> Peter Eisentraut <peter_e@gmx.net> writes:
>> Am Donnerstag, 28. Dezember 2006 18:57 schrieb Bruce Momjian:
>>> I think you can make the case that this should be an error, or at least
>>> that's how it got on the TODO list.  I can always remove it if people
>>> don't want the item completed.
> 
>> The reason this was added is that modular applications expect that a locally 
>> issued BEGIN ... COMMIT executes a transaction, whereas now you don't know 
>> what you're getting.  I think this change would have merit.
> 
> But how is making BEGIN an error going to improve life for such an
> application?  It'll be just as broken.  In fact, if the app depends on
> getting an error from BEGIN in any critical way, it'll be *more* broken
> if it's ever run under the default warning-only setting.

While we are on the topic, I have implemented a poor mans nested 
transaction feature into my database access layer. essentially 
subsequent calls to begin a transaction after the initial begin simply 
increase an internal counter and set a savepoint. as you commit the 
transactions the counter is decreased and the savepoints are released. 
maybe this could be implemented inside postgresql to make the life of 
modular programmers easier?

regards,
Lukas

Alvaro Herrera wrote:
> news.postgresql.org wrote:
> 
>> While we are on the topic, I have implemented a poor mans nested 
>> transaction feature into my database access layer. essentially 
>> subsequent calls to begin a transaction after the initial begin simply 
>> increase an internal counter and set a savepoint. as you commit the 
>> transactions the counter is decreased and the savepoints are released. 
>> maybe this could be implemented inside postgresql to make the life of 
>> modular programmers easier?
> 
> Yeah, it's called "SAVEPOINT foo" and "RELEASE foo".

Err, I think you misunderstood what I said. My implementation uses 
SAVEPOINTs already. The point is having some API where you do not have 
to care of you are already in a transaction or not. Depending on if you 
are it will either open a new transaction or simply place a savepoint.

with the following invented commands:
MBEGIN FOO1 // open transaction; set counter to 1
MBEGIN FOO2 // set savepoint FOO2; set counter to 2
MBEGIN FOO3 // set savepoint FOO3; set counter to 3
MROLLBACK FOO3 // rollback to FOO3; set counter to 2
MCOMMIT FOO2 // release FOO2; set counter to 1
MCOMMIT FOO1 // commit

regards,
Lukas

regards,
Lukas

Tom Lane wrote:
> Lukas Kahwe Smith <smith@pooteeweet.org> writes:
>> Err, I think you misunderstood what I said. My implementation uses 
>> SAVEPOINTs already. The point is having some API where you do not have 
>> to care of you are already in a transaction or not.
> 
> It's not that hard, is it?
> 
>     if (PQtransactionStatus(conn) == PQTRANS_IDLE)
>         PQexec(conn, "BEGIN");
>     else
>         PQexec(conn, "SAVEPOINT foo");

Its not exactly convenient either, especially in the case of modular 
code that may be developed by different people. Anyways, like I said I 
have a solution in my framework to make life of module developers 
easier. Obviously proper nested transactions would be the ideal, but so 
it goes. I was just throwing this out here when I saw Peter's comment.

regards,
Lukas

On Jan 2, 2007, at 2:01 PM, Lukas Kahwe Smith wrote:
> Tom Lane wrote:
>> Lukas Kahwe Smith <smith@pooteeweet.org> writes:
>>> Err, I think you misunderstood what I said. My implementation  
>>> uses SAVEPOINTs already. The point is having some API where you  
>>> do not have to care of you are already in a transaction or not.
>> It's not that hard, is it?
>>     if (PQtransactionStatus(conn) == PQTRANS_IDLE)
>>         PQexec(conn, "BEGIN");
>>     else
>>         PQexec(conn, "SAVEPOINT foo");
>
> Its not exactly convenient either, especially in the case of  
> modular code that may be developed by different people. Anyways,  
> like I said I have a solution in my framework to make life of  
> module developers easier. Obviously proper nested transactions  
> would be the ideal, but so it goes. I was just throwing this out  
> here when I saw Peter's comment.

+1. Right now it's not as big a deal since we don't allow transaction  
control inside functions, but if we ever get that ability it will  
become much more of a pain to write modular code that deals with  
savepoints/subtransactions.
--
Jim Nasby                                            jim@nasby.net
EnterpriseDB      http://enterprisedb.com      512.569.9461 (cell)

On 1/2/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Lukas Kahwe Smith <smith@pooteeweet.org> writes:
> > Err, I think you misunderstood what I said. My implementation uses
> > SAVEPOINTs already. The point is having some API where you do not have
> > to care of you are already in a transaction or not.
>
> It's not that hard, is it?
>
>        if (PQtransactionStatus(conn) == PQTRANS_IDLE)
>                PQexec(conn, "BEGIN");
>        else
>                PQexec(conn, "SAVEPOINT foo");
>
>                        regards, tom lane
>

and  how the releases and commit will happen? all at once or one by one?

-- 
regards,
Jaime Casanova

"Programming today is a race between software engineers striving to
build bigger and better idiot-proof programs and the universe trying
to produce bigger and better idiots.
So far, the universe is winning."                                      Richard Cook