[Tarantool-patches] [PATCH 00/15] sql: prepared statements

[Tarantool-patches] [PATCH 00/15] sql: prepared statements

[Nikita Pettik]

Branch: https://github.com/tarantool/tarantool/tree/np/gh-2592-prepared-statements
Issues: https://github.com/tarantool/tarantool/issues/2592

For details see particular patches and RFC. One difference from RFC
is that there's no LRU replacement policy in current implementation.
User has to remove statements manually. Optional check of values
to be bound I'm going to implement as a follow-up to these patches.

To the one who will review: I've failed to come up with remote (netbox)
:execute() and :unpepare() methods for prepared statement objects.
So there are only cn:execute(query_id) and cn:unprepare(query_id) methods.
Hence we get a small inconsistency between remote and local behaviour:

Local mode

s = box.prepare("SELECT 1;")
tarantool> s
---
- execute: 'function: 0x011240df28'
  query_id: 1
  params: []
  unprepare: 'function: 0x011240df60'
  metadata:
  - name: '1'
    type: integer
  param_count: 0
...

s:execute() -> unfolds into box.execute(s.query_id)

Remote mode

cn = netbox.connect(box.cfg.listen)
s = cn:prepare("SELECT 1;")
tarantool> s
---
  query_id: 1
  params: []
  metadata:
  - name: '1'
    type: integer
  param_count: 0
...

cn:execute(s.query_id)

It seems that introducing :prepare() and :execute() methods might require
a lot of Lua netbox module refactoring (to make up execute/unprepare requests
we need established channel; as a workaround we can attempt at hiding it
in prepared statement object via patching serializable methods).

Nikita Pettik (15):
  sql: remove sql_prepare_v2()
  sql: refactor sql_prepare() and sqlPrepare()
  sql: move sql_prepare() declaration to box/execute.h
  sql: rename sqlPrepare() to sql_compile()
  sql: move sql_finalize() to execute.h
  port: increase padding of struct port
  port: add dump format and request type to port_sql
  sql: resurrect sql_bind_parameter_count() function
  sql: resurrect sql_bind_parameter_name()
  sql: add sql_schema_version()
  sql: introduce sql_stmt_sizeof() function
  box: increment schema_version on ddl operations
  sql: introduce cache for prepared statemets
  box: introduce prepared statements
  netbox: introduce prepared statements

 src/box/CMakeLists.txt          |   1 +
 src/box/alter.cc                |   3 +
 src/box/box.cc                  |  19 ++
 src/box/box.h                   |   1 +
 src/box/ck_constraint.c         |   1 +
 src/box/errcode.h               |   2 +
 src/box/execute.c               | 193 ++++++++++-
 src/box/execute.h               |  66 ++++
 src/box/iproto.cc               |  68 +++-
 src/box/iproto_constants.c      |   7 +-
 src/box/iproto_constants.h      |   5 +
 src/box/lua/cfg.cc              |  12 +
 src/box/lua/execute.c           | 231 +++++++++++++-
 src/box/lua/execute.h           |   2 +-
 src/box/lua/init.c              |   2 +-
 src/box/lua/load_cfg.lua        |   3 +
 src/box/lua/net_box.c           | 104 +++++-
 src/box/lua/net_box.lua         |  28 ++
 src/box/lua/session.c           |  10 +
 src/box/prep_stmt.c             | 108 +++++++
 src/box/prep_stmt.h             |  88 ++++++
 src/box/session.cc              |  14 +
 src/box/session.h               |   2 +
 src/box/sql/analyze.c           |  16 +-
 src/box/sql/legacy.c            |   3 +-
 src/box/sql/prepare.c           |  56 +---
 src/box/sql/sqlInt.h            |  44 ++-
 src/box/sql/vdbe.h              |   2 +-
 src/box/sql/vdbeInt.h           |   1 -
 src/box/sql/vdbeapi.c           |  83 +++--
 src/box/sql/vdbeaux.c           |   5 +-
 src/box/xrow.c                  |  19 +-
 src/box/xrow.h                  |   4 +-
 src/lib/core/port.h             |   2 +-
 test/app-tap/init_script.result |  37 +--
 test/box/admin.result           |   2 +
 test/box/cfg.result             |   7 +
 test/box/cfg.test.lua           |   1 +
 test/box/misc.result            |   5 +
 test/sql/engine.cfg             |   4 +
 test/sql/iproto.result          |   2 +-
 test/sql/prepared.result        | 685 ++++++++++++++++++++++++++++++++++++++++
 test/sql/prepared.test.lua      | 266 ++++++++++++++++
 43 files changed, 2043 insertions(+), 171 deletions(-)
 create mode 100644 src/box/prep_stmt.c
 create mode 100644 src/box/prep_stmt.h
 create mode 100644 test/sql/prepared.result
 create mode 100644 test/sql/prepared.test.lua

-- 
2.15.1

[Tarantool-patches] [PATCH 01/15] sql: remove sql_prepare_v2()

[Nikita Pettik]

There are two versions of the same function (sql_prepare()) which are
almost identical. Let's keep more relevant version sql_prepare_v2() but
rename it to sql_prepare() in order to avoid any mess.

Needed for #3292
---
 src/box/execute.c     |  2 +-
 src/box/sql/legacy.c  |  2 +-
 src/box/sql/prepare.c | 32 ++++----------------------------
 src/box/sql/sqlInt.h  | 25 +++++++++++--------------
 src/box/sql/vdbeapi.c |  2 +-
 5 files changed, 18 insertions(+), 45 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index e8b012e5b..130a3f675 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -443,7 +443,7 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 {
 	struct sql_stmt *stmt;
 	struct sql *db = sql_get();
-	if (sql_prepare_v2(db, sql, len, &stmt, NULL) != 0)
+	if (sql_prepare(db, sql, len, &stmt, NULL) != 0)
 		return -1;
 	assert(stmt != NULL);
 	port_sql_create(port, stmt);
diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c
index 0b1370f4a..bfd1e32b9 100644
--- a/src/box/sql/legacy.c
+++ b/src/box/sql/legacy.c
@@ -70,7 +70,7 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
 		char **azVals = 0;
 
 		pStmt = 0;
-		rc = sql_prepare_v2(db, zSql, -1, &pStmt, &zLeftover);
+		rc = sql_prepare(db, zSql, -1, &pStmt, &zLeftover);
 		assert(rc == 0 || pStmt == NULL);
 		if (rc != 0)
 			continue;
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index e077a8b5e..ba3b7d71f 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -204,36 +204,12 @@ sqlReprepare(Vdbe * p)
 	return 0;
 }
 
-/*
- * Two versions of the official API.  Legacy and new use.  In the legacy
- * version, the original SQL text is not saved in the prepared statement
- * and so if a schema change occurs, an error is returned by
- * sql_step().  In the new version, the original SQL text is retained
- * and the statement is automatically recompiled if an schema change
- * occurs.
- */
-int
-sql_prepare(sql * db,		/* Database handle. */
-		const char *zSql,	/* UTF-8 encoded SQL statement. */
-		int nBytes,		/* Length of zSql in bytes. */
-		sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
-		const char **pzTail)	/* OUT: End of parsed string */
-{
-	int rc = sqlPrepare(db, zSql, nBytes, 0, 0, ppStmt, pzTail);
-	assert(rc == 0 || ppStmt == NULL || *ppStmt == NULL);	/* VERIFY: F13021 */
-	return rc;
-}
-
 int
-sql_prepare_v2(sql * db,	/* Database handle. */
-		   const char *zSql,	/* UTF-8 encoded SQL statement. */
-		   int nBytes,	/* Length of zSql in bytes. */
-		   sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
-		   const char **pzTail	/* OUT: End of parsed string */
-    )
+sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt,
+	    const char **sql_tail)
 {
-	int rc = sqlPrepare(db, zSql, nBytes, 1, 0, ppStmt, pzTail);
-	assert(rc == 0 || ppStmt == NULL || *ppStmt == NULL);	/* VERIFY: F13021 */
+	int rc = sqlPrepare(db, sql, length, 1, 0, stmt, sql_tail);
+	assert(rc == 0 || stmt == NULL || *stmt == NULL);
 	return rc;
 }
 
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index c32cacc04..7a443fac2 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -459,21 +459,18 @@ typedef void (*sql_destructor_type) (void *);
 #define SQL_STATIC      ((sql_destructor_type)0)
 #define SQL_TRANSIENT   ((sql_destructor_type)-1)
 
+/**
+ * Prepare (compile into VDBE byte-code) statement.
+ *
+ * @param db Database handle.
+ * @param sql UTF-8 encoded SQL statement.
+ * @param length Length of @param sql in bytes.
+ * @param[out] stmt A pointer to the prepared statement.
+ * @param[out] sql_tail End of parsed string.
+ */
 int
