From b8347138e94c4e712334508c460cbe0062d21e70 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 13 May 2012 18:06:52 -0400 Subject: [PATCH] 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). --- src/backend/commands/tablespace.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/tablespace.c b/src/backend/commands/tablespace.c index d66ea3b6c1..708bebb54d 100644 --- a/src/backend/commands/tablespace.c +++ b/src/backend/commands/tablespace.c @@ -678,8 +678,9 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo) * with a warning. This is because even though ProcessUtility disallows * DROP TABLESPACE in a transaction block, it's possible that a previous * DROP failed and rolled back after removing the tablespace directories - * and 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. + * and/or symlink. We want to allow a new DROP attempt to succeed at + * removing the catalog entries (and symlink if still present), so we + * should not give a hard error here. */ dirdesc = AllocateDir(linkloc_with_version_dir); if (dirdesc == NULL) @@ -691,8 +692,8 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo) (errcode_for_file_access(), errmsg("could not open directory \"%s\": %m", linkloc_with_version_dir))); - pfree(linkloc_with_version_dir); - return true; + /* The symlink might still exist, so go try to remove it */ + goto remove_symlink; } else if (redo) { @@ -755,8 +756,10 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo) * Windows where junction points lstat() as directories. * * 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); get_parent_directory(linkloc); if (lstat(linkloc, &st) == 0 && S_ISDIR(st.st_mode)) @@ -770,7 +773,7 @@ destroy_tablespace_directories(Oid tablespaceoid, bool redo) else { if (unlink(linkloc) < 0) - ereport(redo ? LOG : ERROR, + ereport(redo ? LOG : (errno == ENOENT ? WARNING : ERROR), (errcode_for_file_access(), errmsg("could not remove symbolic link \"%s\": %m", linkloc)));