Partially revoke attempt to improve performance with many savepoints.

simonat2ndQuadrant · simonat2ndQuadrant · commit df383b03e6f7 · 2011-09-07T12:11:26.000+01:00
Maintain difference between subtransaction release and commit introduced
by earlier patch.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
@@ -269,7 +269,7 @@ static TransactionId RecordTransactionAbort(bool isSubXact);
 static void StartTransaction(void);
 
 static void StartSubTransaction(void);
-static void CommitSubTransaction(bool isTopLevel);
+static void CommitSubTransaction(void);
 static void AbortSubTransaction(void);
 static void CleanupSubTransaction(void);
 static void PushTransaction(void);
@@ -2578,7 +2578,7 @@ CommitTransactionCommand(void)
 		case TBLOCK_SUBRELEASE:
 			do
 			{
-				CommitSubTransaction(false);
+				CommitSubTransaction();
 				s = CurrentTransactionState;	/* changed by pop */
 			} while (s->blockState == TBLOCK_SUBRELEASE);
 
@@ -2588,12 +2588,17 @@ CommitTransactionCommand(void)
 
 			/*
 			 * We were issued a COMMIT, so we end the current subtransaction
-			 * hierarchy and perform final commit.
+			 * hierarchy and perform final commit. We do this by rolling up
+			 * any subtransactions into their parent, which leads to O(N^2)
+			 * operations with respect to resource owners - this isn't that
+			 * bad until we approach a thousands of savepoints but is necessary
+			 * for correctness should after triggers create new resource
+			 * owners.
 			 */
 		case TBLOCK_SUBCOMMIT:
 			do
 			{
-				CommitSubTransaction(true);
+				CommitSubTransaction();
 				s = CurrentTransactionState;	/* changed by pop */
 			} while (s->blockState == TBLOCK_SUBCOMMIT);
 			/* If we had a COMMIT command, finish off the main xact too */
@@ -3745,7 +3750,7 @@ ReleaseCurrentSubTransaction(void)
 			 BlockStateAsString(s->blockState));
 	Assert(s->state == TRANS_INPROGRESS);
 	MemoryContextSwitchTo(CurTransactionContext);
-	CommitSubTransaction(false);
+	CommitSubTransaction();
 	s = CurrentTransactionState;	/* changed by pop */
 	Assert(s->state == TRANS_INPROGRESS);
 }
@@ -4009,13 +4014,9 @@ StartSubTransaction(void)
  *
  *	The caller has to make sure to always reassign CurrentTransactionState
  *	if it has a local pointer to it after calling this function.
- *
- *	isTopLevel means that this CommitSubTransaction() is being issued as a
- *	sequence of actions leading directly to a main transaction commit
- *	allowing some actions to be optimised.
  */
 static void
-CommitSubTransaction(bool isTopLevel)
+CommitSubTransaction(void)
 {
 	TransactionState s = CurrentTransactionState;
 
@@ -4069,14 +4070,10 @@ CommitSubTransaction(bool isTopLevel)
 
 	/*
 	 * Other locks should get transferred to their parent resource owner.
-	 * Doing that is an O(N^2) operation, so if isTopLevel then we can just
-	 * leave the lock records as they are, knowing they will all get released
-	 * by the top level commit using ProcReleaseLocks(). We only optimize
-	 * this for commit; aborts may need to do other cleanup.
 	 */
 	ResourceOwnerRelease(s->curTransactionOwner,
 						 RESOURCE_RELEASE_LOCKS,
-						 true, isTopLevel);
+						 true, false);
 	ResourceOwnerRelease(s->curTransactionOwner,
 						 RESOURCE_RELEASE_AFTER_LOCKS,
 						 true, false);
