From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun, 13 May 2012 22:06:52 +0000 (-0400)
Subject: Fix DROP TABLESPACE to unlink symlink when directory is not there.
X-Git-Tag: REL9_2_BETA2~90
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b8347138e94c4e712334508c460cbe0062d21e70;p=postgresql

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).
---

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)));