Rollback on Error

On Tue, Sep 14, 2004 at 11:39:32AM +0200, Michael Paesold wrote:

I though the postgres behaviour of rolling back the whole transaction was
standard? If that is not the case, I don't understand why core seems to be
against a mode (GUC), where an implicit savepoint is generated before each
statement so that "rollback of the last statement" would be possible.

Well. If such a mode ever becomes available I'll be looking into how to
make sure it never gets turned on. My mind is currently boggling at the
number of things it would break.

A transaction is either committed as a whole, without errors, or not at
all. If you want to do a savepoint after each statement, go right
ahead, nothing is stopping you. I just don't think making it any kind
of default is a very good idea...
--
Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/

Michael Paesold wrote:

I though the postgres behaviour of rolling back the whole transaction
was standard?

No.

If that is not the case, I don't understand why core
seems to be against a mode (GUC), where an implicit savepoint is
generated before each statement so that "rollback of the last
statement" would be possible.

Because it's dangerous in noninteractive mode. When you send a
transaction to the server, you don't want some statements be left out.

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

Peter Eisentraut wrote:

Michael Paesold wrote:

I though the postgres behaviour of rolling back the whole transaction
was standard?

No.

Thanks for putting that straigt.

If that is not the case, I don't understand why core
seems to be against a mode (GUC), where an implicit savepoint is
generated before each statement so that "rollback of the last
statement" would be possible.

Because it's dangerous in noninteractive mode. When you send a
transaction to the server, you don't want some statements be left out.

I understand it's dangerous, it would possibly introduce data-inconsistency
in all applications that don't check for errors after each statement...

Still it would be very useful in interactive psql...
of course, it's bad in scripted mode. Perhaps it could be implemented inside
psql? Would such a patch be accepted?

Regards,
Michael Paesold

On Tue, 2004-09-14 at 11:56, Peter Eisentraut wrote:

Michael Paesold wrote:

I though the postgres behaviour of rolling back the whole transaction
was standard?

No.

If that is not the case, I don't understand why core
seems to be against a mode (GUC), where an implicit savepoint is
generated before each statement so that "rollback of the last
statement" would be possible.

Because it's dangerous in noninteractive mode. When you send a
transaction to the server, you don't want some statements be left out.

But it does not mean the server will commit the transaction for you...
it will throw an exception on the failed query, and it's up to you to
continue the transaction, commit it right away, or to roll it back. Old
code will probably just work fine if it will roll back on any received
error, which it had to do anyway even with the current behavior.
So it is just giving more choice to the developer instead of deciding
for him that the transaction is failed... and yes, there are valid
scenarios where it is quicker to execute a query and based on the fact
that it fails or not do different things, without the need of redoing
the whole transaction executed before.
It's another story that this feature should not be turned on by default
if it has some performance overhead (which I suppose it has, as setting
a savepoint is probably not free).

Just my 2c,
Csaba.

On 14/09/2004 10:39 Michael Paesold wrote:

