[tarantool-patches] Re: [PATCH 4/7] sql: refactor ALTER RENAME code generation
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Aug 29 03:58:16 MSK 2018
Thanks for the patch! See 2 comments below.
On 23/08/2018 19:55, Nikita Pettik wrote:
> 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));
1. ER_SPACE_EXISTS
> + 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;
2. ER_ALTER_SPACE or ER_UNSUPPORTED, on your choice.
> }
> - 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;
> }
>
> /*
More information about the Tarantool-patches
mailing list