Re: [PATCHES] initdb in C
Patch applied. We now have a C version of initdb! --- Andrew Dunstan wrote: Bruce Momjian wrote: Here is a slightly modified version of Andrew's great work in making a C version of initdb. Other than minor cleanups, the only big change was to remove rmdir handling because we using rm -r and rmdir /s in commands/dbcommands.c, so we might as use the same thing for initdb.c rather than having code that traverses the directory tree doing 'rm'. The other change was to remove the unlink code and instead use port/dirmod.c's version. It passes all the regression tests. I have also included a diff against Andrew's version so you can see my changes. It seems Andrew had a very current version of initdb. The only update he missed was the change to test the number of connections before shared buffers --- I made that change myself. I would like to apply this in the next few days to HEAD before initdb.sh drifts. We are no longer using the WIN32_DEV branch because all our work is now in 7.5 head. My comments: I have no problem with shelling out to rmdir - although my goal was to avoid shelling out to anything other than postgres itself. I think recreating the datadir if we didn't create it initially should be OK in that case, and it makes the code simpler. Not handling dot files in the Unix case should also be fine, as we know the directory was empty to start with and we don't create any. Regarding the #endif comments you removed - Peter had asked me to put them in. See http://archives.postgresql.org/pgsql-hackers/2003-10/msg00352.php You put a comment on canonicalise_path() asking if it is needed. The short answer is very much yes. The Windows command processor will accept suitably quoted paths with forward slashes, but barfs badly with mixed forward and back slashes. (See recent discussion on hackers-win32).Removing the trailing slash on a path means we never get ugly double slashes. Feel free to put that info in as a comment if you think it is needed. The changes you made for the final message are not going to work for Windows - if pgpath or pg_data contain spaces we need quotes (even on Unix, although there most people know better than to put spaces in names). Also see above about mixed slashes. Also, putting a \ before pg_data will be nasty if it starts with a drive or network specifier - or even without (you might turn a directory specifier into a network drive specifier). It's just a message, though, and we shouldn't hold up for that - we can patch it to get it right. (Getting the path and slash thing working portably was by far the biggest headache in this project - the rest was just grunt work for the most part, although I learned a few interesting things.) Otherwise I'm fine with this. cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 8: explain analyze is your friend
Re: [PATCHES] initdb in C
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: It passes all the regression tests. I have also included a diff against Andrew's version so you can see my changes. It seems Andrew had a very current version of initdb. The only update he missed was the change to test the number of connections before shared buffers --- I made that change myself. I had some concern about whether Andrew's rewrite had tracked all the recent changes to the initdb shell-script sources. Are you both confident that it is up to date? Yes. Bruce has picked up the one change I didn't track (revision 1.204). cheers andrew ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] initdb in C
Bruce Momjian wrote: It passes all the regression tests. I have also included a diff against Andrew's version so you can see my changes. It seems Andrew had a very current version of initdb. The only update he missed was the change to test the number of connections before shared buffers --- I made that change myself. I would like to apply this in the next few days to HEAD before initdb.sh drifts. We are no longer using the WIN32_DEV branch because all our work is now in 7.5 head. I will double check this over the weekend. cheers andrew ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] initdb in C
Bruce Momjian wrote: Here is a slightly modified version of Andrew's great work in making a C version of initdb. Other than minor cleanups, the only big change was to remove rmdir handling because we using rm -r and rmdir /s in commands/dbcommands.c, so we might as use the same thing for initdb.c rather than having code that traverses the directory tree doing 'rm'. The other change was to remove the unlink code and instead use port/dirmod.c's version. It passes all the regression tests. I have also included a diff against Andrew's version so you can see my changes. It seems Andrew had a very current version of initdb. The only update he missed was the change to test the number of connections before shared buffers --- I made that change myself. I would like to apply this in the next few days to HEAD before initdb.sh drifts. We are no longer using the WIN32_DEV branch because all our work is now in 7.5 head. My comments: I have no problem with shelling out to rmdir - although my goal was to avoid shelling out to anything other than postgres itself. I think recreating the datadir if we didn't create it initially should be OK in that case, and it makes the code simpler. Not handling dot files in the Unix case should also be fine, as we know the directory was empty to start with and we don't create any. Regarding the #endif comments you removed - Peter had asked me to put them in. See http://archives.postgresql.org/pgsql-hackers/2003-10/msg00352.php You put a comment on canonicalise_path() asking if it is needed. The short answer is very much yes. The Windows command processor will accept suitably quoted paths with forward slashes, but barfs badly with mixed forward and back slashes. (See recent discussion on hackers-win32).Removing the trailing slash on a path means we never get ugly double slashes. Feel free to put that info in as a comment if you think it is needed. The changes you made for the final message are not going to work for Windows - if pgpath or pg_data contain spaces we need quotes (even on Unix, although there most people know better than to put spaces in names). Also see above about mixed slashes. Also, putting a \ before pg_data will be nasty if it starts with a drive or network specifier - or even without (you might turn a directory specifier into a network drive specifier). It's just a message, though, and we shouldn't hold up for that - we can patch it to get it right. (Getting the path and slash thing working portably was by far the biggest headache in this project - the rest was just grunt work for the most part, although I learned a few interesting things.) Otherwise I'm fine with this. cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] initdb in C
Andrew Dunstan writes: recreating the datadir if we didn't create it initially should be OK in that case, and it makes the code simpler. That should be avoided, because you'll have trouble recreating the original directory with all its properties such as ownership, permissions, etc., at least not without making the code anything but simpler. There might even be a situation were you are allowed to delete the directory but cannot create a new one. -- Peter Eisentraut [EMAIL PROTECTED] ---(end of broadcast)--- TIP 2: you can get off all lists at once with the unregister command (send unregister YourEmailAddressHere to [EMAIL PROTECTED])
Re: [PATCHES] initdb in C
Peter Eisentraut [EMAIL PROTECTED] writes: Andrew Dunstan writes: recreating the datadir if we didn't create it initially should be OK in that case, and it makes the code simpler. That should be avoided, because you'll have trouble recreating the original directory with all its properties such as ownership, permissions, etc., at least not without making the code anything but simpler. There might even be a situation were you are allowed to delete the directory but cannot create a new one. Consider also the strong likelihood that the data directory's parent directory is owned by root, so that you do not have the ability to delete and recreate the data directory because you don't have write permission on its parent. The main reason initdb is set up to be able to start with an existing-but-empty data dir is exactly because creating that directory may have required permissions that initdb itself hasn't got. regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] initdb in C
Tom Lane wrote: Peter Eisentraut [EMAIL PROTECTED] writes: Andrew Dunstan writes: recreating the datadir if we didn't create it initially should be OK in that case, and it makes the code simpler. That should be avoided, because you'll have trouble recreating the original directory with all its properties such as ownership, permissions, etc., at least not without making the code anything but simpler. There might even be a situation were you are allowed to delete the directory but cannot create a new one. Consider also the strong likelihood that the data directory's parent directory is owned by root, so that you do not have the ability to delete and recreate the data directory because you don't have write permission on its parent. The main reason initdb is set up to be able to start with an existing-but-empty data dir is exactly because creating that directory may have required permissions that initdb itself hasn't got. Again, this directory recreate happens only on Win32, an I thought it would be OK there. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] initdb in C
Peter Eisentraut wrote: Andrew Dunstan writes: recreating the datadir if we didn't create it initially should be OK in that case, and it makes the code simpler. That should be avoided, because you'll have trouble recreating the original directory with all its properties such as ownership, permissions, etc., at least not without making the code anything but simpler. There might even be a situation were you are allowed to delete the directory but cannot create a new one. Recreating the directory only happens on WIN32, where rmdir doesn't allow you to only delete files and subdirectories and not the parent directory. Non-Win32 does rm -rf dir/*. Is that OK? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [PATCHES] initdb in C
Bruce Momjian [EMAIL PROTECTED] writes: Consider also the strong likelihood that the data directory's parent directory is owned by root, Again, this directory recreate happens only on Win32, an I thought it would be OK there. Windows has no concept of directory permissions at all? I thought the more recent versions had at least rudimentary permissions. regards, tom lane ---(end of broadcast)--- TIP 1: subscribe and unsubscribe commands go to [EMAIL PROTECTED]
Re: [PATCHES] initdb in C
Bruce Momjian [EMAIL PROTECTED] writes: Recreating the directory only happens on WIN32, where rmdir doesn't allow you to only delete files and subdirectories and not the parent directory. Non-Win32 does rm -rf dir/*. I think we should forget about invoking rm as a subprocess at all, and just do the recursive directory walk and unlinks for ourselves. We already have code to do this for copy in copydir.c, and unlink would not be any longer. We will probably be forced into implementing database removal for ourselves rather than by 'rm' hacks anyway as soon as tablespaces come to pass; so why contort initdb's behavior for a very transient implementation savings? regards, tom lane ---(end of broadcast)--- TIP 4: Don't 'kill -9' the postmaster
Re: [PATCHES] initdb in C
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Consider also the strong likelihood that the data directory's parent directory is owned by root, Again, this directory recreate happens only on Win32, an I thought it would be OK there. Windows has no concept of directory permissions at all? I thought the more recent versions had at least rudimentary permissions. I found del works for what we need. I have posted the new code already. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [PATCHES] initdb in C
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Recreating the directory only happens on WIN32, where rmdir doesn't allow you to only delete files and subdirectories and not the parent directory. Non-Win32 does rm -rf dir/*. I think we should forget about invoking rm as a subprocess at all, and just do the recursive directory walk and unlinks for ourselves. We already have code to do this for copy in copydir.c, and unlink would not be any longer. We will probably be forced into implementing database removal for ourselves rather than by 'rm' hacks anyway as soon as tablespaces come to pass; so why contort initdb's behavior for a very transient implementation savings? If we want to do that, fine, but I don't want to force the change just for Win32. -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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] initdb in C
Also, I see this at the top of the code: * author: Andrew Dunstan mailto:[EMAIL PROTECTED] * * Copyright (C) 2003 Andrew Dunstan * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California Can I remove the first copyright? -- Bruce Momjian| http://candle.pha.pa.us [EMAIL PROTECTED] | (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 4: Don't 'kill -9' the postmaster
Re: [PATCHES] initdb in C
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Consider also the strong likelihood that the data directory's parent directory is owned by root, Again, this directory recreate happens only on Win32, an I thought it would be OK there. Windows has no concept of directory permissions at all? I thought the more recent versions had at least rudimentary permissions. More than rudimentary on the server versions. I found this info at http://www.cs.wayne.edu/labPages/modes.htm : Windows ACLs Windows NT and Windows 2000 uses Access Control Lists or ACLs to determine what operations a user may perform on a file. Windows ACLs allow one to set permissions with finer control that does the Unix file mode. For example, one can all[ow] a user to append data to a file as opposed to overwiting the file. ACLs also allow one to permit specific users to change the permissions on a file. Perhaps the biggest difference is that ACLs allow us to accord permissions on a user-by-user basis, rather than the three categories of users permitted by Unix file systems. This info applies to directories as well as plain files AFAIK cheers andrew ---(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] initdb in C
Bruce Momjian wrote: Also, I see this at the top of the code: * author: Andrew Dunstan mailto:[EMAIL PROTECTED] * * Copyright (C) 2003 Andrew Dunstan * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California Can I remove the first copyright? Sure. I wasn't sure what our conventions were on that. cheers andrew ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] initdb in C
Bruce Momjian [EMAIL PROTECTED] writes: It passes all the regression tests. I have also included a diff against Andrew's version so you can see my changes. It seems Andrew had a very current version of initdb. The only update he missed was the change to test the number of connections before shared buffers --- I made that change myself. I had some concern about whether Andrew's rewrite had tracked all the recent changes to the initdb shell-script sources. Are you both confident that it is up to date? regards, tom lane ---(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
