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
/varlistentry
varlistentry
+ indexterm
+primaryrollback/primary
+secondarypsql/secondary
+ /indexterm
+ termvarnameON_ERROR_ROLLBACK/varname/term
+ listitem
+ para
+ When in a transaction, wrap every command in a savepoint that is
+ rolled back on error. Thus, errors will not abort an open
+ transaction.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termvarnameON_ERROR_STOP/varname/term
listitem
para
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;
+ }
+ }
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
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
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
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: 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
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
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: [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
/varlistentry
varlistentry
+ indexterm
+primaryrollback/primary
+secondarypsql/secondary
+ /indexterm
+ termvarnameON_ERROR_ROLLBACK/varname/term
+ listitem
+ para
+ When literalon/ (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
+ literaloff/. The on_error_rollback-on mode works by issuing
+ an implicit commandSAVEPONT/ for you, just before each
+ command that is in a transaction block, and rolls back to the
+ savepoint on error.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termvarnameON_ERROR_STOP/varname/term
listitem
para
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
}
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us 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
/varlistentry
varlistentry
+ indexterm
+primaryrollback/primary
+secondarypsql/secondary
+ /indexterm
+ termvarnameON_ERROR_ROLLBACK/varname/term
+ listitem
+ para
+ When literalon/, only in interactive mode, if a statement in
+ a transaction block generates an error, the error is ignored and
+ the transaction continues. When literaloff/ (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 commandSAVEPONT/ for you, just before
+ each command that is in a transaction block, and rolls back to
+ the savepoint on error.
+ /para
+ /listitem
+ /varlistentry
+
+ varlistentry
termvarnameON_ERROR_STOP/varname/term
listitem
para
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;
+
Re: [PATCHES] Shared dependency patch
Alvaro, did you update your patch to address the concerns mentioned below? --- Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: I have updated this patch to the current CVS HEAD. If somebody would be so kind to review this for applying at his earliest convenience, I'd appreciate it. It's not really ready to apply yet, because it's a bit schizophrenic about the users-vs-groups business. You are treating groups as a distinct object class in shdependUpdateAclInfo, but I don't see an OCLASS_GROUP ... isn't this going to fail as soon as someone tries to display a dependency on a group? But I'm not sure it's worth going to the trouble of fixing, seeing that we intend to remove groups soon in favor of roles. Also, you seem to have decided that we don't need dependency types for shared dependencies, which I think is a bad idea. In particular we should have at least DEPENDENCY_PIN, whereupon we can pin the original superuser, whereupon most of the initdb-time dependencies you are currently installing needn't exist at all. That's not just a space savings but a considerable time savings during searches. (Imagine how much slower the regular dependency stuff would be if we hadn't invented DEPENDENCY_PIN and therefore had to explicitly record all dependencies on every system object.) I'm also unconvinced that we would never find a use for DEPENDENCY_AUTO or DEPENDENCY_INTERNAL. I'm inclined to think it would be best to put this on the back burner until after the pg_role catalog changes are finished. Otherwise you'll have to do a fair amount of ultimately-useless work to make the group handling realistic. As far as OCLASS_AM goes, wouldn't it be simpler just to remove the owner column from pg_am? I can't imagine that there will ever be runtime commands to add and remove index access methods, much less such commands that are allowed to non-superusers. So the notion of an owner for an index AM seems like dead weight. (Compare the lack of an owner for languages.) I didn't have a problem with carrying a useless column, but adding infrastructure to support a useless column is a bit much. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend -- 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] Shared dependency patch
On Mon, Apr 25, 2005 at 10:14:05PM -0400, Bruce Momjian wrote: Alvaro, did you update your patch to address the concerns mentioned below? I'm on it right now. I wanted to finish the shared row locking patch first, and now that I'm waiting on someone to review it, I'll give some time to this. Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: I have updated this patch to the current CVS HEAD. If somebody would be so kind to review this for applying at his earliest convenience, I'd appreciate it. It's not really ready to apply yet, because it's a bit schizophrenic about the users-vs-groups business. You are treating groups as a distinct object class in shdependUpdateAclInfo, but I don't see an OCLASS_GROUP ... isn't this going to fail as soon as someone tries to display a dependency on a group? But I'm not sure it's worth going to the trouble of fixing, seeing that we intend to remove groups soon in favor of roles. Also, you seem to have decided that we don't need dependency types for shared dependencies, which I think is a bad idea. In particular we should have at least DEPENDENCY_PIN, whereupon we can pin the original superuser, whereupon most of the initdb-time dependencies you are currently installing needn't exist at all. That's not just a space savings but a considerable time savings during searches. (Imagine how much slower the regular dependency stuff would be if we hadn't invented DEPENDENCY_PIN and therefore had to explicitly record all dependencies on every system object.) I'm also unconvinced that we would never find a use for DEPENDENCY_AUTO or DEPENDENCY_INTERNAL. I'm inclined to think it would be best to put this on the back burner until after the pg_role catalog changes are finished. Otherwise you'll have to do a fair amount of ultimately-useless work to make the group handling realistic. As far as OCLASS_AM goes, wouldn't it be simpler just to remove the owner column from pg_am? I can't imagine that there will ever be runtime commands to add and remove index access methods, much less such commands that are allowed to non-superusers. So the notion of an owner for an index AM seems like dead weight. (Compare the lack of an owner for languages.) I didn't have a problem with carrying a useless column, but adding infrastructure to support a useless column is a bit much. -- Alvaro Herrera ([EMAIL PROTECTED]) No single strategy is always right (Unless the boss says so) (Larry Wall) ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
