[tarantool-patches] [PATCH v4 1/2] sql: remove useless pragmas

[tarantool-patches] [PATCH v4 1/2] sql: remove useless pragmas

[Stanislav Zudin]

The pragmas "query_only" and "read_uncommitted" didn't affect anything
and were removed.
Fixed an error in pragma index_list which caused a segmantation fault.
pragma sql_default_engine accepts only strings.
Thus pragma sql_default_engine('memtx') is a well-formed command,
while pragma sql_default_engine(memtx) or
pragma sql_default_engine("memtx") are considered as ill-formed and
raise an error.

Closes #3733
---
Branch: https://github.com/tarantool/tarantool/tree/stanztt/gh-3733-obsolete-pragmas
Issue: https://github.com/tarantool/tarantool/issues/3733

 src/box/sql/pragma.c                 |  29 ++++-
 src/box/sql/pragma.h                 |  42 +++----
 src/box/sql/sqliteInt.h              |   3 -
 test/sql-tap/gh-3733-pragma.test.lua | 177 +++++++++++++++++++++++++++
 4 files changed, 218 insertions(+), 33 deletions(-)
 create mode 100755 test/sql-tap/gh-3733-pragma.test.lua

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index eef1ed931..7285455c9 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -391,16 +391,32 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
 	struct space *space = space_by_name(tbl_name);
 	if (space == NULL)
 		return;
-	parse->nMem = 5;
+	parse->nMem = 3;
 	struct Vdbe *v = sqlite3GetVdbe(parse);
 	for (uint32_t i = 0; i < space->index_count; ++i) {
 		struct index *idx = space->index[i];
-		sqlite3VdbeMultiLoad(v, 1, "isisi", i, idx->def->name,
+		sqlite3VdbeMultiLoad(v, 1, "isi", i, idx->def->name,
 				     idx->def->opts.is_unique);
-		sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 5);
+		sqlite3VdbeAddOp2(v, OP_ResultRow, 1, 3);
 	}
 }
 
+/*
+ * @brief Check whether the specified token is a string or ID.
+ * @param token - token to be examined
+ * @return true - if the token value is enclosed into quotes (')
+ * @return false in other cases
+ * The empty value is considered to be a string.
+ */
+static bool
+token_is_string(const struct Token* token)
+{
+	if (!token || token->n == 0)
+		return true;
+	return token->n >= 2 && token->z[0] == '\'' &&
+	       token->z[token->n - 1] == '\'';
+}
+
 /*
  * Process a pragma statement.
  *
@@ -601,6 +617,13 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		}
 
 	case PragTyp_DEFAULT_ENGINE: {
+		if (!token_is_string(pValue)) {
+			diag_set(ClientError, ER_ILLEGAL_PARAMS,
+				 "string value is expected");
+			pParse->rc = SQL_TARANTOOL_ERROR;
+			pParse->nErr++;
+			goto pragma_out;
+		}
 		if (sql_default_engine_set(zRight) != 0) {
 			pParse->rc = SQL_TARANTOOL_ERROR;
 			pParse->nErr++;
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index e60801608..168c70561 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -55,22 +55,20 @@ static const char *const pragCName[] = {
 	/*  16 */ "seq",
 	/*  17 */ "name",
 	/*  18 */ "unique",
-	/*  19 */ "origin",
-	/*  20 */ "partial",
 	/* Used by: collation_list */
-	/*  21 */ "seq",
-	/*  22 */ "name",
+	/*  19 */ "seq",
+	/*  20 */ "name",
 	/* Used by: foreign_key_list */
-	/*  23 */ "id",
-	/*  24 */ "seq",
-	/*  25 */ "table",
-	/*  26 */ "from",
-	/*  27 */ "to",
-	/*  28 */ "on_update",
-	/*  29 */ "on_delete",
-	/*  30 */ "match",
+	/*  21 */ "id",
+	/*  22 */ "seq",
+	/*  23 */ "table",
+	/*  24 */ "from",
+	/*  25 */ "to",
+	/*  26 */ "on_update",
+	/*  27 */ "on_delete",
+	/*  28 */ "match",
 	/* Used by: busy_timeout */
-	/*  31 */ "timeout",
+	/*  29 */ "timeout",
 };
 
 /* Definitions of all built-in pragmas */
