On Fri, Mar 21, 2014 at 12:24 PM, MauMau <maumau...@gmail.com> wrote: > From: "Amit Kapila" <amit.kapil...@gmail.com> >> On Wed, Jan 15, 2014 at 8:12 PM, MauMau <maumau...@gmail.com> wrote: >>> >>> Thanks for reviewing and testing the patch. Yes, at first I did what you >>> mentioned, but modified the patch according to some advice in the mail >>> thread. During redo, create_tablespace_directories() needs to handle the >>> case where the $PGDATA/pg_tblspc/xxx is not a symlink but a directory >>> even >>> on UNIX/Linux. Please see TablespaceCreateDbspace is(). >>> destroy_tablespace_directories() doesn't have to handle such situation. >> >> >> If create_tablespace_directories() needs to handle with directory both on >> Windows/Linux, then shouldn't it be a runtime check as in your first >> version rather than compile time check? >> Also isn't that the reason why destroy_tablespace_directories() have >> similar >> check? > > > I see..., and you are correct.
I have thought about the above point again and it seems to me that the above claim (create_tablespace_directories() needs to handle a path which is not a symlink but directory) might not be true. For Example, I could easily think of case where it is required for destroy_tablespace_directories(). 1. Assume a tablespace tbs already exists. 2. Create table t1(c1 int) tablespace tbs; 3. drop table t1; 4. Drop tablespace tbs; 5. Do immediate shutdown (pg_ctl stop -mi); 6. During recovery it will create a table in directory (in function TablespaceCreateDbspace) which needs to be removed by destroy_tablespace_directories(). I am neither aware of, nor could think of such a case for create_tablespace_directories(). Do you have any such case in mind which I might be missing? By saying above, I don't mean that your current patch has any problem; even if there is no such scenario, I think your code is right as stat/isdir check seems to be okay to identify junction points and it avoids ifdef WIN32 check (which I personally think is bit annoying and we should try to avoid such code unless it is must or provides any significant advantage). With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers