On 3 Apr 2018, at 21:27, Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>
wrote:
See 6 comments below.
03.04.2018 19:14, Nikita Pettik пишет:
As a part of SQL data dictionary integration, 'DROP INDEX' and1. Lets in all new code use explicit ==/!= NULL.
'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);
/*2. Out of 66.
* 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
@@ -2214,7 +2216,7 @@ sqlite3CodeDropTable(Parse * pParse, Table * pTab, int3. I looked at the indexes deletion more carefully and noticed, that you can
isView)
* Drop all _space and _index entries that refer to the
* table.
*/
- if (!isView) {
+ if (!is_view) {
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;4. But you have deleted cookie updates in the previous commit. Source code of
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.
*/
OP_DropTable and all
called there functions does not contain cookie changes.
+ sqlite3VdbeAddOp4(v, OP_DropTable, 0, 0, 0, space->def->name, 0);5. Please, use explicit != 0. Why you use !int, the variable seems to be
- 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)
boolean. It slightly confuses.
sqlite3VdbeCountChanges(v);6. Out of 66.
- 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
