Thread: Re: [GENERAL] Rollback on Error
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
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, thiswould 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, Pennsylvania19073
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