@@ -90,7 +88,7 @@ static const PragmaName aPragmaName[] = {
 	{ /* zName:     */ "busy_timeout",
 	 /* ePragTyp:  */ PragTyp_BUSY_TIMEOUT,
 	 /* ePragFlg:  */ PragFlg_Result0,
-	 /* ColNames:  */ 31, 1,
+	 /* ColNames:  */ 29, 1,
 	 /* iArg:      */ 0},
 	{ /* zName:     */ "case_sensitive_like",
 	 /* ePragTyp:  */ PragTyp_CASE_SENSITIVE_LIKE,
@@ -101,7 +99,7 @@ static const PragmaName aPragmaName[] = {
 	{ /* zName:     */ "collation_list",
 	 /* ePragTyp:  */ PragTyp_COLLATION_LIST,
 	 /* ePragFlg:  */ PragFlg_Result0,
-	 /* ColNames:  */ 21, 2,
+	 /* ColNames:  */ 19, 2,
 	 /* iArg:      */ 0},
 #endif
 #if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
@@ -122,7 +120,7 @@ static const PragmaName aPragmaName[] = {
 	 /* ePragTyp:  */ PragTyp_FOREIGN_KEY_LIST,
 	 /* ePragFlg:  */
 	 PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
-	 /* ColNames:  */ 23, 8,
+	 /* ColNames:  */ 21, 8,
 	 /* iArg:      */ 0},
 #if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
 	{ /* zName:     */ "full_column_names",
@@ -142,7 +140,7 @@ static const PragmaName aPragmaName[] = {
 	 /* ePragTyp:  */ PragTyp_INDEX_LIST,
 	 /* ePragFlg:  */
 	 PragFlg_NeedSchema | PragFlg_Result1 | PragFlg_SchemaOpt,
-	 /* ColNames:  */ 16, 5,
+	 /* ColNames:  */ 16, 3,
 	 /* iArg:      */ 0},
 #endif
 #if defined(SQLITE_DEBUG) && !defined(SQLITE_OMIT_PARSER_TRACE)
@@ -153,16 +151,6 @@ static const PragmaName aPragmaName[] = {
 	 /* iArg:      */ 0},
 #endif
 #if !defined(SQLITE_OMIT_FLAG_PRAGMAS)
-	{ /* zName:     */ "query_only",
-	 /* ePragTyp:  */ PragTyp_FLAG,
-	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ SQLITE_QueryOnly},
-	{ /* zName:     */ "read_uncommitted",
-	 /* ePragTyp:  */ PragTyp_FLAG,
-	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
-	 /* ColNames:  */ 0, 0,
-	 /* iArg:      */ SQLITE_ReadUncommitted},
 	{ /* zName:     */ "recursive_triggers",
 	 /* ePragTyp:  */ PragTyp_FLAG,
 	 /* ePragFlg:  */ PragFlg_Result0 | PragFlg_NoColumns1,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index ee24e0337..db7a20aee 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -1575,16 +1575,13 @@ struct sqlite3 {
 #define SQLITE_WhereTrace     0x00008000       /* Debug info about optimizer's work */
 #define SQLITE_VdbeListing    0x00000400	/* Debug listings of VDBE programs */
 #define SQLITE_VdbeAddopTrace 0x00001000	/* Trace sqlite3VdbeAddOp() calls */
-#define SQLITE_ReadUncommitted 0x0004000	/* For shared-cache mode */
 #define SQLITE_ReverseOrder   0x00020000	/* Reverse unordered SELECTs */
 #define SQLITE_RecTriggers    0x00040000	/* Enable recursive triggers */
 #define SQLITE_AutoIndex      0x00100000	/* Enable automatic indexes */
 #define SQLITE_PreferBuiltin  0x00200000	/* Preference to built-in funcs */
 #define SQLITE_EnableTrigger  0x01000000	/* True to enable triggers */
 #define SQLITE_DeferFKs       0x02000000	/* Defer all FK constraints */
-#define SQLITE_QueryOnly      0x04000000	/* Disable database changes */
 #define SQLITE_VdbeEQP        0x08000000	/* Debug EXPLAIN QUERY PLAN */
-#define SQLITE_NoCkptOnClose  0x80000000	/* No checkpoint on close()/DETACH */
 
 /*
  * Bits of the sqlite3.dbOptFlags field that are used by the
diff --git a/test/sql-tap/gh-3733-pragma.test.lua b/test/sql-tap/gh-3733-pragma.test.lua
new file mode 100755
index 000000000..216f0f8dd
--- /dev/null
+++ b/test/sql-tap/gh-3733-pragma.test.lua
@@ -0,0 +1,177 @@
+#!/usr/bin/env tarantool
+test = require("sqltester")
+
+test:plan(17)
+
+---
+--- Prerequisites
+---
+test:do_execsql_test(
+    "pragma-0.1",
+    [[
+        DROP TABLE IF EXISTS gh3733;
+        CREATE TABLE gh3733(id INT primary key, f float);
+        INSERT INTO gh3733 VALUES(1, 0.1), (2, 0.2), (3, 0.3);
+        CREATE INDEX IDX ON GH3733 (id);
+    ]], {
+
+})
+
+---
+--- pragma query_only is not supported
+---
+test:do_catchsql_test(
+    "pragma-1.1",
+    [[
+        pragma query_only;
+    ]], {
+        1, "no such pragma: QUERY_ONLY"
+})
+
+---
+--- pragma read_uncommitted is not supported
+---
+test:do_catchsql_test(
+	"pragma-2.1",
+	[[
+        pragma read_uncommitted;
+    ]], {
+	1, "no such pragma: READ_UNCOMMITTED"
+})
+
+---
+--- pragma index_list returns three columns in a row
+---
+test:do_execsql_test(
+	"pragma-3.1",
+	[[
+        pragma index_list(gh3733)
+    ]], {
+	-- <limit-1.2.1>
+	0, 'pk_unnamed_GH3733_1', 1, 1, 'IDX', 0
+	-- </limit-1.2.1>
+})
+
+---
+--- pragma index_list returns an empty tuple for unknown table
+---
+test:do_execsql_test(
+	"pragma-4.1",
+	[[
+        pragma index_list(fufel);
+    ]], {
+	-- <pragma-4.1>
+	-- </pragma-4.1>
+})
+
+---
+--- pragma index_info returns an empty tuple for unknown index
+---
+test:do_execsql_test(
+	"pragma-5.1",
+	[[
+        pragma index_info(gh3733.IDX)
+    ]], {
+	-- <pragma-5.1>
+	0, 0, 'ID', 0, 'BINARY', 'integer'
+	-- </pragma-5.1>
+})
+
+test:do_execsql_test(
+	"pragma-5.2",
+	[[
+        pragma index_info(no_table);
+    ]], {
+	-- <pragma-5.2>
+	-- </pragma-5.2>
+})
+
+test:do_execsql_test(
+	"pragma-5.3",
+	[[
+        pragma index_info(wrong_table.IDX);
+    ]], {
+	-- <pragma-5.3>
+	-- </pragma-5.3>
+})
+
+test:do_execsql_test(
+	"pragma-5.4",
+	[[
+        pragma index_info(gh3733.wrong_index);
+    ]], {
+	-- <pragma-5.4>
+	-- </pragma-5.4>
+})
+
+---
+--- pragma sql_default_engine requires value
+---
+test:do_catchsql_test(
+	"pragma-6.1",
+	[[
+        pragma sql_default_engine;
+    ]], {
+	1, "Illegal parameters, 'sql_default_engine' was not specified"
+})
+
+---
+--- pragma sql_default_engine accepts string values and rejects IDs
+---
+test:do_catchsql_test(
+	"pragma-7.1",
+	[[
+        pragma sql_default_engine(the_engine);
+    ]], {
+	1, "Illegal parameters, string value is expected"
+})
+test:do_catchsql_test(
+	"pragma-7.2",
+	[[
+        pragma sql_default_engine(THE_ENGINE);
+    ]], {
+	1, "Illegal parameters, string value is expected"
+})
+test:do_catchsql_test(
+	"pragma-7.3",
+	[[
+        pragma sql_default_engine("THE_ENGINE");
+    ]], {
+	1, "Illegal parameters, string value is expected"
+})
+
+test:do_catchsql_test(
+	"pragma-7.4",
+	[[
+        pragma sql_default_engine('THE_ENGINE');
+    ]], {
+	1, "Space engine 'THE_ENGINE' does not exist"
+})
+
+test:do_catchsql_test(
+	"pragma-7.5",
+	[[
+        pragma sql_default_engine(memtx);
+    ]], {
+	1, "Illegal parameters, string value is expected"
+})
+
+test:do_catchsql_test(
+	"pragma-7.6",
+	[[
+        pragma sql_default_engine("memtx");
+    ]], {
+	1, "Illegal parameters, string value is expected"
+})
+
+test:do_execsql_test(
+	"pragma-7.7",
+	[[
+        pragma sql_default_engine('memtx');
+    ]], {
+	-- <pragma-7.7>
+
+	-- </pragma-7.7>
+})
+
+test:finish_test()
-- 
2.17.1

[tarantool-patches] [PATCH v4 2/2] sql: remove busy handler

[Stanislav Zudin]

SQLite's busyHandler functionality and functions sqlite3_sleep(),
sqlite3OsSleep() were not used and were removed.

Closes #3733
---
Branch: https://github.com/tarantool/tarantool/tree/stanztt/gh-3733-obsolete-pragmas
Issue: https://github.com/tarantool/tarantool/issues/3733

 src/box/sql/main.c      | 76 -----------------------------------------
 src/box/sql/os.c        |  6 ----
 src/box/sql/os.h        |  1 -
 src/box/sql/os_unix.c   | 18 ----------
 src/box/sql/pragma.c    | 23 ++++---------
 src/box/sql/pragma.h    |  8 -----
 src/box/sql/sqliteInt.h | 23 -------------
 src/box/sql/vdbe.c      |  1 -
 8 files changed, 6 insertions(+), 150 deletions(-)

diff --git a/src/box/sql/main.c b/src/box/sql/main.c
index 8574d6464..5555cb48e 100644
--- a/src/box/sql/main.c
+++ b/src/box/sql/main.c
@@ -640,43 +640,6 @@ sqlite3ErrStr(int rc)
 	return zErr;
 }
 
-/*
- * This routine implements a busy callback that sleeps and tries
- * again until a timeout value is reached.  The timeout value is
- * an integer number of milliseconds passed in as the first
- * argument.
- */
-static int
-sqliteDefaultBusyCallback(void *ptr,	/* Database connection */
-			  int count)	/* Number of times table has been busy */
-{
-	sqlite3 *db = (sqlite3 *) ptr;
-	int timeout = ((sqlite3 *) ptr)->busyTimeout;
-	if ((count + 1) * 1000 > timeout) {
-		return 0;
-	}
-	sqlite3OsSleep(db->pVfs, 1000000);
-	return 1;
-}
-
-/*
- * This routine sets the busy callback for an Sqlite database to the
- * given callback function with the given argument.
- */
-int
-sqlite3_busy_handler(sqlite3 * db, int (*xBusy) (void *, int), void *pArg)
-{
-#ifdef SQLITE_ENABLE_API_ARMOR
-	if (!sqlite3SafetyCheckOk(db))
-		return SQLITE_MISUSE_BKPT;
-#endif
-	db->busyHandler.xFunc = xBusy;
-	db->busyHandler.pArg = pArg;
-	db->busyHandler.nBusy = 0;
-	db->busyTimeout = 0;
-	return SQLITE_OK;
-}
-
 #ifndef SQLITE_OMIT_PROGRESS_CALLBACK
 /*
  * This routine sets the progress callback for an Sqlite database to the
@@ -705,26 +668,6 @@ sqlite3_progress_handler(sqlite3 * db,
 }
 #endif
 
-/*
- * This routine installs a default busy handler that waits for the
- * specified number of milliseconds before returning 0.
- */
-int
-sqlite3_busy_timeout(sqlite3 * db, int ms)
-{
-#ifdef SQLITE_ENABLE_API_ARMOR
-	if (!sqlite3SafetyCheckOk(db))
-		return SQLITE_MISUSE_BKPT;
-#endif
-	if (ms > 0) {
-		sqlite3_busy_handler(db, sqliteDefaultBusyCallback, (void *)db);
-		db->busyTimeout = ms;
-	} else {
-		sqlite3_busy_handler(db, 0, 0);
-	}
-	return SQLITE_OK;
-}
-
 /*
  * Cause any pending operation to stop at its earliest opportunity.
  */
@@ -1676,25 +1619,6 @@ sqlite3IoerrnomemError(int lineno)
 }
 #endif
 
-/*
- * Sleep for a little while.  Return the amount of time slept.
- */
-int
-sqlite3_sleep(int ms)
-{
-	sqlite3_vfs *pVfs;
-	int rc;
-	pVfs = sqlite3_vfs_find(0);
-	if (pVfs == 0)
-		return 0;
-
-	/* This function works in milliseconds, but the underlying OsSleep()
-	 * API uses microseconds. Hence the 1000's.
-	 */
-	rc = (sqlite3OsSleep(pVfs, 1000 * ms) / 1000);
-	return rc;
-}
-
 /*
  * Enable or disable the extended result codes.
  */
diff --git a/src/box/sql/os.c b/src/box/sql/os.c
index 1fa6b332a..3840258dc 100644
--- a/src/box/sql/os.c
+++ b/src/box/sql/os.c
@@ -128,12 +128,6 @@ sqlite3OsRandomness(sqlite3_vfs * pVfs, int nByte, char *zBufOut)
 	return pVfs->xRandomness(pVfs, nByte, zBufOut);
 }
 
-int
-sqlite3OsSleep(sqlite3_vfs * pVfs, int nMicro)
-{
-	return pVfs->xSleep(pVfs, nMicro);
-}
-
 int
 sqlite3OsGetLastError(sqlite3_vfs * pVfs)
 {
diff --git a/src/box/sql/os.h b/src/box/sql/os.h
index 947658300..c844fe7fa 100644
--- a/src/box/sql/os.h
+++ b/src/box/sql/os.h
@@ -146,7 +146,6 @@ int sqlite3OsUnfetch(sqlite3_file *, i64, void *);
  */
 int sqlite3OsOpen(sqlite3_vfs *, const char *, sqlite3_file *, int, int *);
 int sqlite3OsRandomness(sqlite3_vfs *, int, char *);
-int sqlite3OsSleep(sqlite3_vfs *, int);
 int sqlite3OsGetLastError(sqlite3_vfs *);
 int sqlite3OsCurrentTimeInt64(sqlite3_vfs *, sqlite3_int64 *);
 
diff --git a/src/box/sql/os_unix.c b/src/box/sql/os_unix.c
index b8816e0f9..2cbcd6db1 100644
--- a/src/box/sql/os_unix.c
+++ b/src/box/sql/os_unix.c
@@ -1988,23 +1988,6 @@ unixRandomness(sqlite3_vfs * NotUsed, int nBuf, char *zBuf)
 	return nBuf;
 }
 
-/*
- * Sleep for a little while.  Return the amount of time slept.
- * The argument is the number of microseconds we want to sleep.
- * The return value is the number of microseconds of sleep actually
- * requested from the underlying operating system, a number which
- * might be greater than or equal to the argument, but not less
- * than the argument.
- */
-static int
-unixSleep(sqlite3_vfs * NotUsed, int microseconds)
-{
-	int seconds = (microseconds + 999999) / 1000000;
-	sleep(seconds);
-	UNUSED_PARAMETER(NotUsed);
-	return seconds * 1000000;
-}
-
 /* Fake system time in seconds since 1970. */
 int sqlite3_current_time = 0;
 
@@ -2078,7 +2061,6 @@ unixGetLastError(sqlite3_vfs * NotUsed, int NotUsed2, char *NotUsed3)
     unixOpen,             /* xOpen */                       \
     unixDelete,           /* xDelete */                     \
     unixRandomness,       /* xRandomness */                 \
-    unixSleep,            /* xSleep */                      \
     NULL,                 /* xCurrentTime */                \
     unixGetLastError,     /* xGetLastError */               \
     unixCurrentTimeInt64, /* xCurrentTimeInt64 */           \
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 7285455c9..62ff1f2b2 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -464,7 +464,6 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		zRight = sqlite3NameFromToken(db, pValue);
 	}
 	zTable = sqlite3NameFromToken(db, pValue2);
-	db->busyHandler.nBusy = 0;
 
 	/* Locate the pragma in the lookup table */
 	pPragma = pragmaLocate(zLeft);
@@ -644,24 +643,14 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
 		break;
 	}
 
-	/* *   PRAGMA busy_timeout *   PRAGMA busy_timeout = N *
-	 *
-	 * Call sqlite3_busy_timeout(db, N).  Return the current
-	 * timeout value * if one is set.  If no busy handler
-	 * or a different busy handler is set * then 0 is
-	 * returned.  Setting the busy_timeout to 0 or negative *
-	 * disables the timeout.
-	 */
-	/* case PragTyp_BUSY_TIMEOUT */
 	default:{
-			assert(pPragma->ePragTyp == PragTyp_BUSY_TIMEOUT);
-			if (zRight) {
-				sqlite3_busy_timeout(db, sqlite3Atoi(zRight));
-			}
-			returnSingleInt(v, db->busyTimeout);
-			break;
+		/* We shouldn't get here. */
+		diag_set(ClientError, ER_UNKNOWN);
+		pParse->rc = SQL_TARANTOOL_ERROR;
+		pParse->nErr++;
+		goto pragma_out;
 		}
-	}			/* End of the PRAGMA switch */
+	}
 
 	/* The following block is a no-op unless SQLITE_DEBUG is
 	 * defined. Its only * purpose is to execute assert()
diff --git a/src/box/sql/pragma.h b/src/box/sql/pragma.h
index 168c70561..2857390c4 100644
--- a/src/box/sql/pragma.h
+++ b/src/box/sql/pragma.h
@@ -5,7 +5,6 @@
  */
 
 /* The various pragma types */
