* [tarantool-patches] [PATCH v4 1/2] sql: remove useless pragmas
@ 2019-02-12 7:31 Stanislav Zudin
2019-02-12 7:31 ` [tarantool-patches] [PATCH v4 2/2] sql: remove busy handler Stanislav Zudin
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Stanislav Zudin @ 2019-02-12 7:31 UTC (permalink / raw)
To: tarantool-patches, korablev; +Cc: 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tarantool-patches] [PATCH v4 2/2] sql: remove busy handler
2019-02-12 7:31 [tarantool-patches] [PATCH v4 1/2] sql: remove useless pragmas Stanislav Zudin
@ 2019-02-12 7:31 ` Stanislav Zudin
2019-02-12 17:05 ` [tarantool-patches] " n.pettik
2019-02-12 17:05 ` [tarantool-patches] Re: [PATCH v4 1/2] sql: remove useless pragmas n.pettik
2019-02-18 10:28 ` Kirill Yukhin
2 siblings, 1 reply; 7+ messages in thread
From: Stanislav Zudin @ 2019-02-12 7:31 UTC (permalink / raw)
To: tarantool-patches, korablev; +Cc: 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tarantool-patches] Re: [PATCH v4 1/2] sql: remove useless pragmas
2019-02-12 7:31 [tarantool-patches] [PATCH v4 1/2] sql: remove useless pragmas Stanislav Zudin
2019-02-12 7:31 ` [tarantool-patches] [PATCH v4 2/2] sql: remove busy handler Stanislav Zudin
@ 2019-02-12 17:05 ` n.pettik
2019-02-18 10:28 ` Kirill Yukhin
2 siblings, 0 replies; 7+ messages in thread
From: n.pettik @ 2019-02-12 17:05 UTC (permalink / raw)
To: tarantool-patches; +Cc: szudin
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tarantool-patches] Re: [PATCH v4 2/2] sql: remove busy handler
2019-02-12 7:31 ` [tarantool-patches] [PATCH v4 2/2] sql: remove busy handler Stanislav Zudin
@ 2019-02-12 17:05 ` n.pettik
2019-02-13 7:10 ` Stanislav Zudin
0 siblings, 1 reply; 7+ messages in thread
From: n.pettik @ 2019-02-12 17:05 UTC (permalink / raw)
To: tarantool-patches; +Cc: szudin
> - *
> - * 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tarantool-patches] Re: [PATCH v4 2/2] sql: remove busy handler
2019-02-12 17:05 ` [tarantool-patches] " n.pettik
@ 2019-02-13 7:10 ` Stanislav Zudin
2019-02-13 12:54 ` n.pettik
0 siblings, 1 reply; 7+ messages in thread
From: Stanislav Zudin @ 2019-02-13 7:10 UTC (permalink / raw)
To: tarantool-patches, n.pettik
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tarantool-patches] Re: [PATCH v4 2/2] sql: remove busy handler
2019-02-13 7:10 ` Stanislav Zudin
@ 2019-02-13 12:54 ` n.pettik
0 siblings, 0 replies; 7+ messages in thread
From: n.pettik @ 2019-02-13 12:54 UTC (permalink / raw)
To: tarantool-patches; +Cc: szudin
> 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
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tarantool-patches] Re: [PATCH v4 1/2] sql: remove useless pragmas
2019-02-12 7:31 [tarantool-patches] [PATCH v4 1/2] sql: remove useless pragmas Stanislav Zudin
2019-02-12 7:31 ` [tarantool-patches] [PATCH v4 2/2] sql: remove busy handler Stanislav Zudin
2019-02-12 17:05 ` [tarantool-patches] Re: [PATCH v4 1/2] sql: remove useless pragmas n.pettik
@ 2019-02-18 10:28 ` Kirill Yukhin
2 siblings, 0 replies; 7+ messages in thread
From: Kirill Yukhin @ 2019-02-18 10:28 UTC (permalink / raw)
To: tarantool-patches; +Cc: korablev, Stanislav Zudin
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-18 10:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-12 7:31 [tarantool-patches] [PATCH v4 1/2] sql: remove useless pragmas Stanislav Zudin
2019-02-12 7:31 ` [tarantool-patches] [PATCH v4 2/2] sql: remove busy handler Stanislav Zudin
2019-02-12 17:05 ` [tarantool-patches] " n.pettik
2019-02-13 7:10 ` Stanislav Zudin
2019-02-13 12:54 ` n.pettik
2019-02-12 17:05 ` [tarantool-patches] Re: [PATCH v4 1/2] sql: remove useless pragmas n.pettik
2019-02-18 10:28 ` Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox