[tarantool-patches] [PATCH] sql: after table rename properly update indexes

[tarantool-patches] [PATCH] sql: after table rename properly update indexes

[Kirill Yukhin]

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

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

See 6 comments below and a patch on the branch.

On 13/08/2018 07:54, Kirill Yukhin wrote:
> 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

1. Typo: 'udate'.

> name in create statement.
> 
> Closes #3613
> ---

2. Please, put here the links to the issue and branch.

>   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
> 

3. Running the tests I caught an assertion:

[010] sql-tap/alter.test.lua                          vinyl           [ fail ]
[010] Test failed! Output from reject file sql-tap/alter.reject:
[010] TAP version 13
[010] 1..41
[010] ok - alter-1.1
[010] ok - alter-1.2
[010]
[010] Last 15 lines of Tarantool Log file [Instance "app_server"][/Users/v.shpilevoy/Work/Repositories/tarantool/test/var/010_sql-tap/alter.test.lua:vinyl.tarantool.log]:
[010] ls: */: No such file or directory
[010] Assertion failed: (iid < space->index_count), function sql_update_index_table_name, file /Users/v.shpilevoy/Work/Repositories/tarantool/src/box/sql.c, line 851.

Also just failing test:

[016] sql/gh-3613-idx-alter-update.test.lua           vinyl           [ fail ]
[016]
[016] Test failed! Result content mismatch:
[016] --- sql/gh-3613-idx-alter-update.result	Wed Aug 15 00:41:34 2018
[016] +++ sql/gh-3613-idx-alter-update.reject	Wed Aug 15 00:42:19 2018
[016] @@ -32,6 +32,7 @@
[016]  box.sql.execute('DROP INDEX i ON j3')
[016]  ---
[016]  ...
[016] +-- Cleanup
[016]  box.sql.execute('DROP TABLE j3')
[016]  ---
[016]  ...
[016]


> 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.

> +{
> +	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;

5. You do not need this dummy res. box_update (as all other DML
C API functions) is tolerant to NULL in the last parameter.

> +
> +	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");

6. box_update has already set diag.

> +		return SQL_TARANTOOL_ERROR;
> +	}
> +
> +	return SQLITE_OK;
> +}
> +
>   int
>   tarantoolSqlite3IdxKeyCompare(struct BtCursor *cursor,
>   			      struct UnpackedRecord *unpacked)

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[Kirill Yukhin]

Hello Vlad,

Thanks for review!
My answers are inlined. Updated patch in the bottom.

On 15 авг 01:22, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> See 6 comments below and a patch on the branch.
> 
> On 13/08/2018 07:54, Kirill Yukhin wrote:
> > 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
> 
> 1. Typo: 'udate'.
Fixed.

> 2. Please, put here the links to the issue and branch.
Sure, will do that in future. Here they're, for reference:
Branch: https://github.com/tarantool/tarantool/commits/kyukhin/gh-3613-update-idx-afer-alter
Issue: https://github.com/tarantool/tarantool/issues/3613

> 3. Running the tests I caught an assertion:
> [010] sql-tap/alter.test.lua                          vinyl           [ fail ]
> [010] Test failed! Output from reject file sql-tap/alter.reject:
> [010] TAP version 13
> [010] 1..41
> [010] ok - alter-1.1
> [010] ok - alter-1.2
> [010]
> [010] Last 15 lines of Tarantool Log file [Instance "app_server"][/Users/v.shpilevoy/Work/Repositories/tarantool/test/var/010_sql-tap/alter.test.lua:vinyl.tarantool.log]:
> [010] ls: */: No such file or directory
> [010] Assertion failed: (iid < space->index_count), function sql_update_index_table_name, file /Users/v.shpilevoy/Work/Repositories/tarantool/src/box/sql.c, line 851.
> 
> Also just failing test:
Good catch! I've added re-fetch of space pointer after alter.

> > 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

> > +	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;
> 5. You do not need this dummy res. box_update (as all other DML
> C API functions) is tolerant to NULL in the last parameter.
Fixed.