-#define PragTyp_BUSY_TIMEOUT                   1
 #define PragTyp_CASE_SENSITIVE_LIKE            2
 #define PragTyp_COLLATION_LIST                 3
 #define PragTyp_FLAG                           5
@@ -67,8 +66,6 @@ static const char *const pragCName[] = {
 	/*  26 */ "on_update",
 	/*  27 */ "on_delete",
 	/*  28 */ "match",
-	/* Used by: busy_timeout */
-	/*  29 */ "timeout",
 };
 
 /* Definitions of all built-in pragmas */
@@ -85,11 +82,6 @@ typedef struct PragmaName {
  * to be sorted. For more info see pragma_locate function.
  */
 static const PragmaName aPragmaName[] = {
-	{ /* zName:     */ "busy_timeout",
-	 /* ePragTyp:  */ PragTyp_BUSY_TIMEOUT,
-	 /* ePragFlg:  */ PragFlg_Result0,
-	 /* ColNames:  */ 29, 1,
-	 /* iArg:      */ 0},
 	{ /* zName:     */ "case_sensitive_like",
 	 /* ePragTyp:  */ PragTyp_CASE_SENSITIVE_LIKE,
 	 /* ePragFlg:  */ PragFlg_NoColumns,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index db7a20aee..1633c1a35 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -329,7 +329,6 @@ struct sqlite3_vfs {
 		      int flags, int *pOutFlags);
 	int (*xDelete) (sqlite3_vfs *, const char *zName, int syncDir);
 	int (*xRandomness) (sqlite3_vfs *, int nByte, char *zOut);
-	int (*xSleep) (sqlite3_vfs *, int microseconds);
 	int (*xCurrentTime) (sqlite3_vfs *, double *);
 	int (*xGetLastError) (sqlite3_vfs *, int, char *);
 	/*
@@ -825,7 +824,6 @@ struct sqlite3_io_methods {
 #define SQLITE_FCNTL_VFSNAME                11
 #define SQLITE_FCNTL_POWERSAFE_OVERWRITE    12
 #define SQLITE_FCNTL_PRAGMA                 13
-#define SQLITE_FCNTL_BUSYHANDLER            14
 #define SQLITE_FCNTL_TEMPFILENAME           15
 #define SQLITE_FCNTL_MMAP_SIZE              16
 #define SQLITE_FCNTL_TRACE                  17
@@ -835,9 +833,6 @@ struct sqlite3_io_methods {
 int
 sqlite3_os_init(void);
 
-int
-sqlite3_busy_timeout(sqlite3 *, int ms);
-
 sqlite3_int64
 sqlite3_soft_heap_limit64(sqlite3_int64 N);
 
@@ -1310,22 +1305,6 @@ extern const int sqlite3one;
 #undef WHERETRACE_ENABLED
 #endif
 
-/*
- * An instance of the following structure is used to store the busy-handler
- * callback for a given sqlite handle.
- *
- * The sqlite.busyHandler member of the sqlite struct contains the busy
- * callback for the database handle. Each pager opened via the sqlite
- * handle is passed a pointer to sqlite.busyHandler. The busy-handler
- * callback is currently invoked only from within pager.c.
- */
-typedef struct BusyHandler BusyHandler;
-struct BusyHandler {
-	int (*xFunc) (void *, int);	/* The busy callback */
-	void *pArg;		/* First arg to busy callback */
-	int nBusy;		/* Incremented with each busy call */
-};
-
 /*
  * A convenience macro that returns the number of elements in
  * an array.
@@ -1554,8 +1533,6 @@ struct sqlite3 {
 	unsigned nProgressOps;	/* Number of opcodes for progress callback */
 #endif
 	Hash aFunc;		/* Hash table of connection functions */
-	BusyHandler busyHandler;	/* Busy callback */
-	int busyTimeout;	/* Busy handler timeout, in msec */
 	int *pnBytesFreed;	/* If not NULL, increment this in DbFree() */
 };
 
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9fc362f0a..481727b5e 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -666,7 +666,6 @@ int sqlite3VdbeExec(Vdbe *p)
 	p->iCurrentTime = 0;
 	assert(p->explain==0);
 	p->pResultSet = 0;
-	db->busyHandler.nBusy = 0;
 	if (db->u1.isInterrupted) goto abort_due_to_interrupt;
 	sqlite3VdbeIOTraceSql(p);
 #ifndef SQLITE_OMIT_PROGRESS_CALLBACK
-- 
2.17.1

[tarantool-patches] Re: [PATCH v4 2/2] sql: remove busy handler

[n.pettik]

> -	 *
> -	 * Call sqlite3_busy_timeout(db, N).  Return the current
> -	 * timeout value * if one is set.  If no busy handler
> -	 * or a different busy handler is set * then 0 is
> -	 * returned.  Setting the busy_timeout to 0 or negative *
> -	 * disables the timeout.
> -	 */
> -	/* case PragTyp_BUSY_TIMEOUT */
> 	default:{
> -			assert(pPragma->ePragTyp == PragTyp_BUSY_TIMEOUT);
> -			if (zRight) {
> -				sqlite3_busy_timeout(db, sqlite3Atoi(zRight));
> -			}
> -			returnSingleInt(v, db->busyTimeout);
> -			break;
> +		/* We shouldn't get here. */
> +		diag_set(ClientError, ER_UNKNOWN);
> +		pParse->rc = SQL_TARANTOOL_ERROR;
> +		pParse->nErr++;
> +		goto pragma_out;

If we really can’t reach here under no circumstances
(which seems to be true - pragmaLocate checks this) then
it’s better to place unreachable(); assert. Fix this please.

[tarantool-patches] Re: [PATCH v4 2/2] sql: remove busy handler

[Stanislav Zudin]

On 12.02.2019 20:05, n.pettik wrote:
>> -	 *
>> -	 * Call sqlite3_busy_timeout(db, N).  Return the current
>> -	 * timeout value * if one is set.  If no busy handler
>> -	 * or a different busy handler is set * then 0 is
>> -	 * returned.  Setting the busy_timeout to 0 or negative *
>> -	 * disables the timeout.
>> -	 */
>> -	/* case PragTyp_BUSY_TIMEOUT */
>> 	default:{
>> -			assert(pPragma->ePragTyp == PragTyp_BUSY_TIMEOUT);
>> -			if (zRight) {
>> -				sqlite3_busy_timeout(db, sqlite3Atoi(zRight));
>> -			}
>> -			returnSingleInt(v, db->busyTimeout);
>> -			break;
>> +		/* We shouldn't get here. */
>> +		diag_set(ClientError, ER_UNKNOWN);
>> +		pParse->rc = SQL_TARANTOOL_ERROR;
>> +		pParse->nErr++;
>> +		goto pragma_out;
> If we really can’t reach here under no circumstances
> (which seems to be true - pragmaLocate checks this) then
> it’s better to place unreachable(); assert. Fix this please.
Please find the fix below.


  src/box/sql/pragma.c | 9 ++-------
  1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 62ff1f2b2..1b456e0b5 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -643,13 +643,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First 
part of [schema.]id field */
          break;
      }

-    default:{
-        /* We shouldn't get here. */
-        diag_set(ClientError, ER_UNKNOWN);
-        pParse->rc = SQL_TARANTOOL_ERROR;
-        pParse->nErr++;
-        goto pragma_out;
-        }
+    default:
+        unreachable();
      }

      /* The following block is a no-op unless SQLITE_DEBUG is
-- 
2.17.1

[tarantool-patches] Re: [PATCH v4 2/2] sql: remove busy handler

[n.pettik]

> On 13 Feb 2019, at 10:10, Stanislav Zudin <szudin@tarantool.org> wrote:
> On 12.02.2019 20:05, n.pettik wrote:
>>> -	 *
>>> -	 * Call sqlite3_busy_timeout(db, N).  Return the current
>>> -	 * timeout value * if one is set.  If no busy handler
>>> -	 * or a different busy handler is set * then 0 is
>>> -	 * returned.  Setting the busy_timeout to 0 or negative *
>>> -	 * disables the timeout.
>>> -	 */
>>> -	/* case PragTyp_BUSY_TIMEOUT */
>>> 	default:{
>>> -			assert(pPragma->ePragTyp == PragTyp_BUSY_TIMEOUT);
>>> -			if (zRight) {
>>> -				sqlite3_busy_timeout(db, sqlite3Atoi(zRight));
>>> -			}
>>> -			returnSingleInt(v, db->busyTimeout);
>>> -			break;
>>> +		/* We shouldn't get here. */
>>> +		diag_set(ClientError, ER_UNKNOWN);
>>> +		pParse->rc = SQL_TARANTOOL_ERROR;
>>> +		pParse->nErr++;
>>> +		goto pragma_out;
>> If we really can’t reach here under no circumstances
>> (which seems to be true - pragmaLocate checks this) then
>> it’s better to place unreachable(); assert. Fix this please.
> Please find the fix below.

Ok, squash it, fix remove closes #… directive from first commit
message and then patch-set LGTM.

> 
>  src/box/sql/pragma.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 62ff1f2b2..1b456e0b5 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -643,13 +643,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
>          break;
>      }
> 
> -    default:{
> -        /* We shouldn't get here. */
> -        diag_set(ClientError, ER_UNKNOWN);
> -        pParse->rc = SQL_TARANTOOL_ERROR;
> -        pParse->nErr++;
> -        goto pragma_out;
> -        }
> +    default:
> +        unreachable();
>      }
> 
>      /* The following block is a no-op unless SQLITE_DEBUG is
> -- 
> 2.17.1
> 

[tarantool-patches] Re: [PATCH v4 1/2] sql: remove useless pragmas

[n.pettik]

When you send patch consisting from >= 2 commits,
you should organise it as patch-set with covering letter:
https://tarantool.io/en/doc/2.1/dev_guide/developer_guidelines/

Quote:

‘''
If there are multiple patches you want to submit in one go (e.g. this is a big feature which requires some preparatory patches to be committed first), you should send each patch in a separate email in reply to a cover letter. To format a patch series accordingly, pass the following options to git format-patch:
…
‘’'


> On 12 Feb 2019, at 10:31, Stanislav Zudin <szudin@tarantool.org> wrote:
> 
> The pragmas "query_only" and "read_uncommitted" didn't affect anything
> and were removed.
> Fixed an error in pragma index_list which caused a segmantation fault.
> pragma sql_default_engine accepts only strings.
> Thus pragma sql_default_engine('memtx') is a well-formed command,
> while pragma sql_default_engine(memtx) or
> pragma sql_default_engine("memtx") are considered as ill-formed and
> raise an error.
> 
> Closes #3733

You twice close issue - in both commits.
Please, do it only in the last one.

The rest in this patch is OK.

[tarantool-patches] Re: [PATCH v4 1/2] sql: remove useless pragmas

[Kirill Yukhin]

Hello,

On 12 Feb 10:31, Stanislav Zudin wrote:
> The pragmas "query_only" and "read_uncommitted" didn't affect anything
> and were removed.
> Fixed an error in pragma index_list which caused a segmantation fault.
> pragma sql_default_engine accepts only strings.
> Thus pragma sql_default_engine('memtx') is a well-formed command,
> while pragma sql_default_engine(memtx) or
> pragma sql_default_engine("memtx") are considered as ill-formed and
> raise an error.
> 
> Closes #3733
> ---
> Branch: https://github.com/tarantool/tarantool/tree/stanztt/gh-3733-obsolete-pragmas
> Issue: https://github.com/tarantool/tarantool/issues/3733

I've checked your patch into 2.1 branch.

---
Regards, Kirill Yukhin