[tarantool-patches] Re: [PATCH 3/4] sql: allow SAVEPOINT statement outside transaction
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri May 4 17:12:32 MSK 2018
Hello. Thanks for contributing!
See below 3 comments.
On 03/05/2018 21:49, Nikita Pettik wrote:
> Before this patch, usage of SAVEPOINT statement outside transaction or
> inside transaction started in Lua, led to assertion fault.
> Now, failed assert is substituted with checks to test transaction status.
>
> Closes #3313
> ---
> src/box/sql/vdbe.c | 8 +++++-
> test/sql/gh-3313-savepoints-outside-txn.result | 32 ++++++++++++++++++++++++
> test/sql/gh-3313-savepoints-outside-txn.test.lua | 18 +++++++++++++
> 3 files changed, 57 insertions(+), 1 deletion(-)
> create mode 100644 test/sql/gh-3313-savepoints-outside-txn.result
> create mode 100644 test/sql/gh-3313-savepoints-outside-txn.test.lua
>
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 1192fc399..6ea04901c 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2867,7 +2867,13 @@ case OP_Savepoint: {
> Savepoint *pTmp;
> struct sql_txn *psql_txn = p->psql_txn;
>
> - assert(psql_txn);
> + if (psql_txn == NULL) {
> + assert(!box_txn());
> + sqlite3VdbeError(p, "cannot process savepoint: "
> + "there is no active transaction");
> + rc = SQLITE_ERROR;
> + goto abort_due_to_error;
> + }
1. Lets use diag_set(ClientError, ER_SAVEPOINT_NO_TRANSACTION) and return TARANTOOL_ERROR.
It must behave like box.savepoint.
> diff --git a/test/sql/gh-3313-savepoints-outside-txn.result b/test/sql/gh-3313-savepoints-outside-txn.result
> new file mode 100644
> index 000000000..702d3e815
> --- /dev/null
> +++ b/test/sql/gh-3313-savepoints-outside-txn.result
> @@ -0,0 +1,32 @@
> +test_run = require('test_run').new()
> +---
> +...
> +test_run:cmd("setopt delimiter ';'")
2. Why do you need here a special delimiter? Your test does not
contain multi-line statements.
> +---
> +- true
> +...
> +-- These tests check that SQL savepoints properly work outside
> +-- transactions as well as inside transactions started in Lua.
> +--
> +-- box.cfg()
3. Garbage comment.
More information about the Tarantool-patches
mailing list