I did worked on testing the patch on Windows, test scenario mentioned above
thread is reproducible and the provided patch resolve the issue. In case of
junction or directory unlink function
(deprecated<http://msdn.microsoft.com/en-us/library/ms235350.aspx>)
returns -1 with errno “EACCES” (i.e. Permission denied). As you have
followed destroy_tablespace_directories() function, Is there any specific
reason not to use same logic to detect type of the file/link i.e.
“(lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))”, It also seems have
more appropriate error message i.e.

src/backend/commands/tablespace.c

> ….
>     745         /*
>     746          * Try to remove the symlink.  We must however deal with
> the possibility
>     747          * that it's a directory instead of a symlink --- this
> could happen during
>     748          * WAL replay (see TablespaceCreateDbspace), and it is
> also the case on
>     749          * Windows where junction points lstat() as directories.
>     750          *
>     751          * Note: in the redo case, we'll return true if this final
> step fails;
>     752          * there's no point in retrying it.  Also, ENOENT should
> provoke no more
>     753          * than a warning.
>     754          */
>     755 remove_symlink:
>     756         linkloc = pstrdup(linkloc_with_version_dir);
>     757         get_parent_directory(linkloc);
>     758         if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
>     759         {
>     760                 if (rmdir(linkloc) < 0)
>     761                         ereport(redo ? LOG : ERROR,
>     762                                         (errcode_for_file_access(),
>     763                                          errmsg("could not remove
> directory \"%s\": %m",
>     764                                                         linkloc)));
>     765         }
>     766         else
>     767         {
>     768                 if (unlink(linkloc) < 0)
>     769                         ereport(redo ? LOG : (errno == ENOENT ?
> WARNING : ERROR),
>     770                                         (errcode_for_file_access(),
>     771                                          errmsg("could not remove
> symbolic link \"%s\": %m",
>     772                                                         linkloc)));
>     773         }
> ….


Other than this patch looks good to me.

Regards,
Muhammad Asif Naeem



On Thu, Oct 31, 2013 at 8:03 PM, MauMau <maumau...@gmail.com> wrote:

> From: "MauMau" <maumau...@gmail.com>
>
>  I thought the same situation could happen as in
>> destroy_tablespace_directories(), but it doesn't seem to apply on second
>> thought.  Revised patch attached, which is very simple based on compile
>> time
>> check.
>>
>
> Sorry, I was careless to leave an old comment.  Please use this version.
>
> Regards
> MauMau
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
>

Reply via email to