Re: [PATCHES] Single-Transaction Utility options
Patch applied. Thanks. --- Simon Riggs wrote: On Fri, 2005-12-16 at 15:56 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Fri, 2005-12-16 at 13:59 -0500, Tom Lane wrote: Would -1 work, or just confuse people? That was my preference, I just thought it wouldn't be popular... So I'll happily change that. OK. While you're at it, I didn't like the long name either ;-). We do not use the abbrevation txn anywhere, so I think it's bad to introduce it here. I'd vote for spelling out --single-transaction, or maybe just --single. I believe you can abbreviate long switch names to any unique prefix, so there's not really any more typing here. Changes as discussed. singletransaction.patch attached. options: -1 or --single-transaction Functions work as described. Best Regards, Simon Riggs [ Attachment, skipping... ] ---(end of broadcast)--- TIP 9: In versions below 8.0, 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 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Single-Transaction Utility options
Patch applied. Thanks. --- Simon Riggs wrote: On Sat, 2005-12-17 at 20:03 +0100, Peter Eisentraut wrote: Simon Riggs wrote: Changes as discussed. singletransaction.patch attached. I meant to ask, why is this not the default or only behavior? Historically, it didn't work that way, so I hadn't thought to change that behaviour. We could I suppose... but I'm happy with just an option to do --single-transaction. Your patch does not contain a documentation update, and so the user has no information about why to use this option or not. I was waiting for tech approval of the patch before writing the docs. A doc patch is enclosed. Best Regards, Simon Riggs [ Attachment, skipping... ] ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster -- 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 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Single-Transaction Utility options
On Sat, 2005-12-17 at 20:03 +0100, Peter Eisentraut wrote: Simon Riggs wrote: Changes as discussed. singletransaction.patch attached. I meant to ask, why is this not the default or only behavior? Historically, it didn't work that way, so I hadn't thought to change that behaviour. We could I suppose... but I'm happy with just an option to do --single-transaction. Your patch does not contain a documentation update, and so the user has no information about why to use this option or not. I was waiting for tech approval of the patch before writing the docs. A doc patch is enclosed. Best Regards, Simon Riggs Index: doc/src/sgml/ref/pg_restore.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/pg_restore.sgml,v retrieving revision 1.56 diff -c -r1.56 pg_restore.sgml *** doc/src/sgml/ref/pg_restore.sgml 1 Nov 2005 21:09:50 - 1.56 --- doc/src/sgml/ref/pg_restore.sgml 18 Dec 2005 18:51:57 - *** *** 448,453 --- 448,466 /para /listitem /varlistentry + + varlistentry + termoption-1/option/term + termoption--single-transaction/option/term + listitem +para + Force the restore to execute as a single transaction. Either all + SQL statements complete successfully, or no changes are applied. This + option also forces --exit-on-error. +/para + /listitem + /varlistentry + /variablelist /para /refsect1 Index: doc/src/sgml/ref/psql-ref.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/ref/psql-ref.sgml,v retrieving revision 1.155 diff -c -r1.155 psql-ref.sgml *** doc/src/sgml/ref/psql-ref.sgml 9 Dec 2005 19:19:17 - 1.155 --- doc/src/sgml/ref/psql-ref.sgml 18 Dec 2005 18:52:00 - *** *** 463,468 --- 463,480 /listitem /varlistentry + varlistentry + termoption-1/option/term + termoption--single-transaction/option/term + listitem +para + When psql executes a script with the -f option, this additional option + will force the script to execute as a single transaction. Either all + SQL statements complete successfully, or no changes are applied. +/para + /listitem + /varlistentry + varlistentry termoption-?//term termoption--help//term ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Single-Transaction Utility options
Tom Lane wrote: I believe Peter's question was rhetorical: what he meant to point out is that the documentation needs to explain what is the reason for having this switch, ie, in what cases would you use it or not use it? Just saying what it does isn't really adequate docs. I once considered implementing this myself but found it infeasible for some reason I don't remember. Nevertheless I always thought that having an atomic restore ought to be a non-optional feature. Are there situations where one would not want to use it? (And if so, which one is the more normal case?) -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Single-Transaction Utility options
On Sun, 2005-12-18 at 14:04 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Sat, 2005-12-17 at 20:03 +0100, Peter Eisentraut wrote: I meant to ask, why is this not the default or only behavior? Historically, it didn't work that way, so I hadn't thought to change that behaviour. We could I suppose... but I'm happy with just an option to do --single-transaction. I believe Peter's question was rhetorical: what he meant to point out is that the documentation needs to explain what is the reason for having this switch, ie, in what cases would you use it or not use it? Just saying what it does isn't really adequate docs. Well, you know the reason: to allow pg_restore and psql take advantage of the COPY optimization I'm just about to submit. When that patch is accepted, I'll update these docs to explain that. But the two patches are separable, since the -1 still has value anyway. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 1: 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] Single-Transaction Utility options
On Sun, 2005-12-18 at 21:51 +0100, Peter Eisentraut wrote: Tom Lane wrote: I believe Peter's question was rhetorical: what he meant to point out is that the documentation needs to explain what is the reason for having this switch, ie, in what cases would you use it or not use it? Just saying what it does isn't really adequate docs. I once considered implementing this myself but found it infeasible for some reason I don't remember. Nevertheless I always thought that having an atomic restore ought to be a non-optional feature. Are there situations where one would not want to use it? (And if so, which one is the more normal case?) You're thinking is good. I guess if restores never failed, I'd be inclined to agree 100%, but I'm at about 80% right now. I'd say: if the patch is accepted technically, lets debate this point more widely on -hackers. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Single-Transaction Utility options
Peter Eisentraut [EMAIL PROTECTED] writes: I once considered implementing this myself but found it infeasible for some reason I don't remember. Nevertheless I always thought that having an atomic restore ought to be a non-optional feature. Are there situations where one would not want to use it? Absolutely. As a nontrivial example, I *very* often load dumps sent to me by other people which are full of GRANT/REVOKE commands referencing users that don't exist in my installation. Since, most of the time, I don't particularly care about the ownership/privileges of the tables involved, having to create those users would just be a PITA. More generally, the pg_dump output has always been designed around the assumption that failed commands are non-fatal. Look at all those unportable SET commands that we don't give you an option to omit. regards, tom lane ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Single-Transaction Utility options
On Fri, 2005-12-16 at 15:56 -0500, Tom Lane wrote:
Simon Riggs [EMAIL PROTECTED] writes:
On Fri, 2005-12-16 at 13:59 -0500, Tom Lane wrote:
Would -1 work, or just confuse people?
That was my preference, I just thought it wouldn't be popular...
So I'll happily change that.
OK. While you're at it, I didn't like the long name either ;-).
We do not use the abbrevation txn anywhere, so I think it's bad to
introduce it here. I'd vote for spelling out --single-transaction,
or maybe just --single. I believe you can abbreviate long switch names
to any unique prefix, so there's not really any more typing here.
Changes as discussed. singletransaction.patch attached.
options:
-1 or --single-transaction
Functions work as described.
Best Regards, Simon Riggs
Index: src/bin/pg_dump/pg_backup.h
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup.h,v
retrieving revision 1.37
diff -c -r1.37 pg_backup.h
*** src/bin/pg_dump/pg_backup.h 15 Oct 2005 02:49:38 - 1.37
--- src/bin/pg_dump/pg_backup.h 17 Dec 2005 16:51:25 -
***
*** 115,120
--- 115,122
int suppressDumpWarnings; /* Suppress output of WARNING entries
* to stderr */
+ boolsingle_txn;
+
} RestoreOptions;
/*
Index: src/bin/pg_dump/pg_backup_archiver.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_backup_archiver.c,v
retrieving revision 1.118
diff -c -r1.118 pg_backup_archiver.c
*** src/bin/pg_dump/pg_backup_archiver.c 22 Nov 2005 18:17:28 - 1.118
--- src/bin/pg_dump/pg_backup_archiver.c 17 Dec 2005 16:51:27 -
***
*** 217,222
--- 217,225
AH-stage = STAGE_PROCESSING;
+ if (ropt-single_txn)
+ ahprintf(AH, BEGIN;\n\n);
+
/*
* Drop the items at the start, in reverse order
*/
***
*** 365,370
--- 368,376
}
}
+ if (ropt-single_txn)
+ ahprintf(AH, COMMIT;\n\n);
+
if (AH-public.verbose)
dumpTimestamp(AH, Completed on, time(NULL));
Index: src/bin/pg_dump/pg_restore.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/pg_dump/pg_restore.c,v
retrieving revision 1.73
diff -c -r1.73 pg_restore.c
*** src/bin/pg_dump/pg_restore.c 15 Oct 2005 02:49:39 - 1.73
--- src/bin/pg_dump/pg_restore.c 17 Dec 2005 16:51:27 -
***
*** 111,116
--- 111,117
{use-list, 1, NULL, 'L'},
{username, 1, NULL, 'U'},
{verbose, 0, NULL, 'v'},
+ {single-transaction, 0, NULL, '1'},
/*
* the following options don't have an equivalent short option letter,
***
*** 142,148
}
}
! while ((c = getopt_long(argc, argv, acCd:ef:F:h:iI:lL:n:Op:P:RsS:t:T:uU:vWxX:,
cmdopts, NULL)) != -1)
{
switch (c)
--- 143,149
}
}
! while ((c = getopt_long(argc, argv, acCd:ef:F:h:iI:lL:n:Op:P:RsS:t:T:uU:vWxX:1,
cmdopts, NULL)) != -1)
{
switch (c)
***
*** 185,193
--- 186,200
opts-tocFile = strdup(optarg);
break;
+ case 'n': /* Dump data for this schema only */
+ opts-selTypes = 1;
+ opts-schemaNames = strdup(optarg);
+ break;
+
case 'O':
opts-noOwner = 1;
break;
+
case 'p':
if (strlen(optarg) != 0)
opts-pgport = strdup(optarg);
***
*** 223,233
opts-tableNames = strdup(optarg);
break;
- case 'n': /* Dump data for this schema only */
- opts-selTypes = 1;
- opts-schemaNames = strdup(optarg);
- break;
-
case 'u':
opts-requirePassword = true;
opts-username = simple_prompt(User name: , 100, true);
--- 230,235
***
*** 268,273
--- 270,280
case 0:
break;
+ case '1': /* Restore data in a single transaction */
+ opts-single_txn = true;
+ opts-exit_on_error = true;
+ break;
+
default:
fprintf(stderr, _(Try \%s --help\ for more information.\n), progname);
exit(1);
***
*** 394,399
--- 401,407
printf(_( -X use-set-session-authorization, --use-set-session-authorization\n
use SESSION AUTHORIZATION commands instead of\n
OWNER TO commands\n));
+ printf(_( -1, --single-transaction restore as a single transaction\n));
printf(_(\nConnection options:\n));
printf(_( -h, --host=HOSTNAME database server host or socket directory\n));
Index: src/bin/psql/command.c
===
RCS file: /projects/cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.155
diff -c -r1.155 command.c
*** src/bin/psql/command.c 8 Dec 2005 21:18:22 - 1.155
--- src/bin/psql/command.c 17 Dec 2005 16:51:29 -
***
*** 523,529
else
{
Re: [PATCHES] Single-Transaction Utility options
Simon Riggs wrote: Changes as discussed. singletransaction.patch attached. I meant to ask, why is this not the default or only behavior? Your patch does not contain a documentation update, and so the user has no information about why to use this option or not. -- Peter Eisentraut http://developer.postgresql.org/~petere/ ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Single-Transaction Utility options
Simon Riggs [EMAIL PROTECTED] writes: The following patches add a -N option to psql and pgrestore. -N seems an entirely random name for the switch ... can't we do better? I see that -t, -T, -s, -S, -x and -X are all taken, which lets out the obvious choices ... but I'd rather have no single-letter abbreviation at all than one that has zero relationship to the function of the switch. Would -1 work, or just confuse people? Also, I don't actually see any point to this in psql, as you can always do begin; \i file end; It's only pg_restore that you really need it for. Dropping the psql part of the patch might give us a little more maneuvering room as far as the switch name goes. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] Single-Transaction Utility options
Simon Riggs wrote: The following patches add a -N option to psql and pgrestore. This option adds a BEGIN at the start and a COMMIT at the end of all commands, causing all statements to be executed as a single transaction. Why use it around the whole file and not only around that particular table's operations? Also why force it to activate the abort-on-error mode? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] Single-Transaction Utility options
On Fri, 2005-12-16 at 13:59 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: The following patches add a -N option to psql and pgrestore. -N seems an entirely random name for the switch ... can't we do better? I see that -t, -T, -s, -S, -x and -X are all taken, which lets out the obvious choices ... but I'd rather have no single-letter abbreviation at all than one that has zero relationship to the function of the switch. Almost. Stands for traNsaction Would -1 work, or just confuse people? That was my preference, I just thought it wouldn't be popular... So I'll happily change that. Also, I don't actually see any point to this in psql, as you can always do begin; \i file end; It's only pg_restore that you really need it for. Dropping the psql part of the patch might give us a little more maneuvering room as far as the switch name goes. Of course, you're right... and that is all the patch does in fact. It seemed easier to do -1 than the SQL above, especially when doing it from a single command line. But the main reason was to make all the utilities work the same, if possible. Having different options on each utility makes it easier to make mistakes, so I'd rather have the same option everywhere that it could apply. I couldn't see any reason not to do it, either. Best Regards, Simon Riggs ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Single-Transaction Utility options
On Fri, 2005-12-16 at 16:04 -0300, Alvaro Herrera wrote: Simon Riggs wrote: The following patches add a -N option to psql and pgrestore. This option adds a BEGIN at the start and a COMMIT at the end of all commands, causing all statements to be executed as a single transaction. Why use it around the whole file and not only around that particular table's operations? You could. That just behaves slightly differently. Maybe we should have options for both? Also why force it to activate the abort-on-error mode? For what reason would you want it to keep running? Best Regards, Simon Riggs ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [PATCHES] Single-Transaction Utility options
Simon Riggs [EMAIL PROTECTED] writes: On Fri, 2005-12-16 at 13:59 -0500, Tom Lane wrote: Would -1 work, or just confuse people? That was my preference, I just thought it wouldn't be popular... So I'll happily change that. OK. While you're at it, I didn't like the long name either ;-). We do not use the abbrevation txn anywhere, so I think it's bad to introduce it here. I'd vote for spelling out --single-transaction, or maybe just --single. I believe you can abbreviate long switch names to any unique prefix, so there's not really any more typing here. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] Single-Transaction Utility options
Simon Riggs wrote: On Fri, 2005-12-16 at 16:04 -0300, Alvaro Herrera wrote: Simon Riggs wrote: The following patches add a -N option to psql and pgrestore. This option adds a BEGIN at the start and a COMMIT at the end of all commands, causing all statements to be executed as a single transaction. Why use it around the whole file and not only around that particular table's operations? You could. That just behaves slightly differently. Maybe we should have options for both? Are they different enough to warrant having two switches? IIRC the point was to have the COPY in the same transaction as the CREATE TABLE, right? In what way is it worse to have each table in its own transaction? Also why force it to activate the abort-on-error mode? For what reason would you want it to keep running? To restore the rest of the tables in the dump I presume ... I mean, the behaviors are orthogonal really (_if_ you take the stance that this should be used on a per table basis rather than a file basis, that is. Because if you abort the transaction then clearly there's no point in keep running, as everything will be rejected by the server anyway.) -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] Single-Transaction Utility options
Simon Riggs [EMAIL PROTECTED] writes: Why use it around the whole file and not only around that particular table's operations? You could. That just behaves slightly differently. pg_dump does not always produce all the commands affecting a single table together, so I don't think you can actually get the desired results --- certainly it would be a nontrivial amount of work to get any useful behavior like that. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [PATCHES] Single-Transaction Utility options
Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: Why use it around the whole file and not only around that particular table's operations? You could. That just behaves slightly differently. pg_dump does not always produce all the commands affecting a single table together, so I don't think you can actually get the desired results --- certainly it would be a nontrivial amount of work to get any useful behavior like that. Ah, quite true. I withdraw my comments then. -- Alvaro Herrerahttp://www.CommandPrompt.com/ PostgreSQL Replication, Consulting, Custom Development, 24x7 support ---(end of broadcast)--- TIP 1: 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
