* [tarantool-patches] [PATCH 0/2] rework SQL 'DROP' routine
@ 2018-04-03 16:14 Nikita Pettik
2018-04-03 16:14 ` [tarantool-patches] [PATCH 1/2] sql: remove obsolete SQLite routine Nikita Pettik
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Nikita Pettik @ 2018-04-03 16:14 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik
Branch: https://github.com/tarantool/tarantool/tree/np/gh-3222-rework-drop
Issue: https://github.com/tarantool/tarantool/issues/3222
This small patch-set implements part of #3222 issue: rewrite functions
implementing drop table and drop index in order to avoid using
struct Table and its lookup in SQLite data-dictionary.
However, as far as currently triggers aren't part of Tarantool's spaces,
it is still required to use struct Table to drop them:
triggers are held in _trigger space, which format is: ['name', 'sql'],
i.e. there is no field for name of parent table. Hence, we are able
to point out table's triggers only via accessing struct Table.
Both patches are mainly about refactoring.
First patch removes useless wrappers, obsolete SQLite specific functions
and struct fields.
Second one replaces usages of struct Table with struct space in drop
routine, fixes codestyle, provides doxygen style comments.
Nikita Pettik (2):
sql: remove obsolete SQLite routine
sql: rework 'DROP INDEX' and 'DROP TABLE' handling
src/box/sql/alter.c | 13 +-
src/box/sql/analyze.c | 25 ++-
src/box/sql/build.c | 442 ++++++++++++++++++------------------------------
src/box/sql/delete.c | 5 +-
src/box/sql/expr.c | 9 +-
src/box/sql/fkey.c | 3 +-
src/box/sql/insert.c | 10 +-
src/box/sql/parse.c | 6 +-
src/box/sql/parse.y | 6 +-
src/box/sql/pragma.c | 18 +-
src/box/sql/prepare.c | 29 ----
src/box/sql/select.c | 4 +-
src/box/sql/sqliteInt.h | 17 +-
src/box/sql/trigger.c | 7 +-
src/box/sql/update.c | 2 +-
src/box/sql/vdbe.c | 1 -
src/box/sql/vdbeInt.h | 1 -
src/box/sql/vdbeaux.c | 1 -
src/box/sql/where.c | 2 -
src/box/sql/wherecode.c | 7 +-
20 files changed, 217 insertions(+), 391 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH 1/2] sql: remove obsolete SQLite routine
2018-04-03 16:14 [tarantool-patches] [PATCH 0/2] rework SQL 'DROP' routine Nikita Pettik
@ 2018-04-03 16:14 ` Nikita Pettik
2018-04-03 17:54 ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-03 16:14 ` [tarantool-patches] [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling Nikita Pettik
2018-04-05 14:12 ` [tarantool-patches] Re: [PATCH 0/2] rework SQL 'DROP' routine Kirill Yukhin
2 siblings, 1 reply; 12+ messages in thread
From: Nikita Pettik @ 2018-04-03 16:14 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik
Some of legacy functions seem to be useless, since they serve as
wrappers around others; the rest rely on capabilities which are no
longer relevant. This patch provides slight refactoring of such
functions.
Removed entities:
- sqlite3LocateTableItem() - replaced with sqlite3LocateTable();
- sqlite3FindTable() - replaced with sqlite3HashFind();
- sqlite3ColumnOfIndex() - in Tarantool order of columns always the same;
- sqlite3FindIndex() - replaced with sqlite3LocateIndex();
- sqlite3CodeVerifySchema();
- sqlite3SchemaToIndex();
- sqlite3MultiWrite();
- Parse->cookieMast;
- Vdbe->usesStmtJournal;
---
src/box/sql/alter.c | 13 ++-
src/box/sql/analyze.c | 25 +++---
src/box/sql/build.c | 213 ++++++++++--------------------------------------
src/box/sql/delete.c | 5 +-
src/box/sql/expr.c | 9 +-
src/box/sql/fkey.c | 3 +-
src/box/sql/insert.c | 10 +--
src/box/sql/pragma.c | 18 ++--
src/box/sql/prepare.c | 29 -------
src/box/sql/select.c | 4 +-
src/box/sql/sqliteInt.h | 11 +--
src/box/sql/trigger.c | 7 +-
src/box/sql/update.c | 2 +-
src/box/sql/vdbe.c | 1 -
src/box/sql/vdbeInt.h | 1 -
src/box/sql/vdbeaux.c | 1 -
src/box/sql/where.c | 2 -
src/box/sql/wherecode.c | 7 +-
18 files changed, 83 insertions(+), 278 deletions(-)
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index 054c0856c..a19324ed2 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -84,10 +84,9 @@ sqlite3AlterRenameTable(Parse * pParse, /* Parser context. */
goto exit_rename_table;
assert(pSrc->nSrc == 1);
- pTab = sqlite3LocateTableItem(pParse, 0, &pSrc->a[0]);
+ pTab = sqlite3LocateTable(pParse, 0, pSrc->a[0].zName);
if (!pTab)
goto exit_rename_table;
- assert(sqlite3SchemaToIndex(pParse->db, pTab->pSchema) == 0);
user_session->sql_flags |= SQLITE_PreferBuiltin;
@@ -99,7 +98,7 @@ sqlite3AlterRenameTable(Parse * pParse, /* Parser context. */
/* Check that a table named 'zName' does not already exist
* in database. If so, this is an error.
*/
- if (sqlite3FindTable(db, zName)) {
+ if (sqlite3HashFind(&db->pSchema->tblHash, zName) != NULL) {
sqlite3ErrorMsg(pParse,
"there is already another table or index with this name: %s",
zName);
@@ -122,7 +121,7 @@ sqlite3AlterRenameTable(Parse * pParse, /* Parser context. */
if (v == 0) {
goto exit_rename_table;
}
- sqlite3BeginWriteOperation(pParse, false);
+ sql_set_multi_write(pParse, false);
/* Drop and reload the internal table schema. */
reloadTableSchema(pParse, pTab, zName);
@@ -163,7 +162,7 @@ sqlite3AlterFinishAddColumn(Parse * pParse, Token * pColDef)
zTab = &pNew->zName[16]; /* Skip the "sqlite_altertab_" prefix on the name */
pCol = &pNew->aCol[pNew->nCol - 1];
pDflt = pCol->pDflt;
- pTab = sqlite3FindTable(db, zTab);
+ pTab = sqlite3HashFind(&db->pSchema->tblHash, zTab);;
assert(pTab);
/* If the default value for the new column was specified with a
@@ -257,7 +256,7 @@ sqlite3AlterBeginAddColumn(Parse * pParse, SrcList * pSrc)
assert(pParse->pNewTable == 0);
if (db->mallocFailed)
goto exit_begin_add_column;
- pTab = sqlite3LocateTableItem(pParse, 0, &pSrc->a[0]);
+ pTab = sqlite3LocateTable(pParse, 0, pSrc->a[0].zName);
if (!pTab)
goto exit_begin_add_column;
@@ -305,7 +304,7 @@ sqlite3AlterBeginAddColumn(Parse * pParse, SrcList * pSrc)
pNew->nTabRef = 1;
/* Begin a transaction and increment the schema cookie. */
- sqlite3BeginWriteOperation(pParse, 0);
+ sql_set_multi_write(pParse, 0);
v = sqlite3GetVdbe(pParse);
if (!v)
goto exit_begin_add_column;
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index fb46641a2..25e93aa15 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -156,7 +156,7 @@ openStatTable(Parse * pParse, /* Parsing context */
const char *zTab = aTable[i];
Table *pStat;
/* The table already exists, because it is a system space */
- pStat = sqlite3FindTable(db, zTab);
+ pStat = sqlite3HashFind(&db->pSchema->tblHash, zTab);
aRoot[i] = pStat->tnum;
aCreateTbl[i] = 0;
if (zWhere) {
@@ -831,7 +831,6 @@ analyzeOneTable(Parse * pParse, /* Parser context */
/* Do not gather statistics on system tables */
return;
}
- assert(sqlite3SchemaToIndex(db, pTab->pSchema) == 0);
/* Establish a read-lock on the table at the shared-cache level.
* Open a read-only cursor on the table. Also allocate a cursor number
@@ -907,7 +906,6 @@ analyzeOneTable(Parse * pParse, /* Parser context */
pParse->nMem = MAX(pParse->nMem, regPrev + nColTest);
/* Open a read-only cursor on the index being analyzed. */
- assert(sqlite3SchemaToIndex(db, pIdx->pSchema) == 0);
struct space *space =
space_by_id(SQLITE_PAGENO_TO_SPACEID(pIdx->tnum));
assert(space != NULL);
@@ -1028,7 +1026,7 @@ analyzeOneTable(Parse * pParse, /* Parser context */
int nPkColumn = (int)index_column_count(pPk);
regKeyStat = sqlite3GetTempRange(pParse, nPkColumn);
for (j = 0; j < nPkColumn; j++) {
- k = sqlite3ColumnOfIndex(pIdx, pPk->aiColumn[j]);
+ k = pPk->aiColumn[j];
assert(k >= 0 && k < pTab->nCol);
sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, k, regKeyStat + j);
VdbeComment((v, "%s", pTab->aCol[pPk->aiColumn[j]].zName));
@@ -1124,7 +1122,7 @@ analyzeDatabase(Parse * pParse)
int iMem;
int iTab;
- sqlite3BeginWriteOperation(pParse, 0);
+ sql_set_multi_write(pParse, 0);
iStatCur = pParse->nTab;
pParse->nTab += 3;
openStatTable(pParse, iStatCur, 0, 0);
@@ -1148,8 +1146,7 @@ analyzeTable(Parse * pParse, Table * pTab, Index * pOnlyIdx)
int iStatCur;
assert(pTab != 0);
- assert(sqlite3SchemaToIndex(pParse->db, pTab->pSchema) == 0);
- sqlite3BeginWriteOperation(pParse, 0);
+ sql_set_multi_write(pParse, 0);
iStatCur = pParse->nTab;
pParse->nTab += 3;
if (pOnlyIdx) {
@@ -1296,7 +1293,7 @@ analysisLoader(void *pData, int argc, char **argv, char **NotUsed)
if (argv == 0 || argv[0] == 0 || argv[2] == 0) {
return 0;
}
- pTable = sqlite3FindTable(pInfo->db, argv[0]);
+ pTable = sqlite3HashFind(&pInfo->db->pSchema->tblHash, argv[0]);
if (pTable == 0) {
return 0;
}
@@ -1305,7 +1302,7 @@ analysisLoader(void *pData, int argc, char **argv, char **NotUsed)
} else if (sqlite3_stricmp(argv[0], argv[1]) == 0) {
pIndex = sqlite3PrimaryKeyIndex(pTable);
} else {
- pIndex = sqlite3FindIndex(pInfo->db, argv[1], pTable);
+ pIndex = sqlite3HashFind(&pTable->idxHash, argv[1]);
}
z = argv[2];
@@ -1638,14 +1635,14 @@ loadStat4(sqlite3 * db)
Table *pTab = 0; /* Pointer to stat table */
assert(db->lookaside.bDisable);
- pTab = sqlite3FindTable(db, "_sql_stat4");
+ pTab = sqlite3HashFind(&db->pSchema->tblHash, "_sql_stat4");
if (pTab) {
rc = loadStatTbl(db,
pTab,
"SELECT \"tbl\",\"idx\",count(*) FROM \"_sql_stat4\" GROUP BY \"tbl\",\"idx\"",
"SELECT \"tbl\",\"idx\",\"neq\",\"nlt\",\"ndlt\",\"sample\" FROM \"_sql_stat4\"");
}
-
+
return rc;
}
@@ -1690,10 +1687,8 @@ sqlite3AnalysisLoad(sqlite3 * db)
/* Load new statistics out of the _sql_stat1 table */
sInfo.db = db;
- if (sqlite3FindTable(db, "_sql_stat1") != 0) {
- zSql = "SELECT \"tbl\",\"idx\",\"stat\" FROM \"_sql_stat1\"";
- rc = sqlite3_exec(db, zSql, analysisLoader, &sInfo, 0);
- }
+ zSql = "SELECT \"tbl\",\"idx\",\"stat\" FROM \"_sql_stat1\"";
+ rc = sqlite3_exec(db, zSql, analysisLoader, &sInfo, 0);
/* Set appropriate defaults on all indexes not in the _sql_stat1 table */
for (j = sqliteHashFirst(&db->pSchema->tblHash); j;
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 5e3ed0f39..61194e06b 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -93,23 +93,16 @@ sqlite3FinishCoding(Parse * pParse)
* transaction on each used database and to verify the schema cookie
* on each used database.
*/
- if (db->mallocFailed == 0
- && (DbMaskNonZero(pParse->cookieMask) || pParse->pConstExpr)
- ) {
+ if (db->mallocFailed == 0 || pParse->pConstExpr) {
int i;
assert(sqlite3VdbeGetOp(v, 0)->opcode == OP_Init);
sqlite3VdbeJumpHere(v, 0);
- if (DbMaskTest(pParse->cookieMask, 0) != 0) {
- if (pParse->initiateTTrans)
- sqlite3VdbeAddOp0(v, OP_TTransaction);
-
- if (db->init.busy == 0)
- sqlite3VdbeChangeP5(v, 1);
-
- VdbeComment((v, "usesStmtJournal=%d",
- pParse->mayAbort
- && pParse->isMultiWrite));
- }
+ if (pParse->initiateTTrans)
+ sqlite3VdbeAddOp0(v, OP_TTransaction);
+ if (db->init.busy == 0)
+ sqlite3VdbeChangeP5(v, 1);
+ VdbeComment((v, "Multiple changes: %d, may abort: %d",
+ pParse->isMultiWrite, pParse->mayAbort));
/* Code constant expressions that where factored out of inner loops */
if (pParse->pConstExpr) {
@@ -183,32 +176,10 @@ sqlite3NestedParse(Parse * pParse, const char *zFormat, ...)
pParse->nested--;
}
-/*
- * Locate the in-memory structure that describes a particular database
- * table given the name of that table and (optionally) the name of the
- * database containing the table. Return NULL if not found.
- *
- * If zDatabase is 0, all databases are searched for the table and the
- * first matching table is returned. (No checking for duplicate table
- * names is done.) The search order is TEMP first, then MAIN, then any
- * auxiliary databases added using the ATTACH command.
- *
- * See also sqlite3LocateTable().
- */
-Table *
-sqlite3FindTable(sqlite3 * db, const char *zName)
-{
- return sqlite3HashFind(&db->pSchema->tblHash, zName);
-}
-
/*
* Locate the in-memory structure that describes a particular database
* table given the name of that table. Return NULL if not found.
* Also leave an error message in pParse->zErrMsg.
- *
- * The difference between this routine and sqlite3FindTable() is that this
- * routine leaves an error message in pParse->zErrMsg where
- * sqlite3FindTable() does not.
*/
Table *
sqlite3LocateTable(Parse * pParse, /* context in which to report errors */
@@ -220,7 +191,7 @@ sqlite3LocateTable(Parse * pParse, /* context in which to report errors */
assert(pParse->db->pSchema != NULL);
- p = sqlite3FindTable(pParse->db, zName);
+ p = sqlite3HashFind(&pParse->db->pSchema->tblHash, zName);
if (p == 0) {
const char *zMsg =
flags & LOCATE_VIEW ? "no such view" : "no such table";
@@ -233,48 +204,19 @@ sqlite3LocateTable(Parse * pParse, /* context in which to report errors */
return p;
}
-/*
- * Locate the table identified by *p.
- *
- * This is a wrapper around sqlite3LocateTable(). The difference between
- * sqlite3LocateTable() and this function is that this function restricts
- * the search to schema (p->pSchema) if it is not NULL. p->pSchema may be
- * non-NULL if it is part of a view or trigger program definition. See
- * sqlite3FixSrcList() for details.
- */
-Table *
-sqlite3LocateTableItem(Parse * pParse, u32 flags, struct SrcList_item * p)
-{
- return sqlite3LocateTable(pParse, flags, p->zName);
-}
-
-/*
- * Locate the in-memory structure that describes
- * a particular index given the name of that index
- * and the name of the database that contains the index.
- * Return NULL if not found.
- */
-Index *
-sqlite3FindIndex(MAYBE_UNUSED sqlite3 * db, const char *zName, Table * pTab)
-{
- assert(pTab);
-
- return sqlite3HashFind(&pTab->idxHash, zName);
-}
-
Index *
sqlite3LocateIndex(sqlite3 * db, const char *zName, const char *zTable)
{
assert(zName);
assert(zTable);
- Table *pTab = sqlite3FindTable(db, zTable);
+ Table *pTab = sqlite3HashFind(&db->pSchema->tblHash, zTable);
if (pTab == 0) {
return 0;
}
- return sqlite3FindIndex(db, zName, pTab);
+ return sqlite3HashFind(&pTab->idxHash, zName);
}
/*
@@ -557,18 +499,6 @@ sqlite3PrimaryKeyIndex(Table * pTab)
return p;
}
-/*
- * Return the column of index pIdx that corresponds to table
- * column iCol. Return -1 if not found.
- */
-i16
-sqlite3ColumnOfIndex(Index * pIdx, i16 iCol)
-{
- /* TARANTOOL: Data layout is the same in every index. */
- (void) pIdx;
- return iCol;
-}
-
/*
* Begin constructing a new table representation in memory. This is
* the first of several action routines that get called in response
@@ -614,7 +544,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
return;
assert(db->pSchema != NULL);
- pTable = sqlite3FindTable(db, zName);
+ pTable = sqlite3HashFind(&db->pSchema->tblHash, zName);
if (pTable) {
if (!noErr) {
sqlite3ErrorMsg(pParse,
@@ -622,7 +552,6 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
zName);
} else {
assert(!db->init.busy || CORRUPT_DB);
- sqlite3CodeVerifySchema(pParse);
}
goto begin_table_error;
}
@@ -654,7 +583,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
* now.
*/
if (!db->init.busy && (v = sqlite3GetVdbe(pParse)) != 0)
- sqlite3BeginWriteOperation(pParse, 1);
+ sql_set_multi_write(pParse, 1);
/* Normal (non-error) return. */
return;
@@ -2039,7 +1968,6 @@ sqlite3CreateView(Parse * pParse, /* The parsing context */
p = pParse->pNewTable;
if (p == 0 || pParse->nErr)
goto create_view_fail;
- sqlite3SchemaToIndex(db, p->pSchema);
sqlite3FixInit(&sFix, pParse, "view", pName);
if (sqlite3FixSelect(&sFix, pSelect))
goto create_view_fail;
@@ -2206,26 +2134,22 @@ sqliteViewResetAll(sqlite3 * db)
#define sqliteViewResetAll(A,B)
#endif /* SQLITE_OMIT_VIEW */
-/*
- * Remove entries from the sqlite_statN tables (for N in (1,2,3))
- * after a DROP INDEX or DROP TABLE command.
+/**
+ * Remove entries from the _sql_stat1 and _sql_stat4
+ * system spaces after a DROP INDEX or DROP TABLE command.
+ *
+ * @param pParse Parsing context.
+ * @param zType Type of entry to be deleted:
+ * 'idx' or 'tbl' string literal.
+ * @param zName Name of index or table.
*/
static void
-sqlite3ClearStatTables(Parse * pParse, /* The parsing context */
- const char *zType, /* "idx" or "tbl" */
- const char *zName /* Name of index or table */
- )
+sql_clear_stat_spaces(Parse * pParse, const char *zType, const char *zName)
{
- int i;
- for (i = 1; i <= 4; i++) {
- char zTab[24];
- sqlite3_snprintf(sizeof(zTab), zTab, "_sql_stat%d", i);
- if (sqlite3FindTable(pParse->db, zTab)) {
- sqlite3NestedParse(pParse,
- "DELETE FROM \"%s\" WHERE \"%s\"=%Q",
- zTab, zType, zName);
- }
- }
+ sqlite3NestedParse(pParse, "DELETE FROM \"_sql_stat1\" WHERE \"%s\"=%Q",
+ zType, zName);
+ sqlite3NestedParse(pParse, "DELETE FROM \"_sql_stat4\" WHERE \"%s\"=%Q",
+ zType, zName);
}
/*
@@ -2240,7 +2164,6 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
v = sqlite3GetVdbe(pParse);
assert(v != 0);
- sqlite3BeginWriteOperation(pParse, 1);
/*
* Drop all triggers associated with the table being
* dropped. Code is generated to remove entries from
@@ -2376,15 +2299,12 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
if (noErr)
db->suppressErr++;
assert(isView == 0 || isView == LOCATE_VIEW);
- pTab = sqlite3LocateTableItem(pParse, isView, &pName->a[0]);
+ pTab = sqlite3LocateTable(pParse, isView, pName->a[0].zName);
if (noErr)
db->suppressErr--;
- if (pTab == 0) {
- if (noErr)
- sqlite3CodeVerifySchema(pParse);
+ if (pTab == 0)
goto exit_drop_table;
- }
#ifndef SQLITE_OMIT_VIEW
/* Ensure DROP TABLE is not used on a view, and DROP VIEW is not used
* on a table.
@@ -2414,8 +2334,8 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
* space_id from _space.
*/
- sqlite3BeginWriteOperation(pParse, 1);
- sqlite3ClearStatTables(pParse, "tbl", pTab->zName);
+ sql_set_multi_write(pParse, 1);
+ sql_clear_stat_spaces(pParse, "tbl", pTab->zName);
sqlite3FkDropTable(pParse, pName, pTab);
sqlite3CodeDropTable(pParse, pTab, isView);
@@ -2881,7 +2801,7 @@ sqlite3CreateIndex(Parse * pParse, /* All information about this parse */
*/
assert(0);
}
- pTab = sqlite3LocateTableItem(pParse, 0, &pTblName->a[0]);
+ pTab = sqlite3LocateTable(pParse, 0, pTblName->a[0].zName);
assert(db->mallocFailed == 0 || pTab == 0);
if (pTab == 0)
goto exit_create_index;
@@ -2921,21 +2841,20 @@ sqlite3CreateIndex(Parse * pParse, /* All information about this parse */
goto exit_create_index;
assert(pName->z != 0);
if (!db->init.busy) {
- if (sqlite3FindTable(db, zName) != 0) {
+ if (sqlite3HashFind(&db->pSchema->tblHash, zName) != 0) {
sqlite3ErrorMsg(pParse,
"there is already a table named %s",
zName);
goto exit_create_index;
}
}
- if (sqlite3FindIndex(db, zName, pTab) != 0) {
+ if (sqlite3HashFind(&pTab->idxHash, zName) != 0) {
if (!ifNotExist) {
sqlite3ErrorMsg(pParse,
"index %s.%s already exists",
pTab->zName, zName);
} else {
assert(!db->init.busy);
- sqlite3CodeVerifySchema(pParse);
}
goto exit_create_index;
}
@@ -3080,9 +2999,6 @@ sqlite3CreateIndex(Parse * pParse, /* All information about this parse */
if (pParse->pNewTable == 0)
estimateIndexWidth(pIndex);
- assert(pTab->iPKey < 0
- || sqlite3ColumnOfIndex(pIndex, pTab->iPKey) >= 0);
-
if (pTab == pParse->pNewTable) {
/* This routine has been called to create an automatic index as a
* result of a PRIMARY KEY or UNIQUE clause on a column definition, or
@@ -3191,7 +3107,7 @@ sqlite3CreateIndex(Parse * pParse, /* All information about this parse */
if (v == 0)
goto exit_create_index;
- sqlite3BeginWriteOperation(pParse, 1);
+ sql_set_multi_write(pParse, 1);
sqlite3VdbeAddOp2(v, OP_SIDtoPtr, BOX_INDEX_ID,
@@ -3387,8 +3303,6 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)
if (!ifExists) {
sqlite3ErrorMsg(pParse, "no such index: %s.%S",
zTableName, pName, 0);
- } else {
- sqlite3CodeVerifySchema(pParse);
}
pParse->checkSchema = 1;
goto exit_drop_index;
@@ -3417,7 +3331,7 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)
* But firstly, delete statistics since schema
* changes after DDL.
*/
- sqlite3ClearStatTables(pParse, "idx", pIndex->zName);
+ sql_clear_stat_spaces(pParse, "idx", pIndex->zName);
int record_reg = ++pParse->nMem;
int space_id_reg = ++pParse->nMem;
sqlite3VdbeAddOp2(v, OP_Integer, SQLITE_PAGENO_TO_SPACEID(pIndex->tnum),
@@ -3934,58 +3848,18 @@ sqlite3Savepoint(Parse * pParse, int op, Token * pName)
}
}
-/*
- * Record the fact that the schema cookie will need to be verified
- * for database. The code to actually verify the schema cookie
- * will occur at the end of the top-level VDBE and will be generated
- * later, by sqlite3FinishCoding().
- */
-void
-sqlite3CodeVerifySchema(Parse * pParse)
-{
- Parse *pToplevel = sqlite3ParseToplevel(pParse);
-
- if (DbMaskTest(pToplevel->cookieMask, 0) == 0) {
- DbMaskSet(pToplevel->cookieMask, 0);
- }
-}
-
-/*
- * Generate VDBE code that prepares for doing an operation that
- * might change the database.
- *
- * This routine starts a new transaction if we are not already within
- * a transaction. If we are already within a transaction, then a checkpoint
- * is set if the setStatement parameter is true. A checkpoint should
- * be set for operations that might fail (due to a constraint) part of
- * the way through and which will need to undo some writes without having to
- * rollback the whole transaction. For operations where all constraints
- * can be checked before any changes are made to the database, it is never
- * necessary to undo a write and the checkpoint should not be set.
+/**
+ * Set flag in parse context, which indicates that during query
+ * execution multiple insertion/updates may occur.
*/
void
-sqlite3BeginWriteOperation(Parse * pParse, int setStatement)
+sql_set_multi_write(Parse *pParse, int setStatement)
{
Parse *pToplevel = sqlite3ParseToplevel(pParse);
- sqlite3CodeVerifySchema(pParse);
DbMaskSet(pToplevel->writeMask, 0);
pToplevel->isMultiWrite |= setStatement;
}
-/*
- * Indicate that the statement currently under construction might write
- * more than one entry (example: deleting one row then inserting another,
- * inserting multiple rows in a table, or inserting a row and index entries.)
- * If an abort occurs after some of these writes have completed, then it will
- * be necessary to undo the completed writes.
- */
-void
-sqlite3MultiWrite(Parse * pParse)
-{
- Parse *pToplevel = sqlite3ParseToplevel(pParse);
- pToplevel->isMultiWrite = 1;
-}
-
/*
* The code generator calls this routine if is discovers that it is
* possible to abort a statement prior to completion. In order to
@@ -4100,7 +3974,7 @@ reindexTable(Parse * pParse, Table * pTab, char const *zColl)
for (pIndex = pTab->pIndex; pIndex; pIndex = pIndex->pNext) {
if (zColl == 0 || collationMatch(zColl, pIndex)) {
- sqlite3BeginWriteOperation(pParse, 0);
+ sql_set_multi_write(pParse, 0);
sqlite3RefillIndex(pParse, pIndex, -1);
}
}
@@ -4175,7 +4049,7 @@ sqlite3Reindex(Parse * pParse, Token * pName1, Token * pName2)
z = sqlite3NameFromToken(db, pName1);
if (z == 0)
return;
- pTab = sqlite3FindTable(db, z);
+ pTab = sqlite3HashFind(&db->pSchema->tblHash, z);
if (pTab) {
reindexTable(pParse, pTab, 0);
sqlite3DbFree(db, z);
@@ -4185,16 +4059,15 @@ sqlite3Reindex(Parse * pParse, Token * pName1, Token * pName2)
zTable = sqlite3NameFromToken(db, pName2);
}
- pTab = sqlite3FindTable(db, zTable);
+ pTab = sqlite3HashFind(&db->pSchema->tblHash, zTable);
if (pTab == 0) {
sqlite3ErrorMsg(pParse, "no such table: %s", zTable);
goto exit_reindex;
}
- pIndex = sqlite3FindIndex(db, z, pTab);
-
+ pIndex = sqlite3HashFind(&pTab->idxHash, z);
if (pIndex) {
- sqlite3BeginWriteOperation(pParse, 0);
+ sql_set_multi_write(pParse, 0);
sqlite3RefillIndex(pParse, pIndex, -1);
return;
}
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index 7d1d71577..a07ef1980 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -56,7 +56,7 @@ sqlite3SrcListLookup(Parse * pParse, SrcList * pSrc)
struct SrcList_item *pItem = pSrc->a;
Table *pTab;
assert(pItem && pSrc->nSrc == 1);
- pTab = sqlite3LocateTableItem(pParse, 0, pItem);
+ pTab = sqlite3LocateTable(pParse, 0, pItem->zName);
sqlite3DeleteTable(pParse->db, pItem->pTab);
pItem->pTab = pTab;
if (pTab) {
@@ -113,7 +113,6 @@ sqlite3MaterializeView(Parse * pParse, /* Parsing context */
Select *pSel;
SrcList *pFrom;
sqlite3 *db = pParse->db;
- assert(sqlite3SchemaToIndex(db, pView->pSchema) == 0);
pWhere = sqlite3ExprDup(db, pWhere, 0);
pFrom = sqlite3SrcListAppend(db, 0, 0);
if (pFrom) {
@@ -325,7 +324,7 @@ sqlite3DeleteFrom(Parse * pParse, /* The parser context */
}
if (pParse->nested == 0)
sqlite3VdbeCountChanges(v);
- sqlite3BeginWriteOperation(pParse, 1);
+ sql_set_multi_write(pParse, 1);
/* If we are trying to delete from a view, realize that view into
* an ephemeral table.
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index f4e3cf735..d981790e6 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2455,9 +2455,6 @@ sqlite3FindInIndex(Parse * pParse, /* Parsing context */
assert(p->pEList->a[0].pExpr != 0); /* Because of isCandidateForInOpt(p) */
assert(p->pSrc != 0); /* Because of isCandidateForInOpt(p) */
pTab = p->pSrc->a[0].pTab;
-
- sqlite3CodeVerifySchema(pParse);
-
assert(v); /* sqlite3GetVdbe() has always been previously called */
Index *pIdx; /* Iterator variable */
@@ -3585,8 +3582,7 @@ sqlite3ExprCodeGetColumnOfTable(Vdbe * v, /* The VDBE under construction */
int regOut /* Extract the value into this register */
)
{
- int x = sqlite3ColumnOfIndex(sqlite3PrimaryKeyIndex(pTab), iCol);
- sqlite3VdbeAddOp3(v, OP_Column, iTabCur, x, regOut);
+ sqlite3VdbeAddOp3(v, OP_Column, iTabCur, iCol, regOut);
if (iCol >= 0) {
sqlite3ColumnDefault(v, pTab, iCol, regOut);
}
@@ -5315,8 +5311,7 @@ exprIdxCover(Walker * pWalker, Expr * pExpr)
{
if (pExpr->op == TK_COLUMN
&& pExpr->iTable == pWalker->u.pIdxCover->iCur
- && sqlite3ColumnOfIndex(pWalker->u.pIdxCover->pIdx,
- pExpr->iColumn) < 0) {
+ && pExpr->iColumn < 0) {
pWalker->eCode = 1;
return WRC_Abort;
}
diff --git a/src/box/sql/fkey.c b/src/box/sql/fkey.c
index f4a6b2707..f56b6d9cd 100644
--- a/src/box/sql/fkey.c
+++ b/src/box/sql/fkey.c
@@ -961,7 +961,8 @@ sqlite3FkCheck(Parse * pParse, /* Parse context */
* early.
*/
if (pParse->disableTriggers) {
- pTo = sqlite3FindTable(db, pFKey->zTo);
+ pTo = sqlite3HashFind(&db->pSchema->tblHash,
+ pFKey->zTo);
} else {
pTo = sqlite3LocateTable(pParse, 0, pFKey->zTo);
}
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 4a57b23f5..2719bbaf4 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -422,7 +422,7 @@ sqlite3Insert(Parse * pParse, /* Parser context */
goto insert_cleanup;
if (pParse->nested == 0)
sqlite3VdbeCountChanges(v);
- sqlite3BeginWriteOperation(pParse, pSelect || pTrigger);
+ sql_set_multi_write(pParse, pSelect || pTrigger);
#ifndef SQLITE_OMIT_XFER_OPT
/* If the statement is of the form
@@ -1398,8 +1398,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
if (pIdx != pPk) {
for (i = 0; i < nPkCol; i++) {
assert(pPk->aiColumn[i] >= 0);
- x = sqlite3ColumnOfIndex(pIdx,
- pPk->aiColumn[i]);
+ x = pPk->aiColumn[i];
sqlite3VdbeAddOp3(v, OP_Column,
iThisCur, x, regR + i);
VdbeComment((v, "%s.%s", pTab->zName,
@@ -1462,7 +1461,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
default: {
Trigger *pTrigger = 0;
assert(onError == ON_CONFLICT_ACTION_REPLACE);
- sqlite3MultiWrite(pParse);
+ sql_set_multi_write(pParse, 1);
if (user_session->
sql_flags & SQLITE_RecTriggers) {
pTrigger =
@@ -1833,7 +1832,7 @@ xferOptimization(Parse * pParse, /* Parser context */
* we have to check the semantics.
*/
pItem = pSelect->pSrc->a;
- pSrc = sqlite3LocateTableItem(pParse, 0, pItem);
+ pSrc = sqlite3LocateTable(pParse, 0, pItem->zName);
if (pSrc == 0) {
return 0; /* FROM clause does not contain a real table */
}
@@ -1919,7 +1918,6 @@ xferOptimization(Parse * pParse, /* Parser context */
sqlite3_xferopt_count++;
#endif
v = sqlite3GetVdbe(pParse);
- sqlite3CodeVerifySchema(pParse);
iSrc = pParse->nTab++;
iDest = pParse->nTab++;
regData = sqlite3GetTempReg(pParse);
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 23b4c73b5..04b4020dd 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -358,7 +358,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Column *pCol;
Index *pPk = sqlite3PrimaryKeyIndex(pTab);
pParse->nMem = 6;
- sqlite3CodeVerifySchema(pParse);
sqlite3ViewGetColumnNames(pParse, pTab);
for (i = 0, pCol = pTab->aCol; i < pTab->nCol;
i++, pCol++) {
@@ -397,7 +396,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Index *pIdx;
HashElem *i;
pParse->nMem = 4;
- sqlite3CodeVerifySchema(pParse);
for (i = sqliteHashFirst(&db->pSchema->tblHash); i;
i = sqliteHashNext(i)) {
Table *pTab = sqliteHashData(i);
@@ -441,7 +439,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
pParse->nMem = 3;
}
mx = index_column_count(pIdx);
- sqlite3CodeVerifySchema(pParse);
assert(pParse->nMem <=
pPragma->nPragCName);
for (i = 0; i < mx; i++) {
@@ -481,10 +478,10 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Index *pIdx;
Table *pTab;
int i;
- pTab = sqlite3FindTable(db, zRight);
+ pTab = sqlite3HashFind(&db->pSchema->tblHash,
+ zRight);
if (pTab) {
pParse->nMem = 5;
- sqlite3CodeVerifySchema(pParse);
for (pIdx = pTab->pIndex, i = 0; pIdx;
pIdx = pIdx->pNext, i++) {
const char *azOrigin[] =
@@ -541,13 +538,13 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
if (zRight) {
FKey *pFK;
Table *pTab;
- pTab = sqlite3FindTable(db, zRight);
+ pTab = sqlite3HashFind(&db->pSchema->tblHash,
+ zRight);
if (pTab) {
pFK = pTab->pFKey;
if (pFK) {
int i = 0;
pParse->nMem = 8;
- sqlite3CodeVerifySchema(pParse);
while (pFK) {
int j;
for (j = 0;
@@ -597,7 +594,6 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
pParse->nMem += 4;
regKey = ++pParse->nMem;
regRow = ++pParse->nMem;
- sqlite3CodeVerifySchema(pParse);
k = sqliteHashFirst(&db->pSchema->tblHash);
while (k) {
if (zRight) {
@@ -619,7 +615,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
for (i = 1, pFK = pTab->pFKey; pFK;
i++, pFK = pFK->pNextFrom) {
pParent =
- sqlite3FindTable(db, pFK->zTo);
+ sqlite3HashFind(&db->pSchema->tblHash,
+ pFK->zTo);
if (pParent == 0)
continue;
pIdx = 0;
@@ -657,7 +654,8 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
for (i = 1, pFK = pTab->pFKey; pFK;
i++, pFK = pFK->pNextFrom) {
pParent =
- sqlite3FindTable(db, pFK->zTo);
+ sqlite3HashFind(&db->pSchema->tblHash,
+ pFK->zTo);
pIdx = 0;
aiCols = 0;
if (pParent) {
diff --git a/src/box/sql/prepare.c b/src/box/sql/prepare.c
index 0ea181d44..7a16074dc 100644
--- a/src/box/sql/prepare.c
+++ b/src/box/sql/prepare.c
@@ -204,35 +204,6 @@ sqlite3InitDatabase(sqlite3 * db)
return rc;
}
-/*
- * Convert a schema pointer into the 0 index that indicates
- * that schema refers to a single database.
- * This method is inherited from SQLite, which has several dbs.
- * But we have only one, so it is used only in assertions.
- */
-int
-sqlite3SchemaToIndex(sqlite3 * db, Schema * pSchema)
-{
- int i = -1000000;
-
- /* If pSchema is NULL, then return -1000000. This happens when code in
- * expr.c is trying to resolve a reference to a transient table (i.e. one
- * created by a sub-select). In this case the return value of this
- * function should never be used.
- *
- * We return -1000000 instead of the more usual -1 simply because using
- * -1000000 as the incorrect index into db->aDb[] is much
- * more likely to cause a segfault than -1 (of course there are assert()
- * statements too, but it never hurts to play the odds).
- */
- if (pSchema) {
- if (db->pSchema == pSchema) {
- i = 0;
- }
- assert(i == 0);
- }
- return i;
-}
/*
* Free all memory allocations in the pParse object
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index c7f87f340..6ff8dc25e 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -4745,7 +4745,7 @@ selectExpander(Walker * pWalker, Select * p)
/* An ordinary table or view name in the FROM clause */
assert(pFrom->pTab == 0);
pFrom->pTab = pTab =
- sqlite3LocateTableItem(pParse, 0, pFrom);
+ sqlite3LocateTable(pParse, 0, pFrom->zName);
if (pTab == 0)
return WRC_Abort;
if (pTab->nTabRef >= 0xffff) {
@@ -6155,8 +6155,6 @@ sqlite3Select(Parse * pParse, /* The parser context */
Index *pBest; /* Best index found so far */
int iRoot = pTab->tnum; /* Root page of scanned b-tree */
- sqlite3CodeVerifySchema(pParse);
-
/* Search for the index that has the lowest scan cost.
*
* (2011-04-15) Do not do a full scan of an unordered index.
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index f43711a7f..fcafb65b4 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2947,7 +2947,6 @@ struct Parse {
ExprList *pConstExpr; /* Constant expressions */
Token constraintName; /* Name of the constraint currently being parsed */
yDbMask writeMask; /* Start a write transaction on these databases */
- yDbMask cookieMask; /* Bitmask of schema verified databases */
int regRoot; /* Register holding root page number for new objects */
int nMaxArg; /* Max args passed to user function by sub-program */
#ifdef SELECTTRACE_ENABLED
@@ -3550,7 +3549,6 @@ int sqlite3ColumnsFromExprList(Parse *, ExprList *, i16 *, Column **);
void sqlite3SelectAddColumnTypeAndCollation(Parse *, Table *, Select *);
Table *sqlite3ResultSetOfSelect(Parse *, Select *);
Index *sqlite3PrimaryKeyIndex(Table *);
-i16 sqlite3ColumnOfIndex(Index *, i16);
void sqlite3StartTable(Parse *, Token *, int);
void sqlite3AddColumn(Parse *, Token *, Token *);
void sqlite3AddNotNull(Parse *, int);
@@ -3669,12 +3667,9 @@ int sqlite3ExprCodeExprList(Parse *, ExprList *, int, int, u8);
void sqlite3ExprIfTrue(Parse *, Expr *, int, int);
void sqlite3ExprIfFalse(Parse *, Expr *, int, int);
void sqlite3ExprIfFalseDup(Parse *, Expr *, int, int);
-Table *sqlite3FindTable(sqlite3 *, const char *);
#define LOCATE_VIEW 0x01
#define LOCATE_NOERR 0x02
Table *sqlite3LocateTable(Parse *, u32 flags, const char *);
-Table *sqlite3LocateTableItem(Parse *, u32 flags, struct SrcList_item *);
-Index *sqlite3FindIndex(sqlite3 *, const char *, Table *);
Index *sqlite3LocateIndex(sqlite3 *, const char *, const char *);
void sqlite3UnlinkAndDeleteTable(sqlite3 *, const char *);
void sqlite3UnlinkAndDeleteIndex(sqlite3 *, Index *);
@@ -3692,7 +3687,6 @@ void sqlite3PrngSaveState(void);
void sqlite3PrngRestoreState(void);
#endif
void sqlite3RollbackAll(Vdbe *, int);
-void sqlite3CodeVerifySchema(Parse *);
void sqlite3BeginTransaction(Parse *, int);
void sqlite3CommitTransaction(Parse *);
void sqlite3RollbackTransaction(Parse *);
@@ -3718,7 +3712,7 @@ void sqlite3GenerateConstraintChecks(Parse *, Table *, int *, int, int, int,
void sqlite3CompleteInsertion(Parse *, Table *, int, int *, int, u8);
int sqlite3OpenTableAndIndices(Parse *, Table *, int, u8, int, u8 *, int *,
int *, u8, u8);
-void sqlite3BeginWriteOperation(Parse *, int);
+void sql_set_multi_write(Parse *, int);
void sqlite3MultiWrite(Parse *);
void sqlite3MayAbort(Parse *);
void sqlite3HaltConstraint(Parse *, int, int, char *, i8, u8);
@@ -3912,8 +3906,6 @@ struct coll *sqlite3GetCollSeq(Parse *, struct coll *, const char *);
char sqlite3AffinityType(const char *, u8 *);
void sqlite3Analyze(Parse *, Token *);
int sqlite3InvokeBusyHandler(BusyHandler *);
-int sqlite3FindDb(sqlite3 *, Token *);
-int sqlite3FindDbName(const char *);
int sqlite3AnalysisLoad(sqlite3 *);
void sqlite3DeleteIndexSamples(sqlite3 *, Index *);
void sqlite3DefaultRowEst(Index *);
@@ -3925,7 +3917,6 @@ void sqlite3RegisterLikeFunctions(sqlite3 *, int);
int sqlite3IsLikeFunction(sqlite3 *, Expr *, int *, char *);
void sqlite3SchemaClear(sqlite3 *);
Schema *sqlite3SchemaCreate(sqlite3 *);
-int sqlite3SchemaToIndex(sqlite3 * db, Schema *);
KeyInfo *sqlite3KeyInfoAlloc(sqlite3 *, int, int);
void sqlite3KeyInfoUnref(KeyInfo *);
KeyInfo *sqlite3KeyInfoRef(KeyInfo *);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index 859b421db..decbc8c21 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -129,7 +129,6 @@ sqlite3BeginTrigger(Parse * pParse, /* The parse context of the CREATE TRIGGER s
zName);
} else {
assert(!db->init.busy);
- sqlite3CodeVerifySchema(pParse);
}
goto trigger_cleanup;
}
@@ -215,7 +214,6 @@ sqlite3FinishTrigger(Parse * pParse, /* Parser context */
if (NEVER(pParse->nErr) || !pTrig)
goto triggerfinish_cleanup;
zName = pTrig->zName;
- assert(sqlite3SchemaToIndex(pParse->db, pTrig->pSchema) == 0);
pTrig->step_list = pStepList;
while (pStepList) {
pStepList->pTrig = pTrig;
@@ -295,7 +293,7 @@ sqlite3FinishTrigger(Parse * pParse, /* Parser context */
iFirstCol = pParse->nMem + 1;
pParse->nMem += 2;
- sqlite3BeginWriteOperation(pParse, 0);
+ sql_set_multi_write(pParse, 0);
sqlite3VdbeAddOp4(v,
OP_String8, 0, iFirstCol, 0,
zName, P4_STATIC);
@@ -528,8 +526,6 @@ sqlite3DropTrigger(Parse * pParse, SrcList * pName, int noErr)
if (!noErr) {
sqlite3ErrorMsg(pParse, "no such trigger: %S", pName,
0);
- } else {
- sqlite3CodeVerifySchema(pParse);
}
pParse->checkSchema = 1;
goto drop_trigger_cleanup;
@@ -674,7 +670,6 @@ targetSrcList(Parse * pParse, /* The parsing context */
assert(pSrc->nSrc > 0);
pSrc->a[pSrc->nSrc - 1].zName =
sqlite3DbStrDup(db, pStep->zTarget);
- assert(sqlite3SchemaToIndex(db, pStep->pTrig->pSchema) == 0);
}
return pSrc;
}
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index 7727ae5d5..ad6aad5e6 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -288,7 +288,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
goto update_cleanup;
if (pParse->nested == 0)
sqlite3VdbeCountChanges(v);
- sqlite3BeginWriteOperation(pParse, 1);
+ sql_set_multi_write(pParse, 1);
/* Allocate required registers. */
regOldPk = regNewPk = ++pParse->nMem;
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 329d46886..6de1b917a 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1419,7 +1419,6 @@ case OP_ResultRow: {
*/
if (SQLITE_OK!=(rc = sqlite3VdbeCheckFk(p, 0))) {
assert(user_session->sql_flags&SQLITE_CountRows);
- assert(p->usesStmtJournal);
goto abort_due_to_error;
}
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index fcb45c8a8..f23879b1c 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -428,7 +428,6 @@ struct Vdbe {
bft explain:2; /* True if EXPLAIN present on SQL command */
bft changeCntOn:1; /* True to update the change-counter */
bft runOnlyOnce:1; /* Automatically expire on reset */
- bft usesStmtJournal:1; /* True if uses a statement journal */
bft isPrepareV2:1; /* True if prepared with prepare_v2() */
u32 aCounter[5]; /* Counters used by sqlite3_stmt_status() */
char *zSql; /* Text of the SQL statement that generated this */
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index e4335524a..bb121a318 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -2148,7 +2148,6 @@ sqlite3VdbeMakeReady(Vdbe * p, /* The VDBE */
assert(EIGHT_BYTE_ALIGNMENT(&x.pSpace[x.nFree]));
resolveP2Values(p, &nArg);
- p->usesStmtJournal = (u8) (pParse->isMultiWrite && pParse->mayAbort);
if (pParse->explain && nMem < 10) {
nMem = 10;
}
diff --git a/src/box/sql/where.c b/src/box/sql/where.c
index 79de5d69a..d34faef9e 100644
--- a/src/box/sql/where.c
+++ b/src/box/sql/where.c
@@ -4616,7 +4616,6 @@ sqlite3WhereBegin(Parse * pParse, /* The parser context */
#endif /* SQLITE_ENABLE_COLUMN_USED_MASK */
}
}
- sqlite3CodeVerifySchema(pParse);
}
pWInfo->iTop = sqlite3VdbeCurrentAddr(v);
if (db->mallocFailed)
@@ -4814,7 +4813,6 @@ sqlite3WhereEnd(WhereInfo * pWInfo)
if (pOp->opcode == OP_Column) {
int x = pOp->p2;
assert(pIdx->pTable == pTab);
- x = sqlite3ColumnOfIndex(pIdx, x);
if (x >= 0) {
pOp->p2 = x;
pOp->p1 = pLevel->iIdxCur;
diff --git a/src/box/sql/wherecode.c b/src/box/sql/wherecode.c
index 8b06baeff..53cd30f14 100644
--- a/src/box/sql/wherecode.c
+++ b/src/box/sql/wherecode.c
@@ -847,7 +847,7 @@ codeCursorHintCheckExpr(Walker * pWalker, Expr * pExpr)
assert(pHint->pIdx != 0);
if (pExpr->op == TK_COLUMN
&& pExpr->iTable == pHint->iTabCur
- && sqlite3ColumnOfIndex(pHint->pIdx, pExpr->iColumn) < 0) {
+ && (pExpr->iColumn < 0)) {
pWalker->eCode = 1;
}
return WRC_Continue;
@@ -921,8 +921,6 @@ codeCursorHintFixExpr(Walker * pWalker, Expr * pExpr)
pExpr->iTable = reg;
} else if (pHint->pIdx != 0) {
pExpr->iTable = pHint->iIdxCur;
- pExpr->iColumn =
- sqlite3ColumnOfIndex(pHint->pIdx, pExpr->iColumn);
assert(pExpr->iColumn >= 0);
}
} else if (pExpr->op == TK_AGG_FUNCTION) {
@@ -1517,8 +1515,7 @@ sqlite3WhereCodeOneLoopStart(WhereInfo * pWInfo, /* Complete information about t
int nPkCol = index_column_count(pPk);
int iKeyReg = sqlite3GetTempRange(pParse, nPkCol);
for (j = 0; j < nPkCol; j++) {
- k = sqlite3ColumnOfIndex(pIdx,
- pPk->aiColumn[j]);
+ k = pPk->aiColumn[j];
sqlite3VdbeAddOp3(v, OP_Column, iIdxCur, k,
iKeyReg + j);
}
--
2.15.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling
2018-04-03 16:14 [tarantool-patches] [PATCH 0/2] rework SQL 'DROP' routine Nikita Pettik
2018-04-03 16:14 ` [tarantool-patches] [PATCH 1/2] sql: remove obsolete SQLite routine Nikita Pettik
@ 2018-04-03 16:14 ` Nikita Pettik
2018-04-03 18:27 ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-05 14:12 ` [tarantool-patches] Re: [PATCH 0/2] rework SQL 'DROP' routine Kirill Yukhin
2 siblings, 1 reply; 12+ messages in thread
From: Nikita Pettik @ 2018-04-03 16:14 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik
As a part of SQL data dictionary integration, 'DROP INDEX' and
'DROP TABLE' statements proccessing has been refactored in order
to avoid using SQL specific internal structures.
However, triggers still aren't transfered to server, so to drop them
it is required to lookup SQL table and emit apporpriate opcodes.
Also, added comments and fixed codestyle for functions processing
'DROP' routine.
Part of #3222.
---
src/box/sql/build.c | 241 ++++++++++++++++++++++++++----------------------
src/box/sql/parse.c | 6 +-
src/box/sql/parse.y | 6 +-
src/box/sql/sqliteInt.h | 6 +-
4 files changed, 140 insertions(+), 119 deletions(-)
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 61194e06b..219cc974b 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -46,6 +46,7 @@
#include "sqliteInt.h"
#include "vdbeInt.h"
#include "tarantoolInt.h"
+#include "box/box.h"
#include "box/sequence.h"
#include "box/session.h"
#include "box/identifier.h"
@@ -2152,48 +2153,50 @@ sql_clear_stat_spaces(Parse * pParse, const char *zType, const char *zName)
zType, zName);
}
-/*
+/**
* Generate code to drop a table.
+ * This routine includes dropping triggers, sequences,
+ * all indexes and entry from _space space.
+ *
+ * @param parse_context Current parsing context.
+ * @param space Space to be dropped.
+ * @param is_view True, if space is
*/
static void
-sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
+sql_code_drop_table(struct Parse *parse_context, struct space *space,
+ bool is_view)
{
- Vdbe *v;
- sqlite3 *db = pParse->db;
- Trigger *pTrigger;
-
- v = sqlite3GetVdbe(pParse);
+ struct sqlite3 *db = parse_context->db;
+ struct Vdbe *v = sqlite3GetVdbe(parse_context);
assert(v != 0);
/*
* Drop all triggers associated with the table being
* dropped. Code is generated to remove entries from
* _trigger. OP_DropTrigger will remove it from internal
* SQL structures.
- */
- pTrigger = pTab->pTrigger;
- /* Do not account triggers deletion - they will be accounted
+ *
+ * Do not account triggers deletion - they will be accounted
* in DELETE from _space below.
*/
- pParse->nested++;
-
- while (pTrigger) {
- assert(pTrigger->pSchema == pTab->pSchema);
- sqlite3DropTriggerPtr(pParse, pTrigger);
- pTrigger = pTrigger->pNext;
- }
- pParse->nested--;
+ parse_context->nested++;
+ Table *table = sqlite3HashFind(&parse_context->db->pSchema->tblHash,
+ space->def->name);
+ struct Trigger *trigger = table->pTrigger;
+ while (trigger != NULL) {
+ sqlite3DropTriggerPtr(parse_context, trigger);
+ trigger = trigger->pNext;
+ }
+ parse_context->nested--;
/*
* Remove any entries of the _sequence and _space_sequence
* space associated with the table being dropped. This is
* done before the table is dropped from internal schema.
*/
- int idx_rec_reg = ++pParse->nMem;
- int space_id_reg = ++pParse->nMem;
- int space_id = SQLITE_PAGENO_TO_SPACEID(pTab->tnum);
- struct space *space = space_by_id(space_id);
- assert(space != NULL);
+ int idx_rec_reg = ++parse_context->nMem;
+ int space_id_reg = ++parse_context->nMem;
+ int space_id = space->def->id;
sqlite3VdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
- if (pTab->tabFlags & TF_Autoincrement) {
+ if (space->sequence != NULL) {
/* Delete entry from _space_sequence. */
sqlite3VdbeAddOp3(v, OP_MakeRecord, space_id_reg, 1,
idx_rec_reg);
@@ -2201,8 +2204,7 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
idx_rec_reg);
VdbeComment((v, "Delete entry from _space_sequence"));
/* Delete entry by id from _sequence. */
- assert(space->sequence != NULL);
- int sequence_id_reg = ++pParse->nMem;
+ int sequence_id_reg = ++parse_context->nMem;
sqlite3VdbeAddOp2(v, OP_Integer, space->sequence->def->id,
sequence_id_reg);
sqlite3VdbeAddOp3(v, OP_MakeRecord, sequence_id_reg, 1,
@@ -2214,7 +2216,7 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
* Drop all _space and _index entries that refer to the
* table.
*/
- if (!isView) {
+ if (!is_view) {
uint32_t index_count = space->index_count;
if (index_count > 1) {
/*
@@ -2257,71 +2259,78 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
sqlite3VdbeAddOp2(v, OP_SDelete, BOX_SPACE_ID, idx_rec_reg);
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
VdbeComment((v, "Delete entry from _space"));
- /* Remove the table entry from SQLite's internal schema and modify
- * the schema cookie.
+ /*
+ * Remove the table entry from SQLite's internal schema
+ * and modify the schema cookie.
*/
+ sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, space->def->name, 0);
- sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, pTab->zName, 0);
-
- /* FIXME: DDL is impossible inside a transaction so far.
+ /*
* Replace to _space/_index will fail if active
* transaction. So, don't pretend, that we are able to
* anything back. To be fixed when transactions
* DDL are enabled.
*/
- /* sqlite3ChangeCookie(pParse); */
sqliteViewResetAll(db);
}
-/*
+/**
* This routine is called to do the work of a DROP TABLE statement.
- * pName is the name of the table to be dropped.
+ *
+ * @param parse_context Current parsing context.
+ * @param table_name_list List containing table name.
+ * @param is_view True, if statement is really 'DROP VIEW'.
+ * @param if_exists True, if statement contains 'IF EXISTS' clause.
*/
void
-sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
+sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
+ bool is_view, bool if_exists)
{
- Table *pTab;
- Vdbe *v = sqlite3GetVdbe(pParse);
- sqlite3 *db = pParse->db;
-
+ struct Vdbe *v = sqlite3GetVdbe(parse_context);
+ struct sqlite3 *db = parse_context->db;
if (v == NULL || db->mallocFailed) {
goto exit_drop_table;
}
- /* Activate changes counting here to account
+ /*
+ * Activate changes counting here to account
* DROP TABLE IF NOT EXISTS, if the table really does not
* exist.
*/
- if (!pParse->nested)
+ if (!parse_context->nested)
sqlite3VdbeCountChanges(v);
- assert(pParse->nErr == 0);
- assert(pName->nSrc == 1);
- assert(db->pSchema != NULL);
- if (noErr)
- db->suppressErr++;
- assert(isView == 0 || isView == LOCATE_VIEW);
- pTab = sqlite3LocateTable(pParse, isView, pName->a[0].zName);
- if (noErr)
- db->suppressErr--;
-
- if (pTab == 0)
+ assert(parse_context->nErr == 0);
+ assert(table_name_list->nSrc == 1);
+ assert(is_view == 0 || is_view == LOCATE_VIEW);
+ const char *space_name = table_name_list->a[0].zName;
+ uint32_t space_id = box_space_id_by_name(space_name,
+ strlen(space_name));
+ if (space_id == BOX_ID_NIL) {
+ if (!is_view && !if_exists)
+ sqlite3ErrorMsg(parse_context, "no such table: %s",
+ space_name);
+ if (is_view && !if_exists)
+ sqlite3ErrorMsg(parse_context, "no such view: %s",
+ space_name);
goto exit_drop_table;
-#ifndef SQLITE_OMIT_VIEW
- /* Ensure DROP TABLE is not used on a view, and DROP VIEW is not used
- * on a table.
+ }
+ struct space *space = space_by_id(space_id);
+ assert(space != NULL);
+ /*
+ * Ensure DROP TABLE is not used on a view,
+ * and DROP VIEW is not used on a table.
*/
- if (isView && !space_is_view(pTab)) {
- sqlite3ErrorMsg(pParse, "use DROP TABLE to delete table %s",
- pTab->zName);
+ if (is_view && !space->def->opts.is_view) {
+ sqlite3ErrorMsg(parse_context, "use DROP TABLE to delete table %s",
+ space_name);
goto exit_drop_table;
}
- if (!isView && space_is_view(pTab)) {
- sqlite3ErrorMsg(pParse, "use DROP VIEW to delete view %s",
- pTab->zName);
+ if (!is_view && space->def->opts.is_view) {
+ sqlite3ErrorMsg(parse_context, "use DROP VIEW to delete view %s",
+ space_name);
goto exit_drop_table;
}
-#endif
-
- /* Generate code to remove the table from Tarantool and internal SQL
+ /*
+ * Generate code to remove the table from Tarantool and internal SQL
* tables. Basically, it consists from 3 stages:
* 1. Delete statistics from _stat1 and _stat4 tables (if any exist)
* 2. In case of presence of FK constraints, i.e. current table is child
@@ -2334,13 +2343,14 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
* space_id from _space.
*/
- sql_set_multi_write(pParse, 1);
- sql_clear_stat_spaces(pParse, "tbl", pTab->zName);
- sqlite3FkDropTable(pParse, pName, pTab);
- sqlite3CodeDropTable(pParse, pTab, isView);
+ sql_set_multi_write(parse_context, 1);
+ sql_clear_stat_spaces(parse_context, "tbl", space_name);
+ struct Table *tab = sqlite3HashFind(&db->pSchema->tblHash, space_name);
+ sqlite3FkDropTable(parse_context, table_name_list, tab);
+ sql_code_drop_table(parse_context, space, is_view);
exit_drop_table:
- sqlite3SrcListDelete(db, pName);
+ sqlite3SrcListDelete(db, table_name_list);
}
/*
@@ -3267,60 +3277,66 @@ index_is_unique_not_null(const Index *idx)
!index->def->key_def->is_nullable);
}
-/*
+/**
* This routine will drop an existing named index. This routine
* implements the DROP INDEX statement.
+ *
+ * @param parse_context Current parsing context.
+ * @param index_name_list List containing index name.
+ * @param table_token Token representing table name.
+ * @param if_exists True, if statement contains 'IF EXISTS' clause.
*/
void
-sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)
+sql_drop_index(struct Parse *parse_context, struct SrcList *index_name_list,
+ struct Token *table_token, bool if_exists)
{
- Index *pIndex;
- Vdbe *v = sqlite3GetVdbe(pParse);
- sqlite3 *db = pParse->db;
- char *zTableName = 0;
-
- assert(pParse->nErr == 0); /* Never called with prior errors */
- assert(pName2 != 0);
-
+ struct Vdbe *v = sqlite3GetVdbe(parse_context);
+ assert(v != NULL);
+ struct sqlite3 *db = parse_context->db;
+ /* Never called with prior errors. */
+ assert(parse_context->nErr == 0);
+ assert(table_token != NULL);
+ const char *table_name = sqlite3NameFromToken(db, table_token);
if (db->mallocFailed) {
goto exit_drop_index;
}
- assert(v != NULL);
- /* Do not account nested operations: the count of such
+ /*
+ * Do not account nested operations: the count of such
* operations depends on Tarantool data dictionary internals,
* such as data layout in system spaces.
*/
- if (!pParse->nested)
+ if (!parse_context->nested)
sqlite3VdbeCountChanges(v);
- assert(pName->nSrc == 1);
- assert(db->pSchema != NULL);
-
- assert(pName2->n > 0);
- zTableName = sqlite3NameFromToken(db, pName2);
-
- pIndex = sqlite3LocateIndex(db, pName->a[0].zName, zTableName);
- if (pIndex == 0) {
- if (!ifExists) {
- sqlite3ErrorMsg(pParse, "no such index: %s.%S",
- zTableName, pName, 0);
- }
- pParse->checkSchema = 1;
+ assert(index_name_list->nSrc == 1);
+ assert(table_token->n > 0);
+ uint32_t space_id = box_space_id_by_name(table_name,
+ strlen(table_name));
+ if (space_id == BOX_ID_NIL) {
+ if (!if_exists)
+ sqlite3ErrorMsg(parse_context, "no such space: %s",
+ table_name);
+ goto exit_drop_index;
+ }
+ const char *index_name = index_name_list->a[0].zName;
+ uint32_t index_id = box_index_id_by_name(space_id, index_name,
+ strlen(index_name));
+ if (index_id == BOX_ID_NIL) {
+ if (!if_exists)
+ sqlite3ErrorMsg(parse_context, "no such index: %s.%s",
+ table_name, index_name);
goto exit_drop_index;
}
- uint32_t space_id = SQLITE_PAGENO_TO_SPACEID(pIndex->tnum);
- uint32_t index_id = SQLITE_PAGENO_TO_INDEXID(pIndex->tnum);
struct space *space = space_by_id(space_id);
assert(space != NULL);
struct index *index = space_index(space, index_id);
assert(index != NULL);
-
/*
* If index has been created by user, it has its SQL
* statement. Otherwise (i.e. PK and UNIQUE indexes,
* which are created alongside with table) it is NULL.
*/
if (index->def->opts.sql == NULL) {
- sqlite3ErrorMsg(pParse, "index associated with UNIQUE "
+ sqlite3ErrorMsg(parse_context, "index associated with UNIQUE "
"or PRIMARY KEY constraint cannot be dropped",
0);
goto exit_drop_index;
@@ -3331,24 +3347,27 @@ sqlite3DropIndex(Parse * pParse, SrcList * pName, Token * pName2, int ifExists)
* But firstly, delete statistics since schema
* changes after DDL.
*/
- sql_clear_stat_spaces(pParse, "idx", pIndex->zName);
- int record_reg = ++pParse->nMem;
- int space_id_reg = ++pParse->nMem;
- sqlite3VdbeAddOp2(v, OP_Integer, SQLITE_PAGENO_TO_SPACEID(pIndex->tnum),
- space_id_reg);
- sqlite3VdbeAddOp2(v, OP_Integer, SQLITE_PAGENO_TO_INDEXID(pIndex->tnum),
- space_id_reg + 1);
+ sql_clear_stat_spaces(parse_context, "idx", index->def->name);
+ int record_reg = ++parse_context->nMem;
+ int space_id_reg = ++parse_context->nMem;
+ sqlite3VdbeAddOp2(v, OP_Integer, space_id, space_id_reg);
+ sqlite3VdbeAddOp2(v, OP_Integer, index_id, space_id_reg + 1);
sqlite3VdbeAddOp3(v, OP_MakeRecord, space_id_reg, 2, record_reg);
sqlite3VdbeAddOp2(v, OP_SDelete, BOX_INDEX_ID, record_reg);
sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
- sqlite3ChangeCookie(pParse);
-
+ /*
+ * Still need to cleanup internal SQL structures.
+ * Should be removed when SQL and Tarantool
+ * data-dictionaries will be completely merged.
+ */
+ Index *pIndex = sqlite3LocateIndex(db, index_name, table_name);
+ assert(pIndex != NULL);
sqlite3VdbeAddOp3(v, OP_DropIndex, 0, 0, 0);
sqlite3VdbeAppendP4(v, pIndex, P4_INDEX);
exit_drop_index:
- sqlite3SrcListDelete(db, pName);
- sqlite3DbFree(db, zTableName);
+ sqlite3SrcListDelete(db, index_name_list);
+ sqlite3DbFree(db, (void *) table_name);
}
/*
diff --git a/src/box/sql/parse.c b/src/box/sql/parse.c
index 9f547d25e..bf34b2cab 100644
--- a/src/box/sql/parse.c
+++ b/src/box/sql/parse.c
@@ -2530,7 +2530,7 @@ static void yy_reduce(
case 70: /* cmd ::= DROP TABLE ifexists fullname */
#line 361 "parse.y"
{
- sqlite3DropTable(pParse, yymsp[0].minor.yy387, 0, yymsp[-1].minor.yy52);
+ sql_drop_table(pParse, yymsp[0].minor.yy387, 0, yymsp[-1].minor.yy52);
}
#line 2536 "parse.c"
break;
@@ -2544,7 +2544,7 @@ static void yy_reduce(
case 74: /* cmd ::= DROP VIEW ifexists fullname */
#line 375 "parse.y"
{
- sqlite3DropTable(pParse, yymsp[0].minor.yy387, 1, yymsp[-1].minor.yy52);
+ sql_drop_table(pParse, yymsp[0].minor.yy387, 1, yymsp[-1].minor.yy52);
}
#line 2550 "parse.c"
break;
@@ -3445,7 +3445,7 @@ static void yy_reduce(
case 210: /* cmd ::= DROP INDEX ifexists fullname ON nm */
#line 1310 "parse.y"
{
- sqlite3DropIndex(pParse, yymsp[-2].minor.yy387, &yymsp[0].minor.yy0, yymsp[-3].minor.yy52);
+ sql_drop_index(pParse, yymsp[-2].minor.yy387, &yymsp[0].minor.yy0, yymsp[-3].minor.yy52);
}
#line 3451 "parse.c"
break;
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 9ee7daccf..6440c2ea0 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -359,7 +359,7 @@ resolvetype(A) ::= REPLACE. {A = ON_CONFLICT_ACTION_REPLACE;}
////////////////////////// The DROP TABLE /////////////////////////////////////
//
cmd ::= DROP TABLE ifexists(E) fullname(X). {
- sqlite3DropTable(pParse, X, 0, E);
+ sql_drop_table(pParse, X, 0, E);
}
%type ifexists {int}
ifexists(A) ::= IF EXISTS. {A = 1;}
@@ -373,7 +373,7 @@ cmd ::= createkw(X) VIEW ifnotexists(E) nm(Y) eidlist_opt(C)
sqlite3CreateView(pParse, &X, &Y, C, S, E);
}
cmd ::= DROP VIEW ifexists(E) fullname(X). {
- sqlite3DropTable(pParse, X, 1, E);
+ sql_drop_table(pParse, X, 1, E);
}
%endif SQLITE_OMIT_VIEW
@@ -1308,7 +1308,7 @@ collate(C) ::= COLLATE id. {C = 1;}
///////////////////////////// The DROP INDEX command /////////////////////////
//
cmd ::= DROP INDEX ifexists(E) fullname(X) ON nm(Y). {
- sqlite3DropIndex(pParse, X, &Y, E);
+ sql_drop_index(pParse, X, &Y, E);
}
///////////////////////////// The PRAGMA command /////////////////////////////
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index fcafb65b4..5e19b565a 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -3588,7 +3588,8 @@ int sqlite3ViewGetColumnNames(Parse *, Table *);
#if SQLITE_MAX_ATTACHED>30
int sqlite3DbMaskAllZero(yDbMask);
#endif
-void sqlite3DropTable(Parse *, SrcList *, int, int);
+void
+sql_drop_table(struct Parse *, struct SrcList *, bool, bool);
void sqlite3DeleteTable(sqlite3 *, Table *);
void sqlite3Insert(Parse *, SrcList *, Select *, IdList *, int);
void *sqlite3ArrayAllocate(sqlite3 *, void *, int, int *, int *);
@@ -3612,7 +3613,8 @@ bool
index_is_unique(Index *);
void sqlite3CreateIndex(Parse *, Token *, SrcList *, ExprList *, int, Token *,
Expr *, int, int, u8);
-void sqlite3DropIndex(Parse *, SrcList *, Token *, int);
+void
+sql_drop_index(struct Parse *, struct SrcList *, struct Token *, bool);
int sqlite3Select(Parse *, Select *, SelectDest *);
Select *sqlite3SelectNew(Parse *, ExprList *, SrcList *, Expr *, ExprList *,
Expr *, ExprList *, u32, Expr *, Expr *);
--
2.15.1
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: remove obsolete SQLite routine
2018-04-03 16:14 ` [tarantool-patches] [PATCH 1/2] sql: remove obsolete SQLite routine Nikita Pettik
@ 2018-04-03 17:54 ` Vladislav Shpilevoy
2018-04-04 18:09 ` n.pettik
0 siblings, 1 reply; 12+ messages in thread
From: Vladislav Shpilevoy @ 2018-04-03 17:54 UTC (permalink / raw)
To: Nikita Pettik, tarantool-patches
Hello. See 7 comments below.
03.04.2018 19:14, Nikita Pettik пишет:
> Some of legacy functions seem to be useless, since they serve as
> wrappers around others; the rest rely on capabilities which are no
> longer relevant. This patch provides slight refactoring of such
> functions.
>
> Removed entities:
> - sqlite3LocateTableItem() - replaced with sqlite3LocateTable();
> - sqlite3FindTable() - replaced with sqlite3HashFind();
> - sqlite3ColumnOfIndex() - in Tarantool order of columns always the same;
> - sqlite3FindIndex() - replaced with sqlite3LocateIndex();
> - sqlite3CodeVerifySchema();
> - sqlite3SchemaToIndex();
> - sqlite3MultiWrite();
> - Parse->cookieMast;
> - Vdbe->usesStmtJournal;
> ---
> src/box/sql/alter.c | 13 ++-
> src/box/sql/analyze.c | 25 +++---
> src/box/sql/build.c | 213 ++++++++++--------------------------------------
> src/box/sql/delete.c | 5 +-
> src/box/sql/expr.c | 9 +-
> src/box/sql/fkey.c | 3 +-
> src/box/sql/insert.c | 10 +--
> src/box/sql/pragma.c | 18 ++--
> src/box/sql/prepare.c | 29 -------
> src/box/sql/select.c | 4 +-
> src/box/sql/sqliteInt.h | 11 +--
> src/box/sql/trigger.c | 7 +-
> src/box/sql/update.c | 2 +-
> src/box/sql/vdbe.c | 1 -
> src/box/sql/vdbeInt.h | 1 -
> src/box/sql/vdbeaux.c | 1 -
> src/box/sql/where.c | 2 -
> src/box/sql/wherecode.c | 7 +-
> 18 files changed, 83 insertions(+), 278 deletions(-)
>
> diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
> index 054c0856c..a19324ed2 100644
> --- a/src/box/sql/alter.c
> +++ b/src/box/sql/alter.c
> @@ -122,7 +121,7 @@ sqlite3AlterRenameTable(Parse * pParse, /* Parser context. */
> if (v == 0) {
> goto exit_rename_table;
> }
> - sqlite3BeginWriteOperation(pParse, false);
> + sql_set_multi_write(pParse, false);
1. Can you please alongside with this patch make sql_set_multi_write
take boolean as the last
argument? Now it sometimes gets true/false, sometimes 1/0. It is strange.
>
> /* Drop and reload the internal table schema. */
> reloadTableSchema(pParse, pTab, zName);
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 5e3ed0f39..61194e06b 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -93,23 +93,16 @@ sqlite3FinishCoding(Parse * pParse)
> * transaction on each used database and to verify the schema cookie
> * on each used database.
> */
> - if (db->mallocFailed == 0
> - && (DbMaskNonZero(pParse->cookieMask) || pParse->pConstExpr)
> - ) {
> + if (db->mallocFailed == 0 || pParse->pConstExpr) {
2. Please, together with cookie mask checking remove or refactor a
comment above.
> @@ -2240,7 +2164,6 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
>
> v = sqlite3GetVdbe(pParse);
> assert(v != 0);
> - sqlite3BeginWriteOperation(pParse, 1);
3. Why did you delete it with no replacement by sql_set_multi_write(false) ?
> @@ -2376,15 +2299,12 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
> if (noErr)
> db->suppressErr++;
> assert(isView == 0 || isView == LOCATE_VIEW);
> - pTab = sqlite3LocateTableItem(pParse, isView, &pName->a[0]);
> + pTab = sqlite3LocateTable(pParse, isView, pName->a[0].zName);
> if (noErr)
> db->suppressErr--;
>
> - if (pTab == 0) {
> - if (noErr)
> - sqlite3CodeVerifySchema(pParse);
> + if (pTab == 0)
4. Lets use ==/!= NULL in all new code. Same about checking
sqlite3HashFind results. And if it is
possible with not huge diff, can you please rename sqlite3HashFind to
sql_hash_find ?
5. sqlite3MultiWrite in commit message is listed among deleted
functions, but its declaration still exists.
6. cookieMast - typo in commit message. And how about do not list
deleted functions in a commit body?
I can not imagine, that somebody except me will search for any of these
functions. And the list is deprecated -
for example, sqlite3BeginWriteOperation is deleted too, but does not
presence in the list.
7. How about remove DbMaskTest, DbMaskZero and other dbmask shit?
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling
2018-04-03 16:14 ` [tarantool-patches] [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling Nikita Pettik
@ 2018-04-03 18:27 ` Vladislav Shpilevoy
2018-04-04 18:11 ` n.pettik
0 siblings, 1 reply; 12+ messages in thread
From: Vladislav Shpilevoy @ 2018-04-03 18:27 UTC (permalink / raw)
To: tarantool-patches, Nikita Pettik
See 6 comments below.
03.04.2018 19:14, Nikita Pettik пишет:
> As a part of SQL data dictionary integration, 'DROP INDEX' and
> 'DROP TABLE' statements proccessing has been refactored in order
> to avoid using SQL specific internal structures.
> However, triggers still aren't transfered to server, so to drop them
> it is required to lookup SQL table and emit apporpriate opcodes.
> Also, added comments and fixed codestyle for functions processing
> 'DROP' routine.
>
> Part of #3222.
> ---
> src/box/sql/build.c | 241 ++++++++++++++++++++++++++----------------------
> src/box/sql/parse.c | 6 +-
> src/box/sql/parse.y | 6 +-
> src/box/sql/sqliteInt.h | 6 +-
> 4 files changed, 140 insertions(+), 119 deletions(-)
>
> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
> index 61194e06b..219cc974b 100644
> --- a/src/box/sql/build.c
> +++ b/src/box/sql/build.c
> @@ -46,6 +46,7 @@
> #include "sqliteInt.h"
> #include "vdbeInt.h"
> #include "tarantoolInt.h"
> +#include "box/box.h"
> #include "box/sequence.h"
> #include "box/session.h"
> #include "box/identifier.h"
> @@ -2152,48 +2153,50 @@ sql_clear_stat_spaces(Parse * pParse, const char *zType, const char *zName)
> zType, zName);
> }
>
> -/*
> +/**
> * Generate code to drop a table.
> + * This routine includes dropping triggers, sequences,
> + * all indexes and entry from _space space.
> + *
> + * @param parse_context Current parsing context.
> + * @param space Space to be dropped.
> + * @param is_view True, if space is
> */
> static void
> -sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
> +sql_code_drop_table(struct Parse *parse_context, struct space *space,
> + bool is_view)
> {
> - Vdbe *v;
> - sqlite3 *db = pParse->db;
> - Trigger *pTrigger;
> -
> - v = sqlite3GetVdbe(pParse);
>
> + struct sqlite3 *db = parse_context->db;
> + struct Vdbe *v = sqlite3GetVdbe(parse_context);
> assert(v != 0);
1. Lets in all new code use explicit ==/!= NULL.
> /*
> * Drop all triggers associated with the table being
> * dropped. Code is generated to remove entries from
> * _trigger. OP_DropTrigger will remove it from internal
> * SQL structures.
> - */
> - pTrigger = pTab->pTrigger;
> - /* Do not account triggers deletion - they will be accounted
> + *
> + * Do not account triggers deletion - they will be accounted
2. Out of 66.
> @@ -2214,7 +2216,7 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
> * Drop all _space and _index entries that refer to the
> * table.
> */
> - if (!isView) {
> + if (!is_view) {
3. I looked at the indexes deletion more carefully and noticed, that you
can avoid any allocations
here. Why can not you just walk through space->index array and generate
OP_SDelete opcodes for
each index, starting from 1 (to skip primary)? AFAIK the entire index
array is not changed until you
reach VdbeExec, so you can do not save index identifiers on region to
generate opcodes.
> uint32_t index_count = space->index_count;
> if (index_count > 1) {
> /*
> @@ -2257,71 +2259,78 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
> sqlite3VdbeAddOp2(v, OP_SDelete, BOX_SPACE_ID, idx_rec_reg);
> sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
> VdbeComment((v, "Delete entry from _space"));
> - /* Remove the table entry from SQLite's internal schema and modify
> - * the schema cookie.
> + /*
> + * Remove the table entry from SQLite's internal schema
> + * and modify the schema cookie.
> */
4. But you have deleted cookie updates in the previous commit. Source
code of OP_DropTable and all
called there functions does not contain cookie changes.
> + sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, space->def->name, 0);
>
> - sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, pTab->zName, 0);
> -
> - /* FIXME: DDL is impossible inside a transaction so far.
> + /*
> * Replace to _space/_index will fail if active
> * transaction. So, don't pretend, that we are able to
> * anything back. To be fixed when transactions
> * DDL are enabled.
> */
> - /* sqlite3ChangeCookie(pParse); */
> sqliteViewResetAll(db);
> }
>
> -/*
> +/**
> * This routine is called to do the work of a DROP TABLE statement.
> - * pName is the name of the table to be dropped.
> + *
> + * @param parse_context Current parsing context.
> + * @param table_name_list List containing table name.
> + * @param is_view True, if statement is really 'DROP VIEW'.
> + * @param if_exists True, if statement contains 'IF EXISTS' clause.
> */
> void
> -sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
> +sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
> + bool is_view, bool if_exists)
> {
> - Table *pTab;
> - Vdbe *v = sqlite3GetVdbe(pParse);
> - sqlite3 *db = pParse->db;
> -
> + struct Vdbe *v = sqlite3GetVdbe(parse_context);
> + struct sqlite3 *db = parse_context->db;
> if (v == NULL || db->mallocFailed) {
> goto exit_drop_table;
> }
> - /* Activate changes counting here to account
> + /*
> + * Activate changes counting here to account
> * DROP TABLE IF NOT EXISTS, if the table really does not
> * exist.
> */
> - if (!pParse->nested)
> + if (!parse_context->nested)
5. Please, use explicit != 0. Why you use !int, the variable seems to be
boolean. It slightly confuses.
> sqlite3VdbeCountChanges(v);
> - assert(pParse->nErr == 0);
> - assert(pName->nSrc == 1);
> - assert(db->pSchema != NULL);
> - if (noErr)
> - db->suppressErr++;
> - assert(isView == 0 || isView == LOCATE_VIEW);
> - pTab = sqlite3LocateTable(pParse, isView, pName->a[0].zName);
> - if (noErr)
> - db->suppressErr--;
> -
> - if (pTab == 0)
> + assert(parse_context->nErr == 0);
> + assert(table_name_list->nSrc == 1);
> + assert(is_view == 0 || is_view == LOCATE_VIEW);
> + const char *space_name = table_name_list->a[0].zName;
> + uint32_t space_id = box_space_id_by_name(space_name,
> + strlen(space_name));
> + if (space_id == BOX_ID_NIL) {
> + if (!is_view && !if_exists)
> + sqlite3ErrorMsg(parse_context, "no such table: %s",
> + space_name);
> + if (is_view && !if_exists)
> + sqlite3ErrorMsg(parse_context, "no such view: %s",
> + space_name);
> goto exit_drop_table;
> -#ifndef SQLITE_OMIT_VIEW
> - /* Ensure DROP TABLE is not used on a view, and DROP VIEW is not used
> - * on a table.
> + }
> + struct space *space = space_by_id(space_id);
> + assert(space != NULL);
> + /*
> + * Ensure DROP TABLE is not used on a view,
> + * and DROP VIEW is not used on a table.
> */
> - if (isView && !space_is_view(pTab)) {
> - sqlite3ErrorMsg(pParse, "use DROP TABLE to delete table %s",
> - pTab->zName);
> + if (is_view && !space->def->opts.is_view) {
> + sqlite3ErrorMsg(parse_context, "use DROP TABLE to delete table %s",
> + space_name);
> goto exit_drop_table;
> }
> - if (!isView && space_is_view(pTab)) {
> - sqlite3ErrorMsg(pParse, "use DROP VIEW to delete view %s",
> - pTab->zName);
> + if (!is_view && space->def->opts.is_view) {
> + sqlite3ErrorMsg(parse_context, "use DROP VIEW to delete view %s",
> + space_name);
> goto exit_drop_table;
> }
> -#endif
> -
> - /* Generate code to remove the table from Tarantool and internal SQL
> + /*
> + * Generate code to remove the table from Tarantool and internal SQL
6. Out of 66.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: remove obsolete SQLite routine
2018-04-03 17:54 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-04-04 18:09 ` n.pettik
2018-04-05 11:16 ` Vladislav Shpilevoy
0 siblings, 1 reply; 12+ messages in thread
From: n.pettik @ 2018-04-04 18:09 UTC (permalink / raw)
To: tarantool-patches; +Cc: Vladislav Shpilevoy
[-- Attachment #1: Type: text/plain, Size: 20595 bytes --]
> On 3 Apr 2018, at 20:54, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>
> Hello. See 7 comments below.
>
> 03.04.2018 19:14, Nikita Pettik пишет:
>> Some of legacy functions seem to be useless, since they serve as
>> wrappers around others; the rest rely on capabilities which are no
>> longer relevant. This patch provides slight refactoring of such
>> functions.
>>
>> Removed entities:
>> - sqlite3LocateTableItem() - replaced with sqlite3LocateTable();
>> - sqlite3FindTable() - replaced with sqlite3HashFind();
>> - sqlite3ColumnOfIndex() - in Tarantool order of columns always the same;
>> - sqlite3FindIndex() - replaced with sqlite3LocateIndex();
>> - sqlite3CodeVerifySchema();
>> - sqlite3SchemaToIndex();
>> - sqlite3MultiWrite();
>> - Parse->cookieMast;
>> - Vdbe->usesStmtJournal;
>> ---
>> src/box/sql/alter.c | 13 ++-
>> src/box/sql/analyze.c | 25 +++---
>> src/box/sql/build.c | 213 ++++++++++--------------------------------------
>> src/box/sql/delete.c | 5 +-
>> src/box/sql/expr.c | 9 +-
>> src/box/sql/fkey.c | 3 +-
>> src/box/sql/insert.c | 10 +--
>> src/box/sql/pragma.c | 18 ++--
>> src/box/sql/prepare.c | 29 -------
>> src/box/sql/select.c | 4 +-
>> src/box/sql/sqliteInt.h | 11 +--
>> src/box/sql/trigger.c | 7 +-
>> src/box/sql/update.c | 2 +-
>> src/box/sql/vdbe.c | 1 -
>> src/box/sql/vdbeInt.h | 1 -
>> src/box/sql/vdbeaux.c | 1 -
>> src/box/sql/where.c | 2 -
>> src/box/sql/wherecode.c | 7 +-
>> 18 files changed, 83 insertions(+), 278 deletions(-)
>>
>> diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
>> index 054c0856c..a19324ed2 100644
>> --- a/src/box/sql/alter.c
>> +++ b/src/box/sql/alter.c
>> @@ -122,7 +121,7 @@ sqlite3AlterRenameTable(Parse * pParse, /* Parser context. */
>> if (v == 0) {
>> goto exit_rename_table;
>> }
>> - sqlite3BeginWriteOperation(pParse, false);
>> + sql_set_multi_write(pParse, false);
> 1. Can you please alongside with this patch make sql_set_multi_write take boolean as the last
> argument? Now it sometimes gets true/false, sometimes 1/0. It is strange.
Done.
>> /* Drop and reload the internal table schema. */
>> reloadTableSchema(pParse, pTab, zName);
>>
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index 5e3ed0f39..61194e06b 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -93,23 +93,16 @@ sqlite3FinishCoding(Parse * pParse)
>> * transaction on each used database and to verify the schema cookie
>> * on each used database.
>> */
>> - if (db->mallocFailed == 0
>> - && (DbMaskNonZero(pParse->cookieMask) || pParse->pConstExpr)
>> - ) {
>> + if (db->mallocFailed == 0 || pParse->pConstExpr) {
> 2. Please, together with cookie mask checking remove or refactor a comment above.
Fixed.
>> @@ -2240,7 +2164,6 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
>> v = sqlite3GetVdbe(pParse);
>> assert(v != 0);
>> - sqlite3BeginWriteOperation(pParse, 1);
> 3. Why did you delete it with no replacement by sql_set_multi_write(false) ?
Because in this particular case is is completely useless:
table dropping is implemented as hardcoded opcodes sequences.
AFAIK this mask is used to provide kind of optimisation/checks during
query compilation.
Also, removed:
@@ -2414,8 +2326,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
* space_id from _space.
*/
- sqlite3BeginWriteOperation(pParse, 1);
sql_clear_stat_spaces(pParse, "tbl", pTab->zName);
sqlite3FkDropTable(pParse, pName, pTab);
sqlite3CodeDropTable(pParse, pTab, isView);
>> @@ -2376,15 +2299,12 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
>> if (noErr)
>> db->suppressErr++;
>> assert(isView == 0 || isView == LOCATE_VIEW);
>> - pTab = sqlite3LocateTableItem(pParse, isView, &pName->a[0]);
>> + pTab = sqlite3LocateTable(pParse, isView, pName->a[0].zName);
>> if (noErr)
>> db->suppressErr--;
>> - if (pTab == 0) {
>> - if (noErr)
>> - sqlite3CodeVerifySchema(pParse);
>> + if (pTab == 0)
> 4. Lets use ==/!= NULL in all new code.
Done.
> Same about checking sqlite3HashFind results. And if it is
> possible with not huge diff, can you please rename sqlite3HashFind to sql_hash_find ?
Is it worth doing? It is going to disappear soon.
>
> 5. sqlite3MultiWrite in commit message is listed among deleted functions, but its declaration still exists.
Fixed:
@@ -3718,8 +3689,8 @@ void sqlite3GenerateConstraintChecks(Parse *, Table *, int *, int, int, int,
void sqlite3CompleteInsertion(Parse *, Table *, int, int *, int, u8);
int sqlite3OpenTableAndIndices(Parse *, Table *, int, u8, int, u8 *, int *,
int *, u8, u8);
-void sqlite3BeginWriteOperation(Parse *, int);
-void sqlite3MultiWrite(Parse *);
+void
+sql_set_multi_write(Parse *, bool);
>
> 6. cookieMast - typo in commit message. And how about do not list deleted functions in a commit body?
> I can not imagine, that somebody except me will search for any of these functions. And the list is deprecated -
> for example, sqlite3BeginWriteOperation is deleted too, but does not presence in the list.
Ok, fixed commit message. Left only description.
>
> 7. How about remove DbMaskTest, DbMaskZero and other dbmask shit?
As you wish. Done.
Diff is below:
diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c
index a19324ed2..129ef823c 100644
--- a/src/box/sql/alter.c
+++ b/src/box/sql/alter.c
@@ -85,7 +85,7 @@ sqlite3AlterRenameTable(Parse * pParse, /* Parser context. */
assert(pSrc->nSrc == 1);
pTab = sqlite3LocateTable(pParse, 0, pSrc->a[0].zName);
- if (!pTab)
+ if (pTab == NULL)
goto exit_rename_table;
user_session->sql_flags |= SQLITE_PreferBuiltin;
@@ -257,7 +257,7 @@ sqlite3AlterBeginAddColumn(Parse * pParse, SrcList * pSrc)
if (db->mallocFailed)
goto exit_begin_add_column;
pTab = sqlite3LocateTable(pParse, 0, pSrc->a[0].zName);
- if (!pTab)
+ if (pTab == NULL)
goto exit_begin_add_column;
/* Make sure this is not an attempt to ALTER a view. */
@@ -304,7 +304,7 @@ sqlite3AlterBeginAddColumn(Parse * pParse, SrcList * pSrc)
pNew->nTabRef = 1;
/* Begin a transaction and increment the schema cookie. */
- sql_set_multi_write(pParse, 0);
+ sql_set_multi_write(pParse, false);
v = sqlite3GetVdbe(pParse);
if (!v)
goto exit_begin_add_column;
diff --git a/src/box/sql/analyze.c b/src/box/sql/analyze.c
index 25e93aa15..60f4eaac4 100644
--- a/src/box/sql/analyze.c
+++ b/src/box/sql/analyze.c
@@ -157,6 +157,7 @@ openStatTable(Parse * pParse, /* Parsing context */
Table *pStat;
/* The table already exists, because it is a system space */
pStat = sqlite3HashFind(&db->pSchema->tblHash, zTab);
+ assert(pStat != NULL);
aRoot[i] = pStat->tnum;
aCreateTbl[i] = 0;
if (zWhere) {
@@ -1122,7 +1123,7 @@ analyzeDatabase(Parse * pParse)
int iMem;
int iTab;
- sql_set_multi_write(pParse, 0);
+ sql_set_multi_write(pParse, false);
iStatCur = pParse->nTab;
pParse->nTab += 3;
openStatTable(pParse, iStatCur, 0, 0);
@@ -1146,7 +1147,7 @@ analyzeTable(Parse * pParse, Table * pTab, Index * pOnlyIdx)
int iStatCur;
assert(pTab != 0);
- sql_set_multi_write(pParse, 0);
+ sql_set_multi_write(pParse, true);
iStatCur = pParse->nTab;
pParse->nTab += 3;
if (pOnlyIdx) {
@@ -1294,9 +1295,8 @@ analysisLoader(void *pData, int argc, char **argv, char **NotUsed)
return 0;
}
pTable = sqlite3HashFind(&pInfo->db->pSchema->tblHash, argv[0]);
- if (pTable == 0) {
+ if (pTable == NULL)
return 0;
- }
if (argv[1] == 0) {
pIndex = 0;
} else if (sqlite3_stricmp(argv[0], argv[1]) == 0) {
@@ -1631,19 +1631,17 @@ loadStatTbl(sqlite3 * db, /* Database handle */
static int
loadStat4(sqlite3 * db)
{
- int rc = SQLITE_OK; /* Result codes from subroutines */
Table *pTab = 0; /* Pointer to stat table */
assert(db->lookaside.bDisable);
pTab = sqlite3HashFind(&db->pSchema->tblHash, "_sql_stat4");
- if (pTab) {
- rc = loadStatTbl(db,
- pTab,
- "SELECT \"tbl\",\"idx\",count(*) FROM \"_sql_stat4\" GROUP BY \"tbl\",\"idx\"",
- "SELECT \"tbl\",\"idx\",\"neq\",\"nlt\",\"ndlt\",\"sample\" FROM \"_sql_stat4\"");
- }
-
- return rc;
+ /* _slq_stat4 is a system space, so it always exists. */
+ assert(pTab != NULL);
+ return loadStatTbl(db, pTab,
+ "SELECT \"tbl\",\"idx\",count(*) FROM \"_sql_stat4\""
+ " GROUP BY \"tbl\",\"idx\"",
+ "SELECT \"tbl\",\"idx\",\"neq\",\"nlt\",\"ndlt\","
+ "\"sample\" FROM \"_sql_stat4\"");
}
/*
diff --git a/src/box/sql/build.c b/src/box/sql/build.c
index 61194e06b..c50847a02 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -86,13 +86,6 @@ sqlite3FinishCoding(Parse * pParse)
|| sqlite3VdbeAssertMayAbort(v, pParse->mayAbort));
if (v) {
sqlite3VdbeAddOp0(v, OP_Halt);
-
- /* The cookie mask contains one bit for each database file open.
- * (Bit 0 is for main, bit 1 is for temp, and so forth.) Bits are
- * set for each database that is used. Generate code to start a
- * transaction on each used database and to verify the schema cookie
- * on each used database.
- */
if (db->mallocFailed == 0 || pParse->pConstExpr) {
int i;
assert(sqlite3VdbeGetOp(v, 0)->opcode == OP_Init);
@@ -192,7 +185,7 @@ sqlite3LocateTable(Parse * pParse, /* context in which to report errors */
assert(pParse->db->pSchema != NULL);
p = sqlite3HashFind(&pParse->db->pSchema->tblHash, zName);
- if (p == 0) {
+ if (p == NULL) {
const char *zMsg =
flags & LOCATE_VIEW ? "no such view" : "no such table";
if ((flags & LOCATE_NOERR) == 0) {
@@ -212,9 +205,8 @@ sqlite3LocateIndex(sqlite3 * db, const char *zName, const char *zTable)
Table *pTab = sqlite3HashFind(&db->pSchema->tblHash, zTable);
- if (pTab == 0) {
- return 0;
- }
+ if (pTab == NULL)
+ return NULL;
return sqlite3HashFind(&pTab->idxHash, zName);
}
@@ -545,7 +537,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
assert(db->pSchema != NULL);
pTable = sqlite3HashFind(&db->pSchema->tblHash, zName);
- if (pTable) {
+ if (pTable != NULL) {
if (!noErr) {
sqlite3ErrorMsg(pParse,
"table %s already exists",
@@ -583,7 +575,7 @@ sqlite3StartTable(Parse *pParse, Token *pName, int noErr)
* now.
*/
if (!db->init.busy && (v = sqlite3GetVdbe(pParse)) != 0)
- sql_set_multi_write(pParse, 1);
+ sql_set_multi_write(pParse, true);
/* Normal (non-error) return. */
return;
@@ -2303,7 +2295,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
if (noErr)
db->suppressErr--;
- if (pTab == 0)
+ if (pTab == NULL)
goto exit_drop_table;
#ifndef SQLITE_OMIT_VIEW
/* Ensure DROP TABLE is not used on a view, and DROP VIEW is not used
@@ -2334,7 +2326,7 @@ sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
* space_id from _space.
*/
- sql_set_multi_write(pParse, 1);
+ sql_set_multi_write(pParse, true);
sql_clear_stat_spaces(pParse, "tbl", pTab->zName);
sqlite3FkDropTable(pParse, pName, pTab);
sqlite3CodeDropTable(pParse, pTab, isView);
@@ -2841,14 +2833,15 @@ sqlite3CreateIndex(Parse * pParse, /* All information about this parse */
goto exit_create_index;
assert(pName->z != 0);
if (!db->init.busy) {
- if (sqlite3HashFind(&db->pSchema->tblHash, zName) != 0) {
+ if (sqlite3HashFind(&db->pSchema->tblHash, zName) !=
+ NULL) {
sqlite3ErrorMsg(pParse,
"there is already a table named %s",
zName);
goto exit_create_index;
}
}
- if (sqlite3HashFind(&pTab->idxHash, zName) != 0) {
+ if (sqlite3HashFind(&pTab->idxHash, zName) != NULL) {
if (!ifNotExist) {
sqlite3ErrorMsg(pParse,
"index %s.%s already exists",
@@ -3107,7 +3100,7 @@ sqlite3CreateIndex(Parse * pParse, /* All information about this parse */
if (v == 0)
goto exit_create_index;
- sql_set_multi_write(pParse, 1);
+ sql_set_multi_write(pParse, true);
sqlite3VdbeAddOp2(v, OP_SIDtoPtr, BOX_INDEX_ID,
@@ -3853,11 +3846,10 @@ sqlite3Savepoint(Parse * pParse, int op, Token * pName)
* execution multiple insertion/updates may occur.
*/
void
-sql_set_multi_write(Parse *pParse, int setStatement)
+sql_set_multi_write(struct Parse *parse_context, bool is_set)
{
- Parse *pToplevel = sqlite3ParseToplevel(pParse);
- DbMaskSet(pToplevel->writeMask, 0);
- pToplevel->isMultiWrite |= setStatement;
+ Parse *pToplevel = sqlite3ParseToplevel(parse_context);
+ pToplevel->isMultiWrite |= is_set;
}
/*
@@ -3974,7 +3966,7 @@ reindexTable(Parse * pParse, Table * pTab, char const *zColl)
for (pIndex = pTab->pIndex; pIndex; pIndex = pIndex->pNext) {
if (zColl == 0 || collationMatch(zColl, pIndex)) {
- sql_set_multi_write(pParse, 0);
+ sql_set_multi_write(pParse, false);
sqlite3RefillIndex(pParse, pIndex, -1);
}
}
@@ -4050,7 +4042,7 @@ sqlite3Reindex(Parse * pParse, Token * pName1, Token * pName2)
if (z == 0)
return;
pTab = sqlite3HashFind(&db->pSchema->tblHash, z);
- if (pTab) {
+ if (pTab != NULL) {
reindexTable(pParse, pTab, 0);
sqlite3DbFree(db, z);
return;
@@ -4066,8 +4058,8 @@ sqlite3Reindex(Parse * pParse, Token * pName1, Token * pName2)
}
pIndex = sqlite3HashFind(&pTab->idxHash, z);
- if (pIndex) {
- sql_set_multi_write(pParse, 0);
+ if (pIndex != NULL) {
+ sql_set_multi_write(pParse, false);
sqlite3RefillIndex(pParse, pIndex, -1);
return;
}
diff --git a/src/box/sql/delete.c b/src/box/sql/delete.c
index a07ef1980..9f4ce1026 100644
--- a/src/box/sql/delete.c
+++ b/src/box/sql/delete.c
@@ -59,12 +59,10 @@ sqlite3SrcListLookup(Parse * pParse, SrcList * pSrc)
pTab = sqlite3LocateTable(pParse, 0, pItem->zName);
sqlite3DeleteTable(pParse->db, pItem->pTab);
pItem->pTab = pTab;
- if (pTab) {
+ if (pTab != NULL)
pTab->nTabRef++;
- }
- if (sqlite3IndexedByLookup(pParse, pItem)) {
- pTab = 0;
- }
+ if (sqlite3IndexedByLookup(pParse, pItem))
+ pTab = NULL;
return pTab;
}
@@ -324,7 +322,7 @@ sqlite3DeleteFrom(Parse * pParse, /* The parser context */
}
if (pParse->nested == 0)
sqlite3VdbeCountChanges(v);
- sql_set_multi_write(pParse, 1);
+ sql_set_multi_write(pParse, true);
/* If we are trying to delete from a view, realize that view into
* an ephemeral table.
diff --git a/src/box/sql/insert.c b/src/box/sql/insert.c
index 2719bbaf4..704e48d9c 100644
--- a/src/box/sql/insert.c
+++ b/src/box/sql/insert.c
@@ -1461,7 +1461,7 @@ sqlite3GenerateConstraintChecks(Parse * pParse, /* The parser context */
default: {
Trigger *pTrigger = 0;
assert(onError == ON_CONFLICT_ACTION_REPLACE);
- sql_set_multi_write(pParse, 1);
+ sql_set_multi_write(pParse, true);
if (user_session->
sql_flags & SQLITE_RecTriggers) {
pTrigger =
@@ -1769,9 +1769,8 @@ xferOptimization(Parse * pParse, /* Parser context */
int regData, regTupleid; /* Registers holding data and tupleid */
struct session *user_session = current_session();
- if (pSelect == 0) {
+ if (pSelect == 0)
return 0; /* Must be of the form INSERT INTO ... SELECT ... */
- }
if (pParse->pWith || pSelect->pWith) {
/* Do not attempt to process this query if there are an WITH clauses
* attached to it. Proceeding may generate a false "no such table: xxx"
@@ -1833,7 +1832,7 @@ xferOptimization(Parse * pParse, /* Parser context */
*/
pItem = pSelect->pSrc->a;
pSrc = sqlite3LocateTable(pParse, 0, pItem->zName);
- if (pSrc == 0) {
+ if (pSrc == NULL) {
return 0; /* FROM clause does not contain a real table */
}
if (pSrc == pDest) {
diff --git a/src/box/sql/pragma.c b/src/box/sql/pragma.c
index 04b4020dd..c19a811ff 100644
--- a/src/box/sql/pragma.c
+++ b/src/box/sql/pragma.c
@@ -480,7 +480,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
int i;
pTab = sqlite3HashFind(&db->pSchema->tblHash,
zRight);
- if (pTab) {
+ if (pTab != NULL) {
pParse->nMem = 5;
for (pIdx = pTab->pIndex, i = 0; pIdx;
pIdx = pIdx->pNext, i++) {
@@ -540,7 +540,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
Table *pTab;
pTab = sqlite3HashFind(&db->pSchema->tblHash,
zRight);
- if (pTab) {
+ if (pTab != NULL) {
pFK = pTab->pFKey;
if (pFK) {
int i = 0;
@@ -617,7 +617,7 @@ sqlite3Pragma(Parse * pParse, Token * pId, /* First part of [schema.]id field */
pParent =
sqlite3HashFind(&db->pSchema->tblHash,
pFK->zTo);
- if (pParent == 0)
+ if (pParent == NULL)
continue;
pIdx = 0;
x = sqlite3FkLocateIndex(pParse,
diff --git a/src/box/sql/select.c b/src/box/sql/select.c
index 6ff8dc25e..679aa2246 100644
--- a/src/box/sql/select.c
+++ b/src/box/sql/select.c
@@ -4746,7 +4746,7 @@ selectExpander(Walker * pWalker, Select * p)
assert(pFrom->pTab == 0);
pFrom->pTab = pTab =
sqlite3LocateTable(pParse, 0, pFrom->zName);
- if (pTab == 0)
+ if (pTab == NULL)
return WRC_Abort;
if (pTab->nTabRef >= 0xffff) {
sqlite3ErrorMsg(pParse,
diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h
index 3234f992b..6d78f791d 100644
--- a/src/box/sql/sqliteInt.h
+++ b/src/box/sql/sqliteInt.h
@@ -2887,25 +2887,6 @@ struct TriggerPrg {
u32 aColmask[2]; /* Masks of old.*, new.* columns accessed */
};
-/*
- * The yDbMask datatype for the bitmask of all attached databases.
- */
-#if SQLITE_MAX_ATTACHED>30
-typedef unsigned char yDbMask[(SQLITE_MAX_ATTACHED + 9) / 8];
-#define DbMaskTest(M,I) (((M)[(I)/8]&(1<<((I)&7)))!=0)
-#define DbMaskZero(M) memset((M),0,sizeof(M))
-#define DbMaskSet(M,I) (M)[(I)/8]|=(1<<((I)&7))
-#define DbMaskAllZero(M) sqlite3DbMaskAllZero(M)
-#define DbMaskNonZero(M) (sqlite3DbMaskAllZero(M)==0)
-#else
-typedef unsigned int yDbMask;
-#define DbMaskTest(M,I) (((M)&(((yDbMask)1)<<(I)))!=0)
-#define DbMaskZero(M) (M)=0
-#define DbMaskSet(M,I) (M)|=(((yDbMask)1)<<(I))
-#define DbMaskAllZero(M) (M)==0
-#define DbMaskNonZero(M) (M)!=0
-#endif
-
/*
* An SQL parser context. A copy of this structure is passed through
* the parser and down into all the parser action routine in order to
@@ -2946,7 +2927,6 @@ struct Parse {
int *aLabel; /* Space to hold the labels */
ExprList *pConstExpr; /* Constant expressions */
Token constraintName; /* Name of the constraint currently being parsed */
- yDbMask writeMask; /* Start a write transaction on these databases */
int regRoot; /* Register holding root page number for new objects */
int nMaxArg; /* Max args passed to user function by sub-program */
#ifdef SELECTTRACE_ENABLED
@@ -3585,9 +3565,6 @@ int sqlite3ViewGetColumnNames(Parse *, Table *);
#define sqlite3ViewGetColumnNames(A,B) 0
#endif
-#if SQLITE_MAX_ATTACHED>30
-int sqlite3DbMaskAllZero(yDbMask);
-#endif
void sqlite3DropTable(Parse *, SrcList *, int, int);
void sqlite3DeleteTable(sqlite3 *, Table *);
void sqlite3Insert(Parse *, SrcList *, Select *, IdList *, int);
diff --git a/src/box/sql/trigger.c b/src/box/sql/trigger.c
index decbc8c21..a102c32b6 100644
--- a/src/box/sql/trigger.c
+++ b/src/box/sql/trigger.c
@@ -293,7 +293,7 @@ sqlite3FinishTrigger(Parse * pParse, /* Parser context */
iFirstCol = pParse->nMem + 1;
pParse->nMem += 2;
- sql_set_multi_write(pParse, 0);
+ sql_set_multi_write(pParse, false);
sqlite3VdbeAddOp4(v,
OP_String8, 0, iFirstCol, 0,
zName, P4_STATIC);
diff --git a/src/box/sql/update.c b/src/box/sql/update.c
index ad6aad5e6..778519bd6 100644
--- a/src/box/sql/update.c
+++ b/src/box/sql/update.c
@@ -288,7 +288,7 @@ sqlite3Update(Parse * pParse, /* The parser context */
goto update_cleanup;
if (pParse->nested == 0)
sqlite3VdbeCountChanges(v);
- sql_set_multi_write(pParse, 1);
+ sql_set_multi_write(pParse, true);
/* Allocate required registers. */
regOldPk = regNewPk = ++pParse->nMem;
[-- Attachment #2: Type: text/html, Size: 62506 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling
2018-04-03 18:27 ` [tarantool-patches] " Vladislav Shpilevoy
@ 2018-04-04 18:11 ` n.pettik
2018-04-05 11:20 ` Vladislav Shpilevoy
0 siblings, 1 reply; 12+ messages in thread
From: n.pettik @ 2018-04-04 18:11 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 11718 bytes --]
> On 3 Apr 2018, at 21:27, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>
> See 6 comments below.
>
> 03.04.2018 19:14, Nikita Pettik пишет:
>> As a part of SQL data dictionary integration, 'DROP INDEX' and
>> 'DROP TABLE' statements proccessing has been refactored in order
>> to avoid using SQL specific internal structures.
>> However, triggers still aren't transfered to server, so to drop them
>> it is required to lookup SQL table and emit apporpriate opcodes.
>> Also, added comments and fixed codestyle for functions processing
>> 'DROP' routine.
>>
>> Part of #3222.
>> ---
>> src/box/sql/build.c | 241 ++++++++++++++++++++++++++----------------------
>> src/box/sql/parse.c | 6 +-
>> src/box/sql/parse.y | 6 +-
>> src/box/sql/sqliteInt.h | 6 +-
>> 4 files changed, 140 insertions(+), 119 deletions(-)
>>
>> diff --git a/src/box/sql/build.c b/src/box/sql/build.c
>> index 61194e06b..219cc974b 100644
>> --- a/src/box/sql/build.c
>> +++ b/src/box/sql/build.c
>> @@ -46,6 +46,7 @@
>> #include "sqliteInt.h"
>> #include "vdbeInt.h"
>> #include "tarantoolInt.h"
>> +#include "box/box.h"
>> #include "box/sequence.h"
>> #include "box/session.h"
>> #include "box/identifier.h"
>> @@ -2152,48 +2153,50 @@ sql_clear_stat_spaces(Parse * pParse, const char *zType, const char *zName)
>> zType, zName);
>> }
>> -/*
>> +/**
>> * Generate code to drop a table.
>> + * This routine includes dropping triggers, sequences,
>> + * all indexes and entry from _space space.
>> + *
>> + * @param parse_context Current parsing context.
>> + * @param space Space to be dropped.
>> + * @param is_view True, if space is
>> */
>> static void
>> -sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
>> +sql_code_drop_table(struct Parse *parse_context, struct space *space,
>> + bool is_view)
>> {
>> - Vdbe *v;
>> - sqlite3 *db = pParse->db;
>> - Trigger *pTrigger;
>> -
>> - v = sqlite3GetVdbe(pParse);
>>
>> + struct sqlite3 *db = parse_context->db;
>> + struct Vdbe *v = sqlite3GetVdbe(parse_context);
>> assert(v != 0);
> 1. Lets in all new code use explicit ==/!= NULL.
Fixed:
- assert(v != 0);
+ assert(v != NULL);
>> /*
>> * Drop all triggers associated with the table being
>> * dropped. Code is generated to remove entries from
>> * _trigger. OP_DropTrigger will remove it from internal
>> * SQL structures.
>> - */
>> - pTrigger = pTab->pTrigger;
>> - /* Do not account triggers deletion - they will be accounted
>> + *
>> + * Do not account triggers deletion - they will be accounted
> 2. Out of 66.
Fixed:
- * Do not account triggers deletion - they will be accounted
- * in DELETE from _space below.
+ * Do not account triggers deletion - they will be
+ * accounted in DELETE from _space below.
>> @@ -2214,7 +2216,7 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
>> * Drop all _space and _index entries that refer to the
>> * table.
>> */
>> - if (!isView) {
>> + if (!is_view) {
> 3. I looked at the indexes deletion more carefully and noticed, that you can avoid any allocations
> here. Why can not you just walk through space->index array and generate OP_SDelete opcodes for
> each index, starting from 1 (to skip primary)? AFAIK the entire index array is not changed until you
> reach VdbeExec, so you can do not save index identifiers on region to generate opcodes.
IDK why I didn’t do it in this way (probably due to my ‘paranoia’).
Reworked:
- uint32_t *iids =
- (uint32_t *) region_alloc(&fiber()->gc,
- sizeof(uint32_t) *
- (index_count - 1));
- /* Save index ids to be deleted except for PK. */
for (uint32_t i = 1; i < index_count; ++i) {
- iids[i - 1] = space->index[i]->def->iid;
- }
- for (uint32_t i = 0; i < index_count - 1; ++i) {
- sqlite3VdbeAddOp2(v, OP_Integer, iids[i],
+ sqlite3VdbeAddOp2(v, OP_Integer,
+ space->index[i]->def->iid,
space_id_reg + 1);
sqlite3VdbeAddOp3(v, OP_MakeRecord,
space_id_reg, 2, idx_rec_reg);
@@ -2233,7 +2226,7 @@ sql_code_drop_table(struct Parse *parse_context, struct space *space,
idx_rec_reg);
VdbeComment((v,
"Remove secondary index iid = %u",
- iids[i]));
+ space->index[i]->def->iid));
>> uint32_t index_count = space->index_count;
>> if (index_count > 1) {
>> /*
>> @@ -2257,71 +2259,78 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int isView)
>> sqlite3VdbeAddOp2(v, OP_SDelete, BOX_SPACE_ID, idx_rec_reg);
>> sqlite3VdbeChangeP5(v, OPFLAG_NCHANGE);
>> VdbeComment((v, "Delete entry from _space"));
>> - /* Remove the table entry from SQLite's internal schema and modify
>> - * the schema cookie.
>> + /*
>> + * Remove the table entry from SQLite's internal schema
>> + * and modify the schema cookie.
>> */
> 4. But you have deleted cookie updates in the previous commit. Source code of OP_DropTable and all
> called there functions does not contain cookie changes.
Removed obsolete comment:
- /*
- * Remove the table entry from SQLite's internal schema
- * and modify the schema cookie.
- */
+ /* Remove the table entry from SQLite's internal schema. */
>> + sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, space->def->name, 0);
>> - sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, pTab->zName, 0);
>> -
>> - /* FIXME: DDL is impossible inside a transaction so far.
>> + /*
>> * Replace to _space/_index will fail if active
>> * transaction. So, don't pretend, that we are able to
>> * anything back. To be fixed when transactions
>> * DDL are enabled.
>> */
>> - /* sqlite3ChangeCookie(pParse); */
>> sqliteViewResetAll(db);
>> }
>> -/*
>> +/**
>> * This routine is called to do the work of a DROP TABLE statement.
>> - * pName is the name of the table to be dropped.
>> + *
>> + * @param parse_context Current parsing context.
>> + * @param table_name_list List containing table name.
>> + * @param is_view True, if statement is really 'DROP VIEW'.
>> + * @param if_exists True, if statement contains 'IF EXISTS' clause.
>> */
>> void
>> -sqlite3DropTable(Parse * pParse, SrcList * pName, int isView, int noErr)
>> +sql_drop_table(struct Parse *parse_context, struct SrcList *table_name_list,
>> + bool is_view, bool if_exists)
>> {
>> - Table *pTab;
>> - Vdbe *v = sqlite3GetVdbe(pParse);
>> - sqlite3 *db = pParse->db;
>> -
>> + struct Vdbe *v = sqlite3GetVdbe(parse_context);
>> + struct sqlite3 *db = parse_context->db;
>> if (v == NULL || db->mallocFailed) {
>> goto exit_drop_table;
>> }
>> - /* Activate changes counting here to account
>> + /*
>> + * Activate changes counting here to account
>> * DROP TABLE IF NOT EXISTS, if the table really does not
>> * exist.
>> */
>> - if (!pParse->nested)
>> + if (!parse_context->nested)
> 5. Please, use explicit != 0. Why you use !int, the variable seems to be boolean. It slightly confuses.
This this not != check, but vice versa == 0 check.
! In this case would better fit, wouldn’t it?
(Since any number except for 0 would be converted to false)
>> sqlite3VdbeCountChanges(v);
>> - assert(pParse->nErr == 0);
>> - assert(pName->nSrc == 1);
>> - assert(db->pSchema != NULL);
>> - if (noErr)
>> - db->suppressErr++;
>> - assert(isView == 0 || isView == LOCATE_VIEW);
>> - pTab = sqlite3LocateTable(pParse, isView, pName->a[0].zName);
>> - if (noErr)
>> - db->suppressErr--;
>> -
>> - if (pTab == 0)
>> + assert(parse_context->nErr == 0);
>> + assert(table_name_list->nSrc == 1);
>> + assert(is_view == 0 || is_view == LOCATE_VIEW);
>> + const char *space_name = table_name_list->a[0].zName;
>> + uint32_t space_id = box_space_id_by_name(space_name,
>> + strlen(space_name));
>> + if (space_id == BOX_ID_NIL) {
>> + if (!is_view && !if_exists)
>> + sqlite3ErrorMsg(parse_context, "no such table: %s",
>> + space_name);
>> + if (is_view && !if_exists)
>> + sqlite3ErrorMsg(parse_context, "no such view: %s",
>> + space_name);
>> goto exit_drop_table;
>> -#ifndef SQLITE_OMIT_VIEW
>> - /* Ensure DROP TABLE is not used on a view, and DROP VIEW is not used
>> - * on a table.
>> + }
>> + struct space *space = space_by_id(space_id);
>> + assert(space != NULL);
>> + /*
>> + * Ensure DROP TABLE is not used on a view,
>> + * and DROP VIEW is not used on a table.
>> */
>> - if (isView && !space_is_view(pTab)) {
>> - sqlite3ErrorMsg(pParse, "use DROP TABLE to delete table %s",
>> - pTab->zName);
>> + if (is_view && !space->def->opts.is_view) {
>> + sqlite3ErrorMsg(parse_context, "use DROP TABLE to delete table %s",
>> + space_name);
>> goto exit_drop_table;
>> }
>> - if (!isView && space_is_view(pTab)) {
>> - sqlite3ErrorMsg(pParse, "use DROP VIEW to delete view %s",
>> - pTab->zName);
>> + if (!is_view && space->def->opts.is_view) {
>> + sqlite3ErrorMsg(parse_context, "use DROP VIEW to delete view %s",
>> + space_name);
>> goto exit_drop_table;
>> }
>> -#endif
>> -
>> - /* Generate code to remove the table from Tarantool and internal SQL
>> + /*
>> + * Generate code to remove the table from Tarantool and internal SQL
> 6. Out of 66.
Fixed whole comment (to fit into 66 chars):
- * Generate code to remove the table from Tarantool and internal SQL
- * tables. Basically, it consists from 3 stages:
- * 1. Delete statistics from _stat1 and _stat4 tables (if any exist)
- * 2. In case of presence of FK constraints, i.e. current table is child
- * or parent, then start new transaction and erase from table
- * all data row by row. On each deletion check whether any FK
- * violations have occurred. If ones take place, then rollback
- * transaction and halt VDBE. Otherwise, commit transaction.
- * 3. Drop table by truncating (if step 2 was skipped), removing
- * indexes from _index table and eventually tuple with corresponding
- * space_id from _space.
+ * Generate code to remove the table from Tarantool
+ * and internal SQL tables. Basically, it consists
+ * from 3 stages:
+ * 1. Delete statistics from _stat1 and _stat4 tables.
+ * 2. In case of presence of FK constraints, i.e. current
+ * table is child or parent, then start new transaction
+ * and erase from table all data row by row. On each
+ * deletion check whether any FK violations have
+ * occurred. If ones take place, then rollback
+ * transaction and halt VDBE.
+ * 3. Drop table by truncating (if step 2 was skipped),
+ * removing indexes from _index space and eventually
+ * tuple with corresponding space_id from _space.
*/
[-- Attachment #2: Type: text/html, Size: 39685 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: remove obsolete SQLite routine
2018-04-04 18:09 ` n.pettik
@ 2018-04-05 11:16 ` Vladislav Shpilevoy
2018-04-05 12:13 ` n.pettik
0 siblings, 1 reply; 12+ messages in thread
From: Vladislav Shpilevoy @ 2018-04-05 11:16 UTC (permalink / raw)
To: n.pettik, tarantool-patches
>> Same about checking sqlite3HashFind results. And if it is
>> possible with not huge diff, can you please rename sqlite3HashFind to
>> sql_hash_find ?
>
> Is it worth doing? It is going to disappear soon.
Ok, then lets leave as is.
> @@ -1146,7 +1147,7 @@ analyzeTable(Parse * pParse, Table * pTab, Index
> * pOnlyIdx)
> int iStatCur;
> assert(pTab != 0);
> -sql_set_multi_write(pParse, 0);
> +sql_set_multi_write(pParse, true);
1. Typo? 0 != true.
> @@ -1769,9 +1769,8 @@ xferOptimization(Parse * pParse,/* Parser context */
> int regData, regTupleid;/* Registers holding data and tupleid */
> struct session *user_session = current_session();
> -if (pSelect == 0) {
> +if (pSelect == 0)
2. == NULL.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling
2018-04-04 18:11 ` n.pettik
@ 2018-04-05 11:20 ` Vladislav Shpilevoy
0 siblings, 0 replies; 12+ messages in thread
From: Vladislav Shpilevoy @ 2018-04-05 11:20 UTC (permalink / raw)
To: tarantool-patches, n.pettik
LGTM.
04.04.2018 21:11, n.pettik пишет:
>
>> On 3 Apr 2018, at 21:27, Vladislav Shpilevoy
>> <v.shpilevoy@tarantool.org <mailto:v.shpilevoy@tarantool.org>> wrote:
>>
>> See 6 comments below.
>>
>> 03.04.2018 19:14, Nikita Pettik пишет:
>>> As a part of SQL data dictionary integration, 'DROP INDEX' and
>>> 'DROP TABLE' statements proccessing has been refactored in order
>>> to avoid using SQL specific internal structures.
>>> However, triggers still aren't transfered to server, so to drop them
>>> it is required to lookup SQL table and emit apporpriate opcodes.
>>> Also, added comments and fixed codestyle for functions processing
>>> 'DROP' routine.
>>>
>>> Part of #3222.
>>> ---
>>> src/box/sql/build.c | 241
>>> ++++++++++++++++++++++++++----------------------
>>> src/box/sql/parse.c | 6 +-
>>> src/box/sql/parse.y | 6 +-
>>> src/box/sql/sqliteInt.h | 6 +-
>>> 4 files changed, 140 insertions(+), 119 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: remove obsolete SQLite routine
2018-04-05 11:16 ` Vladislav Shpilevoy
@ 2018-04-05 12:13 ` n.pettik
2018-04-05 13:28 ` Vladislav Shpilevoy
0 siblings, 1 reply; 12+ messages in thread
From: n.pettik @ 2018-04-05 12:13 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
> On 5 Apr 2018, at 14:16, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>
>
>>> Same about checking sqlite3HashFind results. And if it is
>>> possible with not huge diff, can you please rename sqlite3HashFind to sql_hash_find ?
>>
>> Is it worth doing? It is going to disappear soon.
> Ok, then lets leave as is.
>> @@ -1146,7 +1147,7 @@ analyzeTable(Parse * pParse, Table * pTab, Index * pOnlyIdx)
>> int iStatCur;
>> assert(pTab != 0);
>> -sql_set_multi_write(pParse, 0);
>> +sql_set_multi_write(pParse, true);
> 1. Typo? 0 != true.
Typo. Fixed on branch:
- sql_set_multi_write(pParse, true);
+ sql_set_multi_write(pParse, false);
>> @@ -1769,9 +1769,8 @@ xferOptimization(Parse * pParse,/* Parser context */
>> int regData, regTupleid;/* Registers holding data and tupleid */
>> struct session *user_session = current_session();
>> -if (pSelect == 0) {
>> +if (pSelect == 0)
> 2. == NULL.
Typo. Fixed on branch:
- if (pSelect == 0)
+ if (pSelect == NULL)
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 1/2] sql: remove obsolete SQLite routine
2018-04-05 12:13 ` n.pettik
@ 2018-04-05 13:28 ` Vladislav Shpilevoy
0 siblings, 0 replies; 12+ messages in thread
From: Vladislav Shpilevoy @ 2018-04-05 13:28 UTC (permalink / raw)
To: tarantool-patches, n.pettik
Now the entire patchset LGTM.
05.04.2018 15:13, n.pettik пишет:
>> On 5 Apr 2018, at 14:16, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>>
>>>> Same about checking sqlite3HashFind results. And if it is
>>>> possible with not huge diff, can you please rename sqlite3HashFind to sql_hash_find ?
>>> Is it worth doing? It is going to disappear soon.
>> Ok, then lets leave as is.
>>> @@ -1146,7 +1147,7 @@ analyzeTable(Parse * pParse, Table * pTab, Index * pOnlyIdx)
>>> int iStatCur;
>>> assert(pTab != 0);
>>> -sql_set_multi_write(pParse, 0);
>>> +sql_set_multi_write(pParse, true);
>> 1. Typo? 0 != true.
> Typo. Fixed on branch:
>
> - sql_set_multi_write(pParse, true);
> + sql_set_multi_write(pParse, false);
>
>>> @@ -1769,9 +1769,8 @@ xferOptimization(Parse * pParse,/* Parser context */
>>> int regData, regTupleid;/* Registers holding data and tupleid */
>>> struct session *user_session = current_session();
>>> -if (pSelect == 0) {
>>> +if (pSelect == 0)
>> 2. == NULL.
> Typo. Fixed on branch:
>
> - if (pSelect == 0)
> + if (pSelect == NULL)
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [tarantool-patches] Re: [PATCH 0/2] rework SQL 'DROP' routine
2018-04-03 16:14 [tarantool-patches] [PATCH 0/2] rework SQL 'DROP' routine Nikita Pettik
2018-04-03 16:14 ` [tarantool-patches] [PATCH 1/2] sql: remove obsolete SQLite routine Nikita Pettik
2018-04-03 16:14 ` [tarantool-patches] [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling Nikita Pettik
@ 2018-04-05 14:12 ` Kirill Yukhin
2 siblings, 0 replies; 12+ messages in thread
From: Kirill Yukhin @ 2018-04-05 14:12 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy, Nikita Pettik
Hello Nikita,
On 03 апр 19:14, Nikita Pettik wrote:
> Branch: https://github.com/tarantool/tarantool/tree/np/gh-3222-rework-drop
> Issue: https://github.com/tarantool/tarantool/issues/3222
>
> This small patch-set implements part of #3222 issue: rewrite functions
> implementing drop table and drop index in order to avoid using
> struct Table and its lookup in SQLite data-dictionary.
> However, as far as currently triggers aren't part of Tarantool's spaces,
> it is still required to use struct Table to drop them:
> triggers are held in _trigger space, which format is: ['name', 'sql'],
> i.e. there is no field for name of parent table. Hence, we are able
> to point out table's triggers only via accessing struct Table.
>
> Both patches are mainly about refactoring.
> First patch removes useless wrappers, obsolete SQLite specific functions
> and struct fields.
> Second one replaces usages of struct Table with struct space in drop
> routine, fixes codestyle, provides doxygen style comments.
>
> Nikita Pettik (2):
> sql: remove obsolete SQLite routine
> sql: rework 'DROP INDEX' and 'DROP TABLE' handling
Patchset looks perfect now, I've checked it into 2.0.
--
Regards, Kirill Yukhin
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2018-04-05 14:12 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-03 16:14 [tarantool-patches] [PATCH 0/2] rework SQL 'DROP' routine Nikita Pettik
2018-04-03 16:14 ` [tarantool-patches] [PATCH 1/2] sql: remove obsolete SQLite routine Nikita Pettik
2018-04-03 17:54 ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-04 18:09 ` n.pettik
2018-04-05 11:16 ` Vladislav Shpilevoy
2018-04-05 12:13 ` n.pettik
2018-04-05 13:28 ` Vladislav Shpilevoy
2018-04-03 16:14 ` [tarantool-patches] [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling Nikita Pettik
2018-04-03 18:27 ` [tarantool-patches] " Vladislav Shpilevoy
2018-04-04 18:11 ` n.pettik
2018-04-05 11:20 ` Vladislav Shpilevoy
2018-04-05 14:12 ` [tarantool-patches] Re: [PATCH 0/2] rework SQL 'DROP' routine Kirill Yukhin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox