Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I'm finding it hard to visualize a non-interactive script making any good use of such a setting. Without a way to test whether you got an error or not, it would amount to an ignore errors within transactions mode, which seems a pretty bad idea. Can you show a plausible use-case for such a thing? I could have used this yesterday. I was populating a test table with a primary key on two columns and needed to add a bunch of random rows. I generated a 10_000 line file of one insert statement each. Rather than worrying about collisions, I could simply \rollbackonerror (or whatever we're calling it today :) and silently discard the handful that happen to violate the primary key constraint and let the rest insert. - -- Greg Sabino Mullane [EMAIL PROTECTED] PGP Key: 0x14964AC8 200504270754 http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8 -BEGIN PGP SIGNATURE- iD8DBQFCb33NvJuQZxSWSsgRAvdfAJwMqysSpVI2BDh9wENT2jxMZnspagCfRlHJ 9ElhNydsz2FsCc1JgI5R+gU= =h9AW -END PGP SIGNATURE- ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
On 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: [PATCHES] Cleaning up unreferenced table files
On Tue, 26 Apr 2005, Alvaro Herrera wrote:
You forgot the attachment?
Damn. It happens time after time...
- HeikkiIndex: doc/src/sgml/maintenance.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/maintenance.sgml,v
retrieving revision 1.41
diff -c -r1.41 maintenance.sgml
*** doc/src/sgml/maintenance.sgml 20 Feb 2005 02:21:26 - 1.41
--- doc/src/sgml/maintenance.sgml 26 Apr 2005 22:02:22 -
***
*** 474,479
--- 474,496
/para
/sect1
+ sect1 id=cleanup-after-crash
+ titleCleanup after crash/title
+
+ indexterm zone=cleanup-after-crash
+primarystale file/primary
+ /indexterm
+
+ para
+productnamePostgreSQL/productname recovers automatically after crash
+using the write-ahead log (see xref linkend=wal) and no manual
+operations are normally needed. However, if there was a transaction
running
+when the crash occured that created or dropped a relation, the
+transaction might have left a stale file in the data directory. If this
+happens, you will get a notice in the log file stating which files can be
+deleted.
+ /para
+ /sect1
sect1 id=logfile-maintenance
titleLog File Maintenance/title
Index: src/backend/access/transam/xlog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.188
diff -c -r1.188 xlog.c
*** src/backend/access/transam/xlog.c 23 Apr 2005 18:49:54 - 1.188
--- src/backend/access/transam/xlog.c 26 Apr 2005 22:02:22 -
***
*** 42,47
--- 42,48
#include utils/builtins.h
#include utils/guc.h
#include utils/relcache.h
+ #include utils/cleanup.h
/*
***
*** 4520,4525
--- 4521,4528
CreateCheckPoint(true, true);
+ CleanupStaleRelFiles();
+
/*
* Close down recovery environment
*/
Index: src/backend/catalog/catalog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/catalog/catalog.c,v
retrieving revision 1.59
diff -c -r1.59 catalog.c
*** src/backend/catalog/catalog.c 14 Apr 2005 20:03:23 - 1.59
--- src/backend/catalog/catalog.c 26 Apr 2005 22:02:22 -
***
*** 106,111
--- 106,144
return path;
}
+ /*
+ * GetTablespacePath - construct path to a tablespace symbolic link
+ *
+ * Result is a palloc'd string.
+ *
+ * XXX this must agree with relpath and GetDatabasePath!
+ */
+ char *
+ GetTablespacePath(Oid spcNode)
+ {
+ int pathlen;
+ char *path;
+
+ Assert(spcNode != GLOBALTABLESPACE_OID);
+
+ if (spcNode == DEFAULTTABLESPACE_OID)
+ {
+ /* The default tablespace is {datadir}/base */
+ pathlen = strlen(DataDir) + 5 + 1;
+ path = (char *) palloc(pathlen);
+ snprintf(path, pathlen, %s/base,
+DataDir);
+ }
+ else
+ {
+ /* All other tablespaces have symlinks in pg_tblspc */
+ pathlen = strlen(DataDir) + 11 + OIDCHARS + 1;
+ path = (char *) palloc(pathlen);
+ snprintf(path, pathlen, %s/pg_tblspc/%u,
+DataDir, spcNode);
+ }
+ return path;
+ }
/*
* IsSystemRelation
Index: src/backend/commands/tablespace.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/tablespace.c,v
retrieving revision 1.17
diff -c -r1.17 tablespace.c
*** src/backend/commands/tablespace.c 14 Apr 2005 20:03:24 - 1.17
--- src/backend/commands/tablespace.c 26 Apr 2005 22:02:23 -
***
*** 341,348
/*
* All seems well, create the symlink
*/
! linkloc = (char *) palloc(strlen(DataDir) + 11 + 10 + 1);
! sprintf(linkloc, %s/pg_tblspc/%u, DataDir, tablespaceoid);
if (symlink(location, linkloc) 0)
ereport(ERROR,
--- 341,347
/*
* All seems well, create the symlink
*/
! linkloc = GetTablespacePath(tablespaceoid);
if (symlink(location, linkloc) 0)
ereport(ERROR,
***
*** 495,502
char *subfile;
struct stat st;
! location = (char *) palloc(strlen(DataDir) + 11 + 10 + 1);
! sprintf(location, %s/pg_tblspc/%u, DataDir, tablespaceoid);
/*
* Check if the tablespace still contains any files. We try to rmdir
--- 494,500
char *subfile;
struct stat st;
! location = GetTablespacePath(tablespaceoid);
/*
* Check if the tablespace still contains any files. We try to rmdir
Re: [PATCHES] Cleaning up unreferenced table files
On Tue, 26 Apr 2005, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: I feel that crashes that leaves behind stale files are rare. Indeed, and getting more so all the time ... How so? Have changes been made in those parts of the code? which makes me question the value of doing anything about this at all. What bothers me is that we currently have no means of knowing if it happens and how often it happens, so we're just guessing that it's rare. How rare? We don't know. If nobody ever runs into this issue in production, and this whole exercise turns out to be completely unnecessary, at least we'll know. That alone makes me feel better. The only drawback of the patch that I can see is the performance impact on recovery. And I think the time it takes to scan the data directories isn't much compared to WAL replay. - Heikki ---(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] Cleaning up unreferenced table files
Heikki Linnakangas [EMAIL PROTECTED] writes: On Tue, 26 Apr 2005, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: I feel that crashes that leaves behind stale files are rare. Indeed, and getting more so all the time ... How so? Have changes been made in those parts of the code? No, just that the overall reliability of Postgres keeps improving. At the time that TODO entry was created, I don't think we even had the ability to roll back table creates/drops properly, so there were scenarios in which unreferenced files could be left behind without even assuming any software error. And the prevalence of backend crashes was way higher than it is now, too. So I think a good argument could be made that the TODO item isn't nearly as important as it was at the time. If nobody ever runs into this issue in production, and this whole exercise turns out to be completely unnecessary, at least we'll know. That alone makes me feel better. We will know no such thing, unless the patch is made to announce the problem so intrusively that people are certain to see it *and report it to us*. Which I don't think will be acceptable. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Cleaning up unreferenced table files
On Wed, 27 Apr 2005, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: On Tue, 26 Apr 2005, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: I feel that crashes that leaves behind stale files are rare. Indeed, and getting more so all the time ... How so? Have changes been made in those parts of the code? No, just that the overall reliability of Postgres keeps improving. At the time that TODO entry was created, I don't think we even had the ability to roll back table creates/drops properly, so there were scenarios in which unreferenced files could be left behind without even assuming any software error. And the prevalence of backend crashes was way higher than it is now, too. So I think a good argument could be made that the TODO item isn't nearly as important as it was at the time. *shrug*. I won't push any harder if you feel strongly against the patch. Should we just remove the TODO item? And/or document the issue? - Heikki ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: 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: [PATCHES] Cleaning up unreferenced table files
Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: On Tue, 26 Apr 2005, Tom Lane wrote: Heikki Linnakangas [EMAIL PROTECTED] writes: I feel that crashes that leaves behind stale files are rare. Indeed, and getting more so all the time ... How so? Have changes been made in those parts of the code? No, just that the overall reliability of Postgres keeps improving. At the time that TODO entry was created, I don't think we even had the ability to roll back table creates/drops properly, so there were scenarios in which unreferenced files could be left behind without even assuming any software error. And the prevalence of backend crashes was way higher than it is now, too. So I think a good argument could be made that the TODO item isn't nearly as important as it was at the time. If nobody ever runs into this issue in production, and this whole exercise turns out to be completely unnecessary, at least we'll know. That alone makes me feel better. We will know no such thing, unless the patch is made to announce the problem so intrusively that people are certain to see it *and report it to us*. Which I don't think will be acceptable. Well, if putting it in the server logs isn't enough, I don't know what is. I think we do need the patch, at least to find out if there is an issue we don't know about. I don't see the hard in it. Heikki, would you time startup and tell us what percentage of time is taken by the routines? Or I can do it. -- 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] Cleaning up unreferenced table files
Bruce Momjian pgman@candle.pha.pa.us writes: I think we do need the patch, at least to find out if there is an issue we don't know about. My point is that we won't find out anything, because we will have no idea if people are noticing the log entries at all, much less telling us about 'em. Of course if they do tell us then we'd know something, but what I am expecting is a vast silence. We will not be able to tell the difference between no problem and very infrequent problem any better than we can now. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Bruce Momjian pgman@candle.pha.pa.us writes: Tom Lane wrote: Well, that's just a matter of choosing good (ie short) names for the backslash commands. I was trying to be clear rather than proposing names I would actually want to use ;-). Any suggestions? Well, if we allowed ON_ERROR_ROLLBACK to work in non-interactive sessions we could just do: \set ON_ERROR_ROLLBACK on DROP TABLE foo; \set ON_ERROR_ROLLBACK off That isn't the same thing at all. The syntax I was proposing allows the script writer to define a savepoint covering multiple statements, whereas the above does not. Maybe what we really need is a rollback or release savepoint operation, defined as ROLLBACK TO foo if in error state, RELEASE foo if not in error state. This is essentially the thing that a script writer has to have and can't do for himself due to the lack of any conditional ability in psql scripts. We could imagine implementing that either as a SQL command or as a psql backslash command ... I don't have a strong feeling either way. regards, tom lane ---(end of broadcast)--- TIP 8: explain analyze is your friend
Re: [HACKERS] [PATCHES] Continue transactions after errors in psql
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us 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: [PATCHES] Cleaning up unreferenced table files
Tom Lane wrote: Bruce Momjian pgman@candle.pha.pa.us writes: I think we do need the patch, at least to find out if there is an issue we don't know about. My point is that we won't find out anything, because we will have no idea if people are noticing the log entries at all, much less telling us about 'em. Of course if they do tell us then we'd know something, but what I am expecting is a vast silence. We will not be able to tell the difference between no problem and very infrequent problem any better than we can now. I think people do look at their logs. We are certainly better off finding it than having no reporting at all. -- 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: [HACKERS] [PATCHES] Continue transactions after errors in psql
pgman wrote:
Tom Lane wrote:
Bruce Momjian pgman@candle.pha.pa.us 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
/varlistentry
varlistentry
+ indexterm
+primaryrollback/primary
+secondarypsql/secondary
+ /indexterm
+ termvarnameON_ERROR_ROLLBACK/varname/term
+ listitem
+ para
+ When literalon/, if a statement in a transaction block
+ generates an error, the error is ignored and the transaction
+ continues. When literalinteractive/, such errors are only
+ ignored in interactive sessions, and not when reading script
+ files. 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 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)
+
