From b18c9c8060365ffb2487934d1a2c693a95152fd8 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jdavis@postgresql.org>
Date: Fri, 11 Sep 2020 14:21:56 -0700
Subject: [PATCH] drop storage for DROP command

---
 cstore_fdw.c | 75 +++++++++++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 27 deletions(-)

diff --git a/cstore_fdw.c b/cstore_fdw.c
index 073a68130..5ad465807 100644
--- a/cstore_fdw.c
+++ b/cstore_fdw.c
@@ -127,7 +127,7 @@ static uint64 CopyIntoCStoreTable(const CopyStmt *copyStatement,
 								  const char *queryString);
 static uint64 CopyOutCStoreTable(CopyStmt *copyStatement, const char *queryString);
 static void CStoreProcessAlterTableCommand(AlterTableStmt *alterStatement);
-static List * DroppedCStoreFilenameList(DropStmt *dropStatement);
+static List * DroppedCStoreRelidList(DropStmt *dropStatement);
 static List * FindCStoreTables(List *tableList);
 static List * OpenRelationsForTruncate(List *cstoreTableList);
 static void InitializeRelFileNode(Relation relation);
@@ -369,17 +369,43 @@ CStoreProcessUtility(Node * parseTree, const char * queryString,
 		}
 		else
 		{
-			ListCell *fileListCell = NULL;
-			List *droppedTables = DroppedCStoreFilenameList((DropStmt *) parseTree);
+			List		*dropRelids = DroppedCStoreRelidList((DropStmt *) parseTree);
+			List		*dropFiles	= NIL;
+			ListCell	*lc			= NULL;
+
+			/* drop smgr storage */
+			foreach(lc, dropRelids)
+			{
+				Oid				 relid			 = lfirst_oid(lc);
+				Relation		 relation		 = cstore_fdw_open(relid, AccessExclusiveLock);
+				CStoreOptions	*cstoreOptions	 = CStoreGetOptions(relid);
+				char			*defaultfilename = CStoreDefaultFilePath(relid);
+
+				RelationOpenSmgr(relation);
+				RelationDropStorage(relation);
+				heap_close(relation, AccessExclusiveLock);
+
+				/*
+				 * Skip files that are placed in default location, they are handled
+				 * by sql drop trigger. Both paths are generated by code, use
+				 * of strcmp is safe here.
+				 */
+				if (strcmp(defaultfilename, cstoreOptions->filename) == 0)
+				{
+					continue;
+				}
+
+				dropFiles = lappend(dropFiles, cstoreOptions->filename);
+			}
 
 			CALL_PREVIOUS_UTILITY(parseTree, queryString, context, paramListInfo,
 								  destReceiver, completionTag);
 
-			foreach(fileListCell, droppedTables)
+			/* drop files */
+			foreach(lc, dropFiles)
 			{
-				char *fileName = lfirst(fileListCell);
-				//TODO: relation storage is not dropped
-				DeleteCStoreTableFiles(fileName);
+				char *filename = lfirst(lc);
+				DeleteCStoreTableFiles(filename);
 			}
 		}
 	}
@@ -783,13 +809,13 @@ CStoreProcessAlterTableCommand(AlterTableStmt *alterStatement)
 
 
 /*
- * DropppedCStoreFilenameList extracts and returns the list of cstore file names
+ * DropppedCStoreRelidList extracts and returns the list of cstore relids
  * from DROP table statement
  */
 static List *
-DroppedCStoreFilenameList(DropStmt *dropStatement)
+DroppedCStoreRelidList(DropStmt *dropStatement)
 {
-	List *droppedCStoreFileList = NIL;
+	List *droppedCStoreRelidList = NIL;
 
 	if (dropStatement->removeType == OBJECT_FOREIGN_TABLE)
 	{
@@ -802,26 +828,13 @@ DroppedCStoreFilenameList(DropStmt *dropStatement)
 			Oid relationId = RangeVarGetRelid(rangeVar, AccessShareLock, true);
 			if (CStoreTable(relationId))
 			{
-				CStoreOptions *cstoreOptions = CStoreGetOptions(relationId);
-				char *defaultfilename = CStoreDefaultFilePath(relationId);
-
-				/*
-				 * Skip files that are placed in default location, they are handled
-				 * by sql drop trigger. Both paths are generated by code, use
-				 * of strcmp is safe here.
-				 */
-				if (strcmp(defaultfilename, cstoreOptions->filename) == 0)
-				{
-					continue;
-				}
-
-				droppedCStoreFileList = lappend(droppedCStoreFileList,
-												cstoreOptions->filename);
+				droppedCStoreRelidList = lappend_oid(droppedCStoreRelidList,
+													 relationId);
 			}
 		}
 	}
 
-	return droppedCStoreFileList;
+	return droppedCStoreRelidList;
 }
 
 
@@ -1254,7 +1267,15 @@ cstore_clean_table_resources(PG_FUNCTION_ARGS)
 	struct stat fileStat;
 	int statResult = -1;
 
-	//TODO: relation storage is not dropped
+	/*
+	 * TODO: Event triggers do not offer the relfilenode of the
+	 * dropped table, and by the time the sql_drop event trigger
+	 * is called, the object is already gone so we can't look it
+	 * up. Therefore, we can't drop the Smgr storage here, which
+	 * means that cascaded drops of cstore foreign tables will
+	 * leak storage.
+	 */
+
 	appendStringInfo(filePath, "%s/%s/%d/%d", DataDir, CSTORE_FDW_NAME,
 					 (int) MyDatabaseId, (int) relationId);