-sql_prepare(sql * db,	/* Database handle */
-		const char *zSql,	/* SQL statement, UTF-8 encoded */
-		int nByte,	/* Maximum length of zSql in bytes. */
-		sql_stmt ** ppStmt,	/* OUT: Statement handle */
-		const char **pzTail	/* OUT: Pointer to unused portion of zSql */
-	);
-
-int
-sql_prepare_v2(sql * db,	/* Database handle */
-		   const char *zSql,	/* SQL statement, UTF-8 encoded */
-		   int nByte,	/* Maximum length of zSql in bytes. */
-		   sql_stmt ** ppStmt,	/* OUT: Statement handle */
-		   const char **pzTail	/* OUT: Pointer to unused portion of zSql */
-	);
+sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt,
+	    const char **sql_tail);
 
 int
 sql_step(sql_stmt *);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index b91d16a9d..9d9c5168e 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -452,7 +452,7 @@ sqlStep(Vdbe * p)
 		checkProfileCallback(db, p);
 
 	if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) {
-		/* If this statement was prepared using sql_prepare_v2(), and an
+		/* If this statement was prepared using sql_prepare(), and an
 		 * error has occurred, then return an error.
 		 */
 		if (p->is_aborted)
-- 
2.15.1

[Tarantool-patches] [PATCH 02/15] sql: refactor sql_prepare() and sqlPrepare()

[Nikita Pettik]

- Removed saveSqlFlag as argument from sqlPrepare(). It was used to
  indicate that its caller is sql_prepare_v2() not sql_prepare().
  Since in previous commit we've left only one version of this function
  let's remove this flag at all.

- Removed struct db from list of sql_prepare() arguments. There's one
  global database handler and it can be obtained by sql_get() call.
  Hence, it makes no sense to pass around this argument.

Needed for #3292
---
 src/box/execute.c     |  3 +--
 src/box/sql/analyze.c | 15 +++++++--------
 src/box/sql/legacy.c  |  2 +-
 src/box/sql/prepare.c | 10 ++++------
 src/box/sql/sqlInt.h  |  3 +--
 src/box/sql/vdbe.h    |  2 +-
 src/box/sql/vdbeInt.h |  1 -
 src/box/sql/vdbeapi.c |  2 +-
 src/box/sql/vdbeaux.c |  5 +----
 9 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 130a3f675..0b21386b5 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -442,8 +442,7 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 			struct region *region)
 {
 	struct sql_stmt *stmt;
-	struct sql *db = sql_get();
-	if (sql_prepare(db, sql, len, &stmt, NULL) != 0)
+	if (sql_prepare(sql, len, &stmt, NULL) != 0)
 		return -1;
 	assert(stmt != NULL);
 	port_sql_create(port, stmt);
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index b9858c8d6..1eba1e206 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1343,7 +1343,7 @@ sample_compare(const void *a, const void *b, void *arg)
  * @retval 0 on success, -1 otherwise.
  */
 static int
-load_stat_from_space(struct sql *db, const char *sql_select_prepare,
+load_stat_from_space(const char *sql_select_prepare,
 		     const char *sql_select_load, struct index_stat *stats)
 {
 	struct index **indexes = NULL;
@@ -1359,7 +1359,7 @@ load_stat_from_space(struct sql *db, const char *sql_select_prepare,
 		}
 	}
 	sql_stmt *stmt = NULL;
-	int rc = sql_prepare(db, sql_select_prepare, -1, &stmt, 0);
+	int rc = sql_prepare(sql_select_prepare, -1, &stmt, 0);
 	if (rc)
 		goto finalize;
 	uint32_t current_idx_count = 0;
@@ -1427,7 +1427,7 @@ load_stat_from_space(struct sql *db, const char *sql_select_prepare,
 	rc = sql_finalize(stmt);
 	if (rc)
 		goto finalize;
-	rc = sql_prepare(db, sql_select_load, -1, &stmt, 0);
+	rc = sql_prepare(sql_select_load, -1, &stmt, 0);
 	if (rc)
 		goto finalize;
 	struct index *prev_index = NULL;
@@ -1505,12 +1505,11 @@ load_stat_from_space(struct sql *db, const char *sql_select_prepare,
 }
 
 static int
-load_stat_to_index(struct sql *db, const char *sql_select_load,
-		   struct index_stat **stats)
+load_stat_to_index(const char *sql_select_load, struct index_stat **stats)
 {
 	assert(stats != NULL && *stats != NULL);
 	struct sql_stmt *stmt = NULL;
-	if (sql_prepare(db, sql_select_load, -1, &stmt, 0) != 0)
+	if (sql_prepare(sql_select_load, -1, &stmt, 0) != 0)
 		return -1;
 	uint32_t current_idx_count = 0;
 	while (sql_step(stmt) == SQL_ROW) {
@@ -1696,7 +1695,7 @@ sql_analysis_load(struct sql *db)
 	const char *load_query = "SELECT \"tbl\",\"idx\",\"neq\",\"nlt\","
 				 "\"ndlt\",\"sample\" FROM \"_sql_stat4\"";
 	/* Load the statistics from the _sql_stat4 table. */
-	if (load_stat_from_space(db, init_query, load_query, stats) != 0)
+	if (load_stat_from_space(init_query, load_query, stats) != 0)
 		goto fail;
 	/*
 	 * Now we have complete statistics for each index
@@ -1739,7 +1738,7 @@ sql_analysis_load(struct sql *db)
 	 */
 	const char *order_query = "SELECT \"tbl\",\"idx\" FROM "
 				  "\"_sql_stat4\" GROUP BY \"tbl\",\"idx\"";
-	if (load_stat_to_index(db, order_query, heap_stats) == 0)
+	if (load_stat_to_index(order_query, heap_stats) == 0)
 		return box_txn_commit();
 fail:
 	box_txn_rollback();
diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c
index bfd1e32b9..16507b334 100644
--- a/src/box/sql/legacy.c
+++ b/src/box/sql/legacy.c
@@ -70,7 +70,7 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
 		char **azVals = 0;
 
 		pStmt = 0;
-		rc = sql_prepare(db, zSql, -1, &pStmt, &zLeftover);
+		rc = sql_prepare(zSql, -1, &pStmt, &zLeftover);
 		assert(rc == 0 || pStmt == NULL);
 		if (rc != 0)
 			continue;
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index ba3b7d71f..f72f9aa23 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -46,7 +46,6 @@ static int
 sqlPrepare(sql * db,	/* Database handle. */
 	       const char *zSql,	/* UTF-8 encoded SQL statement. */
 	       int nBytes,	/* Length of zSql in bytes. */
-	       int saveSqlFlag,	/* True to copy SQL text into the sql_stmt */
 	       Vdbe * pReprepare,	/* VM being reprepared */
 	       sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
 	       const char **pzTail	/* OUT: End of parsed string */
@@ -156,8 +155,7 @@ sqlPrepare(sql * db,	/* Database handle. */
 
 	if (db->init.busy == 0) {
 		Vdbe *pVdbe = sParse.pVdbe;
-		sqlVdbeSetSql(pVdbe, zSql, (int)(sParse.zTail - zSql),
-				  saveSqlFlag);
+		sqlVdbeSetSql(pVdbe, zSql, (int)(sParse.zTail - zSql));
 	}
 	if (sParse.pVdbe != NULL && (rc != 0 || db->mallocFailed)) {
 		sqlVdbeFinalize(sParse.pVdbe);
@@ -192,7 +190,7 @@ sqlReprepare(Vdbe * p)
 	zSql = sql_sql((sql_stmt *) p);
 	assert(zSql != 0);	/* Reprepare only called for prepare_v2() statements */
 	db = sqlVdbeDb(p);
-	if (sqlPrepare(db, zSql, -1, 0, p, &pNew, 0) != 0) {
+	if (sqlPrepare(db, zSql, -1, p, &pNew, 0) != 0) {
 		assert(pNew == 0);
 		return -1;
 	}
@@ -205,10 +203,10 @@ sqlReprepare(Vdbe * p)
 }
 
 int
-sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt,
+sql_prepare(const char *sql, int length, struct sql_stmt **stmt,
 	    const char **sql_tail)
 {
-	int rc = sqlPrepare(db, sql, length, 1, 0, stmt, sql_tail);
+	int rc = sqlPrepare(sql_get(), sql, length, 0, stmt, sql_tail);
 	assert(rc == 0 || stmt == NULL || *stmt == NULL);
 	return rc;
 }
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 7a443fac2..ccb11bcc1 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -462,14 +462,13 @@ typedef void (*sql_destructor_type) (void *);
 /**
  * Prepare (compile into VDBE byte-code) statement.
  *
- * @param db Database handle.
  * @param sql UTF-8 encoded SQL statement.
  * @param length Length of @param sql in bytes.
  * @param[out] stmt A pointer to the prepared statement.
  * @param[out] sql_tail End of parsed string.
  */
 int
-sql_prepare(struct sql *db, const char *sql, int length, struct sql_stmt **stmt,
+sql_prepare(const char *sql, int length, struct sql_stmt **stmt,
 	    const char **sql_tail);
 
 int
diff --git a/src/box/sql/vdbe.h b/src/box/sql/vdbe.h
index 582d48a1f..573577355 100644
--- a/src/box/sql/vdbe.h
+++ b/src/box/sql/vdbe.h
@@ -251,7 +251,7 @@ void sqlVdbeSetNumCols(Vdbe *, int);
 int sqlVdbeSetColName(Vdbe *, int, int, const char *, void (*)(void *));
 void sqlVdbeCountChanges(Vdbe *);
 sql *sqlVdbeDb(Vdbe *);
-void sqlVdbeSetSql(Vdbe *, const char *z, int n, int);
+void sqlVdbeSetSql(Vdbe *, const char *z, int n);
 void sqlVdbeSwap(Vdbe *, Vdbe *);
 VdbeOp *sqlVdbeTakeOpArray(Vdbe *, int *, int *);
 sql_value *sqlVdbeGetBoundValue(Vdbe *, int, u8);
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 0f32b4cd6..078ebc34e 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -421,7 +421,6 @@ struct Vdbe {
 	bft explain:2;		/* True if EXPLAIN present on SQL command */
 	bft changeCntOn:1;	/* True to update the change-counter */
 	bft runOnlyOnce:1;	/* Automatically expire on reset */
-	bft isPrepareV2:1;	/* True if prepared with prepare_v2() */
 	u32 aCounter[5];	/* Counters used by sql_stmt_status() */
 	char *zSql;		/* Text of the SQL statement that generated this */
 	void *pFree;		/* Free this when deleting the vdbe */
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 9d9c5168e..a396f74d6 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -451,7 +451,7 @@ sqlStep(Vdbe * p)
 	if (rc != SQL_ROW)
 		checkProfileCallback(db, p);
 
-	if (p->isPrepareV2 && rc != SQL_ROW && rc != SQL_DONE) {
+	if (rc != SQL_ROW && rc != SQL_DONE) {
 		/* If this statement was prepared using sql_prepare(), and an
 		 * error has occurred, then return an error.
 		 */
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index a1d658648..619105820 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -89,14 +89,12 @@ sql_vdbe_prepare(struct Vdbe *vdbe)
  * Remember the SQL string for a prepared statement.
  */
 void
-sqlVdbeSetSql(Vdbe * p, const char *z, int n, int isPrepareV2)
+sqlVdbeSetSql(Vdbe * p, const char *z, int n)
 {
-	assert(isPrepareV2 == 1 || isPrepareV2 == 0);
 	if (p == 0)
 		return;
 	assert(p->zSql == 0);
 	p->zSql = sqlDbStrNDup(p->db, z, n);
-	p->isPrepareV2 = (u8) isPrepareV2;
 }
 
 /*
@@ -120,7 +118,6 @@ sqlVdbeSwap(Vdbe * pA, Vdbe * pB)
 	zTmp = pA->zSql;
 	pA->zSql = pB->zSql;
 	pB->zSql = zTmp;
-	pB->isPrepareV2 = pA->isPrepareV2;
 }
 
 /*
-- 
2.15.1

[Tarantool-patches] [PATCH 03/15] sql: move sql_prepare() declaration to box/execute.h

[Nikita Pettik]

We are going to split sql_prepare_and_execute() into several explicit
and logically separated steps:

1. sql_prepare() -- compile VDBE byte-code
2. sql_bind() -- bind variables (if there are any)
3. sql_execute() -- query (byte-code) execution in virtual machine

For instance, for dry-run we are interested only in query preparation.
Contrary, if we had prepared statement cache, we could skip query
preparation and handle only bind and execute steps.

To avoid inclusion of sql/sqlInt.h header (which gathers almost all SQL
specific functions and constants) let's move sql_prepare() to
box/execute.h header (which already holds sql_prepare_and_execute()).

Needed for #3292
---
 src/box/execute.h     | 12 ++++++++++++
 src/box/sql/analyze.c |  1 +
 src/box/sql/legacy.c  |  1 +
 src/box/sql/sqlInt.h  | 12 ------------
 4 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/src/box/execute.h b/src/box/execute.h
index a2fd4d1b7..a6000c08b 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -89,6 +89,18 @@ struct port_sql {
 
 extern const struct port_vtab port_sql_vtab;
 
+/**
+ * Prepare (compile into VDBE byte-code) statement.
+ *
+ * @param sql UTF-8 encoded SQL statement.
+ * @param length Length of @param sql in bytes.
+ * @param[out] stmt A pointer to the prepared statement.
+ * @param[out] sql_tail End of parsed string.
+ */
+int
+sql_prepare(const char *sql, int length, struct sql_stmt **stmt,
+	    const char **sql_tail);
+
 #if defined(__cplusplus)
 } /* extern "C" { */
 #endif
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 1eba1e206..9a66f8254 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -106,6 +106,7 @@
  */
 
 #include "box/box.h"
+#include "box/execute.h"
 #include "box/index.h"
 #include "box/key_def.h"
 #include "box/schema.h"
diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c
index 16507b334..e3a2c77ca 100644
--- a/src/box/sql/legacy.c
+++ b/src/box/sql/legacy.c
@@ -37,6 +37,7 @@
  */
 
 #include "sqlInt.h"
+#include "box/execute.h"
 #include "box/session.h"
 
 /*
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index ccb11bcc1..1bac252af 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -459,18 +459,6 @@ typedef void (*sql_destructor_type) (void *);
 #define SQL_STATIC      ((sql_destructor_type)0)
 #define SQL_TRANSIENT   ((sql_destructor_type)-1)
 
-/**
- * Prepare (compile into VDBE byte-code) statement.
- *
- * @param sql UTF-8 encoded SQL statement.
- * @param length Length of @param sql in bytes.
- * @param[out] stmt A pointer to the prepared statement.
- * @param[out] sql_tail End of parsed string.
- */
-int
-sql_prepare(const char *sql, int length, struct sql_stmt **stmt,
-	    const char **sql_tail);
-
 int
 sql_step(sql_stmt *);
 
-- 
2.15.1

[Tarantool-patches] [PATCH 04/15] sql: rename sqlPrepare() to sql_compile()

[Nikita Pettik]

sql_prepare() is going not only to compile statement, but also to save it
to the prepared statement cache. So we'd better rename sqlPrepare()
which is static wrapper around sql_prepare() and make it non-static.
Where it is possible let's use sql_compile() instead of sql_prepare().

Needed for #2592
---
 src/box/execute.c     |  2 +-
 src/box/sql/analyze.c |  6 +++---
 src/box/sql/legacy.c  |  2 +-
 src/box/sql/prepare.c | 21 ++++++---------------
 src/box/sql/sqlInt.h  | 13 +++++++++++++
 5 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 0b21386b5..83680b70f 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -442,7 +442,7 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 			struct region *region)
 {
 	struct sql_stmt *stmt;
-	if (sql_prepare(sql, len, &stmt, NULL) != 0)
+	if (sql_compile(sql, len, NULL, &stmt, NULL) != 0)
 		return -1;
 	assert(stmt != NULL);
 	port_sql_create(port, stmt);
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 9a66f8254..a887a2bf3 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -1360,7 +1360,7 @@ load_stat_from_space(const char *sql_select_prepare,
 		}
 	}
 	sql_stmt *stmt = NULL;
-	int rc = sql_prepare(sql_select_prepare, -1, &stmt, 0);
+	int rc = sql_compile(sql_select_prepare, -1, NULL, &stmt, 0);
 	if (rc)
 		goto finalize;
 	uint32_t current_idx_count = 0;
@@ -1428,7 +1428,7 @@ load_stat_from_space(const char *sql_select_prepare,
 	rc = sql_finalize(stmt);
 	if (rc)
 		goto finalize;
-	rc = sql_prepare(sql_select_load, -1, &stmt, 0);
+	rc = sql_compile(sql_select_load, -1, NULL, &stmt, 0);
 	if (rc)
 		goto finalize;
 	struct index *prev_index = NULL;
@@ -1510,7 +1510,7 @@ load_stat_to_index(const char *sql_select_load, struct index_stat **stats)
 {
 	assert(stats != NULL && *stats != NULL);
 	struct sql_stmt *stmt = NULL;
-	if (sql_prepare(sql_select_load, -1, &stmt, 0) != 0)
+	if (sql_compile(sql_select_load, -1, NULL, &stmt, 0) != 0)
 		return -1;
 	uint32_t current_idx_count = 0;
 	while (sql_step(stmt) == SQL_ROW) {
diff --git a/src/box/sql/legacy.c b/src/box/sql/legacy.c
index e3a2c77ca..93f927dea 100644
--- a/src/box/sql/legacy.c
+++ b/src/box/sql/legacy.c
@@ -71,7 +71,7 @@ sql_exec(sql * db,	/* The database on which the SQL executes */
 		char **azVals = 0;
 
 		pStmt = 0;
-		rc = sql_prepare(zSql, -1, &pStmt, &zLeftover);
+		rc = sql_compile(zSql, -1, NULL, &pStmt, &zLeftover);
 		assert(rc == 0 || pStmt == NULL);
 		if (rc != 0)
 			continue;
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index f72f9aa23..780085e56 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -39,18 +39,11 @@
 #include "box/space.h"
 #include "box/session.h"
 
-/*
- * Compile the UTF-8 encoded SQL statement zSql into a statement handle.
- */
-static int
-sqlPrepare(sql * db,	/* Database handle. */
-	       const char *zSql,	/* UTF-8 encoded SQL statement. */
-	       int nBytes,	/* Length of zSql in bytes. */
-	       Vdbe * pReprepare,	/* VM being reprepared */
-	       sql_stmt ** ppStmt,	/* OUT: A pointer to the prepared statement */
-	       const char **pzTail	/* OUT: End of parsed string */
-    )
+int
+sql_compile(const char *zSql, int nBytes, struct Vdbe *pReprepare,
+	    sql_stmt **ppStmt, const char **pzTail)
 {
+	struct sql *db = sql_get();
 	int rc = 0;	/* Result code */
 	Parse sParse;		/* Parsing context */
 	sql_parser_create(&sParse, db, current_session()->sql_flags);
@@ -185,12 +178,10 @@ sqlReprepare(Vdbe * p)
 {
 	sql_stmt *pNew;
 	const char *zSql;
-	sql *db;
 
 	zSql = sql_sql((sql_stmt *) p);
 	assert(zSql != 0);	/* Reprepare only called for prepare_v2() statements */
-	db = sqlVdbeDb(p);
-	if (sqlPrepare(db, zSql, -1, p, &pNew, 0) != 0) {
+	if (sql_compile(zSql, -1, p, &pNew, 0) != 0) {
 		assert(pNew == 0);
 		return -1;
 	}
@@ -206,7 +197,7 @@ int
 sql_prepare(const char *sql, int length, struct sql_stmt **stmt,
 	    const char **sql_tail)
 {
-	int rc = sqlPrepare(sql_get(), sql, length, 0, stmt, sql_tail);
+	int rc = sql_compile(sql, length, 0, stmt, sql_tail);
 	assert(rc == 0 || stmt == NULL || *stmt == NULL);
 	return rc;
 }
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1bac252af..4a28e5fb9 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -459,6 +459,19 @@ typedef void (*sql_destructor_type) (void *);
 #define SQL_STATIC      ((sql_destructor_type)0)
 #define SQL_TRANSIENT   ((sql_destructor_type)-1)
 
+/**
+ * Compile the UTF-8 encoded SQL statement zSql into a statement handle.
+ *
+ * @param sql UTF-8 encoded SQL statement.
+ * @param sql_len Length of @sql in bytes.
+ * @param re_prepared VM being re-compiled. Can be NULL.
+ * @param[out] stmt A pointer to the compiled statement.
+ * @param[out] sql_tail End of parsed string.
+ */
+int
+sql_compile(const char *sql, int bytes_count, struct Vdbe *re_prepared,
+	    sql_stmt **stmt, const char **sql_tail);
+
 int
 sql_step(sql_stmt *);
 
-- 
2.15.1

[Tarantool-patches] [PATCH 05/15] sql: move sql_finalize() to execute.h

[Nikita Pettik]

We are going to make prepared statement cache be session local. Hence,
when sessions is destroyed we should erase its cache and deallocate each
prepared statement in it. As a consequence, we should be able to call
sql_finalize() from box/ submodule. So let's move its signature to
box/execute.h

 Need for #2592
---
 src/box/ck_constraint.c | 1 +
 src/box/execute.h       | 3 +++
 src/box/sql/sqlInt.h    | 3 ---
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/box/ck_constraint.c b/src/box/ck_constraint.c
index d21717f94..6b9d44775 100644
--- a/src/box/ck_constraint.c
+++ b/src/box/ck_constraint.c
@@ -29,6 +29,7 @@
  * SUCH DAMAGE.
  */
 #include "box/session.h"
+#include "execute.h"
 #include "bind.h"
 #include "ck_constraint.h"
 #include "errcode.h"
diff --git a/src/box/execute.h b/src/box/execute.h
index a6000c08b..a85fca5fc 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -89,6 +89,9 @@ struct port_sql {
 
 extern const struct port_vtab port_sql_vtab;
 
+int
+sql_finalize(struct sql_stmt *stmt);
+
 /**
  * Prepare (compile into VDBE byte-code) statement.
  *
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 4a28e5fb9..553a48d60 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -510,9 +510,6 @@ sql_value *
 sql_column_value(sql_stmt *,
 		     int iCol);
 
-int
-sql_finalize(sql_stmt * pStmt);
-
 /*
  * Terminate the current execution of an SQL statement and reset
  * it back to its starting state so that it can be reused.
-- 
2.15.1

[Tarantool-patches] [PATCH 06/15] port: increase padding of struct port

[Nikita Pettik]

We are going to extend context of struct port_sql. One already inherits
struct port_tuple, which makes it size barely fits into 48 bytes of
padding of basic structure (struct port). Hence, let's increase padding
a bit to be able to add at least one more member to struct port_sql.
---
 src/lib/core/port.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lib/core/port.h b/src/lib/core/port.h
index d61342287..bfdfa4656 100644
--- a/src/lib/core/port.h
+++ b/src/lib/core/port.h
@@ -122,7 +122,7 @@ struct port {
 	 * Implementation dependent content. Needed to declare
 	 * an abstract port instance on stack.
 	 */
-	char pad[48];
+	char pad[52];
 };
 
 /** Is not inlined just to be exported. */
-- 
2.15.1

[Tarantool-patches] [PATCH 07/15] port: add dump format and request type to port_sql

[Nikita Pettik]

Dump formats of DQL and DML queries are different: the last one contains
number of affected rows and optionally list of autoincremented ids; the
first one comprises all meta-information including column names of
resulting set and their types. What is more, dump format is going to be
different for execute and prepare requests. So let's introduce separate
member to struct port_sql responsible for dump format to be used.

What is more, prepared statement finalization is required only for
PREPARE-AND-EXECUTE requests. So let's keep request type in port as well.

Note that C standard specifies that enums are integers, but it does not
specify the size. Hence, let's use simple uint8 - mentioned enums are
small enough to fit into it.

Needed for #2592
---
 src/box/execute.c     | 32 +++++++++++++++++++++++---------
 src/box/execute.h     | 24 ++++++++++++++++++++++++
 src/box/lua/execute.c | 20 +++++++++++++-------
 3 files changed, 60 insertions(+), 16 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 83680b70f..d2a999099 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -100,7 +100,9 @@ static void
 port_sql_destroy(struct port *base)
 {
 	port_tuple_vtab.destroy(base);
-	sql_finalize(((struct port_sql *)base)->stmt);
+	struct port_sql *port_sql = (struct port_sql *) base;
+	if (port_sql->request == PREPARE_AND_EXECUTE)
+		sql_finalize(((struct port_sql *)base)->stmt);
 }
 
 const struct port_vtab port_sql_vtab = {
@@ -114,11 +116,15 @@ const struct port_vtab port_sql_vtab = {
 };
 
 static void
-port_sql_create(struct port *port, struct sql_stmt *stmt)
+port_sql_create(struct port *port, struct sql_stmt *stmt,
+		enum sql_dump_format dump_format, enum sql_request_type request)
 {
 	port_tuple_create(port);
-	((struct port_sql *)port)->stmt = stmt;
 	port->vtab = &port_sql_vtab;
+	struct port_sql *port_sql = (struct port_sql *) port;
+	port_sql->stmt = stmt;
+	port_sql->dump_format = dump_format;
+	port_sql->request = request;
 }
 
 /**
@@ -324,9 +330,10 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
 {
 	assert(port->vtab == &port_sql_vtab);
 	sql *db = sql_get();
-	struct sql_stmt *stmt = ((struct port_sql *)port)->stmt;
-	int column_count = sql_column_count(stmt);
-	if (column_count > 0) {
+	struct port_sql *sql_port = (struct port_sql *)port;
+	struct sql_stmt *stmt = sql_port->stmt;
+	switch (sql_port->dump_format) {
+	case DQL_EXECUTE: {
 		int keys = 2;
 		int size = mp_sizeof_map(keys);
 		char *pos = (char *) obuf_alloc(out, size);
@@ -335,7 +342,7 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
 			return -1;
 		}
 		pos = mp_encode_map(pos, keys);
-		if (sql_get_metadata(stmt, out, column_count) != 0)
+		if (sql_get_metadata(stmt, out, sql_column_count(stmt)) != 0)
 			return -1;
 		size = mp_sizeof_uint(IPROTO_DATA);
 		pos = (char *) obuf_alloc(out, size);
@@ -346,7 +353,9 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
 		pos = mp_encode_uint(pos, IPROTO_DATA);
 		if (port_tuple_vtab.dump_msgpack(port, out) < 0)
 			return -1;
-	} else {
+		break;
+	}
+	case DML_EXECUTE: {
 		int keys = 1;
 		assert(((struct port_tuple *)port)->size == 0);
 		struct stailq *autoinc_id_list =
@@ -395,6 +404,9 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
 				      mp_encode_int(buf, id_entry->id);
 			}
 		}
+		break;
+	}
+	default: unreachable();
 	}
 	return 0;
 }
@@ -445,7 +457,9 @@ sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 	if (sql_compile(sql, len, NULL, &stmt, NULL) != 0)
 		return -1;
 	assert(stmt != NULL);
-	port_sql_create(port, stmt);
+	enum sql_dump_format dump_format = sql_column_count(stmt) > 0 ?
+					   DQL_EXECUTE : DML_EXECUTE;
+	port_sql_create(port, stmt, dump_format, PREPARE_AND_EXECUTE);
 	if (sql_bind(stmt, bind, bind_count) == 0 &&
 	    sql_execute(stmt, port, region) == 0)
 		return 0;
diff --git a/src/box/execute.h b/src/box/execute.h
index a85fca5fc..6702a18cc 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -46,6 +46,23 @@ enum sql_info_key {
 	sql_info_key_MAX,
 };
 
+/**
+ * One of possible formats used to dump msgpack/Lua.
+ * For details see port_sql_dump_msgpack() and port_sql_dump_lua().
+ */
+enum sql_dump_format {
+	DQL_EXECUTE = 0,
+	DML_EXECUTE = 1,
+	DQL_PREPARE = 2,
+	DML_PREPARE = 3,
+};
+
+enum sql_request_type {
+	PREPARE_AND_EXECUTE = 0,
+	PREPARE = 1,
+	EXECUTE_PREPARED = 2
+};
+
 extern const char *sql_info_key_strs[];
 
 struct region;
@@ -85,6 +102,13 @@ struct port_sql {
 	struct port_tuple port_tuple;
 	/* Prepared SQL statement. */
 	struct sql_stmt *stmt;
+	/**
+	 * Dump format depends on type of SQL query: DML or DQL;
+	 * and on type of SQL request: execute or prepare.
+	 */
+	uint8_t dump_format;
+	/** enum sql_request_type */
+	uint8_t request;
 };
 
 extern const struct port_vtab port_sql_vtab;
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index 76ecdd541..1b2f8d235 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -45,18 +45,21 @@ port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
 	assert(is_flat == false);
 	assert(port->vtab == &port_sql_vtab);
 	struct sql *db = sql_get();
-	struct sql_stmt *stmt = ((struct port_sql *)port)->stmt;
-	int column_count = sql_column_count(stmt);
-	if (column_count > 0) {
+	struct port_sql *port_sql = (struct port_sql *)port;
+	struct sql_stmt *stmt = port_sql->stmt;
+	switch (port_sql->dump_format) {
+	case DQL_EXECUTE: {
 		lua_createtable(L, 0, 2);
-		lua_sql_get_metadata(stmt, L, column_count);
+		lua_sql_get_metadata(stmt, L, sql_column_count(stmt));
 		lua_setfield(L, -2, "metadata");
 		port_tuple_vtab.dump_lua(port, L, false);
 		lua_setfield(L, -2, "rows");
-	} else {
-		assert(((struct port_tuple *)port)->size == 0);
+		break;
+	}
+	case DML_EXECUTE: {
+		assert(((struct port_tuple *) port)->size == 0);
 		struct stailq *autoinc_id_list =
-			vdbe_autoinc_id_list((struct Vdbe *)stmt);
+			vdbe_autoinc_id_list((struct Vdbe *) stmt);
 		lua_createtable(L, 0, stailq_empty(autoinc_id_list) ? 1 : 2);
 
 		luaL_pushuint64(L, db->nChange);
@@ -77,6 +80,9 @@ port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
 				sql_info_key_strs[SQL_INFO_AUTOINCREMENT_IDS];
 			lua_setfield(L, -2, field_name);
 		}
+		break;
+	}
+	default: unreachable();
 	}
 }
 
-- 
2.15.1

[Tarantool-patches] [PATCH 08/15] sql: resurrect sql_bind_parameter_count() function

[Nikita Pettik]

This function is present in sql/vdbeapi.c source file, its prototype is
missing in any header file. It makes impossible to use it. Let's add
prototype declaration to sql/sqlInt.h (as other parameter
setters/getters) and refactor a bit in accordance with our codestyle.

Need for #2592
---
 src/box/sql/sqlInt.h  |  6 ++++++
 src/box/sql/vdbeapi.c | 10 +++-------
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 553a48d60..e6d8bc2e3 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -679,6 +679,12 @@ int
 sql_bind_zeroblob64(sql_stmt *, int,
 			sql_uint64);
 
+/**
+ * Return the number of wildcards that should be bound to.
+ */
+int
+sql_bind_parameter_count(sql_stmt *stmt);
+
 /**
  * Perform pointer parameter binding for the prepared sql
  * statement.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index a396f74d6..93b8906e5 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -1051,15 +1051,11 @@ sql_bind_zeroblob64(sql_stmt * pStmt, int i, sql_uint64 n)
 	return sql_bind_zeroblob(pStmt, i, n);
 }
 
-/*
- * Return the number of wildcards that can be potentially bound to.
- * This routine is added to support DBD::sql.
- */
 int
-sql_bind_parameter_count(sql_stmt * pStmt)
+sql_bind_parameter_count(sql_stmt *stmt)
 {
-	Vdbe *p = (Vdbe *) pStmt;
-	return p ? p->nVar : 0;
+	struct Vdbe *p = (struct Vdbe *) stmt;
+	return p->nVar;
 }
 
 /*
-- 
2.15.1

[Tarantool-patches] [PATCH 09/15] sql: resurrect sql_bind_parameter_name()

[Nikita Pettik]

We may need to get name of parameter to be bound by its index position.
So let's resurrect sql_bind_parameter_name() - put its prototype to
sql/sqlInt.h header and update codestyle.

Need for #2592
---
 src/box/sql/sqlInt.h  |  7 +++++++
 src/box/sql/vdbeapi.c | 14 ++++----------
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index e6d8bc2e3..1c02bd972 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -685,6 +685,13 @@ sql_bind_zeroblob64(sql_stmt *, int,
 int
 sql_bind_parameter_count(sql_stmt *stmt);
 
+/**
+ * Return the name of a wildcard parameter. Return NULL if the index
+ * is out of range or if the wildcard is unnamed.
+ */
+const char *
+sql_bind_parameter_name(sql_stmt *stmt, int i);
+
 /**
  * Perform pointer parameter binding for the prepared sql
  * statement.
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 93b8906e5..482a42288 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -1058,18 +1058,12 @@ sql_bind_parameter_count(sql_stmt *stmt)
 	return p->nVar;
 }
 
-/*
- * Return the name of a wildcard parameter.  Return NULL if the index
- * is out of range or if the wildcard is unnamed.
- *
- * The result is always UTF-8.
- */
 const char *
-sql_bind_parameter_name(sql_stmt * pStmt, int i)
+sql_bind_parameter_name(sql_stmt *stmt, int i)
 {
-	Vdbe *p = (Vdbe *) pStmt;
-	if (p == 0)
-		return 0;
+	struct Vdbe *p = (struct Vdbe *) stmt;
+	if (p == NULL)
+		return NULL;
 	return sqlVListNumToName(p->pVList, i);
 }
 
-- 
2.15.1

[Tarantool-patches] [PATCH 10/15] sql: add sql_schema_version()

[Nikita Pettik]

Let's introduce interface function to get schema version of prepared
statement. It is required since sturct sql_stmt (i.e. prepared
statement) is an opaque object and in fact is an alias to struct Vdbe.

Need for #2592
---
 src/box/sql/sqlInt.h  | 3 +++
 src/box/sql/vdbeapi.c | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 1c02bd972..3a1e6d9b4 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -561,6 +561,9 @@ sql_column_name(sql_stmt *, int N);
 const char *
 sql_column_datatype(sql_stmt *, int N);
 
+uint32_t
+sql_schema_version(sql_stmt *stmt);
+
 int
 sql_initialize(void);
 
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 482a42288..23c4a250a 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -798,6 +798,12 @@ sql_column_decltype(sql_stmt * pStmt, int N)
 			  COLNAME_DECLTYPE);
 }
 
+uint32_t
+sql_schema_version(sql_stmt *stmt)
+{
+	return ((struct Vdbe *)stmt)->schema_ver;
+}
+
 /******************************* sql_bind_  **************************
  *
  * Routines used to attach values to wildcards in a compiled SQL statement.
-- 
2.15.1

[Tarantool-patches] [PATCH 11/15] sql: introduce sql_stmt_sizeof() function

[Nikita Pettik]

To implement memory quota of prepared statement cache, we have to
estimate size of prepared statement. This function attempts at that.

Part of #2592
---
 src/box/execute.h     |  8 ++++++++
 src/box/sql/vdbeapi.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+)

diff --git a/src/box/execute.h b/src/box/execute.h
index 6702a18cc..d5b4d8421 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -116,6 +116,14 @@ extern const struct port_vtab port_sql_vtab;
 int
 sql_finalize(struct sql_stmt *stmt);
 
+/**
+ * Calculate estimated size of memory occupied by VM.
+ * See sqlVdbeMakeReady() for details concerning allocated
+ * memory.
+ */
+size_t
+sql_stmt_sizeof(const struct sql_stmt *stmt);
+
 /**
  * Prepare (compile into VDBE byte-code) statement.
  *
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 23c4a250a..f5a7c1ab7 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -804,6 +804,55 @@ sql_schema_version(sql_stmt *stmt)
 	return ((struct Vdbe *)stmt)->schema_ver;
 }
 
+size_t
+sql_stmt_sizeof(const sql_stmt *stmt)
+{
+	struct Vdbe *v = (struct Vdbe *) stmt;
+	size_t size = sizeof(*v);
+	/* Resulting set */
+	size += sizeof(struct Mem) * v->nResColumn * COLNAME_N;
+	/* Opcodes */
+	size += sizeof(struct VdbeOp) * v->nOp;
+	/* Memory cells */
+	size += sizeof(struct Mem) * v->nMem;
+	/* Bindings */
+	size += sizeof(struct Mem) * v->nVar;
+	/* Bindings included in the result set */
+	size += sizeof(uint32_t) * v->res_var_count;
+	/* Cursors */
+	size += sizeof(struct VdbeCursor *) * v->nCursor;
+
+	for (int i = 0; i < v->nOp; ++i) {
+		/* Estimate size of p4 operand. */
+		if (v->aOp[i].p4type != P4_NOTUSED) {
+			switch (v->aOp[i].p4type) {
+				case P4_DYNAMIC:
+				case P4_STATIC:
+					size += strlen(v->aOp[i].p4.z);
+					break;
+				case P4_BOOL:
+					size += sizeof(v->aOp[i].p4.b);
+					break;
+				case P4_INT32:
+					size += sizeof(v->aOp[i].p4.i);
+					break;
+				case P4_UINT64:
+				case P4_INT64:
+					size += sizeof(*v->aOp[i].p4.pI64);
+					break;
+				case P4_REAL:
+					size += sizeof(*v->aOp[i].p4.pReal);
+					break;
+				default:
+					size += sizeof(v->aOp[i].p4.p);
+					break;
+			}
+		}
+	}
+	size += strlen(v->zSql);
+	return size;
+}
+
 /******************************* sql_bind_  **************************
  *
  * Routines used to attach values to wildcards in a compiled SQL statement.
-- 
2.15.1

[Tarantool-patches] [PATCH 12/15] box: increment schema_version on ddl operations

[Nikita Pettik]

Some DDL operations such as SQL trigger alter, check and foreign
constraint alter don't result in schema version change. On the other
hand, we are going to rely on schema version to determine expired
prepared statements: for instance, if FK constraint has been created
after DML statement preparation, the latter may ignore FK constraint
(instead of proper "statement has expired" error). Let's fix it and
account schema change on each DDL operation.

Need for #2592
---
 src/box/alter.cc | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/box/alter.cc b/src/box/alter.cc
index 941b638ea..3b7f79b5a 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -4110,6 +4110,7 @@ on_replace_dd_trigger(struct trigger * /* trigger */, void *event)
 
 	txn_stmt_on_rollback(stmt, on_rollback);
 	txn_stmt_on_commit(stmt, on_commit);
+	++schema_version;
 }
 
 /**
@@ -4565,6 +4566,7 @@ on_replace_dd_fk_constraint(struct trigger * /* trigger*/, void *event)
 		space_reset_fk_constraint_mask(child_space);
 		space_reset_fk_constraint_mask(parent_space);
 	}
+	++schema_version;
 }
 
 /** Create an instance of check constraint definition by tuple. */
@@ -4777,6 +4779,7 @@ on_replace_dd_ck_constraint(struct trigger * /* trigger*/, void *event)
 	txn_stmt_on_commit(stmt, on_commit);
 
 	trigger_run_xc(&on_alter_space, space);
+	++schema_version;
 }
 
 /** A trigger invoked on replace in the _func_index space. */
-- 
2.15.1

[Tarantool-patches] [PATCH 13/15] sql: introduce cache for prepared statemets

[Nikita Pettik]

This patch introduces cache (as data structure) to handle prepared
statements and a set of interface functions (insert, delete, find,
erase) to operate on it. Cache under the hood is hash table with integer
ids as keys and prepared statements (struct sql_stmt which is an alias
for struct Vdbe) as values. Size of cache is regulated by quota via
box.cfg{sql_cache_size} parameter. Cache is supposed to be attached to
session, which means it is destroyed when session is ended. To erase
session manually, there's special handle - session's method
sql_cache_erase(). Default cache size is assumed to be 5 Mb (like in
PosgreSQL).

Part of #2592
---
 src/box/CMakeLists.txt          |   1 +
 src/box/box.cc                  |  19 +++++++
 src/box/box.h                   |   1 +
 src/box/errcode.h               |   1 +
 src/box/lua/cfg.cc              |  12 +++++
 src/box/lua/load_cfg.lua        |   3 ++
 src/box/lua/session.c           |  10 ++++
 src/box/prep_stmt.c             | 108 ++++++++++++++++++++++++++++++++++++++++
 src/box/prep_stmt.h             |  88 ++++++++++++++++++++++++++++++++
 src/box/session.cc              |  14 ++++++
 src/box/session.h               |   2 +
 test/app-tap/init_script.result |  37 +++++++-------
 test/box/admin.result           |   2 +
 test/box/cfg.result             |   7 +++
 test/box/cfg.test.lua           |   1 +
 test/box/misc.result            |   1 +
 16 files changed, 289 insertions(+), 18 deletions(-)
 create mode 100644 src/box/prep_stmt.c
 create mode 100644 src/box/prep_stmt.h

diff --git a/src/box/CMakeLists.txt b/src/box/CMakeLists.txt
index 9d2fcea4b..5fea534eb 100644
--- a/src/box/CMakeLists.txt
+++ b/src/box/CMakeLists.txt
@@ -124,6 +124,7 @@ add_library(box STATIC
     sql.c
     bind.c
     execute.c
+    prep_stmt.c
     wal.c
     call.c
     merger.c
diff --git a/src/box/box.cc b/src/box/box.cc
index d71afa114..341be9caa 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -590,6 +590,15 @@ box_check_vinyl_options(void)
 	}
 }
 
+static void
+box_check_sql_cache_size(int size)
+{
+	if (size < 0) {
+		tnt_raise(ClientError, ER_CFG, "sql_cache_size",
+			  "must be non-negative");
+	}
+}
+
 void
 box_check_config()
 {
@@ -611,6 +620,7 @@ box_check_config()
 	box_check_memtx_memory(cfg_geti64("memtx_memory"));
 	box_check_memtx_min_tuple_size(cfg_geti64("memtx_min_tuple_size"));
 	box_check_vinyl_options();
+	box_check_sql_cache_size(cfg_geti("sql_cache_size"));
 }
 
 /*
@@ -877,6 +887,14 @@ box_set_net_msg_max(void)
 				IPROTO_FIBER_POOL_SIZE_FACTOR);
 }
 
+void
+box_set_prepared_stmt_cache_size(void)
+{
+	int cache_sz = cfg_geti("sql_cache_size");
+	box_check_sql_cache_size(cache_sz);
+	sql_prepared_stmt_cache_set_size(cache_sz);
+}
+
 /* }}} configuration bindings */
 
 /**
@@ -2087,6 +2105,7 @@ box_cfg_xc(void)
 	box_check_instance_uuid(&instance_uuid);
 	box_check_replicaset_uuid(&replicaset_uuid);
 
+	box_set_prepared_stmt_cache_size();
 	box_set_net_msg_max();
 	box_set_readahead();
 	box_set_too_long_threshold();
diff --git a/src/box/box.h b/src/box/box.h
index ccd527bd5..f2e88c8a9 100644
--- a/src/box/box.h
+++ b/src/box/box.h
@@ -235,6 +235,7 @@ void box_set_replication_sync_lag(void);
 void box_set_replication_sync_timeout(void);
 void box_set_replication_skip_conflict(void);
 void box_set_net_msg_max(void);
+void box_set_prepared_stmt_cache_size(void);
 
 extern "C" {
 #endif /* defined(__cplusplus) */
diff --git a/src/box/errcode.h b/src/box/errcode.h
index c660b1c70..ee44f61b3 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -258,6 +258,7 @@ struct errcode_record {
 	/*203 */_(ER_BOOTSTRAP_READONLY,	"Trying to bootstrap a local read-only instance as master") \
 	/*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT,	"SQL expects exactly one argument returned from %s, got %d")\
 	/*205 */_(ER_FUNC_INVALID_RETURN_TYPE,	"Function '%s' returned value of invalid type: expected %s got %s") \
+	/*206 */_(ER_SQL_PREPARE,		"Failed to prepare SQL statement: %s") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/lua/cfg.cc b/src/box/lua/cfg.cc
index 4884ce013..42070fe49 100644
--- a/src/box/lua/cfg.cc
+++ b/src/box/lua/cfg.cc
@@ -274,6 +274,17 @@ lbox_cfg_set_net_msg_max(struct lua_State *L)
 	return 0;
 }
 
+static int
+lbox_set_prepared_stmt_cache_size(struct lua_State *L)
+{
+	try {
+		box_set_prepared_stmt_cache_size();
+	} catch (Exception *) {
+		luaT_error(L);
+	}
+	return 0;
+}
+
 static int
 lbox_cfg_set_worker_pool_threads(struct lua_State *L)
 {
@@ -378,6 +389,7 @@ box_lua_cfg_init(struct lua_State *L)
 		{"cfg_set_replication_sync_timeout", lbox_cfg_set_replication_sync_timeout},
 		{"cfg_set_replication_skip_conflict", lbox_cfg_set_replication_skip_conflict},
 		{"cfg_set_net_msg_max", lbox_cfg_set_net_msg_max},
+		{"cfg_set_sql_cache_size", lbox_set_prepared_stmt_cache_size},
 		{NULL, NULL}
 	};
 
diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua
index e7f62cf4e..fa74fcbab 100644
--- a/src/box/lua/load_cfg.lua
+++ b/src/box/lua/load_cfg.lua
@@ -81,6 +81,7 @@ local default_cfg = {
     feedback_host         = "https://feedback.tarantool.io",
     feedback_interval     = 3600,
     net_msg_max           = 768,
+    sql_cache_size        = 5 * 1024 * 1024,
 }
 
 -- types of available options
@@ -144,6 +145,7 @@ local template_cfg = {
     feedback_host         = 'string',
     feedback_interval     = 'number',
     net_msg_max           = 'number',
+    sql_cache_size        = 'number',
 }
 
 local function normalize_uri(port)
@@ -250,6 +252,7 @@ local dynamic_cfg = {
     instance_uuid           = check_instance_uuid,
     replicaset_uuid         = check_replicaset_uuid,
     net_msg_max             = private.cfg_set_net_msg_max,
+    sql_cache_size          = private.cfg_set_sql_cache_size,
 }
 
 local dynamic_cfg_skip_at_load = {
diff --git a/src/box/lua/session.c b/src/box/lua/session.c
index b9495e7a6..60f5c03a0 100644
--- a/src/box/lua/session.c
+++ b/src/box/lua/session.c
@@ -366,6 +366,15 @@ lbox_push_on_access_denied_event(struct lua_State *L, void *event)
 	return 3;
 }
 
+static int
+lbox_session_sql_cache_erase(struct lua_State *L)
+{
+	(void) L;
+	struct session *session = current_session();
+	sql_stmt_cache_erase(&session->prepared_stmt_cache);
+	return 0;
+}
+
 /**
  * Push a message using a protocol, depending on a session type.
  * @param L Lua state. First argument on the stack is data to
@@ -476,6 +485,7 @@ box_lua_session_init(struct lua_State *L)
 		{"on_auth", lbox_session_on_auth},
 		{"on_access_denied", lbox_session_on_access_denied},
 		{"push", lbox_session_push},
+		{"sql_cache_erase", lbox_session_sql_cache_erase},
 		{NULL, NULL}
 	};
 	luaL_register_module(L, sessionlib_name, sessionlib);
diff --git a/src/box/prep_stmt.c b/src/box/prep_stmt.c
new file mode 100644
index 000000000..d254ed453
--- /dev/null
+++ b/src/box/prep_stmt.c
@@ -0,0 +1,108 @@
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include "prep_stmt.h"
+
+#include "assoc.h"
+#include "error.h"
+#include "execute.h"
+#include "session.h"
+
+/** Default cache size is 5 Mb. */
+size_t prep_stmt_cache_size = 5 * 1024 * 1024;
+
+int
+sql_prepared_stmt_cache_insert(struct sql_stmt *stmt, uint32_t *id)
+{
+	assert(stmt != NULL);
+	struct session *session = current_session();
+	if (session->prepared_stmt_cache.mem_used + sql_stmt_sizeof(stmt) >
+	    session->prepared_stmt_cache.mem_quota) {
+		diag_set(ClientError, ER_SQL_PREPARE,
+			 "prepared statement cache is full");
+		return -1;
+	}
+	*id = session->prepared_stmt_cache.current_id;
+	const struct mh_i32ptr_node_t node = { *id, stmt } ;
+	struct mh_i32ptr_node_t *old_node = NULL;
+	struct mh_i32ptr_t *hash = session->prepared_stmt_cache.hash;
+	mh_int_t i = mh_i32ptr_put(hash, &node, &old_node, NULL);
+	if (i == mh_end(hash)) {
+		diag_set(OutOfMemory, 0, "mh_i32ptr_put", "mh_i32ptr_node_t");
+		return -1;
+	}
+	assert(old_node == NULL);
+	session->prepared_stmt_cache.current_id++;
+	session->prepared_stmt_cache.mem_used += sql_stmt_sizeof(stmt);
+	return 0;
+}
+
+void
+sql_prepared_stmt_cache_delete(struct sql_stmt *stmt, uint32_t id)
+{
+	struct session *session = current_session();
+	struct mh_i32ptr_t *hash = session->prepared_stmt_cache.hash;
+	mh_int_t id_i = mh_i32ptr_find(hash, id, NULL);
+	mh_i32ptr_del(hash, id_i, NULL);
+	session->prepared_stmt_cache.mem_used -= sql_stmt_sizeof(stmt);
+	sql_finalize(stmt);
+}
+
+struct sql_stmt *
+sql_prepared_stmt_cache_find(uint32_t id)
+{
+	struct session *session = current_session();
+	struct mh_i32ptr_t *hash = session->prepared_stmt_cache.hash;
+	mh_int_t stmt = mh_i32ptr_find(hash, id, NULL);
+	if (stmt == mh_end(hash))
+		return NULL;
+	return mh_i32ptr_node(hash, stmt)->val;;
+}
+
+void
+sql_stmt_cache_erase(struct prep_stmt_cache *stmts)
+{
+	assert(stmts != NULL);
+	mh_int_t i;
+	mh_foreach(stmts->hash, i) {
+		struct sql_stmt *stmt =
+			(struct sql_stmt *) mh_i32ptr_node(stmts->hash, i)->val;
+		sql_finalize(stmt);
+		mh_i32ptr_del(stmts->hash, i, NULL);
+	}
+	/* Reset size to the default state. */
+	stmts->mem_used = sizeof(*stmts);
+}
+
+void
+sql_prepared_stmt_cache_set_size(size_t size)
+{
+	prep_stmt_cache_size = size;
+}
diff --git a/src/box/prep_stmt.h b/src/box/prep_stmt.h
new file mode 100644
index 000000000..4de0c321e
--- /dev/null
+++ b/src/box/prep_stmt.h
@@ -0,0 +1,88 @@
+#ifndef INCLUDES_PREP_STMT_H
+#define INCLUDES_PREP_STMT_H
+/*
+ * Copyright 2010-2019, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+#include <stdint.h>
+#include <stdio.h>
+
+#if defined(__cplusplus)
+extern "C" {
+#endif
+
+extern size_t prep_stmt_cache_size;
+
+struct prep_stmt_cache {
+	/** Size of memory currently occupied by prepared statements. */
+	size_t mem_used;
+	/**
+	 * Cache is created per session, so cache can't be re-sized
+	 * after @prep_stmt_cache is changed. Max memory size that
+	 * can be used for cache.
+	 */
+	size_t mem_quota;
+	/** Counter of sequential ids. */
+	uint32_t current_id;
+	/** Query ID -> struct prepared_stmt hash.*/
+	struct mh_i32ptr_t *hash;
+};
+
+struct sql_stmt;
+
+/**
+ * Save prepared statement to the prepared statement cache.
+ * Account cache size change. If the cache is full (i.e. memory
+ * quota is exceeded) diag error is raised. In case of success
+ * return id of prepared statement via output parameter @id.
+ */
+int
+sql_prepared_stmt_cache_insert(struct sql_stmt *stmt, uint32_t *id);
+
+/** Find entry by id. In case of search fail it returns NULL. */
+struct sql_stmt *
+sql_prepared_stmt_cache_find(uint32_t id);
+
+/** Remove entry from cache. Account cache size change. */
+void
+sql_prepared_stmt_cache_delete(struct sql_stmt *stmt, uint32_t id);
+
+/** Remove all elements from cache and deallocate them. */
+void
+sql_stmt_cache_erase(struct prep_stmt_cache *stmts);
+
+/** Set @prep_stmt_cache_size value. */
+void
+sql_prepared_stmt_cache_set_size(size_t size);
+
+#if defined(__cplusplus)
+} /* extern "C" { */
+#endif
+
+#endif
diff --git a/src/box/session.cc b/src/box/session.cc
index 59bf226dd..c34be4228 100644
--- a/src/box/session.cc
+++ b/src/box/session.cc
@@ -36,6 +36,7 @@
 #include "user.h"
 #include "error.h"
 #include "tt_static.h"
+#include "execute.h"
 
 const char *session_type_strs[] = {
 	"background",
@@ -147,6 +148,17 @@ session_create(enum session_type type)
 	struct mh_i64ptr_node_t node;
 	node.key = session->id;
 	node.val = session;
+	session->prepared_stmt_cache.mem_quota = prep_stmt_cache_size;
+	session->prepared_stmt_cache.mem_used =
+		sizeof(session->prepared_stmt_cache);
+	session->prepared_stmt_cache.hash = mh_i32ptr_new();
+	if (session->prepared_stmt_cache.hash == NULL) {
+		diag_set(OutOfMemory, 0, "mh_i32ptr_new",
+			  "prepared statement cache");
+		mempool_free(&session_pool, session);
+		return NULL;
+	}
+	session->prepared_stmt_cache.current_id = 0;
 
 	mh_int_t k = mh_i64ptr_put(session_registry, &node, NULL, NULL);
 
@@ -232,6 +244,8 @@ session_destroy(struct session *session)
 	session_storage_cleanup(session->id);
 	struct mh_i64ptr_node_t node = { session->id, NULL };
 	mh_i64ptr_remove(session_registry, &node, NULL);
+	sql_stmt_cache_erase(&session->prepared_stmt_cache);
+	mh_i32ptr_delete(session->prepared_stmt_cache.hash);
 	mempool_free(&session_pool, session);
 }
 
diff --git a/src/box/session.h b/src/box/session.h
index 85a2d940b..96837ba07 100644
--- a/src/box/session.h
+++ b/src/box/session.h
@@ -36,6 +36,7 @@
 #include "fiber.h"
 #include "user.h"
 #include "authentication.h"
+#include "prep_stmt.h"
 
 #if defined(__cplusplus)
 extern "C" {
@@ -105,6 +106,7 @@ struct session {
 	struct credentials credentials;
 	/** Trigger for fiber on_stop to cleanup created on-demand session */
 	struct trigger fiber_on_stop;
+	struct prep_stmt_cache prepared_stmt_cache;
 };
 
 struct session_vtab {
diff --git a/test/app-tap/init_script.result b/test/app-tap/init_script.result
index 799297ba0..551a0bbeb 100644
--- a/test/app-tap/init_script.result
+++ b/test/app-tap/init_script.result
@@ -31,24 +31,25 @@ box.cfg
 26	replication_sync_timeout:300
 27	replication_timeout:1
 28	slab_alloc_factor:1.05
-29	strip_core:true
-30	too_long_threshold:0.5
-31	vinyl_bloom_fpr:0.05
-32	vinyl_cache:134217728
-33	vinyl_dir:.
-34	vinyl_max_tuple_size:1048576
-35	vinyl_memory:134217728
-36	vinyl_page_size:8192
-37	vinyl_read_threads:1
-38	vinyl_run_count_per_level:2
-39	vinyl_run_size_ratio:3.5
-40	vinyl_timeout:60
-41	vinyl_write_threads:4
-42	wal_dir:.
-43	wal_dir_rescan_delay:2
-44	wal_max_size:268435456
-45	wal_mode:write
-46	worker_pool_threads:4
+29	sql_cache_size:5242880
+30	strip_core:true
+31	too_long_threshold:0.5
+32	vinyl_bloom_fpr:0.05
+33	vinyl_cache:134217728
+34	vinyl_dir:.
+35	vinyl_max_tuple_size:1048576
+36	vinyl_memory:134217728
+37	vinyl_page_size:8192
+38	vinyl_read_threads:1
+39	vinyl_run_count_per_level:2
+40	vinyl_run_size_ratio:3.5
+41	vinyl_timeout:60
+42	vinyl_write_threads:4
+43	wal_dir:.
+44	wal_dir_rescan_delay:2
+45	wal_max_size:268435456
+46	wal_mode:write
+47	worker_pool_threads:4
 --
 -- Test insert from detached fiber
 --
diff --git a/test/box/admin.result b/test/box/admin.result
index 6126f3a97..852c1cde8 100644
--- a/test/box/admin.result
+++ b/test/box/admin.result
@@ -83,6 +83,8 @@ cfg_filter(box.cfg)
     - 1
   - - slab_alloc_factor
     - 1.05
+  - - sql_cache_size
+    - 5242880
   - - strip_core
     - true
   - - too_long_threshold
diff --git a/test/box/cfg.result b/test/box/cfg.result
index 5370bb870..9542e6375 100644
--- a/test/box/cfg.result
+++ b/test/box/cfg.result
@@ -71,6 +71,8 @@ cfg_filter(box.cfg)
  |     - 1
  |   - - slab_alloc_factor
  |     - 1.05
+ |   - - sql_cache_size
+ |     - 5242880
  |   - - strip_core
  |     - true
  |   - - too_long_threshold
@@ -170,6 +172,8 @@ cfg_filter(box.cfg)
  |     - 1
  |   - - slab_alloc_factor
  |     - 1.05
+ |   - - sql_cache_size
+ |     - 5242880
  |   - - strip_core
  |     - true
  |   - - too_long_threshold
@@ -315,6 +319,9 @@ box.cfg{memtx_memory = box.cfg.memtx_memory}
 box.cfg{vinyl_memory = box.cfg.vinyl_memory}
  | ---
  | ...
+box.cfg{sql_cache_size = 1024}
+ | ---
+ | ...
 
 --------------------------------------------------------------------------------
 -- Test of default cfg options
diff --git a/test/box/cfg.test.lua b/test/box/cfg.test.lua
index 56ccb6767..e129568e6 100644
--- a/test/box/cfg.test.lua
+++ b/test/box/cfg.test.lua
@@ -51,6 +51,7 @@ box.cfg{replicaset_uuid = '12345678-0123-5678-1234-abcdefabcdef'}
 
 box.cfg{memtx_memory = box.cfg.memtx_memory}
 box.cfg{vinyl_memory = box.cfg.vinyl_memory}
+box.cfg{sql_cache_size = 1024}
 
 --------------------------------------------------------------------------------
 -- Test of default cfg options
diff --git a/test/box/misc.result b/test/box/misc.result
index b2930515b..78ffbf1dc 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -535,6 +535,7 @@ t;
   203: box.error.BOOTSTRAP_READONLY
   204: box.error.SQL_FUNC_WRONG_RET_COUNT
   205: box.error.FUNC_INVALID_RETURN_TYPE
+  206: box.error.SQL_PREPARE
 ...
 test_run:cmd("setopt delimiter ''");
 ---
-- 
2.15.1

[Tarantool-patches] [PATCH 14/15] box: introduce prepared statements

[Nikita Pettik]

This patch introduces local prepared statements. Support of prepared
statements in IProto protocol and netbox is added in the next patch.

Prepared statement is an opaque instance of SQL Virtual Machine. It can
be executed several times without necessity of query recompilation. To
achieve this one can use box.prepare(...) function. It takes string of
SQL query to be prepared; returns extended set of meta-information
including query id, parameter's types and names, types and names of
columns of the resulting set, count of parameters to be bound. Lua
object representing result of :prepare() call also features two methods
- :unprepare() and :execute(). First one corresponds to
box.unprepare(stmt.query_id), i.e. automatically substitutes id of prepared
statement to be unprepared. After "unpreparation" statement is removed
from cache and all resources related to it are released.
:execute() method does the same - it unfolds into box.execute(stmt.id).
Query ids are local to each session, sequential and start from 0.
After schema changes all prepared statement located in cache are
considered to be expired - cache should be erased and all statements
re-prepared. It can be done with box.session.sql_cache_erase().
It is worth noting that box.execute() now accepts two types of first
parameter: in case it is string then original :execute() method is
called (which treats it as string containing SQL statement to be
compiled and executed); if it is number - it is supposed to be id of
prepared statement to be executed.

SQL cache memory limit is regulated by box{sql_cache_size} which can be
set dynamically.

Part of #2592
---
 src/box/errcode.h          |   1 +
 src/box/execute.c          |  72 ++++-
 src/box/execute.h          |  29 +-
 src/box/lua/execute.c      | 211 ++++++++++++++-
 src/box/lua/execute.h      |   2 +-
 src/box/lua/init.c         |   2 +-
 src/box/sql/prepare.c      |   9 -
 test/box/misc.result       |   3 +
 test/sql/engine.cfg        |   3 +
 test/sql/prepared.result   | 654 +++++++++++++++++++++++++++++++++++++++++++++
 test/sql/prepared.test.lua | 222 +++++++++++++++
 11 files changed, 1184 insertions(+), 24 deletions(-)
 create mode 100644 test/sql/prepared.result
 create mode 100644 test/sql/prepared.test.lua

diff --git a/src/box/errcode.h b/src/box/errcode.h
index ee44f61b3..6ecd37d75 100644
--- a/src/box/errcode.h
+++ b/src/box/errcode.h
@@ -259,6 +259,7 @@ struct errcode_record {
 	/*204 */_(ER_SQL_FUNC_WRONG_RET_COUNT,	"SQL expects exactly one argument returned from %s, got %d")\
 	/*205 */_(ER_FUNC_INVALID_RETURN_TYPE,	"Function '%s' returned value of invalid type: expected %s got %s") \
 	/*206 */_(ER_SQL_PREPARE,		"Failed to prepare SQL statement: %s") \
+	/*207 */_(ER_WRONG_QUERY_ID,		"Prepared statement with corresponding id %d does not exist") \
 
 /*
  * !IMPORTANT! Please follow instructions at start of the file
diff --git a/src/box/execute.c b/src/box/execute.c
index d2a999099..3fd1afd8a 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -45,6 +45,7 @@
 #include "tuple.h"
 #include "sql/vdbe.h"
 #include "box/lua/execute.h"
+#include "box/prep_stmt.h"
 
 const char *sql_info_key_strs[] = {
 	"row_count",
@@ -411,6 +412,35 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
 	return 0;
 }
 
+int
+sql_prepare(const char *sql, int len, struct port *port)
+{
+	struct sql_stmt *stmt;
+	if (sql_compile(sql, len, NULL, &stmt, NULL) != 0)
+		return -1;
+	uint32_t stmt_id;
+	if (sql_prepared_stmt_cache_insert(stmt, &stmt_id) != 0)
+		return -1;
+	enum sql_dump_format dump_format = sql_column_count(stmt) > 0 ?
+					   DQL_PREPARE : DML_PREPARE;
+	port_sql_create(port, stmt, dump_format, PREPARE);
+	((struct port_sql *)port)->query_id = stmt_id;
+
+	return 0;
+}
+
+int
+sql_unprepare(uint32_t query_id)
+{
+	struct sql_stmt *stmt = sql_prepared_stmt_cache_find(query_id);
+	if (stmt == NULL) {
+		diag_set(ClientError, ER_WRONG_QUERY_ID, query_id);
+		return -1;
+	}
+	sql_prepared_stmt_cache_delete(stmt, query_id);
+	return 0;
+}
+
 /**
  * Execute prepared SQL statement.
  *
@@ -426,7 +456,7 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
  * @retval  0 Success.
  * @retval -1 Error.
  */
-static inline int
+static int
 sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region)
 {
 	int rc, column_count = sql_column_count(stmt);
@@ -448,6 +478,46 @@ sql_execute(struct sql_stmt *stmt, struct port *port, struct region *region)
 	return 0;
 }
 
+int
+sql_execute_prepared(uint32_t query_id, const struct sql_bind *bind,
+		     uint32_t bind_count, struct port *port,
+		     struct region *region)
+{
+	struct sql_stmt *stmt = sql_prepared_stmt_cache_find(query_id);
+	if (stmt == NULL) {
+		diag_set(ClientError, ER_WRONG_QUERY_ID, query_id);
+		return -1;
+	}
+	if (sql_schema_version(stmt) != box_schema_version()) {
+		diag_set(ClientError, ER_SQL_EXECUTE, "statement has expired");
+		return -1;
+	}
+	/*
+	 * Each fiber has its own session so even if during statement
+	 * execution yield occurs, another fiber can't access this
+	 * particular statement.
+	 */
+	if (sql_stmt_busy(stmt)) {
+		panic("Prepared statement is malformed: "
+		      "its status is RUN before execution");
+	}
+	if (sql_bind(stmt, bind, bind_count) != 0)
+		return -1;
+	enum sql_dump_format dump_format = sql_column_count(stmt) > 0 ?
+					   DQL_EXECUTE : DML_EXECUTE;
+	port_sql_create(port, stmt, dump_format, EXECUTE_PREPARED);
+	if (sql_bind(stmt, bind, bind_count) != 0)
+		return -1;
+	if (sql_execute(stmt, port, region) != 0) {
+		port_destroy(port);
+		sql_reset(stmt);
+		return -1;
+	}
+	sql_reset(stmt);
+
+	return 0;
+}
+
 int
 sql_prepare_and_execute(const char *sql, int len, const struct sql_bind *bind,
 			uint32_t bind_count, struct port *port,
diff --git a/src/box/execute.h b/src/box/execute.h
index d5b4d8421..ef2648b34 100644
--- a/src/box/execute.h
+++ b/src/box/execute.h
@@ -109,6 +109,11 @@ struct port_sql {
 	uint8_t dump_format;
 	/** enum sql_request_type */
 	uint8_t request;
+	/**
+	 * In case of "prepare" request user receives id of query
+	 * using which query can be executed later.
+	 */
+	uint32_t query_id;
 };
 
 extern const struct port_vtab port_sql_vtab;
@@ -128,13 +133,27 @@ sql_stmt_sizeof(const struct sql_stmt *stmt);
  * Prepare (compile into VDBE byte-code) statement.
  *
  * @param sql UTF-8 encoded SQL statement.
- * @param length Length of @param sql in bytes.
- * @param[out] stmt A pointer to the prepared statement.
- * @param[out] sql_tail End of parsed string.
+ * @param len Length of @param sql in bytes.
+ * @param port Port to store request response.
+ */
+int
+sql_prepare(const char *sql, int len, struct port *port);
+
+/**
+ * Remove entry from cache and release any occupied resources.
+ * In case of wrong query id the error is raised.
+ */
+int
+sql_unprepare(uint32_t query_id);
+
+/**
+ * Execute prepared statement via given id. At the end statement
+ * is not destroyed. Otherwise this function is similar to sql_execute().
  */
 int
-sql_prepare(const char *sql, int length, struct sql_stmt **stmt,
-	    const char **sql_tail);
+sql_execute_prepared(uint32_t query_id, const struct sql_bind *bind,
+		     uint32_t bind_count, struct port *port,
+		     struct region *region);
 
 #if defined(__cplusplus)
 } /* extern "C" { */
diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
index 1b2f8d235..981f4e325 100644
--- a/src/box/lua/execute.c
+++ b/src/box/lua/execute.c
@@ -5,6 +5,8 @@
 #include "box/port.h"
 #include "box/execute.h"
 #include "box/bind.h"
+#include "box/prep_stmt.h"
+#include "box/schema.h"
 
 /**
  * Serialize a description of the prepared statement.
@@ -38,6 +40,102 @@ lua_sql_get_metadata(struct sql_stmt *stmt, struct lua_State *L,
 	}
 }
 
+static inline void
+lua_sql_get_params_metadata(struct sql_stmt *stmt, struct lua_State *L)
+{
+	int bind_count = sql_bind_parameter_count(stmt);
+	lua_createtable(L, bind_count, 0);
+	for (int i = 0; i < bind_count; ++i) {
+		lua_createtable(L, 0, 2);
+		const char *name = sql_bind_parameter_name(stmt, i);
+		if (name == NULL)
+			name = "?";
+		const char *type = "ANY";
+		lua_pushstring(L, name);
+		lua_setfield(L, -2, "name");
+		lua_pushstring(L, type);
+		lua_setfield(L, -2, "type");
+		lua_rawseti(L, -2, i + 1);
+	}
+}
+
+/** Forward declaration to avoid code movement. */
+static int
+lbox_execute(struct lua_State *L);
+
+/**
+ * Prepare SQL statement: compile it and save to the cache.
+ * In fact it is wrapper around box.execute() which unfolds
+ * it to box.execute(stmt.query_id).
+ */
+static int
+lbox_execute_prepared(struct lua_State *L)
+{
+	int top = lua_gettop(L);
+
+	if ((top != 1 && top != 2) || ! lua_istable(L, 1))
+		return luaL_error(L, "Usage: statement:execute([, params])");
+	lua_getfield(L, 1, "query_id");
+	if (!lua_isnumber(L, -1))
+		return luaL_error(L, "Query id is expected to be numeric");
+	lua_remove(L, 1);
+	if (top == 2) {
+		/*
+		 * Stack state (before remove operation):
+		 * 1 Prepared statement object (Lua table)
+		 * 2 Bindings (Lua table)
+		 * 3 Id (fetched from PS table) - top of stack
+		 *
+		 * We should make it suitable to pass arguments to
+		 * lbox_execute(), i.e. after manipulations stack
+		 * should look like:
+		 * 1 Id
+		 * 2 Bindings - top of stack
+		 * Since there's no swap operation, we firstly remove
+		 * PS object, then copy table of values to be bound to
+		 * the top of stack (push), and finally remove original
+		 * bindings from stack.
+		 */
+		lua_pushvalue(L, 1);
+		lua_remove(L, 1);
+	}
+	return lbox_execute(L);
+}
+
+/**
+ * Unprepare statement: remove it from prepared statements cache.
+ * This function can be called two ways: as member of prepared
+ * statement handle ()
+ */
+static int
+lbox_unprepare(struct lua_State *L)
+{
+	int top = lua_gettop(L);
+
+	if (top != 1 || (! lua_istable(L, 1) && ! lua_isnumber(L, 1))) {
+		return luaL_error(L, "Usage: statement:unprepare() or "\
+				     "box.unprepare(query_id)");
+	}
+	lua_Integer query_id;
+	if (lua_istable(L, 1)) {
+		lua_getfield(L, -1, "query_id");
+		if (! lua_isnumber(L, -1)) {
+			return luaL_error(L, "Query id is expected "\
+					     " to be numeric");
+		}
+		query_id = lua_tointeger(L, -1);
+		lua_pop(L, 1);
+	} else {
+		query_id = lua_tonumber(L, 1);
+	}
+	if (query_id < 0)
+		return luaL_error(L, "Query id can't be negative");
+	if (sql_unprepare((uint32_t) query_id) != 0)
+		return luaT_push_nil_and_error(L);
+	return 0;
+}
+
+
 void
 port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
 {
@@ -82,6 +180,61 @@ port_sql_dump_lua(struct port *port, struct lua_State *L, bool is_flat)
 		}
 		break;
 	}
+	case DQL_PREPARE: {
+		/* Format is following:
+		 * query_id,
+		 * param_count,
+		 * params {name, type},
+		 * metadata {name, type}
+		 * execute(), unprepare()
+		 */
+		lua_createtable(L, 0, 6);
+		/* query_id */
+		luaL_pushuint64(L, port_sql->query_id);
+		lua_setfield(L, -2, "query_id");
+		/* param_count */
+		luaL_pushuint64(L, sql_bind_parameter_count(stmt));
+		lua_setfield(L, -2, "param_count");
+		/* params map */
+		lua_sql_get_params_metadata(stmt, L);
+		lua_setfield(L, -2, "params");
+		/* metadata */
+		lua_sql_get_metadata(stmt, L, sql_column_count(stmt));
+		lua_setfield(L, -2, "metadata");
+		/* execute function */
+		lua_pushcfunction(L, lbox_execute_prepared);
+		lua_setfield(L, -2, "execute");
+		/* unprepare function */
+		lua_pushcfunction(L, lbox_unprepare);
+		lua_setfield(L, -2, "unprepare");
+		break;
+	}
+	case DML_PREPARE : {
+		assert(((struct port_tuple *) port)->size == 0);
+		/* Format is following:
+		 * query_id,
+		 * param_count,
+		 * params {name, type},
+		 * execute(), unprepare()
+		 */
+		lua_createtable(L, 0, 5);
+		/* query_id */
+		luaL_pushuint64(L, port_sql->query_id);
+		lua_setfield(L, -2, "query_id");
+		/* param_count */
+		luaL_pushuint64(L, sql_bind_parameter_count(stmt));
+		lua_setfield(L, -2, "param_count");
+		/* params map */
+		lua_sql_get_params_metadata(stmt, L);
+		lua_setfield(L, -2, "params");
+		/* execute function */
+		lua_pushcfunction(L, lbox_execute_prepared);
+		lua_setfield(L, -2, "execute");
+		/* unprepare function */
+		lua_pushcfunction(L, lbox_unprepare);
+		lua_setfield(L, -2, "unprepare");
+		break;
+	}
 	default: unreachable();
 	}
 }
@@ -251,10 +404,10 @@ lbox_execute(struct lua_State *L)
 	struct port port;
 	int top = lua_gettop(L);
 
-	if ((top != 1 && top != 2) || ! lua_isstring(L, 1))
-		return luaL_error(L, "Usage: box.execute(sqlstring[, params])");
-
-	const char *sql = lua_tolstring(L, 1, &length);
+	if ((top != 1 && top != 2) || ! lua_isstring(L, 1)) {
+		return luaL_error(L, "Usage: box.execute(sqlstring[, params]) "
+				     "or box.execute(query_id[, params])");
+	}
 
 	if (top == 2) {
 		if (! lua_istable(L, 2))
@@ -263,9 +416,44 @@ lbox_execute(struct lua_State *L)
 		if (bind_count < 0)
 			return luaT_push_nil_and_error(L);
 	}
+	/*
+	 * lua_isstring() returns true for numeric values as well,
+	 * so test explicit type instead.
+	 */
+	if (lua_type(L, 1) == LUA_TSTRING) {
+		const char *sql = lua_tolstring(L, 1, &length);
+		if (sql_prepare_and_execute(sql, length, bind, bind_count, &port,
+					    &fiber()->gc) != 0)
+			return luaT_push_nil_and_error(L);
+	} else {
+		assert(lua_type(L, 1) == LUA_TNUMBER);
+		lua_Integer query_id = lua_tointeger(L, 1);
+		if (query_id < 0)
+			return luaL_error(L, "Query id can't be negative");
+		if (sql_execute_prepared(query_id, bind, bind_count, &port,
+					 &fiber()->gc) != 0)
+			return luaT_push_nil_and_error(L);
+	}
+	port_dump_lua(&port, L, false);
+	port_destroy(&port);
+	return 1;
+}
 
-	if (sql_prepare_and_execute(sql, length, bind, bind_count, &port,
-				    &fiber()->gc) != 0)
+/**
+ * Prepare SQL statement: compile it and save to the cache.
+ */
+static int
+lbox_prepare(struct lua_State *L)
+{
+	size_t length;
+	struct port port;
+	int top = lua_gettop(L);
+
+	if ((top != 1 && top != 2) || ! lua_isstring(L, 1))
+		return luaL_error(L, "Usage: box.prepare(sqlstring[, params])");
+
+	const char *sql = lua_tolstring(L, 1, &length);
+	if (sql_prepare(sql, length, &port) != 0)
 		return luaT_push_nil_and_error(L);
 	port_dump_lua(&port, L, false);
 	port_destroy(&port);
@@ -273,11 +461,20 @@ lbox_execute(struct lua_State *L)
 }
 
 void
-box_lua_execute_init(struct lua_State *L)
+box_lua_sql_init(struct lua_State *L)
 {
 	lua_getfield(L, LUA_GLOBALSINDEX, "box");
 	lua_pushstring(L, "execute");
 	lua_pushcfunction(L, lbox_execute);
 	lua_settable(L, -3);
+
+	lua_pushstring(L, "prepare");
+	lua_pushcfunction(L, lbox_prepare);
+	lua_settable(L, -3);
+
+	lua_pushstring(L, "unprepare");
+	lua_pushcfunction(L, lbox_unprepare);
+	lua_settable(L, -3);
+
 	lua_pop(L, 1);
 }
diff --git a/src/box/lua/execute.h b/src/box/lua/execute.h
index 23e193fa4..bafd67615 100644
--- a/src/box/lua/execute.h
+++ b/src/box/lua/execute.h
@@ -66,6 +66,6 @@ lua_sql_bind_list_decode(struct lua_State *L, struct sql_bind **out_bind,
 			 int idx);
 
 void
-box_lua_execute_init(struct lua_State *L);
+box_lua_sql_init(struct lua_State *L);
 
 #endif /* INCLUDES_TARANTOOL_LUA_EXECUTE_H */
diff --git a/src/box/lua/init.c b/src/box/lua/init.c
index 7ffed409d..7be520e09 100644
--- a/src/box/lua/init.c
+++ b/src/box/lua/init.c
@@ -314,7 +314,7 @@ box_lua_init(struct lua_State *L)
 	box_lua_ctl_init(L);
 	box_lua_session_init(L);
 	box_lua_xlog_init(L);
-	box_lua_execute_init(L);
+	box_lua_sql_init(L);
 	luaopen_net_box(L);
 	lua_pop(L, 1);
 	tarantool_lua_console_init(L);
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 780085e56..61a50cbdc 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -193,15 +193,6 @@ sqlReprepare(Vdbe * p)
 	return 0;
 }
 
-int
-sql_prepare(const char *sql, int length, struct sql_stmt **stmt,
-	    const char **sql_tail)
-{
-	int rc = sql_compile(sql, length, 0, stmt, sql_tail);
-	assert(rc == 0 || stmt == NULL || *stmt == NULL);
-	return rc;
-}
-
 void
 sql_parser_create(struct Parse *parser, struct sql *db, uint32_t sql_flags)
 {
diff --git a/test/box/misc.result b/test/box/misc.result
index 78ffbf1dc..b75f615d5 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -73,6 +73,7 @@ t
   - on_commit
   - on_rollback
   - once
+  - prepare
   - priv
   - rollback
   - rollback_to_savepoint
@@ -86,6 +87,7 @@ t
   - space
   - stat
   - tuple
+  - unprepare
 ...
 t = nil
 ---
@@ -536,6 +538,7 @@ t;
   204: box.error.SQL_FUNC_WRONG_RET_COUNT
   205: box.error.FUNC_INVALID_RETURN_TYPE
   206: box.error.SQL_PREPARE
+  207: box.error.WRONG_QUERY_ID
 ...
 test_run:cmd("setopt delimiter ''");
 ---
diff --git a/test/sql/engine.cfg b/test/sql/engine.cfg
index 284c42082..a1b4b0fc5 100644
--- a/test/sql/engine.cfg
+++ b/test/sql/engine.cfg
@@ -9,6 +9,9 @@
         "remote": {"remote": "true"},
         "local": {"remote": "false"}
     },
+    "prepared.test.lua": {
+        "local": {"remote": "false"}
+    },
     "*": {
         "memtx": {"engine": "memtx"},
         "vinyl": {"engine": "vinyl"}
diff --git a/test/sql/prepared.result b/test/sql/prepared.result
new file mode 100644
index 000000000..1646af94f
--- /dev/null
+++ b/test/sql/prepared.result
@@ -0,0 +1,654 @@
+-- test-run result file version 2
+remote = require('net.box')
+ | ---
+ | ...
+test_run = require('test_run').new()
+ | ---
+ | ...
+fiber = require('fiber')
+ | ---
+ | ...
+
+-- Wrappers to make remote and local execution interface return
+-- same result pattern.
+--
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+execute = function(...)
+    local res, err = box.execute(...)
+    if err ~= nil then
+        error(err)
+    end
+    return res
+end;
+ | ---
+ | ...
+prepare = function(...)
+    local res, err = box.prepare(...)
+    if err ~= nil then
+        error(err)
+    end
+    return res
+end;
+ | ---
+ | ...
+unprepare = function(...)
+    local res, err = box.unprepare(...)
+    if err ~= nil then
+        error(err)
+    end
+    return res
+end;
+ | ---
+ | ...
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | - true
+ | ...
+
+-- Test local interface and basic capabilities of prepared statements.
+--
+execute('CREATE TABLE test (id INT PRIMARY KEY, a NUMBER, b TEXT)')
+ | ---
+ | - row_count: 1
+ | ...
+space = box.space.TEST
+ | ---
+ | ...
+space:replace{1, 2, '3'}
+ | ---
+ | - [1, 2, '3']
+ | ...
+space:replace{4, 5, '6'}
+ | ---
+ | - [4, 5, '6']
+ | ...
+space:replace{7, 8.5, '9'}
+ | ---
+ | - [7, 8.5, '9']
+ | ...
+s, e = prepare("SELECT * FROM test WHERE id = ? AND a = ?;")
+ | ---
+ | ...
+assert(e == nil)
+ | ---
+ | - true
+ | ...
+assert(s ~= nil)
+ | ---
+ | - true
+ | ...
+s.query_id
+ | ---
+ | - 0
+ | ...
+s.metadata
+ | ---
+ | - - name: ID
+ |     type: integer
+ |   - name: A
+ |     type: number
+ |   - name: B
+ |     type: string
+ | ...
+s.params
+ | ---
+ | - - name: '?'
+ |     type: ANY
+ |   - name: '?'
+ |     type: ANY
+ | ...
+s.params_count
+ | ---
+ | - null
+ | ...
+execute(s.query_id, {1, 2})
+ | ---
+ | - metadata:
+ |   - name: ID
+ |     type: integer
+ |   - name: A
+ |     type: number
+ |   - name: B
+ |     type: string
+ |   rows:
+ |   - [1, 2, '3']
+ | ...
+execute(s.query_id, {1, 3})
+ | ---
+ | - metadata:
+ |   - name: ID
+ |     type: integer
+ |   - name: A
+ |     type: number
+ |   - name: B
+ |     type: string
+ |   rows: []
+ | ...
+s:execute({1, 2})
+ | ---
+ | - metadata:
+ |   - name: ID
+ |     type: integer
+ |   - name: A
+ |     type: number
+ |   - name: B
+ |     type: string
+ |   rows:
+ |   - [1, 2, '3']
+ | ...
+s:execute({1, 3})
+ | ---
+ | - metadata:
+ |   - name: ID
+ |     type: integer
+ |   - name: A
+ |     type: number
+ |   - name: B
+ |     type: string
+ |   rows: []
+ | ...
+s:unprepare()
+ | ---
+ | ...
+
+-- Test preparation of different types of queries.
+-- Let's start from DDL. It doesn't make much sense since
+-- any prepared DDL statement can be executed once, but
+-- anyway make sure that no crashes occur.
+--
+s = prepare("CREATE INDEX i1 ON test(a)")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+s = prepare("DROP INDEX i1 ON test;")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+s = prepare("CREATE VIEW v AS SELECT * FROM test;")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+s = prepare("DROP VIEW v;")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+s = prepare("ALTER TABLE test RENAME TO test1")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 0
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+box.execute("CREATE TABLE test2 (id INT PRIMARY KEY);")
+ | ---
+ | - row_count: 1
+ | ...
+s = prepare("ALTER TABLE test2 ADD CONSTRAINT fk1 FOREIGN KEY (id) REFERENCES test2")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+box.space.TEST2:drop()
+ | ---
+ | ...
+
+s = prepare("CREATE TRIGGER tr1 INSERT ON test1 FOR EACH ROW BEGIN DELETE FROM test1; END;")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+s = prepare("DROP TRIGGER tr1;")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+s = prepare("DROP TABLE test1;")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id)
+ | ---
+ | - error: 'Failed to execute SQL statement: statement has expired'
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+-- DQL
+--
+execute('CREATE TABLE test (id INT PRIMARY KEY, a NUMBER, b TEXT)')
+ | ---
+ | - row_count: 1
+ | ...
+space = box.space.TEST
+ | ---
+ | ...
+space:replace{1, 2, '3'}
+ | ---
+ | - [1, 2, '3']
+ | ...
+space:replace{4, 5, '6'}
+ | ---
+ | - [4, 5, '6']
+ | ...
+space:replace{7, 8.5, '9'}
+ | ---
+ | - [7, 8.5, '9']
+ | ...
+s = prepare("SELECT a FROM test WHERE b = '3';")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - metadata:
+ |   - name: A
+ |     type: number
+ |   rows:
+ |   - [2]
+ | ...
+execute(s.query_id)
+ | ---
+ | - metadata:
+ |   - name: A
+ |     type: number
+ |   rows:
+ |   - [2]
+ | ...
+s:execute()
+ | ---
+ | - metadata:
+ |   - name: A
+ |     type: number
+ |   rows:
+ |   - [2]
+ | ...
+s:execute()
+ | ---
+ | - metadata:
+ |   - name: A
+ |     type: number
+ |   rows:
+ |   - [2]
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+s = prepare("SELECT count(*), count(a - 3), max(b), abs(id) FROM test WHERE b = '3';")
+ | ---
+ | ...
+execute(s.query_id)
+ | ---
+ | - metadata:
+ |   - name: count(*)
+ |     type: integer
+ |   - name: count(a - 3)
+ |     type: integer
+ |   - name: max(b)
+ |     type: scalar
+ |   - name: abs(id)
+ |     type: number
+ |   rows:
+ |   - [1, 1, '3', 1]
+ | ...
+execute(s.query_id)
+ | ---
+ | - metadata:
+ |   - name: count(*)
+ |     type: integer
+ |   - name: count(a - 3)
+ |     type: integer
+ |   - name: max(b)
+ |     type: scalar
+ |   - name: abs(id)
+ |     type: number
+ |   rows:
+ |   - [1, 1, '3', 1]
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+-- Let's try something a bit more complicated. For instance recursive
+-- query displaying Mandelbrot set.
+--
+s = prepare([[WITH RECURSIVE \
+                  xaxis(x) AS (VALUES(-2.0) UNION ALL SELECT x+0.05 FROM xaxis WHERE x<1.2), \
+                  yaxis(y) AS (VALUES(-1.0) UNION ALL SELECT y+0.1 FROM yaxis WHERE y<1.0), \
+                  m(iter, cx, cy, x, y) AS ( \
+                      SELECT 0, x, y, 0.0, 0.0 FROM xaxis, yaxis \
+                      UNION ALL \
+                      SELECT iter+1, cx, cy, x*x-y*y + cx, 2.0*x*y + cy FROM m \
+                          WHERE (x*x + y*y) < 4.0 AND iter<28), \
+                      m2(iter, cx, cy) AS ( \
+                          SELECT max(iter), cx, cy FROM m GROUP BY cx, cy), \
+                      a(t) AS ( \
+                          SELECT group_concat( substr(' .+*#', 1+LEAST(iter/7,4), 1), '') \
+                              FROM m2 GROUP BY cy) \
+                  SELECT group_concat(TRIM(TRAILING FROM t),x'0a') FROM a;]])
+ | ---
+ | ...
+
+res = execute(s.query_id)
+ | ---
+ | ...
+res.metadata
+ | ---
+ | - - name: group_concat(TRIM(TRAILING FROM t),x'0a')
+ |     type: string
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+-- Workflow with bindings is still the same.
+--
+s = prepare("SELECT a FROM test WHERE b = ?;")
+ | ---
+ | ...
+execute(s.query_id, {'6'})
+ | ---
+ | - metadata:
+ |   - name: A
+ |     type: number
+ |   rows:
+ |   - [5]
+ | ...
+execute(s.query_id, {'9'})
+ | ---
+ | - metadata:
+ |   - name: A
+ |     type: number
+ |   rows:
+ |   - [8.5]
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+-- DML
+s = prepare("INSERT INTO test VALUES (?, ?, ?);")
+ | ---
+ | ...
+execute(s.query_id, {5, 6, '7'})
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id, {6, 10, '7'})
+ | ---
+ | - row_count: 1
+ | ...
+execute(s.query_id, {9, 11, '7'})
+ | ---
+ | - row_count: 1
+ | ...
+unprepare(s.query_id)
+ | ---
+ | - null
+ | ...
+
+-- EXPLAIN and PRAGMA work fine as well.
+--
+s1 = prepare("EXPLAIN SELECT a FROM test WHERE b = '3';")
+ | ---
+ | ...
+res = execute(s1.query_id)
+ | ---
+ | ...
+res.metadata
+ | ---
+ | - - name: addr
+ |     type: INTEGER
+ |   - name: opcode
+ |     type: TEXT
+ |   - name: p1
+ |     type: INTEGER
+ |   - name: p2
+ |     type: INTEGER
+ |   - name: p3
+ |     type: INTEGER
+ |   - name: p4
+ |     type: TEXT
+ |   - name: p5
+ |     type: TEXT
+ |   - name: comment
+ |     type: TEXT
+ | ...
+assert(res.rows ~= nil)
+ | ---
+ | - true
+ | ...
+
+s2 = prepare("EXPLAIN QUERY PLAN SELECT a FROM test WHERE b = '3';")
+ | ---
+ | ...
+res = execute(s2.query_id)
+ | ---
+ | ...
+res.metadata
+ | ---
+ | - - name: selectid
+ |     type: INTEGER
+ |   - name: order
+ |     type: INTEGER
+ |   - name: from
+ |     type: INTEGER
+ |   - name: detail
+ |     type: TEXT
+ | ...
+assert(res.rows ~= nil)
+ | ---
+ | - true
+ | ...
+
+s3 = prepare("PRAGMA count_changes;")
+ | ---
+ | ...
+execute(s3.query_id)
+ | ---
+ | - metadata:
+ |   - name: defer_foreign_keys
+ |     type: INTEGER
+ |   rows:
+ |   - [0]
+ | ...
+
+unprepare(s3.query_id)
+ | ---
+ | - null
+ | ...
+unprepare(s2.query_id)
+ | ---
+ | - null
+ | ...
+unprepare(s1.query_id)
+ | ---
+ | - null
+ | ...
+
+-- Make sure cache memory limit can't be exceeed. We have to
+-- create separate fiber (in local mode) since cache is local
+-- to session. After cache creation its size is fixed and can't
+-- be reconfigured. Also test that ids in each session start from 0.
+--
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+box.cfg{sql_cache_size = 3000}
+res = nil;
+ | ---
+ | ...
+_ = fiber.create(function()
+    s = prepare("SELECT * FROM test;")
+    res = s.query_id
+end);
+ | ---
+ | ...
+while res == nil do fiber.sleep(0.00001) end;
+ | ---
+ | ...
+assert(res == 0);
+ | ---
+ | - true
+ | ...
+
+ok = nil
+res = nil
+_ = fiber.create(function()
+    for i = 1, 5 do
+        pcall(prepare, "SELECT * FROM test;")
+    end
+    ok, res = pcall(prepare, "SELECT * FROM test;")
+end);
+ | ---
+ | ...
+while ok == nil do fiber.sleep(0.00001) end;
+ | ---
+ | ...
+assert(ok == false);
+ | ---
+ | - true
+ | ...
+res;
+ | ---
+ | - 'Failed to prepare SQL statement: prepared statement cache is full'
+ | ...
+
+-- Make sure cache can be purged with box.session.sql_cache_erase()
+--
+res = nil;
+ | ---
+ | ...
+ok = nil;
+ | ---
+ | ...
+_ = fiber.create(function()
+    for i = 1, 5 do
+        pcall(prepare, "SELECT * FROM test;")
+    end
+    box.session.sql_cache_erase()
+    ok, res = pcall(prepare, "SELECT * FROM test;")
+end);
+ | ---
+ | ...
+while ok == nil do fiber.sleep(0.00001) end;
+ | ---
+ | ...
+assert(ok == true);
+ | ---
+ | - true
+ | ...
+assert(res ~= nil);
+ | ---
+ | - true
+ | ...
+
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | - true
+ | ...
+
+box.space.TEST:drop()
+ | ---
+ | ...
diff --git a/test/sql/prepared.test.lua b/test/sql/prepared.test.lua
new file mode 100644
index 000000000..680f0bdb3
--- /dev/null
+++ b/test/sql/prepared.test.lua
@@ -0,0 +1,222 @@
+remote = require('net.box')
+test_run = require('test_run').new()
+fiber = require('fiber')
+
+-- Wrappers to make remote and local execution interface return
+-- same result pattern.
+--
+test_run:cmd("setopt delimiter ';'")
+execute = function(...)
+    local res, err = box.execute(...)
+    if err ~= nil then
+        error(err)
+    end
+    return res
+end;
+prepare = function(...)
+    local res, err = box.prepare(...)
+    if err ~= nil then
+        error(err)
+    end
+    return res
+end;
+unprepare = function(...)
+    local res, err = box.unprepare(...)
+    if err ~= nil then
+        error(err)
+    end
+    return res
+end;
+test_run:cmd("setopt delimiter ''");
+
+-- Test local interface and basic capabilities of prepared statements.
+--
+execute('CREATE TABLE test (id INT PRIMARY KEY, a NUMBER, b TEXT)')
+space = box.space.TEST
+space:replace{1, 2, '3'}
+space:replace{4, 5, '6'}
+space:replace{7, 8.5, '9'}
+s, e = prepare("SELECT * FROM test WHERE id = ? AND a = ?;")
+assert(e == nil)
+assert(s ~= nil)
+s.query_id
+s.metadata
+s.params
+s.params_count
+execute(s.query_id, {1, 2})
+execute(s.query_id, {1, 3})
+s:execute({1, 2})
+s:execute({1, 3})
+s:unprepare()
+
+-- Test preparation of different types of queries.
+-- Let's start from DDL. It doesn't make much sense since
+-- any prepared DDL statement can be executed once, but
+-- anyway make sure that no crashes occur.
+--
+s = prepare("CREATE INDEX i1 ON test(a)")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+s = prepare("DROP INDEX i1 ON test;")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+s = prepare("CREATE VIEW v AS SELECT * FROM test;")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+s = prepare("DROP VIEW v;")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+s = prepare("ALTER TABLE test RENAME TO test1")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+box.execute("CREATE TABLE test2 (id INT PRIMARY KEY);")
+s = prepare("ALTER TABLE test2 ADD CONSTRAINT fk1 FOREIGN KEY (id) REFERENCES test2")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+box.space.TEST2:drop()
+
+s = prepare("CREATE TRIGGER tr1 INSERT ON test1 FOR EACH ROW BEGIN DELETE FROM test1; END;")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+s = prepare("DROP TRIGGER tr1;")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+s = prepare("DROP TABLE test1;")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+-- DQL
+--
+execute('CREATE TABLE test (id INT PRIMARY KEY, a NUMBER, b TEXT)')
+space = box.space.TEST
+space:replace{1, 2, '3'}
+space:replace{4, 5, '6'}
+space:replace{7, 8.5, '9'}
+s = prepare("SELECT a FROM test WHERE b = '3';")
+execute(s.query_id)
+execute(s.query_id)
+s:execute()
+s:execute()
+unprepare(s.query_id)
+
+s = prepare("SELECT count(*), count(a - 3), max(b), abs(id) FROM test WHERE b = '3';")
+execute(s.query_id)
+execute(s.query_id)
+unprepare(s.query_id)
+
+-- Let's try something a bit more complicated. For instance recursive
+-- query displaying Mandelbrot set.
+--
+s = prepare([[WITH RECURSIVE \
+                  xaxis(x) AS (VALUES(-2.0) UNION ALL SELECT x+0.05 FROM xaxis WHERE x<1.2), \
+                  yaxis(y) AS (VALUES(-1.0) UNION ALL SELECT y+0.1 FROM yaxis WHERE y<1.0), \
+                  m(iter, cx, cy, x, y) AS ( \
+                      SELECT 0, x, y, 0.0, 0.0 FROM xaxis, yaxis \
+                      UNION ALL \
+                      SELECT iter+1, cx, cy, x*x-y*y + cx, 2.0*x*y + cy FROM m \
+                          WHERE (x*x + y*y) < 4.0 AND iter<28), \
+                      m2(iter, cx, cy) AS ( \
+                          SELECT max(iter), cx, cy FROM m GROUP BY cx, cy), \
+                      a(t) AS ( \
+                          SELECT group_concat( substr(' .+*#', 1+LEAST(iter/7,4), 1), '') \
+                              FROM m2 GROUP BY cy) \
+                  SELECT group_concat(TRIM(TRAILING FROM t),x'0a') FROM a;]])
+
+res = execute(s.query_id)
+res.metadata
+unprepare(s.query_id)
+
+-- Workflow with bindings is still the same.
+--
+s = prepare("SELECT a FROM test WHERE b = ?;")
+execute(s.query_id, {'6'})
+execute(s.query_id, {'9'})
+unprepare(s.query_id)
+
+-- DML
+s = prepare("INSERT INTO test VALUES (?, ?, ?);")
+execute(s.query_id, {5, 6, '7'})
+execute(s.query_id, {6, 10, '7'})
+execute(s.query_id, {9, 11, '7'})
+unprepare(s.query_id)
+
+-- EXPLAIN and PRAGMA work fine as well.
+--
+s1 = prepare("EXPLAIN SELECT a FROM test WHERE b = '3';")
+res = execute(s1.query_id)
+res.metadata
+assert(res.rows ~= nil)
+
+s2 = prepare("EXPLAIN QUERY PLAN SELECT a FROM test WHERE b = '3';")
+res = execute(s2.query_id)
+res.metadata
+assert(res.rows ~= nil)
+
+s3 = prepare("PRAGMA count_changes;")
+execute(s3.query_id)
+
+unprepare(s3.query_id)
+unprepare(s2.query_id)
+unprepare(s1.query_id)
+
+-- Make sure cache memory limit can't be exceeed. We have to
+-- create separate fiber (in local mode) since cache is local
+-- to session. After cache creation its size is fixed and can't
+-- be reconfigured. Also test that ids in each session start from 0.
+--
+test_run:cmd("setopt delimiter ';'")
+box.cfg{sql_cache_size = 3000}
+res = nil;
+_ = fiber.create(function()
+    s = prepare("SELECT * FROM test;")
+    res = s.query_id
+end);
+while res == nil do fiber.sleep(0.00001) end;
+assert(res == 0);
+
+ok = nil
+res = nil
+_ = fiber.create(function()
+    for i = 1, 5 do
+        pcall(prepare, "SELECT * FROM test;")
+    end
+    ok, res = pcall(prepare, "SELECT * FROM test;")
+end);
+while ok == nil do fiber.sleep(0.00001) end;
+assert(ok == false);
+res;
+
+-- Make sure cache can be purged with box.session.sql_cache_erase()
+--
+res = nil;
+ok = nil;
+_ = fiber.create(function()
+    for i = 1, 5 do
+        pcall(prepare, "SELECT * FROM test;")
+    end
+    box.session.sql_cache_erase()
+    ok, res = pcall(prepare, "SELECT * FROM test;")
+end);
+while ok == nil do fiber.sleep(0.00001) end;
+assert(ok == true);
+assert(res ~= nil);
+
+test_run:cmd("setopt delimiter ''");
+
+box.space.TEST:drop()
-- 
2.15.1

[Tarantool-patches] [PATCH 15/15] netbox: introduce prepared statements

[Nikita Pettik]

This patch introduces support of prepared statements in IProto
protocol. To achieve this new IProto command is added: IPROTO_PREPARE.
It can come whether with IPROTO_SQL_TEXT key and it means to prepare SQL
statement (for details see previous commit) or with IPROTO_QUERY_ID
which in turn results in prepared statement invalidation. Also to reply
on PREPARE request a few response keys are added: IPROTO_BIND_METADATA
(contains parameters metadata), IPROTO_BIND_COUNT (count of parameters
to be bound), IPROTO_QUERY_ID (contains id of prepared statement).

Closes #2592

@TarantoolBot document
Title: Prepared statements in SQL

Now it is possible to 'prepare' (i.e. compile into byte-code and save to
the cache) statement and execute it several times. Mechanism is similar
to ones in other DBs. Prepared statement is identified by numeric
sequential ids, which are returned alongside with prepared statement
handle. Prepared statement cache is local to session. Its size is
adjusted by box.cfg{sql_cache_size} variable (can be set dynamically;
note that size of already created caches is not changed). Any DDL
operation leads to expiration of all prepared statements: they should
be manually removed. It can be done with box.session.sql_cache_erase().
Prepared statements are available in local mode (i.e. via box.prepare()
function) and are supported in IProto protocol. Typical workflow with
prepared statements is following:

s = box.prepare("SELECT * FROM t WHERE id = ?;")
s:execute({1}) or box.execute(s.query_id, {1})
s:execute({2}) or box.execute(s.query_id, {2})
s:unprepare() or box.unprepare(s.query_id)

In terms of remote connection:

cn = netbox:connect(addr)
s = cn:prepare("SELECT * FROM t WHERE id = ?;")
cn:execute(s.query_id, {1})
cn:unprepare(s.query_id)
---
 src/box/execute.c          |  86 +++++++++++++++++++++++++++
 src/box/iproto.cc          |  68 +++++++++++++++++----
 src/box/iproto_constants.c |   7 ++-
 src/box/iproto_constants.h |   5 ++
 src/box/lua/net_box.c      | 104 +++++++++++++++++++++++++++++++--
 src/box/lua/net_box.lua    |  28 +++++++++
 src/box/xrow.c             |  19 ++++--
 src/box/xrow.h             |   4 +-
 test/box/misc.result       |   1 +
 test/sql/engine.cfg        |   1 +
 test/sql/iproto.result     |   2 +-
 test/sql/prepared.result   | 143 +++++++++++++++++++++++++++------------------
 test/sql/prepared.test.lua |  94 +++++++++++++++++++++--------
 13 files changed, 458 insertions(+), 104 deletions(-)

diff --git a/src/box/execute.c b/src/box/execute.c
index 3fd1afd8a..51b309ac1 100644
--- a/src/box/execute.c
+++ b/src/box/execute.c
@@ -326,6 +326,65 @@ sql_get_metadata(struct sql_stmt *stmt, struct obuf *out, int column_count)
 	return 0;
 }
 
