[tarantool-patches] Re: [PATCH 1/2] sql: after table rename properly update indexes
Kirill Yukhin
kyukhin at tarantool.org
Wed Aug 22 09:39:43 MSK 2018
Hello Nikita,
My answers are inlined.
Updated patch in the bottom. Branch force-pushed.
On 21 авг 23:38, n.pettik wrote:
>
> > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> > index dc5146f..821c7a9 100644
> > --- a/src/box/sql/vdbe.c
> > +++ b/src/box/sql/vdbe.c
> > @@ -4625,13 +4625,35 @@ 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) {
> > + struct index_def *def = space->index[i]->def;
> > + if (def->opts.sql == NULL)
> > + continue;
> > + char *sql_stmt;
> > + rc = sql_index_update_table_name(def, zNewTableName, &sql_stmt);
>
> Where do you free memory for sql_stmt? table_rename allocates new
> string using sqlite3DbMalloc AFAIK.
Yes, it is. This is a clear leak. Fixed.
> > + if (rc != SQLITE_OK)
>
> rc != 0 (or better: if (sql_index_update_table_name() != 0)).
>
> > + goto abort_due_to_error;
> > + space = space_by_id(space_id);
> > + sql_init_callback(&init, zNewTableName, space_id,
> > + space->index[i]->def->iid, sql_stmt);
> > + rc = init.rc;
> > + if (rc != SQLITE_OK) {
>
> rc != 0 (or better: if (init.rc != 0)
Fixed.
Here's iterative diff:
1 file changed, 4 insertions(+), 3 deletions(-)
src/box/sql/vdbe.c | 7 ++++---
modified src/box/sql/vdbe.c
@@ -4663,14 +4663,15 @@ case OP_RenameTable: {
continue;
char *sql_stmt;
rc = sql_index_update_table_name(def, zNewTableName, &sql_stmt);
- if (rc != SQLITE_OK)
+ if (rc != 0)
goto abort_due_to_error;
space = space_by_id(space_id);
sql_init_callback(&init, zNewTableName, space_id,
space->index[i]->def->iid, sql_stmt);
- rc = init.rc;
- if (rc != SQLITE_OK) {
+ sqlite3DbFree(db, sql_stmt);
+ if (init.rc != 0) {
sqlite3CommitInternalChanges();
+ rc = init.rc;
db->init.busy = 0;
goto abort_due_to_error;
}
>
> The rest seems to be ok.
>
Thanks! I'll check updated patch into 2.0 branch.
--
Regards, Kirill Yukhin
More information about the Tarantool-patches
mailing list