Re: [PATCHES] Single-Transaction Utility options

2006-02-11 Thread Bruce Momjian

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

2006-02-11 Thread Bruce Momjian

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

2005-12-18 Thread Simon Riggs
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

2005-12-18 Thread Peter Eisentraut
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

2005-12-18 Thread Simon Riggs
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

2005-12-18 Thread Simon Riggs
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

2005-12-18 Thread Tom Lane
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

2005-12-17 Thread Simon Riggs
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

2005-12-17 Thread Peter Eisentraut
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

2005-12-16 Thread Tom Lane
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

2005-12-16 Thread Alvaro Herrera
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

2005-12-16 Thread Jaime Casanova
 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


maybe it could be handy if you do this to migrate from one version to another...
pg_dumpall base | psql

--
regards,
Jaime Casanova
(DBA: DataBase Aniquilator ;)

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [PATCHES] Single-Transaction Utility options

2005-12-16 Thread Simon Riggs
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

2005-12-16 Thread Simon Riggs
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

2005-12-16 Thread Tom Lane
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

2005-12-16 Thread Alvaro Herrera
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

2005-12-16 Thread Tom Lane
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

2005-12-16 Thread Alvaro Herrera
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