Fix DROP TABLESPACE to unlink symlink when directory is not there.

If the tablespace directory is missing entirely, we allow DROP TABLESPACE
to go through, on the grounds that it should be possible to clean up the
catalog entry in such a situation.  However, we forgot that the pg_tblspc
symlink might still be there.  We should try to remove the symlink too
(but not fail if it's no longer there), since not doing so can lead to
weird behavior subsequently, as per report from Michael Nolan.

There was some discussion of adding dependency links to prevent DROP
TABLESPACE when the catalogs still contain references to the tablespace.
That might be worth doing too, but it's an orthogonal question, and in
any case wouldn't be back-patchable.

Back-patch to 9.0, which is as far back as the logic looks like this.
We could possibly do something similar in 8.x, but given the lack of
reports I'm not sure it's worth the trouble, and anyway the case could
not arise in the form the logic is meant to cover (namely, a post-DROP
transaction rollback having resurrected the pg_tablespace entry after
some or all of the filesystem infrastructure is gone).
This commit is contained in:
Tom Lane 2012-05-13 18:06:52 -04:00
parent 966970ed63
commit b8347138e9

View File

@ -678,8 +678,9 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo)
* with a warning. This is because even though ProcessUtility disallows * with a warning. This is because even though ProcessUtility disallows
* DROP TABLESPACE in a transaction block, it's possible that a previous * DROP TABLESPACE in a transaction block, it's possible that a previous
* DROP failed and rolled back after removing the tablespace directories * DROP failed and rolled back after removing the tablespace directories
* and symlink. We want to allow a new DROP attempt to succeed at * and/or symlink. We want to allow a new DROP attempt to succeed at
* removing the catalog entries, so we should not give a hard error here. * removing the catalog entries (and symlink if still present), so we
* should not give a hard error here.
*/ */
dirdesc = AllocateDir(linkloc_with_version_dir); dirdesc = AllocateDir(linkloc_with_version_dir);
if (dirdesc == NULL) if (dirdesc == NULL)
@ -691,8 +692,8 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo)
(errcode_for_file_access(), (errcode_for_file_access(),
errmsg("could not open directory \"%s\": %m", errmsg("could not open directory \"%s\": %m",
linkloc_with_version_dir))); linkloc_with_version_dir)));
pfree(linkloc_with_version_dir); /* The symlink might still exist, so go try to remove it */
return true; goto remove_symlink;
} }
else if (redo) else if (redo)
{ {
@ -755,8 +756,10 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo)
* Windows where junction points lstat() as directories. * Windows where junction points lstat() as directories.
* *
* Note: in the redo case, we'll return true if this final step fails; * Note: in the redo case, we'll return true if this final step fails;
* there's no point in retrying it. * there's no point in retrying it. Also, ENOENT should provoke no more
* than a warning.
*/ */
remove_symlink:
linkloc = pstrdup(linkloc_with_version_dir); linkloc = pstrdup(linkloc_with_version_dir);
get_parent_directory(linkloc); get_parent_directory(linkloc);
if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode)) if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode))
@ -770,7 +773,7 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo)
else else
{ {
if (unlink(linkloc) < 0) if (unlink(linkloc) < 0)
ereport(redo ? LOG : ERROR, ereport(redo ? LOG : (errno == ENOENT ? WARNING : ERROR),
(errcode_for_file_access(), (errcode_for_file_access(),
errmsg("could not remove symbolic link \"%s\": %m", errmsg("could not remove symbolic link \"%s\": %m",
linkloc))); linkloc)));