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 28CFF2A4A1 for ; Thu, 23 Aug 2018 18:56:15 -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 3qLMHsgZkIJP for ; Thu, 23 Aug 2018 18:56:15 -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 CFA842A330 for ; Thu, 23 Aug 2018 18:56:14 -0400 (EDT) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 4/7] sql: refactor ALTER RENAME code generation Date: Fri, 24 Aug 2018 01:55:50 +0300 Message-Id: <793300224e2f072e2559566577d0fd7e56740295.1535064700.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 We are going to remove legacy cache of struct Table, so lets get rid of using SQLite tables in alter routine. Part of #3561 --- src/box/sql/alter.c | 114 ++++++++++++++------------------------------ src/box/sql/parse.y | 2 +- src/box/sql/sqliteInt.h | 13 ++++- src/box/sql/vdbe.c | 5 +- test/sql-tap/alter.test.lua | 4 +- 5 files changed, 53 insertions(+), 85 deletions(-) diff --git a/src/box/sql/alter.c b/src/box/sql/alter.c index 349589be4..320dfdd05 100644 --- a/src/box/sql/alter.c +++ b/src/box/sql/alter.c @@ -34,91 +34,51 @@ * that implements the ALTER TABLE command. */ #include "sqliteInt.h" -#include "src/box/session.h" -#include "tarantoolInt.h" +#include "box/box.h" +#include "box/schema.h" -/* - * Generate code to drop and reload the internal representation of table - * pTab from the database, including triggers. - * Argument zName is the name of the table in the database schema at - * the time the generated code is executed. This can be different from - * pTab->zName if this function is being called to code part of an - * "ALTER TABLE RENAME TO" statement. - */ -static void -reloadTableSchema(Parse * pParse, Table * pTab, const char *zName) -{ - Vdbe *v; - v = sqlite3GetVdbe(pParse); - if (NEVER(v == 0)) - return; - - char *zNewName = sqlite3MPrintf(pParse->db, "%s", zName); - sqlite3VdbeAddOp4(v, OP_RenameTable, pTab->def->id, 0, 0, zNewName, - P4_DYNAMIC); -} - -/* - * Generate code to implement the "ALTER TABLE xxx RENAME TO yyy" - * command. - */ void -sqlite3AlterRenameTable(Parse * pParse, /* Parser context. */ - SrcList * pSrc, /* The table to rename. */ - Token * pName) /* The new table name. */ +sql_alter_table_rename(struct Parse *parse, struct SrcList *src_tab, + struct Token *new_name_tk) { - Table *pTab; /* Table being renamed */ - char *zName = 0; /* NULL-terminated version of pName */ - sqlite3 *db = pParse->db; /* Database connection */ - Vdbe *v; - uint32_t savedDbFlags; /* Saved value of db->flags */ - struct session *user_session = current_session(); - - savedDbFlags = user_session->sql_flags; - - if (NEVER(db->mallocFailed)) - goto exit_rename_table; - assert(pSrc->nSrc == 1); - - pTab = sqlite3LocateTable(pParse, 0, pSrc->a[0].zName); - if (pTab == NULL) - goto exit_rename_table; - - user_session->sql_flags |= SQLITE_PreferBuiltin; - - /* Get a NULL terminated version of the new table name. */ - zName = sqlite3NameFromToken(db, pName); - if (!zName) - goto exit_rename_table; - - /* Check that a table named 'zName' does not already exist - * in database. If so, this is an error. - */ - if (sqlite3HashFind(&db->pSchema->tblHash, zName) != NULL) { - sqlite3ErrorMsg(pParse, - "there is already another table or index with this name: %s", - zName); + assert(src_tab->nSrc == 1); + struct sqlite3 *db = parse->db; + char *new_name = sqlite3NameFromToken(db, new_name_tk); + if (new_name == NULL) goto exit_rename_table; + /* Check that new name isn't occupied by another table. */ + uint32_t space_id = box_space_id_by_name(new_name, strlen(new_name)); + if (space_id != BOX_ID_NIL) { + diag_set(ClientError, ER_SQL_EXECUTE, tt_sprintf("there is " + "already another table with this name: %s", new_name)); + goto tnt_error; } - if (pTab->def->opts.is_view) { - sqlite3ErrorMsg(pParse, "view %s may not be altered", - pTab->def->name); - goto exit_rename_table; + const char *tbl_name = src_tab->a[0].zName; + space_id = box_space_id_by_name(tbl_name, strlen(tbl_name)); + if (space_id == BOX_ID_NIL) { + diag_set(ClientError, ER_NO_SUCH_SPACE, tbl_name); + goto tnt_error; } - /* Begin a transaction for database. */ - v = sqlite3GetVdbe(pParse); - if (v == 0) { - goto exit_rename_table; + struct space *space = space_by_id(space_id); + assert(space != NULL); + if (space->def->opts.is_view) { + diag_set(ClientError, ER_SQL_EXECUTE, + "view may not be altered"); + goto tnt_error; } - sql_set_multi_write(pParse, false); - + sql_set_multi_write(parse, false); /* Drop and reload the internal table schema. */ - reloadTableSchema(pParse, pTab, zName); - - exit_rename_table: - sqlite3SrcListDelete(db, pSrc); - sqlite3DbFree(db, zName); - user_session->sql_flags = savedDbFlags; + struct Vdbe *v = sqlite3GetVdbe(parse); + sqlite3VdbeAddOp4(v, OP_RenameTable, space_id, 0, 0, new_name, + P4_DYNAMIC); +exit_rename_table: + sqlite3SrcListDelete(db, src_tab); + return; +tnt_error: + sqlite3DbFree(db, new_name); + parse->rc = SQL_TARANTOOL_ERROR; + parse->nErr++; + goto exit_rename_table; } /* diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y index 60c0a8eed..92ce5a5bc 100644 --- a/src/box/sql/parse.y +++ b/src/box/sql/parse.y @@ -1432,7 +1432,7 @@ cmd ::= ANALYZE nm(X). {sqlite3Analyze(pParse, &X);} //////////////////////// ALTER TABLE table ... //////////////////////////////// cmd ::= ALTER TABLE fullname(X) RENAME TO nm(Z). { - sqlite3AlterRenameTable(pParse,X,&Z); + sql_alter_table_rename(pParse,X,&Z); } cmd ::= ALTER TABLE fullname(X) ADD CONSTRAINT nm(Z) FOREIGN KEY diff --git a/src/box/sql/sqliteInt.h b/src/box/sql/sqliteInt.h index 96e2fff9f..3ae8db1bc 100644 --- a/src/box/sql/sqliteInt.h +++ b/src/box/sql/sqliteInt.h @@ -4427,7 +4427,18 @@ extern int sqlite3PendingByte; #endif #endif void sqlite3Reindex(Parse *, Token *, Token *); -void sqlite3AlterRenameTable(Parse *, SrcList *, Token *); + +/** + * Generate code to implement the "ALTER TABLE xxx RENAME TO yyy" + * command. + * + * @param parse Current parsing context. + * @param src_tab The table to rename. + * @param new_name_tk Token containing new name of the table. + */ +void +sql_alter_table_rename(struct Parse *parse, struct SrcList *src_tab, + struct Token *new_name_tk); /** * Return the length (in bytes) of the token that begins at z[0]. diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index d54d169ee..ad251ca99 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -4599,7 +4599,6 @@ case OP_RenameTable: { struct space *space; const char *zOldTableName; const char *zNewTableName; - Table *pTab; struct init_data init; char *zSqlStmt; @@ -4610,15 +4609,13 @@ case OP_RenameTable: { struct sql_trigger *triggers = space->sql_triggers; zOldTableName = space_name(space); assert(zOldTableName); - pTab = sqlite3HashFind(&db->pSchema->tblHash, zOldTableName); - assert(pTab); zNewTableName = pOp->p4.z; zOldTableName = sqlite3DbStrNDup(db, zOldTableName, sqlite3Strlen30(zOldTableName)); rc = sql_rename_table(space_id, zNewTableName, &zSqlStmt); if (rc) goto abort_due_to_error; - sqlite3UnlinkAndDeleteTable(db, pTab->def->name); + sqlite3UnlinkAndDeleteTable(db, space->def->name); init.db = db; init.pzErrMsg = &p->zErrMsg; diff --git a/test/sql-tap/alter.test.lua b/test/sql-tap/alter.test.lua index db87c7003..ffd0ba6dc 100755 --- a/test/sql-tap/alter.test.lua +++ b/test/sql-tap/alter.test.lua @@ -72,7 +72,7 @@ test:do_catchsql_test( ALTER TABLE none RENAME TO hi; ]], { -- - 1, "no such table: NONE" + 1, "Space 'NONE' does not exist" -- }) @@ -83,7 +83,7 @@ test:do_catchsql_test( ALTER TABLE t2 RENAME TO t3; ]], { -- - 1, "there is already another table or index with this name: T3" + 1, "Failed to execute SQL statement: there is already another table with this name: T3" -- }) -- 2.15.1