Thread: psql: rollback only last query on error
I am sorry for kind of reposting this, but I have not got any response to my patch sent for comment to hackers (Subject: Rollback on error): http://archives.postgresql.org/pgsql-hackers/2004-09/msg00576.php I just want to find out, if I should try to solve the issues with this patch (and add regression tests, docs, etc.) now or leave it till after release of version 8.0. Reading responses on my intial post I think the feature is not unwelcome, at least if implemented well, so that it will not do anything unexpected. I understand this is beta now, and this is a new feature in psql. Nevertheless I believe it is good to include the feature now, because a) It increases the testing of savepoints since more people will use savepoints (all who activate the mode in psql that my patch provides). b) Given I finish the open issues, the patch has no backward compatibility issues with scripts etc., even if you put \set IMPLICIT_SAVEPOINTS 'on' in .psqlrc. For IMPLICIT_SAVEPOINTS 'off' there is no change at all. c) The code change is rather local and does not add much complexity. d) Some people trying 8.0 for the first time might find the current behavior of psql odd if they are used to oracle, mssql etc. At least at the interactive level, the patch would give them the option to have their accustomed way of handling e.g. typos. Thank you for your time and thank you for any response! Best Regards, Michael Paesold P.S. attached is a version of the patch with more/better comments.
Attachment
On Tue, Sep 21, 2004 at 02:30:17PM +0200, Michael Paesold wrote: > I am sorry for kind of reposting this, but I have not got any response to my > patch sent for comment to hackers (Subject: Rollback on error): > http://archives.postgresql.org/pgsql-hackers/2004-09/msg00576.php > > I just want to find out, if I should try to solve the issues with this patch > (and add regression tests, docs, etc.) now or leave it till after release of > version 8.0. One potential problem I see with the patch is that it opens lots of savepoints but does not release any. In this mode, given autocommit off (and even without that), there's potential for lots and lots of savepoints. Not sure how to fix that given that you shouldn't release user-specified savepoints ... Also, you need to do something with \i. I think the global variable will be a less intrusive approach, at least while we are in beta. When 8.1 development starts, you can submit a patch to clean up. -- Alvaro Herrera (<alvherre[a]dcc.uchile.cl>) "Cada quien es cada cual y baja las escaleras como quiere" (JMSerrat)
Alvaro Herrera wrote: > On Tue, Sep 21, 2004 at 02:30:17PM +0200, Michael Paesold wrote: > > http://archives.postgresql.org/pgsql-hackers/2004-09/msg00576.php > > One potential problem I see with the patch is that it opens lots of > savepoints but does not release any. In this mode, given autocommit off > (and even without that), there's potential for lots and lots of > savepoints. Not sure how to fix that given that you shouldn't release > user-specified savepoints ... Well, given that EXCEPTION blocks in Pl/pgSQL and the like also never release savepoints I think there will be more issues with savepoints in that area. Nevertheless, I have added a note to the documentation about the feature that warns of the possible consequences of those many savepoints. Also Tom Lane mentioned in another thread that even releasing savepoints does not really help because of the other resources used, e.g. for trx id locks, that can't be released at all till COMMIT. > Also, you need to do something with \i. I think the global variable > will be a less intrusive approach, at least while we are in beta. When > 8.1 development starts, you can submit a patch to clean up. I have added a field to the pset settings to do that. Everything works now as I expected it to do. Using \i and psql <file.sql the feature is disabled, otherwise it's controlled by the variable IMPLICIT_SAVEPOINTS and the transaction state (of course not useful when AUTOCOMMIT is on and no BEGIN issued). A final thing is the name for this option... I don't really like IMPLICIT_SAVEPOINTS. Other ideas were AUTO_SAVEPOINT, STATEMENT_SAVEPOINTS or STATEMENT_ROLLBACK. They aren't really good either... Patch is attached for review. Please decide if this can be included with PostgreSQL 8.0 or not. If so, I would like to add a regression test for the feature. In that case, could you please tell me where to put that one? Best Regards, Michael
Attachment
"Michael Paesold" <mpaesold@gmx.at> writes:
> Alvaro Herrera wrote:
>> One potential problem I see with the patch is that it opens lots of
>> savepoints but does not release any.
> Well, given that EXCEPTION blocks in Pl/pgSQL and the like also never
> release savepoints
That statement is flat wrong.
regards, tom lane
> "Michael Paesold" <mpaesold@gmx.at> writes: > > Alvaro Herrera wrote: > >> One potential problem I see with the patch is that it opens lots of > >> savepoints but does not release any. > > > Well, given that EXCEPTION blocks in Pl/pgSQL and the like also never > > release savepoints > > That statement is flat wrong. I have to say that I am sorry that I misunderstood that. Regards, Michael Paesold -- +++ GMX DSL Premiumtarife 3 Monate gratis* + WLAN-Router 0,- EUR* +++ Clevere DSL-Nutzer wechseln jetzt zu GMX: http://www.gmx.net/de/go/dsl