Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
On Sun, 2006-11-05 at 15:02 +, Simon Riggs wrote:
Code comments now discuss relative paths also.
Patch containing just the minor cleanup of docs and code comments.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
Index: doc/src/sgml/backup.sgml
===
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/backup.sgml,v
retrieving revision 2.93
diff -c -r2.93 backup.sgml
*** doc/src/sgml/backup.sgml 4 Nov 2006 18:20:27 - 2.93
--- doc/src/sgml/backup.sgml 6 Nov 2006 08:21:22 -
***
*** 599,605
In writing your archive command, you should assume that the file names to
be archived may be up to 64 characters long and may contain any
combination of ASCII letters, digits, and dots. It is not necessary to
! remember the original full path (literal%p/) but it is necessary to
remember the file name (literal%f/).
/para
--- 599,605
In writing your archive command, you should assume that the file names to
be archived may be up to 64 characters long and may contain any
combination of ASCII letters, digits, and dots. It is not necessary to
! remember the original relative path (literal%p/) but it is necessary to
remember the file name (literal%f/).
/para
Index: src/backend/access/transam/xlog.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.252
diff -c -r1.252 xlog.c
*** src/backend/access/transam/xlog.c 18 Oct 2006 22:44:11 - 1.252
--- src/backend/access/transam/xlog.c 6 Nov 2006 08:21:31 -
***
*** 2417,2423
switch (sp[1])
{
case 'p':
! /* %p: full path of target file */
sp++;
StrNCpy(dp, xlogpath, endp - dp);
make_native_path(dp);
--- 2417,2423
switch (sp[1])
{
case 'p':
! /* %p: relative path of target file */
sp++;
StrNCpy(dp, xlogpath, endp - dp);
make_native_path(dp);
Index: src/backend/postmaster/pgarch.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/postmaster/pgarch.c,v
retrieving revision 1.25
diff -c -r1.25 pgarch.c
*** src/backend/postmaster/pgarch.c 7 Aug 2006 17:41:42 - 1.25
--- src/backend/postmaster/pgarch.c 6 Nov 2006 08:21:33 -
***
*** 417,423
switch (sp[1])
{
case 'p':
! /* %p: full path of source file */
sp++;
StrNCpy(dp, pathname, endp - dp);
make_native_path(dp);
--- 417,423
switch (sp[1])
{
case 'p':
! /* %p: relative path of source file */
sp++;
StrNCpy(dp, pathname, endp - dp);
make_native_path(dp);
---(end of broadcast)---
TIP 4: Have you searched our list archives?
http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
On Sat, 2006-11-04 at 13:29 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: Since 8.1 has done this all along and no one's actually complained about it, I guess no one is using scripts that do cd. I'm inclined to go with Bernd's suggestion to change the docs to match the code, but does anyone have a contrary opinion? +1 Doc bug for 8.2, feature request for 8.3, unless Windows bites. Looking back in the archives, I note that one of the arguments for making the server use relative paths everywhere was so that it'd be robust against things like DBAs moving directories that contain live postmasters. If we provide a %P option, or otherwise encourage people to write scripts that depend on the absolute path of $PGDATA, we'd lose some of this robustness. So that might be an argument for leaving the code as-is indefinitely ... not a very strong argument maybe, but it's more than just we're-too-lazy-to-add-%P. Anyway, I've corrected the documentation in HEAD and 8.1. I think I can fulfil Bernd, Florian and Martijn's wishes by supplying an additional substitutable parameter %d which is replaced by the DataDir. This allows people to use an absolute directory if they wish, allows us to continue with the functionality of %p as-is and all without a possible confusion between %p and %P. It also allows %d to be used as an identifier which might be used to locate the appropriate archive for those with multiple servers without editing the archive_command for each of those servers. So using %d/%p will give you the absolute path for forward-slashers. Works for archive and recovery. Patch included, code and docs. Code comments now discuss relative paths also. Comments? -- Simon Riggs EnterpriseDB http://www.enterprisedb.com Index: doc/src/sgml/backup.sgml === RCS file: /projects/cvsroot/pgsql/doc/src/sgml/backup.sgml,v retrieving revision 2.93 diff -c -r2.93 backup.sgml *** doc/src/sgml/backup.sgml 4 Nov 2006 18:20:27 - 2.93 --- doc/src/sgml/backup.sgml 5 Nov 2006 14:44:34 - *** *** 521,527 any literal%p/ is replaced by the path name of the file to archive, while any literal%f/ is replaced by the file name only. (The path name is relative to the working directory of the server, ! i.e., the cluster's data directory.) Write literal%%/ if you need to embed an actual literal%/ character in the command. The simplest useful command is something like --- 521,528 any literal%p/ is replaced by the path name of the file to archive, while any literal%f/ is replaced by the file name only. (The path name is relative to the working directory of the server, ! i.e., the cluster's data directory, which can also be specified ! using %d, if required) Write literal%%/ if you need to embed an actual literal%/ character in the command. The simplest useful command is something like *** *** 599,605 In writing your archive command, you should assume that the file names to be archived may be up to 64 characters long and may contain any combination of ASCII letters, digits, and dots. It is not necessary to ! remember the original full path (literal%p/) but it is necessary to remember the file name (literal%f/). /para --- 600,606 In writing your archive command, you should assume that the file names to be archived may be up to 64 characters long and may contain any combination of ASCII letters, digits, and dots. It is not necessary to ! remember the original relative path (literal%p/) but it is necessary to remember the file name (literal%f/). /para *** *** 919,925 replaced by the name of the desired log file, and literal%p/, which is replaced by the path name to copy the log file to. (The path name is relative to the working directory of the server, ! i.e., the cluster's data directory.) Write literal%%/ if you need to embed an actual literal%/ character in the command. The simplest useful command is something like --- 920,927 replaced by the name of the desired log file, and literal%p/, which is replaced by the path name to copy the log file to. (The path name is relative to the working directory of the server, ! i.e., the cluster's data directory, which can also be specified ! using %d, if required.) Write literal%%/ if you need to embed an actual literal%/ character in the command. The simplest useful command is something like *** *** 1010,1016 and any literal%p/ is replaced by the path name to copy it to on the server. (The path name is relative to the working directory of the server, ! i.e.,
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
Simon Riggs [EMAIL PROTECTED] writes: On Sat, 2006-11-04 at 13:29 -0500, Tom Lane wrote: Looking back in the archives, I note that one of the arguments for making the server use relative paths everywhere was so that it'd be robust against things like DBAs moving directories that contain live postmasters. If we provide a %P option, or otherwise encourage people to write scripts that depend on the absolute path of $PGDATA, we'd lose some of this robustness. I think I can fulfil Bernd, Florian and Martijn's wishes by supplying an additional substitutable parameter %d which is replaced by the DataDir. This fails to respond to the concern that DataDir might be out-of-date. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
On Sun, 2006-11-05 at 11:10 -0500, Tom Lane wrote: Simon Riggs [EMAIL PROTECTED] writes: On Sat, 2006-11-04 at 13:29 -0500, Tom Lane wrote: Looking back in the archives, I note that one of the arguments for making the server use relative paths everywhere was so that it'd be robust against things like DBAs moving directories that contain live postmasters. If we provide a %P option, or otherwise encourage people to write scripts that depend on the absolute path of $PGDATA, we'd lose some of this robustness. I think I can fulfil Bernd, Florian and Martijn's wishes by supplying an additional substitutable parameter %d which is replaced by the DataDir. This fails to respond to the concern that DataDir might be out-of-date. I'm not suggesting that the option is necessary, but I am suggesting offering it to those who consider it useful. Let's allow it, but document the concern about its use in certain circumstances. I'm pretty sure most people don't move live postmasters very frequently, plus it isn't clear to me why we should support the people that want that to do that, yet not the people who want the absolute-path option. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
Simon Riggs [EMAIL PROTECTED] writes: I'm pretty sure most people don't move live postmasters very frequently, plus it isn't clear to me why we should support the people that want that to do that, yet not the people who want the absolute-path option. As already discussed upthread, anyone who wants the path can get it from `pwd` or local equivalent --- and that mechanism is robust (as long as the directory move doesn't happen while any particular instance of the script is running). I don't see why we should go out of our way to provide a bad substitute for pwd. BTW, I note that some post-startup uses of DataDir have crept back in, in places like utils/adt/dbsize.c. I'll be sure to clean those up before release... regards, tom lane ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
On Sun, 2006-11-05 at 11:49 -0500, Tom Lane wrote: I don't see why we should go out of our way to provide a bad substitute for pwd. That argument is conclusive. Agreed. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
Simon Riggs [EMAIL PROTECTED] writes: On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: Since 8.1 has done this all along and no one's actually complained about it, I guess no one is using scripts that do cd. I'm inclined to go with Bernd's suggestion to change the docs to match the code, but does anyone have a contrary opinion? +1 Doc bug for 8.2, feature request for 8.3, unless Windows bites. Looking back in the archives, I note that one of the arguments for making the server use relative paths everywhere was so that it'd be robust against things like DBAs moving directories that contain live postmasters. If we provide a %P option, or otherwise encourage people to write scripts that depend on the absolute path of $PGDATA, we'd lose some of this robustness. So that might be an argument for leaving the code as-is indefinitely ... not a very strong argument maybe, but it's more than just we're-too-lazy-to-add-%P. Anyway, I've corrected the documentation in HEAD and 8.1. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: Since 8.1 has done this all along and no one's actually complained about it, I guess no one is using scripts that do cd. I'm inclined to go with Bernd's suggestion to change the docs to match the code, but does anyone have a contrary opinion? Arguably you could give people a choice, say %P for the absolute path and %p for the relative one. In Unix you can easily prepend $PWD to the string, but I don't know how easy that is in Windows. Have a nice day, -- Martijn van Oosterhout kleptog@svana.org http://svana.org/kleptog/ From each according to his ability. To each according to his ability to litigate. signature.asc Description: Digital signature
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
Tom Lane wrote: Bernd Helmle [EMAIL PROTECTED] writes: Since 8.1 has done this all along and no one's actually complained about it, I guess no one is using scripts that do cd. I'm inclined to go with Bernd's suggestion to change the docs to match the code, but does anyone have a contrary opinion? I think supplying the absolute path makes archiving scripts less error-prone, which is a good time. So I'd vote for absolute paths. greetings, Florian Pflug ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
On Fri, 2006-11-03 at 17:34 +0100, Martijn van Oosterhout wrote: On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: Since 8.1 has done this all along and no one's actually complained about it, I guess no one is using scripts that do cd. I'm inclined to go with Bernd's suggestion to change the docs to match the code, but does anyone have a contrary opinion? In Unix you can easily prepend $PWD to the string, but I don't know how easy that is in Windows. Windows input anyone? Given the lack of a comprehensive test suite at this stage, I'd vote on the side of least change right now. We know the existing mechanism works, and as Martijn point out there is a workaround, plus as Tom discusses this would only happen if people cd which in my book would be bad programming form anyway. +1 Doc bug for 8.2, feature request for 8.3, unless Windows bites. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
Simon Riggs wrote: On Fri, 2006-11-03 at 17:34 +0100, Martijn van Oosterhout wrote: On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: Since 8.1 has done this all along and no one's actually complained about it, I guess no one is using scripts that do cd. I'm inclined to go with Bernd's suggestion to change the docs to match the code, but does anyone have a contrary opinion? In Unix you can easily prepend $PWD to the string, but I don't know how easy that is in Windows. Windows input anyone? Of course you can get the current directory on Windows, if that's what the question is. cheers andrew ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] [PATCHES] Bug in WAL backup documentation
Since 8.1 has done this all along and no one's actually complained about it, I guess no one is using scripts that do cd. I'm inclined to go with Bernd's suggestion to change the docs to match the code, but does anyone have a contrary opinion? In Unix you can easily prepend $PWD to the string, but I don't know how easy that is in Windows. Windows input anyone? %CD% gives the same as $PWD in a command shell: C:\Program Files\Microsoft Visual Studio 8\VCecho %CD% C:\Program Files\Microsoft Visual Studio 8\VC //Magnus ---(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
