Optimize objects removal in subtransaction rollback

postgrespro · Feb 27, 2019 · 21df5c3 · 21df5c3
1 parent b53645f
commit 21df5c3
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 13 deletions.
diff --git a/expected/pg_variables_trans.out b/expected/pg_variables_trans.out
@@ -1881,7 +1881,7 @@ RELEASE comm;
 SELECT pgv_get('vars', 'any1',NULL::text);
 ERROR:  unrecognized variable "any1"
 COMMIT;
--- Test for PGPRO-2440
+-- Tests for PGPRO-2440
 SELECT pgv_insert('vars3', 'r3', row(1 :: integer, NULL::varchar), true);
  pgv_insert 
 ------------
@@ -1909,6 +1909,32 @@ SELECT pgv_delete('vars3', 'r3', 3);
  t
 (1 row)
 
+BEGIN;
+SELECT pgv_set('vars1', 't1', ''::text);
+ pgv_set 
+---------
+
+(1 row)
+
+SELECT pgv_set('vars2', 't2', ''::text, true);
+ pgv_set 
+---------
+
+(1 row)
+
+SAVEPOINT sp1;
+SAVEPOINT sp2;
+SELECT pgv_free();
+ pgv_free 
+----------
+
+(1 row)
+
+ERROR;
+ERROR:  syntax error at or near "ERROR"
+LINE 1: ERROR;
+        ^
+COMMIT;
 SELECT pgv_free();
  pgv_free 
 ----------

diff --git a/pg_variables.c b/pg_variables.c
@@ -1672,7 +1672,6 @@ removeObject(TransObject *object, TransObjectType type)
 		package = (Package *) object;
 
 		/* Regular variables had already removed */
-		//Here we should think, when regular HTAB should be removed
 		if (package->hctxRegular)
 			MemoryContextDelete(package->hctxRegular);
 		if (package->hctxTransact)
@@ -1696,10 +1695,11 @@ removeObject(TransObject *object, TransObjectType type)
 	hash_search(hash, object->name, HASH_REMOVE, &found);
 
 	/* Remove package if it became empty */
-	if (type == TRANS_VARIABLE &&
-		isObjectChangedInCurrentTrans(&package->transObject) &&
-		isPackageEmpty(package))
+	if (type == TRANS_VARIABLE && isPackageEmpty(package))
+	{
+		Assert(isObjectChangedInCurrentTrans(&package->transObject));
 		GetActualState(&package->transObject)->is_valid = false;
+	}
 
 	resetVariablesCache(true);
 }
@@ -1740,14 +1740,8 @@ rollbackSavepoint(TransObject *object, TransObjectType type)
 	state = GetActualState(object);
 	if (type == TRANS_PACKAGE)
 	{
-		if (!state->is_valid)
+		if (!state->is_valid && !isPackageEmpty((Package *)object))
 		{
-			if (isPackageEmpty((Package *)object))
-			{
-				removeObject(object, TRANS_PACKAGE);
-				return;
-			}
-
 			if (dlist_has_next(&object->states, &state->node))
 			{
 				dlist_pop_head_node(&object->states);

diff --git a/sql/pg_variables_trans.sql b/sql/pg_variables_trans.sql
@@ -482,7 +482,7 @@ RELEASE comm;
 SELECT pgv_get('vars', 'any1',NULL::text);
 COMMIT;
 
--- Test for PGPRO-2440
+-- Tests for PGPRO-2440
 SELECT pgv_insert('vars3', 'r3', row(1 :: integer, NULL::varchar), true);
 BEGIN;
 SELECT pgv_insert('vars3', 'r3', row(2 :: integer, NULL::varchar), true);
@@ -491,4 +491,13 @@ SELECT pgv_insert('vars3', 'r3', row(3 :: integer, NULL::varchar), true);
 COMMIT;
 SELECT pgv_delete('vars3', 'r3', 3);
 
+BEGIN;
+SELECT pgv_set('vars1', 't1', ''::text);
+SELECT pgv_set('vars2', 't2', ''::text, true);
+SAVEPOINT sp1;
+SAVEPOINT sp2;
+SELECT pgv_free();
+ERROR;
+COMMIT;
+
 SELECT pgv_free();