From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 5FE2F20E66 for ; Sun, 9 Dec 2018 16:30:42 -0500 (EST) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id KpC60_PzVkzA for ; Sun, 9 Dec 2018 16:30:42 -0500 (EST) Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [94.100.177.112]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 9FE6120E68 for ; Sun, 9 Dec 2018 16:30:41 -0500 (EST) From: Nikita Pettik Subject: [tarantool-patches] [PATCH 2/6] sql: don't update SQL string during renaming Date: Mon, 10 Dec 2018 00:30:22 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Nikita Pettik 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( -- }) +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"; ]], { -- - 1, "Failed to execute SQL statement: can't modify name of space created not via SQL facilities" + 0 -- }) +test:do_execsql_test( + "alter-2.4", + [[ + SELECT "f2" from "new_lua_space" WHERE "f2" >= 3 ORDER BY "id"; + ]], { + -- + 4, 3, 3 + -- +}) + test:do_execsql_test( "alter-3.1", [[ -- 2.15.1