From f4791fcb106114143134d25c8a1abe745750ae6e Mon Sep 17 00:00:00 2001
From: Jelte Fennema <github-tech@jeltef.nl>
Date: Wed, 10 Jun 2020 17:04:07 +0200
Subject: [PATCH] Remove SwallowErrors by using PathNameDeleteTemporaryDir
 (#3893)

This is a different version of #3634. It also removes SwallowErrors, but
instead of modifying our own functions to not throw errors, it uses the
postgres built in `PathNameDeleteTemporaryDir` function. This function
does not throw errors.

Since this change is for a bugfix, I tried to minimize the changes.

PRs with the following changes would be good to do separately from this
PR:
1. Use PathName(Create|Open|Delete)Temporary(File|Dir) to open and
   remove all files/dirs instead of our own custom file functions.
2. Prefix our outmost files/directories with `PG_TEMP_FILE_PREFIX` so
   that they are identified by Postgres as temporary files, which will be
   removed at postmaster start. This way we do not have to do this cleanup
   ourselves.
3. Store the files in the temporary table space if it exists.

Fixes #3634
Fixes #3618
---
 .../executor/intermediate_results.c           |  2 +-
 .../transaction/transaction_management.c      | 48 +------------------
 2 files changed, 2 insertions(+), 48 deletions(-)

diff --git a/src/backend/distributed/executor/intermediate_results.c b/src/backend/distributed/executor/intermediate_results.c
index bb5d02ba1..ca213e01e 100644
--- a/src/backend/distributed/executor/intermediate_results.c
+++ b/src/backend/distributed/executor/intermediate_results.c
@@ -688,7 +688,7 @@ RemoveIntermediateResultsDirectory(void)
 {
 	if (CreatedResultsDirectory)
 	{
-		CitusRemoveDirectory(IntermediateResultsDirectory());
+		PathNameDeleteTemporaryDir(IntermediateResultsDirectory());
 
 		CreatedResultsDirectory = false;
 	}
diff --git a/src/backend/distributed/transaction/transaction_management.c b/src/backend/distributed/transaction/transaction_management.c
index 418ecfce2..67fa8f30d 100644
--- a/src/backend/distributed/transaction/transaction_management.c
+++ b/src/backend/distributed/transaction/transaction_management.c
@@ -112,7 +112,6 @@ static void ResetShardPlacementTransactionState(void);
 static void AdjustMaxPreparedTransactions(void);
 static void PushSubXact(SubTransactionId subId);
 static void PopSubXact(SubTransactionId subId);
-static void SwallowErrors(void (*func)());
 static bool MaybeExecutingUDF(void);
 static void ResetGlobalVariables(void);
 
@@ -297,7 +296,7 @@ CoordinatedTransactionCallback(XactEvent event, void *arg)
 				 * RemoveIntermediateResultsDirectory.
 				 */
 				AtEOXact_Files(false);
-				SwallowErrors(RemoveIntermediateResultsDirectory);
+				RemoveIntermediateResultsDirectory();
 			}
 			ResetShardPlacementTransactionState();
 
@@ -669,51 +668,6 @@ ActiveSubXactContexts(void)
 }
 
 
-/*
- * If an ERROR is thrown while processing a transaction the ABORT handler is called.
- * ERRORS thrown during ABORT are not treated any differently, the ABORT handler is also
- * called during processing of those. If an ERROR was raised the first time through it's
- * unlikely that the second try will succeed; more likely that an ERROR will be thrown
- * again. This loop continues until Postgres notices and PANICs, complaining about a stack
- * overflow.
- *
- * Instead of looping and crashing, SwallowErrors lets us attempt to continue running the
- * ABORT logic. This wouldn't be safe in most other parts of the codebase, in
- * approximately none of the places where we emit ERROR do we first clean up after
- * ourselves! It's fine inside the ABORT handler though; Postgres is going to clean
- * everything up before control passes back to us.
- */
-static void
-SwallowErrors(void (*func)())
-{
-	MemoryContext savedContext = CurrentMemoryContext;
-
-	PG_TRY();
-	{
-		func();
-	}
-	PG_CATCH();
-	{
-		ErrorData *edata = CopyErrorData();
-
-		/* don't try to intercept PANIC or FATAL, let those breeze past us */
-		if (edata->elevel != ERROR)
-		{
-			PG_RE_THROW();
-		}
-
-		/* turn the ERROR into a WARNING and emit it */
-		edata->elevel = WARNING;
-		ThrowErrorData(edata);
-
-		/* leave the error handling system */
-		FlushErrorState();
-		MemoryContextSwitchTo(savedContext);
-	}
-	PG_END_TRY();
-}
-
-
 /*
  * IsMultiStatementTransaction determines whether the current statement is
  * part of a bigger multi-statement transaction. This is the case when the