[tarantool-patches] [PATCH 2/6] sql: don't update SQL string during renaming
Nikita Pettik
korablev at tarantool.org
Mon Dec 10 00:30:22 MSK 2018
Since SQL string containing "CREATE TABLE ..." statement is not used
anymore for ordinary tables/space, it makes no sense to modify it during
renaming. Hence, now rename routine needs only to update name in _space,
so it can be done using simple update operation.
Moreover, now we are able to rename spaces created from Lua-land.
Part of #2647
---
src/box/sql.c | 187 ++++++++------------------------------------
src/box/sql/tarantoolInt.h | 3 +-
src/box/sql/vdbe.c | 4 +-
test/sql-tap/alter.test.lua | 32 +++++++-
4 files changed, 63 insertions(+), 163 deletions(-)
diff --git a/src/box/sql.c b/src/box/sql.c
index 7b41c9926..e9fa89df9 100644
--- a/src/box/sql.c
+++ b/src/box/sql.c
@@ -697,119 +697,6 @@ rename_fail:
return SQL_TARANTOOL_ERROR;
}
-int
-sql_rename_table(uint32_t space_id, const char *new_name, char **sql_stmt)
-{
- assert(space_id != 0);
- assert(new_name != NULL);
- assert(sql_stmt != NULL);
-
- box_tuple_t *tuple;
- uint32_t key_len = mp_sizeof_uint(space_id) + mp_sizeof_array(1);
- char *key_begin = (char*) region_alloc(&fiber()->gc, key_len);
- if (key_begin == NULL) {
- diag_set(OutOfMemory, key_len, "region_alloc", "key_begin");
- return SQL_TARANTOOL_ERROR;
- }
- char *key = mp_encode_array(key_begin, 1);
- key = mp_encode_uint(key, space_id);
- if (box_index_get(BOX_SPACE_ID, 0, key_begin, key, &tuple) != 0)
- return SQL_TARANTOOL_ERROR;
- assert(tuple != NULL);
-
- /* Code below relies on format of _space. If number of fields or their
- * order will ever change, this code should be changed too.
- */
- assert(tuple_field_count(tuple) == 7);
- const char *sql_stmt_map = box_tuple_field(tuple, 5);
-
- if (sql_stmt_map == NULL || mp_typeof(*sql_stmt_map) != MP_MAP)
- goto rename_fail;
- uint32_t map_size = mp_decode_map(&sql_stmt_map);
- if (map_size != 1)
- goto rename_fail;
- const char *sql_str = mp_decode_str(&sql_stmt_map, &key_len);
-
- /* If this table hasn't been created via SQL facilities,
- * we can't do anything yet.
- */
- if (sqlite3StrNICmp(sql_str, "sql", 3) != 0)
- goto rename_fail;
- uint32_t sql_stmt_decoded_len;
- const char *sql_stmt_old = mp_decode_str(&sql_stmt_map,
- &sql_stmt_decoded_len);
- uint32_t old_name_len;
- const char *old_name = box_tuple_field(tuple, 2);
- old_name = mp_decode_str(&old_name, &old_name_len);
- uint32_t new_name_len = strlen(new_name);
-
- *sql_stmt = (char*)region_alloc(&fiber()->gc, sql_stmt_decoded_len + 1);
- if (*sql_stmt == NULL) {
- diag_set(OutOfMemory, sql_stmt_decoded_len + 1, "region_alloc",
- "sql_stmt");
- return SQL_TARANTOOL_ERROR;
- }
- memcpy(*sql_stmt, sql_stmt_old, sql_stmt_decoded_len);
- *(*sql_stmt + sql_stmt_decoded_len) = '\0';
- bool is_quoted = false;
- *sql_stmt = rename_table(db, *sql_stmt, new_name, &is_quoted);
- if (*sql_stmt == NULL)
- goto rename_fail;
-
- /* If old table name isn't quoted, then need to reserve space for quotes. */
- uint32_t sql_stmt_len = sql_stmt_decoded_len +
- new_name_len - old_name_len +
- 2 * (!is_quoted);
-
- assert(sql_stmt_len > 0);
- /* Construct new msgpack to insert to _space.
- * Since we have changed only name of table and create statement,
- * there is no need to decode/encode other fields of tuple,
- * just memcpy constant parts.
- */
- char *new_tuple = (char*)region_alloc(&fiber()->gc, tuple->bsize +
- mp_sizeof_str(sql_stmt_len));
- if (new_tuple == NULL) {
- free(*sql_stmt);
- *sql_stmt = NULL;
- diag_set(OutOfMemory,
- tuple->bsize + mp_sizeof_str(sql_stmt_len),
- "region_alloc", "new_tuple");
- return SQL_TARANTOOL_ERROR;
- }
-
- char *new_tuple_end = new_tuple;
- const char *data_begin = tuple_data(tuple);
- const char *data_end = tuple_field(tuple, 2);
- uint32_t data_size = data_end - data_begin;
- memcpy(new_tuple, data_begin, data_size);
- new_tuple_end += data_size;
- new_tuple_end = mp_encode_str(new_tuple_end, new_name, new_name_len);
- data_begin = tuple_field(tuple, 3);
- data_end = tuple_field(tuple, 5);
- data_size = data_end - data_begin;
- memcpy(new_tuple_end, data_begin, data_size);
- new_tuple_end += data_size;
- new_tuple_end = mp_encode_map(new_tuple_end, 1);
- new_tuple_end = mp_encode_str(new_tuple_end, "sql", 3);
- new_tuple_end = mp_encode_str(new_tuple_end, *sql_stmt, sql_stmt_len);
- data_begin = tuple_field(tuple, 6);
- data_end = (char*) tuple + tuple_size(tuple);
- data_size = data_end - data_begin;
- memcpy(new_tuple_end, data_begin, data_size);
- new_tuple_end += data_size;
-
- if (box_replace(BOX_SPACE_ID, new_tuple, new_tuple_end, NULL) != 0)
- return SQL_TARANTOOL_ERROR;
- else
- return SQLITE_OK;
-
-rename_fail:
- diag_set(ClientError, ER_SQL_EXECUTE, "can't modify name of space "
- "created not via SQL facilities");
- return SQL_TARANTOOL_ERROR;
-}
-
/** Callback to forward and error from mpstream methods. */
static void
set_encode_error(void *error_ctx)
@@ -817,74 +704,64 @@ set_encode_error(void *error_ctx)
*(bool *)error_ctx = true;
}
-/**
- * Encode index options @opts into message pack with @stream.
- * @param stream Message Pack Stream to use on encode.
- * @param opts Index options to encode.
- */
-static void
-mpstream_encode_index_opts(struct mpstream *stream,
- const struct index_opts *opts)
-{
- mpstream_encode_map(stream, 2);
- mpstream_encode_str(stream, "unique");
- mpstream_encode_bool(stream, opts->is_unique);
- mpstream_encode_str(stream, "sql");
- mpstream_encode_strn(stream, opts->sql,
- opts->sql != NULL ? strlen(opts->sql) : 0);
-}
-
int
-sql_index_update_table_name(struct index_def *def, const char *new_tbl_name,
- char **sql_stmt)
+sql_rename_table(uint32_t space_id, const char *new_name)
{
- assert(new_tbl_name != NULL);
-
- bool is_quoted = false;
- *sql_stmt = rename_table(db, def->opts.sql, new_tbl_name, &is_quoted);
- if (*sql_stmt == NULL)
- return -1;
-
+ assert(space_id != 0);
+ assert(new_name != NULL);
struct region *region = &fiber()->gc;
size_t used = region_used(region);
struct mpstream stream;
bool is_error = false;
mpstream_init(&stream, region, region_reserve_cb, region_alloc_cb,
set_encode_error, &is_error);
-
/* Encode key. */
- mpstream_encode_array(&stream, 2);
- mpstream_encode_uint(&stream, def->space_id);
- mpstream_encode_uint(&stream, def->iid);
+ mpstream_encode_array(&stream, 1);
+ mpstream_encode_uint(&stream, space_id);
- /* Encode op. */
+ /* Encode op and new name. */
uint32_t op_offset = stream.pos - stream.buf;
mpstream_encode_array(&stream, 1);
mpstream_encode_array(&stream, 3);
mpstream_encode_str(&stream, "=");
- mpstream_encode_uint(&stream, BOX_INDEX_FIELD_OPTS);
-
- /* Encode index opts. */
- struct index_opts opts = def->opts;
- opts.sql = *sql_stmt;
- mpstream_encode_index_opts(&stream, &opts);
-
+ mpstream_encode_uint(&stream, BOX_SPACE_FIELD_NAME);
+ mpstream_encode_str(&stream, new_name);
mpstream_flush(&stream);
if (is_error) {
diag_set(OutOfMemory, stream.pos - stream.buf,
"mpstream_flush", "stream");
- return -1;
+ return SQL_TARANTOOL_ERROR;
}
size_t sz = region_used(region) - used;
char *raw = region_join(region, sz);
if (raw == NULL) {
diag_set(OutOfMemory, sz, "region_join", "raw");
- return -1;
+ return SQL_TARANTOOL_ERROR;
}
- return box_update(BOX_INDEX_ID, 0, raw, raw + op_offset,
- raw + op_offset, raw + sz, 0, NULL);
+ if (box_update(BOX_SPACE_ID, 0, raw, raw + op_offset, raw + op_offset,
+ raw + sz, 0, NULL) != 0)
+ return SQL_TARANTOOL_ERROR;
+ return 0;
+}
+
+/**
+ * Encode index options @opts into message pack with @stream.
+ * @param stream Message Pack Stream to use on encode.
+ * @param opts Index options to encode.
+ */
+static void
+mpstream_encode_index_opts(struct mpstream *stream,
+ const struct index_opts *opts)
+{
+ mpstream_encode_map(stream, 2);
+ mpstream_encode_str(stream, "unique");
+ mpstream_encode_bool(stream, opts->is_unique);
+ mpstream_encode_str(stream, "sql");
+ mpstream_encode_strn(stream, opts->sql,
+ opts->sql != NULL ? strlen(opts->sql) : 0);
}
+
int
tarantoolSqlite3IdxKeyCompare(struct BtCursor *cursor,
struct UnpackedRecord *unpacked)
diff --git a/src/box/sql/tarantoolInt.h b/src/box/sql/tarantoolInt.h
index 3ff14d53a..e963132fe 100644
--- a/src/box/sql/tarantoolInt.h
+++ b/src/box/sql/tarantoolInt.h
@@ -72,12 +72,11 @@ int tarantoolSqlite3ClearTable(struct space *space, uint32_t *tuple_count);
*
* @param space_id Table's space identifier.
* @param new_name new name of table
- * @param[out] sql_stmt CREATE TABLE statement for new name table, can be NULL.
*
* @retval SQLITE_OK on success, SQLITE_TARANTOOL_ERROR otherwise.
*/
int
-sql_rename_table(uint32_t space_id, const char *new_name, char **sql_stmt);
+sql_rename_table(uint32_t space_id, const char *new_name);
/**
* Update CREATE INDEX field (def->opt.sql) replacing table name
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index bf6f4cdd1..e6b413c70 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -4713,7 +4713,6 @@ case OP_RenameTable: {
struct space *space;
const char *zOldTableName;
const char *zNewTableName;
- char *zSqlStmt;
space_id = pOp->p1;
space = space_by_id(space_id);
@@ -4725,7 +4724,7 @@ case OP_RenameTable: {
zNewTableName = pOp->p4.z;
zOldTableName = sqlite3DbStrNDup(db, zOldTableName,
sqlite3Strlen30(zOldTableName));
- rc = sql_rename_table(space_id, zNewTableName, &zSqlStmt);
+ rc = sql_rename_table(space_id, zNewTableName);
if (rc) goto abort_due_to_error;
/*
* Rebuild 'CREATE TRIGGER' expressions of all triggers
@@ -4753,7 +4752,6 @@ case OP_RenameTable: {
trigger = next_trigger;
}
sqlite3DbFree(db, (void*)zOldTableName);
- sqlite3DbFree(db, (void*)zSqlStmt);
break;
}
diff --git a/test/sql-tap/alter.test.lua b/test/sql-tap/alter.test.lua
index 7402526d9..1aad555c0 100755
--- a/test/sql-tap/alter.test.lua
+++ b/test/sql-tap/alter.test.lua
@@ -1,6 +1,6 @@
#!/usr/bin/env tarantool
test = require("sqltester")
-test:plan(41)
+test:plan(43)
test:do_execsql_test(
"alter-1.1",
@@ -87,16 +87,42 @@ test:do_catchsql_test(
-- </alter-2.2>
})
+test:do_test(
+ "alter-2.3.prepare",
+ function()
+ format = {}
+ format[1] = { name = 'id', type = 'integer'}
+ format[2] = { name = 'f2', type = 'number'}
+ s = box.schema.create_space('t', {format = format})
+ i = s:create_index('i', {parts= {1, 'integer'}})
+
+ s:replace{1, 4}
+ s:replace{2, 2}
+ s:replace{3, 3}
+ s:replace{4, 3}
+ end,
+ {})
+
test:do_catchsql_test(
"alter-2.3",
[[
- ALTER TABLE "_space" RENAME TO space;
+ ALTER TABLE "t" RENAME TO "new_lua_space";
]], {
-- <alter-2.3>
- 1, "Failed to execute SQL statement: can't modify name of space created not via SQL facilities"
+ 0
-- </alter-2.3>
})
+test:do_execsql_test(
+ "alter-2.4",
+ [[
+ SELECT "f2" from "new_lua_space" WHERE "f2" >= 3 ORDER BY "id";
+ ]], {
+ -- <alter-2.4>
+ 4, 3, 3
+ -- </alter-2.4>
+})
+
test:do_execsql_test(
"alter-3.1",
[[
--
2.15.1
More information about the Tarantool-patches
mailing list