Re: [HACKERS] [PATCHES] Continue transactions after errors in psql

2005-04-28 Thread Bruce Momjian

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

2005-04-27 Thread Bruce Momjian
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

2005-04-27 Thread Bruce Momjian
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

2005-04-27 Thread Tom Lane
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

2005-04-27 Thread Bruce Momjian
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

2005-04-27 Thread Robert Treat
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

2005-04-27 Thread Greg Sabino Mullane

-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

2005-04-26 Thread John DeSoi
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

2005-04-26 Thread Tom Lane
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

2005-04-26 Thread Andrew Dunstan

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

2005-04-26 Thread Tom Lane
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

2005-04-26 Thread Andrew Dunstan

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

2005-04-26 Thread Tom Lane
"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

2005-04-26 Thread Tom Lane
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

2005-04-26 Thread Joshua D. Drake

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

2005-04-26 Thread Tom Lane
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

2005-04-26 Thread Michael Paesold
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

2005-04-26 Thread Tom Lane
"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

2005-04-26 Thread Richard Huxton
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

2005-04-26 Thread Michael Paesold
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

2005-04-26 Thread Greg Sabino Mullane

-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

2005-04-25 Thread Bruce Momjian
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

2005-04-25 Thread Bruce Momjian
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

2005-04-25 Thread Bruce Momjian
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

2005-04-25 Thread Michael Paesold
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

2005-04-25 Thread Bruce Momjian
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

2005-04-25 Thread Bruce Momjian
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

2005-04-25 Thread Bruce Momjian
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

2005-04-25 Thread Bruce Momjian
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

2005-04-25 Thread Alvaro Herrera
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

2005-04-25 Thread Bruce Momjian
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

2005-03-13 Thread Greg Sabino Mullane

-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

2005-03-07 Thread Tom Lane
"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

2005-03-07 Thread Michael Paesold
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

2005-03-07 Thread Greg Sabino Mullane

-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

2005-03-07 Thread Michael Paesold
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

2005-03-06 Thread Greg Sabino Mullane

-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

2005-02-14 Thread Bruce Momjian

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

2005-02-01 Thread Greg Sabino Mullane

-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

2005-01-30 Thread Michael Paesold
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

2005-01-29 Thread Alvaro Herrera
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

2005-01-29 Thread Michael Paesold
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

2005-01-28 Thread Greg Sabino Mullane

-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

2005-01-28 Thread Robert Treat
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

2005-01-28 Thread Michael Paesold
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

2005-01-28 Thread Christopher Kings-Lynne
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

2005-01-27 Thread Robert Treat
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

2005-01-25 Thread Greg Sabino Mullane
-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)
! 			{
!