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

[Kirill Shcherbatov <kshcherbatov@tarantool.org>]

Hi! Thank you for participating. I've reworked this patch to open transaction before
index_create_iterator.
=========================================

diff --git a/src/box/sql.c b/src/box/sql.c
index d2cc0a9..0440edf 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -585,6 +585,7 @@ int tarantoolSqlite3EphemeralClearTable(BtCursor *pCur)
 	assert(pCur);
 	assert(pCur->curFlags & BTCF_TEphemCursor);
 
+	assert(space_is_memtx(pCur->space));
 	struct iterator *it = index_create_iterator(*pCur->space->index,
 						    ITER_ALL, nil_key,
 						    0 /* part_count */);
@@ -1134,12 +1135,26 @@ cursor_seek(BtCursor *pCur, int *pRes)
 		return SQL_TARANTOOL_ITERATOR_FAIL;
 	}
 
+	struct space *space = pCur->space;
+	struct txn *txn = NULL;
+
+	assert(space->def->id != 0 || space_is_memtx(space));
+	if (space->def->id != 0 &&
+	    space->def->id != BOX_SQL_STAT4_ID &&
+	    space->def->id != BOX_SQL_STAT1_ID) {
+		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) {
+		if (txn != NULL)
+			txn_rollback_stmt();
 		pCur->eState = CURSOR_INVALID;
 		return SQL_TARANTOOL_ITERATOR_FAIL;
 	}
+	if (txn != NULL)
+		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..a692b89 100644
--- a/test/sql/triggers.result
+++ b/test/sql/triggers.result
@@ -246,3 +246,123 @@ 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)")
+---
+...
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+function f()
+ box.sql.execute("START TRANSACTION")
+ box.sql.execute("INSERT INTO test VALUES (1)")
+ box.sql.execute("SELECT * FROM test2")
+ box.sql.execute("COMMIT")
+end;
+---
+...
+f();
+---
+- 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;");
+---
+...
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
diff --git a/test/sql/triggers.test.lua b/test/sql/triggers.test.lua
index e019c00..a9df278 100644
--- a/test/sql/triggers.test.lua
+++ b/test/sql/triggers.test.lua
@@ -95,3 +95,56 @@ 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)")
+test_run:cmd("setopt delimiter ';'")
+function f()
+ box.sql.execute("START TRANSACTION")
+ box.sql.execute("INSERT INTO test VALUES (1)")
+ box.sql.execute("SELECT * FROM test2")
+ box.sql.execute("COMMIT")
+end;
+f();
+box.sql.execute("ROLLBACK;");
+box.sql.execute("DROP TABLE test;");
+box.sql.execute("DROP TABLE test2;");
+test_run:cmd("setopt delimiter ''");