Skip to content

Add xact support: bugfix #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 6, 2018
Merged

Add xact support: bugfix #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c037953
Fix HTAB name setting in makePackHTAB()
CherkashinSergey Aug 3, 2018
f731e68
Fix package state rollback
CherkashinSergey Aug 5, 2018
054e746
Fix tests
CherkashinSergey Aug 5, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions expected/pg_variables_trans.out
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1758,11 +1758,11 @@ SELECT pgv_free();

(1 row)

SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;
package
---------
vars2
vars
vars2
(2 rows)

SELECT * FROM pgv_list() ORDER BY package, name;
Expand All @@ -1771,11 +1771,11 @@ SELECT * FROM pgv_list() ORDER BY package, name;
(0 rows)

RELEASE sp5;
SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;
package
---------
vars2
vars
vars2
(2 rows)

SELECT * FROM pgv_list() ORDER BY package, name;
Expand All @@ -1784,7 +1784,7 @@ SELECT * FROM pgv_list() ORDER BY package, name;
(0 rows)

RELEASE sp4;
SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;
package
---------
vars
Expand All @@ -1796,7 +1796,7 @@ SELECT * FROM pgv_list() ORDER BY package, name;
(0 rows)

COMMIT;
SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;
package
---------
(0 rows)
Expand Down
101 changes: 49 additions & 52 deletions pg_variables.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ static void removeFromChangedVars(Package *package);

/* Constructors */
static void makePackHTAB(Package *package, bool is_trans);

static inline ChangedObject *
makeChangedObject(TransObject *object, MemoryContext ctx);

#define CHECK_ARGS_FOR_NULL() \
do { \
Expand Down Expand Up @@ -1247,8 +1248,7 @@ static void
makePackHTAB(Package *package, bool is_trans)
{
HASHCTL ctl;
char key[NAMEDATALEN],
hash_name[BUFSIZ];
char hash_name[BUFSIZ];

if (is_trans)
package->hctxTransact = AllocSetContextCreate(ModuleContext,
Expand All @@ -1260,7 +1260,7 @@ makePackHTAB(Package *package, bool is_trans)
ALLOCSET_DEFAULT_SIZES);

snprintf(hash_name, BUFSIZ, "%s variables hash for package \"%s\"",
is_trans ? "Transactional" : "Regular", key);
is_trans ? "Transactional" : "Regular", package->transObject.name);
Copy link
Contributor

@za-arthur za-arthur Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут можно использовать макрос GetName().

Copy link
Collaborator Author

@CherkashinSergey CherkashinSergey Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вообще да. Я уже и забыл про наличие макросов =)
Закоммитить? Или сам?

Copy link
Contributor

@za-arthur za-arthur Aug 6, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Закоммитил

ctl.keysize = NAMEDATALEN;
ctl.entrysize = sizeof(Variable);
ctl.hcxt = (is_trans ? package->hctxTransact : package->hctxRegular);
Expand Down Expand Up @@ -1677,6 +1677,9 @@ rollbackSavepoint(TransObject *object, TransObjectType type)
/* Restore regular vars HTAB */
makePackHTAB((Package *) object, false);
}
else
/* Pass current state to parent level */
releaseSavepoint(object, TRANS_PACKAGE);
}
else
{
Expand All @@ -1696,39 +1699,57 @@ static void
releaseSavepoint(TransObject *object, TransObjectType type)
{
dlist_head *states;

Assert(GetActualState(object)->level == GetCurrentTransactionNestLevel());
states = &object->states;

/* Object existed in parent transaction */
if (dlist_has_next(states, dlist_head_node(states)))
/* Mark object as changed in parent transaction... */
if (!dlist_is_empty(changesStack) /* ...if there is an upper level... */
/* ...and object is not yet in list of that level changes. */
&& !isObjectChangedInUpperTrans(object))
{
TransState *stateToDelete;
dlist_node *nodeToDelete;
ChangedObject *co_new;
ChangesStackNode *csn;

/* Remove previous state */
nodeToDelete = dlist_next_node(states, dlist_head_node(states));
stateToDelete = dlist_container(TransState, node, nodeToDelete);
removeState(object, type, stateToDelete);
}
/*
* Impossible to push in upper list existing node
* because it was created in another context
*/
csn = dlist_head_element(ChangesStackNode, node, changesStack);
co_new = makeChangedObject(object, csn->ctx);
dlist_push_head(type == TRANS_PACKAGE ? csn->changedPacksList :
csn->changedVarsList,
&co_new->node);

/*
* Object has no more previous states and can be completely removed if
* necessary
*/
if (!GetActualState(object)->is_valid &&
!dlist_has_next(states, dlist_head_node(states)))
{
removeObject(object, type);
}
/* Change subxact level due to release */
else
{
TransState *state;
states = &object->states;

state = GetActualState(object);
state->level--;
/* If object existed in parent transaction... */
if (dlist_has_next(states, dlist_head_node(states)))
{
TransState *stateToDelete;
dlist_node *nodeToDelete;

/* ...remove its previous state */
nodeToDelete = dlist_next_node(states, dlist_head_node(states));
stateToDelete = dlist_container(TransState, node, nodeToDelete);
removeState(object, type, stateToDelete);
}

/*
* Object has no more previous states and can be completely removed if
* necessary
*/
if (!GetActualState(object)->is_valid &&
!dlist_has_next(states, dlist_head_node(states)))
{
removeObject(object, type);
return;
}
}

/* Change subxact level due to release */
GetActualState(object)->level--;
}

/*
Expand Down Expand Up @@ -1961,31 +1982,7 @@ processChanges(Action action)
GetActualState(variable)->is_valid = false;
}

/* Did this object change at parent level? */
if (dlist_is_empty(changesStack) ||
isObjectChangedInUpperTrans(object))
{
/* We just have to drop previous state */
releaseSavepoint(object, i ? TRANS_VARIABLE : TRANS_PACKAGE);
}
else
{
/* Mark object as changed at parent level */
ChangedObject *co_new;
ChangesStackNode *csn;

/*
* Impossible to push in upper list existing node
* because it was created in another context
*/
csn = dlist_head_element(ChangesStackNode, node, changesStack);
co_new = makeChangedObject(object, csn->ctx);
dlist_push_head(i ? csn->changedVarsList :
csn->changedPacksList, &co_new->node);

/* Change subxact level due to release */
GetActualState(object)->level--;
}
releaseSavepoint(object, i ? TRANS_VARIABLE : TRANS_PACKAGE);
break;
}
}
Expand Down
8 changes: 4 additions & 4 deletions sql/pg_variables_trans.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -441,16 +441,16 @@ SELECT pgv_set('vars2', 'trans2', 'trans2 variable exists'::text, true);
SAVEPOINT sp4;
SAVEPOINT sp5;
SELECT pgv_free();
SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;
SELECT * FROM pgv_list() ORDER BY package, name;
RELEASE sp5;
SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;
SELECT * FROM pgv_list() ORDER BY package, name;
RELEASE sp4;
SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;
SELECT * FROM pgv_list() ORDER BY package, name;
COMMIT;
SELECT package FROM pgv_stats();
SELECT package FROM pgv_stats() ORDER BY package;

BEGIN;
SELECT pgv_set('vars', 'trans1', 'package created'::text, true);
Expand Down