Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Applied. --- pgman wrote: > pgman wrote: > > Tom Lane wrote: > > > Bruce Momjian writes: > > > > Tom Lane wrote: > > > >> Well, that's just a matter of choosing good (ie short) names for the > > > >> backslash commands. I was trying to be clear rather than proposing > > > >> names I would actually want to use ;-). Any suggestions? > > > > > > > Well, if we allowed ON_ERROR_ROLLBACK to work in non-interactive > > > > sessions we could just do: > > > > > > > \set ON_ERROR_ROLLBACK on > > > > DROP TABLE foo; > > > > \set ON_ERROR_ROLLBACK off > > > > > > That isn't the same thing at all. The syntax I was proposing allows the > > > script writer to define a savepoint covering multiple statements, > > > whereas the above does not. > > > > Well, it fits the use case posted, that is to conditionally roll back a > > _single_ failed query. I don't see the need to add a new > > infrastructure/command unless people have a use case for rolling back a > > group of statements on failure. I have no seen such a description yet. > > OK, updated patch that allows for 'on/interactive/off'. Seems there are > enough use cases to add an 'interactive' option. > > -- > 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 > Index: doc/src/sgml/ref/psql-ref.sgml > === > RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v > retrieving revision 1.134 > diff -c -c -r1.134 psql-ref.sgml > *** doc/src/sgml/ref/psql-ref.sgml14 Mar 2005 06:19:01 - 1.134 > --- doc/src/sgml/ref/psql-ref.sgml28 Apr 2005 03:35:00 - > *** > *** 2050,2055 > --- 2050,2077 > > > > + > +rollback > +psql > + > + ON_ERROR_ROLLBACK > + > + > + When on, if a statement in a transaction block > + generates an error, the error is ignored and the transaction > + continues. When interactive, such errors are only > + ignored in interactive sessions, and not when reading script > + files. When off (the default), a statement in a > + transaction block that generates an error aborts the entire > + transaction. The on_error_rollback-on mode works by issuing an > + implicit SAVEPONT for you, just before each command > + that is in a transaction block, and rolls back to the savepoint > + on error. > + > + > + > + > + > ON_ERROR_STOP > > > Index: src/bin/psql/common.c > === > RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v > retrieving revision 1.96 > diff -c -c -r1.96 common.c > *** src/bin/psql/common.c 22 Feb 2005 04:40:52 - 1.96 > --- src/bin/psql/common.c 28 Apr 2005 03:35:01 - > *** > *** 941,951 > bool > SendQuery(const char *query) > { > ! PGresult *results; > ! TimevalStruct before, > ! after; > ! boolOK; > ! > if (!pset.db) > { > psql_error("You are currently not connected to a database.\n"); > --- 941,953 > bool > SendQuery(const char *query) > { > ! PGresult*results; > ! TimevalStruct before, after; > ! bool OK, on_error_rollback_savepoint = false; > ! PGTransactionStatusType transaction_status; > ! static bool on_error_rollback_warning = false; > ! const char *rollback_str; > ! > if (!pset.db) > { > psql_error("You are currently not connected to a database.\n"); > *** > *** 973,979 > > SetCancelConn(); > > ! if (PQtransactionStatus(pset.db) == PQTRANS_IDLE && > !GetVariableBool(pset.vars, "AUTOCOMMIT") && > !command_no_begin(query)) > { > --- 975,983 > > SetCancelConn(); > > ! transaction_status = PQtransactionStatus(pset.db); > ! > ! if (transaction_status == PQTRANS_IDLE && > !GetVariableBool(pset.vars, "AUTOCOMMIT") && > !command_no_begin(query)) > { > *** > *** 987,992 > --- 991,1023 > } > PQclear(results); > } > + else if (transaction_status == PQTRANS_INTRANS && > + (rollback_str = GetVariable(pset.vars, > "ON_ERROR_ROLLBACK")) != NULL && > + /* !off and !interactive is 'on' */ > + pg_strcasecmp(rollback_str, "off") != 0 && > + (pset.cur_cmd_interactive || > +
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
pgman wrote: > Tom Lane wrote: > > Bruce Momjian writes: > > > Tom Lane wrote: > > >> Well, that's just a matter of choosing good (ie short) names for the > > >> backslash commands. I was trying to be clear rather than proposing > > >> names I would actually want to use ;-). Any suggestions? > > > > > Well, if we allowed ON_ERROR_ROLLBACK to work in non-interactive > > > sessions we could just do: > > > > > \set ON_ERROR_ROLLBACK on > > > DROP TABLE foo; > > > \set ON_ERROR_ROLLBACK off > > > > That isn't the same thing at all. The syntax I was proposing allows the > > script writer to define a savepoint covering multiple statements, > > whereas the above does not. > > Well, it fits the use case posted, that is to conditionally roll back a > _single_ failed query. I don't see the need to add a new > infrastructure/command unless people have a use case for rolling back a > group of statements on failure. I have no seen such a description yet. OK, updated patch that allows for 'on/interactive/off'. Seems there are enough use cases to add an 'interactive' option. -- 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 Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.134 diff -c -c -r1.134 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 14 Mar 2005 06:19:01 - 1.134 --- doc/src/sgml/ref/psql-ref.sgml 28 Apr 2005 03:35:00 - *** *** 2050,2055 --- 2050,2077 + +rollback +psql + + ON_ERROR_ROLLBACK + + + When on, if a statement in a transaction block + generates an error, the error is ignored and the transaction + continues. When interactive, such errors are only + ignored in interactive sessions, and not when reading script + files. When off (the default), a statement in a + transaction block that generates an error aborts the entire + transaction. The on_error_rollback-on mode works by issuing an + implicit SAVEPONT for you, just before each command + that is in a transaction block, and rolls back to the savepoint + on error. + + + + + ON_ERROR_STOP Index: src/bin/psql/common.c === RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v retrieving revision 1.96 diff -c -c -r1.96 common.c *** src/bin/psql/common.c 22 Feb 2005 04:40:52 - 1.96 --- src/bin/psql/common.c 28 Apr 2005 03:35:01 - *** *** 941,951 bool SendQuery(const char *query) { ! PGresult *results; ! TimevalStruct before, ! after; ! boolOK; ! if (!pset.db) { psql_error("You are currently not connected to a database.\n"); --- 941,953 bool SendQuery(const char *query) { ! PGresult*results; ! TimevalStruct before, after; ! bool OK, on_error_rollback_savepoint = false; ! PGTransactionStatusType transaction_status; ! static bool on_error_rollback_warning = false; ! const char *rollback_str; ! if (!pset.db) { psql_error("You are currently not connected to a database.\n"); *** *** 973,979 SetCancelConn(); ! if (PQtransactionStatus(pset.db) == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { --- 975,983 SetCancelConn(); ! transaction_status = PQtransactionStatus(pset.db); ! ! if (transaction_status == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { *** *** 987,992 --- 991,1023 } PQclear(results); } + else if (transaction_status == PQTRANS_INTRANS && +(rollback_str = GetVariable(pset.vars, "ON_ERROR_ROLLBACK")) != NULL && +/* !off and !interactive is 'on' */ +pg_strcasecmp(rollback_str, "off") != 0 && +(pset.cur_cmd_interactive || + pg_strcasecmp(rollback_str, "interactive") != 0)) + { + if (on_error_rollback_warning == false && pset.sversion < 8) + { + fprintf(stderr, _("The server version (%d) does not support savepoints for ON_ERROR_ROLLBACK.\n"), +
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: > Bruce Momjian writes: > > Tom Lane wrote: > >> Well, that's just a matter of choosing good (ie short) names for the > >> backslash commands. I was trying to be clear rather than proposing > >> names I would actually want to use ;-). Any suggestions? > > > Well, if we allowed ON_ERROR_ROLLBACK to work in non-interactive > > sessions we could just do: > > > \set ON_ERROR_ROLLBACK on > > DROP TABLE foo; > > \set ON_ERROR_ROLLBACK off > > That isn't the same thing at all. The syntax I was proposing allows the > script writer to define a savepoint covering multiple statements, > whereas the above does not. Well, it fits the use case posted, that is to conditionally roll back a _single_ failed query. I don't see the need to add a new infrastructure/command unless people have a use case for rolling back a group of statements on failure. I have no seen such a description yet. -- 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 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Bruce Momjian writes: > Tom Lane wrote: >> Well, that's just a matter of choosing good (ie short) names for the >> backslash commands. I was trying to be clear rather than proposing >> names I would actually want to use ;-). Any suggestions? > Well, if we allowed ON_ERROR_ROLLBACK to work in non-interactive > sessions we could just do: > \set ON_ERROR_ROLLBACK on > DROP TABLE foo; > \set ON_ERROR_ROLLBACK off That isn't the same thing at all. The syntax I was proposing allows the script writer to define a savepoint covering multiple statements, whereas the above does not. Maybe what we really need is a "rollback or release savepoint" operation, defined as "ROLLBACK TO foo if in error state, RELEASE foo if not in error state". This is essentially the thing that a script writer has to have and can't do for himself due to the lack of any conditional ability in psql scripts. We could imagine implementing that either as a SQL command or as a psql backslash command ... I don't have a strong feeling either way. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: > Andrew Dunstan <[EMAIL PROTECTED]> writes: > >>> \begin_ignore_error > >>> DROP TABLE foo; > >>> \end_ignore_error > > > I meant it's a lot to type ;-) > > Well, that's just a matter of choosing good (ie short) names for the > backslash commands. I was trying to be clear rather than proposing > names I would actually want to use ;-). Any suggestions? Well, if we allowed ON_ERROR_ROLLBACK to work in non-interactive sessions we could just do: \set ON_ERROR_ROLLBACK on DROP TABLE foo; \set ON_ERROR_ROLLBACK off No new syntax required. Seems this variable is going to need an 'interactive' setting, which means it isn't boolean anymore. Also, should we allow 'true/false' to work with these seetings? We do that with boolean columns in SQL. -- 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 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
On Tue, 2005-04-26 at 10:28, Tom Lane wrote: > "Greg Sabino Mullane" <[EMAIL PROTECTED]> writes: > > To reiterate my opinion, I think the behavior should be the same > > for interactive and non-interactive sessions. Not only will it > > prevent nasty surprises, but unless we make a third 'setting', > > there will be no way to enable this in non-interactive scripts, > > which is something that I would want to be able to do. > > I'm finding it hard to visualize a non-interactive script making > any good use of such a setting. Without a way to test whether > you got an error or not, it would amount to an "ignore errors > within transactions" mode, which seems a pretty bad idea. > > Can you show a plausible use-case for such a thing? > I plan to use it in scripts that push site meta-data out to our test servers, where the list of sites are all different so any static data dump is bound to fail on some foreign key checks (but I don't care which ones fail as long as some go over). I'm sure others can come up with different scenarios, but more importantly is I don't see a good reason to treat this setting different from all others and explicitly forbid this use from people, especially when I can imagine people coming from other dbs where this behavior is more common who might in fact expect it to work this way. Robert Treat -- Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL ---(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
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 > I'm finding it hard to visualize a non-interactive script making > any good use of such a setting. Without a way to test whether > you got an error or not, it would amount to an "ignore errors > within transactions" mode, which seems a pretty bad idea. > > Can you show a plausible use-case for such a thing? I could have used this yesterday. I was populating a test table with a primary key on two columns and needed to add a bunch of random rows. I generated a 10_000 line file of one insert statement each. Rather than worrying about collisions, I could simply \rollbackonerror (or whatever we're calling it today :) and silently discard the handful that happen to violate the primary key constraint and let the rest insert. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200504270754 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFCb33NvJuQZxSWSsgRAvdfAJwMqysSpVI2BDh9wENT2jxMZnspagCfRlHJ 9ElhNydsz2FsCc1JgI5R+gU= =h9AW -END PGP SIGNATURE- ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
On Apr 26, 2005, at 10:35 AM, Tom Lane wrote: Once you've got such an infrastructure, it makes sense to allow an interactive mode that automatically puts such things around each statement. But I can't really see the argument for using such a behavior in a script. Scripts are too stupid. Would it be possible to have a command line switch and/or a psql variable to control "interactive"? If I recall correctly, the setting depends on tty and there are possible interactive uses of psql outside of a terminal session. With so many things depending on this, it would be nice to be able to override the default. Thanks, John DeSoi, Ph.D. http://pgedit.com/ Power Tools for PostgreSQL ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Andrew Dunstan <[EMAIL PROTECTED]> writes: >>> \begin_ignore_error >>> DROP TABLE foo; >>> \end_ignore_error > I meant it's a lot to type ;-) Well, that's just a matter of choosing good (ie short) names for the backslash commands. I was trying to be clear rather than proposing names I would actually want to use ;-). Any suggestions? regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: Andrew Dunstan <[EMAIL PROTECTED]> writes: Tom Lane wrote: I would far rather see people code explicit markers around statements whose failure can be ignored. That is, a script that needs this behavior ought to look like BEGIN; \begin_ignore_error DROP TABLE foo; \end_ignore_error CREATE ... ... COMMIT; That's a lot of work. How so? It's a minuscule extension to the psql patch already coded: just provide backslash commands to invoke the bits of code already written. I meant it's a lot to type ;-) In this particular case I would actually like to see us provide "DROP IF EXISTS ..." or some such. That's substantially more work, with substantially less scope of applicability: it would only solve the issue for DROP. True. I wasn't suggesting it as an alternative in the general case. I still think it's worth doing, though - I have often seen it requested and can't think of a compelling reason not to provide it. But maybe that's off topic ;-) cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Andrew Dunstan <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> I would far rather see people code explicit markers around statements >> whose failure can be ignored. That is, a script that needs this >> behavior ought to look like >> >> BEGIN; >> \begin_ignore_error >> DROP TABLE foo; >> \end_ignore_error >> CREATE ... >> ... >> COMMIT; > That's a lot of work. How so? It's a minuscule extension to the psql patch already coded: just provide backslash commands to invoke the bits of code already written. > In this particular case I would actually like to > see us provide "DROP IF EXISTS ..." or some such. That's substantially more work, with substantially less scope of applicability: it would only solve the issue for DROP. regards, tom lane ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: Richard Huxton writes: Michael Paesold wrote: I just don't see why non-interactive mode does need such a switch because there is no way to check if there was an error. So just put two queries there and hope one will work? DROP TABLE foo; CREATE TABLE foo... Unconvincing. What if the drop fails for permission reasons, rather than because the table's not there? Then the CREATE will fail too ... but now the script bulls ahead regardless, with who knows what bad consequences. I would far rather see people code explicit markers around statements whose failure can be ignored. That is, a script that needs this behavior ought to look like BEGIN; \begin_ignore_error DROP TABLE foo; \end_ignore_error CREATE ... ... COMMIT; That's a lot of work. In this particular case I would actually like to see us provide "DROP IF EXISTS ..." or some such. My instinct on this facility is that distinguishing between interactive and noninteractive use is likely to be highly confusing. So I would favor behaviour that is consistent and defaults to off. cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
"Joshua D. Drake" <[EMAIL PROTECTED]> writes: >> BEGIN; >> \begin_ignore_error >> DROP TABLE foo; >> \end_ignore_error >> CREATE ... >> ... >> COMMIT; > That seems awful noisy. Why not just: >BEGIN: >DROP TABLE foo; >ERROR: table foo does not exist; >CONTINUE; >etc Well, ignoring questions of how we choose to spell the commands, the thing I'd not like about the second alternative is that it doesn't afford any control over the number of statements rolled back upon error. regards, tom lane ---(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
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Philip Warner <[EMAIL PROTECTED]> writes: > Also, the blunder-on-regardless approach is popular in pg_dump, or so I'm > told ;-). Sure, but pg_dump scripts don't try to execute as a single transaction. None of this discussion applies to the behavior outside an explicit transaction block. regards, tom lane ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
I would far rather see people code explicit markers around statements whose failure can be ignored. That is, a script that needs this behavior ought to look like BEGIN; \begin_ignore_error DROP TABLE foo; \end_ignore_error CREATE ... ... COMMIT; That seems awful noisy. Why not just: BEGIN: DROP TABLE foo; ERROR: table foo does not exist; CONTINUE; etc Sincerely, Joshua D. Drake Command Prompt, Inc. ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Richard Huxton writes: > Michael Paesold wrote: >> I just don't see why non-interactive mode does need such a switch >> because there is no way to check if there was an error. So just put two >> queries there and hope one will work? > DROP TABLE foo; > CREATE TABLE foo... Unconvincing. What if the drop fails for permission reasons, rather than because the table's not there? Then the CREATE will fail too ... but now the script bulls ahead regardless, with who knows what bad consequences. I would far rather see people code explicit markers around statements whose failure can be ignored. That is, a script that needs this behavior ought to look like BEGIN; \begin_ignore_error DROP TABLE foo; \end_ignore_error CREATE ... ... COMMIT; where I'm supposing that we invent psql backslash commands to cue the sending of SAVEPOINT and RELEASE-or-ROLLBACK commands. (Anyone got a better idea for the names than that?) Once you've got such an infrastructure, it makes sense to allow an interactive mode that automatically puts such things around each statement. But I can't really see the argument for using such a behavior in a script. Scripts are too stupid. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Richard Huxton wrote: Michael Paesold wrote: But people (like me for example) will want to enable this behaviour by default. So they (me too) will put the option in .psqlrc. It is then enabled "by default". But then many of my scripts will destroy data instead of just erroring out. I just don't see why non-interactive mode does need such a switch because there is no way to check if there was an error. So just put two queries there and hope one will work? DROP TABLE foo; CREATE TABLE foo... This would be: \set AUTOCOMMIT off DROP TABLE foo; -- error, rolled back CREATE TABLE foo ... COMMIT; You could as well do: \set AUTOCOMMIT on -- default DROP TABLE foo; -- print error message CREATE TABLE foo ... There is not much difference, except for locking, ok. I see your point, but I don't think this makes enabling it by default (even in .psqlrc) any safer. Best Regards, Michael Paesold ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
"Greg Sabino Mullane" <[EMAIL PROTECTED]> writes: > To reiterate my opinion, I think the behavior should be the same > for interactive and non-interactive sessions. Not only will it > prevent nasty surprises, but unless we make a third 'setting', > there will be no way to enable this in non-interactive scripts, > which is something that I would want to be able to do. I'm finding it hard to visualize a non-interactive script making any good use of such a setting. Without a way to test whether you got an error or not, it would amount to an "ignore errors within transactions" mode, which seems a pretty bad idea. Can you show a plausible use-case for such a thing? regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Michael Paesold wrote: But people (like me for example) will want to enable this behaviour by default. So they (me too) will put the option in .psqlrc. It is then enabled "by default". But then many of my scripts will destroy data instead of just erroring out. I just don't see why non-interactive mode does need such a switch because there is no way to check if there was an error. So just put two queries there and hope one will work? DROP TABLE foo; CREATE TABLE foo... -- Richard Huxton Archonet Ltd ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Greg Sabino Mullane wrote: To reiterate my opinion, I think the behavior should be the same for interactive and non-interactive sessions. Not only will it prevent nasty surprises, but unless we make a third 'setting', there will be no way to enable this in non-interactive scripts, which is something that I would want to be able to do. > I don't buy the "but what if I set it in .psqlrc and forget" argument. That could be applied to a lot of things you could put in there. This setting defaults to "off" and must be explicitly enabled. I'd be okay with a "smart" mode that explicitly enables the interactive/non-interactive split. But people (like me for example) will want to enable this behaviour by default. So they (me too) will put the option in .psqlrc. It is then enabled "by default". But then many of my scripts will destroy data instead of just erroring out. I just don't see why non-interactive mode does need such a switch because there is no way to check if there was an error. So just put two queries there and hope one will work? If you really want this for scripts, there must be two options: * one to put savely into .psqlrc (what some people will want, I have \set AUTOCOMMIT off in my .psqlrc file, too, and I know I am not the only one) * another one that will also work in scripts I hope you understand and accept the issue here. Best Regards, Michael Paesold ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 To reiterate my opinion, I think the behavior should be the same for interactive and non-interactive sessions. Not only will it prevent nasty surprises, but unless we make a third 'setting', there will be no way to enable this in non-interactive scripts, which is something that I would want to be able to do. I don't buy the "but what if I set it in .psqlrc and forget" argument. That could be applied to a lot of things you could put in there. This setting defaults to "off" and must be explicitly enabled. I'd be okay with a "smart" mode that explicitly enables the interactive/non-interactive split. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200504260737 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFCbilxvJuQZxSWSsgRAgf8AJ9/NcsU/5A0V9isGvQy4sjba/aukgCgoFbp otSb0vVLfnL7mIt99rA4Piw= =1vVP -END PGP SIGNATURE- ---(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
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: > Bruce Momjian writes: > > I think everyone agrees this should only work in interactive mode. I > > think the only unknown is if it should be 'on' by default in interactive > > mode? Does it make sense to follow the standard in interactive mode if > > we don't follow it in non-interative mode? > > I doubt it's a good idea to change the default for this at all; in > particular, making the default interactive behavior different from > the noninteractive behavior seems like a recipe for problems. Agreed. New patch attached. I will apply tomorrow. -- 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 Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.134 diff -c -c -r1.134 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 14 Mar 2005 06:19:01 - 1.134 --- doc/src/sgml/ref/psql-ref.sgml 26 Apr 2005 00:35:48 - *** *** 2050,2055 --- 2050,2075 + +rollback +psql + + ON_ERROR_ROLLBACK + + + When on, only in interactive mode, if a statement in + a transaction block generates an error, the error is ignored and + the transaction continues. When off (the default), a + statement in a transaction block that generates an error aborts + the entire transaction. The on_error_rollback-on mode works by + issuing an implicit SAVEPONT for you, just before + each command that is in a transaction block, and rolls back to + the savepoint on error. + + + + + ON_ERROR_STOP Index: src/bin/psql/common.c === RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v retrieving revision 1.96 diff -c -c -r1.96 common.c *** src/bin/psql/common.c 22 Feb 2005 04:40:52 - 1.96 --- src/bin/psql/common.c 26 Apr 2005 00:35:50 - *** *** 941,951 bool SendQuery(const char *query) { ! PGresult *results; ! TimevalStruct before, ! after; ! boolOK; ! if (!pset.db) { psql_error("You are currently not connected to a database.\n"); --- 941,952 bool SendQuery(const char *query) { ! PGresult*results; ! TimevalStruct before, after; ! bool OK, on_error_rollback_savepoint = false; ! PGTransactionStatusType transaction_status; ! static bool on_error_rollback_warning = false; ! if (!pset.db) { psql_error("You are currently not connected to a database.\n"); *** *** 973,979 SetCancelConn(); ! if (PQtransactionStatus(pset.db) == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { --- 974,982 SetCancelConn(); ! transaction_status = PQtransactionStatus(pset.db); ! ! if (transaction_status == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { *** *** 987,992 --- 990,1019 } PQclear(results); } + else if (transaction_status == PQTRANS_INTRANS && +pset.cur_cmd_interactive && +GetVariableBool(pset.vars, "ON_ERROR_ROLLBACK")) + { + if (on_error_rollback_warning == false && pset.sversion < 8) + { + fprintf(stderr, _("The server version (%d) does not support savepoints for ON_ERROR_ROLLBACK.\n"), + pset.sversion); + on_error_rollback_warning = true; + } + else + { + results = PQexec(pset.db, "SAVEPOINT pg_psql_temporary_savepoint"); + if (PQresultStatus(results) != PGRES_COMMAND_OK) + { + psql_error("%s", PQerrorMessage(pset.db)); + PQclear(results); + ResetCancelConn(); + return false; + } + PQclear(results); + on_error_rollback_savepoint = true; + } + } if (pset.timing) GETTIMEOFDAY(&before); *** *** 1005,1010 --- 1032,1072 PQclear(results); + /* If we made
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Bruce Momjian wrote: > Greg Sabino Mullane wrote: > > > The SQL-Standard itself says that errors inside transactions should only > > > rollback the last statement, if possible. So why is that not implemented > > > in > > > PostgreSQL? What I read from past discussions here, is because it's just > > > unsave and will lead to data-garbage if you aren't very careful. > > > > That's a good point: if that is indeed what the standard says, we should > > probably see about following it. Rolling back to the last savepoint seems > > a reasonable behavior to me. > > The question is what to make the default: > > o disable it by default for all sessions (current patch) > o enable it by default only for interactive sessions, like AUTOCOMMIT > o enable it by default for all sessions (breaks too many apps) > o add a third mode called 'ttyonly' and figure out a default Based on the comments I received, and the mention that ignoring errors is part of the SQL standard, I chose the second option, patch attached: $ psql test Welcome to psql 8.1devel, the PostgreSQL interactive terminal. Type: \copyright for distribution terms \h for help with SQL commands \? for help with psql commands \g or terminate with semicolon to execute query \q to quit test=> BEGIN; BEGIN test=> asdf; ERROR: syntax error at or near "asdf" at character 1 LINE 1: asdf; ^ test=> SELECT 1; ?column? -- 1 (1 row) test=> COMMIT; COMMIT Can someone confirm that this is the way Oracle works as well? I checked on IRC and isql does it. I am uncertain how applications behave. -- 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 Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.134 diff -c -c -r1.134 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 14 Mar 2005 06:19:01 - 1.134 --- doc/src/sgml/ref/psql-ref.sgml 25 Apr 2005 20:01:05 - *** *** 2050,2055 --- 2050,2075 + +rollback +psql + + ON_ERROR_ROLLBACK + + + When on (the default), in interactive mode, + ignore errors generated by commands in a transaction block, + rather than aborting the transaction. Ignoring errors never + happens in non-interactive mode or if the value is + off. The on_error_rollback-on mode works by issuing + an implicit SAVEPONT for you, just before each + command that is in a transaction block, and rolls back to the + savepoint on error. + + + + + ON_ERROR_STOP Index: src/bin/psql/common.c === RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v retrieving revision 1.96 diff -c -c -r1.96 common.c *** src/bin/psql/common.c 22 Feb 2005 04:40:52 - 1.96 --- src/bin/psql/common.c 25 Apr 2005 20:01:08 - *** *** 941,951 bool SendQuery(const char *query) { ! PGresult *results; ! TimevalStruct before, ! after; ! boolOK; ! if (!pset.db) { psql_error("You are currently not connected to a database.\n"); --- 941,952 bool SendQuery(const char *query) { ! PGresult*results; ! TimevalStruct before, after; ! bool OK, on_error_rollback_savepoint = false; ! PGTransactionStatusType transaction_status; ! static bool on_error_rollback_warning = false; ! if (!pset.db) { psql_error("You are currently not connected to a database.\n"); *** *** 973,979 SetCancelConn(); ! if (PQtransactionStatus(pset.db) == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { --- 974,982 SetCancelConn(); ! transaction_status = PQtransactionStatus(pset.db); ! ! if (transaction_status == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { *** *** 987,992 --- 990,1019 } PQclear(results); } + else if (transaction_status == PQTRANS_INTRANS && +pset.cur_cmd_interactive && +
Re: [PATCHES] Continue transactions after errors in psql
Alvaro Herrera wrote: > On Mon, Apr 25, 2005 at 12:34:00PM -0400, Bruce Momjian wrote: > > > > Finally had a chance to sit down at look at this afresh, and I'm > > > pretty sure I've got all the kinks worked out this time. Apologies > > > for not attaching, but my mail system is not working well enough > > > at the moment. So, please try to break this patch: > > This will only be activated when using interactive input, right? > Seems dangerous to apply the setting to scripts. What if the user > enables the feature in .psqlrc and then forgets? I just posted this question to hacker to get votes. -- 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 ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Continue transactions after errors in psql
Bruce Momjian wrote: Michael Paesold wrote: Some suggestions in random order: * I think you should use PSQLexec instead of using PQexec directly. PSQLexec is used by all \-commands and prints out queries with -E, which is very helpful for debugging. -E display queries that internal commands generate It is now \set ON_ERROR_ROLLBACK, and PQexec seems right for that. Also, this isn't something like \d where anyone would want to see the queries, I think. I just thought it was nice for debugging. E.g. your example below would be more easy to analyze if one could see the queries with -E. * You do not check for the server version before activating \reseterror. -> use PQserverVersion() to check for >= 8 Added. Patch just posted. Ok, looks good. * Perhaps the name should be \reseterrors (plural)? Just my personal opinion though. Changed, as you see above. My first patch for this feature (last year) also used \set. I think this is more consistent. On the other hand there is no auto-completition for \set. Perhaps this should be added later. * If I read the code correctly, you now don't destroy user savepoints anymore, but on the other hand, you do not release the psql savepoint after >> a user-defined savepoint is released. In other words, each time a user creates a savepoint, one psql savepoint is left on the subxact stack. I don't know if this is a real problem, though. Interesting. I thought this would fail, but it doesn't: [example...] Yeah, I tried that earlier. What Greg's code does, effectively, is to move the savepoint down below the SAVEPOINt/RELEASE/ROLLBACK so it doesn't discard the user command. Nice trick: [code...] I think it is quite good. But note: I did not say that the feature broke user savepoint, I just mentioned that with user savepoints, some (internal) savepoint could be left on the stack (in the server) until the user defined savepoints below the interal ones would be released. Nevertheless, I think this is no problem in the real-world. * You have not yet implemented a way to savely put \reseterror in .psqlrc. I previously suggested an AUTO setting (additional to ON/OFF) that disables \reseterror when reading from a non-tty. So putting \reseterror AUTO in .psqlrc would be save. Good question, or rather, should ON_ERROR_ROLLBACK have an effect when commands come from a file? There is no way to test for the error in psql so it seems you would never want the transaction to continue after an error. I am inclined to make ON_ERROR_ROLLBACK work only for interactive sessions, just like ON_ERROR_STOP works only for non-interactive sessions. +1 for disabling ON_ERROR_ROLLBACK if pset.cur_cmd_interactive is false. Or provide another switch that can be put in .psqlrc and is only activated for pset.cur_cmd_interactive. Btw. thanks Bruce for getting this done. Best Regards, Michael Paesold ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Continue transactions after errors in psql
Greg Sabino Mullane wrote: > > The current way is ok for me at the moment. I still think there is a better > > way (parsing statements like it's already done for > > no-transaction-allowed-statements), but hey, as soon as your patch will be > > applied, I can myself propose another patch to improve this. ;-) > > Parsing the statment will not help: even if the statement is a savepoint, we > need to wrap it in case we need to roll it back. The only other option I > can see to my patch is to, upon a successful user savepoint creation, > roll back their savepoint and immediately reissue it. That seems worse to > me than having N*2 savepoints though. Agreed. -- 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 ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: > "Michael Paesold" <[EMAIL PROTECTED]> writes: > > I do think so. In it's current state, would you yourself put \reseterror in > > your .psqlrc? Or even an /etc/psqlrc? > > It would break all my scripts that must either succeed or fail -- now they > > will produce garbage in my databases when something goes wrong! > > This is sounding a whole lot like the concerns that prompted us to > reject server-side autocommit a little while ago. > > The problem with rejiggering error-handling behavior is that you *will* > break existing code, on a rather fundamental level, and it's not even > obvious that it's broken until after things have gone badly wrong. > > I don't have a good solution, but I do think that you need to set things > up so that an application or script must invoke the new behavior > explicitly. Hidden defaults that silently change such behavior look > like land mines waiting to be stepped on. Right, this is off by default. We might be able to make it on by default if we have it enabled only for interactive psql's. We need to discuss this. -- 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 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Continue transactions after errors in psql
Greg Sabino Mullane wrote: > > * If I read the code correctly, you now don't destroy user savepoints > > anymore, but on the other hand, you do not release the psql savepoint after > > a user-defined savepoint is released. In other words, each time a user > > creates a savepoint, one psql savepoint is left on the subxact stack. I > > don't know if this is a real problem, though. > > Correct. More detail: we release our own temporary savepoint, unless the user > has successfully implemented their own savepoint. We need to do this so that > we > do not clobber the user's savepoint. The larger problem is that "our" > savepoints > and the user's savepoints tend to clobber each other. The normal flow of > things > is to issue our savepoint, then the user's command, and then check to see if > the > command succcessfully completed, and if we are still in a transaction. If we > are > no longer in a transaction, we do nothing, as it means that our savepoint has > been > destroyed, so we don't need to worry about it. Otherwise, if the command > failed, > we issue a rollback of our savepoint, which is guaranteed to be there because > the > user cannot have removed it, because their command did not succeed. Now the > tricky > part: If the transaction is still active, and the command succeeded, and the > command > was not SAVEPOINT, ROLLBACK TO, or RELEASE, we issue a release of our > savepoint, > which is not strictly necessary, but is a good idea so we don't build up a > large > chunk of old savepoints. Aside: we check if the command they issued was a > savepoint- > manipulating one by not parsing the SQL (yuck) but by simply checking the > cmdResult > string. Although there is no way to tell "RELEASE" from "RELEASE TO" from > this check, > we know it cannot be the former because we are still in a transaction. :) If > it was > one of those three commands, we do not issue a release. If they issued a > successful > release or rollback, then it just clobbered our savepoint, which now no > longer exists. > If it was a savepoint, we cannot release, or we will clobber their savepoint, > which > was created after ours. We could theoretically try and figure out beforehand > if > they are issuing a savepoint command, but we must wrap it anyway in case it > fails so > we can rollback and not have it end the outer transaction. Thus, we create > one extra > savepoint every time the user issues a savepoint. Until they rollback or > release, of > course, in which case they also remove an equal number of our savepoints as > their > savepoints. So it doubles the number of savepoints a user currently has, but > this > is the price we pay for having the feature. Oh, here's his description. I updated the patch comments: + /* + * Do nothing if they are messing with savepoints themselves: + * If the user did RELEASE or ROLLBACK, our savepoint is gone. + * If they issued a SAVEPOINT, releasing ours would remove theirs. + */ -- 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 ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Continue transactions after errors in psql
Michael Paesold wrote: > Greg Sabino Mullane wrote: > > > Finally had a chance to sit down at look at this afresh, and I'm > > pretty sure I've got all the kinks worked out this time. Apologies > > for not attaching, but my mail system is not working well enough > > at the moment. So, please try to break this patch: > > > > http://www.gtsm.com/pg/psql_error_recovery.diff > > > Some suggestions in random order: > > * I think you should use PSQLexec instead of using PQexec directly. PSQLexec > is used by all \-commands and prints out queries with -E, which is very > helpful for debugging. > > -E display queries that internal commands generate It is now \set ON_ERROR_ROLLBACK, and PQexec seems right for that. Also, this isn't something like \d where anyone would want to see the queries, I think. > > * You do not check for the server version before activating \reseterror. > -> use PQserverVersion() to check for >= 8 Added. Patch just posted. > * Perhaps the name should be \reseterrors (plural)? Just my personal opinion > though. Changed, as you see above. > * If I read the code correctly, you now don't destroy user savepoints > anymore, but on the other hand, you do not release the psql savepoint after > a user-defined savepoint is released. In other words, each time a user > creates a savepoint, one psql savepoint is left on the subxact stack. I > don't know if this is a real problem, though. Interesting. I thought this would fail, but it doesn't: test=> \set ON_ERROR_ROLLBACK on test=> begin; BEGIN test=> savepoint user1; SAVEPOINT test=> lkjasdfsadf; ERROR: syntax error at or near "lkjasdfsadf" at character 1 LINE 1: lkjasdfsadf; ^ test=> rollback to savepoint user1; ROLLBACK and I think this is the reason: test=> BEGIN; BEGIN test=> SAVEPOINT psql1; SAVEPOINT test=> SAVEPOINT user1; SAVEPOINT test=> SAVEPOINT psql1; SAVEPOINT test=> lkjasd; ERROR: syntax error at or near "lkjasd" at character 1 LINE 1: lkjasd; ^ test=> ROLLBACK TO psql1; ROLLBACK test=> ROLLBACK TO user1; ROLLBACK What Greg's code does, effectively, is to move the savepoint down below the SAVEPOINt/RELEASE/ROLLBACK so it doesn't discard the user command. Nice trick: + /* +* Do nothing if they are messing with savepoints themselves: +* doing otherwise would cause us to remove their savepoint, +* or have us rollback our savepoint they have just removed +*/ + if (strcmp(PQcmdStatus(results), "SAVEPOINT") == 0 || + strcmp(PQcmdStatus(results), "RELEASE") == 0 || + strcmp(PQcmdStatus(results), "ROLLBACK") ==0) + results = NULL; > * You have not yet implemented a way to savely put \reseterror in .psqlrc. I > previously suggested an AUTO setting (additional to ON/OFF) that disables > \reseterror when reading from a non-tty. So putting \reseterror AUTO in > .psqlrc would be save. Good question, or rather, should ON_ERROR_ROLLBACK have an effect when commands come from a file? There is no way to test for the error in psql so it seems you would never want the transaction to continue after an error. I am inclined to make ON_ERROR_ROLLBACK work only for interactive sessions, just like ON_ERROR_STOP works only for non-interactive sessions. Comments? -- 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 ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Continue transactions after errors in psql
On Mon, Apr 25, 2005 at 12:34:00PM -0400, Bruce Momjian wrote: > > Finally had a chance to sit down at look at this afresh, and I'm > > pretty sure I've got all the kinks worked out this time. Apologies > > for not attaching, but my mail system is not working well enough > > at the moment. So, please try to break this patch: This will only be activated when using interactive input, right? Seems dangerous to apply the setting to scripts. What if the user enables the feature in .psqlrc and then forgets? -- Alvaro Herrera (<[EMAIL PROTECTED]>) "Y eso te lo doy firmado con mis lágrimas" (Fiebre del Loco) ---(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
Re: [PATCHES] Continue transactions after errors in psql
Greg Sabino Mullane wrote: [ There is text before PGP section. ] > > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > > Finally had a chance to sit down at look at this afresh, and I'm > pretty sure I've got all the kinks worked out this time. Apologies > for not attaching, but my mail system is not working well enough > at the moment. So, please try to break this patch: > > http://www.gtsm.com/pg/psql_error_recovery.diff I have modified Greg's patch to fit in better with the existing psql code. I changed the command to \set ON_ERROR_ROLLBACK, which seems to fit better. Here is an illustration of its use (patch attached): test=> BEGIN; BEGIN test=> lkjasdf; ERROR: syntax error at or near "lkjasdf" at character 1 LINE 1: lkjasdf; ^ test=> SELECT 1; ERROR: current transaction is aborted, commands ignored until end of transaction block test=> COMMIT; ROLLBACK test=> \set ON_ERROR_ROLLBACK on test=> BEGIN; BEGIN test=> lkjasdf; ERROR: syntax error at or near "lkjasdf" at character 1 LINE 1: lkjasdf; ^ test=> SELECT 1; ?column? -- 1 (1 row) test=> COMMIT; COMMIT -- 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 Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.134 diff -c -c -r1.134 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 14 Mar 2005 06:19:01 - 1.134 --- doc/src/sgml/ref/psql-ref.sgml 25 Apr 2005 16:26:03 - *** *** 2050,2055 --- 2050,2070 + +rollback +psql + + ON_ERROR_ROLLBACK + + + When in a transaction, wrap every command in a savepoint that is + rolled back on error. Thus, errors will not abort an open + transaction. + + + + + ON_ERROR_STOP Index: src/bin/psql/common.c === RCS file: /cvsroot/pgsql/src/bin/psql/common.c,v retrieving revision 1.96 diff -c -c -r1.96 common.c *** src/bin/psql/common.c 22 Feb 2005 04:40:52 - 1.96 --- src/bin/psql/common.c 25 Apr 2005 16:26:05 - *** *** 941,950 bool SendQuery(const char *query) { ! PGresult *results; ! TimevalStruct before, ! after; ! boolOK; if (!pset.db) { --- 941,950 bool SendQuery(const char *query) { ! PGresult*results; ! TimevalStruct before, after; ! bool OK, on_error_rollback_savepoint = false; ! PGTransactionStatusType transaction_status; if (!pset.db) { *** *** 973,979 SetCancelConn(); ! if (PQtransactionStatus(pset.db) == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { --- 973,981 SetCancelConn(); ! transaction_status = PQtransactionStatus(pset.db); ! ! if (transaction_status == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { *** *** 987,992 --- 989,1016 } PQclear(results); } + else if (transaction_status == PQTRANS_INTRANS && +GetVariableBool(pset.vars, "ON_ERROR_ROLLBACK")) + { + if (pset.sversion < 8) + { + fprintf(stderr, _("The server version (%d) does not support savepoints for ON_ERROR_ROLLBACK.\n"), + pset.sversion); + } + else + { + results = PQexec(pset.db, "SAVEPOINT pg_psql_temporary_savepoint"); + if (PQresultStatus(results) != PGRES_COMMAND_OK) + { + psql_error("%s", PQerrorMessage(pset.db)); + PQclear(results); + ResetCancelConn(); + return false; + } + PQclear(results); + on_error_rollback_savepoint = true; + } + } if (pset.timing) GETTIMEOFDAY(&before); *** *** 1005,1010 --- 1029,1069 PQclear(re
Re: [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Tom Lane wrote: > I don't have a good solution, but I do think that you need to set things > up so that an application or script must invoke the new behavior > explicitly. Hidden defaults that silently change such behavior look > like land mines waiting to be stepped on. Michael Paesold wrote: > I think without tty-detection, the patch just conflicts with PostgreSQL > philosophy that the user should be kept save from unintended > data-destruction. I don't know if I would go that far. This is a setting that must be explicitly enabled, so it's more a case of caveat emptor is you choose to enable it. I don't like the idea of the behavior changing so radically just depending on the tty state. Maybe if we call it "ttyonly" or something instead of "auto"...? > The SQL-Standard itself says that errors inside transactions should only > rollback the last statement, if possible. So why is that not implemented in > PostgreSQL? What I read from past discussions here, is because it's just > unsave and will lead to data-garbage if you aren't very careful. That's a good point: if that is indeed what the standard says, we should probably see about following it. Rolling back to the last savepoint seems a reasonable behavior to me. > The current way is ok for me at the moment. I still think there is a better > way (parsing statements like it's already done for > no-transaction-allowed-statements), but hey, as soon as your patch will be > applied, I can myself propose another patch to improve this. ;-) Parsing the statment will not help: even if the statement is a savepoint, we need to wrap it in case we need to roll it back. The only other option I can see to my patch is to, upon a successful user savepoint creation, roll back their savepoint and immediately reissue it. That seems worse to me than having N*2 savepoints though. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200503121836 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFCM30WvJuQZxSWSsgRAryZAKCyIDYd36mAaU464AbPkHe9zkYI+QCfU+Fb 7A2WJwLJcOvzJDHjRnr55v4= =UJ8E -END PGP SIGNATURE- ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Continue transactions after errors in psql
"Michael Paesold" <[EMAIL PROTECTED]> writes: > I do think so. In it's current state, would you yourself put \reseterror in > your .psqlrc? Or even an /etc/psqlrc? > It would break all my scripts that must either succeed or fail -- now they > will produce garbage in my databases when something goes wrong! This is sounding a whole lot like the concerns that prompted us to reject server-side autocommit a little while ago. The problem with rejiggering error-handling behavior is that you *will* break existing code, on a rather fundamental level, and it's not even obvious that it's broken until after things have gone badly wrong. I don't have a good solution, but I do think that you need to set things up so that an application or script must invoke the new behavior explicitly. Hidden defaults that silently change such behavior look like land mines waiting to be stepped on. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Continue transactions after errors in psql
Greg Sabino Mullane wrote: * You have not yet implemented a way to savely put \reseterror in .psqlrc. I previously suggested an AUTO setting (additional to ON/OFF) that disables \reseterror when reading from a non-tty. So putting \reseterror AUTO in ..psqlrc would be save. Hmm...I suppose we could do that. Do we have anything else that does something similar? I guess I'm not convinced that we need to change a switch's behavior based on the tty status. I do think so. In it's current state, would you yourself put \reseterror in your .psqlrc? Or even an /etc/psqlrc? It would break all my scripts that must either succeed or fail -- now they will produce garbage in my databases when something goes wrong! In my opinion, the behaviour should depend on tty in all settings, but I am o.k. with an AUTO setting, because so it's at least usable. I think without tty-detection, the patch just conflicts with PostgreSQL philosophy that the user should be kept save from unintended data-destruction. The SQL-Standard itself says that errors inside transactions should only rollback the last statement, if possible. So why is that not implemented in PostgreSQL? What I read from past discussions here, is because it's just unsave and will lead to data-garbage if you aren't very careful. * If I read the code correctly, you now don't destroy user savepoints anymore, but on the other hand, you do not release the psql savepoint after a user-defined savepoint is released. In other words, each time a user creates a savepoint, one psql savepoint is left on the subxact stack. I don't know if this is a real problem, though. Correct. More detail: we release our own temporary savepoint, unless the user has successfully implemented their own savepoint... The current way is ok for me at the moment. I still think there is a better way (parsing statements like it's already done for no-transaction-allowed-statements), but hey, as soon as your patch will be applied, I can myself propose another patch to improve this. ;-) Best Regards, Michael Paesold ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 > * I think you should use PSQLexec instead of using PQexec directly. PSQLexec > is used by all \-commands and prints out queries with -E, which is very > helpful for debugging. But these are not backslash commands, but almost directly analogous to the BEGINs emitted by psql when in AutoCommit mode. On the other hand, it might be neat to see all the savepoints psql will automagically create for you, so I could go either way. > * You do not check for the server version before activating \reseterror. > -> use PQserverVersion() to check for >= 8 Thanks, it was in an earlier version, promise. This should be in command.c: if (pset.sversion < 8) { fprintf(stderr, _("The server version (%d) does not support savepoints.\n"), pset.sversion); } > * Perhaps the name should be \reseterrors (plural)? Just my personal > opinion though. Nah, less errors from people typing the wrong thing if we keep it shorter. > * You have not yet implemented a way to savely put \reseterror in .psqlrc. I > previously suggested an AUTO setting (additional to ON/OFF) that disables > \reseterror when reading from a non-tty. So putting \reseterror AUTO in > ..psqlrc would be save. Hmm...I suppose we could do that. Do we have anything else that does something similar? I guess I'm not convinced that we need to change a switch's behavior based on the tty status. > * If I read the code correctly, you now don't destroy user savepoints > anymore, but on the other hand, you do not release the psql savepoint after > a user-defined savepoint is released. In other words, each time a user > creates a savepoint, one psql savepoint is left on the subxact stack. I > don't know if this is a real problem, though. Correct. More detail: we release our own temporary savepoint, unless the user has successfully implemented their own savepoint. We need to do this so that we do not clobber the user's savepoint. The larger problem is that "our" savepoints and the user's savepoints tend to clobber each other. The normal flow of things is to issue our savepoint, then the user's command, and then check to see if the command succcessfully completed, and if we are still in a transaction. If we are no longer in a transaction, we do nothing, as it means that our savepoint has been destroyed, so we don't need to worry about it. Otherwise, if the command failed, we issue a rollback of our savepoint, which is guaranteed to be there because the user cannot have removed it, because their command did not succeed. Now the tricky part: If the transaction is still active, and the command succeeded, and the command was not SAVEPOINT, ROLLBACK TO, or RELEASE, we issue a release of our savepoint, which is not strictly necessary, but is a good idea so we don't build up a large chunk of old savepoints. Aside: we check if the command they issued was a savepoint- manipulating one by not parsing the SQL (yuck) but by simply checking the cmdResult string. Although there is no way to tell "RELEASE" from "RELEASE TO" from this check, we know it cannot be the former because we are still in a transaction. :) If it was one of those three commands, we do not issue a release. If they issued a successful release or rollback, then it just clobbered our savepoint, which now no longer exists. If it was a savepoint, we cannot release, or we will clobber their savepoint, which was created after ours. We could theoretically try and figure out beforehand if they are issuing a savepoint command, but we must wrap it anyway in case it fails so we can rollback and not have it end the outer transaction. Thus, we create one extra savepoint every time the user issues a savepoint. Until they rollback or release, of course, in which case they also remove an equal number of our savepoints as their savepoints. So it doubles the number of savepoints a user currently has, but this is the price we pay for having the feature. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200503070028 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFCK+a6vJuQZxSWSsgRAsGRAJ99vJ0Mlzzl8MWBv262K//h0NasLwCgiBHZ o2tgPvfwHR8zSJ1TAJ5/x30= =itOf -END PGP SIGNATURE- ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Continue transactions after errors in psql
Greg Sabino Mullane wrote: Finally had a chance to sit down at look at this afresh, and I'm pretty sure I've got all the kinks worked out this time. Apologies for not attaching, but my mail system is not working well enough at the moment. So, please try to break this patch: http://www.gtsm.com/pg/psql_error_recovery.diff Some suggestions in random order: * I think you should use PSQLexec instead of using PQexec directly. PSQLexec is used by all \-commands and prints out queries with -E, which is very helpful for debugging. -E display queries that internal commands generate * You do not check for the server version before activating \reseterror. -> use PQserverVersion() to check for >= 8 * Perhaps the name should be \reseterrors (plural)? Just my personal opinion though. * If I read the code correctly, you now don't destroy user savepoints anymore, but on the other hand, you do not release the psql savepoint after a user-defined savepoint is released. In other words, each time a user creates a savepoint, one psql savepoint is left on the subxact stack. I don't know if this is a real problem, though. * You have not yet implemented a way to savely put \reseterror in .psqlrc. I previously suggested an AUTO setting (additional to ON/OFF) that disables \reseterror when reading from a non-tty. So putting \reseterror AUTO in .psqlrc would be save. Otherwise, I could not find a way to break it. :-) Best Regards, Michael Paesold ---(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
Re: [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Finally had a chance to sit down at look at this afresh, and I'm pretty sure I've got all the kinks worked out this time. Apologies for not attaching, but my mail system is not working well enough at the moment. So, please try to break this patch: http://www.gtsm.com/pg/psql_error_recovery.diff - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200503060152 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFCKqjXvJuQZxSWSsgRArIsAJ9wR99qOOCrfS8Hly7xNnWHV/RSHwCfQUac V2Xr4prpz50+nJJe6ci1rLY= =F6aX -END PGP SIGNATURE- ---(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
Re: [PATCHES] Continue transactions after errors in psql
This thread has been saved for the 8.1 release: http://momjian.postgresql.org/cgi-bin/pgpatches2 --- Greg Sabino Mullane wrote: > -BEGIN PGP SIGNED MESSAGE- > Hash: SHA1 > > > Attached is a patch that takes advantage of savepoints to enable > transactions to continue even after errors in psql. The name of it > is \reseterror, and it is off by default. It's backwards compatible, > and allows things like this to work on 8.0 and up servers: > > \reseterror > BEGIN; > DELETE FROM foobar; > INSERT INTO foobar(a) VALUES(1); > ISNER INTO foobar(a) VALUES(2); > INSERT INTO foobar(a) VALUES(3); > COMMIT; > > Doing a SELECT(a) FROM foobar will show two values, 1 and 3. This > is a great help for those of us that tend to type typos into our > psql session, and end up cursing as we have to restart our current > transaction. :) > > - -- > Greg Sabino Mullane [EMAIL PROTECTED] > PGP Key: 0x14964AC8 200501252203 > http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 > -BEGIN PGP SIGNATURE- > > iD8DBQFB9wlpvJuQZxSWSsgRAsAzAKCxQ/JtR6/RXgV39uDTm9FIxCIp8QCeKC6T > 2l10ef5DHkmFC2dSMQLNHjg= > =HKv9 > -END PGP SIGNATURE- > [ 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 ---(end of broadcast)--- TIP 3: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 > Do you think it's better to create a server-side function or > handle that in the client? How hard would it be to implement > such a function? And what should it return? Only the name of > the current top savepoint? I think handled on the client. Otherwise, this will not work for 8.0 and I'd like to see it able to do so. I tbink the logic presented so far is good: I'll work on getting a new patch out as soon as I can. Still, a server-side function would eventually be nice, perhaps have it return a setof savepoint names in order, allowing one to easily grab the whole list or just the "latest/current" one. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200502012248 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFCAE5IvJuQZxSWSsgRAqy7AJ4wo03ir/qRlRUxdC4sXId4OvlsswCgy50l ldB3hFJaO88sBV1rfbADwwU= =la3h -END PGP SIGNATURE- ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [PATCHES] Continue transactions after errors in psql
Alvaro Herrera wrote: >Michael Paesold wrote: Alvaro suggested to implement such a function. It is not there yet. I think you would have to access the sub xact stack, but I have not looked into that code for quite some time. http://archives.postgresql.org/pgsql-general/2004-10/msg00370.php The only problem with this idea is that the function cannot be run when the transaction is in aborted state. Not sure if that is a problem or not. What happens if the user does I don't think there is a problem. If the transaction is in aborted state, we only care if the last statement has aborted the transaction. Otherwise we would not issue our savepoint at all. In that case, i.e. if the last statement aborted the transaction, we can roll it back anyway, can't we? There can't be a savepoint on top of us, because that would have failed right now. Is my logic wrong? SAVEPOINT foo; SLECT 1; ROLLBACK TO foo; all in one command in psql? I know psql sends that as three commands, so maybe it's not an issue. As far as I remember psql splits the three commands, so there would be an implicit savepoint for each individual statement: * SAVEPOINT pg_psql_savepoint; -- [1] SAVEPOINT foo; * SAVEPOINT pg_psql_savepoint; -- [2] SLECT 1; * ROLLBACK TO pg_psql_savepoint; -- [2] * SAVEPOINT pg_psql_savepoint; -- [3] ROLLBACK TO foo; * RELEASE pg_psql_savepoint; -- [3] * RELEASE pg_psql_savepoint; -- [1], because pg_psql_savepoint is on top of the stack now again I hope you get the point. ;-) Do you think it's better to create a server-side function or handle that in the client? How hard would it be to implement such a function? And what should it return? Only the name of the current top savepoint? Best Regards, Michael Paesold ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Continue transactions after errors in psql
On Sat, Jan 29, 2005 at 01:04:36PM +0100, Michael Paesold wrote: > Greg Sabino Mullane wrote: > > >Michael Paesold wrote: > >>2) Implement a server-side function to get the savepoints from the server > >>and query that before every release. > > > >I could not find a way to do this. Is there any interface to the list? > > Alvaro suggested to implement such a function. It is not there yet. I think > you would have to access the sub xact stack, but I have not looked into > that code for quite some time. > http://archives.postgresql.org/pgsql-general/2004-10/msg00370.php The only problem with this idea is that the function cannot be run when the transaction is in aborted state. Not sure if that is a problem or not. What happens if the user does SAVEPOINT foo; SLECT 1; ROLLBACK TO foo; all in one command in psql? I know psql sends that as three commands, so maybe it's not an issue. -- Alvaro Herrera (<[EMAIL PROTECTED]>) "Doing what he did amounts to sticking his fingers under the hood of the implementation; if he gets his fingers burnt, it's his problem." (Tom Lane) ---(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
Re: [PATCHES] Continue transactions after errors in psql
Greg Sabino Mullane wrote: Michael Paesold wrote: 2) Implement a server-side function to get the savepoints from the server and query that before every release. I could not find a way to do this. Is there any interface to the list? Alvaro suggested to implement such a function. It is not there yet. I think you would have to access the sub xact stack, but I have not looked into that code for quite some time. http://archives.postgresql.org/pgsql-general/2004-10/msg00370.php First, I'm not of the opinion that it should automatically be turned off when running non-interactively. That's too much assuming of what the user wants, when this is a settable flag. However, it should be settable via a script to a definite state. So \reseterror will take an optional argument, "off" or "on", which sets it rather than toggles it. Discussion here last year showed some concern from people that this feature could bite people and is not really safe. Perhaps the best way would be to create three states: \reseterrors (on|off|auto) where auto means it's only active for interactive queries. (auto could be named interactive) The other problem is not stepping on other people's savepoints. The best solution we came up with was to check for savepoint commands ourselves, similar to the way psql already checks for transaction affecting commands, and handle things appropriately. Specifically, if someone issues a savepoint while in \reseterror mode, it switches off automatically*. Since the implementation of reseterror is pretty much a lazy shortcut to issuing savepoints yourself, it should be safe to say that you do not want to mix manual and automatic ones, and we'll back off (with a message) if you issue your own. Plus there will be a warning in the docs to be careful about mixing savepoints and the \reseterror method. * We could also switch it back on after rollback or release, but this would entail a little more tracking. Comments? I would prefer a solution, where the feature is not disabled as soon as I use my own savepoints. I like \reseterror because it prevents making typos from aborting my transaction. Explicit savepoints I rather use to try a whole bunch of statements and then decide if I want to commit so far. I can still make typos. If you don't want to, I can implement such a savepoint stack. I don't think it's that hard. The parsing, as you mentioned, should also not be too hard, because the infrastructure (removing white space) is already there. Best Regards, Michael Paesold ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Michael Paesold wrote: > 2) Implement a server-side function to get the savepoints from the server > and query that before every release. I could not find a way to do this. Is there any interface to the list? I looked over the patch from Michael Paesold, and talked extensively with Robert Treat about this, and here is the solution Robert and I came up with: (thanks to both for their work) First, I'm not of the opinion that it should automatically be turned off when running non-interactively. That's too much assuming of what the user wants, when this is a settable flag. However, it should be settable via a script to a definite state. So \reseterror will take an optional argument, "off" or "on", which sets it rather than toggles it. The patch Robert provided shold catch the problem of "good command-commit". The other problem is not stepping on other people's savepoints. The best solution we came up with was to check for savepoint commands ourselves, similar to the way psql already checks for transaction affecting commands, and handle things appropriately. Specifically, if someone issues a savepoint while in \reseterror mode, it switches off automatically*. Since the implementation of reseterror is pretty much a lazy shortcut to issuing savepoints yourself, it should be safe to say that you do not want to mix manual and automatic ones, and we'll back off (with a message) if you issue your own. Plus there will be a warning in the docs to be careful about mixing savepoints and the \reseterror method. * We could also switch it back on after rollback or release, but this would entail a little more tracking. Comments? - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200501282306 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFB+wzovJuQZxSWSsgRAt5eAJ9BVMtYZ9H+A76cNdUuhv4GpXeCwQCdFVsi +mgg6ZzMylgHgdfiVn4yI5o= =CpZQ -END PGP SIGNATURE- ---(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
Re: [PATCHES] Continue transactions after errors in psql
On Fri, 2005-01-28 at 04:46, Christopher Kings-Lynne wrote: > > I've attached a revised patch which fixes the problem, however I'm sure > > there > > is a better way. Thanks to Neil for putting up with me on irc :-) > > How about calling the savepoint pg_psql_savepoint instead, that way it > follows our 'don't begin things with pg_' philosophy. > I was actually thinking of calling it something like pg_ thinking that would be pretty unique within a transaction, though having a specific documented name seemed ok too. Robert Treat -- Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] Continue transactions after errors in psql
Robert Treat wrote: I've attached a revised patch which fixes the problem, however I'm sure there is a better way. Thanks to Neil for putting up with me on irc :-) In September 2004 I had already sent a patch to implement this behaviour, the patch, still in the archives, is here: http://archives.postgresql.org/pgsql-patches/2004-09/bin00040.bin (savepoints.patch) There are some issues it addressed: Assuming you put this option in your .psqlrc file, you will still probably not want this to be active when you execute commands from a file (non-interactive). So pset.notty must be checked. Again, when using \i, resetting errors seems dangerous. Using \i should also temporarily disable those savepoints. The real problem with my patch was, that it did not release the savepoints. Why? Look at this example (with the current patch reseterrors patch): template1=# \reseterror Reset error is on. template1=# BEGIN; BEGIN template1=# SAVEPOINT a; SAVEPOINT template1=# CREATE TABLE TEST ( a integer); CREATE TABLE template1=# ROLLBACK TO a; ERROR: no such savepoint So to get this right, you have to track savepoints created by the user and only release psql savepoints when there is no user savepoint "sitting on top of" your savepoint. Two ways come to my mind: 1) Parse SQL for savepoint and rollback to and create a stack of all savepoints. Then you can always release all savepoints as long as they are your own. 2) Implement a server-side function to get the savepoints from the server and query that before every release. What do you think? Best Regards, Michael Paesold ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
Re: [PATCHES] Continue transactions after errors in psql
I've attached a revised patch which fixes the problem, however I'm sure there is a better way. Thanks to Neil for putting up with me on irc :-) How about calling the savepoint pg_psql_savepoint instead, that way it follows our 'don't begin things with pg_' philosophy. Chris ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] Continue transactions after errors in psql
On Tuesday 25 January 2005 22:07, Greg Sabino Mullane wrote: > Attached is a patch that takes advantage of savepoints to enable > transactions to continue even after errors in psql. The name of it > is \reseterror, and it is off by default. It's backwards compatible, > and allows things like this to work on 8.0 and up servers: > > \reseterror > BEGIN; > DELETE FROM foobar; > INSERT INTO foobar(a) VALUES(1); > ISNER INTO foobar(a) VALUES(2); > INSERT INTO foobar(a) VALUES(3); > COMMIT; > > Doing a SELECT(a) FROM foobar will show two values, 1 and 3. This > is a great help for those of us that tend to type typos into our > psql session, and end up cursing as we have to restart our current > transaction. :) I've been testing this patch and found the following bug: test=# \reseterror Reset error is on. test=# begin; BEGIN test=# select * from t; c --- 1 (1 row) test=# delete from t; DELETE 1 test=# select * from tt; ERROR: relation "tt" does not exist ERROR: relation "tt" does not exist test=# select * from t; c --- (0 rows) test=# commit; COMMIT ERROR: RELEASE SAVEPOINT may only be used in transaction blocks ERROR: RELEASE SAVEPOINT may only be used in transaction blocks I've attached a revised patch which fixes the problem, however I'm sure there is a better way. Thanks to Neil for putting up with me on irc :-) -- Robert Treat Build A Brighter Lamp :: Linux Apache {middleware} PostgreSQL Index: command.c === RCS file: /projects/cvsroot/pgsql/src/bin/psql/command.c,v retrieving revision 1.139 diff -c -r1.139 command.c *** command.c 1 Jan 2005 05:43:08 - 1.139 --- command.c 28 Jan 2005 06:42:03 - *** *** 646,651 --- 646,672 puts(gettext("Query buffer reset (cleared).")); } + /* \reseterror -- use savepoints to make transaction errors recoverable */ + else if (strcmp(cmd, "reseterror") == 0) + { + if (pset.sversion < 8) + { + printf(gettext("The server version (%d) does not support savepoints.\n"), + pset.sversion); + } + else + { + pset.reseterror = !pset.reseterror; + if (!quiet) + { + if (pset.reseterror) + puts(gettext("Reset error is on.")); + else + puts(gettext("Reset error is off.")); + } + } + } + /* \s save history in a file or show it on the screen */ else if (strcmp(cmd, "s") == 0) { Index: common.c === RCS file: /projects/cvsroot/pgsql/src/bin/psql/common.c,v retrieving revision 1.95 diff -c -r1.95 common.c *** common.c 1 Jan 2005 05:43:08 - 1.95 --- common.c 28 Jan 2005 06:42:03 - *** *** 941,950 bool SendQuery(const char *query) { ! PGresult *results; TimevalStruct before, after; bool OK; if (!pset.db) { --- 941,951 bool SendQuery(const char *query) { ! PGresult *results, *res; TimevalStruct before, after; bool OK; + PGTransactionStatusType tstatus; if (!pset.db) { *** *** 973,979 SetCancelConn(); ! if (PQtransactionStatus(pset.db) == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { --- 974,982 SetCancelConn(); ! tstatus = PQtransactionStatus(pset.db); ! ! if (PQTRANS_IDLE == tstatus && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { *** *** 987,992 --- 990,1010 } PQclear(results); } + else { + /* If we are in error recovery mode and inside a transaction, + possibly issue a temporary savepoint */ + if (PQTRANS_INTRANS==tstatus && pset.reseterror) { + res = PQexec(pset.db, "SAVEPOINT psql_savepoint"); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + { + psql_error("%s", PQerrorMessage(pset.db)); + PQclear(res); + ResetCancelConn(); + return false; + } + PQclear(res); + } + } if (pset.timing) GETTIMEOFDAY(&before); *** *** 1001,1008 /* but printing results isn't: */ if (OK) ! OK = PrintQueryResults(results); ! PQclear(results); /* Possible microtiming output */ --- 1019,1049 /* but printing results isn't: */ if (OK) ! OK = PrintQueryResults(results); ! ! /* If in error recovery mode, release the savepoint */ ! ! if (PQTRANS_INTRANS==tstatus && pset.reseterror) { ! tstatus = PQtransactionStatus(pset.db); ! ! if (PQTRANS_INERROR==tstatus) ! res = PQexec(pset.db, "ROLLBACK TO psql_savepoint"); ! else if (PQTRANS_IDLE==tstatus) ! /* COMMITing leaves us in PQTRANS_IDLE so we can't release the save point here */ ! res = PQexec(pset.db, "SELECT 1"); ! else ! res = PQexec(pset.db, "RELEASE psql_savepoint"); ! ! if (PQresultStatus(res) != PGRES_COMMAND_OK) ! { ! psql_error("%s", PQerrorMessage(pset.db)); ! PQclea
[PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Attached is a patch that takes advantage of savepoints to enable transactions to continue even after errors in psql. The name of it is \reseterror, and it is off by default. It's backwards compatible, and allows things like this to work on 8.0 and up servers: \reseterror BEGIN; DELETE FROM foobar; INSERT INTO foobar(a) VALUES(1); ISNER INTO foobar(a) VALUES(2); INSERT INTO foobar(a) VALUES(3); COMMIT; Doing a SELECT(a) FROM foobar will show two values, 1 and 3. This is a great help for those of us that tend to type typos into our psql session, and end up cursing as we have to restart our current transaction. :) - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200501252203 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFB9wlpvJuQZxSWSsgRAsAzAKCxQ/JtR6/RXgV39uDTm9FIxCIp8QCeKC6T 2l10ef5DHkmFC2dSMQLNHjg= =HKv9 -END PGP SIGNATURE- Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.130 diff -c -r1.130 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 14 Jan 2005 00:24:23 - 1.130 --- doc/src/sgml/ref/psql-ref.sgml 26 Jan 2005 02:57:05 - *** *** 1604,1609 --- 1604,1619 +\reseterror + + + Toggles allowing transactions to continue after errors. + + + + + + \s [ filename ] Index: src/bin/psql/command.c === RCS file: /projects/cvsroot/pgsql/src/bin/psql/command.c,v retrieving revision 1.139 diff -c -r1.139 command.c *** src/bin/psql/command.c 1 Jan 2005 05:43:08 - 1.139 --- src/bin/psql/command.c 26 Jan 2005 02:57:05 - *** *** 646,651 --- 646,672 puts(gettext("Query buffer reset (cleared).")); } + /* \reseterror -- use savepoints to make transaction errors recoverable */ + else if (strcmp(cmd, "reseterror") == 0) + { + if (pset.sversion < 8) + { + printf(gettext("The server version (%d) does not support savepoints.\n"), + pset.sversion); + } + else + { + pset.reseterror = !pset.reseterror; + if (!quiet) + { + if (pset.reseterror) + puts(gettext("Reset error is on.")); + else + puts(gettext("Reset error is off.")); + } + } + } + /* \s save history in a file or show it on the screen */ else if (strcmp(cmd, "s") == 0) { Index: src/bin/psql/common.c === RCS file: /projects/cvsroot/pgsql/src/bin/psql/common.c,v retrieving revision 1.95 diff -c -r1.95 common.c *** src/bin/psql/common.c 1 Jan 2005 05:43:08 - 1.95 --- src/bin/psql/common.c 26 Jan 2005 02:57:05 - *** *** 941,950 bool SendQuery(const char *query) { ! PGresult *results; TimevalStruct before, after; bool OK; if (!pset.db) { --- 941,951 bool SendQuery(const char *query) { ! PGresult *results, *res; TimevalStruct before, after; bool OK; + PGTransactionStatusType tstatus; if (!pset.db) { *** *** 973,979 SetCancelConn(); ! if (PQtransactionStatus(pset.db) == PQTRANS_IDLE && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { --- 974,982 SetCancelConn(); ! tstatus = PQtransactionStatus(pset.db); ! ! if (PQTRANS_IDLE == tstatus && !GetVariableBool(pset.vars, "AUTOCOMMIT") && !command_no_begin(query)) { *** *** 987,992 --- 990,1010 } PQclear(results); } + else { + /* If we are in error recovery mode and inside a transaction, + possibly issue a temporary savepoint */ + if (PQTRANS_INTRANS==tstatus && pset.reseterror) { + res = PQexec(pset.db, "SAVEPOINT psql_savepoint"); + if (PQresultStatus(res) != PGRES_COMMAND_OK) + { + psql_error("%s", PQerrorMessage(pset.db)); + PQclear(res); + ResetCancelConn(); + return false; + } + PQclear(res); + } + } if (pset.timing) GETTIMEOFDAY(&before); *** *** 1001,1008 /* but printing results isn't: */ if (OK) ! OK = PrintQueryResults(results); ! PQclear(results); /* Possible microtiming output */ --- 1019,1041 /* but printing results isn't: */ if (OK) ! OK = PrintQueryResults(results); ! ! /* If in error recovery mode, release the savepoint */ ! if (PQTRANS_INTRANS==tstatus && pset.reseterror) { ! tstatus = PQtransactionStatus(pset.db); ! res = PQexec(pset.db, PQTRANS_INERROR==tstatus ? ! "ROLLBACK TO psql_savepoint" : "RELEASE psql_savepoint"); ! if (PQresultStatus(res) != PGRES_COMMAND_OK) ! { !