+static inline int
+sql_get_params_metadata(struct sql_stmt *stmt, struct obuf *out)
+{
+	int bind_count = sql_bind_parameter_count(stmt);
+	int size = mp_sizeof_uint(IPROTO_BIND_METADATA) +
+		   mp_sizeof_array(bind_count);
+	char *pos = (char *) obuf_alloc(out, size);
+	if (pos == NULL) {
+		diag_set(OutOfMemory, size, "obuf_alloc", "pos");
+		return -1;
+	}
+	pos = mp_encode_uint(pos, IPROTO_BIND_METADATA);
+	pos = mp_encode_array(pos, bind_count);
+	for (int i = 0; i < bind_count; ++i) {
+		size_t size = mp_sizeof_map(2) +
+			      mp_sizeof_uint(IPROTO_FIELD_NAME) +
+			      mp_sizeof_uint(IPROTO_FIELD_TYPE);
+		const char *name = sql_bind_parameter_name(stmt, i);
+		if (name == NULL)
+			name = "?";
+		const char *type = "ANY";
+		size += mp_sizeof_str(strlen(name));
+		size += mp_sizeof_str(strlen(type));
+		char *pos = (char *) obuf_alloc(out, size);
+		if (pos == NULL) {
+			diag_set(OutOfMemory, size, "obuf_alloc", "pos");
+			return -1;
+		}
+		pos = mp_encode_map(pos, 2);
+		pos = mp_encode_uint(pos, IPROTO_FIELD_NAME);
+		pos = mp_encode_str(pos, name, strlen(name));
+		pos = mp_encode_uint(pos, IPROTO_FIELD_TYPE);
+		pos = mp_encode_str(pos, type, strlen(type));
+	}
+	return 0;
+}
+
+static int
+sql_get_prepare_common_keys(struct sql_stmt *stmt, struct obuf *out, int keys,
+			    uint32_t query_id)
+{
+	int size = mp_sizeof_map(keys) +
+		mp_sizeof_uint(IPROTO_QUERY_ID) + mp_sizeof_uint(query_id) +
+		mp_sizeof_uint(IPROTO_BIND_COUNT) + mp_sizeof_uint(sql_bind_parameter_count(stmt));
+	char *pos = (char *) obuf_alloc(out, size);
+	if (pos == NULL) {
+		diag_set(OutOfMemory, size, "obuf_alloc", "pos");
+		return -1;
+	}
+	pos = mp_encode_map(pos, keys);
+	pos = mp_encode_uint(pos, IPROTO_QUERY_ID);
+	pos = mp_encode_uint(pos, query_id);
+	pos = mp_encode_uint(pos, IPROTO_BIND_COUNT);
+	pos = mp_encode_uint(pos, sql_bind_parameter_count(stmt));
+	if (sql_get_params_metadata(stmt, out) != 0)
+		return -1;
+	return 0;
+}
+
 static int
 port_sql_dump_msgpack(struct port *port, struct obuf *out)
 {
@@ -407,6 +466,33 @@ port_sql_dump_msgpack(struct port *port, struct obuf *out)
 		}
 		break;
 	}
