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

[Stanislav Zudin <szudin@tarantool.org>]

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