From 56096ff2dc1404437999746c6b812204aefb984d Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Fri, 2 Aug 2019 19:40:55 +0300
Subject: [PATCH] txn: merge struct sql_txn into struct txn
This procedure is processed in several steps. Firstly, we add name
to struct txn_savepoint since we should be capable of operating on named
savepoints (which in turn is SQL feature). Still, anonymous (in the sense
of name absence) savepoints are also valid. Then, we add list (as
implementation of stailq) of savepoints to struct txn: it allows us to
find savepoint by its name. Finally, we patch rollback to/release
savepoint routines: for rollback tail of the list containing savepoints
is cut (but subject of rollback routine remains in the list); for
release routine we cut tail including node being released.
---
src/box/sql/vdbe.c | 84 ++++++++++--------------------------
src/box/sql/vdbe.h | 9 ----
src/box/sql/vdbeInt.h | 3 +-
src/box/sql/vdbeaux.c | 45 -------------------
src/box/txn.c | 70 ++++++++++++++++++++++++------
src/box/txn.h | 36 +++++++++++++---
test/sql/savepoints.result | 17 ++++++++
test/sql/savepoints.test.lua | 12 ++++++
8 files changed, 139 insertions(+), 137 deletions(-)
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index cbcd571efd..55ebdf6e91 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2840,13 +2840,9 @@ case OP_Count: { /* out2 */
case OP_Savepoint: {
int p1; /* Value of P1 operand */
char *zName; /* Name of savepoint */
- Savepoint *pNew;
- Savepoint *pSavepoint;
- Savepoint *pTmp;
struct txn *txn = in_txn();
- struct sql_txn *psql_txn = txn != NULL ? txn->psql_txn : NULL;
- if (psql_txn == NULL) {
+ if (txn == NULL) {
assert(!box_txn());
diag_set(ClientError, ER_NO_TRANSACTION);
goto abort_due_to_error;
@@ -2857,69 +2853,31 @@ case OP_Savepoint: {
/* Assert that the p1 parameter is valid. Also that if there is no open
* transaction, then there cannot be any savepoints.
*/
- assert(psql_txn->pSavepoint == NULL || box_txn());
+ assert(rlist_empty(&txn->savepoints) || box_txn());
assert(p1==SAVEPOINT_BEGIN||p1==SAVEPOINT_RELEASE||p1==SAVEPOINT_ROLLBACK);
if (p1==SAVEPOINT_BEGIN) {
- /* Create a new savepoint structure. */
- pNew = sql_savepoint(p, zName);
- /* Link the new savepoint into the database handle's list. */
- pNew->pNext = psql_txn->pSavepoint;
- psql_txn->pSavepoint = pNew;
+ /*
+ * Savepoint is available by its name so we don't
+ * care about object itself.
+ */
+ if (txn_savepoint_new(txn, zName) == NULL)
+ goto abort_due_to_error;
} else {
/* Find the named savepoint. If there is no such savepoint, then an
* an error is returned to the user.
*/
- for(
- pSavepoint = psql_txn->pSavepoint;
- pSavepoint && sqlStrICmp(pSavepoint->zName, zName);
- pSavepoint = pSavepoint->pNext
- );
- if (!pSavepoint) {
+ struct txn_savepoint *sv = txn_savepoint_by_name(txn, zName);
+ if (sv == NULL) {
diag_set(ClientError, ER_NO_SUCH_SAVEPOINT);
goto abort_due_to_error;
+ }
+ if (p1 == SAVEPOINT_RELEASE) {
+ txn_savepoint_release(sv);
} else {
-
- /* Determine whether or not this is a transaction savepoint. If so,
- * and this is a RELEASE command, then the current transaction
- * is committed.
- */
- int isTransaction = pSavepoint->pNext == 0;
- if (isTransaction && p1==SAVEPOINT_RELEASE) {
- if ((rc = sqlVdbeCheckFk(p, 1)) != 0)
- goto vdbe_return;
- sqlVdbeHalt(p);
- if (p->is_aborted)
- goto abort_due_to_error;
- } else {
- if (p1==SAVEPOINT_ROLLBACK)
- box_txn_rollback_to_savepoint(pSavepoint->tnt_savepoint);
- }
-
- /* Regardless of whether this is a RELEASE or ROLLBACK, destroy all
- * savepoints nested inside of the savepoint being operated on.
- */
- while (psql_txn->pSavepoint != pSavepoint) {
- pTmp = psql_txn->pSavepoint;
- psql_txn->pSavepoint = pTmp->pNext;
- /*
- * Since savepoints are stored in region, we do not
- * have to destroy them
- */
- }
-
- /* If it is a RELEASE, then destroy the savepoint being operated on
- * too. If it is a ROLLBACK TO, then set the number of deferred
- * constraint violations present in the database to the value stored
- * when the savepoint was created.
- */
- if (p1==SAVEPOINT_RELEASE) {
- assert(pSavepoint == psql_txn->pSavepoint);
- psql_txn->pSavepoint = pSavepoint->pNext;
- } else {
- txn->fk_deferred_count =
- pSavepoint->tnt_savepoint->fk_deferred_count;
- }
+ assert(p1 == SAVEPOINT_ROLLBACK);
+ if (box_txn_rollback_to_savepoint(sv) != 0)
+ goto abort_due_to_error;
}
}
@@ -2957,7 +2915,11 @@ case OP_CheckViewReferences: {
* Otherwise, raise an error with appropriate error message.
*/
case OP_TransactionBegin: {
- if (sql_txn_begin() != 0)
+ if (in_txn()) {
+ diag_set(ClientError, ER_ACTIVE_TRANSACTION);
+ goto abort_due_to_error;
+ }
+ if (txn_begin() == NULL)
goto abort_due_to_error;
p->auto_commit = false ;
break;
@@ -3013,7 +2975,7 @@ case OP_TransactionRollback: {
*/
case OP_TTransaction: {
if (!box_txn()) {
- if (sql_txn_begin() != 0)
+ if (txn_begin() == NULL)
goto abort_due_to_error;
} else {
p->anonymous_savepoint = sql_savepoint(p, NULL);
@@ -4860,7 +4822,6 @@ case OP_FkCounter: {
if (((p->sql_flags & SQL_DeferFKs) != 0 || pOp->p1 != 0) &&
!p->auto_commit) {
struct txn *txn = in_txn();
- assert(txn != NULL && txn->psql_txn != NULL);
txn->fk_deferred_count += pOp->p2;
} else {
p->nFkConstraint += pOp->p2;
@@ -4884,7 +4845,6 @@ case OP_FkIfZero: { /* jump */
if (((p->sql_flags & SQL_DeferFKs) != 0 || pOp->p1 != 0) &&
!p->auto_commit) {
struct txn *txn = in_txn();
- assert(txn != NULL && txn->psql_txn != NULL);
if (txn->fk_deferred_count == 0)
goto jump_to_p2;
} else {
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 8f16202ba4..06f2588056 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -172,15 +172,6 @@ struct SubProgram {
*/
Vdbe *sqlVdbeCreate(Parse *);
-/**
- * Allocate and initialize SQL-specific struct which completes
- * original Tarantool's txn struct using region allocator.
- *
- * @retval NULL on OOM, new psql_txn struct on success.
- **/
-struct sql_txn *
-sql_alloc_txn();
-
/**
* Prepare given VDBE to execution: initialize structs connected
* with transaction routine: autocommit mode, deferred foreign
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index f77c019fb7..34297e6787 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -454,8 +454,7 @@ u32 sqlVdbeSerialGet(const unsigned char *, u32, Mem *);
int sqlVdbeExec(Vdbe *);
int sqlVdbeList(Vdbe *);
-int
-sql_txn_begin();
+
Savepoint *
sql_savepoint(Vdbe *p,
const char *zName);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index bbbb99c704..b77fffaec2 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -76,39 +76,12 @@ sqlVdbeCreate(Parse * pParse)
return p;
}
-struct sql_txn *
-sql_alloc_txn()
-{
- struct sql_txn *txn = region_alloc_object(&fiber()->gc,
- struct sql_txn);
- if (txn == NULL) {
- diag_set(OutOfMemory, sizeof(struct sql_txn), "region",
- "struct sql_txn");
- return NULL;
- }
- txn->pSavepoint = NULL;
- return txn;
-}
-
int
sql_vdbe_prepare(struct Vdbe *vdbe)
{
assert(vdbe != NULL);
struct txn *txn = in_txn();
vdbe->auto_commit = txn == NULL;
- if (txn != NULL) {
- /*
- * If transaction has been started in Lua, then
- * sql_txn is NULL. On the other hand, it is not
- * critical, since in Lua it is impossible to
- * check FK violations, at least now.
- */
- if (txn->psql_txn == NULL) {
- txn->psql_txn = sql_alloc_txn();
- if (txn->psql_txn == NULL)
- return -1;
- }
- }
return 0;
}
@@ -1998,24 +1971,6 @@ sqlVdbeCheckFk(Vdbe * p, int deferred)
return 0;
}
-int
-sql_txn_begin()
-{
- if (in_txn()) {
- diag_set(ClientError, ER_ACTIVE_TRANSACTION);
- return -1;
- }
- struct txn *ptxn = txn_begin(false);
- if (ptxn == NULL)
- return -1;
- ptxn->psql_txn = sql_alloc_txn();
- if (ptxn->psql_txn == NULL) {
- box_txn_rollback();
- return -1;
- }
- return 0;
-}
-
Savepoint *
sql_savepoint(MAYBE_UNUSED Vdbe *p, const char *zName)
{
diff --git a/src/box/txn.c b/src/box/txn.c
index b57240846f..1002c21368 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -221,8 +221,8 @@ txn_begin()
txn->signature = -1;
txn->engine = NULL;
txn->engine_tx = NULL;
- txn->psql_txn = NULL;
txn->fk_deferred_count = 0;
+ rlist_create(&txn->savepoints);
txn->fiber = NULL;
fiber_set_txn(fiber(), txn);
/* fiber_on_yield is initialized by engine on demand */
@@ -734,28 +734,53 @@ box_txn_alloc(size_t size)
alignof(union natural_align));
}
-box_txn_savepoint_t *
-box_txn_savepoint()
+struct txn_savepoint *
+txn_savepoint_new(struct txn *txn, const char *name)
{
- struct txn *txn = in_txn();
- if (txn == NULL) {
- diag_set(ClientError, ER_NO_TRANSACTION);
- return NULL;
- }
+ assert(txn == in_txn());
+ size_t svp_sz = sizeof(struct txn_savepoint);
+ int name_len = name != NULL ? strlen(name) : 0;
+ svp_sz += name_len;
struct txn_savepoint *svp =
- (struct txn_savepoint *) region_alloc_object(&txn->region,
- struct txn_savepoint);
+ (struct txn_savepoint *) region_alloc(&txn->region, svp_sz);
if (svp == NULL) {
- diag_set(OutOfMemory, sizeof(*svp),
- "region", "struct txn_savepoint");
+ diag_set(OutOfMemory, svp_sz, "region", "svp");
return NULL;
}
svp->stmt = stailq_last(&txn->stmts);
svp->in_sub_stmt = txn->in_sub_stmt;
svp->fk_deferred_count = txn->fk_deferred_count;
+ if (name != NULL)
+ memcpy(svp->name, name, name_len + 1);
+ else
+ svp->name[0] = 0;
+ rlist_add_entry(&txn->savepoints, svp, link);
return svp;
}
+struct txn_savepoint *
+txn_savepoint_by_name(struct txn *txn, const char *name)
+{
+ assert(txn == in_txn());
+ struct txn_savepoint *sv;
+ rlist_foreach_entry(sv, &txn->savepoints, link) {
+ if (strcmp(sv->name, name) == 0)
+ return sv;
+ }
+ return NULL;
+}
+
+box_txn_savepoint_t *
+box_txn_savepoint()
+{
+ struct txn *txn = in_txn();
+ if (txn == NULL) {
+ diag_set(ClientError, ER_NO_TRANSACTION);
+ return NULL;
+ }
+ return txn_savepoint_new(txn, NULL);
+}
+
int
box_txn_rollback_to_savepoint(box_txn_savepoint_t *svp)
{
@@ -779,10 +804,31 @@ box_txn_rollback_to_savepoint(box_txn_savepoint_t *svp)
return -1;
}
txn_rollback_to_svp(txn, svp->stmt);
+ /* Discard from list all newer savepoints. */
+ RLIST_HEAD(discard);
+ rlist_cut_before(&discard, &txn->savepoints, &svp->link);
txn->fk_deferred_count = svp->fk_deferred_count;
return 0;
}
+void
+txn_savepoint_release(struct txn_savepoint *svp)
+{
+ struct txn *txn = in_txn();
+ assert(txn != NULL);
+ /* Make sure that savepoint hasn't been released yet. */
+ struct txn_stmt *stmt = svp->stmt == NULL ? NULL :
+ stailq_entry(svp->stmt, struct txn_stmt, next);
+ assert(stmt == NULL || (stmt->space != NULL && stmt->row != NULL));
+ (void) stmt;
+ /*
+ * Discard current savepoint alongside with all
+ * created after it savepoints.
+ */
+ RLIST_HEAD(discard);
+ rlist_cut_before(&discard, &txn->savepoints, rlist_next(&svp->link));
+}
+
static void
txn_on_stop(struct trigger *trigger, void *event)
{
diff --git a/src/box/txn.h b/src/box/txn.h
index f795cb76fb..da12feebfc 100644
--- a/src/box/txn.h
+++ b/src/box/txn.h
@@ -43,7 +43,6 @@ extern "C" {
/** box statistics */
extern struct rmean *rmean_box;
-struct Savepoint;
struct engine;
struct space;
@@ -128,15 +127,21 @@ struct txn_savepoint {
* state violating any number of deferred FK constraints.
*/
uint32_t fk_deferred_count;
+ /** Organize savepoints into linked list. */
+ struct rlist link;
+ /**
+ * Optional name of savepoint. If savepoint lacks
+ * name (i.e. anonymous savepoint available only by
+ * reference to the object), name[0] == ''. Otherwise,
+ * memory for name is reserved in the same memory chunk
+ * as struct txn_savepoint itself - name is placed
+ * right after structure (see txn_savepoint_new()).
+ */
+ char name[1];
};
extern double too_long_threshold;
-struct sql_txn {
- /** List of active SQL savepoints. */
- struct Savepoint *pSavepoint;
-};
-
/**
* An element of list of autogenerated ids, being returned as SQL
* response metadata.
@@ -220,7 +225,8 @@ struct txn {
* SQL specific property.
*/
uint32_t fk_deferred_count;
- struct sql_txn *psql_txn;
+ /** List of savepoints to find savepoint by name. */
+ struct rlist savepoints;
};
static inline bool
@@ -466,6 +472,22 @@ txn_current_stmt(struct txn *txn)
return stailq_entry(stmt, struct txn_stmt, next);
}
+/**
+ * Allocate new savepoint object using region allocator.
+ * Savepoint is allowed to be anonymous (i.e. without
+ * name).
+ */
+struct txn_savepoint *
+txn_savepoint_new(struct txn *txn, const char *name);
+
+/** Find savepoint by its name in savepoint list. */
+struct txn_savepoint *
+txn_savepoint_by_name(struct txn *txn, const char *name);
+
+/** Remove given and all newer entries from savepoint list. */
+void
+txn_savepoint_release(struct txn_savepoint *svp);
+
/**
* FFI bindings: do not throw exceptions, do not accept extra
* arguments
diff --git a/test/sql/savepoints.result b/test/sql/savepoints.result
index 78957173eb..b5a0b7f464 100644
--- a/test/sql/savepoints.result
+++ b/test/sql/savepoints.result
@@ -58,6 +58,23 @@ release_sv();
box.commit();
---
...
+release_sv_2 = function()
+ box.begin()
+ box.execute('SAVEPOINT t1;')
+ box.execute('SAVEPOINT t2;')
+ box.execute('SAVEPOINT t3;')
+ box.execute('RELEASE SAVEPOINT t2;')
+ local _, err = box.execute('ROLLBACK TO t1;')
+ assert(err == nil)
+end;
+---
+...
+release_sv_2();
+---
+...
+box.commit();
+---
+...
release_sv_fail = function()
box.begin()
box.execute('SAVEPOINT t1;')
diff --git a/test/sql/savepoints.test.lua b/test/sql/savepoints.test.lua
index d96b04600b..a4312c114c 100644
--- a/test/sql/savepoints.test.lua
+++ b/test/sql/savepoints.test.lua
@@ -31,6 +31,18 @@ end;
release_sv();
box.commit();
+release_sv_2 = function()
+ box.begin()
+ box.execute('SAVEPOINT t1;')
+ box.execute('SAVEPOINT t2;')
+ box.execute('SAVEPOINT t3;')
+ box.execute('RELEASE SAVEPOINT t2;')
+ local _, err = box.execute('ROLLBACK TO t1;')
+ assert(err == nil)
+end;
+release_sv_2();
+box.commit();
+
release_sv_fail = function()
box.begin()
box.execute('SAVEPOINT t1;')
--
GitLab