On Fri, Mar 21, 2014 at 12:24 PM, MauMau <maumau307@gmail.com> wrote:
> From: "Amit Kapila" <amit.kapila16@gmail.com>
>> On Wed, Jan 15, 2014 at 8:12 PM, MauMau <maumau307@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
removedby 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