From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 4F6964003E9 for ; Fri, 18 Oct 2019 16:47:15 +0300 (MSK) From: Nikita Pettik Date: Fri, 18 Oct 2019 16:47:10 +0300 Message-Id: <20191018134710.69147-1-korablev@tarantool.org> Subject: [Tarantool-patches] [PATCH] sql: remove expmask from prepared statement List-Id: Tarantool development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org expmask indicated necessity to recompile statement after parameter was bound: it might turn out that parameter can affect query plan. However, part of this mechanism has been removed long ago as a SQLite's legacy. In its current state expmask is likely to be useless and assertions involving it are obviously unsuitable. This patch completely removes expmask and related routines. Closes #4566 --- Branch: https://github.com/tarantool/tarantool/tree/np/gh-4566-remove-exp-mask Issue: https://github.com/tarantool/tarantool/issues/4566 src/box/sql/vdbe.h | 1 - src/box/sql/vdbeInt.h | 1 - src/box/sql/vdbeapi.c | 19 ------------------- src/box/sql/vdbeaux.c | 16 ---------------- src/box/sql/vdbemem.c | 1 - src/box/sql/whereexpr.c | 2 -- test/sql/bind.result | 32 ++++++++++++++++++++++++++++++++ test/sql/bind.test.lua | 9 +++++++++ 8 files changed, 41 insertions(+), 40 deletions(-) diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h index 29ff99867..582d48a1f 100644 --- a/src/box/sql/vdbe.h +++ b/src/box/sql/vdbe.h @@ -255,7 +255,6 @@ void sqlVdbeSetSql(Vdbe *, const char *z, int n, int); void sqlVdbeSwap(Vdbe *, Vdbe *); VdbeOp *sqlVdbeTakeOpArray(Vdbe *, int *, int *); sql_value *sqlVdbeGetBoundValue(Vdbe *, int, u8); -void sqlVdbeSetVarmask(Vdbe *, int); char *sqlVdbeExpandSql(Vdbe *, const char *); int sqlMemCompare(const Mem *, const Mem *, const struct coll *); diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h index 104a05613..0f32b4cd6 100644 --- a/src/box/sql/vdbeInt.h +++ b/src/box/sql/vdbeInt.h @@ -428,7 +428,6 @@ struct Vdbe { VdbeFrame *pFrame; /* Parent frame */ VdbeFrame *pDelFrame; /* List of frame objects to free on VM reset */ int nFrame; /* Number of frames in pFrame list */ - u32 expmask; /* Binding to these vars invalidates VM */ SubProgram *pProgram; /* Linked list of all sub-programs used by VM */ /** Parser flags with which this object was built. */ uint32_t sql_flags; diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c index bf40b44fa..b91d16a9d 100644 --- a/src/box/sql/vdbeapi.c +++ b/src/box/sql/vdbeapi.c @@ -112,9 +112,6 @@ sql_clear_bindings(sql_stmt * pStmt) sqlVdbeMemRelease(&p->aVar[i]); p->aVar[i].flags = MEM_Null; } - if (p->isPrepareV2 && p->expmask) { - p->expired = 1; - } return rc; } @@ -826,22 +823,6 @@ vdbeUnbind(Vdbe * p, int i) sqlVdbeMemRelease(pVar); pVar->flags = MEM_Null; pVar->field_type = field_type_MAX; - - /* If the bit corresponding to this variable in Vdbe.expmask is set, then - * binding a new value to this variable invalidates the current query plan. - * - * IMPLEMENTATION-OF: R-48440-37595 If the specific value bound to host - * parameter in the WHERE clause might influence the choice of query plan - * for a statement, then the statement will be automatically recompiled, - * as if there had been a schema change, on the first sql_step() call - * following any change to the bindings of that parameter. - */ - if (p->isPrepareV2 && - ((i < 32 && p->expmask & ((u32) 1 << i)) - || p->expmask == 0xffffffff) - ) { - p->expired = 1; - } return 0; } diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c index 40918e7e7..a1d658648 100644 --- a/src/box/sql/vdbeaux.c +++ b/src/box/sql/vdbeaux.c @@ -2988,22 +2988,6 @@ sqlVdbeGetBoundValue(Vdbe * v, int iVar, u8 aff) return 0; } -/* - * Configure SQL variable iVar so that binding a new value to it signals - * to sql_reoptimize() that re-preparing the statement may result - * in a better query plan. - */ -void -sqlVdbeSetVarmask(Vdbe * v, int iVar) -{ - assert(iVar > 0); - if (iVar > 32) { - v->expmask = 0xffffffff; - } else { - v->expmask |= ((u32) 1 << (iVar - 1)); - } -} - int sqlVdbeCompareMsgpack(const char **key1, struct UnpackedRecord *unpacked, int key2_idx) diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c index 15276c224..dcfc59820 100644 --- a/src/box/sql/vdbemem.c +++ b/src/box/sql/vdbemem.c @@ -1596,7 +1596,6 @@ stat4ValueFromExpr(Parse * pParse, /* Parse context */ ) { Vdbe *v; int iBindVar = pExpr->iColumn; - sqlVdbeSetVarmask(pParse->pVdbe, iBindVar); if ((v = pParse->pReprepare) != 0) { pVal = valueNew(db, pAlloc); if (pVal) { diff --git a/src/box/sql/whereexpr.c b/src/box/sql/whereexpr.c index 98615b118..d9b5c78f5 100644 --- a/src/box/sql/whereexpr.c +++ b/src/box/sql/whereexpr.c @@ -314,7 +314,6 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix, if (pVal && sql_value_type(pVal) == MP_STR) { z = (char *)sql_value_text(pVal); } - sqlVdbeSetVarmask(pParse->pVdbe, iCol); assert(pRight->op == TK_VARIABLE || pRight->op == TK_REGISTER); } else if (op == TK_STRING) { z = pRight->u.zToken; @@ -336,7 +335,6 @@ like_optimization_is_valid(Parse *pParse, Expr *pExpr, Expr **ppPrefix, *ppPrefix = pPrefix; if (op == TK_VARIABLE) { Vdbe *v = pParse->pVdbe; - sqlVdbeSetVarmask(v, pRight->iColumn); if (*pisComplete && pRight->u.zToken[1]) { /* If the rhs of the LIKE expression is a variable, and the current * value of the variable means there is no need to invoke the LIKE diff --git a/test/sql/bind.result b/test/sql/bind.result index 86126e86f..82a058c90 100644 --- a/test/sql/bind.result +++ b/test/sql/bind.result @@ -337,3 +337,35 @@ box.execute('SELECT $2', {1, 2, 3}) - null - 'Failed to execute SQL statement: The number of parameters is too large' ... +-- gh-4566: bind variable to LIKE argument resulted to crash. +-- +box.execute("CREATE TABLE t (id INT PRIMARY KEY, a TEXT);") +--- +- row_count: 1 +... +box.execute("SELECT * FROM t WHERE a LIKE ?;", {'a%'}); +--- +- metadata: + - name: ID + type: integer + - name: A + type: string + rows: [] +... +box.execute("INSERT INTO t VALUES (1, 'aA'), (2, 'Ba'), (3, 'A');") +--- +- row_count: 3 +... +box.execute("SELECT * FROM t WHERE a LIKE ?;", {'a%'}); +--- +- metadata: + - name: ID + type: integer + - name: A + type: string + rows: + - [1, 'aA'] +... +box.space.T:drop() +--- +... diff --git a/test/sql/bind.test.lua b/test/sql/bind.test.lua index 29d71edcd..2ced7775a 100644 --- a/test/sql/bind.test.lua +++ b/test/sql/bind.test.lua @@ -124,3 +124,12 @@ box.execute('DROP TABLE test') box.execute('SELECT ?', {1, 2}) box.execute('SELECT $2', {1, 2, 3}) + +-- gh-4566: bind variable to LIKE argument resulted to crash. +-- +box.execute("CREATE TABLE t (id INT PRIMARY KEY, a TEXT);") +box.execute("SELECT * FROM t WHERE a LIKE ?;", {'a%'}); +box.execute("INSERT INTO t VALUES (1, 'aA'), (2, 'Ba'), (3, 'A');") +box.execute("SELECT * FROM t WHERE a LIKE ?;", {'a%'}); + +box.space.T:drop() -- 2.15.1