v2-0003-EnsurePortalSnapshotExists-failed-assertion.patch

text/plain

Filename: v2-0003-EnsurePortalSnapshotExists-failed-assertion.patch
Type: text/plain
Part: 0
Message: Re: [BUG] failed assertion in EnsurePortalSnapshotExists()

Patch

Format: unified
Series: patch v2-0003
File+
src/backend/access/transam/xact.c 2 1
src/backend/tcop/pquery.c 9 9
src/backend/utils/mmgr/portalmem.c 4 1
src/backend/utils/time/snapmgr.c 14 2
src/include/utils/portal.h 3 1
src/include/utils/snapmgr.h 1 0
src/test/regress/expected/portals.out 18 0
src/test/regress/sql/portals.sql 21 0
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 6597ec45a9..ef5450afe8 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -4863,7 +4863,8 @@ CommitSubTransaction(void)
 	AfterTriggerEndSubXact(true);
 	AtSubCommit_Portals(s->subTransactionId,
 						s->parent->subTransactionId,
-						s->parent->curTransactionOwner);
+						s->parent->curTransactionOwner,
+						s->parent->nestingLevel);
 	AtEOSubXact_LargeObject(true, s->subTransactionId,
 							s->parent->subTransactionId);
 	AtSubCommit_Notify();
diff --git a/src/backend/tcop/pquery.c b/src/backend/tcop/pquery.c
index b5797042ab..a6b320ac9b 100644
--- a/src/backend/tcop/pquery.c
+++ b/src/backend/tcop/pquery.c
@@ -472,9 +472,9 @@ PortalStart(Portal portal, ParamListInfo params,
 
 				/* Must set snapshot before starting executor. */
 				if (snapshot)
-					PushActiveSnapshot(snapshot);
+					PushActiveSnapshotWithLevel(snapshot, portal->nestingLevel);
 				else
-					PushActiveSnapshot(GetTransactionSnapshot());
+					PushActiveSnapshotWithLevel(GetTransactionSnapshot(), portal->nestingLevel);
 
 				/*
 				 * We could remember the snapshot in portal->portalSnapshot,
@@ -915,7 +915,7 @@ PortalRunSelect(Portal portal,
 			nprocessed = RunFromStore(portal, direction, (uint64) count, dest);
 		else
 		{
-			PushActiveSnapshot(queryDesc->snapshot);
+			PushActiveSnapshotWithLevel(queryDesc->snapshot, portal->nestingLevel);
 			ExecutorRun(queryDesc, direction, (uint64) count,
 						portal->run_once);
 			nprocessed = queryDesc->estate->es_processed;
@@ -955,7 +955,7 @@ PortalRunSelect(Portal portal,
 			nprocessed = RunFromStore(portal, direction, (uint64) count, dest);
 		else
 		{
-			PushActiveSnapshot(queryDesc->snapshot);
+			PushActiveSnapshotWithLevel(queryDesc->snapshot, portal->nestingLevel);
 			ExecutorRun(queryDesc, direction, (uint64) count,
 						portal->run_once);
 			nprocessed = queryDesc->estate->es_processed;
@@ -1137,8 +1137,8 @@ PortalRunUtility(Portal portal, PlannedStmt *pstmt,
 			portal->holdSnapshot = snapshot;
 		}
 		/* In any case, make the snapshot active and remember it in portal */
-		PushActiveSnapshot(snapshot);
-		/* PushActiveSnapshot might have copied the snapshot */
+		PushActiveSnapshotWithLevel(snapshot, portal->nestingLevel);
+		/* PushActiveSnapshotWithLevel might have copied the snapshot */
 		portal->portalSnapshot = GetActiveSnapshot();
 	}
 	else
@@ -1704,7 +1704,7 @@ DoPortalRewind(Portal portal)
 	queryDesc = portal->queryDesc;
 	if (queryDesc)
 	{
-		PushActiveSnapshot(queryDesc->snapshot);
+		PushActiveSnapshotWithLevel(queryDesc->snapshot, portal->nestingLevel);
 		ExecutorRewind(queryDesc);
 		PopActiveSnapshot();
 	}
@@ -1785,7 +1785,7 @@ EnsurePortalSnapshotExists(void)
 	Assert(portal->portalSnapshot == NULL);
 
 	/* Create a new snapshot and make it active */
-	PushActiveSnapshot(GetTransactionSnapshot());
-	/* PushActiveSnapshot might have copied the snapshot */
+	PushActiveSnapshotWithLevel(GetTransactionSnapshot(), portal->nestingLevel);
+	/* PushActiveSnapshotWithLevel might have copied the snapshot */
 	portal->portalSnapshot = GetActiveSnapshot();
 }
diff --git a/src/backend/utils/mmgr/portalmem.c b/src/backend/utils/mmgr/portalmem.c
index 5c30e141f5..89b28e26c4 100644
--- a/src/backend/utils/mmgr/portalmem.c
+++ b/src/backend/utils/mmgr/portalmem.c
@@ -210,6 +210,7 @@ CreatePortal(const char *name, bool allowDup, bool dupSilent)
 	portal->cleanup = PortalCleanup;
 	portal->createSubid = GetCurrentSubTransactionId();
 	portal->activeSubid = portal->createSubid;
+	portal->nestingLevel = GetCurrentTransactionNestLevel();
 	portal->strategy = PORTAL_MULTI_QUERY;
 	portal->cursorOptions = CURSOR_OPT_NO_SCROLL;
 	portal->atStart = true;
