[tarantool-patches] [PATCH v2] sql: remove useless pragmas

[tarantool-patches] [PATCH v2] 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.
pragmas index_list and index_info return an error when receive an 
invalid table or index names.
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 an 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                 |  65 +++++++++-
 src/box/sql/pragma.h                 |  42 +++---
 src/box/sql/sqliteInt.h              |   3 -
 test/sql-tap/gh-3733-pragma.test.lua | 184 +++++++++++++++++++++++++++
 4 files changed, 257 insertions(+), 37 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..158e6269b 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -342,15 +342,37 @@ sql_pragma_index_info(struct Parse *parse,
 		      MAYBE_UNUSED const PragmaName *pragma,
 		      const char *tbl_name, const char *idx_name)
 {
-	if (idx_name == NULL || tbl_name == NULL)
+	if (idx_name == NULL) {
+		diag_set(ClientError, ER_ILLEGAL_PARAMS,
+			 "index name was not specified");
+		parse->rc = SQL_TARANTOOL_ERROR;
+		parse->nErr++;
+		return;
+	}
+	if(tbl_name == NULL) {
+		diag_set(ClientError, ER_ILLEGAL_PARAMS,
+			 "table name was not specified");
+		parse->rc = SQL_TARANTOOL_ERROR;
+		parse->nErr++;
 		return;
+	}
+
 	struct space *space = space_by_name(tbl_name);
-	if (space == NULL)
+	if (space == NULL) {
+		diag_set(ClientError, ER_NO_SUCH_SPACE,
+			 tbl_name);
+		parse->rc = SQL_TARANTOOL_ERROR;
+		parse->nErr++;
 		return;
+	}
+
 	uint32_t iid = box_index_id_by_name(space->def->id, idx_name,
 					    strlen(idx_name));
-	if (iid == BOX_ID_NIL)
+	if (iid == BOX_ID_NIL) {
+		sqlite3ErrorMsg(parse, "no such index: %s", idx_name);
 		return;
+	}
+
 	struct index *idx = space_index(space, iid);
 	assert(idx != NULL);
 	parse->nMem = 6;
@@ -389,18 +411,39 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
 	if (tbl_name == NULL)
 		return;
 	struct space *space = space_by_name(tbl_name);
-	if (space == NULL)
+	if (space == NULL) {
+		diag_set(ClientError, ER_NO_SUCH_SPACE,
+			 tbl_name);
+		parse->rc = SQL_TARANTOOL_ERROR;
+		parse->nErr++;
 		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 pToken - 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 Token* pToken)
+{
+	if (!pToken || pToken->n == 0)
+		return true;
+	return  (pToken->n >= 2) &&
+		(pToken->z[0] == '\'') &&
+		(pToken->z[pToken->n - 1] == '\'');
+}
+
 /*
  * Process a pragma statement.
  *
@@ -601,6 +644,14 @@ 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..5b0dcd26a
--- /dev/null
+++ b/test/sql-tap/gh-3733-pragma.test.lua
@@ -0,0 +1,184 @@
+#!/usr/bin/env tarantool
+test = require("sqltester")
+
+test:plan(18)
+
+---
+--- 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 error for unknown table
+---
+test:do_catchsql_test(
+	"pragma-4.1",
+	[[
+        pragma index_list(fufel);
+    ]], {
+	1, "Space 'FUFEL' does not exist"
+})
+
+---
+--- pragma index_info returns an error for unknown index
+---
+test:do_execsql_test(
+	"pragma-5.1",
+	[[
+        pragma index_info(gh3733.IDX)
+    ]], {
+	-- <limit-1.2.1>
+	0, 0, 'ID', 0, 'BINARY', 'integer'
+	-- </limit-1.2.1>
+})
+
+test:do_catchsql_test(
+	"pragma-5.2",
+	[[
+        pragma index_info(no_table);
+    ]], {
+	1, "Illegal parameters, table name was not specified"
+})
+
+test:do_catchsql_test(
+	"pragma-5.3",
+	[[
+        pragma index_info(wrong_table.IDX);
+    ]], {
+	1, "Space 'WRONG_TABLE' does not exist"
+})
+
+test:do_catchsql_test(
+	"pragma-5.4",
+	[[
+        pragma index_info(gh3733.wrong_index);
+    ]], {
+	1, "no such index: WRONG_INDEX"
+})
+
+---
+--- 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>
+})
+
+---
+--- cleanup
+---
+test:do_execsql_test(
+	"pragma-8.1",
+	[[
+        DROP TABLE IF EXISTS gh3733;
+    ]], {
+
+})
+
+test:finish_test()
-- 
2.17.1

[tarantool-patches] Re: [PATCH v2] sql: remove useless pragmas

[n.pettik]

> On 8 Feb 2019, at 13:19, 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.
> pragmas index_list and index_info return an error when receive an 
> invalid table or index names.
> 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 an ill-formed and
> raise an error.
> 
> Closes #3733

Please, fix commit subject: now it is not only about removal of useless pragmas.

> ---
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index eef1ed931..158e6269b 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -342,15 +342,37 @@ sql_pragma_index_info(struct Parse *parse,
> 		      MAYBE_UNUSED const PragmaName *pragma,
> 		      const char *tbl_name, const char *idx_name)
> {
> -	if (idx_name == NULL || tbl_name == NULL)
> +	if (idx_name == NULL) {
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS,
> +			 "index name was not specified");
> +		parse->rc = SQL_TARANTOOL_ERROR;
> +		parse->nErr++;
> +		return;
> +	}

I’m sorry, but it seems that I interpreted comment from
P.Gulutzan in a wrong way. We shouldn’t throw any errors,
according to his clarification. Please, check out his comment
and make sure this time I am right. If really so, please discard
changes to sql_pragma_index_info().

> +	if(tbl_name == NULL) {
> +		diag_set(ClientError, ER_ILLEGAL_PARAMS,
> +			 "table name was not specified");
> +		parse->rc = SQL_TARANTOOL_ERROR;
> +		parse->nErr++;
> 		return;
> +	}
> +
> 	struct space *space = space_by_name(tbl_name);
> -	if (space == NULL)
> +	if (space == NULL) {
> +		diag_set(ClientError, ER_NO_SUCH_SPACE,
> +			 tbl_name);
> +		parse->rc = SQL_TARANTOOL_ERROR;
> +		parse->nErr++;
> 		return;
> +	}
> +
> 	uint32_t iid = box_index_id_by_name(space->def->id, idx_name,
> 					    strlen(idx_name));
> -	if (iid == BOX_ID_NIL)
> +	if (iid == BOX_ID_NIL) {
> +		sqlite3ErrorMsg(parse, "no such index: %s", idx_name);

Note that in this case you don’t need set sqlite error:
box_index_id_by_name will set diag message. So, you should
only set appropriate error code in parse struct.

> 		return;
> +	}
> +
> 	struct index *idx = space_index(space, iid);
> 	assert(idx != NULL);
> 	parse->nMem = 6;
> @@ -389,18 +411,39 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
> 	if (tbl_name == NULL)
> 		return;
> 	struct space *space = space_by_name(tbl_name);
> -	if (space == NULL)
> +	if (space == NULL) {
> +		diag_set(ClientError, ER_NO_SUCH_SPACE,
> +			 tbl_name);
> +		parse->rc = SQL_TARANTOOL_ERROR;
> +		parse->nErr++;

Seems that this change should be discarded as well.

> 		return;
> +/*
> + * @brief Check whether the specified token is a string or ID.
> + * @param pToken - 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 Token* pToken)
> +{
> +	if (!pToken || pToken->n == 0)
> +		return true;
> +	return  (pToken->n >= 2) &&
> +		(pToken->z[0] == '\'') &&
> +		(pToken->z[pToken->n - 1] == '\'');
> +}

I fixed codestyle violations:

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 158e6269b..3938ae429 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -428,20 +428,20 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
        }
 }
 
-/*
+/**
  * @brief Check whether the specified token is a string or ID.
- * @param pToken - token to be examined
+ * @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 Token* pToken)
+static bool
+token_is_string(const struct Token *token)
 {
-	if (!pToken || pToken->n == 0)
+	if (token == NULL || token->n == 0)
		return true;
-	return  (pToken->n >= 2) &&
-               (pToken->z[0] == '\'') &&
-               (pToken->z[pToken->n - 1] == '\'');
+	return token->n >= 2 && token->z[0] == '\'' &&
+	       token->z[token->n - 1] == '\'';
 }
 
 /*

> +
> /*
>  * Process a pragma statement.
>  *
> @@ -601,6 +644,14 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
> 		}
> 
> 	case PragTyp_DEFAULT_ENGINE: {
> +		if (!token_is_string(pValue))

I’d better use sql_token() function.

> +		{

Nit: put bracer at the previous line:

if (…) {

> +			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/test/sql-tap/gh-3733-pragma.test.lua b/test/sql-tap/gh-3733-pragma.test.lua
> new file mode 100755
> index 000000000..5b0dcd26a
> --- /dev/null
> +---
> +--- cleanup
> +—

In sql-tap/ tests we don’t need cleanup:
it is done automatically after execution. 

> +test:do_execsql_test(
> +	"pragma-8.1",
> +	[[
> +        DROP TABLE IF EXISTS gh3733;
> +    ]], {
> +
> +})
> +
> +test:finish_test()

[tarantool-patches] [PATCH v3] sql: remove useless pragmas and unused code

[Stanislav Zudin]

On 08.02.2019 22:12, n.pettik wrote:
>
>> On 8 Feb 2019, at 13:19, 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.
>> pragmas index_list and index_info return an error when receive an
>> invalid table or index names.
>> 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 an ill-formed and
>> raise an error.
>>
>> Closes #3733
> Please, fix commit subject: now it is not only about removal of useless pragmas.
Ok.
>
>> ---
>> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
>> index eef1ed931..158e6269b 100644
>> --- a/src/box/sql/pragma.c
>> +++ b/src/box/sql/pragma.c
>> @@ -342,15 +342,37 @@ sql_pragma_index_info(struct Parse *parse,
>> 		      MAYBE_UNUSED const PragmaName *pragma,
>> 		      const char *tbl_name, const char *idx_name)
>> {
>> -	if (idx_name == NULL || tbl_name == NULL)
>> +	if (idx_name == NULL) {
>> +		diag_set(ClientError, ER_ILLEGAL_PARAMS,
>> +			 "index name was not specified");
>> +		parse->rc = SQL_TARANTOOL_ERROR;
>> +		parse->nErr++;
>> +		return;
>> +	}
> I’m sorry, but it seems that I interpreted comment from
> P.Gulutzan in a wrong way. We shouldn’t throw any errors,
> according to his clarification. Please, check out his comment
> and make sure this time I am right. If really so, please discard
> changes to sql_pragma_index_info().
Done.
>> +	if(tbl_name == NULL) {
>> +		diag_set(ClientError, ER_ILLEGAL_PARAMS,
>> +			 "table name was not specified");
>> +		parse->rc = SQL_TARANTOOL_ERROR;
>> +		parse->nErr++;
>> 		return;
>> +	}
>> +
>> 	struct space *space = space_by_name(tbl_name);
>> -	if (space == NULL)
>> +	if (space == NULL) {
>> +		diag_set(ClientError, ER_NO_SUCH_SPACE,
>> +			 tbl_name);
>> +		parse->rc = SQL_TARANTOOL_ERROR;
>> +		parse->nErr++;
>> 		return;
>> +	}
>> +
>> 	uint32_t iid = box_index_id_by_name(space->def->id, idx_name,
>> 					    strlen(idx_name));
>> -	if (iid == BOX_ID_NIL)
>> +	if (iid == BOX_ID_NIL) {
>> +		sqlite3ErrorMsg(parse, "no such index: %s", idx_name);
> Note that in this case you don’t need set sqlite error:
> box_index_id_by_name will set diag message. So, you should
> only set appropriate error code in parse struct.
>> 		return;
>> +	}
>> +
>> 	struct index *idx = space_index(space, iid);
>> 	assert(idx != NULL);
>> 	parse->nMem = 6;
>> @@ -389,18 +411,39 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
>> 	if (tbl_name == NULL)
>> 		return;
>> 	struct space *space = space_by_name(tbl_name);
>> -	if (space == NULL)
>> +	if (space == NULL) {
>> +		diag_set(ClientError, ER_NO_SUCH_SPACE,
>> +			 tbl_name);
>> +		parse->rc = SQL_TARANTOOL_ERROR;
>> +		parse->nErr++;
> Seems that this change should be discarded as well.
Done
>
>> 		return;
>> +/*
>> + * @brief Check whether the specified token is a string or ID.
>> + * @param pToken - 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 Token* pToken)
>> +{
>> +	if (!pToken || pToken->n == 0)
>> +		return true;
>> +	return  (pToken->n >= 2) &&
>> +		(pToken->z[0] == '\'') &&
>> +		(pToken->z[pToken->n - 1] == '\'');
>> +}
> I fixed codestyle violations:
>
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 158e6269b..3938ae429 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -428,20 +428,20 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
>          }
>   }
>   
> -/*
> +/**
>    * @brief Check whether the specified token is a string or ID.
> - * @param pToken - token to be examined
> + * @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 Token* pToken)
> +static bool
> +token_is_string(const struct Token *token)
>   {
> -	if (!pToken || pToken->n == 0)
> +	if (token == NULL || token->n == 0)
> 		return true;
> -	return  (pToken->n >= 2) &&
> -               (pToken->z[0] == '\'') &&
> -               (pToken->z[pToken->n - 1] == '\'');
> +	return token->n >= 2 && token->z[0] == '\'' &&
> +	       token->z[token->n - 1] == '\'';
>   }
thx.
>   
>   /*
>
>> +
>> /*
>>   * Process a pragma statement.
>>   *
>> @@ -601,6 +644,14 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
>> 		}
>>
>> 	case PragTyp_DEFAULT_ENGINE: {
>> +		if (!token_is_string(pValue))
> I’d better use sql_token() function.
sql_token() examines every character in a string, while 
token_is_string() - only the leading and the trailing ones.
But if you insist i'll use sql_token.
>> +		{
> Nit: put bracer at the previous line:
>
> if (…) {
fixed.
>
>> +			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/test/sql-tap/gh-3733-pragma.test.lua b/test/sql-tap/gh-3733-pragma.test.lua
>> new file mode 100755
>> index 000000000..5b0dcd26a
>> --- /dev/null
>> +---
>> +--- cleanup
>> +—
> In sql-tap/ tests we don’t need cleanup:
> it is done automatically after execution.
>
Got it. Removed the final "drop table" statement.
>> +test:do_execsql_test(
>> +	"pragma-8.1",
>> +	[[
>> +        DROP TABLE IF EXISTS gh3733;
>> +    ]], {
>> +
>> +})
>> +
>> +test:finish_test()
The code changes are below.

Removed the last traces of SQLite's busyHandler.
Removed error handling, so the pragmas index_info & index_list return
an empty tuple in a case of 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                 | 50 ++++++----------------------
  src/box/sql/sqliteInt.h              | 17 ----------
  test/sql-tap/gh-3733-pragma.test.lua | 41 ++++++++++-------------
  3 files changed, 28 insertions(+), 80 deletions(-)

diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 7391f034b..62ff1f2b2 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -342,37 +342,15 @@ sql_pragma_index_info(struct Parse *parse,
                MAYBE_UNUSED const PragmaName *pragma,
                const char *tbl_name, const char *idx_name)
  {
-    if (idx_name == NULL) {
-        diag_set(ClientError, ER_ILLEGAL_PARAMS,
-             "index name was not specified");
-        parse->rc = SQL_TARANTOOL_ERROR;
-        parse->nErr++;
-        return;
-    }
-    if(tbl_name == NULL) {
-        diag_set(ClientError, ER_ILLEGAL_PARAMS,
-             "table name was not specified");
-        parse->rc = SQL_TARANTOOL_ERROR;
-        parse->nErr++;
+    if (idx_name == NULL || tbl_name == NULL)
          return;
-    }
-
      struct space *space = space_by_name(tbl_name);
-    if (space == NULL) {
-        diag_set(ClientError, ER_NO_SUCH_SPACE,
-             tbl_name);
-        parse->rc = SQL_TARANTOOL_ERROR;
-        parse->nErr++;
+    if (space == NULL)
          return;
-    }
-
      uint32_t iid = box_index_id_by_name(space->def->id, idx_name,
                          strlen(idx_name));
-    if (iid == BOX_ID_NIL) {
-        sqlite3ErrorMsg(parse, "no such index: %s", idx_name);
+    if (iid == BOX_ID_NIL)
          return;
-    }
-
      struct index *idx = space_index(space, iid);
      assert(idx != NULL);
      parse->nMem = 6;
@@ -411,13 +389,8 @@ sql_pragma_index_list(struct Parse *parse, const 
char *tbl_name)
      if (tbl_name == NULL)
          return;
      struct space *space = space_by_name(tbl_name);
-    if (space == NULL) {
-        diag_set(ClientError, ER_NO_SUCH_SPACE,
-             tbl_name);
-        parse->rc = SQL_TARANTOOL_ERROR;
-        parse->nErr++;
+    if (space == NULL)
          return;
-    }
      parse->nMem = 3;
      struct Vdbe *v = sqlite3GetVdbe(parse);
      for (uint32_t i = 0; i < space->index_count; ++i) {
@@ -430,18 +403,18 @@ sql_pragma_index_list(struct Parse *parse, const 
char *tbl_name)

  /*
   * @brief Check whether the specified token is a string or ID.
- * @param pToken - token to be examined
+ * @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 Token* pToken)
+static bool
+token_is_string(const struct Token* token)
  {
-    if (!pToken || pToken->n == 0)
+    if (!token || token->n == 0)
          return true;
-    return  (pToken->n >= 2) &&
-        (pToken->z[0] == '\'') &&
-        (pToken->z[pToken->n - 1] == '\'');
+    return token->n >= 2 && token->z[0] == '\'' &&
+           token->z[token->n - 1] == '\'';
  }

  /*
@@ -643,8 +616,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,    /* 
First part of [schema.]id field */
          }

      case PragTyp_DEFAULT_ENGINE: {
-        if (!token_is_string(pValue))
-        {
+        if (!token_is_string(pValue)) {
              diag_set(ClientError, ER_ILLEGAL_PARAMS,
                   "string value is expected");
              pParse->rc = SQL_TARANTOOL_ERROR;
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 50668b775..1633c1a35 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -824,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
@@ -1306,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.
diff --git a/test/sql-tap/gh-3733-pragma.test.lua 
b/test/sql-tap/gh-3733-pragma.test.lua
index 5b0dcd26a..216f0f8dd 100755
--- a/test/sql-tap/gh-3733-pragma.test.lua
+++ b/test/sql-tap/gh-3733-pragma.test.lua
@@ -1,7 +1,7 @@
  #!/usr/bin/env tarantool
  test = require("sqltester")

-test:plan(18)
+test:plan(17)

  ---
  --- Prerequisites
@@ -53,51 +53,55 @@ test:do_execsql_test(
  })

  ---
---- pragma index_list returns an error for unknown table
+--- pragma index_list returns an empty tuple for unknown table
  ---
-test:do_catchsql_test(
+test:do_execsql_test(
      "pragma-4.1",
      [[
          pragma index_list(fufel);
      ]], {
-    1, "Space 'FUFEL' does not exist"
+    -- <pragma-4.1>
+    -- </pragma-4.1>
  })

  ---
---- pragma index_info returns an error for unknown index
+--- pragma index_info returns an empty tuple for unknown index
  ---
  test:do_execsql_test(
      "pragma-5.1",
      [[
          pragma index_info(gh3733.IDX)
      ]], {
-    -- <limit-1.2.1>
+    -- <pragma-5.1>
      0, 0, 'ID', 0, 'BINARY', 'integer'
-    -- </limit-1.2.1>
+    -- </pragma-5.1>
  })

-test:do_catchsql_test(
+test:do_execsql_test(
      "pragma-5.2",
      [[
          pragma index_info(no_table);
      ]], {
-    1, "Illegal parameters, table name was not specified"
+    -- <pragma-5.2>
+    -- </pragma-5.2>
  })

-test:do_catchsql_test(
+test:do_execsql_test(
      "pragma-5.3",
      [[
          pragma index_info(wrong_table.IDX);
      ]], {
-    1, "Space 'WRONG_TABLE' does not exist"
+    -- <pragma-5.3>
+    -- </pragma-5.3>
  })

-test:do_catchsql_test(
+test:do_execsql_test(
      "pragma-5.4",
      [[
          pragma index_info(gh3733.wrong_index);
      ]], {
-    1, "no such index: WRONG_INDEX"
+    -- <pragma-5.4>
+    -- </pragma-5.4>
  })

  ---
@@ -170,15 +174,4 @@ test:do_execsql_test(
      -- </pragma-7.7>
  })

----
---- cleanup
----
-test:do_execsql_test(
-    "pragma-8.1",
-    [[
-        DROP TABLE IF EXISTS gh3733;
-    ]], {
-
-})
-
  test:finish_test()
-- 
2.17.1

[tarantool-patches] Re: [PATCH v3] sql: remove useless pragmas and unused code

[n.pettik]

> On 11 Feb 2019, at 16:17, Stanislav Zudin <szudin@tarantool.org> wrote:
> 
> On 08.02.2019 22:12, n.pettik wrote:
>> 
>>> On 8 Feb 2019, at 13:19, 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.
>>> pragmas index_list and index_info return an error when receive an
>>> invalid table or index names.
>>> 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 an ill-formed and
>>> raise an error.
>>> 
>>> Closes #3733
>> Please, fix commit subject: now it is not only about removal of useless pragmas.
> Ok.

Please, don’t put fixes to a HEAD~2 commit in a separate commit -
it is impossible to review. What you need to do is (interactive) rebase:
git rebase -i HEAD~2

Then pick commits to be edited and fix whatever you need.
After that, add changed files and continue rebasing with
git rebase —continue. However, in your case fixes are already
done, but they should be split into two commits. To process
this procedure, you should reset HEAD:

git reset HEAD~

This command will drop head commit and move all changed
made in that commit into unstaged area.
Then, by using interactive add command (git add -i) you
form two commits: one will contains fixes for the first commit
(where you remove all pragmas except for busy_handler),
another one - for the second commit.

Then, you will get this:

commit #4 Fixes for commit #2
commit #3 Fixes for commit #1
commit #2 sql: remove busy handler routine
commit #1 sql: remove useless pragmas

After that, you use git rebase:

git rebase -I HEAD~4

In rebase menu you can move commits. Lets make this:

commit #4 Fixes for commit #2
commit #3 sql: remove busy handler routine
commit #2 Fixes for commit #1
commit #1 sql: remove useless pragmas

Finally, you should squash commit #2 into commit #1
and commit #4 into commit #3. To do that in rebase
menu set fixup flag near commits #2 and #4.
That’s it. After you finish procedure, please push updated
patches and send them in whatever form you want.

>>> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
>>> index eef1ed931..158e6269b 100644
>>> --- a/src/box/sql/pragma.c
>>> +++ b/src/box/sql/pragma.c
>>> @@ -342,15 +342,37 @@ sql_pragma_index_info(struct Parse *parse,
>>> 		      MAYBE_UNUSED const PragmaName *pragma,
>>> 		      const char *tbl_name, const char *idx_name)
>>> {
>>> -	if (idx_name == NULL || tbl_name == NULL)
>>> +	if (idx_name == NULL) {
>>> +		diag_set(ClientError, ER_ILLEGAL_PARAMS,
>>> +			 "index name was not specified");
>>> +		parse->rc = SQL_TARANTOOL_ERROR;
>>> +		parse->nErr++;
>>> +		return;
>>> +	}
>> I’m sorry, but it seems that I interpreted comment from
>> P.Gulutzan in a wrong way. We shouldn’t throw any errors,
>> according to his clarification. Please, check out his comment
>> and make sure this time I am right. If really so, please discard
>> changes to sql_pragma_index_info().
> Done.
>>> +	if(tbl_name == NULL) {
>>> +		diag_set(ClientError, ER_ILLEGAL_PARAMS,
>>> +			 "table name was not specified");
>>> +		parse->rc = SQL_TARANTOOL_ERROR;
>>> +		parse->nErr++;
>>> 		return;
>>> +	}
>>> +
>>> 	struct space *space = space_by_name(tbl_name);
>>> -	if (space == NULL)
>>> +	if (space == NULL) {
>>> +		diag_set(ClientError, ER_NO_SUCH_SPACE,
>>> +			 tbl_name);
>>> +		parse->rc = SQL_TARANTOOL_ERROR;
>>> +		parse->nErr++;
>>> 		return;
>>> +	}
>>> +
>>> 	uint32_t iid = box_index_id_by_name(space->def->id, idx_name,
>>> 					    strlen(idx_name));
>>> -	if (iid == BOX_ID_NIL)
>>> +	if (iid == BOX_ID_NIL) {
>>> +		sqlite3ErrorMsg(parse, "no such index: %s", idx_name);
>> Note that in this case you don’t need set sqlite error:
>> box_index_id_by_name will set diag message. So, you should
>> only set appropriate error code in parse struct.
>>> 		return;
>>> +	}
>>> +
>>> 	struct index *idx = space_index(space, iid);
>>> 	assert(idx != NULL);
>>> 	parse->nMem = 6;
>>> @@ -389,18 +411,39 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
>>> 	if (tbl_name == NULL)
>>> 		return;
>>> 	struct space *space = space_by_name(tbl_name);
>>> -	if (space == NULL)
>>> +	if (space == NULL) {
>>> +		diag_set(ClientError, ER_NO_SUCH_SPACE,
>>> +			 tbl_name);
>>> +		parse->rc = SQL_TARANTOOL_ERROR;
>>> +		parse->nErr++;
>> Seems that this change should be discarded as well.
> Done
>> 
>>> 		return;
>>> +/*
>>> + * @brief Check whether the specified token is a string or ID.
>>> + * @param pToken - 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 Token* pToken)
>>> +{
>>> +	if (!pToken || pToken->n == 0)
>>> +		return true;
>>> +	return  (pToken->n >= 2) &&
>>> +		(pToken->z[0] == '\'') &&
>>> +		(pToken->z[pToken->n - 1] == '\'');
>>> +}
>> I fixed codestyle violations:
>> 
>> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
>> index 158e6269b..3938ae429 100644
>> --- a/src/box/sql/pragma.c
>> +++ b/src/box/sql/pragma.c
>> @@ -428,20 +428,20 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
>>         }
>>  }
>>  -/*
>> +/**
>>   * @brief Check whether the specified token is a string or ID.
>> - * @param pToken - token to be examined
>> + * @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 Token* pToken)
>> +static bool
>> +token_is_string(const struct Token *token)
>>  {
>> -	if (!pToken || pToken->n == 0)
>> +	if (token == NULL || token->n == 0)
>> 		return true;
>> -	return  (pToken->n >= 2) &&
>> -               (pToken->z[0] == '\'') &&
>> -               (pToken->z[pToken->n - 1] == '\'');
>> +	return token->n >= 2 && token->z[0] == '\'' &&
>> +	       token->z[token->n - 1] == '\'';
>>  }
> thx.
>>    /*
>> 
>>> +
>>> /*
>>>  * Process a pragma statement.
>>>  *
>>> @@ -601,6 +644,14 @@ sqlite3Pragma(Parse * pParse, Token * pId,	/* First part of [schema.]id field */
>>> 		}
>>> 
>>> 	case PragTyp_DEFAULT_ENGINE: {
>>> +		if (!token_is_string(pValue))
>> I’d better use sql_token() function.
> sql_token() examines every character in a string, while token_is_string() - only the leading and the trailing ones.
> But if you insist i'll use sql_token.
>>> +		{
>> Nit: put bracer at the previous line:
>> 
>> if (…) {
> fixed.
>> 
>>> +			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/test/sql-tap/gh-3733-pragma.test.lua b/test/sql-tap/gh-3733-pragma.test.lua
>>> new file mode 100755
>>> index 000000000..5b0dcd26a
>>> --- /dev/null
>>> +---
>>> +--- cleanup
>>> +—
>> In sql-tap/ tests we don’t need cleanup:
>> it is done automatically after execution.
>> 
> Got it. Removed the final "drop table" statement.
>>> +test:do_execsql_test(
>>> +	"pragma-8.1",
>>> +	[[
>>> +        DROP TABLE IF EXISTS gh3733;
>>> +    ]], {
>>> +
>>> +})
>>> +
>>> +test:finish_test()
> The code changes are below.
> 
> Removed the last traces of SQLite's busyHandler.
> Removed error handling, so the pragmas index_info & index_list return
> an empty tuple in a case of 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                 | 50 ++++++----------------------
>  src/box/sql/sqliteInt.h              | 17 ----------
>  test/sql-tap/gh-3733-pragma.test.lua | 41 ++++++++++-------------
>  3 files changed, 28 insertions(+), 80 deletions(-)
> 
> diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
> index 7391f034b..62ff1f2b2 100644
> --- a/src/box/sql/pragma.c
> +++ b/src/box/sql/pragma.c
> @@ -342,37 +342,15 @@ sql_pragma_index_info(struct Parse *parse,
>                MAYBE_UNUSED const PragmaName *pragma,
>                const char *tbl_name, const char *idx_name)
>  {
> -    if (idx_name == NULL) {
> -        diag_set(ClientError, ER_ILLEGAL_PARAMS,
> -             "index name was not specified");
> -        parse->rc = SQL_TARANTOOL_ERROR;
> -        parse->nErr++;
> -        return;
> -    }
> -    if(tbl_name == NULL) {
> -        diag_set(ClientError, ER_ILLEGAL_PARAMS,
> -             "table name was not specified");
> -        parse->rc = SQL_TARANTOOL_ERROR;
> -        parse->nErr++;
> +    if (idx_name == NULL || tbl_name == NULL)
>          return;
> -    }
> -
>      struct space *space = space_by_name(tbl_name);
> -    if (space == NULL) {
> -        diag_set(ClientError, ER_NO_SUCH_SPACE,
> -             tbl_name);
> -        parse->rc = SQL_TARANTOOL_ERROR;
> -        parse->nErr++;
> +    if (space == NULL)
>          return;
> -    }
> -
>      uint32_t iid = box_index_id_by_name(space->def->id, idx_name,
>                          strlen(idx_name));
> -    if (iid == BOX_ID_NIL) {
> -        sqlite3ErrorMsg(parse, "no such index: %s", idx_name);
> +    if (iid == BOX_ID_NIL)
>          return;
> -    }
> -
>      struct index *idx = space_index(space, iid);
>      assert(idx != NULL);
>      parse->nMem = 6;
> @@ -411,13 +389,8 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
>      if (tbl_name == NULL)
>          return;
>      struct space *space = space_by_name(tbl_name);
> -    if (space == NULL) {
> -        diag_set(ClientError, ER_NO_SUCH_SPACE,
> -             tbl_name);
> -        parse->rc = SQL_TARANTOOL_ERROR;
> -        parse->nErr++;
> +    if (space == NULL)
>          return;
> -    }
>      parse->nMem = 3;
>      struct Vdbe *v = sqlite3GetVdbe(parse);
>      for (uint32_t i = 0; i < space->index_count; ++i) {
> @@ -430,18 +403,18 @@ sql_pragma_index_list(struct Parse *parse, const char *tbl_name)
> 
>  /*
>   * @brief Check whether the specified token is a string or ID.
> - * @param pToken - token to be examined
> + * @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 Token* pToken)
> +static bool
> +token_is_string(const struct Token* token)
>  {
> -    if (!pToken || pToken->n == 0)
> +    if (!token || token->n == 0)
>          return true;
> -    return  (pToken->n >= 2) &&
> -        (pToken->z[0] == '\'') &&
> -        (pToken->z[pToken->n - 1] == '\'');
> +    return token->n >= 2 && token->z[0] == '\'' &&
> +           token->z[token->n - 1] == '\'';
>  }
> 
>  /*
> @@ -643,8 +616,7 @@ sqlite3Pragma(Parse * pParse, Token * pId,    /* First part of [schema.]id field */
>          }
> 
>      case PragTyp_DEFAULT_ENGINE: {
> -        if (!token_is_string(pValue))
> -        {
> +        if (!token_is_string(pValue)) {
>              diag_set(ClientError, ER_ILLEGAL_PARAMS,
>                   "string value is expected");
>              pParse->rc = SQL_TARANTOOL_ERROR;
> diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
> index 50668b775..1633c1a35 100644
> --- a/src/box/sql/sqliteInt.h
> +++ b/src/box/sql/sqliteInt.h
> @@ -824,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
> @@ -1306,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.
> diff --git a/test/sql-tap/gh-3733-pragma.test.lua b/test/sql-tap/gh-3733-pragma.test.lua
> index 5b0dcd26a..216f0f8dd 100755
> --- a/test/sql-tap/gh-3733-pragma.test.lua
> +++ b/test/sql-tap/gh-3733-pragma.test.lua
> @@ -1,7 +1,7 @@
>  #!/usr/bin/env tarantool
>  test = require("sqltester")
> 
> -test:plan(18)
> +test:plan(17)
> 
>  ---
>  --- Prerequisites
> @@ -53,51 +53,55 @@ test:do_execsql_test(
>  })
> 
>  ---
> ---- pragma index_list returns an error for unknown table
> +--- pragma index_list returns an empty tuple for unknown table
>  ---
> -test:do_catchsql_test(
> +test:do_execsql_test(
>      "pragma-4.1",
>      [[
>          pragma index_list(fufel);
>      ]], {
> -    1, "Space 'FUFEL' does not exist"
> +    -- <pragma-4.1>
> +    -- </pragma-4.1>
>  })
> 
>  ---
> ---- pragma index_info returns an error for unknown index
> +--- pragma index_info returns an empty tuple for unknown index
>  ---
>  test:do_execsql_test(
>      "pragma-5.1",
>      [[
>          pragma index_info(gh3733.IDX)
>      ]], {
> -    -- <limit-1.2.1>
> +    -- <pragma-5.1>
>      0, 0, 'ID', 0, 'BINARY', 'integer'
> -    -- </limit-1.2.1>
> +    -- </pragma-5.1>
>  })
> 
> -test:do_catchsql_test(
> +test:do_execsql_test(
>      "pragma-5.2",
>      [[
>          pragma index_info(no_table);
>      ]], {
> -    1, "Illegal parameters, table name was not specified"
> +    -- <pragma-5.2>
> +    -- </pragma-5.2>
>  })
> 
> -test:do_catchsql_test(
> +test:do_execsql_test(
>      "pragma-5.3",
>      [[
>          pragma index_info(wrong_table.IDX);
>      ]], {
> -    1, "Space 'WRONG_TABLE' does not exist"
> +    -- <pragma-5.3>
> +    -- </pragma-5.3>
>  })
> 
> -test:do_catchsql_test(
> +test:do_execsql_test(
>      "pragma-5.4",
>      [[
>          pragma index_info(gh3733.wrong_index);
>      ]], {
> -    1, "no such index: WRONG_INDEX"
> +    -- <pragma-5.4>
> +    -- </pragma-5.4>
>  })
> 
>  ---
> @@ -170,15 +174,4 @@ test:do_execsql_test(
>      -- </pragma-7.7>
>  })
> 
> ----
> ---- cleanup
> ----
> -test:do_execsql_test(
> -    "pragma-8.1",
> -    [[
> -        DROP TABLE IF EXISTS gh3733;
> -    ]], {
> -
> -})
> -
>  test:finish_test()
> -- 
> 2.17.1
> 
>