From: Kirill Yukhin <kyukhin@tarantool.org> To: v.shpilevoy@tarantool.org Cc: tarantool-patches@freelists.org, Kirill Yukhin <kyukhin@tarantool.org> Subject: [tarantool-patches] [PATCH] sql: after table rename properly update indexes Date: Mon, 13 Aug 2018 07:54:19 +0300 [thread overview] Message-ID: <b9f53b11c907e1e076efbf223b822ad4c722a4ed.1534135985.git.kyukhin@tarantool.org> (raw) After table was altered due to rename, indexes were not updated. Re-create all table's indexes (in legacy SQL DD), also perform udate of corresponding entry in _index for each index to fix table name in create statement. Closes #3613 --- src/box/sql.c | 55 +++++++++++++++++++++++++++--- src/box/sql/build.c | 6 ++-- src/box/sql/tarantoolInt.h | 25 ++++++++++++-- src/box/sql/vdbe.c | 27 +++++++++++++-- test/sql/gh-3613-idx-alter-update.result | 37 ++++++++++++++++++++ test/sql/gh-3613-idx-alter-update.test.lua | 21 ++++++++++++ 6 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 test/sql/gh-3613-idx-alter-update.result create mode 100644 test/sql/gh-3613-idx-alter-update.test.lua diff --git a/src/box/sql.c b/src/box/sql.c index ae12cae..e2d3cc1 100644 --- a/src/box/sql.c +++ b/src/box/sql.c @@ -840,6 +840,55 @@ rename_fail: return SQL_TARANTOOL_ERROR; } +int +sql_update_index_table_name(uint32_t space_id, uint32_t iid, + const char *new_tbl_name, char **sql_stmt) +{ + assert(space_id != 0); + assert(new_tbl_name != NULL); + struct space *space = space_cache_find(space_id); + assert(space != NULL); + assert(iid < space->index_count); + struct index_def *def = space->index[iid]->def; + + bool is_quoted = false; + *sql_stmt = rename_table(db, *sql_stmt, new_tbl_name, &is_quoted); + + uint32_t key_len = mp_sizeof_uint(space_id) + mp_sizeof_uint(iid) + + mp_sizeof_array(2); + uint32_t new_opts_sz = tarantoolSqlite3MakeIdxOpts(def->opts.is_unique, + *sql_stmt, NULL); + uint32_t op_sz = mp_sizeof_array(1) + mp_sizeof_array(3) + + mp_sizeof_str(1) + mp_sizeof_uint(4) + new_opts_sz; + + char *key_begin = (char*) region_alloc(&fiber()->gc, key_len + op_sz); + if (key_begin == NULL) { + diag_set(OutOfMemory, key_len, "region_alloc", "key_begin"); + return SQL_TARANTOOL_ERROR; + } + char *key = mp_encode_array(key_begin, 2); + key = mp_encode_uint(key, space_id); + key = mp_encode_uint(key, iid); + + char *op_begin = key; + char *op = mp_encode_array(op_begin, 1); + op = mp_encode_array(op, 3); + op = mp_encode_str(op, "=", 1); + op = mp_encode_uint(op, 4); + op += tarantoolSqlite3MakeIdxOpts(def->opts.is_unique, *sql_stmt, op); + + box_tuple_t *res = NULL; + + if (box_update(BOX_INDEX_ID, 0, key_begin, key, op_begin, op, + 0, &res) != 0) { + diag_set(ClientError, ER_UPDATE_FIELD, 4, + "while updating SQL field"); + return SQL_TARANTOOL_ERROR; + } + + return SQLITE_OK; +} + int tarantoolSqlite3IdxKeyCompare(struct BtCursor *cursor, struct UnpackedRecord *unpacked) @@ -1494,13 +1543,11 @@ int tarantoolSqlite3MakeIdxParts(SqliteIndex *pIndex, void *buf) * "sql": "CREATE INDEX student_by_name ON students(name)" * } */ -int tarantoolSqlite3MakeIdxOpts(SqliteIndex *index, const char *zSql, void *buf) +int tarantoolSqlite3MakeIdxOpts(bool is_unique, const char *zSql, void *buf) { const struct Enc *enc = get_enc(buf); char *base = buf, *p; - (void)index; - p = enc->encode_map(base, 2); /* Mark as unique pk and unique indexes */ p = enc->encode_str(p, "unique", 6); @@ -1511,7 +1558,7 @@ int tarantoolSqlite3MakeIdxOpts(SqliteIndex *index, const char *zSql, void *buf) * INSERT OR REPLACE/IGNORE uniqueness checks will be also done by * Tarantool. */ - p = enc->encode_bool(p, IsUniqueIndex(index)); + p = enc->encode_bool(p, is_unique); p = enc->encode_str(p, "sql", 3); p = enc->encode_str(p, zSql, zSql ? strlen(zSql) : 0); return (int)(p - base); diff --git a/src/box/sql/build.c b/src/box/sql/build.c index cdf2bfc..6247f18 100644 --- a/src/box/sql/build.c +++ b/src/box/sql/build.c @@ -1245,13 +1245,15 @@ createIndex(Parse * pParse, Index * pIndex, int iSpaceId, int iIndexId, /* Format "opts" and "parts" for _index entry. */ zOpts = sqlite3DbMallocRaw(pParse->db, - tarantoolSqlite3MakeIdxOpts(pIndex, zSql, + tarantoolSqlite3MakeIdxOpts(IsUniqueIndex(pIndex), + zSql, NULL) + tarantoolSqlite3MakeIdxParts(pIndex, NULL) + 2); if (!zOpts) return; - zOptsSz = tarantoolSqlite3MakeIdxOpts(pIndex, zSql, zOpts); + zOptsSz = tarantoolSqlite3MakeIdxOpts(IsUniqueIndex(pIndex), zSql, + zOpts); zParts = zOpts + zOptsSz + 1; zPartsSz = tarantoolSqlite3MakeIdxParts(pIndex, zParts); #if SQLITE_DEBUG diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h index 94517f6..560acf1 100644 --- a/src/box/sql/tarantoolInt.h +++ b/src/box/sql/tarantoolInt.h @@ -89,6 +89,23 @@ int tarantoolSqlite3ClearTable(struct space *space); int sql_rename_table(uint32_t space_id, const char *new_name, char **sql_stmt); +/** + * Update CREATE INDEX field (def->opt.sql) replacing table name + * w/ new one in _index space. + * + * @param space_id Table's space identifier. + * @param iid Index identifier. + * @param new_tbl_name new name of table + * @param[out] sql_stmt CREATE INDEX statement for new name table, + * can be NULL. + * + * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise. + */ +int +sql_update_index_table_name(uint32_t space_id, uint32_t iid, + const char *new_tbl_name, char **sql_stmt); + + /* Alter trigger statement after rename table. */ int tarantoolSqlite3RenameTrigger(const char *zTriggerName, const char *zOldName, const char *zNewName); @@ -171,12 +188,16 @@ fkey_encode_links(const struct fkey_def *def, int type, char *buf); */ int tarantoolSqlite3MakeIdxParts(Index * index, void *buf); -/* +/** * Format "opts" dictionary for _index entry. * Returns result size. * If buf==NULL estimate result size. + * + * @param is_unique Flag if index is unique. + * @param zSql SQL create stmt. + * @param[out] buf destination buffer. */ -int tarantoolSqlite3MakeIdxOpts(Index * index, const char *zSql, void *buf); +int tarantoolSqlite3MakeIdxOpts(bool is_unique, const char *zSql, void *buf); /** * Extract next id from _sequence space. diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index dc5146f..f70ce1b 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -4625,13 +4625,36 @@ case OP_RenameTable: { db->init.busy = 1; init.rc = SQLITE_OK; sql_init_callback(&init, zNewTableName, space_id, 0, zSqlStmt); - db->init.busy = 0; rc = init.rc; - if (rc) { + if (rc != SQLITE_OK) { sqlite3CommitInternalChanges(); + db->init.busy = 0; goto abort_due_to_error; } + /* Space was altered, refetch the pointer. */ + space = space_by_id(space_id); + for (uint32_t i = 0; i < space->index_count; ++i) { + char *sql_stmt = space->index[i]->def->opts.sql; + if (sql_stmt == NULL) + continue; + uint32_t iid = space->index[i]->def->iid; + rc = sql_update_index_table_name(space_id, iid, + zNewTableName, &sql_stmt); + if (rc != SQLITE_OK) + goto abort_due_to_error; + + sql_init_callback(&init, zNewTableName, space_id, iid, + sql_stmt); + rc = init.rc; + if (rc != SQLITE_OK) { + sqlite3CommitInternalChanges(); + db->init.busy = 0; + goto abort_due_to_error; + } + } + db->init.busy = 0; + /* * Rebuild 'CREATE TRIGGER' expressions of all triggers * created on this table. Sure, this action is not atomic diff --git a/test/sql/gh-3613-idx-alter-update.result b/test/sql/gh-3613-idx-alter-update.result new file mode 100644 index 0000000..6f0b688 --- /dev/null +++ b/test/sql/gh-3613-idx-alter-update.result @@ -0,0 +1,37 @@ +test_run = require('test_run').new() +--- +... +engine = test_run:get_cfg('engine') +--- +... +box.sql.execute('pragma sql_default_engine=\''..engine..'\'') +--- +... +box.sql.execute('CREATE TABLE t (s1 INT PRIMARY KEY)') +--- +... +box.sql.execute('CREATE INDEX i ON t (s1)') +--- +... +box.sql.execute('ALTER TABLE t RENAME TO j3') +--- +... +-- Due to gh-3613, next stmt caused segfault +box.sql.execute('DROP INDEX i ON j3') +--- +... +box.sql.execute('CREATE INDEX i ON j3 (s1)') +--- +... +-- Check that _index was altered properly +box.snapshot() +--- +- ok +... +test_run:cmd('restart server default') +box.sql.execute('DROP INDEX i ON j3') +--- +... +box.sql.execute('DROP TABLE j3') +--- +... diff --git a/test/sql/gh-3613-idx-alter-update.test.lua b/test/sql/gh-3613-idx-alter-update.test.lua new file mode 100644 index 0000000..287096b --- /dev/null +++ b/test/sql/gh-3613-idx-alter-update.test.lua @@ -0,0 +1,21 @@ +test_run = require('test_run').new() +engine = test_run:get_cfg('engine') +box.sql.execute('pragma sql_default_engine=\''..engine..'\'') + +box.sql.execute('CREATE TABLE t (s1 INT PRIMARY KEY)') +box.sql.execute('CREATE INDEX i ON t (s1)') +box.sql.execute('ALTER TABLE t RENAME TO j3') + +-- Due to gh-3613, next stmt caused segfault +box.sql.execute('DROP INDEX i ON j3') + +box.sql.execute('CREATE INDEX i ON j3 (s1)') + +-- Check that _index was altered properly +box.snapshot() +test_run:cmd('restart server default') + +box.sql.execute('DROP INDEX i ON j3') + +-- Cleanup +box.sql.execute('DROP TABLE j3') -- 2.16.2
next reply other threads:[~2018-08-13 4:54 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-13 4:54 Kirill Yukhin [this message] 2018-08-14 22:22 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-15 8:54 ` Kirill Yukhin 2018-08-16 22:24 ` Vladislav Shpilevoy 2018-08-17 5:08 ` Kirill Yukhin 2018-08-17 8:31 ` Vladislav Shpilevoy 2018-08-17 13:48 ` Kirill Yukhin 2018-08-17 18:26 ` n.pettik 2018-08-20 5:21 ` Kirill Yukhin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=b9f53b11c907e1e076efbf223b822ad4c722a4ed.1534135985.git.kyukhin@tarantool.org \ --to=kyukhin@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [tarantool-patches] [PATCH] sql: after table rename properly update indexes' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox