[tarantool-patches] Re: [PATCH 3/4] sql: allow SAVEPOINT statement outside transaction

n.pettik korablev at tarantool.org
Sat May 5 22:15:06 MSK 2018


> 1. Lets use diag_set(ClientError, ER_SAVEPOINT_NO_TRANSACTION) and return TARANTOOL_ERROR.
> It must behave like box.savepoint.

Ok:

@@ -2869,9 +2869,8 @@ case OP_Savepoint: {
 
        if (psql_txn == NULL) {
                assert(!box_txn());
-               sqlite3VdbeError(p, "cannot process savepoint: "
-                                   "there is no active transaction");
-                rc = SQLITE_ERROR;
+               diag_set(ClientError, ER_SAVEPOINT_NO_TRANSACTION);
+               rc = SQL_TARANTOOL_ERROR;

>> 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.

Sorry, it is garbage after copying files. Removed.

>> +---
>> +- true
>> +...
>> +-- These tests check that SQL savepoints properly work outside
>> +-- transactions as well as inside transactions started in Lua.
>> +--
>> +-- box.cfg()
> 
> 3. Garbage comment.

Removed.

Also, taking into consideration comment from review of last patch,
I have renamed test file and merge tests from both tickets.





More information about the Tarantool-patches mailing list