diff --git a/README.md b/README.md index d017a32..862153a 100644 --- a/README.md +++ b/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 diff --git a/pg_variables.c b/pg_variables.c index 349ea1a..1fc79a2 100755 --- a/pg_variables.c +++ b/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); } diff --git a/pg_variables.h b/pg_variables.h index 383e2c7..1357244 100755 --- a/pg_variables.h +++ b/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)