From 8b8b689596a9d54ec5f5250236c4e5e71edbe8e4 Mon Sep 17 00:00:00 2001
From: Nikita Pettik <korablev@tarantool.org>
Date: Fri, 16 Aug 2019 21:13:52 +0300
Subject: [PATCH] txn: erase old savepoint in case of name collision
Name duplicates are allowed for savepoints (both in our SQL
implementation and in ANSI specification). ANSI SQL states that previous
savepoint should be deleted. What is more, our doc confirms this fact
and says that "...it is released before the new savepoint is set."
Unfortunately, it's not true - currently old savepoint remains in the
list. For instance:
SAVEPOINT t;
SAVEPOINT t;
RELEASE SAVEPOINT t;
RELEASE SAVEPOINT t; -- no error is raised
Let's fix this and remove old savepoint from the list.
---
src/box/txn.c | 14 ++++++++++++--
test/sql/savepoints.result | 24 ++++++++++++++++++++++++
test/sql/savepoints.test.lua | 19 +++++++++++++++++++
3 files changed, 55 insertions(+), 2 deletions(-)
diff --git a/src/box/txn.c b/src/box/txn.c
index 1002c21368..38b1b595f0 100644
--- a/src/box/txn.c
+++ b/src/box/txn.c
@@ -750,10 +750,20 @@ txn_savepoint_new(struct txn *txn, const char *name)
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)
+ if (name != NULL) {
+ /*
+ * If savepoint with given name already exists,
+ * erase it from the list. This has to be done
+ * in accordance with ANSI SQL compliance.
+ */
+ struct txn_savepoint *old_svp =
+ txn_savepoint_by_name(txn, name);
+ if (old_svp != NULL)
+ rlist_del(&old_svp->link);
memcpy(svp->name, name, name_len + 1);
- else
+ } else {
svp->name[0] = 0;
+ }
rlist_add_entry(&txn->savepoints, svp, link);
return svp;
}
diff --git a/test/sql/savepoints.result b/test/sql/savepoints.result
index b5a0b7f464..e48db302a8 100644
--- a/test/sql/savepoints.result
+++ b/test/sql/savepoints.result
@@ -95,3 +95,27 @@ release_sv_fail();
box.commit();
---
...
+-- Make sure that if the current transaction has a savepoint
+-- with the same name, the old savepoint is deleted and
+-- a new one is set. Note that no error should be raised.
+--
+collision_sv_2 = function()
+ box.begin()
+ box.execute('SAVEPOINT t1;')
+ box.execute('SAVEPOINT t2;')
+ local _,err = box.execute('SAVEPOINT t1;')
+ assert(err == nil)
+ box.execute('RELEASE SAVEPOINT t1;')
+ local _,err = box.execute('RELEASE SAVEPOINT t1;')
+ assert(err ~= nil)
+ local _, err = box.execute('ROLLBACK TO t2;')
+ assert(err == nil)
+end;
+---
+...
+collision_sv_2();
+---
+...
+box.commit();
+---
+...
diff --git a/test/sql/savepoints.test.lua b/test/sql/savepoints.test.lua
index a4312c114c..99622a4db0 100644
--- a/test/sql/savepoints.test.lua
+++ b/test/sql/savepoints.test.lua
@@ -56,3 +56,22 @@ release_sv_fail = function()
end;
release_sv_fail();
box.commit();
+
+-- Make sure that if the current transaction has a savepoint
+-- with the same name, the old savepoint is deleted and
+-- a new one is set. Note that no error should be raised.
+--
+collision_sv_2 = function()
+ box.begin()
+ box.execute('SAVEPOINT t1;')
+ box.execute('SAVEPOINT t2;')
+ local _,err = box.execute('SAVEPOINT t1;')
+ assert(err == nil)
+ box.execute('RELEASE SAVEPOINT t1;')
+ local _,err = box.execute('RELEASE SAVEPOINT t1;')
+ assert(err ~= nil)
+ local _, err = box.execute('ROLLBACK TO t2;')
+ assert(err == nil)
+end;
+collision_sv_2();
+box.commit();
--
GitLab