> > +	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");
> 
> 6. box_update has already set diag.
Removed.

--
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.c b/src/box/sql.c
index ae12cae..d9c4df2 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -840,6 +840,49 @@ rename_fail:
 	return SQL_TARANTOOL_ERROR;
 }
 
+int
+sql_update_index_table_name(uint32_t space_id, struct index_def *def,
+			    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);
+	uint32_t iid = def->iid;
+
+	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);
+
+	if (box_update(BOX_INDEX_ID, 0, key_begin, key, op_begin, op,
+		       0, NULL) != 0)
+		return SQL_TARANTOOL_ERROR;
+
+	return SQLITE_OK;
+}
+
 int
 tarantoolSqlite3IdxKeyCompare(struct BtCursor *cursor,
 			      struct UnpackedRecord *unpacked)
@@ -1494,13 +1537,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 +1552,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..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);
+
+
 /* 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..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);
+		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
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..2dcb88e
--- /dev/null
+++ b/test/sql/gh-3613-idx-alter-update.result
@@ -0,0 +1,38 @@
+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')
+---
+...
+-- Cleanup
+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')

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[Vladislav Shpilevoy]

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

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[Kirill Yukhin]

Hello Vlad,

My answers are inlined, patch in the bottom, branch force-pushed.

On 17 авг 01:24, Vladislav Shpilevoy wrote:
> 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.
I've picked most of parts of your patch, thanks!

> > > > 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.
Forgot to remove space_id, thanks for fixing that.

> > 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?
I've accidentally force pushed my branch removing your fixes.
I think we need to post such a `review fixes` patches in the bottom of
review message: it can be easily applied w/o git at all and will be saved
in history. I'll udate SOP.

> > --- 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.
Removed.

> > 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.
We must re-fetch space pointer here since sql_update_index_table_name() causes
space to alter so pointer might change (and it does sporadically on vinyl
tests).

--
Regards, Kirill Yukhin

commit c2af7245a9da1f07d0b29a9e80164b1eea6a7a54
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                              | 57 ++++++++++++++++++++++--------
 src/box/sql/build.c                        |  7 ++--
 src/box/sql/tarantoolInt.h                 | 23 ++++++++++--
 src/box/sql/vdbe.c                         | 27 ++++++++++++--
 test-run                                   |  2 +-
 test/sql/gh-3613-idx-alter-update.result   | 38 ++++++++++++++++++++
 test/sql/gh-3613-idx-alter-update.test.lua | 21 +++++++++++
 7 files changed, 153 insertions(+), 22 deletions(-)

diff --git a/src/box/sql.c b/src/box/sql.c
index ae12cae..9485899 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -840,6 +840,46 @@ rename_fail:
 	return SQL_TARANTOOL_ERROR;
 }
 
+int
+sql_update_index_table_name(struct index_def *def, const char *new_tbl_name,
+			    char **sql_stmt)
+{
+	assert(new_tbl_name != NULL);
+	uint32_t iid = def->iid;
+
+	bool is_quoted = false;
+	*sql_stmt = rename_table(db, def->opts.sql, new_tbl_name, &is_quoted);
+
+	uint32_t key_len = mp_sizeof_uint(def->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, def->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);
+
+	if (box_update(BOX_INDEX_ID, 0, key_begin, key, op_begin, op,
+		       0, NULL) != 0)
+		return SQL_TARANTOOL_ERROR;
+
+	return SQLITE_OK;
+}
+
 int
 tarantoolSqlite3IdxKeyCompare(struct BtCursor *cursor,
 			      struct UnpackedRecord *unpacked)
@@ -1484,23 +1524,12 @@ int tarantoolSqlite3MakeIdxParts(SqliteIndex *pIndex, void *buf)
 	return p - base;
 }
 
-/*
- * Format "opts" dictionary for _index entry.
- * Returns result size.
- * If buf==NULL estimate result size.
- *
- * Ex: {
- *   "unique": "true",
- *   "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 +1540,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..8d30e1c 100644
--- a/src/box/sql/build.c
+++ b/src/box/sql/build.c
@@ -1245,13 +1245,14 @@ createIndex(Parse * pParse, Index * pIndex, int iSpaceId, int iIndexId,
 
 	/* Format "opts" and "parts" for _index entry. */
 	zOpts = sqlite3DbMallocRaw(pParse->db,
-				   tarantoolSqlite3MakeIdxOpts(pIndex, zSql,
-							       NULL) +
+				   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..536b0e3 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -89,6 +89,20 @@ 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 idef Index definition.
+ * @param new_tbl_name new name of table
+ * @param[out] sql_stmt New CREATE INDEX statement.
+ *
+ * @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise.
+ */
+int
+sql_update_index_table_name(struct index_def *idef, 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 +185,17 @@ 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..82ad54e 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) {
+		struct index_def *def = space->index[i]->def;
+		if (def->opts.sql == NULL)
+			continue;
+		char *sql_stmt;
+		rc = sql_update_index_table_name(def, zNewTableName, &sql_stmt);
+		if (rc != SQLITE_OK)
+			goto abort_due_to_error;
+
+		space = space_by_id(space_id);
+		sql_init_callback(&init, zNewTableName, space_id, 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
diff --git a/test-run b/test-run
index ed45e1d..0aa25ae 160000
--- a/test-run
+++ b/test-run
@@ -1 +1 @@
-Subproject commit ed45e1dbd36ab9109b84ef7189ef9d7e4b813fb9
+Subproject commit 0aa25ae8a9d4af977b3c3478cba3ccdc4ef81d35
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..2dcb88e
--- /dev/null
+++ b/test/sql/gh-3613-idx-alter-update.result
@@ -0,0 +1,38 @@
+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')
+---
+...
+-- Cleanup
+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')

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

> diff --git a/test-run b/test-run
> index ed45e1d..0aa25ae 160000
> --- a/test-run
> +++ b/test-run
> @@ -1 +1 @@
> -Subproject commit ed45e1dbd36ab9109b84ef7189ef9d7e4b813fb9
> +Subproject commit 0aa25ae8a9d4af977b3c3478cba3ccdc4ef81d35

Test-run is updated accidentally.

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[Kirill Yukhin]

Hello Vlad,
On 17 авг 11:31, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes!
> 
> > diff --git a/test-run b/test-run
> > index ed45e1d..0aa25ae 160000
> > --- a/test-run
> > +++ b/test-run
> > @@ -1 +1 @@
> > -Subproject commit ed45e1dbd36ab9109b84ef7189ef9d7e4b813fb9
> > +Subproject commit 0aa25ae8a9d4af977b3c3478cba3ccdc4ef81d35
> 
> Test-run is updated accidentally.

I've discarded the hunk & force-pushed the branch. Don't think
repost is needed here.

--
Regards, Kirill Yukhin

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[n.pettik]

>> Hi! Thanks for the fixes!
>> 
>>> diff --git a/test-run b/test-run
>>> index ed45e1d..0aa25ae 160000
>>> --- a/test-run
>>> +++ b/test-run
>>> @@ -1 +1 @@
>>> -Subproject commit ed45e1dbd36ab9109b84ef7189ef9d7e4b813fb9
>>> +Subproject commit 0aa25ae8a9d4af977b3c3478cba3ccdc4ef81d35
>> 
>> Test-run is updated accidentally.
> 
> I've discarded the hunk & force-pushed the branch. Don't think
> repost is needed here.

Hello, guys. I found this seagfault on updated branch:

tarantool> box.sql.execute('CREATE TABLE t (s1 INT PRIMARY KEY)')
---
...

tarantool> box.sql.execute('CREATE INDEX i ON t (s1)')
---
...

tarantool> box.sql.execute('ALTER TABLE t RENAME TO j3')

Ctrl+C

tarantool> n:tarantool n.pettik$ ./src/tarantool 
Tarantool 2.0.4-838-g9d879b042
type 'help' for interactive help
tarantool> box.cfg{}
2018-08-17 21:26:17.797 [23781] main/101/interactive C> Tarantool 2.0.4-838-g9d879b042
2018-08-17 21:26:17.797 [23781] main/101/interactive C> log level 5
2018-08-17 21:26:17.798 [23781] main/101/interactive I> mapping 268435456 bytes for memtx tuple arena...
2018-08-17 21:26:17.798 [23781] main/101/interactive I> mapping 134217728 bytes for vinyl tuple arena...
2018-08-17 21:26:17.807 [23781] main/101/interactive I> recovery start
2018-08-17 21:26:17.807 [23781] main/101/interactive I> recovering from `./00000000000000000000.snap'
2018-08-17 21:26:17.849 [23781] main/101/interactive I> recover from `./00000000000000000000.xlog'
2018-08-17 21:26:17.849 [23781] main/101/interactive I> done `./00000000000000000000.xlog'
2018-08-17 21:26:17.850 [23781] main/101/interactive I> recover from `./00000000000000000006.xlog'
2018-08-17 21:26:17.850 [23781] main/101/interactive I> done `./00000000000000000006.xlog'
Segmentation fault
  code: SEGV_MAPERR
  addr: 0x0
  context: 0x10881f4c8
  siginfo: 0x10881f460
Current time: 1534530377
Please file a bug at http://github.com/tarantool/tarantool/issues
Attempting backtrace... Note: since the server has already crashed, 
this may fail as well
#0  0x105e087ed in print_backtrace+d
#1  0x105cbb613 in _ZL12sig_fatal_cbiP9__siginfoPv+1f3
#2  0x7fffcae1eb3a in _sigtramp+1a
#3  0x1060690b3 in corruptSchema+93
#4  0x106068f5a in sql_init_callback+25a
#5  0x105dbe73a in space_foreach_put_cb+da
#6  0x105d86460 in space_foreach+2b0
#7  0x105dbe646 in tarantoolSqlite3LoadSchema+166
#8  0x10606927a in sqlite3InitDatabase+7a
#9  0x105dba3de in sql_load_schema+de
#10 0x105d99fe7 in _ZL10box_cfg_xcv+7a7
#11 0x105d9973d in box_cfg+d
#12 0x105cb9e81 in load_cfg+861
#13 0x105dca8c1 in _ZL13lbox_cfg_loadP9lua_State+11
#14 0x105e3653d in lj_BC_FUNCC+44
#15 0x105e64b90 in lua_pcall+210
#16 0x105dec823 in luaT_call+23
#17 0x105de3761 in lua_main+d1
#18 0x105de3548 in run_script_f+8e8
#19 0x105cbaf0a in _ZL16fiber_cxx_invokePFiP13__va_list_tagES0_+1a
#20 0x105e042db in fiber_loop+bb
#21 0x106023947 in coro_init+57
Abort trap: 6

[tarantool-patches] Re: [PATCH] sql: after table rename properly update indexes

[Kirill Yukhin]

Hello Nikita,
On 17 авг 21:26, n.pettik wrote:
> 
> >> Hi! Thanks for the fixes!
> >> 
> >>> diff --git a/test-run b/test-run
> >>> index ed45e1d..0aa25ae 160000
> >>> --- a/test-run
> >>> +++ b/test-run
> >>> @@ -1 +1 @@
> >>> -Subproject commit ed45e1dbd36ab9109b84ef7189ef9d7e4b813fb9
> >>> +Subproject commit 0aa25ae8a9d4af977b3c3478cba3ccdc4ef81d35
> >> 
> >> Test-run is updated accidentally.
> > 
> > I've discarded the hunk & force-pushed the branch. Don't think
> > repost is needed here.
> 
> Hello, guys. I found this seagfault on updated branch:

This is a separate issue which was uncovered by my patch.
So, I'll re-send my change as a patchset of 2 patches: one
fising original issue and another, which fixes this one.

So, please disregard this patch.

--
Regards, Kirill Yukhin