+	case DQL_PREPARE: {
+		/* Format is following:
+		 * query_id,
+		 * param_count,
+		 * params {name, type},
+		 * metadata {name, type}
+		 */
+		int keys = 4;
+		if (sql_get_prepare_common_keys(stmt, out, keys,
+						sql_port->query_id) != 0)
+			return -1;
+		if (sql_get_metadata(stmt, out, sql_column_count(stmt)) != 0)
+			return -1;
+		break;
+	}
+	case DML_PREPARE: {
+		/* Format is following:
+		 * query_id,
+		 * param_count,
+		 * params {name, type},
+		 */
+		int keys = 3;
+		if (sql_get_prepare_common_keys(stmt, out, keys,
+						sql_port->query_id) != 0)
+			return -1;
+		break;
+		}
 	default: unreachable();
 	}
 	return 0;
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index 8f899fed8..266d1c28b 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -178,7 +178,7 @@ struct iproto_msg
 		struct call_request call;
 		/** Authentication request. */
 		struct auth_request auth;
-		/* SQL request, if this is the EXECUTE request. */
+		/* SQL request, if this is the EXECUTE/PREPARE request. */
 		struct sql_request sql;
 		/** In case of iproto parse error, saved diagnostics. */
 		struct diag diag;
@@ -1155,6 +1155,7 @@ static const struct cmsg_hop *dml_route[IPROTO_TYPE_STAT_MAX] = {
 	call_route,                             /* IPROTO_CALL */
 	sql_route,                              /* IPROTO_EXECUTE */
 	NULL,                                   /* IPROTO_NOP */
+	sql_route,                              /* IPROTO_PREPARE */
 };
 
 static const struct cmsg_hop join_route[] = {
@@ -1210,6 +1211,7 @@ iproto_msg_decode(struct iproto_msg *msg, const char **pos, const char *reqend,
 		cmsg_init(&msg->base, call_route);
 		break;
 	case IPROTO_EXECUTE:
+	case IPROTO_PREPARE:
 		if (xrow_decode_sql(&msg->header, &msg->sql) != 0)
 			goto error;
 		cmsg_init(&msg->base, sql_route);
@@ -1647,23 +1649,64 @@ tx_process_sql(struct cmsg *m)
 	int bind_count = 0;
 	const char *sql;
 	uint32_t len;
+	bool is_unprepare = false;
 
 	tx_fiber_init(msg->connection->session, msg->header.sync);
 
 	if (tx_check_schema(msg->header.schema_version))
 		goto error;
-	assert(msg->header.type == IPROTO_EXECUTE);
+	assert(msg->header.type == IPROTO_EXECUTE ||
+	       msg->header.type == IPROTO_PREPARE);
 	tx_inject_delay();
 	if (msg->sql.bind != NULL) {
 		bind_count = sql_bind_list_decode(msg->sql.bind, &bind);
 		if (bind_count < 0)
 			goto error;
 	}
-	sql = msg->sql.sql_text;
-	sql = mp_decode_str(&sql, &len);
-	if (sql_prepare_and_execute(sql, len, bind, bind_count, &port,
-				    &fiber()->gc) != 0)
-		goto error;
+	/*
+	 * There are four options:
+	 * 1. Prepare SQL query (IPROTO_PREPARE + SQL string);
+	 * 2. Unprepare SQL query (IPROTO_PREPARE + query id);
+	 * 3. Execute SQL query (IPROTO_EXECUTE + SQL string);
+	 * 4. Execute prepared query (IPROTO_EXECUTE + query id).
+	 */
+	if (msg->header.type == IPROTO_EXECUTE) {
+		if (msg->sql.sql_text != NULL) {
+			assert(msg->sql.query_id == NULL);
+			sql = msg->sql.sql_text;
+			sql = mp_decode_str(&sql, &len);
+			if (sql_prepare_and_execute(sql, len, bind, bind_count,
+						    &port, &fiber()->gc) != 0)
+				goto error;
+		} else {
+			assert(msg->sql.sql_text == NULL);
+			assert(msg->sql.query_id != NULL);
+			sql = msg->sql.query_id;
+			uint32_t query_id = mp_decode_uint(&sql);
+			if (sql_execute_prepared(query_id, bind, bind_count,
+						 &port, &fiber()->gc) != 0)
+				goto error;
+		}
+	} else {
+		/* IPROTO_PREPARE */
+		if (msg->sql.sql_text != NULL) {
+			assert(msg->sql.query_id == NULL);
+			sql = msg->sql.sql_text;
+			sql = mp_decode_str(&sql, &len);
+			if (sql_prepare(sql, len, &port) != 0)
+				goto error;
+		} else {
+			/* UNPREPARE */
+			assert(msg->sql.sql_text == NULL);
+			assert(msg->sql.query_id != NULL);
+			sql = msg->sql.query_id;
+			uint32_t query_id = mp_decode_uint(&sql);
+			if (sql_unprepare(query_id) != 0)
+				goto error;
+			is_unprepare = true;
+		}
+	}
+
 	/*
 	 * Take an obuf only after execute(). Else the buffer can
 	 * become out of date during yield.
@@ -1675,12 +1718,15 @@ tx_process_sql(struct cmsg *m)
 		port_destroy(&port);
 		goto error;
 	}
-	if (port_dump_msgpack(&port, out) != 0) {
+	/* Nothing to dump in case of UNPREPARE request. */
+	if (! is_unprepare) {
+		if (port_dump_msgpack(&port, out) != 0) {
+			port_destroy(&port);
+			obuf_rollback_to_svp(out, &header_svp);
+			goto error;
+		}
 		port_destroy(&port);
-		obuf_rollback_to_svp(out, &header_svp);
-		goto error;
 	}
-	port_destroy(&port);
 	iproto_reply_sql(out, &header_svp, msg->header.sync, schema_version);
 	iproto_wpos_create(&msg->wpos, out);
 	return;
diff --git a/src/box/iproto_constants.c b/src/box/iproto_constants.c
index 09ded1ecb..2be8e5768 100644
--- a/src/box/iproto_constants.c
+++ b/src/box/iproto_constants.c
@@ -107,6 +107,7 @@ const char *iproto_type_strs[] =
 	"CALL",
 	"EXECUTE",
 	NULL, /* NOP */
+	"PREPARE",
 };
 
 #define bit(c) (1ULL<<IPROTO_##c)
@@ -124,6 +125,7 @@ const uint64_t iproto_body_key_map[IPROTO_TYPE_STAT_MAX] = {
 	0,                                                     /* CALL */
 	0,                                                     /* EXECUTE */
 	0,                                                     /* NOP */
+	0,                                                     /* PREPARE */
 };
 #undef bit
 
@@ -179,8 +181,8 @@ const char *iproto_key_strs[IPROTO_KEY_MAX] = {
 	"data",             /* 0x30 */
 	"error",            /* 0x31 */
 	"metadata",         /* 0x32 */
-	NULL,               /* 0x33 */
-	NULL,               /* 0x34 */
+	"bind meta",        /* 0x33 */
+	"bind count",       /* 0x34 */
 	NULL,               /* 0x35 */
 	NULL,               /* 0x36 */
 	NULL,               /* 0x37 */
@@ -195,6 +197,7 @@ const char *iproto_key_strs[IPROTO_KEY_MAX] = {
 	"SQL text",         /* 0x40 */
 	"SQL bind",         /* 0x41 */
 	"SQL info",         /* 0x42 */
+	"query id",         /* 0x43 */
 };
 
 const char *vy_page_info_key_strs[VY_PAGE_INFO_KEY_MAX] = {
diff --git a/src/box/iproto_constants.h b/src/box/iproto_constants.h
index 5e8a7d483..eecac5875 100644
--- a/src/box/iproto_constants.h
+++ b/src/box/iproto_constants.h
@@ -110,6 +110,8 @@ enum iproto_key {
 	 * ]
 	 */
 	IPROTO_METADATA = 0x32,
+	IPROTO_BIND_METADATA = 0x33,
+	IPROTO_BIND_COUNT = 0x34,
 
 	/* Leave a gap between response keys and SQL keys. */
 	IPROTO_SQL_TEXT = 0x40,
@@ -120,6 +122,7 @@ enum iproto_key {
 	 * }
 	 */
 	IPROTO_SQL_INFO = 0x42,
+	IPROTO_QUERY_ID = 0x43,
 	IPROTO_KEY_MAX
 };
 
@@ -203,6 +206,8 @@ enum iproto_type {
 	IPROTO_EXECUTE = 11,
 	/** No operation. Treated as DML, used to bump LSN. */
 	IPROTO_NOP = 12,
+	/** Prepare/un-prepare SQL statement. */
+	IPROTO_PREPARE = 13,
 	/** The maximum typecode used for box.stat() */
 	IPROTO_TYPE_STAT_MAX,
 
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 001af95dc..efb139cdc 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -570,10 +570,48 @@ netbox_encode_execute(lua_State *L)
 
 	mpstream_encode_map(&stream, 3);
 
-	size_t len;
-	const char *query = lua_tolstring(L, 3, &len);
-	mpstream_encode_uint(&stream, IPROTO_SQL_TEXT);
-	mpstream_encode_strn(&stream, query, len);
+	if (lua_type(L, 3) == LUA_TNUMBER) {
+		uint32_t query_id = lua_tointeger(L, 3);
+		mpstream_encode_uint(&stream, IPROTO_QUERY_ID);
+		mpstream_encode_uint(&stream, query_id);
+	} else {
+		size_t len;
+		const char *query = lua_tolstring(L, 3, &len);
+		mpstream_encode_uint(&stream, IPROTO_SQL_TEXT);
+		mpstream_encode_strn(&stream, query, len);
+	}
+
+	mpstream_encode_uint(&stream, IPROTO_SQL_BIND);
+	luamp_encode_tuple(L, cfg, &stream, 4);
+
+	mpstream_encode_uint(&stream, IPROTO_OPTIONS);
+	luamp_encode_tuple(L, cfg, &stream, 5);
+
+	netbox_encode_request(&stream, svp);
+	return 0;
+}
+
+static int
+netbox_encode_prepare(lua_State *L)
+{
+	if (lua_gettop(L) < 5)
+		return luaL_error(L, "Usage: netbox.encode_preapare(ibuf, "\
+				  "sync, query, parameters, options)");
+	struct mpstream stream;
+	size_t svp = netbox_prepare_request(L, &stream, IPROTO_PREPARE);
+
+	mpstream_encode_map(&stream, 3);
+
+	if (lua_type(L, 3) == LUA_TNUMBER) {
+		uint32_t query_id = lua_tointeger(L, 3);
+		mpstream_encode_uint(&stream, IPROTO_QUERY_ID);
+		mpstream_encode_uint(&stream, query_id);
+	} else {
+		size_t len;
+		const char *query = lua_tolstring(L, 3, &len);
+		mpstream_encode_uint(&stream, IPROTO_SQL_TEXT);
+		mpstream_encode_strn(&stream, query, len);
+	}
 
 	mpstream_encode_uint(&stream, IPROTO_SQL_BIND);
 	luamp_encode_tuple(L, cfg, &stream, 4);
@@ -752,6 +790,62 @@ netbox_decode_execute(struct lua_State *L)
 	return 2;
 }
 
+static int
+netbox_decode_prepare(struct lua_State *L)
+{
+	uint32_t ctypeid;
+	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
+	assert(mp_typeof(*data) == MP_MAP);
+	uint32_t map_size = mp_decode_map(&data);
+	int query_id_idx = 0, meta_idx = 0, bind_meta_idx = 0,
+	    bind_count_idx = 0;
+	uint32_t query_id = 0;
+	for (uint32_t i = 0; i < map_size; ++i) {
+		uint32_t key = mp_decode_uint(&data);
+		switch(key) {
+		case IPROTO_QUERY_ID: {
+			query_id = mp_decode_uint(&data);
+			luaL_pushuint64(L, query_id);
+			query_id_idx = i - map_size;
+			break;
+		}
+		case IPROTO_METADATA: {
+			netbox_decode_metadata(L, &data);
+			meta_idx = i - map_size;
+			break;
+		}
+		case IPROTO_BIND_METADATA: {
+			netbox_decode_metadata(L, &data);
+			bind_meta_idx = i - map_size;
+			break;
+		}
+		default: {
+			assert(key == IPROTO_BIND_COUNT);
+			uint32_t bind_count = mp_decode_uint(&data);
+			luaL_pushuint64(L, bind_count);
+			bind_count_idx = i - map_size;
+			break;
+		}}
+	}
+	/* These fields must be present in response. */
+	assert(query_id_idx * bind_meta_idx * bind_count_idx != 0);
+	/* General meta is presented only in DQL responses. */
+	lua_createtable(L, 0, meta_idx != 0 ? 4 : 3);
+	lua_pushvalue(L, query_id_idx - 1);
+	lua_setfield(L, -2, "query_id");
+	lua_pushvalue(L, bind_count_idx - 1);
+	lua_setfield(L, -2, "bind_count");
+	lua_pushvalue(L, bind_meta_idx - 1);
+	lua_setfield(L, -2, "params");
+	if (meta_idx != 0) {
+		lua_pushvalue(L, meta_idx - 1);
+		lua_setfield(L, -2, "metadata");
+	}
+
+	*(const char **)luaL_pushcdata(L, ctypeid) = data;
+	return 2;
+}
+
 int
 luaopen_net_box(struct lua_State *L)
 {
@@ -767,11 +861,13 @@ luaopen_net_box(struct lua_State *L)
 		{ "encode_update",  netbox_encode_update },
 		{ "encode_upsert",  netbox_encode_upsert },
 		{ "encode_execute", netbox_encode_execute},
+		{ "encode_prepare", netbox_encode_prepare},
 		{ "encode_auth",    netbox_encode_auth },
 		{ "decode_greeting",netbox_decode_greeting },
 		{ "communicate",    netbox_communicate },
 		{ "decode_select",  netbox_decode_select },
 		{ "decode_execute", netbox_decode_execute },
+		{ "decode_prepare", netbox_decode_prepare },
 		{ NULL, NULL}
 	};
 	/* luaL_register_module polutes _G */
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 31a8c16b7..cd7f87a6a 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -104,6 +104,8 @@ local method_encoder = {
     upsert  = internal.encode_upsert,
     select  = internal.encode_select,
     execute = internal.encode_execute,
+    prepare = internal.encode_prepare,
+    unprepare = internal.encode_prepare,
     get     = internal.encode_select,
     min     = internal.encode_select,
     max     = internal.encode_select,
@@ -128,6 +130,8 @@ local method_decoder = {
     upsert  = decode_nil,
     select  = internal.decode_select,
     execute = internal.decode_execute,
+    prepare = internal.decode_prepare,
+    unprepare = decode_nil,
     get     = decode_get,
     min     = decode_get,
     max     = decode_get,
@@ -1192,6 +1196,30 @@ function remote_methods:execute(query, parameters, sql_opts, netbox_opts)
                          sql_opts or {})
 end
 
+function remote_methods:prepare(query, parameters, sql_opts, netbox_opts)
+    check_remote_arg(self, "prepare")
+    if type(query) ~= "string" then
+        box.error(box.error.SQL_PREPARE, "expected string as SQL statement")
+    end
+    if sql_opts ~= nil then
+        box.error(box.error.UNSUPPORTED, "prepare", "options")
+    end
+    return self:_request('prepare', netbox_opts, nil, query, parameters or {},
+                         sql_opts or {})
+end
+
+function remote_methods:unprepare(query, parameters, sql_opts, netbox_opts)
+    check_remote_arg(self, "unprepare")
+    if type(query) ~= "number" then
+        box.error("query id is expected to be numeric")
+    end
+    if sql_opts ~= nil then
+        box.error(box.error.UNSUPPORTED, "unprepare", "options")
+    end
+    return self:_request('unprepare', netbox_opts, nil, query, parameters or {},
+                         sql_opts or {})
+end
+
 function remote_methods:wait_state(state, timeout)
     check_remote_arg(self, 'wait_state')
     if timeout == nil then
diff --git a/src/box/xrow.c b/src/box/xrow.c
index 18bf08971..95f8213de 100644
--- a/src/box/xrow.c
+++ b/src/box/xrow.c
@@ -576,9 +576,11 @@ error:
 	uint32_t map_size = mp_decode_map(&data);
 	request->sql_text = NULL;
 	request->bind = NULL;
+	request->query_id = NULL;
 	for (uint32_t i = 0; i < map_size; ++i) {
 		uint8_t key = *data;
-		if (key != IPROTO_SQL_BIND && key != IPROTO_SQL_TEXT) {
+		if (key != IPROTO_SQL_BIND && key != IPROTO_SQL_TEXT &&
+		    key != IPROTO_QUERY_ID) {
 			mp_check(&data, end);   /* skip the key */
 			mp_check(&data, end);   /* skip the value */
 			continue;
@@ -588,12 +590,21 @@ error:
 			goto error;
 		if (key == IPROTO_SQL_BIND)
 			request->bind = value;
-		else
+		else if (key == IPROTO_SQL_TEXT)
 			request->sql_text = value;
+		else
+			request->query_id = value;
+	}
+	if (request->sql_text != NULL && request->query_id != NULL) {
+		xrow_on_decode_err(row->body[0].iov_base, end, ER_INVALID_MSGPACK,
+				   "SQL text and query id are incompatible "\
+				   "options in one request: choose one");
+		return -1;
 	}
-	if (request->sql_text == NULL) {
+	if (request->sql_text == NULL && request->query_id == NULL) {
 		xrow_on_decode_err(row->body[0].iov_base, end, ER_MISSING_REQUEST_FIELD,
-			 iproto_key_name(IPROTO_SQL_TEXT));
+			 tt_sprintf("%s or %s", iproto_key_name(IPROTO_SQL_TEXT),
+				    iproto_key_name(IPROTO_QUERY_ID)));
 		return -1;
 	}
 	if (data != end)
diff --git a/src/box/xrow.h b/src/box/xrow.h
index 60def2d3c..aca35aa6a 100644
--- a/src/box/xrow.h
+++ b/src/box/xrow.h
@@ -526,10 +526,12 @@ int
 iproto_reply_error(struct obuf *out, const struct error *e, uint64_t sync,
 		   uint32_t schema_version);
 
-/** EXECUTE request. */
+/** EXECUTE/PREPARE request. */
 struct sql_request {
 	/** SQL statement text. */
 	const char *sql_text;
+	/** Id of prepared statement. In this case @sql_text == NULL. */
+	const char *query_id;
 	/** MessagePack array of parameters. */
 	const char *bind;
 };
diff --git a/test/box/misc.result b/test/box/misc.result
index b75f615d5..d48b7cfcc 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -231,6 +231,7 @@ t;
   - EVAL
   - CALL
   - ERROR
+  - PREPARE
   - REPLACE
   - UPSERT
   - AUTH
diff --git a/test/sql/engine.cfg b/test/sql/engine.cfg
index a1b4b0fc5..e38bec24e 100644
--- a/test/sql/engine.cfg
+++ b/test/sql/engine.cfg
@@ -10,6 +10,7 @@
         "local": {"remote": "false"}
     },
     "prepared.test.lua": {
+        "remote": {"remote": "true"},
         "local": {"remote": "false"}
     },
     "*": {
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 1e5c30aec..5a94a07d8 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -119,7 +119,7 @@ cn:execute('select id as identifier from test where a = 5;')
 -- netbox API errors.
 cn:execute(100)
 ---
-- error: Syntax error near '100'
+- error: Prepared statement with corresponding id 100 does not exist
 ...
 cn:execute('select 1', nil, {dry_run = true})
 ---
diff --git a/test/sql/prepared.result b/test/sql/prepared.result
index 1646af94f..0de6882c6 100644
--- a/test/sql/prepared.result
+++ b/test/sql/prepared.result
@@ -12,34 +12,52 @@ fiber = require('fiber')
 -- Wrappers to make remote and local execution interface return
 -- same result pattern.
 --
-test_run:cmd("setopt delimiter ';'")
+is_remote = test_run:get_cfg('remote') == 'true'
  | ---
- | - true
  | ...
-execute = function(...)
-    local res, err = box.execute(...)
-    if err ~= nil then
-        error(err)
-    end
-    return res
-end;
+execute = nil
  | ---
  | ...
-prepare = function(...)
-    local res, err = box.prepare(...)
-    if err ~= nil then
-        error(err)
-    end
-    return res
-end;
+prepare = nil
+ | ---
+ | ...
+unprepare = nil
  | ---
  | ...
-unprepare = function(...)
-    local res, err = box.unprepare(...)
-    if err ~= nil then
-        error(err)
+
+test_run:cmd("setopt delimiter ';'")
+ | ---
+ | - true
+ | ...
+if is_remote then
+    box.schema.user.grant('guest','read, write, execute', 'universe')
+    box.schema.user.grant('guest', 'create', 'space')
+    cn = remote.connect(box.cfg.listen)
+    execute = function(...) return cn:execute(...) end
+    prepare = function(...) return cn:prepare(...) end
+    unprepare = function(...) return cn:unprepare(...) end
+else
+    execute = function(...)
+        local res, err = box.execute(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
+    end
+    prepare = function(...)
+        local res, err = box.prepare(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
+    end
+    unprepare = function(...)
+        local res, err = box.unprepare(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
     end
-    return res
 end;
  | ---
  | ...
@@ -127,31 +145,25 @@ execute(s.query_id, {1, 3})
  |     type: string
  |   rows: []
  | ...
-s:execute({1, 2})
+
+test_run:cmd("setopt delimiter ';'")
  | ---
- | - metadata:
- |   - name: ID
- |     type: integer
- |   - name: A
- |     type: number
- |   - name: B
- |     type: string
- |   rows:
- |   - [1, 2, '3']
+ | - true
  | ...
-s:execute({1, 3})
+if not is_remote then
+    res = s:execute({1, 2})
+    assert(res ~= nil)
+    res = s:execute({1, 3})
+    assert(res ~= nil)
+    s:unprepare()
+else
+    unprepare(s.query_id)
+end;
  | ---
- | - metadata:
- |   - name: ID
- |     type: integer
- |   - name: A
- |     type: number
- |   - name: B
- |     type: string
- |   rows: []
  | ...
-s:unprepare()
+test_run:cmd("setopt delimiter ''");
  | ---
+ | - true
  | ...
 
 -- Test preparation of different types of queries.
@@ -350,22 +362,23 @@ execute(s.query_id)
  |   rows:
  |   - [2]
  | ...
-s:execute()
+test_run:cmd("setopt delimiter ';'")
  | ---
- | - metadata:
- |   - name: A
- |     type: number
- |   rows:
- |   - [2]
+ | - true
  | ...
-s:execute()
+if not is_remote then
+    res = s:execute()
+    assert(res ~= nil)
+    res = s:execute()
+    assert(res ~= nil)
+end;
  | ---
- | - metadata:
- |   - name: A
- |     type: number
- |   rows:
- |   - [2]
  | ...
+test_run:cmd("setopt delimiter ''");
+ | ---
+ | - true
+ | ...
+
 unprepare(s.query_id)
  | ---
  | - null
@@ -576,6 +589,12 @@ test_run:cmd("setopt delimiter ';'")
  | - true
  | ...
 box.cfg{sql_cache_size = 3000}
+if is_remote then
+    cn:close()
+    cn = remote.connect(box.cfg.listen)
+end;
+ | ---
+ | ...
 res = nil;
  | ---
  | ...
@@ -624,10 +643,14 @@ ok = nil;
  | ---
  | ...
 _ = fiber.create(function()
-    for i = 1, 5 do
-        pcall(prepare, "SELECT * FROM test;")
+    if is_remote then
+        cn:eval("box.session.sql_cache_erase()")
+    else
+        for i = 1, 5 do
+            pcall(prepare, "SELECT * FROM test;")
+        end
+        box.session.sql_cache_erase()
     end
-    box.session.sql_cache_erase()
     ok, res = pcall(prepare, "SELECT * FROM test;")
 end);
  | ---
@@ -644,6 +667,14 @@ assert(res ~= nil);
  | - true
  | ...
 
+if is_remote then
+    cn:close()
+    box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+    box.schema.user.revoke('guest', 'create', 'space')
+end;
+ | ---
+ | ...
+
 test_run:cmd("setopt delimiter ''");
  | ---
  | - true
diff --git a/test/sql/prepared.test.lua b/test/sql/prepared.test.lua
index 680f0bdb3..a89063cdf 100644
--- a/test/sql/prepared.test.lua
+++ b/test/sql/prepared.test.lua
@@ -5,27 +5,41 @@ fiber = require('fiber')
 -- Wrappers to make remote and local execution interface return
 -- same result pattern.
 --
+is_remote = test_run:get_cfg('remote') == 'true'
+execute = nil
+prepare = nil
+unprepare = nil
+
 test_run:cmd("setopt delimiter ';'")
-execute = function(...)
-    local res, err = box.execute(...)
-    if err ~= nil then
-        error(err)
+if is_remote then
+    box.schema.user.grant('guest','read, write, execute', 'universe')
+    box.schema.user.grant('guest', 'create', 'space')
+    cn = remote.connect(box.cfg.listen)
+    execute = function(...) return cn:execute(...) end
+    prepare = function(...) return cn:prepare(...) end
+    unprepare = function(...) return cn:unprepare(...) end
+else
+    execute = function(...)
+        local res, err = box.execute(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
     end
-    return res
-end;
-prepare = function(...)
-    local res, err = box.prepare(...)
-    if err ~= nil then
-        error(err)
+    prepare = function(...)
+        local res, err = box.prepare(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
     end
-    return res
-end;
-unprepare = function(...)
-    local res, err = box.unprepare(...)
-    if err ~= nil then
-        error(err)
+    unprepare = function(...)
+        local res, err = box.unprepare(...)
+        if err ~= nil then
+            error(err)
+        end
+        return res
     end
-    return res
 end;
 test_run:cmd("setopt delimiter ''");
 
@@ -45,9 +59,18 @@ s.params
 s.params_count
 execute(s.query_id, {1, 2})
 execute(s.query_id, {1, 3})
-s:execute({1, 2})
-s:execute({1, 3})
-s:unprepare()
+
+test_run:cmd("setopt delimiter ';'")
+if not is_remote then
+    res = s:execute({1, 2})
+    assert(res ~= nil)
+    res = s:execute({1, 3})
+    assert(res ~= nil)
+    s:unprepare()
+else
+    unprepare(s.query_id)
+end;
+test_run:cmd("setopt delimiter ''");
 
 -- Test preparation of different types of queries.
 -- Let's start from DDL. It doesn't make much sense since
@@ -111,8 +134,15 @@ space:replace{7, 8.5, '9'}
 s = prepare("SELECT a FROM test WHERE b = '3';")
 execute(s.query_id)
 execute(s.query_id)
-s:execute()
-s:execute()
+test_run:cmd("setopt delimiter ';'")
+if not is_remote then
+    res = s:execute()
+    assert(res ~= nil)
+    res = s:execute()
+    assert(res ~= nil)
+end;
+test_run:cmd("setopt delimiter ''");
+
 unprepare(s.query_id)
 
 s = prepare("SELECT count(*), count(a - 3), max(b), abs(id) FROM test WHERE b = '3';")
@@ -182,6 +212,10 @@ unprepare(s1.query_id)
 --
 test_run:cmd("setopt delimiter ';'")
 box.cfg{sql_cache_size = 3000}
+if is_remote then
+    cn:close()
+    cn = remote.connect(box.cfg.listen)
+end;
 res = nil;
 _ = fiber.create(function()
     s = prepare("SELECT * FROM test;")
@@ -207,16 +241,26 @@ res;
 res = nil;
 ok = nil;
 _ = fiber.create(function()
-    for i = 1, 5 do
-        pcall(prepare, "SELECT * FROM test;")
+    if is_remote then
+        cn:eval("box.session.sql_cache_erase()")
+    else
+        for i = 1, 5 do
+            pcall(prepare, "SELECT * FROM test;")
+        end
+        box.session.sql_cache_erase()
     end
-    box.session.sql_cache_erase()
     ok, res = pcall(prepare, "SELECT * FROM test;")
 end);
 while ok == nil do fiber.sleep(0.00001) end;
 assert(ok == true);
 assert(res ~= nil);
 
+if is_remote then
+    cn:close()
+    box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+    box.schema.user.revoke('guest', 'create', 'space')
+end;
+
 test_run:cmd("setopt delimiter ''");
 
 box.space.TEST:drop()
-- 
2.15.1

Re: [Tarantool-patches] [PATCH 13/15] sql: introduce cache for prepared statemets

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [19/11/07 04:33]:
> This patch introduces cache (as data structure) to handle prepared
> statements and a set of interface functions (insert, delete, find,
> erase) to operate on it. Cache under the hood is hash table with integer
> ids as keys and prepared statements (struct sql_stmt which is an alias
> for struct Vdbe) as values. Size of cache is regulated by quota via
> box.cfg{sql_cache_size} parameter. Cache is supposed to be attached to
> session, which means it is destroyed when session is ended. To erase
> session manually, there's special handle - session's method
> sql_cache_erase(). Default cache size is assumed to be 5 Mb (like in
> PosgreSQL).

I admire the depth of architecture vision here - let's just do what postgresql does!!

If tarantool connections are going to be as slow as postgresql
connections (I can imagine iterating over vdbe objects in a
single-threaded environment to free them up on disconnect,
especially in a thundering herd way when a bunch of connections is
established or dropped), why use tarantool at all?

The whole idea of using string identifiers was that the cache is
global and there is zero overhead on connect or disconnect.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 14/15] box: introduce prepared statements

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [19/11/07 04:33]:
> +int
> +sql_unprepare(uint32_t query_id)

this is called deallocate prepare in ansi sql, not unprepare
> +	/**
> +	 * In case of "prepare" request user receives id of query
> +	 * using which query can be executed later.
> +	 */
> +	uint32_t query_id;

Query is something that returns data. You can prepare any
statement. So it's statement id, not query id.


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 13/15] sql: introduce cache for prepared statemets

[Nikita Pettik]

On 11 Nov 02:40, Konstantin Osipov wrote:
> * Nikita Pettik <korablev@tarantool.org> [19/11/07 04:33]:
> > This patch introduces cache (as data structure) to handle prepared
> > statements and a set of interface functions (insert, delete, find,
> > erase) to operate on it. Cache under the hood is hash table with integer
> > ids as keys and prepared statements (struct sql_stmt which is an alias
> > for struct Vdbe) as values. Size of cache is regulated by quota via
> > box.cfg{sql_cache_size} parameter. Cache is supposed to be attached to
> > session, which means it is destroyed when session is ended. To erase
> > session manually, there's special handle - session's method
> > sql_cache_erase(). Default cache size is assumed to be 5 Mb (like in
> > PosgreSQL).
> 
> I admire the depth of architecture vision here - let's just do what postgresql does!!

It's just default value, it can be changed manually to whatever
value user wants. Feel free to suggest better option.
 
> If tarantool connections are going to be as slow as postgresql
> connections (I can imagine iterating over vdbe objects in a
> single-threaded environment to free them up on disconnect,
> especially in a thundering herd way when a bunch of connections is
> established or dropped), why use tarantool at all?
> 
> The whole idea of using string identifiers was that the cache is
> global and there is zero overhead on connect or disconnect.

How kind of identifiers is related to cache locality? I don't understand
how global cache could help us to solve problem of cleaning up VM objects
on disconect (on connect there's no overhead anyway).

Today Kirill came to me and said that there's request from solution
team to make cache global: according to them they use many connections
to execute the same set of queries. That turns out global cache to be
reasonable. However, to allow execute the same prepared statement via
different sessions we should keep trace of original query - its hash
value. So the new proposal is:

- Each session holds map <stmt_id : query_hash>
- There's one global hash <query_hash : stmt>
- Each statement now has reference counter: each "prepare" call via
  different session bumps its value; each "unprepare" call results in
  its decrement. Disconect of session leads to counter decrements of all
  related statements. Statement is not immediately deallocated if its
  counter is 0: we maintaint global list of statements to be freed when
  memory limit is reached
- On "prepare" call we firstly check if there's enough space for statement.
  If memory limit has reached, we traverse list of statements to be freed
  and release all occupied resources. It would allow us to solve possible
  overhead on disconnect event.

I'll update RFC according to this proposal if everyone is OK with it.

Re: [Tarantool-patches] [PATCH 13/15] sql: introduce cache for prepared statemets

[Konstantin Osipov]

* Nikita Pettik <korablev@tarantool.org> [19/11/11 21:27]:
> Today Kirill came to me and said that there's request from solution
> team to make cache global: according to them they use many connections
> to execute the same set of queries. That turns out global cache to be
> reasonable. However, to allow execute the same prepared statement via
> different sessions we should keep trace of original query - its hash
> value. So the new proposal is:

A back of the envelope calculation shows that if you have 40
nodes, 32 cores, 100 connections on each instance, it gives you 

40*32*100*5mb = 625 GB of prepared statement cache!

Why does someone else have to point it out to the core team?

> - Each session holds map <stmt_id : query_hash>
> - There's one global hash <query_hash : stmt>
> - Each statement now has reference counter: each "prepare" call via
>   different session bumps its value; each "unprepare" call results in
>   its decrement. Disconect of session leads to counter decrements of all
>   related statements. Statement is not immediately deallocated if its
>   counter is 0: we maintaint global list of statements to be freed when
>   memory limit is reached
> - On "prepare" call we firstly check if there's enough space for statement.
>   If memory limit has reached, we traverse list of statements to be freed
>   and release all occupied resources. It would allow us to solve possible
>   overhead on disconnect event.

Here's how having cache global is connected to using string
identifiers:

If you make the cache opaque to the client you don't have
this mess with explicit referencing and dereferencing from
sessions.

Besides, as I already mentioned multiple times, a single session
may use multiple references to the same prepared statement - since
the protocol is fully asynchronous.

The cache has to be pure LRU, with automatic allocation and
expiration of objects. This will save CPU cycles on disconnect
as well as make the implementation simpler.

You can't do this if you have numeric identifiers, because you
must keep the object around as long as the identifier is around.

-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 13/15] sql: introduce cache for prepared statemets

[Georgy Kirichenko]

[-- Attachment #1: Type: text/plain, Size: 4924 bytes --]

On Monday, November 11, 2019 9:35:15 PM MSK Konstantin Osipov wrote:
> * Nikita Pettik <korablev@tarantool.org> [19/11/11 21:27]:
> > Today Kirill came to me and said that there's request from solution
> > team to make cache global: according to them they use many connections
> > to execute the same set of queries. That turns out global cache to be
> > reasonable. However, to allow execute the same prepared statement via
> > different sessions we should keep trace of original query - its hash
> 
> > value. So the new proposal is:
> A back of the envelope calculation shows that if you have 40
> nodes, 32 cores, 100 connections on each instance, it gives you
> 
> 40*32*100*5mb = 625 GB of prepared statement cache!
Why did you make this fake up?
1. why did you pack instances so dense?
2. why did you use 5mb instead of 1-2-5kb, for instance.
4. why did you count connections because the only thing is matter is count of 
unique sql statements.
4. why did you use the word `cache`.
Prepared statements and statement cache are the completely different 
substances. Obviously, if somebody prepared a statement then they wish the 
statement to do not be evicted from the prepared statements list.
> 
> Why does someone else have to point it out to the core team?
Seriously?
> 
> > - Each session holds map <stmt_id : query_hash>
> > - There's one global hash <query_hash : stmt>
> > - Each statement now has reference counter: each "prepare" call via
> > 
> >   different session bumps its value; each "unprepare" call results in
> >   its decrement. Disconect of session leads to counter decrements of all
> >   related statements. Statement is not immediately deallocated if its
> >   counter is 0: we maintaint global list of statements to be freed when
> >   memory limit is reached
> > 
> > - On "prepare" call we firstly check if there's enough space for
> > statement.
> > 
> >   If memory limit has reached, we traverse list of statements to be freed
> >   and release all occupied resources. It would allow us to solve possible
> >   overhead on disconnect event.
> 
> Here's how having cache global is connected to using string
> identifiers:
> 
> If you make the cache opaque to the client you don't have
> this mess with explicit referencing and dereferencing from
> sessions.
User should explicitly unprepare any statement they prepared before. Or it 
would be done by on_session_stop trigger. As we do not want to evict a 
prepared statement from prepared statement list, really.
> 
> Besides, as I already mentioned multiple times, a single session
> may use multiple references to the same prepared statement - since
> the protocol is fully asynchronous.
Wrong. The connection could but, please, do not forget about streams (which 
would encapsulate a single user session) we want to introduce in the near 
future.
> 
> The cache has to be pure LRU, with automatic allocation and
> expiration of objects. This will save CPU cycles on disconnect
> as well as make the implementation simpler.
One more time: we do not talk about statement cache but we discuss the 
prepared statement.
In any case we should prefer MPI instead of LRU and the discuss hot/cold zones 
politics. 
> 
> You can't do this if you have numeric identifiers, because you
> must keep the object around as long as the identifier is around.
Yes, and it is the feature requirement - if a statement was prepared then it 
should be prepared until user did not request the statement unprepare.

So I would like to explain my own vision of the feature:
1. Each instance has its own prepared statements table from which a statement 
vm could be fetched using the statement key (sql source string hash, for 
instance). Also the table may contain some meta: list of accessed objects (to 
prevent us from access checking on each invocation), count of invocation, sql 
source, reference counter.
2. Each session has a limit which limits the amount of resources, count of 
prepared statement for instance.
3. Each session contain the list of prepared statement identifiers used by this 
session.
4. Statement cache could be used by the feature but it is not essential. At 
least it relaxes the feature implementation complexity - we should not 
implement a good statement cache, eviction politics and other stuff like this 
right now.
5. If a client loses their session (crash, network failure) then it should 
reestablish a session and reprepare all statement it wish to use. It is more 
simply to implement in comparison with any caching, eviction, tracking and 
other things you suggested.
6. Right now a connection could contain only one session and it is not a stop-
factor - the only thing we should keep in mind - prepared statement relates to 
a session not to a connection. Then, when we will implement streams and 
multiple sessions per connection, we should not do any more changes to support 
prepared statements in this circumstances.

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [Tarantool-patches] [PATCH 13/15] sql: introduce cache for prepared statemets

[Konstantin Osipov]

* Georgy Kirichenko <georgy@tarantool.org> [19/11/12 11:03]:
> > > value. So the new proposal is:
> > A back of the envelope calculation shows that if you have 40
> > nodes, 32 cores, 100 connections on each instance, it gives you
> > 
> > 40*32*100*5mb = 625 GB of prepared statement cache!
> Why did you make this fake up?
> 1. why did you pack instances so dense?

What you should have done is looked at the worst case. 

The case above is not the worst. The desnity doesn't matter much 
- halve it and you get 300GB, even 50G for a cache on 40 nodes 
is too much.

> 2. why did you use 5mb instead of 1-2-5kb, for instance.

That's the default cache size. And it's a small size, too, a
single compiled AST for a SELECT can easily take 16-64kb (measure
it!), so it's a cache for ~100-200 statements.

> 4. why did you count connections because the only thing is matter is count of 
> unique sql statements.

With a per-session cache, you need to count connections. With a
global cache the numbers are not bad.

> 4. why did you use the word `cache`.
> Prepared statements and statement cache are the completely different 
> substances. Obviously, if somebody prepared a statement then they wish the 
> statement to do not be evicted from the prepared statements list.

This is some kind of 1980's myth. The reason everybody does it
this way is that virtually all databases that support prepared
statements are vertically scalable process-per-connection.

Otherwise, the approach has no benefits. In particular, there is
no value in making the user explicitly deallocate a prepared
statement. It's a pain to not be able to re-use a prepared
statement handle across connections. 

There are some risks in having a global transparent cache, too:
i.e. one has to watch out for security issues, if the same
statement can be reused across connections with different
credentials. But these risks do not materialize in case of
tarantool.

> > 
> > If you make the cache opaque to the client you don't have
> > this mess with explicit referencing and dereferencing from
> > sessions.
> User should explicitly unprepare any statement they prepared before. Or it 
> would be done by on_session_stop trigger. As we do not want to evict a 
> prepared statement from prepared statement list, really.

There is no reason for this, except copying someone else's design.
You're just making users do more work, and doing more work on the
server side on disconnect.

> Yes, and it is the feature requirement - if a statement was prepared then it 
> should be prepared until user did not request the statement unprepare.

There is no such requirement. 

> So I would like to explain my own vision of the feature:
> 1. Each instance has its own prepared statements table from which a statement 
> vm could be fetched using the statement key (sql source string hash, for 
> instance). Also the table may contain some meta: list of accessed objects (to 
> prevent us from access checking on each invocation), count of invocation, sql 
> source, reference counter.
> 2. Each session has a limit which limits the amount of resources, count of 
> prepared statement for instance.

So users have double trouble: not only they need to explicitly
deallocate prepare, they also need to be able to handle an
out-of-resources error, such as
out-of-prepared-statements-cache-memory. 

This "vision" happens to copy all the bugs of Oracle. 

- connect/disconnect is slow. 
- users have to do more work
- there is a new kind of error - out of statement cache - which
  one has to be prepared for. There is no way to work this error
  around on application side
- the server side still has to handle concurrent execution 
  of the same prepared statement, the protocol allows it today,
  so the implementaiton either has to ban it or create a copy on
  the fly (i.e. implicitly prepare another vdbe anyway).

The only benefit of explicit allocate/deallocate is that there is
some kind of guarantee that the statement is not going to get
parsed again when it is executed. This is very little value: I'm
not aware of cases when this is needed, and one can create a view instead. 

> 3. Each session contain the list of prepared statement identifiers used by this 
> session.
> 4. Statement cache could be used by the feature but it is not essential. At 
> least it relaxes the feature implementation complexity - we should not 
> implement a good statement cache, eviction politics and other stuff like this 
> right now.
> 5. If a client loses their session (crash, network failure) then it should 
> reestablish a session and reprepare all statement it wish to use. It is more 
> simply to implement in comparison with any caching, eviction, tracking and 
> other things you suggested.

No, it's not simple to implement, certainly not simpler than a
global cache with its own opaque eviction strategy. The simplest
implementation of the global cache is not caching anything at all
and preparing before every execute - which is good enough for JDBC
and ODBC.

> 6. Right now a connection could contain only one session and it is not a stop-
> factor - the only thing we should keep in mind - prepared statement relates to 
> a session not to a connection. Then, when we will implement streams and 
> multiple sessions per connection, we should not do any more changes to support 
> prepared statements in this circumstances.

If you suggest that a single session can execute only one
statement concurrently, you should be explicit about it - and it's
a huge limitation.


-- 
Konstantin Osipov, Moscow, Russia

Re: [Tarantool-patches] [PATCH 13/15] sql: introduce cache for prepared statemets

[Georgy Kirichenko]

[-- Attachment #1: Type: text/plain, Size: 8712 bytes --]

On Tuesday, November 12, 2019 11:50:52 AM MSK Konstantin Osipov wrote:
> * Georgy Kirichenko <georgy@tarantool.org> [19/11/12 11:03]:
> > > > value. So the new proposal is:
> > > A back of the envelope calculation shows that if you have 40
> > > nodes, 32 cores, 100 connections on each instance, it gives you
> > > 
> > > 40*32*100*5mb = 625 GB of prepared statement cache!
> > 
> > Why did you make this fake up?
> > 1. why did you pack instances so dense?
> 
> What you should have done is looked at the worst case.
> 
> The case above is not the worst. The desnity doesn't matter much
> - halve it and you get 300GB, even 50G for a cache on 40 nodes
> is too much.
> 
> > 2. why did you use 5mb instead of 1-2-5kb, for instance.
> 
> That's the default cache size. And it's a small size, too, a
> single compiled AST for a SELECT can easily take 16-64kb (measure
> it!), so it's a cache for ~100-200 statements.
I do not talk about cache size. Which cache, why cache? I consider only a 
prepared statement resource footprint and count of prepared statements on a 
single instance. There is no cache yet.
> 
> > 4. why did you count connections because the only thing is matter is count
> > of unique sql statements.
> 
> With a per-session cache, you need to count connections. With a
> global cache the numbers are not bad.
We will have more than one session per connection, please keep it in mind. All 
statements in a session would execute sequentially otherwise there is no 
transaction consistency in a session boundaries. 
> 
> > 4. why did you use the word `cache`.
> > Prepared statements and statement cache are the completely different
> > substances. Obviously, if somebody prepared a statement then they wish the
> > statement to do not be evicted from the prepared statements list.
> 
> This is some kind of 1980's myth. 
And it does not mean that this approach is bad.
> The reason everybody does it
> this way is that virtually all databases that support prepared
> statements are vertically scalable process-per-connection.
> 
> Otherwise, the approach has no benefits. In particular, there is
> no value in making the user explicitly deallocate a prepared
> statement. It's a pain to not be able to re-use a prepared
> statement handle across connections.
Your approach suggested there is no more prepared statements because there is 
no difference between prepared and unprepared except its argument marshaling 
which could be done using client library. Because simple statement also should 
be put into a statement cache. And if I prepared a statement I definitely want 
my prepared statement still persist without any dependencies how many other 
statement were executed.  
Anyway, in case on reconnect a session should be reestablished so there is no 
big deal to reprepare all statement. And it is no a big deal to iterate over 
session prepared statement maps to decrement all corresponding statement 
reference counts.
Also my approach allows to reuse a statement prepared by other session because 
they both would share the prepared statement reference (sql source hash).
Additionally, it is a very strange opinion that a client should be able to 
reuse prepared statement event in cases when a server crashes or a client 
initiates a connection to another server or event cluster. Otherwise there is 
no difference between prepared statements and simple unprepared statements.
> 
> There are some risks in having a global transparent cache, too:
> i.e. one has to watch out for security issues, if the same
> statement can be reused across connections with different
> credentials. But these risks do not materialize in case of
> tarantool.
Wrong, please keep in mind multisession connection, vshard and proxy.
Also a prepared statement sql could contain session-local variables. 
> 
> > > If you make the cache opaque to the client you don't have
> > > this mess with explicit referencing and dereferencing from
> > > sessions.
> > 
> > User should explicitly unprepare any statement they prepared before. Or it
> > would be done by on_session_stop trigger. As we do not want to evict a
> > prepared statement from prepared statement list, really.
> 
> There is no reason for this, except copying someone else's design.
> You're just making users do more work, and doing more work on the
> server side on disconnect.
Amount of work I suggest to do is much less in comparison with a good 
statement cache (simple LRU is completely not enough), statement repreparation 
if it is unknown or evicted, cache statistic, global cache sharing and access 
checking. My approach is straightforward and predictable.
> 
> > Yes, and it is the feature requirement - if a statement was prepared then
> > it should be prepared until user did not request the statement unprepare.
> There is no such requirement.
> 
> > So I would like to explain my own vision of the feature:
> > 1. Each instance has its own prepared statements table from which a
> > statement vm could be fetched using the statement key (sql source string
> > hash, for instance). Also the table may contain some meta: list of
> > accessed objects (to prevent us from access checking on each invocation),
> > count of invocation, sql source, reference counter.
> > 2. Each session has a limit which limits the amount of resources, count of
> > prepared statement for instance.
> 
> So users have double trouble: not only they need to explicitly
> deallocate prepare, they also need to be able to handle an
> out-of-resources error, such as
> out-of-prepared-statements-cache-memory.
Yes, and it is completely Ok.
> 
> This "vision" happens to copy all the bugs of Oracle.
You compare my approach with the most popular business database, thanks a lot.
> 
> - connect/disconnect is slow.
Please estimate costs of disconnect in terms of network pings, sql planning 
and wal latency (the only thing the server should do is to decrement around 
100-200 integers)
> - users have to do more work
Please estimate amount of work in comparison with users business logic 
implementation
> - there is a new kind of error - out of statement cache - which
I would like repeat: we do not talk about statement cache.
>   one has to be prepared for. There is no way to work this error
>   around on application side
> - the server side still has to handle concurrent execution
>   of the same prepared statement, the protocol allows it today,
>   so the implementaiton either has to ban it or create a copy on
>   the fly (i.e. implicitly prepare another vdbe anyway).
I do not see how did you make it up. Any prepared statement implementation 
should be ably to handle many prepared statement executions in parallel.
The other thing my approach suggests: vdbe should be able executed in 
parallel, but this limitation is applicable anyway.
> 
> The only benefit of explicit allocate/deallocate is that there is
> some kind of guarantee that the statement is not going to get
> parsed again when it is executed. This is very little value: I'm
> not aware of cases when this is needed, and one can create a view instead.
Please, make a proof.
> 
> > 3. Each session contain the list of prepared statement identifiers used by
> > this session.
> > 4. Statement cache could be used by the feature but it is not essential.
> > At
> > least it relaxes the feature implementation complexity - we should not
> > implement a good statement cache, eviction politics and other stuff like
> > this right now.
> > 5. If a client loses their session (crash, network failure) then it should
> > reestablish a session and reprepare all statement it wish to use. It is
> > more simply to implement in comparison with any caching, eviction,
> > tracking and other things you suggested.
> 
> No, it's not simple to implement, certainly not simpler than a
> global cache with its own opaque eviction strategy. The simplest
> implementation of the global cache is not caching anything at all
> and preparing before every execute - which is good enough for JDBC
> and ODBC.
Well, so you suggested not to implement prepared statements.
> 
> > 6. Right now a connection could contain only one session and it is not a
> > stop- factor - the only thing we should keep in mind - prepared statement
> > relates to a session not to a connection. Then, when we will implement
> > streams and multiple sessions per connection, we should not do any more
> > changes to support prepared statements in this circumstances.
> 
> If you suggest that a single session can execute only one
> statement concurrently, you should be explicit about it - and it's
> a huge limitation.
Please make a distinction between connection and session, we already discussed 
it some month before.


[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 488 bytes --]