I just read this in the MySQL manual:
(http://dev.mysql.com/doc/mysql/en/InnoDB_Error_handling.html)

"Error handling in InnoDB is not always the same as specified in the SQL
standard. According to the standard, any error during an SQL statement
should cause the rollback of that statement. InnoDB sometimes rolls back
only part of the statement, or the whole transaction. The following items
describe how InnoDB performs error handling:"

Ignore InnoDB, but read "According to the standard, any error during an
SQL
statement should cause the rollback of that statement"...

I though the postgres behaviour of rolling back the whole transaction was
standard? If that is not the case, I don't understand why core seems to be
against a mode (GUC), where an implicit savepoint is generated before each
statement so that "rollback of the last statement" would be possible.

Is MySQL wrong on the standard, or has postgresql core attitude changed to
support the standard where possible - at least if the user wants?

Well, you could go off and read the standards of course but I'll save you
the time and tell you that PostrgreSQL does it correctly.

But seriously, are you so naive that you would believe _anything_ MySQL AB 
say? 
-- 
Paul Thomas
+------------------------------+-------------------------------------------+
| Thomas Micro Systems Limited | Software Solutions for Business           |
| Computer Consultants         | http://www.thomas-micro-systems-ltd.co.uk |
+------------------------------+-------------------------------------------+

"Michael Paesold" <mpaesold@gmx.at> writes:

I though the postgres behaviour of rolling back the whole transaction was
standard?

Not really.

If that is not the case, I don't understand why core seems to be
against a mode (GUC), where an implicit savepoint is generated before each
statement so that "rollback of the last statement" would be possible.

Because we learned our lesson with the ill-fated autocommit GUC
variable. You can't have fundamental transactional semantics depending
on the phase of the moon, but from the point of view of application
code, anything that can be flipped as easily as a GUC variable is an
unknown.

If you've been following recent -hackers discussions you will also
realize that a forced savepoint for every statement is untenable
from a performance perspective anyway.

regards, tom lane

Tom Lane wrote:

"Michael Paesold" <mpaesold@gmx.at> writes:

...

If that is not the case, I don't understand why core seems to be
against a mode (GUC), where an implicit savepoint is generated before

each

statement so that "rollback of the last statement" would be possible.

Because we learned our lesson with the ill-fated autocommit GUC
variable. You can't have fundamental transactional semantics depending
on the phase of the moon, but from the point of view of application
code, anything that can be flipped as easily as a GUC variable is an
unknown.

If you've been following recent -hackers discussions you will also
realize that a forced savepoint for every statement is untenable
from a performance perspective anyway.

Well, I read the threads "Re: beta1 & beta2 & Windows & heavy load" and
"Cleaning up recovery from subtransaction start failure".

I see, that savepoint/subxacts are rather expensive, at least wrt
shared-memory (lock on xid).

On the other hand, the scenario of a psql option (read: I have given up the
idea of a backend implementation) to rollback only last statement on error
is quite different.

In the mentioned thread, several thousand rows where deleted, each firing a
trigger creating a subxact. But in usual interactive administrative
sessions, you would perhaps have 5 to 20 statements in a usual transaction.
At least according to my personal experience.

Am I right in asserting that psql could recognize if a session is
interactive or scripted (\i, psql < script.sql)? And it already has user
settable options, (\set, e.g. autocommit)...

So a proper patch implementing a corresponding mode in psql that wraps each
statement (of a multi statement transaction) in a subxact, if the user
enables it, would be accepted or even called welcome?

Just thinking about...

BEGIN;
SET TRANSACTION ISOLATION SERIALIZABLE;
SELECT * FROM a WHERE id = 5;
UPDATE a SET .... WHERE id = 5;

-- check everything is ok...
SELEC * FROM a WHERE id = 5;

--> ERROR: syntax error at or near "SELEC" at character 1

-- arghh... typo do it all over again...

Best Regards,
Michael Paesold

"Michael Paesold" <mpaesold@gmx.at> writes:

Tom Lane wrote:

"Michael Paesold" <mpaesold@gmx.at> writes:

If that is not the case, I don't understand why core seems to be
against a mode (GUC), where an implicit savepoint is generated before
each statement so that "rollback of the last statement" would be possible.

Because we learned our lesson with the ill-fated autocommit GUC
variable. You can't have fundamental transactional semantics depending
on the phase of the moon, but from the point of view of application
code, anything that can be flipped as easily as a GUC variable is an
unknown.

On the other hand, the scenario of a psql option (read: I have given up the
idea of a backend implementation) to rollback only last statement on error
is quite different.

Sure (and we already have one for autocommit). But I thought you were
asking about a backend implementation.

regards, tom lane

Tom Lane wrote:

"Michael Paesold" <mpaesold@gmx.at> writes:

On the other hand, the scenario of a psql option (read: I have
given up the idea of a backend implementation) to rollback only
last statement on error is quite different.

Sure (and we already have one for autocommit). But I thought you were
asking about a backend implementation.

I have implemented what I have suggested for psql. I have attached a first
patch for review here, because I have a few questions. Also I want to make
sure the whole thing is reasonable.

I have named the option "IMPLICIT_SAVEPOINTS", because that's what it is. If
someone has a better name that would describe the purpose of the feature, I
am happy to change it.

The feature is activated, if
* \set IMPLICIT_SAVEPOINTS 'on'
* connection is in "idle in transaction" state
* psql session is interactive

The code executes an implicit "SAVEPOINT pg_internal_psql" in
common.c/SendQuery to which it will try to rollback to, if the executed
query fails.

Open questions:
* Should psql print a notice in the case of that rollback?
Something like "Rollback of last statement successful."?

* What is currently missing, is a detection of \i ... obviously this feature
should not be used for each query in \i. Perhaps only for the whole \i
command?
So what should I do to detect \i?
Add an extra argument to MainLoop, SendQuery and process_file()? (many
changes)
Add a global variable in common.c/h (e.g. bool
deactivate_implicit_savepoints) that can be used in process_file to
temporarily deactivate the code path?
(more local changes, but rather a hack imho)

Please have a look at the patch and comment.

Best Regards,
Michael Paesold

I assume this is to be saved for 8.1.

This has been saved for the 8.1 release:

http:/momjian.postgresql.org/cgi-bin/pgpatches2

---------------------------------------------------------------------------

Michael Paesold wrote:

Tom Lane wrote:

"Michael Paesold" <mpaesold@gmx.at> writes:

On the other hand, the scenario of a psql option (read: I have
given up the idea of a backend implementation) to rollback only
last statement on error is quite different.

Sure (and we already have one for autocommit). But I thought you were
asking about a backend implementation.

I have implemented what I have suggested for psql. I have attached a first
patch for review here, because I have a few questions. Also I want to make
sure the whole thing is reasonable.

I have named the option "IMPLICIT_SAVEPOINTS", because that's what it is. If
someone has a better name that would describe the purpose of the feature, I
am happy to change it.

The feature is activated, if
* \set IMPLICIT_SAVEPOINTS 'on'
* connection is in "idle in transaction" state
* psql session is interactive

The code executes an implicit "SAVEPOINT pg_internal_psql" in
common.c/SendQuery to which it will try to rollback to, if the executed
query fails.

Open questions:
* Should psql print a notice in the case of that rollback?
Something like "Rollback of last statement successful."?

* What is currently missing, is a detection of \i ... obviously this feature
should not be used for each query in \i. Perhaps only for the whole \i
command?
So what should I do to detect \i?
Add an extra argument to MainLoop, SendQuery and process_file()? (many
changes)
Add a global variable in common.c/h (e.g. bool
deactivate_implicit_savepoints) that can be used in process_file to
temporarily deactivate the code path?
(more local changes, but rather a hack imho)

Please have a look at the patch and comment.

Best Regards,
Michael Paesold

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I assume this is to be saved for 8.1.

This has been saved for the 8.1 release:
http:/momjian.postgresql.org/cgi-bin/pgpatches2

It is not remotely ready to apply yet, so please do not put it in the
queue.

regards, tom lane

Tom Lane wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I assume this is to be saved for 8.1.

This has been saved for the 8.1 release:
http:/momjian.postgresql.org/cgi-bin/pgpatches2

It is not remotely ready to apply yet, so please do not put it in the
queue.

That queue isn't for 8.1 ready-to-apply stuff. It just says "saved".

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Bruce Momjian <pgman@candle.pha.pa.us> wrote:

Bruce Momjian <pgman@candle.pha.pa.us> writes:

I assume this is to be saved for 8.1.

I assumed that to, so I did not want to disturb any more now.

This has been saved for the 8.1 release:
http:/momjian.postgresql.org/cgi-bin/pgpatches2

Tom Lane wrote:

It is not remotely ready to apply yet, so please do not put it in the
queue.

I hope you will be willing to comment on the issues when times come. I am
not really satisfied myself, but without further discussion I did not want
to continue to work on it. Anyway, I understand this is not the right time
now (8.0 beta).

Best Regards,
Michael Paesold

On Fri, Oct 08, 2004 at 08:40:56PM +0200, Michael Paesold wrote:

I hope you will be willing to comment on the issues when times come. I am
not really satisfied myself, but without further discussion I did not want
to continue to work on it. Anyway, I understand this is not the right time
now (8.0 beta).

I think it would be wise to create a function to discover what savepoints
are available, and from that know when to release an automatically-
established savepoint.

Of course, the function would only work when the backend is not in abort
state, but I think that's a reasonable restriction.

--
Alvaro Herrera (<alvherre[@]dcc.uchile.cl>)
"Ni aun el genio muy grande llegar�a muy lejos
si tuviera que sacarlo todo de su propio interior" (Goethe)

Alvaro Herrera Munoz wrote:

I think it would be wise to create a function to discover what savepoints
are available, and from that know when to release an automatically-
established savepoint.

Of course, the function would only work when the backend is not in abort
state, but I think that's a reasonable restriction.

Ok, that would need a set returning function in the backend, right? Could
you help me write it when time comes? (I don't feel backend hacking savvy.)

What about the resources consumed by savepoints
a) that can be freed by a RELEASE (trx state stack, what else?)
versus
b) that cannot be freed (xid locks, anything else?)

