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

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

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

================================

Some sql requests are complex and could contain R/W with
multiple spaces. As we have no ability to make such changes
transactionaly, we have to disallow such requests.
Since now iterators in SQL start transaction so we cant prevent
such vicious and dangerous things.

Closes #3551
---
 src/box/sql.c              |   8 ++++
 test/sql/triggers.result   | 104 +++++++++++++++++++++++++++++++++++++++++++++
 test/sql/triggers.test.lua |  45 ++++++++++++++++++++
 3 files changed, 157 insertions(+)

diff --git a/src/box/sql.c b/src/box/sql.c
index d48c3cf..db08fcf 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -1118,12 +1118,20 @@ cursor_seek(BtCursor *pCur, int *pRes)
 		return SQL_TARANTOOL_ITERATOR_FAIL;
 	}
 
+	struct space *space = pCur->space;
+	struct txn *txn = in_txn();
+	if (space->def->id != 0) {
+		if (txn_begin_ro_stmt(space, &txn) != 0)
+			return SQL_TARANTOOL_ERROR;
+	}
 	struct iterator *it = index_create_iterator(pCur->index, pCur->iter_type,
 						    key, part_count);
 	if (it == NULL) {
+		txn_rollback_stmt();
 		pCur->eState = CURSOR_INVALID;
 		return SQL_TARANTOOL_ITERATOR_FAIL;
 	}
+	txn_commit_ro_stmt(txn);
 	pCur->iter = it;
 	pCur->eState = CURSOR_VALID;
 
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;");
+---
+...
+box.sql.execute("DROP TABLE test;");
+---
+...
+box.sql.execute("DROP TABLE test2;");
+---
+...
diff --git a/test/sql/triggers.test.lua b/test/sql/triggers.test.lua
index e019c00..c7f377c 100644
--- a/test/sql/triggers.test.lua
+++ b/test/sql/triggers.test.lua
@@ -95,3 +95,48 @@ box.space._trigger:insert(tuple)
 
 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';")
+
+-- 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';")
+
+-- 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")
+box.sql.execute("ROLLBACK;");
+box.sql.execute("DROP TABLE test;");
+box.sql.execute("DROP TABLE test2;");
-- 
2.7.4