From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id CECA32BDF9 for ; Tue, 3 Apr 2018 12:15:26 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id eqh2YvuiniJv for ; Tue, 3 Apr 2018 12:15:26 -0400 (EDT) Received: from smtp48.i.mail.ru (smtp48.i.mail.ru [94.100.177.108]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 5B5B42BDC2 for ; Tue, 3 Apr 2018 12:15:26 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 2/2] sql: rework 'DROP INDEX' and 'DROP TABLE' handling Date: Tue, 3 Apr 2018 19:14:57 +0300 Message-Id: <2bccf40f38d3e3e3d2182c6c8111050770d69821.1522769126.git.korablev@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, 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