Merge pull request #9 from CherkashinSergey/add_xact_support

Add xact support optimization

Author: funbringer

README.md

@@ -335,6 +335,15 @@ SELECT pgv_get('pack','var_int', NULL::int);
 ERROR:  unrecognized variable "var_int"
 COMMIT;
 ```
+Also you cannot undo removing variable by `ROLLBACK`:
+```sql
+SELECT pgv_set('pack', 'var_int', 122, true);
+BEGIN;
+SELECT pgv_free();
+ROLLBACK;
+SELECT pgv_get('pack', 'var_int', NULL::int);
+ERROR:  unrecognized package "pack"
+```
 If you created transactional variable once, you should use flag `is_transactional`
 every time when you want to change variable value by functions `pgv_set()`,
 `pgv_insert()` and deprecated setters (i.e. `pgv_set_int()`). If you try to

pg_variables.c

@@ -76,7 +76,8 @@ static HashVariableEntry *createVariableInternal(HashPackageEntry *package,
 												 text *name, Oid typid,
 												 bool is_transactional);
 static void createSavepoint(HashPackageEntry *package, HashVariableEntry *variable);
-static bool isVarChangedInTrans(HashVariableEntry *variable);
+static bool isVarChangedInCurrentTrans(HashVariableEntry *variable);
+static bool isVarChangedInUpperTrans(HashVariableEntry *variable);
 static void addToChangedVars(HashPackageEntry *package, HashVariableEntry *variable);
 
 #define CHECK_ARGS_FOR_NULL() \
@@ -100,12 +101,11 @@ static HashPackageEntry *LastPackage = NULL;
 static HashVariableEntry *LastVariable = NULL;
 
 /*
- * List of variables, changed in top level transaction. Used to limit
+ * Stack of lists of variables, changed in each transaction level. Used to limit
  * number of proceeded variables on start of transaction.
  */
-static dlist_head *changedVars = NULL;
-static MemoryContext changedVarsContext = NULL;
 static dlist_head *changedVarsStack = NULL;
+static MemoryContext changedVarsContext = NULL;
 #define get_actual_changed_vars_list() \
 	((dlist_head_element(ChangedVarsStackNode, node, changedVarsStack))-> \
 						 changedVarsList)
@@ -621,7 +621,7 @@ variable_insert(PG_FUNCTION_ARGS)
 					 errmsg("variable \"%s\" already created as %sTRANSACTIONAL",
 							key, LastVariable->is_transactional ? "" : "NOT ")));
 		}
-		if (!isVarChangedInTrans(variable) && variable->is_transactional)
+		if (!isVarChangedInCurrentTrans(variable) && variable->is_transactional)
 		{
 			createSavepoint(package, variable);
 			addToChangedVars(package, variable);
@@ -707,7 +707,7 @@ variable_update(PG_FUNCTION_ARGS)
 	else
 		variable = LastVariable;
 
-	if (variable->is_transactional && !isVarChangedInTrans(variable))
+	if (variable->is_transactional && !isVarChangedInCurrentTrans(variable))
 	{
 		createSavepoint(package, variable);
 		addToChangedVars(package, variable);
@@ -785,7 +785,7 @@ variable_delete(PG_FUNCTION_ARGS)
 	else
 		variable = LastVariable;
 
-	if (variable->is_transactional && !isVarChangedInTrans(variable))
+	if (variable->is_transactional && !isVarChangedInCurrentTrans(variable))
 	{
 		createSavepoint(package, variable);
 		addToChangedVars(package, variable);
@@ -1220,7 +1220,7 @@ remove_packages(PG_FUNCTION_ARGS)
 
 	packagesHash = NULL;
 	ModuleContext = NULL;
-	changedVars = NULL;
+	changedVarsStack = NULL;
 
 	PG_RETURN_VOID();
 }
@@ -1663,7 +1663,7 @@ createVariableInternal(HashPackageEntry *package, text *name, Oid typid,
 		 * For each transaction level there should be own savepoint.
 		 * New value should be stored in a last state.
 		 */
-		if (variable->is_transactional && !isVarChangedInTrans(variable))
+		if (variable->is_transactional && !isVarChangedInCurrentTrans(variable))
 		{
 			createSavepoint(package, variable);
 		}
@@ -1769,6 +1769,11 @@ releaseSavepoint(HashVariableEntry *variable)
 		dlist_delete(nodeToDelete);
 		pfree(historyEntryToDelete);
 	}
+	/*
+	 * If variable was changed in subtransaction, so it is considered it
+	 * was changed in parent transaction.
+	 */
+	(get_actual_value(variable)->level)--;
 }
 
 /*
@@ -1792,22 +1797,32 @@ rollbackSavepoint(HashPackageEntry *package, HashVariableEntry *variable)
  * Check if variable was changed in current transaction level
  */
 static bool
-isVarChangedInTrans(HashVariableEntry *variable)
+isVarChangedInCurrentTrans(HashVariableEntry *variable)
 {
-	dlist_iter iter;
-	dlist_head *changedVars;
+	ValueHistoryEntry   *var_state;
 
 	if (!changedVarsStack)
 		return false;
 
-	changedVars = get_actual_changed_vars_list();
-	dlist_foreach(iter, changedVars)
-	{
-		ChangedVarsNode *cvn;
+	var_state = get_actual_value(variable);
+	return (var_state->level == GetCurrentTransactionNestLevel());
+}
 
-		cvn = dlist_container(ChangedVarsNode, node, iter.cur);
-		if (cvn->variable == variable)
-			return true;
+/*
+ * Check if variable was changed in parent transaction level
+ */
+static bool
+isVarChangedInUpperTrans(HashVariableEntry *variable)
+{
+	ValueHistoryEntry   *var_state,
+						*var_prev_state;
+
+	var_state = get_actual_value(variable);
+
+	if(dlist_has_next(&variable->data, &var_state->node))
+	{
+		var_prev_state = get_history_entry(var_state->node.next);
+		return (var_prev_state->level == (GetCurrentTransactionNestLevel() - 1));
 	}
 	return false;
 }
@@ -1890,12 +1905,12 @@ popChangedVarsStack()
 {
 	if (changedVarsStack)
 	{
-		ChangedVarsStackNode *cvse;
+		ChangedVarsStackNode *cvsn;
 
 		Assert(!dlist_is_empty(changedVarsStack));
-		cvse = dlist_container(ChangedVarsStackNode, node,
+		cvsn = dlist_container(ChangedVarsStackNode, node,
 							   dlist_pop_head_node(changedVarsStack));
-		MemoryContextDelete(cvse->ctx);
+		MemoryContextDelete(cvsn->ctx);
 		if (dlist_is_empty(changedVarsStack))
 		{
 			MemoryContextDelete(changedVarsContext);
@@ -1905,6 +1920,20 @@ popChangedVarsStack()
 	}
 }
 
+/*
+ * Initialize an instance of ChangedVarsNode datatype
+ */
+static inline ChangedVarsNode *
+initChangedVarsNode(MemoryContext ctx, HashPackageEntry *package, HashVariableEntry *variable)
+{
+	ChangedVarsNode *cvn;
+
+	cvn = MemoryContextAllocZero(ctx, sizeof(ChangedVarsNode));
+	cvn->package = package;
+	cvn->variable = variable;
+	return cvn;
+}
+
 /*
  * Add a variable to list of changed vars in current transaction level
  */
@@ -1925,20 +1954,19 @@ addToChangedVars(HashPackageEntry *package, HashVariableEntry *variable)
 
 	Assert(changedVarsStack && changedVarsContext);
 
-	if (!isVarChangedInTrans(variable))
+	if (!isVarChangedInCurrentTrans(variable))
 	{
 		ChangedVarsNode *cvn;
 
 		cvsn = dlist_head_element(ChangedVarsStackNode, node, changedVarsStack);
-		cvn = MemoryContextAllocZero(cvsn->ctx, sizeof(ChangedVarsNode));
-		cvn->package = package;
-		cvn->variable = variable;
+		cvn = initChangedVarsNode(cvsn->ctx, package, variable);
 		dlist_push_head(cvsn->changedVarsList, &cvn->node);
+		get_actual_value(cvn->variable)->level = GetCurrentTransactionNestLevel();
 	}
 }
 
 /*
- * If variable was chenged in some subtransaction, it is considered that it was
+ * If variable was changed in some subtransaction, it is considered that it was
  * changed in parent transaction. So it is important to add this variable to
  * list of changes of parent transaction. But if var was already changed in
  * upper level, it has savepoint there, so we need to release it.
@@ -1957,13 +1985,25 @@ levelUpOrRelease()
 		Assert(!dlist_is_empty(changedVarsStack));
 		dlist_foreach(iter, bottom_list->changedVarsList)
 		{
-			ChangedVarsNode *cvn;
+			ChangedVarsNode *cvn_old;
 
-			cvn = dlist_container(ChangedVarsNode, node, iter.cur);
-			if (isVarChangedInTrans(cvn->variable))
-				releaseSavepoint(cvn->variable);
+			cvn_old = dlist_container(ChangedVarsNode, node, iter.cur);
+			if (isVarChangedInUpperTrans(cvn_old->variable))
+				releaseSavepoint(cvn_old->variable);
 			else
-				addToChangedVars(cvn->package, cvn->variable);
+			{
+				ChangedVarsNode *cvn_new;
+				ChangedVarsStackNode *cvsn;
+
+				/* 
+				 * Impossible to push in upper list existing node because
+				 * it was created in another context
+				 */
+				cvsn = dlist_head_element(ChangedVarsStackNode, node, changedVarsStack);
+				cvn_new = initChangedVarsNode(cvsn->ctx, cvn_old->package, cvn_old->variable);
+				dlist_push_head(cvsn->changedVarsList, &cvn_new->node);
+				(get_actual_value(cvn_new->variable)->level)--;
+			}
 		}
 		MemoryContextDelete(bottom_list->ctx);
 	}

pg_variables.h

@@ -68,6 +68,8 @@ typedef struct ValueHistoryEntry{
 		ScalarVar scalar;
 		RecordVar record;
 	}		value;
+	/* Transaction nest level of current entry */
+	int level;
 } ValueHistoryEntry;
 
 typedef dlist_head ValueHistory;
@@ -139,6 +141,8 @@ extern void insert_savepoint(HashVariableEntry *variable,
 	(&((dlist_head_element(ValueHistoryEntry, node, &variable->data))->value.scalar))
 #define get_actual_value_record(variable) \
 	(&((dlist_head_element(ValueHistoryEntry, node, &variable->data))->value.record))
+#define get_actual_value(variable) \
+	(dlist_head_element(ValueHistoryEntry, node, &variable->data))
 #define get_history_entry(node_ptr) \
 	dlist_container(ValueHistoryEntry, node, node_ptr)