@@ -940,7 +941,8 @@ PortalErrorCleanup(void)
 void
 AtSubCommit_Portals(SubTransactionId mySubid,
 					SubTransactionId parentSubid,
-					ResourceOwner parentXactOwner)
+					ResourceOwner parentXactOwner,
+					int parentNestingLevel)
 {
 	HASH_SEQ_STATUS status;
 	PortalHashEnt *hentry;
@@ -954,6 +956,7 @@ AtSubCommit_Portals(SubTransactionId mySubid,
 		if (portal->createSubid == mySubid)
 		{
 			portal->createSubid = parentSubid;
+			portal->nestingLevel = parentNestingLevel;
 			if (portal->resowner)
 				ResourceOwnerNewParent(portal->resowner, parentXactOwner);
 		}
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 2968c7f7b7..3a46277dd8 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -670,18 +670,30 @@ FreeSnapshot(Snapshot snapshot)
 
 /*
  * PushActiveSnapshot
+ *		Wrapper for PushActiveSnapshotWithLevel()
+ */
+void
+PushActiveSnapshot(Snapshot snap)
+{
+	PushActiveSnapshotWithLevel(snap, GetCurrentTransactionNestLevel());
+}
+
+/*
+ * PushActiveSnapshotWithLevel
  *		Set the given snapshot as the current active snapshot
+ *		Set the given as_level to the given snapshot
  *
  * If the passed snapshot is a statically-allocated one, or it is possibly
  * subject to a future command counter update, create a new long-lived copy
  * with active refcount=1.  Otherwise, only increment the refcount.
  */
 void
-PushActiveSnapshot(Snapshot snap)
+PushActiveSnapshotWithLevel(Snapshot snap, int as_level)
 {
 	ActiveSnapshotElt *newactive;
 
 	Assert(snap != InvalidSnapshot);
+	Assert(ActiveSnapshot == NULL || as_level >= ActiveSnapshot->as_level);
 
 	newactive = MemoryContextAlloc(TopTransactionContext, sizeof(ActiveSnapshotElt));
 
@@ -695,7 +707,7 @@ PushActiveSnapshot(Snapshot snap)
 		newactive->as_snap = snap;
 
 	newactive->as_next = ActiveSnapshot;
-	newactive->as_level = GetCurrentTransactionNestLevel();
+	newactive->as_level = as_level;
 
 	newactive->as_snap->active_count++;
 
diff --git a/src/include/utils/portal.h b/src/include/utils/portal.h
index 2e5bbdd0ec..54b9f4cda5 100644
--- a/src/include/utils/portal.h
+++ b/src/include/utils/portal.h
@@ -130,6 +130,7 @@ typedef struct PortalData
 	 */
 	SubTransactionId createSubid;	/* the creating subxact */
 	SubTransactionId activeSubid;	/* the last subxact with activity */
+	int         nestingLevel;       /* transaction nesting depth */
 
 	/* The query or queries the portal will execute */
 	const char *sourceText;		/* text of query (as of 8.4, never NULL) */
@@ -219,7 +220,8 @@ extern void AtCleanup_Portals(void);
 extern void PortalErrorCleanup(void);
 extern void AtSubCommit_Portals(SubTransactionId mySubid,
 								SubTransactionId parentSubid,
-								ResourceOwner parentXactOwner);
+								ResourceOwner parentXactOwner,
+								int parentNestingLevel);
 extern void AtSubAbort_Portals(SubTransactionId mySubid,
 							   SubTransactionId parentSubid,
 							   ResourceOwner myXactOwner,
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index 44539fe15a..142326950e 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -114,6 +114,7 @@ extern void InvalidateCatalogSnapshot(void);
 extern void InvalidateCatalogSnapshotConditionally(void);
 
 extern void PushActiveSnapshot(Snapshot snapshot);
+extern void PushActiveSnapshotWithLevel(Snapshot snapshot, int as_level);
 extern void PushCopiedSnapshot(Snapshot snapshot);
 extern void UpdateActiveSnapshotCommandId(void);
 extern void PopActiveSnapshot(void);
diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out
index 9da74876e1..55c06bb32d 100644
--- a/src/test/regress/expected/portals.out
+++ b/src/test/regress/expected/portals.out
@@ -1536,3 +1536,21 @@ fetch backward all in c2;
 (3 rows)
 
 rollback;
+-- check re-establishing the portal snapshot with the correct as_level
+create table tst1 (a int primary key);
+insert into tst1 values (1),(2);
+create table tst2 (a int);
+insert into tst2 values (1);
+DO $$
+BEGIN
+  FOR i IN 1..2 LOOP
+    BEGIN
+      INSERT INTO tst1 (a) VALUES (i);
+       exception when unique_violation then update tst2 set a = i;
+     COMMIT;
+    END;
+  END LOOP;
+END;
+$$;
+drop table tst1;
+drop table tst2;
diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql
index eadf6ed942..0a967546a4 100644
--- a/src/test/regress/sql/portals.sql
+++ b/src/test/regress/sql/portals.sql
@@ -581,3 +581,24 @@ declare c2 scroll cursor for select generate_series(1,3) as g;
 fetch all in c2;
 fetch backward all in c2;
 rollback;
+
+-- check re-establishing the portal snapshot with the correct as_level
+create table tst1 (a int primary key);
+insert into tst1 values (1),(2);
+create table tst2 (a int);
+insert into tst2 values (1);
+
+DO $$
+BEGIN
+  FOR i IN 1..2 LOOP
+    BEGIN
+      INSERT INTO tst1 (a) VALUES (i);
+       exception when unique_violation then update tst2 set a = i;
+     COMMIT;
+    END;
+  END LOOP;
+END;
+$$;
+
+drop table tst1;
+drop table tst2;