From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Vladislav Shpilevoy Subject: [PATCH 1/1] lua: update index Lua objects on alter, do not replace Date: Mon, 23 Apr 2018 16:21:57 +0300 Message-Id: <7eb5116165a68e89e7a4c6a1bac5ad01c1ccdcdc.1524489665.git.v.shpilevoy@tarantool.org> To: tarantool-patches@freelists.org Cc: vdavydov.dev@gmail.com List-ID: Before the patch on any space alter Tarantool recreates its indexes leaving old index objects be invalid. To protect a user from errors about accessing outdated index object fields lets update it in place instead of creating a new one. Closes #3285 --- Branch: https://github.com/tarantool/tarantool/tree/gh-3285-index-modify-on-alter Issue: https://github.com/tarantool/tarantool/issues/3285 src/box/lua/space.cc | 62 ++++++++++++++++++----- test/box/alter.result | 129 ++++++++++++++++++++++++++++++++++++++++++++++++ test/box/alter.test.lua | 55 +++++++++++++++++++++ 3 files changed, 234 insertions(+), 12 deletions(-) diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc index b03aa46e9..a26d073be 100644 --- a/src/box/lua/space.cc +++ b/src/box/lua/space.cc @@ -198,13 +198,34 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) lua_settable(L, i); /* push space.index */ lua_getfield(L, i, "index"); } else { - /* Empty the table. */ - lua_pushnil(L); /* first key */ + /* + * Remove only deleted indexes. Other ones must + * be kept for modifying to avoid existing index + * references invalidation. + */ + lua_pushnil(L); while (lua_next(L, -2) != 0) { - lua_pop(L, 1); /* remove the value. */ - lua_pushnil(L); /* set the key to nil. */ - lua_settable(L, -3); - lua_pushnil(L); /* start over. */ + if (lua_isnumber(L, -2)) { + uint32_t iid = (uint32_t) lua_tonumber(L, -2); + if (space_index(space, iid) == NULL) { + lua_pushnumber(L, iid); + lua_pushnil(L); + lua_settable(L, -5); + } + lua_pop(L, 1); + } else { + /* + * Remove all named references due + * to possible renames. They are + * rebuilt below. + */ + assert(lua_isstring(L, -2)); + lua_pushvalue(L, -2); + lua_pushnil(L); + lua_settable(L, -5); + lua_pop(L, 2); + lua_pushnil(L); + } } } /* @@ -217,8 +238,15 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) continue; struct index_def *index_def = index->def; struct index_opts *index_opts = &index_def->opts; - lua_pushnumber(L, index_def->iid); - lua_newtable(L); /* space.index[k] */ + lua_rawgeti(L, -1, index_def->iid); + if (lua_isnil(L, -1)) { + lua_pop(L, 1); + lua_pushnumber(L, index_def->iid); + lua_newtable(L); + lua_settable(L, -3); + lua_rawgeti(L, -1, index_def->iid); + assert(! lua_isnil(L, -1)); + } if (index_def->type == HASH || index_def->type == TREE) { lua_pushboolean(L, index_opts->is_unique); @@ -268,10 +296,23 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) lua_settable(L, -3); /* space.index[k].parts */ + lua_pushstring(L, "sequence_id"); if (k == 0 && space->sequence != NULL) { lua_pushnumber(L, space->sequence->def->id); - lua_setfield(L, -2, "sequence_id"); + } else { + /* + * Remove sequence_id from existing + * index. On new it is ok too - the field + * just is not created. + */ + lua_pushnil(L); } + /* + * Optional attributes must be set via + * 'raw' API to avoid __newindex + * metamethod invocation. + */ + lua_rawset(L, -3); if (space_is_vinyl(space)) { lua_pushstring(L, "options"); @@ -294,9 +335,6 @@ lbox_fillspace(struct lua_State *L, struct space *space, int i) lua_settable(L, -3); } - - lua_settable(L, -3); /* space.index[k] */ - lua_rawgeti(L, -1, index_def->iid); lua_setfield(L, -2, index_def->name); } diff --git a/test/box/alter.result b/test/box/alter.result index a78dd3e91..6bf91686b 100644 --- a/test/box/alter.result +++ b/test/box/alter.result @@ -1548,3 +1548,132 @@ s:format() s:drop() --- ... +-- +-- gh-3285: index Lua object is not updated on alter. +-- +box.internal.collation.create('test', 'ICU', 'ru-RU') +--- +... +s = box.schema.create_space('test') +--- +... +pk = s:create_index('pk') +--- +... +sk1 = s:create_index('b', {unique = false}) +--- +... +sk2 = s:create_index('c', {type = 'hash', parts = {{3, 'unsigned'}}}) +--- +... +sk3 = s:create_index('d', {parts = {{4, 'string'}}}) +--- +... +test_run:cmd("setopt delimiter ';'") +--- +- true +... +function index_count() + local r = 0 + for i, _ in pairs(s.index) do + r = r + 1 + end + return r / 2 +end; +--- +... +function validate_indexes() + assert(s == box.space.test) + assert(sk1 == nil or sk1 == s.index[sk1.name]) + assert(sk2 == nil or sk2 == s.index[sk2.name]) + assert(sk3 == nil or sk3 == s.index[sk3.name]) +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +index_count() +--- +- 4 +... +sk3:rename('dd') +--- +... +sk3.name == 'dd' +--- +- true +... +index_count() +--- +- 4 +... +validate_indexes() +--- +... +sk3:alter({parts = {{4, 'string', collation = 'test'}}}) +--- +... +sk3.parts[1].collation == 'test' +--- +- true +... +index_count() +--- +- 4 +... +validate_indexes() +--- +... +sk3:drop() +--- +... +sk3 = nil +--- +... +index_count() +--- +- 3 +... +validate_indexes() +--- +... +sk2:alter({parts = {{4, 'string'}}}) +--- +... +sk2.parts[1].type == 'string' +--- +- true +... +sk2.parts[1].fieldno == 4 +--- +- true +... +index_count() +--- +- 3 +... +validate_indexes() +--- +... +sk1:alter({unique = true}) +--- +... +sk1.unique +--- +- true +... +index_count() +--- +- 3 +... +validate_indexes() +--- +... +s:drop() +--- +... +box.internal.collation.drop('test') +--- +... diff --git a/test/box/alter.test.lua b/test/box/alter.test.lua index ab7135848..11b8125fe 100644 --- a/test/box/alter.test.lua +++ b/test/box/alter.test.lua @@ -583,3 +583,58 @@ s:format(format) s:format() s:drop() + +-- +-- gh-3285: index Lua object is not updated on alter. +-- +box.internal.collation.create('test', 'ICU', 'ru-RU') +s = box.schema.create_space('test') +pk = s:create_index('pk') +sk1 = s:create_index('b', {unique = false}) +sk2 = s:create_index('c', {type = 'hash', parts = {{3, 'unsigned'}}}) +sk3 = s:create_index('d', {parts = {{4, 'string'}}}) +test_run:cmd("setopt delimiter ';'") +function index_count() + local r = 0 + for i, _ in pairs(s.index) do + r = r + 1 + end + return r / 2 +end; +function validate_indexes() + assert(s == box.space.test) + assert(sk1 == nil or sk1 == s.index[sk1.name]) + assert(sk2 == nil or sk2 == s.index[sk2.name]) + assert(sk3 == nil or sk3 == s.index[sk3.name]) +end; +test_run:cmd("setopt delimiter ''"); +index_count() + +sk3:rename('dd') +sk3.name == 'dd' +index_count() +validate_indexes() + +sk3:alter({parts = {{4, 'string', collation = 'test'}}}) +sk3.parts[1].collation == 'test' +index_count() +validate_indexes() + +sk3:drop() +sk3 = nil +index_count() +validate_indexes() + +sk2:alter({parts = {{4, 'string'}}}) +sk2.parts[1].type == 'string' +sk2.parts[1].fieldno == 4 +index_count() +validate_indexes() + +sk1:alter({unique = true}) +sk1.unique +index_count() +validate_indexes() + +s:drop() +box.internal.collation.drop('test') -- 2.15.1 (Apple Git-101)