From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> To: Kirill Yukhin <kyukhin@tarantool.org> Cc: tarantool-patches@freelists.org Subject: [tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes Date: Fri, 17 Aug 2018 01:24:55 +0300 [thread overview] Message-ID: <a1a91c2e-ad96-b59a-435b-311d9c6f988c@tarantool.org> (raw) In-Reply-To: <20180815085442.7bajsapuf7wrlu3r@tarantool.org> Hi! Thanks for the fixes! Unfortunately you have ignored some of my fixes pushed on the branch, so I pushed them again on a separate commit. Please, be careful doing a force push. >>> 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) >> >> 4. I think, it would be enough to pass index_def (that you have >> on the stack above). It has both index id and space id. And here >> you use space_cache_find and struct space only to get index_def. > Done. This fix cures assert in p 3 1. Unfortunately, not done. > Regards, Kirill Yukhin > > commit 4cf29331f3730658351701122d8714e157e0d77b > Author: Kirill Yukhin <kyukhin@tarantool.org> > Date: Fri Aug 10 15:22:19 2018 +0300 > > sql: after table rename properly update indexes > > After table was altered due to rename, indexes were not updated. > Re-create all table's indexes (in legacy SQL DD), also perform > update of corresponding entry in _index for each index to fix > table name in create statement. > > Closes #3613 > --- > src/box/sql.c | 49 +++++++++++++++++++++++++++--- > 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 | 38 +++++++++++++++++++++++ > test/sql/gh-3613-idx-alter-update.test.lua | 21 +++++++++++++ > 6 files changed, 156 insertions(+), 10 deletions(-) > > 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) + 2. Looks like you has ignored my fixes here and in most other places. Why? > tarantoolSqlite3MakeIdxParts(pIndex, > NULL) + 2); > if (!zOpts) > diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h > index 94517f6..234c94b 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 idef Index definition. > + * @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, struct index_def *idef, > + const char *new_tbl_name, char **sql_stmt); > + > + 3. Redundant empty line. > /* Alter trigger statement after rename table. */ > int tarantoolSqlite3RenameTrigger(const char *zTriggerName, > const char *zOldName, const char *zNewName); > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index dc5146f..1166ddb 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; > + rc = sql_update_index_table_name(space_id, space->index[i]->def, > + zNewTableName, &sql_stmt); > + if (rc != SQLITE_OK) > + goto abort_due_to_error; > + > + space = space_by_id(space_id); 4. The same space is looked up before the cycle. > + sql_init_callback(&init, zNewTableName, space_id, > + space->index[i]->def->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
next prev parent reply other threads:[~2018-08-16 22:24 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-13 4:54 [tarantool-patches] " Kirill Yukhin 2018-08-14 22:22 ` [tarantool-patches] " Vladislav Shpilevoy 2018-08-15 8:54 ` Kirill Yukhin 2018-08-16 22:24 ` Vladislav Shpilevoy [this message] 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=a1a91c2e-ad96-b59a-435b-311d9c6f988c@tarantool.org \ --to=v.shpilevoy@tarantool.org \ --cc=kyukhin@tarantool.org \ --cc=tarantool-patches@freelists.org \ --subject='[tarantool-patches] Re: [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