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 14DBB23E52 for ; Mon, 21 May 2018 17:33:11 -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 InNITE3YsbYN for ; Mon, 21 May 2018 17:33:10 -0400 (EDT) Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (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 4864823E12 for ; Mon, 21 May 2018 17:33:09 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH 1/1] sql: fix out of time auto commit mode detection Date: Tue, 22 May 2018 00:33:06 +0300 Message-Id: <0fecccaea1baa046854e58e9ebffeb207103e7c3.1526938154.git.v.shpilevoy@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: korablev@tarantool.org Autocommit is a mode making a Vdbe programm either do not call box_txn_begin/commit and use Tarantool autocommit mode, or do box_txn_begin + commit hidden from a user. But before the patch it is detected during parsing that is very bad idea. 1. Vdbe conceptually must not depend on any parser time detected constants like autocommit. Vdbe can be executed multiple times, and in an environment different from the parsing time one. For example, parser can be run out of transaction, and Vdbe is executed inside a one. 2. When autocommit is detected to be false, parser creates things on a region hoping they will not be turned into garbage before Vdbe is executed. It is wrong too - region can be truncated between parsing end and Vdbe start (in future). Or Vdbe can be executed in another fiber. Lets detect autocommit mode in runtime, not in compile time. It allows to use region for temporary allocations during parsing, and makes us closer to reusable Vdbe objects. The detection is done once on OP_Init in the top-frame program. Child Vdbe programs (triggers, for example) must not touch any transaction things. Only top parent frame can make such decisions. Thanks to @kshcherbatov for finding the bug out. --- Branch: https://github.com/tarantool/tarantool/tree/fix-sql-auto-commit src/box/sql/select.c | 4 ---- src/box/sql/vdbe.c | 13 +++++++++++++ src/box/sql/vdbeaux.c | 2 +- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/src/box/sql/select.c b/src/box/sql/select.c index 29075d5c2..d26d74625 100644 --- a/src/box/sql/select.c +++ b/src/box/sql/select.c @@ -1937,10 +1937,6 @@ allocVdbe(Parse * pParse) ) { pParse->okConstFactor = 1; } - if (sql_vdbe_prepare(v) != 0) { - sqlite3DbFree(pParse->db, v); - return NULL; - } return v; } diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 3940550d6..5a4024efc 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -5351,6 +5351,19 @@ case OP_Init: { /* jump */ */ assert(pOp->p4.z==0 || strncmp(pOp->p4.z, "-" "- ", 3)==0); assert(pOp==p->aOp); /* Always instruction 0 */ + /* + * Once per execution time prepare the programm: detect + * autocommit, create SQL specific transaction things. To + * guarantee the single call of this function the + * preparation is done in the parent frame only. Child + * programs like triggers must use the information + * received from the parent. + */ + if (p->pFrame == NULL && sql_vdbe_prepare(p) != 0) { + sqlite3DbFree(db, p); + rc = SQL_TARANTOOL_ERROR; + break; + } #ifndef SQLITE_OMIT_TRACE if ((db->mTrace & SQLITE_TRACE_STMT)!=0 diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 734433640..4a12d69ae 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -55,7 +55,7 @@ sqlite3VdbeCreate(Parse * pParse) p = sqlite3DbMallocRawNN(db, sizeof(Vdbe)); if (p == 0) return 0; - memset(&p->aOp, 0, sizeof(Vdbe) - offsetof(Vdbe, aOp)); + memset(p, 0, sizeof(Vdbe)); p->db = db; if (db->pVdbe) { db->pVdbe->pPrev = p; -- 2.15.1 (Apple Git-101)