diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index cbcd571efdc54f0367aceff96dab10589b41d3e5..55ebdf6e91b37dc81d48a929482bf617aee297c3 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 8f16202ba47499c9244da4e26e3903ce94e150f0..06f25880560317d87a98d2a5b82caf91919835a4 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 f77c019fb7677e7bdea99597dc41c2fd2379ae16..34297e678737348c039973a601c2cabb595e8db8 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 bbbb99c7048d780faadaa36c97eb03c915821641..b77fffaec2cafa768af0a15456da5c17c8d4d243 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 b57240846fb3166804dce0872f7e6f62140d34a3..1002c2136878f13becc5f56019d3c236e56faac5 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 f795cb76fb9d82f6908cd566f747051fc4dc2049..da12feebfc248d03eefab86ffef79a561002c23d 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 78957173eb64ab7dd444c621b16db78985829671..b5a0b7f4640f9642fe15f8477e2fbefd633ddf15 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 d96b04600bdbeda0a05b9e8092fe36cc3529ac40..a4312c114cdabeb0b4f7ee37217629a4885a0893 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;')