Is it worth the effort of extra work (not programmer's but runtime ;-)?

Bruce,
in Revision 1.1355 of the TODO you removed a line from the todo list that
said:
-Use nested transactions to prevent syntax errors from aborting a
transaction

I tought this is what I made a patch for, so shouldn't it be back for 8.1?
Or at least something similar?

Best Regards,
Michael Paesold

Michael Paesold wrote:

Alvaro Herrera Munoz wrote:

I think it would be wise to create a function to discover what savepoints
are available, and from that know when to release an automatically-
established savepoint.

Of course, the function would only work when the backend is not in abort
state, but I think that's a reasonable restriction.

Ok, that would need a set returning function in the backend, right? Could
you help me write it when time comes? (I don't feel backend hacking savvy.)

What about the resources consumed by savepoints
a) that can be freed by a RELEASE (trx state stack, what else?)
versus
b) that cannot be freed (xid locks, anything else?)

Is it worth the effort of extra work (not programmer's but runtime ;-)?

Bruce,
in Revision 1.1355 of the TODO you removed a line from the todo list that
said:
-Use nested transactions to prevent syntax errors from aborting a
transaction

I tought this is what I made a patch for, so shouldn't it be back for 8.1?
Or at least something similar?

The item was adjusted to be more specific:

* Add an option to automatically use savepoints for each statement in a
multi-statement transaction.

When enabled, this would allow errors in multi-statement transactions
to be automatically ignored.

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

This has already been implemented in CVS as a psql \set variable:

ON_ERROR_ROLLBACK = 'interactive'

and will appear in 8.1.

---------------------------------------------------------------------------

Michael Paesold wrote:

Tom Lane wrote:

"Michael Paesold" <mpaesold@gmx.at> writes:

On the other hand, the scenario of a psql option (read: I have
given up the idea of a backend implementation) to rollback only
last statement on error is quite different.

Sure (and we already have one for autocommit). But I thought you were
asking about a backend implementation.

I have implemented what I have suggested for psql. I have attached a first
patch for review here, because I have a few questions. Also I want to make
sure the whole thing is reasonable.

I have named the option "IMPLICIT_SAVEPOINTS", because that's what it is. If
someone has a better name that would describe the purpose of the feature, I
am happy to change it.

The feature is activated, if
* \set IMPLICIT_SAVEPOINTS 'on'
* connection is in "idle in transaction" state
* psql session is interactive

The code executes an implicit "SAVEPOINT pg_internal_psql" in
common.c/SendQuery to which it will try to rollback to, if the executed
query fails.

Open questions:
* Should psql print a notice in the case of that rollback?
Something like "Rollback of last statement successful."?

* What is currently missing, is a detection of \i ... obviously this feature
should not be used for each query in \i. Perhaps only for the whole \i
command?
So what should I do to detect \i?
Add an extra argument to MainLoop, SendQuery and process_file()? (many
changes)
Add a global variable in common.c/h (e.g. bool
deactivate_implicit_savepoints) that can be used in process_file to
temporarily deactivate the code path?
(more local changes, but rather a hack imho)

Please have a look at the patch and comment.

Best Regards,
Michael Paesold

[ Attachment, skipping... ]

---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match

-- 
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073