From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 6918420FC6 for ; Fri, 20 Jul 2018 09:52:51 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id oAYjXaTtMsjN for ; Fri, 20 Jul 2018 09:52:51 -0400 (EDT) Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 028FD20F9C for ; Fri, 20 Jul 2018 09:52:50 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 1/1] sql: prevent executing crossengine sql Date: Fri, 20 Jul 2018 16:52:46 +0300 Message-Id: <6134a0afdcfba13ca289e2d42fdd529a2787070a.1532094680.git.kshcherbatov@tarantool.org> Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Kirill Shcherbatov 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 dissallow such requests. This mean that we shouldn't create triggers on memtex spaces writing something in vinyl and so on. Since now, such checks are carried out on sql_finish_coding, the last parsing step. Pay attention, that _sql_stat1 and _sql_stat4 are exceptions. Most requests(excluding ANALYZE) use them only for READ and it is ok. Closes #3551 --- Branch: http://github.com/tarantool/tarantool/tree/kshch/kshch/gh-3551-crossengine-sql Issue: https://github.com/tarantool/tarantool/issues/3551 src/box/sql/build.c | 103 +++++++++++++++++++++++++++++++++++++++++++++ test/sql/triggers.result | 75 +++++++++++++++++++++++++++++++++ test/sql/triggers.test.lua | 34 +++++++++++++++ 3 files changed, 212 insertions(+) diff --git a/src/box/sql/build.c b/src/box/sql/build.c index a64d723..e52c05b 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -54,6 +54,106 @@ #include "box/tuple_format.h" #include "box/coll_id_cache.h" +/** + * Test if bytecode specified with @ops and @ops_cnt contain + * Read/Write operations with spaces of different storage engines. + * This function also fills @subprograms array containing pointers + * to SubProgram structures extracted from OP_Program instructions. + * Note: _stat_space(s) are not taken into account. + * + * @param ops instructions bytecode pointer. + * @param ops_cnt count of instructions. + * @param[out] space_cache pointer to last readed space. + * Required to raise error on engine mistmatch. + * @param[out] subprograms pointer to array of SubProgram to fill. + * May be ommited, in this case, no further action is + * taken. + * @param[out] subprograms_cnt count of subprograms stored in + * @subprograms. + * @retval whether the byte contains the operation code for spikes + * of different engines. + */ +static bool +vdbe_ops_has_crossengine_action(struct VdbeOp *ops, int ops_cnt, + struct space **space_cache, + struct SubProgram **subprograms, + int *subprograms_cnt) +{ + for (int i = 1; i < ops_cnt; i++) { + struct VdbeOp *op = &ops[i]; + if (op->opcode == OP_OpenRead || op->opcode == OP_OpenWrite) { + assert(op->p4type == P4_SPACEPTR); + struct space *space = op->p4.space; + if (space->def->id == BOX_SQL_STAT1_ID || + space->def->id == BOX_SQL_STAT4_ID) + continue; + if (*space_cache != NULL && + space->engine->vtab != (*space_cache)->engine->vtab) + return true; + *space_cache = space; + } else if (subprograms != NULL && op->opcode == OP_Program){ + assert(op->p4type == P4_SUBPROGRAM); + subprograms[*subprograms_cnt] = op->p4.pProgram; + *subprograms_cnt = *subprograms_cnt + 1; + } + } + return false; +} + +/** + * Check the @parse Vdbe program for operations with the spaces + * of different engines. Change parser state. + * @param parse The parsing context. + * @return 0 on success. + * @retval -1 on error. + */ +static int +vdbe_check_crossengine_actions(struct Parse *parse) +{ + if (parse->explain) + return 0; + struct space *space_cache = NULL; + struct Vdbe *v = sqlite3GetVdbe(parse); + struct SubProgram **subprograms_list = NULL; + + for (; v != NULL; v = v->pNext) { + /* The upper-bound of count of OP_Program ops. */ + subprograms_list = malloc(v->nOp*sizeof(struct SubProgram *)); + int subprograms_cnt = 0; + if (subprograms_list == NULL) { + diag_set(OutOfMemory, v->nOp*sizeof(struct SubProgram *), + "calloc", "subprograms_list"); + parse->nErr++; + parse->rc = SQL_TARANTOOL_ERROR; + return -1; + } + /* Check main code and fill subprograms_list. */ + if (vdbe_ops_has_crossengine_action(v->aOp, v->nOp, + &space_cache, + subprograms_list, + &subprograms_cnt)) + goto raise_error; + + for (int i = 0; i < subprograms_cnt; i++) { + struct SubProgram *p = subprograms_list[i]; + if (vdbe_ops_has_crossengine_action(p->aOp, p->nOp, + &space_cache, + NULL, 0)) + goto raise_error; + } + + free(subprograms_list); + } + return 0; + +raise_error: + free(subprograms_list); + diag_set(ClientError, ER_CROSS_ENGINE_TRANSACTION); + parse->rc = SQL_TARANTOOL_ERROR; + parse->nErr++; + return -1; +} + void sql_finish_coding(struct Parse *parse_context) { @@ -66,6 +166,9 @@ sql_finish_coding(struct Parse *parse_context) parse_context->rc = SQLITE_ERROR; return; } + if (vdbe_check_crossengine_actions(parse_context) != 0) + return; + /* * Begin by generating some termination code at the end * of the vdbe program diff --git a/test/sql/triggers.result b/test/sql/triggers.result index dc0a2e5..5b164f6 100644 --- a/test/sql/triggers.result +++ b/test/sql/triggers.result @@ -246,3 +246,78 @@ 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;") +--- +... diff --git a/test/sql/triggers.test.lua b/test/sql/triggers.test.lua index e019c00..9277990 100644 --- a/test/sql/triggers.test.lua +++ b/test/sql/triggers.test.lua @@ -95,3 +95,37 @@ 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;") -- 2.7.4