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

Started by Joshua D. Drakeover 19 years ago20 messageshackers
Jump to latest
#1Joshua D. Drake
jd@commandprompt.com

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

#2Bruce Momjian
bruce@momjian.us
In reply to: Joshua D. Drake (#1)
Re: TODO: Add a GUC to control whether BEGIN inside

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;
BEGIN
test=> BEGIN;
WARNING: there is already a transaction in progress
BEGIN
test=> 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. +

#3Joshua D. Drake
jd@commandprompt.com
In reply to: Bruce Momjian (#2)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#4Tom Lane
tgl@sss.pgh.pa.us
In reply to: Joshua D. Drake (#3)
Re: TODO: Add a GUC to control whether BEGIN inside

"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

#5Joshua D. Drake
jd@commandprompt.com
In reply to: Tom Lane (#4)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#6Bruce Momjian
bruce@momjian.us
In reply to: Joshua D. Drake (#5)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#7Robert Treat
xzilla@users.sourceforge.net
In reply to: Bruce Momjian (#6)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#8Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Treat (#7)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#9Peter Eisentraut
peter_e@gmx.net
In reply to: Tom Lane (#4)
Re: TODO: Add a GUC to control whether BEGIN inside

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/

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#2)
Re: TODO: Add a GUC to control whether BEGIN inside

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/

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Peter Eisentraut (#10)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#12Lukas Kahwe Smith
smith@pooteeweet.org
In reply to: Tom Lane (#11)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#13Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Lukas Kahwe Smith (#12)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#14Lukas Kahwe Smith
smith@pooteeweet.org
In reply to: Alvaro Herrera (#13)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#15Joshua D. Drake
jd@commandprompt.com
In reply to: Peter Eisentraut (#10)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#16Tom Lane
tgl@sss.pgh.pa.us
In reply to: Lukas Kahwe Smith (#14)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#17Gurjeet Singh
singh.gurjeet@gmail.com
In reply to: Joshua D. Drake (#15)
Re: TODO: Add a GUC to control whether BEGIN inside

On 1/2/07, Joshua D. Drake <jd@commandprompt.com> wrote:

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

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

#18Lukas Kahwe Smith
smith@pooteeweet.org
In reply to: Tom Lane (#16)
Re: TODO: Add a GUC to control whether BEGIN inside

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

#19Jim Nasby
Jim.Nasby@BlueTreble.com
In reply to: Lukas Kahwe Smith (#18)
Re: TODO: Add a GUC to control whether BEGIN inside

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)
#20Jaime Casanova
jcasanov@systemguards.com.ec
In reply to: Tom Lane (#16)
Re: TODO: Add a GUC to control whether BEGIN inside

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