[PATCH 1/1] lua: update index Lua objects on alter, do not replace
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Mon Apr 23 16:21:57 MSK 2018
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)
More information about the Tarantool-patches
mailing list