[tarantool-patches] Re: [PATCH v1 2/2] sql: prevent executing crossengine sql

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

On 26/07/2018 10:08, Kirill Shcherbatov wrote:
> I've cherry-picked Vova's path on my branch as it hasn't already merged to 2.0.
> 
>> 1. Please, put a new patch version at the end of letter.
> Ok.
> 
>> 2. crossengine looks like 'междудвижками'. Please, replace with
>> cross-engine.
> ok.
> 
>> 3. 'dissalow', 'dangeros' - no such words.
> fixed.
>> 4. Both commit_ro and rollback_ro already check for txn != NULL so please,
>> remove the redundant checks.
> ok.
> 
>> 5. Wit the same success you could inline f() and remove it. I
>> gave this code to you into the function because I run it in
>> the console. In test-run you anyway use "setopt delimiter" and
>> can avoid this function enclosing. Or you even could put all
>> this code in one line with no "setopt delimiter". There is no
>> so many code.
> I loks ungly.. ok.  You now, box.sql.execute can't process multiple sql statements

1. I did not purposed to use single box.sql.execute.

2. I did not insist on the single line. My proposal was to remove f().

> diff --git a/test/sql/triggers.result b/test/sql/triggers.result
> index dc0a2e5..71ade54 100644
> --- a/test/sql/triggers.result
> +++ b/test/sql/triggers.result
> @@ -246,3 +246,107 @@ box.sql.execute("DROP VIEW V1;")
>   box.sql.execute("DROP TABLE T1;")
>   ---
>   ...
> +--
> +-- gh-3531: Assertion with trigger and two storage engines
> +--
> +-- Case 1: Src 'vinyl' table; Dst 'memtx' table
> +box.sql.execute("PRAGMA sql_default_engine ('vinyl');")
> +---
> +...
> +box.sql.execute("CREATE TABLE m (s1 SCALAR PRIMARY KEY);")
> +---
> +...
> +box.sql.execute("CREATE TRIGGER m1 BEFORE UPDATE ON m FOR EACH ROW BEGIN UPDATE n SET s2 = DATETIME('now'); END;")
> +---
> +...
> +box.sql.execute("PRAGMA sql_default_engine('memtx');")
> +---
> +...
> +box.sql.execute("CREATE TABLE n (s1 CHAR PRIMARY KEY, s2 char);")
> +---
> +...
> +box.sql.execute("INSERT INTO m VALUES ('');")
> +---
> +...
> +box.sql.execute("INSERT INTO n VALUES ('',null);")
> +---
> +...
> +box.sql.execute("UPDATE m SET s1 = 'The Rain In Spain';")
> +---
> +- error: A multi-statement transaction can not use multiple storage engines
> +...
> +-- ANALYZE operates with _sql_stat{1,4} tables should work
> +box.sql.execute("ANALYZE m;")
> +---
> +...
> +box.sql.execute("DROP TABLE m;")
> +---
> +...
> +box.sql.execute("DROP TABLE n;")
> +---
> +...
> +-- Case 2: Src 'memtx' table; Dst 'vinyl' table
> +box.sql.execute("PRAGMA sql_default_engine ('memtx');")
> +---
> +...
> +box.sql.execute("CREATE TABLE m (s1 SCALAR PRIMARY KEY);")
> +---
> +...
> +box.sql.execute("CREATE TRIGGER m1 BEFORE UPDATE ON m FOR EACH ROW BEGIN UPDATE n SET s2 = DATETIME('now'); END;")
> +---
> +...
> +box.sql.execute("PRAGMA sql_default_engine('vinyl');")
> +---
> +...
> +box.sql.execute("CREATE TABLE n (s1 CHAR PRIMARY KEY, s2 char);")
> +---
> +...
> +box.sql.execute("INSERT INTO m VALUES ('');")
> +---
> +...
> +box.sql.execute("INSERT INTO n VALUES ('',null);")
> +---
> +...
> +box.sql.execute("UPDATE m SET s1 = 'The Rain In Spain';")
> +---
> +- error: A multi-statement transaction can not use multiple storage engines
> +...
> +-- ANALYZE operates with _sql_stat{1,4} tables should work
> +box.sql.execute("ANALYZE n;")
> +---
> +...
> +box.sql.execute("DROP TABLE m;")
> +---
> +...
> +box.sql.execute("DROP TABLE n;")
> +---
> +...
> +-- Test SQL Transaction with LUA
> +box.sql.execute("PRAGMA sql_default_engine ('memtx');")
> +---
> +...
> +box.sql.execute("CREATE TABLE test (id INT PRIMARY KEY)")
> +---
> +...
> +box.sql.execute("PRAGMA sql_default_engine='vinyl'")
> +---
> +...
> +box.sql.execute("CREATE TABLE test2 (id INT PRIMARY KEY)")
> +---
> +...
> +box.sql.execute("INSERT INTO test2 VALUES (2)")
> +---
> +...
> +box.sql.execute("START TRANSACTION") box.sql.execute("INSERT INTO test VALUES (1)") box.sql.execute("SELECT * FROM test2")  box.sql.execute("COMMIT")
> +---
> +- error: A multi-statement transaction can not use multiple storage engines
> +...
> +box.sql.execute("ROLLBACK;");

3. Why do you need ';' after execute()?

> +---
> +...
> +box.sql.execute("DROP TABLE test;");
> +---
> +...
> +box.sql.execute("DROP TABLE test2;");
> +---
> +...