Tarantool development patches archive
 help / color / mirror / Atom feed
From: Nikita Pettik <korablev@tarantool.org>
To: tarantool-patches@freelists.org, tarantool-patches@dev.tarantool.org
Subject: [Tarantool-patches] [PATCH] sql: remove expmask from prepared statement
Date: Fri, 18 Oct 2019 16:47:10 +0300	[thread overview]
Message-ID: <20191018134710.69147-1-korablev@tarantool.org> (raw)

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

             reply	other threads:[~2019-10-18 13:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-18 13:47 Nikita Pettik [this message]
2019-10-19 13:33 ` [Tarantool-patches] [tarantool-patches] " Vladislav Shpilevoy
2019-10-20 21:46   ` Nikita Pettik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191018134710.69147-1-korablev@tarantool.org \
    --to=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [Tarantool-patches] [PATCH] sql: remove expmask from prepared statement' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox