* [PATCH 0/2] schema: expose space_mt and index_mt on table @ 2018-03-23 13:07 Vladislav Shpilevoy 2018-03-23 13:07 ` [PATCH 1/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-03-23 13:07 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Branch: http://github.com/tarantool/tarantool/tree/pr-3204-expose-space-index-mt Issue: https://github.com/tarantool/tarantool/issues/3204 Some of users want to extend space and index methods with their own, but do not want to do it for each space, because a metatable are created individually for a space. Move these metatables to box.schema, and lookup into them, when a method was not found in an original metatable. Vladislav Shpilevoy (1): schema: review fixes for box.schema.space/index metatables aleclarson (1): schema: expose space_mt and index_mt on `box.schema` table src/box/lua/schema.lua | 631 ++++++++++++++++++++-------------------- test/box-tap/schema_mt.test.lua | 79 +++++ 2 files changed, 391 insertions(+), 319 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] schema: expose space_mt and index_mt on `box.schema` table 2018-03-23 13:07 [PATCH 0/2] schema: expose space_mt and index_mt on table Vladislav Shpilevoy @ 2018-03-23 13:07 ` Vladislav Shpilevoy 2018-03-23 13:07 ` [PATCH 2/2] schema: review fixes for box.schema.space/index metatables Vladislav Shpilevoy 2018-03-29 10:54 ` [PATCH 0/2] schema: expose space_mt and index_mt on table Vladimir Davydov 2 siblings, 0 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-03-23 13:07 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, aleclarson From: aleclarson <alec.stanford.larson@gmail.com> This commit allows userland to extend the space and index metatables with their own functions or even metamethods. Reducing barriers for this kind of experimentation is vital for user contribution toward the improvement of Tarantool's API. Also, this commit reuses functions between all spaces/indexes. The metatables are exposed on `box` to make them simple to discover. The suggested alternatives were `box.internal` and `box.schema`. In a future commit, we should look into "freezing" the built-in methods so the user cannot accidentally override them. This is especially important when updating Tarantool. Closes #3204 --- src/box/lua/schema.lua | 639 ++++++++++++++++++++-------------------- test/box-tap/schema_mt.test.lua | 74 +++++ 2 files changed, 398 insertions(+), 315 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 5a0f71559..3fb626efd 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1074,218 +1074,348 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box -function box.schema.space.bless(space) - local index_mt = {} - -- __len and __index - index_mt.len = function(index) - check_index_arg(index, 'len') - local ret = builtin.box_index_len(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - -- index.bsize - index_mt.bsize = function(index) - check_index_arg(index, 'bsize') - local ret = builtin.box_index_bsize(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - index_mt.__len = index_mt.len -- Lua 5.2 compatibility - index_mt.__newindex = function(table, index) - return error('Attempt to modify a read-only table') end - index_mt.__index = index_mt - -- min and max - index_mt.min_ffi = function(index, key) - check_index_arg(index, 'min') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_min(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end - end - index_mt.min_luac = function(index, key) - check_index_arg(index, 'min') - key = keify(key) - return internal.min(index.space_id, index.id, key); +local space_mt = {} +space_mt.__index = space_mt +box.schema.space_mt = space_mt + +space_mt.len = function(space) + check_space_arg(space, 'len') + local pk = space.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - index_mt.max_ffi = function(index, key) - check_index_arg(index, 'max') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_max(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + return space.index[0]:len() +end +space_mt.count = function(space, key, opts) + check_space_arg(space, 'count') + local pk = space.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - index_mt.max_luac = function(index, key) - check_index_arg(index, 'max') - key = keify(key) - return internal.max(index.space_id, index.id, key); + return pk:count(key, opts) +end +space_mt.bsize = function(space) + check_space_arg(space, 'bsize') + local s = builtin.space_by_id(space.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, space.name) end - index_mt.random_ffi = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - if builtin.box_index_random(index.space_id, index.id, rnd, - ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + return builtin.space_bsize(s) +end + +space_mt.get = function(space, key) + check_space_arg(space, 'get') + return check_primary_index(space):get(key) +end +space_mt.select = function(space, key, opts) + check_space_arg(space, 'select') + return check_primary_index(space):select(key, opts) +end +space_mt.insert = function(space, tuple) + check_space_arg(space, 'insert') + return internal.insert(space.id, tuple); +end +space_mt.replace = function(space, tuple) + check_space_arg(space, 'replace') + return internal.replace(space.id, tuple); +end +space_mt.put = space_mt.replace; -- put is an alias for replace +space_mt.update = function(space, key, ops) + check_space_arg(space, 'update') + return check_primary_index(space):update(key, ops) +end +space_mt.upsert = function(space, tuple_key, ops, deprecated) + check_space_arg(space, 'upsert') + if deprecated ~= nil then + local msg = "Error: extra argument in upsert call: " + msg = msg .. tostring(deprecated) + msg = msg .. ". Usage :upsert(tuple, operations)" + box.error(box.error.PROC_LUA, msg) end - index_mt.random_luac = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - return internal.random(index.space_id, index.id, rnd); - end - -- iteration - index_mt.pairs_ffi = function(index, key, opts) - check_index_arg(index, 'pairs') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - - local keybuf = ffi.string(pkey, pkey_end - pkey) - local pkeybuf = ffi.cast('const char *', keybuf) - local cdata = builtin.box_index_iterator(index.space_id, index.id, - itype, pkeybuf, pkeybuf + #keybuf); - if cdata == nil then - box.error() - end - return fun.wrap(iterator_gen, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - index_mt.pairs_luac = function(index, key, opts) - check_index_arg(index, 'pairs') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - local keymp = msgpack.encode(key) - local keybuf = ffi.string(keymp, #keymp) - local cdata = internal.iterator(index.space_id, index.id, itype, keymp); - return fun.wrap(iterator_gen_luac, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - - -- index subtree size - index_mt.count_ffi = function(index, key, opts) - check_index_arg(index, 'count') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - local count = builtin.box_index_count(index.space_id, index.id, - itype, pkey, pkey_end); - if count == -1 then - box.error() - end - return tonumber(count) - end - index_mt.count_luac = function(index, key, opts) - check_index_arg(index, 'count') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - return internal.count(index.space_id, index.id, itype, key); - end - - index_mt.get_ffi = function(index, key) - check_index_arg(index, 'get') - local key, key_end = tuple_encode(key) - if builtin.box_index_get(index.space_id, index.id, - key, key_end, ptuple) ~= 0 then - return box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + return internal.upsert(space.id, tuple_key, ops); +end +space_mt.delete = function(space, key) + check_space_arg(space, 'delete') + return check_primary_index(space):delete(key) +end +-- Assumes that spaceno has a TREE (NUM) primary key +-- inserts a tuple after getting the next value of the +-- primary key and returns it back to the user +space_mt.auto_increment = function(space, tuple) + check_space_arg(space, 'auto_increment') + local max_tuple = check_primary_index(space):max() + local max = 0 + if max_tuple ~= nil then + max = max_tuple[1] end - index_mt.get_luac = function(index, key) - check_index_arg(index, 'get') - key = keify(key) - return internal.get(index.space_id, index.id, key) + table.insert(tuple, 1, max + 1) + return space:insert(tuple) +end + +space_mt.pairs = function(space, key, opts) + check_space_arg(space, 'pairs') + local pk = space.index[0] + if pk == nil then + -- empty space without indexes, return empty iterator + return fun.iter({}) + end + return pk:pairs(key, opts) +end +space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility +space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility +space_mt.truncate = function(space) + check_space_arg(space, 'truncate') + return internal.truncate(space.id) +end +space_mt.format = function(space, format) + check_space_arg(space, 'format') + return box.schema.space.format(space.id, format) +end +space_mt.drop = function(space) + check_space_arg(space, 'drop') + check_space_exists(space) + return box.schema.space.drop(space.id, space.name) +end +space_mt.rename = function(space, name) + check_space_arg(space, 'rename') + check_space_exists(space) + return box.schema.space.rename(space.id, name) +end +space_mt.create_index = function(space, name, options) + check_space_arg(space, 'create_index') + check_space_exists(space) + return box.schema.index.create(space.id, name, options) +end +space_mt.run_triggers = function(space, yesno) + check_space_arg(space, 'run_triggers') + local s = builtin.space_by_id(space.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, space.name) end + builtin.space_run_triggers(s, yesno) +end - local function check_select_opts(opts, key_is_nil) - local offset = 0 - local limit = 4294967295 - local iterator = check_iterator_type(opts, key_is_nil) - if opts ~= nil then - if opts.offset ~= nil then - offset = opts.offset - end - if opts.limit ~= nil then - limit = opts.limit - end - end - return iterator, offset, limit +local index_mt = {} +index_mt.__index = index_mt +box.schema.index_mt = index_mt + +-- __len and __index +index_mt.len = function(index) + check_index_arg(index, 'len') + local ret = builtin.box_index_len(index.space_id, index.id) + if ret == -1 then + box.error() + end + return tonumber(ret) +end +-- index.bsize +index_mt.bsize = function(index) + check_index_arg(index, 'bsize') + local ret = builtin.box_index_bsize(index.space_id, index.id) + if ret == -1 then + box.error() + end + return tonumber(ret) +end +index_mt.__len = index_mt.len -- Lua 5.2 compatibility +index_mt.__index = index_mt +-- min and max +index_mt.min_ffi = function(index, key) + check_index_arg(index, 'min') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_min(index.space_id, index.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return + end +end +index_mt.min_luac = function(index, key) + check_index_arg(index, 'min') + key = keify(key) + return internal.min(index.space_id, index.id, key); +end +index_mt.max_ffi = function(index, key) + check_index_arg(index, 'max') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_max(index.space_id, index.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return end +end +index_mt.max_luac = function(index, key) + check_index_arg(index, 'max') + key = keify(key) + return internal.max(index.space_id, index.id, key); +end +index_mt.random_ffi = function(index, rnd) + check_index_arg(index, 'random') + rnd = rnd or math.random() + if builtin.box_index_random(index.space_id, index.id, rnd, + ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return + end +end +index_mt.random_luac = function(index, rnd) + check_index_arg(index, 'random') + rnd = rnd or math.random() + return internal.random(index.space_id, index.id, rnd); +end +-- iteration +index_mt.pairs_ffi = function(index, key, opts) + check_index_arg(index, 'pairs') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - index_mt.select_ffi = function(index, key, opts) - check_index_arg(index, 'select') - local key, key_end = tuple_encode(key) - local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) + local keybuf = ffi.string(pkey, pkey_end - pkey) + local pkeybuf = ffi.cast('const char *', keybuf) + local cdata = builtin.box_index_iterator(index.space_id, index.id, + itype, pkeybuf, pkeybuf + #keybuf); + if cdata == nil then + box.error() + end + return fun.wrap(iterator_gen, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end +index_mt.pairs_luac = function(index, key, opts) + check_index_arg(index, 'pairs') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0); + local keymp = msgpack.encode(key) + local keybuf = ffi.string(keymp, #keymp) + local cdata = internal.iterator(index.space_id, index.id, itype, keymp); + return fun.wrap(iterator_gen_luac, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end + +-- index subtree size +index_mt.count_ffi = function(index, key, opts) + check_index_arg(index, 'count') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); + local count = builtin.box_index_count(index.space_id, index.id, + itype, pkey, pkey_end); + if count == -1 then + box.error() + end + return tonumber(count) +end +index_mt.count_luac = function(index, key, opts) + check_index_arg(index, 'count') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0); + return internal.count(index.space_id, index.id, itype, key); +end - local port = ffi.cast('struct port *', port_tuple) +index_mt.get_ffi = function(index, key) + check_index_arg(index, 'get') + local key, key_end = tuple_encode(key) + if builtin.box_index_get(index.space_id, index.id, + key, key_end, ptuple) ~= 0 then + return box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return + end +end +index_mt.get_luac = function(index, key) + check_index_arg(index, 'get') + key = keify(key) + return internal.get(index.space_id, index.id, key) +end - if builtin.box_select(index.space_id, index.id, - iterator, offset, limit, key, key_end, port) ~= 0 then - return box.error() +local function check_select_opts(opts, key_is_nil) + local offset = 0 + local limit = 4294967295 + local iterator = check_iterator_type(opts, key_is_nil) + if opts ~= nil then + if opts.offset ~= nil then + offset = opts.offset end - - local ret = {} - local entry = port_tuple.first - for i=1,tonumber(port_tuple.size),1 do - ret[i] = tuple_bless(entry.tuple) - entry = entry.next + if opts.limit ~= nil then + limit = opts.limit end - builtin.port_destroy(port); - return ret end + return iterator, offset, limit +end - index_mt.select_luac = function(index, key, opts) - check_index_arg(index, 'select') - local key = keify(key) - local iterator, offset, limit = check_select_opts(opts, #key == 0) - return internal.select(index.space_id, index.id, iterator, - offset, limit, key) - end +index_mt.select_ffi = function(index, key, opts) + check_index_arg(index, 'select') + local key, key_end = tuple_encode(key) + local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) - index_mt.update = function(index, key, ops) - check_index_arg(index, 'update') - return internal.update(index.space_id, index.id, keify(key), ops); - end - index_mt.delete = function(index, key) - check_index_arg(index, 'delete') - return internal.delete(index.space_id, index.id, keify(key)); - end + local port = ffi.cast('struct port *', port_tuple) - index_mt.info = function(index) - return internal.info(index.space_id, index.id); + if builtin.box_select(index.space_id, index.id, + iterator, offset, limit, key, key_end, port) ~= 0 then + return box.error() end - index_mt.drop = function(index) - check_index_arg(index, 'drop') - return box.schema.index.drop(index.space_id, index.id) + local ret = {} + local entry = port_tuple.first + for i=1,tonumber(port_tuple.size),1 do + ret[i] = tuple_bless(entry.tuple) + entry = entry.next end - index_mt.rename = function(index, name) - check_index_arg(index, 'rename') - return box.schema.index.rename(index.space_id, index.id, name) + builtin.port_destroy(port); + return ret +end + +index_mt.select_luac = function(index, key, opts) + check_index_arg(index, 'select') + local key = keify(key) + local iterator, offset, limit = check_select_opts(opts, #key == 0) + return internal.select(index.space_id, index.id, iterator, + offset, limit, key) +end + +index_mt.update = function(index, key, ops) + check_index_arg(index, 'update') + return internal.update(index.space_id, index.id, keify(key), ops); +end +index_mt.delete = function(index, key) + check_index_arg(index, 'delete') + return internal.delete(index.space_id, index.id, keify(key)); +end + +index_mt.info = function(index) + return internal.info(index.space_id, index.id); +end + +index_mt.drop = function(index) + check_index_arg(index, 'drop') + return box.schema.index.drop(index.space_id, index.id) +end +index_mt.rename = function(index, name) + check_index_arg(index, 'rename') + return box.schema.index.rename(index.space_id, index.id, name) +end +index_mt.alter = function(index, options) + check_index_arg(index, 'alter') + if index.id == nil or index.space_id == nil then + box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") end - index_mt.alter = function(index, options) - check_index_arg(index, 'alter') - if index.id == nil or index.space_id == nil then - box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") - end - return box.schema.index.alter(index.space_id, index.id, options) + return box.schema.index.alter(index.space_id, index.id, options) +end + +index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility +index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility + +function box.schema.space.bless(space) + local index_mt = {} + index_mt.__index = function(index, key) + return index_mt[key] or box.schema.index_mt[key] end -- true if reading operations may yield @@ -1294,136 +1424,15 @@ function box.schema.space.bless(space) for _, op in ipairs(read_ops) do if read_yields then -- use Lua/C implmenetation - index_mt[op] = index_mt[op .. "_luac"] + index_mt[op] = box.schema.index_mt[op .. "_luac"] else -- use FFI implementation - index_mt[op] = index_mt[op .. "_ffi"] + index_mt[op] = box.schema.index_mt[op .. "_ffi"] end end - index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility - index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility -- - local space_mt = {} - space_mt.len = function(space) - check_space_arg(space, 'len') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return space.index[0]:len() - end - space_mt.count = function(space, key, opts) - check_space_arg(space, 'count') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return pk:count(key, opts) - end - space_mt.bsize = function(space) - check_space_arg(space, 'bsize') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - return builtin.space_bsize(s) - end - space_mt.__newindex = index_mt.__newindex - space_mt.get = function(space, key) - check_space_arg(space, 'get') - return check_primary_index(space):get(key) - end - space_mt.select = function(space, key, opts) - check_space_arg(space, 'select') - return check_primary_index(space):select(key, opts) - end - space_mt.insert = function(space, tuple) - check_space_arg(space, 'insert') - return internal.insert(space.id, tuple); - end - space_mt.replace = function(space, tuple) - check_space_arg(space, 'replace') - return internal.replace(space.id, tuple); - end - space_mt.put = space_mt.replace; -- put is an alias for replace - space_mt.update = function(space, key, ops) - check_space_arg(space, 'update') - return check_primary_index(space):update(key, ops) - end - space_mt.upsert = function(space, tuple_key, ops, deprecated) - check_space_arg(space, 'upsert') - if deprecated ~= nil then - local msg = "Error: extra argument in upsert call: " - msg = msg .. tostring(deprecated) - msg = msg .. ". Usage :upsert(tuple, operations)" - box.error(box.error.PROC_LUA, msg) - end - return internal.upsert(space.id, tuple_key, ops); - end - space_mt.delete = function(space, key) - check_space_arg(space, 'delete') - return check_primary_index(space):delete(key) - end --- Assumes that spaceno has a TREE (NUM) primary key --- inserts a tuple after getting the next value of the --- primary key and returns it back to the user - space_mt.auto_increment = function(space, tuple) - check_space_arg(space, 'auto_increment') - local max_tuple = check_primary_index(space):max() - local max = 0 - if max_tuple ~= nil then - max = max_tuple[1] - end - table.insert(tuple, 1, max + 1) - return space:insert(tuple) - end - - space_mt.pairs = function(space, key, opts) - check_space_arg(space, 'pairs') - local pk = space.index[0] - if pk == nil then - -- empty space without indexes, return empty iterator - return fun.iter({}) - end - return pk:pairs(key, opts) - end - space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.truncate = function(space) - check_space_arg(space, 'truncate') - return internal.truncate(space.id) - end - space_mt.format = function(space, format) - check_space_arg(space, 'format') - return box.schema.space.format(space.id, format) - end - space_mt.drop = function(space) - check_space_arg(space, 'drop') - check_space_exists(space) - return box.schema.space.drop(space.id, space.name) - end - space_mt.rename = function(space, name) - check_space_arg(space, 'rename') - check_space_exists(space) - return box.schema.space.rename(space.id, name) - end - space_mt.create_index = function(space, name, options) - check_space_arg(space, 'create_index') - check_space_exists(space) - return box.schema.index.create(space.id, name, options) - end - space_mt.run_triggers = function(space, yesno) - check_space_arg(space, 'run_triggers') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - builtin.space_run_triggers(s, yesno) - end - space_mt.__index = space_mt - - setmetatable(space, space_mt) + setmetatable(space, box.schema.space_mt) if type(space.index) == 'table' and space.enabled then for j, index in pairs(space.index) do if type(j) == 'number' then diff --git a/test/box-tap/schema_mt.test.lua b/test/box-tap/schema_mt.test.lua new file mode 100755 index 000000000..7e531dd01 --- /dev/null +++ b/test/box-tap/schema_mt.test.lua @@ -0,0 +1,74 @@ +#!/usr/bin/env tarantool +-- +-- pr-3204: expose space_mt, index_mt into box.schema. +-- + +local tap = require('tap') +local test = tap.test('schema_mt') + +test:plan(7) + +box.cfg{ + log="tarantool.log", +} + +local sp1 = box.schema.space.create('test') +local sp2 = box.schema.space.create('test2') + +test:is( + getmetatable(sp1), + getmetatable(sp2), + 'all spaces use the same metatable' +) + +local idx1 = sp1:create_index('primary') +local idx2 = sp2:create_index('primary') + +test:isnt( + getmetatable(idx1), + getmetatable(idx2), + 'all indexes have their own metatable' +) + +test:is( + idx1.get, + idx2.get, + 'memtx indexes share read methods' +) + +local sp3 = box.schema.space.create('test3', {engine='vinyl'}) +local sp4 = box.schema.space.create('test4', {engine='vinyl'}) + +local idx3 = sp3:create_index('primary') +local idx4 = sp4:create_index('primary') + +test:is( + idx3.get, + idx4.get, + 'vinyl indexes share read methods' +) + +test:isnt( + idx1.get, + idx3.get, + 'memtx and vinyl indexes have separate read methods' +) + +function box.schema.space_mt.foo() end + +test:is( + sp1.foo, + box.schema.space_mt.foo, + 'box.schema.space_mt is mutable' +) + +function box.schema.index_mt.foo() end + +test:is( + idx1.foo, + box.schema.index_mt.foo, + 'box.schema.index_mt is mutable' +) + +test:check() +os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] schema: review fixes for box.schema.space/index metatables 2018-03-23 13:07 [PATCH 0/2] schema: expose space_mt and index_mt on table Vladislav Shpilevoy 2018-03-23 13:07 ` [PATCH 1/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy @ 2018-03-23 13:07 ` Vladislav Shpilevoy 2018-03-29 10:54 ` [PATCH 0/2] schema: expose space_mt and index_mt on table Vladimir Davydov 2 siblings, 0 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-03-23 13:07 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Follow up #3204 --- src/box/lua/schema.lua | 352 +++++++++++++++++++--------------------- test/box-tap/schema_mt.test.lua | 5 + 2 files changed, 173 insertions(+), 184 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 3fb626efd..c3a70db70 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1078,83 +1078,81 @@ local space_mt = {} space_mt.__index = space_mt box.schema.space_mt = space_mt -space_mt.len = function(space) - check_space_arg(space, 'len') - local pk = space.index[0] +function space_mt:len() + check_space_arg(self, 'len') + local pk = self.index[0] if pk == nil then return 0 -- empty space without indexes, return 0 end - return space.index[0]:len() + return self.index[0]:len() end -space_mt.count = function(space, key, opts) - check_space_arg(space, 'count') - local pk = space.index[0] +function space_mt:count(key, opts) + check_space_arg(self, 'count') + local pk = self.index[0] if pk == nil then return 0 -- empty space without indexes, return 0 end return pk:count(key, opts) end -space_mt.bsize = function(space) - check_space_arg(space, 'bsize') - local s = builtin.space_by_id(space.id) +function space_mt:bsize() + check_space_arg(self, 'bsize') + local s = builtin.space_by_id(self.id) if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) + box.error(box.error.NO_SUCH_SPACE, self.name) end return builtin.space_bsize(s) end - -space_mt.get = function(space, key) - check_space_arg(space, 'get') - return check_primary_index(space):get(key) +function space_mt:get(key) + check_space_arg(self, 'get') + return check_primary_index(self):get(key) end -space_mt.select = function(space, key, opts) - check_space_arg(space, 'select') - return check_primary_index(space):select(key, opts) +function space_mt:select(key, opts) + check_space_arg(self, 'select') + return check_primary_index(self):select(key, opts) end -space_mt.insert = function(space, tuple) - check_space_arg(space, 'insert') - return internal.insert(space.id, tuple); +function space_mt:insert(tuple) + check_space_arg(self, 'insert') + return internal.insert(self.id, tuple) end -space_mt.replace = function(space, tuple) - check_space_arg(space, 'replace') - return internal.replace(space.id, tuple); +function space_mt:replace(tuple) + check_space_arg(self, 'replace') + return internal.replace(self.id, tuple) end -space_mt.put = space_mt.replace; -- put is an alias for replace -space_mt.update = function(space, key, ops) - check_space_arg(space, 'update') - return check_primary_index(space):update(key, ops) +space_mt.put = space_mt.replace -- put is an alias for replace +function space_mt:update(key, ops) + check_space_arg(self, 'update') + return check_primary_index(self):update(key, ops) end -space_mt.upsert = function(space, tuple_key, ops, deprecated) - check_space_arg(space, 'upsert') +function space_mt:upsert(tuple_key, ops, deprecated) + check_space_arg(self, 'upsert') if deprecated ~= nil then local msg = "Error: extra argument in upsert call: " msg = msg .. tostring(deprecated) msg = msg .. ". Usage :upsert(tuple, operations)" box.error(box.error.PROC_LUA, msg) end - return internal.upsert(space.id, tuple_key, ops); + return internal.upsert(self.id, tuple_key, ops) end -space_mt.delete = function(space, key) - check_space_arg(space, 'delete') - return check_primary_index(space):delete(key) +function space_mt:delete(key) + check_space_arg(self, 'delete') + return check_primary_index(self):delete(key) end -- Assumes that spaceno has a TREE (NUM) primary key -- inserts a tuple after getting the next value of the -- primary key and returns it back to the user -space_mt.auto_increment = function(space, tuple) - check_space_arg(space, 'auto_increment') - local max_tuple = check_primary_index(space):max() +function space_mt:auto_increment(tuple) + check_space_arg(self, 'auto_increment') + local max_tuple = check_primary_index(self):max() local max = 0 if max_tuple ~= nil then max = max_tuple[1] end table.insert(tuple, 1, max + 1) - return space:insert(tuple) + return self:insert(tuple) end - -space_mt.pairs = function(space, key, opts) - check_space_arg(space, 'pairs') - local pk = space.index[0] +function space_mt:pairs(key, opts) + check_space_arg(self, 'pairs') + local pk = self.index[0] if pk == nil then -- empty space without indexes, return empty iterator return fun.iter({}) @@ -1163,34 +1161,34 @@ space_mt.pairs = function(space, key, opts) end space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility -space_mt.truncate = function(space) - check_space_arg(space, 'truncate') - return internal.truncate(space.id) -end -space_mt.format = function(space, format) - check_space_arg(space, 'format') - return box.schema.space.format(space.id, format) -end -space_mt.drop = function(space) - check_space_arg(space, 'drop') - check_space_exists(space) - return box.schema.space.drop(space.id, space.name) -end -space_mt.rename = function(space, name) - check_space_arg(space, 'rename') - check_space_exists(space) - return box.schema.space.rename(space.id, name) -end -space_mt.create_index = function(space, name, options) - check_space_arg(space, 'create_index') - check_space_exists(space) - return box.schema.index.create(space.id, name, options) -end -space_mt.run_triggers = function(space, yesno) - check_space_arg(space, 'run_triggers') - local s = builtin.space_by_id(space.id) +function space_mt:truncate() + check_space_arg(self, 'truncate') + return internal.truncate(self.id) +end +function space_mt:format(format) + check_space_arg(self, 'format') + return box.schema.space.format(self.id, format) +end +function space_mt:drop() + check_space_arg(self, 'drop') + check_space_exists(self) + return box.schema.space.drop(self.id, self.name) +end +function space_mt:rename(name) + check_space_arg(self, 'rename') + check_space_exists(self) + return box.schema.space.rename(self.id, name) +end +function space_mt:create_index(name, options) + check_space_arg(self, 'create_index') + check_space_exists(self) + return box.schema.index.create(self.id, name, options) +end +function space_mt:run_triggers(yesno) + check_space_arg(self, 'run_triggers') + local s = builtin.space_by_id(self.id) if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) + box.error(box.error.NO_SUCH_SPACE, self.name) end builtin.space_run_triggers(s, yesno) end @@ -1199,19 +1197,17 @@ local index_mt = {} index_mt.__index = index_mt box.schema.index_mt = index_mt --- __len and __index -index_mt.len = function(index) - check_index_arg(index, 'len') - local ret = builtin.box_index_len(index.space_id, index.id) +function index_mt:len() + check_index_arg(self, 'len') + local ret = builtin.box_index_len(self.space_id, self.id) if ret == -1 then box.error() end return tonumber(ret) end --- index.bsize -index_mt.bsize = function(index) - check_index_arg(index, 'bsize') - local ret = builtin.box_index_bsize(index.space_id, index.id) +function index_mt:bsize() + check_index_arg(self, 'bsize') + local ret = builtin.box_index_bsize(self.space_id, self.id) if ret == -1 then box.error() end @@ -1219,120 +1215,106 @@ index_mt.bsize = function(index) end index_mt.__len = index_mt.len -- Lua 5.2 compatibility index_mt.__index = index_mt --- min and max -index_mt.min_ffi = function(index, key) - check_index_arg(index, 'min') +function index_mt:min_ffi(key) + check_index_arg(self, 'min') local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_min(index.space_id, index.id, + if builtin.box_index_min(self.space_id, self.id, pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error + box.error() elseif ptuple[0] ~= nil then return tuple_bless(ptuple[0]) - else - return end end -index_mt.min_luac = function(index, key) - check_index_arg(index, 'min') +function index_mt:min_luac(key) + check_index_arg(self, 'min') key = keify(key) - return internal.min(index.space_id, index.id, key); + return internal.min(self.space_id, self.id, key) end -index_mt.max_ffi = function(index, key) - check_index_arg(index, 'max') +function index_mt:max_ffi(key) + check_index_arg(self, 'max') local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_max(index.space_id, index.id, + if builtin.box_index_max(self.space_id, self.id, pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error + box.error() elseif ptuple[0] ~= nil then return tuple_bless(ptuple[0]) - else - return end end -index_mt.max_luac = function(index, key) - check_index_arg(index, 'max') +function index_mt:max_luac(key) + check_index_arg(self, 'max') key = keify(key) - return internal.max(index.space_id, index.id, key); + return internal.max(self.space_id, self.id, key) end -index_mt.random_ffi = function(index, rnd) - check_index_arg(index, 'random') +function index_mt:random_ffi(rnd) + check_index_arg(self, 'random') rnd = rnd or math.random() - if builtin.box_index_random(index.space_id, index.id, rnd, - ptuple) ~= 0 then - box.error() -- error + if builtin.box_index_random(self.space_id, self.id, rnd, ptuple) ~= 0 then + box.error() elseif ptuple[0] ~= nil then return tuple_bless(ptuple[0]) - else - return end end -index_mt.random_luac = function(index, rnd) - check_index_arg(index, 'random') +function index_mt:random_luac(rnd) + check_index_arg(self, 'random') rnd = rnd or math.random() - return internal.random(index.space_id, index.id, rnd); + return internal.random(self.space_id, self.id, rnd) end --- iteration -index_mt.pairs_ffi = function(index, key, opts) - check_index_arg(index, 'pairs') +function index_mt:pairs_ffi(key, opts) + check_index_arg(self, 'pairs') local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end) local keybuf = ffi.string(pkey, pkey_end - pkey) local pkeybuf = ffi.cast('const char *', keybuf) - local cdata = builtin.box_index_iterator(index.space_id, index.id, - itype, pkeybuf, pkeybuf + #keybuf); + local cdata = builtin.box_index_iterator(self.space_id, self.id, itype, + pkeybuf, pkeybuf + #keybuf) if cdata == nil then box.error() end return fun.wrap(iterator_gen, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) + ffi.gc(cdata, builtin.box_iterator_free)) end -index_mt.pairs_luac = function(index, key, opts) - check_index_arg(index, 'pairs') +function index_mt:pairs_luac(key, opts) + check_index_arg(self, 'pairs') key = keify(key) - local itype = check_iterator_type(opts, #key == 0); + local itype = check_iterator_type(opts, #key == 0) local keymp = msgpack.encode(key) local keybuf = ffi.string(keymp, #keymp) - local cdata = internal.iterator(index.space_id, index.id, itype, keymp); + local cdata = internal.iterator(self.space_id, self.id, itype, keymp) return fun.wrap(iterator_gen_luac, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) + ffi.gc(cdata, builtin.box_iterator_free)) end - --- index subtree size -index_mt.count_ffi = function(index, key, opts) - check_index_arg(index, 'count') +function index_mt:count_ffi(key, opts) + check_index_arg(self, 'count') local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - local count = builtin.box_index_count(index.space_id, index.id, - itype, pkey, pkey_end); + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end) + local count = builtin.box_index_count(self.space_id, self.id, itype, pkey, + pkey_end) if count == -1 then box.error() end return tonumber(count) end -index_mt.count_luac = function(index, key, opts) - check_index_arg(index, 'count') +function index_mt:count_luac(key, opts) + check_index_arg(self, 'count') key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - return internal.count(index.space_id, index.id, itype, key); + local itype = check_iterator_type(opts, #key == 0) + return internal.count(self.space_id, self.id, itype, key) end - -index_mt.get_ffi = function(index, key) - check_index_arg(index, 'get') +function index_mt:get_ffi(key) + check_index_arg(self, 'get') local key, key_end = tuple_encode(key) - if builtin.box_index_get(index.space_id, index.id, - key, key_end, ptuple) ~= 0 then - return box.error() -- error + if builtin.box_index_get(self.space_id, self.id, key, key_end, + ptuple) ~= 0 then + return box.error() elseif ptuple[0] ~= nil then return tuple_bless(ptuple[0]) - else - return end end -index_mt.get_luac = function(index, key) - check_index_arg(index, 'get') +function index_mt:get_luac(key) + check_index_arg(self, 'get') key = keify(key) - return internal.get(index.space_id, index.id, key) + return internal.get(self.space_id, self.id, key) end local function check_select_opts(opts, key_is_nil) @@ -1350,15 +1332,15 @@ local function check_select_opts(opts, key_is_nil) return iterator, offset, limit end -index_mt.select_ffi = function(index, key, opts) - check_index_arg(index, 'select') +function index_mt:select_ffi(key, opts) + check_index_arg(self, 'select') local key, key_end = tuple_encode(key) local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) local port = ffi.cast('struct port *', port_tuple) - if builtin.box_select(index.space_id, index.id, - iterator, offset, limit, key, key_end, port) ~= 0 then + if builtin.box_select(self.space_id, self.id, iterator, offset, limit, key, + key_end, port) ~= 0 then return box.error() end @@ -1368,75 +1350,77 @@ index_mt.select_ffi = function(index, key, opts) ret[i] = tuple_bless(entry.tuple) entry = entry.next end - builtin.port_destroy(port); + builtin.port_destroy(port) return ret end - -index_mt.select_luac = function(index, key, opts) - check_index_arg(index, 'select') +function index_mt:select_luac(key, opts) + check_index_arg(self, 'select') local key = keify(key) local iterator, offset, limit = check_select_opts(opts, #key == 0) - return internal.select(index.space_id, index.id, iterator, - offset, limit, key) + return internal.select(self.space_id, self.id, iterator, offset, limit, key) end - -index_mt.update = function(index, key, ops) - check_index_arg(index, 'update') - return internal.update(index.space_id, index.id, keify(key), ops); +function index_mt:update(key, ops) + check_index_arg(self, 'update') + return internal.update(self.space_id, self.id, keify(key), ops) end -index_mt.delete = function(index, key) - check_index_arg(index, 'delete') - return internal.delete(index.space_id, index.id, keify(key)); +function index_mt:delete(key) + check_index_arg(self, 'delete') + return internal.delete(self.space_id, self.id, keify(key)) end - -index_mt.info = function(index) - return internal.info(index.space_id, index.id); +function index_mt:info() + return internal.info(self.space_id, self.id) end - -index_mt.drop = function(index) - check_index_arg(index, 'drop') - return box.schema.index.drop(index.space_id, index.id) +function index_mt:drop() + check_index_arg(self, 'drop') + return box.schema.index.drop(self.space_id, self.id) end -index_mt.rename = function(index, name) - check_index_arg(index, 'rename') - return box.schema.index.rename(index.space_id, index.id, name) +function index_mt:rename(name) + check_index_arg(self, 'rename') + return box.schema.index.rename(self.space_id, self.id, name) end -index_mt.alter = function(index, options) - check_index_arg(index, 'alter') - if index.id == nil or index.space_id == nil then +function index_mt:alter(options) + check_index_arg(self, 'alter') + if self.id == nil or self.space_id == nil then box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") end - return box.schema.index.alter(index.space_id, index.id, options) + return box.schema.index.alter(self.space_id, self.id, options) end -index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility -index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility - function box.schema.space.bless(space) - local index_mt = {} - index_mt.__index = function(index, key) - return index_mt[key] or box.schema.index_mt[key] + local local_index_mt = {} + -- At first, reference all global index functions. At second, + -- choose an implementation for read operations. They are not + -- in a global index_mt, because they are engine specific. + -- All common index_mt functions must be referenced here to + -- be able to get them using getmetatable(index_object) - if + -- they are only in index_mt, then getmetatable() returns only + -- read ops. + for k, v in pairs(index_mt) do + local_index_mt[k] = v + end + local_index_mt.__index = function(index, key) + return local_index_mt[key] or index_mt[key] end - - -- true if reading operations may yield local read_yields = space.engine == 'vinyl' local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} for _, op in ipairs(read_ops) do if read_yields then -- use Lua/C implmenetation - index_mt[op] = box.schema.index_mt[op .. "_luac"] + local_index_mt[op] = local_index_mt[op .. "_luac"] else -- use FFI implementation - index_mt[op] = box.schema.index_mt[op .. "_ffi"] + local_index_mt[op] = local_index_mt[op .. "_ffi"] end end - -- + -- Lua 5.2 compatibility + local_index_mt.__pairs = local_index_mt.pairs + local_index_mt.__ipairs = local_index_mt.ipairs - setmetatable(space, box.schema.space_mt) + setmetatable(space, space_mt) if type(space.index) == 'table' and space.enabled then for j, index in pairs(space.index) do if type(j) == 'number' then - setmetatable(index, index_mt) + setmetatable(index, local_index_mt) end end end diff --git a/test/box-tap/schema_mt.test.lua b/test/box-tap/schema_mt.test.lua index 7e531dd01..096c4ad60 100755 --- a/test/box-tap/schema_mt.test.lua +++ b/test/box-tap/schema_mt.test.lua @@ -71,4 +71,9 @@ test:is( ) test:check() + +sp1:drop() +sp2:drop() +sp3:drop() +sp4:drop() os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 0/2] schema: expose space_mt and index_mt on table 2018-03-23 13:07 [PATCH 0/2] schema: expose space_mt and index_mt on table Vladislav Shpilevoy 2018-03-23 13:07 ` [PATCH 1/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy 2018-03-23 13:07 ` [PATCH 2/2] schema: review fixes for box.schema.space/index metatables Vladislav Shpilevoy @ 2018-03-29 10:54 ` Vladimir Davydov 2018-03-29 14:31 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2 siblings, 1 reply; 20+ messages in thread From: Vladimir Davydov @ 2018-03-29 10:54 UTC (permalink / raw) To: Vladislav Shpilevoy, Konstantin Osipov; +Cc: tarantool-patches On Fri, Mar 23, 2018 at 04:07:07PM +0300, Vladislav Shpilevoy wrote: > Branch: http://github.com/tarantool/tarantool/tree/pr-3204-expose-space-index-mt > Issue: https://github.com/tarantool/tarantool/issues/3204 > > Some of users want to extend space and index methods with their > own, but do not want to do it for each space, because a metatable > are created individually for a space. I don't understand why anyone would do that. If one wants to implement some extra methods, they can always create a wrapper in Lua. Overwriting a metatable for this looks like a dirty hack. Anyway, the patch is nearly impossible to review, because it is basically a 100% diff. If you still think it's useful and really want it to get reviewed, please split it properly: first move index_mt and space_mt definitions from box.schema.space.bless without introducing any functional changes, then export them. > > Move these metatables to box.schema, and lookup into them, when > a method was not found in an original metatable. > > Vladislav Shpilevoy (1): > schema: review fixes for box.schema.space/index metatables > > aleclarson (1): > schema: expose space_mt and index_mt on `box.schema` table > > src/box/lua/schema.lua | 631 ++++++++++++++++++++-------------------- > test/box-tap/schema_mt.test.lua | 79 +++++ > 2 files changed, 391 insertions(+), 319 deletions(-) > create mode 100755 test/box-tap/schema_mt.test.lua ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table 2018-03-29 10:54 ` [PATCH 0/2] schema: expose space_mt and index_mt on table Vladimir Davydov @ 2018-03-29 14:31 ` Vladislav Shpilevoy 2018-03-29 14:31 ` [PATCH v2 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-03-29 14:31 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Branch: http://github.com/tarantool/tarantool/tree/pr-3204-expose-space-index-mt Issue: https://github.com/tarantool/tarantool/issues/3204 Vladislav Shpilevoy (1): schema: expose space_mt and index_mt on `box.schema` table aleclarson (1): schema: move space_mt and index_mt definition out of space bless src/box/lua/schema.lua | 631 ++++++++++++++++++++-------------------- test/box-tap/schema_mt.test.lua | 79 +++++ 2 files changed, 391 insertions(+), 319 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/2] schema: move space_mt and index_mt definition out of space bless 2018-03-29 14:31 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy @ 2018-03-29 14:31 ` Vladislav Shpilevoy 2018-04-01 9:33 ` Vladimir Davydov 2018-03-29 14:31 ` [PATCH v2 2/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy 2018-04-01 9:37 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladimir Davydov 2 siblings, 1 reply; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-03-29 14:31 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, aleclarson From: aleclarson <alec.stanford.larson@gmail.com> Space_mt and index_mt are created individually for each space inside a giant space.bless function. It makes impossible to implement #3204: exposing space and index metatables to a user, because for this they must be global and shared between all spaces and indexes. Lets move their definition out of space.bless function, and do their duplicate inside. Needed #3204 --- src/box/lua/schema.lua | 608 +++++++++++++++++++--------------------- test/box-tap/schema_mt.test.lua | 79 ++++++ 2 files changed, 373 insertions(+), 314 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 5a0f71559..ff89c8e58 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1074,221 +1074,321 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box -function box.schema.space.bless(space) - local index_mt = {} - -- __len and __index - index_mt.len = function(index) - check_index_arg(index, 'len') - local ret = builtin.box_index_len(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - -- index.bsize - index_mt.bsize = function(index) - check_index_arg(index, 'bsize') - local ret = builtin.box_index_bsize(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - index_mt.__len = index_mt.len -- Lua 5.2 compatibility - index_mt.__newindex = function(table, index) - return error('Attempt to modify a read-only table') end - index_mt.__index = index_mt - -- min and max - index_mt.min_ffi = function(index, key) - check_index_arg(index, 'min') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_min(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end +local space_mt = {} +space_mt.__index = space_mt +box.schema.space_mt = space_mt + +function space_mt:len() + check_space_arg(self, 'len') + local pk = self.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - index_mt.min_luac = function(index, key) - check_index_arg(index, 'min') - key = keify(key) - return internal.min(index.space_id, index.id, key); + return self.index[0]:len() +end +function space_mt:count(key, opts) + check_space_arg(self, 'count') + local pk = self.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - index_mt.max_ffi = function(index, key) - check_index_arg(index, 'max') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_max(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + return pk:count(key, opts) +end +function space_mt:bsize() + check_space_arg(self, 'bsize') + local s = builtin.space_by_id(self.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, self.name) end - index_mt.max_luac = function(index, key) - check_index_arg(index, 'max') - key = keify(key) - return internal.max(index.space_id, index.id, key); + return builtin.space_bsize(s) +end +function space_mt:get(key) + check_space_arg(self, 'get') + return check_primary_index(self):get(key) +end +function space_mt:select(key, opts) + check_space_arg(self, 'select') + return check_primary_index(self):select(key, opts) +end +function space_mt:insert(tuple) + check_space_arg(self, 'insert') + return internal.insert(self.id, tuple) +end +function space_mt:replace(tuple) + check_space_arg(self, 'replace') + return internal.replace(self.id, tuple) +end +space_mt.put = space_mt.replace -- put is an alias for replace +function space_mt:update(key, ops) + check_space_arg(self, 'update') + return check_primary_index(self):update(key, ops) +end +function space_mt:upsert(tuple_key, ops, deprecated) + check_space_arg(self, 'upsert') + if deprecated ~= nil then + local msg = "Error: extra argument in upsert call: " + msg = msg .. tostring(deprecated) + msg = msg .. ". Usage :upsert(tuple, operations)" + box.error(box.error.PROC_LUA, msg) end - index_mt.random_ffi = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - if builtin.box_index_random(index.space_id, index.id, rnd, - ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + return internal.upsert(self.id, tuple_key, ops) +end +function space_mt:delete(key) + check_space_arg(self, 'delete') + return check_primary_index(self):delete(key) +end +-- Assumes that spaceno has a TREE (NUM) primary key +-- inserts a tuple after getting the next value of the +-- primary key and returns it back to the user +function space_mt:auto_increment(tuple) + check_space_arg(self, 'auto_increment') + local max_tuple = check_primary_index(self):max() + local max = 0 + if max_tuple ~= nil then + max = max_tuple[1] + end + table.insert(tuple, 1, max + 1) + return self:insert(tuple) +end +function space_mt:pairs(key, opts) + check_space_arg(self, 'pairs') + local pk = self.index[0] + if pk == nil then + -- empty space without indexes, return empty iterator + return fun.iter({}) + end + return pk:pairs(key, opts) +end +space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility +space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility +function space_mt:truncate() + check_space_arg(self, 'truncate') + return internal.truncate(self.id) +end +function space_mt:format(format) + check_space_arg(self, 'format') + return box.schema.space.format(self.id, format) +end +function space_mt:drop() + check_space_arg(self, 'drop') + check_space_exists(self) + return box.schema.space.drop(self.id, self.name) +end +function space_mt:rename(name) + check_space_arg(self, 'rename') + check_space_exists(self) + return box.schema.space.rename(self.id, name) +end +function space_mt:create_index(name, options) + check_space_arg(self, 'create_index') + check_space_exists(self) + return box.schema.index.create(self.id, name, options) +end +function space_mt:run_triggers(yesno) + check_space_arg(self, 'run_triggers') + local s = builtin.space_by_id(self.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, self.name) end - index_mt.random_luac = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - return internal.random(index.space_id, index.id, rnd); - end - -- iteration - index_mt.pairs_ffi = function(index, key, opts) - check_index_arg(index, 'pairs') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - - local keybuf = ffi.string(pkey, pkey_end - pkey) - local pkeybuf = ffi.cast('const char *', keybuf) - local cdata = builtin.box_index_iterator(index.space_id, index.id, - itype, pkeybuf, pkeybuf + #keybuf); - if cdata == nil then - box.error() - end - return fun.wrap(iterator_gen, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - index_mt.pairs_luac = function(index, key, opts) - check_index_arg(index, 'pairs') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - local keymp = msgpack.encode(key) - local keybuf = ffi.string(keymp, #keymp) - local cdata = internal.iterator(index.space_id, index.id, itype, keymp); - return fun.wrap(iterator_gen_luac, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - - -- index subtree size - index_mt.count_ffi = function(index, key, opts) - check_index_arg(index, 'count') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - local count = builtin.box_index_count(index.space_id, index.id, - itype, pkey, pkey_end); - if count == -1 then - box.error() - end - return tonumber(count) - end - index_mt.count_luac = function(index, key, opts) - check_index_arg(index, 'count') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - return internal.count(index.space_id, index.id, itype, key); - end - - index_mt.get_ffi = function(index, key) - check_index_arg(index, 'get') - local key, key_end = tuple_encode(key) - if builtin.box_index_get(index.space_id, index.id, - key, key_end, ptuple) ~= 0 then - return box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + builtin.space_run_triggers(s, yesno) +end + +local index_mt = {} +index_mt.__index = index_mt +box.schema.index_mt = index_mt + +function index_mt:len() + check_index_arg(self, 'len') + local ret = builtin.box_index_len(self.space_id, self.id) + if ret == -1 then + box.error() end - index_mt.get_luac = function(index, key) - check_index_arg(index, 'get') - key = keify(key) - return internal.get(index.space_id, index.id, key) + return tonumber(ret) +end +function index_mt:bsize() + check_index_arg(self, 'bsize') + local ret = builtin.box_index_bsize(self.space_id, self.id) + if ret == -1 then + box.error() end - - local function check_select_opts(opts, key_is_nil) - local offset = 0 - local limit = 4294967295 - local iterator = check_iterator_type(opts, key_is_nil) - if opts ~= nil then - if opts.offset ~= nil then - offset = opts.offset - end - if opts.limit ~= nil then - limit = opts.limit - end - end - return iterator, offset, limit + return tonumber(ret) +end +index_mt.__len = index_mt.len -- Lua 5.2 compatibility +index_mt.__index = index_mt +function index_mt:min_ffi(key) + check_index_arg(self, 'min') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_min(self.space_id, self.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) end +end +function index_mt:min_luac(key) + check_index_arg(self, 'min') + key = keify(key) + return internal.min(self.space_id, self.id, key) +end +function index_mt:max_ffi(key) + check_index_arg(self, 'max') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_max(self.space_id, self.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + end +end +function index_mt:max_luac(key) + check_index_arg(self, 'max') + key = keify(key) + return internal.max(self.space_id, self.id, key) +end +function index_mt:random_ffi(rnd) + check_index_arg(self, 'random') + rnd = rnd or math.random() + if builtin.box_index_random(self.space_id, self.id, rnd, ptuple) ~= 0 then + box.error() + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + end +end +function index_mt:random_luac(rnd) + check_index_arg(self, 'random') + rnd = rnd or math.random() + return internal.random(self.space_id, self.id, rnd) +end +function index_mt:pairs_ffi(key, opts) + check_index_arg(self, 'pairs') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end) - index_mt.select_ffi = function(index, key, opts) - check_index_arg(index, 'select') - local key, key_end = tuple_encode(key) - local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) - - local port = ffi.cast('struct port *', port_tuple) + local keybuf = ffi.string(pkey, pkey_end - pkey) + local pkeybuf = ffi.cast('const char *', keybuf) + local cdata = builtin.box_index_iterator(self.space_id, self.id, itype, + pkeybuf, pkeybuf + #keybuf) + if cdata == nil then + box.error() + end + return fun.wrap(iterator_gen, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end +function index_mt:pairs_luac(key, opts) + check_index_arg(self, 'pairs') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0) + local keymp = msgpack.encode(key) + local keybuf = ffi.string(keymp, #keymp) + local cdata = internal.iterator(self.space_id, self.id, itype, keymp) + return fun.wrap(iterator_gen_luac, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end +function index_mt:count_ffi(key, opts) + check_index_arg(self, 'count') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end) + local count = builtin.box_index_count(self.space_id, self.id, itype, pkey, + pkey_end) + if count == -1 then + box.error() + end + return tonumber(count) +end +function index_mt:count_luac(key, opts) + check_index_arg(self, 'count') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0) + return internal.count(self.space_id, self.id, itype, key) +end +function index_mt:get_ffi(key) + check_index_arg(self, 'get') + local key, key_end = tuple_encode(key) + if builtin.box_index_get(self.space_id, self.id, key, key_end, + ptuple) ~= 0 then + return box.error() + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + end +end +function index_mt:get_luac(key) + check_index_arg(self, 'get') + key = keify(key) + return internal.get(self.space_id, self.id, key) +end - if builtin.box_select(index.space_id, index.id, - iterator, offset, limit, key, key_end, port) ~= 0 then - return box.error() +local function check_select_opts(opts, key_is_nil) + local offset = 0 + local limit = 4294967295 + local iterator = check_iterator_type(opts, key_is_nil) + if opts ~= nil then + if opts.offset ~= nil then + offset = opts.offset end - - local ret = {} - local entry = port_tuple.first - for i=1,tonumber(port_tuple.size),1 do - ret[i] = tuple_bless(entry.tuple) - entry = entry.next + if opts.limit ~= nil then + limit = opts.limit end - builtin.port_destroy(port); - return ret end + return iterator, offset, limit +end - index_mt.select_luac = function(index, key, opts) - check_index_arg(index, 'select') - local key = keify(key) - local iterator, offset, limit = check_select_opts(opts, #key == 0) - return internal.select(index.space_id, index.id, iterator, - offset, limit, key) - end +function index_mt:select_ffi(key, opts) + check_index_arg(self, 'select') + local key, key_end = tuple_encode(key) + local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) - index_mt.update = function(index, key, ops) - check_index_arg(index, 'update') - return internal.update(index.space_id, index.id, keify(key), ops); - end - index_mt.delete = function(index, key) - check_index_arg(index, 'delete') - return internal.delete(index.space_id, index.id, keify(key)); - end + local port = ffi.cast('struct port *', port_tuple) - index_mt.info = function(index) - return internal.info(index.space_id, index.id); + if builtin.box_select(self.space_id, self.id, iterator, offset, limit, key, + key_end, port) ~= 0 then + return box.error() end - index_mt.drop = function(index) - check_index_arg(index, 'drop') - return box.schema.index.drop(index.space_id, index.id) - end - index_mt.rename = function(index, name) - check_index_arg(index, 'rename') - return box.schema.index.rename(index.space_id, index.id, name) + local ret = {} + local entry = port_tuple.first + for i=1,tonumber(port_tuple.size),1 do + ret[i] = tuple_bless(entry.tuple) + entry = entry.next end - index_mt.alter = function(index, options) - check_index_arg(index, 'alter') - if index.id == nil or index.space_id == nil then - box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") - end - return box.schema.index.alter(index.space_id, index.id, options) + builtin.port_destroy(port) + return ret +end +function index_mt:select_luac(key, opts) + check_index_arg(self, 'select') + local key = keify(key) + local iterator, offset, limit = check_select_opts(opts, #key == 0) + return internal.select(self.space_id, self.id, iterator, offset, limit, key) +end +function index_mt:update(key, ops) + check_index_arg(self, 'update') + return internal.update(self.space_id, self.id, keify(key), ops) +end +function index_mt:delete(key) + check_index_arg(self, 'delete') + return internal.delete(self.space_id, self.id, keify(key)) +end +function index_mt:info() + return internal.info(self.space_id, self.id) +end +function index_mt:drop() + check_index_arg(self, 'drop') + return box.schema.index.drop(self.space_id, self.id) +end +function index_mt:rename(name) + check_index_arg(self, 'rename') + return box.schema.index.rename(self.space_id, self.id, name) +end +function index_mt:alter(options) + check_index_arg(self, 'alter') + if self.id == nil or self.space_id == nil then + box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") end + return box.schema.index.alter(self.space_id, self.id, options) +end - -- true if reading operations may yield +function box.schema.space.bless(space) + local space_mt = table.deepcopy(space_mt) + local index_mt = table.deepcopy(index_mt) local read_yields = space.engine == 'vinyl' local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} for _, op in ipairs(read_ops) do @@ -1302,126 +1402,6 @@ function box.schema.space.bless(space) end index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility - -- - local space_mt = {} - space_mt.len = function(space) - check_space_arg(space, 'len') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return space.index[0]:len() - end - space_mt.count = function(space, key, opts) - check_space_arg(space, 'count') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return pk:count(key, opts) - end - space_mt.bsize = function(space) - check_space_arg(space, 'bsize') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - return builtin.space_bsize(s) - end - space_mt.__newindex = index_mt.__newindex - - space_mt.get = function(space, key) - check_space_arg(space, 'get') - return check_primary_index(space):get(key) - end - space_mt.select = function(space, key, opts) - check_space_arg(space, 'select') - return check_primary_index(space):select(key, opts) - end - space_mt.insert = function(space, tuple) - check_space_arg(space, 'insert') - return internal.insert(space.id, tuple); - end - space_mt.replace = function(space, tuple) - check_space_arg(space, 'replace') - return internal.replace(space.id, tuple); - end - space_mt.put = space_mt.replace; -- put is an alias for replace - space_mt.update = function(space, key, ops) - check_space_arg(space, 'update') - return check_primary_index(space):update(key, ops) - end - space_mt.upsert = function(space, tuple_key, ops, deprecated) - check_space_arg(space, 'upsert') - if deprecated ~= nil then - local msg = "Error: extra argument in upsert call: " - msg = msg .. tostring(deprecated) - msg = msg .. ". Usage :upsert(tuple, operations)" - box.error(box.error.PROC_LUA, msg) - end - return internal.upsert(space.id, tuple_key, ops); - end - space_mt.delete = function(space, key) - check_space_arg(space, 'delete') - return check_primary_index(space):delete(key) - end --- Assumes that spaceno has a TREE (NUM) primary key --- inserts a tuple after getting the next value of the --- primary key and returns it back to the user - space_mt.auto_increment = function(space, tuple) - check_space_arg(space, 'auto_increment') - local max_tuple = check_primary_index(space):max() - local max = 0 - if max_tuple ~= nil then - max = max_tuple[1] - end - table.insert(tuple, 1, max + 1) - return space:insert(tuple) - end - - space_mt.pairs = function(space, key, opts) - check_space_arg(space, 'pairs') - local pk = space.index[0] - if pk == nil then - -- empty space without indexes, return empty iterator - return fun.iter({}) - end - return pk:pairs(key, opts) - end - space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.truncate = function(space) - check_space_arg(space, 'truncate') - return internal.truncate(space.id) - end - space_mt.format = function(space, format) - check_space_arg(space, 'format') - return box.schema.space.format(space.id, format) - end - space_mt.drop = function(space) - check_space_arg(space, 'drop') - check_space_exists(space) - return box.schema.space.drop(space.id, space.name) - end - space_mt.rename = function(space, name) - check_space_arg(space, 'rename') - check_space_exists(space) - return box.schema.space.rename(space.id, name) - end - space_mt.create_index = function(space, name, options) - check_space_arg(space, 'create_index') - check_space_exists(space) - return box.schema.index.create(space.id, name, options) - end - space_mt.run_triggers = function(space, yesno) - check_space_arg(space, 'run_triggers') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - builtin.space_run_triggers(s, yesno) - end - space_mt.__index = space_mt setmetatable(space, space_mt) if type(space.index) == 'table' and space.enabled then diff --git a/test/box-tap/schema_mt.test.lua b/test/box-tap/schema_mt.test.lua new file mode 100755 index 000000000..096c4ad60 --- /dev/null +++ b/test/box-tap/schema_mt.test.lua @@ -0,0 +1,79 @@ +#!/usr/bin/env tarantool +-- +-- pr-3204: expose space_mt, index_mt into box.schema. +-- + +local tap = require('tap') +local test = tap.test('schema_mt') + +test:plan(7) + +box.cfg{ + log="tarantool.log", +} + +local sp1 = box.schema.space.create('test') +local sp2 = box.schema.space.create('test2') + +test:is( + getmetatable(sp1), + getmetatable(sp2), + 'all spaces use the same metatable' +) + +local idx1 = sp1:create_index('primary') +local idx2 = sp2:create_index('primary') + +test:isnt( + getmetatable(idx1), + getmetatable(idx2), + 'all indexes have their own metatable' +) + +test:is( + idx1.get, + idx2.get, + 'memtx indexes share read methods' +) + +local sp3 = box.schema.space.create('test3', {engine='vinyl'}) +local sp4 = box.schema.space.create('test4', {engine='vinyl'}) + +local idx3 = sp3:create_index('primary') +local idx4 = sp4:create_index('primary') + +test:is( + idx3.get, + idx4.get, + 'vinyl indexes share read methods' +) + +test:isnt( + idx1.get, + idx3.get, + 'memtx and vinyl indexes have separate read methods' +) + +function box.schema.space_mt.foo() end + +test:is( + sp1.foo, + box.schema.space_mt.foo, + 'box.schema.space_mt is mutable' +) + +function box.schema.index_mt.foo() end + +test:is( + idx1.foo, + box.schema.index_mt.foo, + 'box.schema.index_mt is mutable' +) + +test:check() + +sp1:drop() +sp2:drop() +sp3:drop() +sp4:drop() +os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/2] schema: move space_mt and index_mt definition out of space bless 2018-03-29 14:31 ` [PATCH v2 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy @ 2018-04-01 9:33 ` Vladimir Davydov 2018-04-01 11:02 ` [PATCH 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 0 siblings, 1 reply; 20+ messages in thread From: Vladimir Davydov @ 2018-04-01 9:33 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches, Konstantin Osipov For the record, as I said before > I don't understand why anyone would do that. If one wants to > implement some extra methods, they can always create a wrapper > in Lua. Overwriting a metatable for this looks like a dirty hack. so this is a purely technical review. On Thu, Mar 29, 2018 at 05:31:44PM +0300, Vladislav Shpilevoy wrote: > +function space_mt:len() > + check_space_arg(self, 'len') > + local pk = self.index[0] > + if pk == nil then > + return 0 -- empty space without indexes, return 0 > end > +end > - space_mt.len = function(space) > - check_space_arg(space, 'len') > - local pk = space.index[0] > - if pk == nil then > - return 0 -- empty space without indexes, return 0 > - end > - return space.index[0]:len() > - end This isn't what I meant when I wrote > move index_mt and space_mt definitions from box.schema.space.bless > without introducing any functional changes, then export them I meant that in the first patch you should just move these functions to the top-level, without doing ANY changes to them, otherwise the patch will be rather difficult to review. Anyway, I don't think the piece of syntax sugar you're trying to push (passing the object to a class method implicitly) is really necessary. We never do it in schema.lua, why do it for space_mt/index_mt now? And it's definitely doesn't belong to this patch. Please remove. > diff --git a/test/box-tap/schema_mt.test.lua b/test/box-tap/schema_mt.test.lua The test should go to patch 2, the one that exports the metatables. ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 0/2] schema: expose space_mt and index_mt on box.schema table 2018-04-01 9:33 ` Vladimir Davydov @ 2018-04-01 11:02 ` Vladislav Shpilevoy 2018-04-01 11:02 ` [PATCH 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy 2018-04-01 11:02 ` [PATCH 2/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy 0 siblings, 2 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-04-01 11:02 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Branch: http://github.com/tarantool/tarantool/tree/pr-3204-expose-space-index-mt Issue: https://github.com/tarantool/tarantool/issues/3204 Space_mt and index_mt are created individually for each space inside a giant space.bless function. It consumes lua memory by duplicating these metatables (which is very limited), and does not allow to add new functions, which would be visible in all spaces and indexes. Lets move space_mt and index_mt definitions out of space.bless function in box.schema.space/index_mt, and allow to expand them with user functions. Vladislav Shpilevoy (2): schema: move space_mt and index_mt definition out of space bless schema: expose space_mt and index_mt on `box.schema` table src/box/lua/schema.lua | 636 ++++++++++++++++++++-------------------- test/box-tap/schema_mt.test.lua | 79 +++++ 2 files changed, 404 insertions(+), 311 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 1/2] schema: move space_mt and index_mt definition out of space bless 2018-04-01 11:02 ` [PATCH 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy @ 2018-04-01 11:02 ` Vladislav Shpilevoy 2018-04-01 11:02 ` [PATCH 2/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy 1 sibling, 0 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-04-01 11:02 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Space_mt and index_mt are created individually for each space inside a giant space.bless function. It makes impossible to implement #3204: exposing space and index metatables to a user, because for this they must be global and shared between all spaces and indexes. Lets move their definition out of space.bless function, and do their duplicate inside. Needed #3204 --- src/box/lua/schema.lua | 619 +++++++++++++++++++++++++------------------------ 1 file changed, 311 insertions(+), 308 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 5a0f71559..8b6f69ad8 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1074,220 +1074,343 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box -function box.schema.space.bless(space) - local index_mt = {} - -- __len and __index - index_mt.len = function(index) - check_index_arg(index, 'len') - local ret = builtin.box_index_len(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - -- index.bsize - index_mt.bsize = function(index) - check_index_arg(index, 'bsize') - local ret = builtin.box_index_bsize(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - index_mt.__len = index_mt.len -- Lua 5.2 compatibility - index_mt.__newindex = function(table, index) - return error('Attempt to modify a read-only table') end - index_mt.__index = index_mt - -- min and max - index_mt.min_ffi = function(index, key) - check_index_arg(index, 'min') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_min(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end +local index_mt = {} +-- __len and __index +index_mt.len = function(index) + check_index_arg(index, 'len') + local ret = builtin.box_index_len(index.space_id, index.id) + if ret == -1 then + box.error() end - index_mt.min_luac = function(index, key) - check_index_arg(index, 'min') - key = keify(key) - return internal.min(index.space_id, index.id, key); + return tonumber(ret) +end +-- index.bsize +index_mt.bsize = function(index) + check_index_arg(index, 'bsize') + local ret = builtin.box_index_bsize(index.space_id, index.id) + if ret == -1 then + box.error() end - index_mt.max_ffi = function(index, key) - check_index_arg(index, 'max') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_max(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + return tonumber(ret) +end +index_mt.__len = index_mt.len -- Lua 5.2 compatibility +index_mt.__newindex = function(table, index) + return error('Attempt to modify a read-only table') +end +index_mt.__index = index_mt +-- min and max +index_mt.min_ffi = function(index, key) + check_index_arg(index, 'min') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_min(index.space_id, index.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return end - index_mt.max_luac = function(index, key) - check_index_arg(index, 'max') - key = keify(key) - return internal.max(index.space_id, index.id, key); +end +index_mt.min_luac = function(index, key) + check_index_arg(index, 'min') + key = keify(key) + return internal.min(index.space_id, index.id, key); +end +index_mt.max_ffi = function(index, key) + check_index_arg(index, 'max') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_max(index.space_id, index.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return end - index_mt.random_ffi = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - if builtin.box_index_random(index.space_id, index.id, rnd, - ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end +end +index_mt.max_luac = function(index, key) + check_index_arg(index, 'max') + key = keify(key) + return internal.max(index.space_id, index.id, key); +end +index_mt.random_ffi = function(index, rnd) + check_index_arg(index, 'random') + rnd = rnd or math.random() + if builtin.box_index_random(index.space_id, index.id, rnd, + ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return end - index_mt.random_luac = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - return internal.random(index.space_id, index.id, rnd); - end - -- iteration - index_mt.pairs_ffi = function(index, key, opts) - check_index_arg(index, 'pairs') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - - local keybuf = ffi.string(pkey, pkey_end - pkey) - local pkeybuf = ffi.cast('const char *', keybuf) - local cdata = builtin.box_index_iterator(index.space_id, index.id, - itype, pkeybuf, pkeybuf + #keybuf); - if cdata == nil then - box.error() - end - return fun.wrap(iterator_gen, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - index_mt.pairs_luac = function(index, key, opts) - check_index_arg(index, 'pairs') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - local keymp = msgpack.encode(key) - local keybuf = ffi.string(keymp, #keymp) - local cdata = internal.iterator(index.space_id, index.id, itype, keymp); - return fun.wrap(iterator_gen_luac, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - - -- index subtree size - index_mt.count_ffi = function(index, key, opts) - check_index_arg(index, 'count') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - local count = builtin.box_index_count(index.space_id, index.id, - itype, pkey, pkey_end); - if count == -1 then - box.error() - end - return tonumber(count) - end - index_mt.count_luac = function(index, key, opts) - check_index_arg(index, 'count') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - return internal.count(index.space_id, index.id, itype, key); - end - - index_mt.get_ffi = function(index, key) - check_index_arg(index, 'get') - local key, key_end = tuple_encode(key) - if builtin.box_index_get(index.space_id, index.id, - key, key_end, ptuple) ~= 0 then - return box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end +end +index_mt.random_luac = function(index, rnd) + check_index_arg(index, 'random') + rnd = rnd or math.random() + return internal.random(index.space_id, index.id, rnd); +end +-- iteration +index_mt.pairs_ffi = function(index, key, opts) + check_index_arg(index, 'pairs') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); + + local keybuf = ffi.string(pkey, pkey_end - pkey) + local pkeybuf = ffi.cast('const char *', keybuf) + local cdata = builtin.box_index_iterator(index.space_id, index.id, + itype, pkeybuf, pkeybuf + #keybuf); + if cdata == nil then + box.error() end - index_mt.get_luac = function(index, key) - check_index_arg(index, 'get') - key = keify(key) - return internal.get(index.space_id, index.id, key) + return fun.wrap(iterator_gen, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end +index_mt.pairs_luac = function(index, key, opts) + check_index_arg(index, 'pairs') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0); + local keymp = msgpack.encode(key) + local keybuf = ffi.string(keymp, #keymp) + local cdata = internal.iterator(index.space_id, index.id, itype, keymp); + return fun.wrap(iterator_gen_luac, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end + +-- index subtree size +index_mt.count_ffi = function(index, key, opts) + check_index_arg(index, 'count') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); + local count = builtin.box_index_count(index.space_id, index.id, + itype, pkey, pkey_end); + if count == -1 then + box.error() end + return tonumber(count) +end +index_mt.count_luac = function(index, key, opts) + check_index_arg(index, 'count') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0); + return internal.count(index.space_id, index.id, itype, key); +end - local function check_select_opts(opts, key_is_nil) - local offset = 0 - local limit = 4294967295 - local iterator = check_iterator_type(opts, key_is_nil) - if opts ~= nil then - if opts.offset ~= nil then - offset = opts.offset - end - if opts.limit ~= nil then - limit = opts.limit - end +index_mt.get_ffi = function(index, key) + check_index_arg(index, 'get') + local key, key_end = tuple_encode(key) + if builtin.box_index_get(index.space_id, index.id, + key, key_end, ptuple) ~= 0 then + return box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return + end +end +index_mt.get_luac = function(index, key) + check_index_arg(index, 'get') + key = keify(key) + return internal.get(index.space_id, index.id, key) +end + +local function check_select_opts(opts, key_is_nil) + local offset = 0 + local limit = 4294967295 + local iterator = check_iterator_type(opts, key_is_nil) + if opts ~= nil then + if opts.offset ~= nil then + offset = opts.offset + end + if opts.limit ~= nil then + limit = opts.limit end - return iterator, offset, limit end + return iterator, offset, limit +end - index_mt.select_ffi = function(index, key, opts) - check_index_arg(index, 'select') - local key, key_end = tuple_encode(key) - local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) +index_mt.select_ffi = function(index, key, opts) + check_index_arg(index, 'select') + local key, key_end = tuple_encode(key) + local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) - local port = ffi.cast('struct port *', port_tuple) + local port = ffi.cast('struct port *', port_tuple) - if builtin.box_select(index.space_id, index.id, - iterator, offset, limit, key, key_end, port) ~= 0 then - return box.error() - end + if builtin.box_select(index.space_id, index.id, + iterator, offset, limit, key, key_end, port) ~= 0 then + return box.error() + end - local ret = {} - local entry = port_tuple.first - for i=1,tonumber(port_tuple.size),1 do - ret[i] = tuple_bless(entry.tuple) - entry = entry.next - end - builtin.port_destroy(port); - return ret + local ret = {} + local entry = port_tuple.first + for i=1,tonumber(port_tuple.size),1 do + ret[i] = tuple_bless(entry.tuple) + entry = entry.next end + builtin.port_destroy(port); + return ret +end + +index_mt.select_luac = function(index, key, opts) + check_index_arg(index, 'select') + local key = keify(key) + local iterator, offset, limit = check_select_opts(opts, #key == 0) + return internal.select(index.space_id, index.id, iterator, + offset, limit, key) +end - index_mt.select_luac = function(index, key, opts) - check_index_arg(index, 'select') - local key = keify(key) - local iterator, offset, limit = check_select_opts(opts, #key == 0) - return internal.select(index.space_id, index.id, iterator, - offset, limit, key) +index_mt.update = function(index, key, ops) + check_index_arg(index, 'update') + return internal.update(index.space_id, index.id, keify(key), ops); +end +index_mt.delete = function(index, key) + check_index_arg(index, 'delete') + return internal.delete(index.space_id, index.id, keify(key)); +end + +index_mt.info = function(index) + return internal.info(index.space_id, index.id); +end + +index_mt.drop = function(index) + check_index_arg(index, 'drop') + return box.schema.index.drop(index.space_id, index.id) +end +index_mt.rename = function(index, name) + check_index_arg(index, 'rename') + return box.schema.index.rename(index.space_id, index.id, name) +end +index_mt.alter = function(index, options) + check_index_arg(index, 'alter') + if index.id == nil or index.space_id == nil then + box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") end + return box.schema.index.alter(index.space_id, index.id, options) +end - index_mt.update = function(index, key, ops) - check_index_arg(index, 'update') - return internal.update(index.space_id, index.id, keify(key), ops); +local space_mt = {} +space_mt.len = function(space) + check_space_arg(space, 'len') + local pk = space.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - index_mt.delete = function(index, key) - check_index_arg(index, 'delete') - return internal.delete(index.space_id, index.id, keify(key)); + return space.index[0]:len() +end +space_mt.count = function(space, key, opts) + check_space_arg(space, 'count') + local pk = space.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - - index_mt.info = function(index) - return internal.info(index.space_id, index.id); + return pk:count(key, opts) +end +space_mt.bsize = function(space) + check_space_arg(space, 'bsize') + local s = builtin.space_by_id(space.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, space.name) end + return builtin.space_bsize(s) +end +space_mt.__newindex = index_mt.__newindex - index_mt.drop = function(index) - check_index_arg(index, 'drop') - return box.schema.index.drop(index.space_id, index.id) +space_mt.get = function(space, key) + check_space_arg(space, 'get') + return check_primary_index(space):get(key) +end +space_mt.select = function(space, key, opts) + check_space_arg(space, 'select') + return check_primary_index(space):select(key, opts) +end +space_mt.insert = function(space, tuple) + check_space_arg(space, 'insert') + return internal.insert(space.id, tuple); +end +space_mt.replace = function(space, tuple) + check_space_arg(space, 'replace') + return internal.replace(space.id, tuple); +end +space_mt.put = space_mt.replace; -- put is an alias for replace +space_mt.update = function(space, key, ops) + check_space_arg(space, 'update') + return check_primary_index(space):update(key, ops) +end +space_mt.upsert = function(space, tuple_key, ops, deprecated) + check_space_arg(space, 'upsert') + if deprecated ~= nil then + local msg = "Error: extra argument in upsert call: " + msg = msg .. tostring(deprecated) + msg = msg .. ". Usage :upsert(tuple, operations)" + box.error(box.error.PROC_LUA, msg) end - index_mt.rename = function(index, name) - check_index_arg(index, 'rename') - return box.schema.index.rename(index.space_id, index.id, name) + return internal.upsert(space.id, tuple_key, ops); +end +space_mt.delete = function(space, key) + check_space_arg(space, 'delete') + return check_primary_index(space):delete(key) +end +-- Assumes that spaceno has a TREE (NUM) primary key +-- inserts a tuple after getting the next value of the +-- primary key and returns it back to the user +space_mt.auto_increment = function(space, tuple) + check_space_arg(space, 'auto_increment') + local max_tuple = check_primary_index(space):max() + local max = 0 + if max_tuple ~= nil then + max = max_tuple[1] end - index_mt.alter = function(index, options) - check_index_arg(index, 'alter') - if index.id == nil or index.space_id == nil then - box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") - end - return box.schema.index.alter(index.space_id, index.id, options) + table.insert(tuple, 1, max + 1) + return space:insert(tuple) +end + +space_mt.pairs = function(space, key, opts) + check_space_arg(space, 'pairs') + local pk = space.index[0] + if pk == nil then + -- empty space without indexes, return empty iterator + return fun.iter({}) + end + return pk:pairs(key, opts) +end +space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility +space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility +space_mt.truncate = function(space) + check_space_arg(space, 'truncate') + return internal.truncate(space.id) +end +space_mt.format = function(space, format) + check_space_arg(space, 'format') + return box.schema.space.format(space.id, format) +end +space_mt.drop = function(space) + check_space_arg(space, 'drop') + check_space_exists(space) + return box.schema.space.drop(space.id, space.name) +end +space_mt.rename = function(space, name) + check_space_arg(space, 'rename') + check_space_exists(space) + return box.schema.space.rename(space.id, name) +end +space_mt.create_index = function(space, name, options) + check_space_arg(space, 'create_index') + check_space_exists(space) + return box.schema.index.create(space.id, name, options) +end +space_mt.run_triggers = function(space, yesno) + check_space_arg(space, 'run_triggers') + local s = builtin.space_by_id(space.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, space.name) end + builtin.space_run_triggers(s, yesno) +end +space_mt.__index = space_mt +function box.schema.space.bless(space) + local index_mt = table.deepcopy(index_mt) + local space_mt = table.deepcopy(space_mt) -- true if reading operations may yield local read_yields = space.engine == 'vinyl' local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} @@ -1302,126 +1425,6 @@ function box.schema.space.bless(space) end index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility - -- - local space_mt = {} - space_mt.len = function(space) - check_space_arg(space, 'len') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return space.index[0]:len() - end - space_mt.count = function(space, key, opts) - check_space_arg(space, 'count') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return pk:count(key, opts) - end - space_mt.bsize = function(space) - check_space_arg(space, 'bsize') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - return builtin.space_bsize(s) - end - space_mt.__newindex = index_mt.__newindex - - space_mt.get = function(space, key) - check_space_arg(space, 'get') - return check_primary_index(space):get(key) - end - space_mt.select = function(space, key, opts) - check_space_arg(space, 'select') - return check_primary_index(space):select(key, opts) - end - space_mt.insert = function(space, tuple) - check_space_arg(space, 'insert') - return internal.insert(space.id, tuple); - end - space_mt.replace = function(space, tuple) - check_space_arg(space, 'replace') - return internal.replace(space.id, tuple); - end - space_mt.put = space_mt.replace; -- put is an alias for replace - space_mt.update = function(space, key, ops) - check_space_arg(space, 'update') - return check_primary_index(space):update(key, ops) - end - space_mt.upsert = function(space, tuple_key, ops, deprecated) - check_space_arg(space, 'upsert') - if deprecated ~= nil then - local msg = "Error: extra argument in upsert call: " - msg = msg .. tostring(deprecated) - msg = msg .. ". Usage :upsert(tuple, operations)" - box.error(box.error.PROC_LUA, msg) - end - return internal.upsert(space.id, tuple_key, ops); - end - space_mt.delete = function(space, key) - check_space_arg(space, 'delete') - return check_primary_index(space):delete(key) - end --- Assumes that spaceno has a TREE (NUM) primary key --- inserts a tuple after getting the next value of the --- primary key and returns it back to the user - space_mt.auto_increment = function(space, tuple) - check_space_arg(space, 'auto_increment') - local max_tuple = check_primary_index(space):max() - local max = 0 - if max_tuple ~= nil then - max = max_tuple[1] - end - table.insert(tuple, 1, max + 1) - return space:insert(tuple) - end - - space_mt.pairs = function(space, key, opts) - check_space_arg(space, 'pairs') - local pk = space.index[0] - if pk == nil then - -- empty space without indexes, return empty iterator - return fun.iter({}) - end - return pk:pairs(key, opts) - end - space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.truncate = function(space) - check_space_arg(space, 'truncate') - return internal.truncate(space.id) - end - space_mt.format = function(space, format) - check_space_arg(space, 'format') - return box.schema.space.format(space.id, format) - end - space_mt.drop = function(space) - check_space_arg(space, 'drop') - check_space_exists(space) - return box.schema.space.drop(space.id, space.name) - end - space_mt.rename = function(space, name) - check_space_arg(space, 'rename') - check_space_exists(space) - return box.schema.space.rename(space.id, name) - end - space_mt.create_index = function(space, name, options) - check_space_arg(space, 'create_index') - check_space_exists(space) - return box.schema.index.create(space.id, name, options) - end - space_mt.run_triggers = function(space, yesno) - check_space_arg(space, 'run_triggers') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - builtin.space_run_triggers(s, yesno) - end - space_mt.__index = space_mt setmetatable(space, space_mt) if type(space.index) == 'table' and space.enabled then -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH 2/2] schema: expose space_mt and index_mt on `box.schema` table 2018-04-01 11:02 ` [PATCH 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-04-01 11:02 ` [PATCH 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy @ 2018-04-01 11:02 ` Vladislav Shpilevoy 2018-04-02 11:28 ` Vladimir Davydov 1 sibling, 1 reply; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-04-01 11:02 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy This commit allows userland to extend the space and index metatables with their own functions or even metamethods. Reducing barriers for this kind of experimentation is vital for user contribution toward the improvement of Tarantool's API. Closes #3204 --- src/box/lua/schema.lua | 21 ++++++++--- test/box-tap/schema_mt.test.lua | 79 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 5 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 8b6f69ad8..6e325a5c5 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1075,6 +1075,7 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box local index_mt = {} +box.schema.index_mt = index_mt -- __len and __index index_mt.len = function(index) check_index_arg(index, 'len') @@ -1289,6 +1290,7 @@ index_mt.alter = function(index, options) end local space_mt = {} +box.schema.space_mt = space_mt space_mt.len = function(space) check_space_arg(space, 'len') local pk = space.index[0] @@ -1409,18 +1411,27 @@ end space_mt.__index = space_mt function box.schema.space.bless(space) - local index_mt = table.deepcopy(index_mt) - local space_mt = table.deepcopy(space_mt) + -- At first, reference all global index functions. At second, + -- choose an implementation for read operations. They are not + -- in a global index_mt, because they are engine specific. + -- All common index_mt functions must be referenced here to + -- be able to get them using getmetatable(index_object) - if + -- they are only in index_mt, then getmetatable() returns only + -- read ops. + local local_index_mt = table.deepcopy(index_mt) + local_index_mt.__index = function(index, key) + return local_index_mt[key] or index_mt[key] + end -- true if reading operations may yield local read_yields = space.engine == 'vinyl' local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} for _, op in ipairs(read_ops) do if read_yields then -- use Lua/C implmenetation - index_mt[op] = index_mt[op .. "_luac"] + local_index_mt[op] = index_mt[op .. "_luac"] else -- use FFI implementation - index_mt[op] = index_mt[op .. "_ffi"] + local_index_mt[op] = index_mt[op .. "_ffi"] end end index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility @@ -1430,7 +1441,7 @@ function box.schema.space.bless(space) if type(space.index) == 'table' and space.enabled then for j, index in pairs(space.index) do if type(j) == 'number' then - setmetatable(index, index_mt) + setmetatable(index, local_index_mt) end end end diff --git a/test/box-tap/schema_mt.test.lua b/test/box-tap/schema_mt.test.lua new file mode 100755 index 000000000..096c4ad60 --- /dev/null +++ b/test/box-tap/schema_mt.test.lua @@ -0,0 +1,79 @@ +#!/usr/bin/env tarantool +-- +-- pr-3204: expose space_mt, index_mt into box.schema. +-- + +local tap = require('tap') +local test = tap.test('schema_mt') + +test:plan(7) + +box.cfg{ + log="tarantool.log", +} + +local sp1 = box.schema.space.create('test') +local sp2 = box.schema.space.create('test2') + +test:is( + getmetatable(sp1), + getmetatable(sp2), + 'all spaces use the same metatable' +) + +local idx1 = sp1:create_index('primary') +local idx2 = sp2:create_index('primary') + +test:isnt( + getmetatable(idx1), + getmetatable(idx2), + 'all indexes have their own metatable' +) + +test:is( + idx1.get, + idx2.get, + 'memtx indexes share read methods' +) + +local sp3 = box.schema.space.create('test3', {engine='vinyl'}) +local sp4 = box.schema.space.create('test4', {engine='vinyl'}) + +local idx3 = sp3:create_index('primary') +local idx4 = sp4:create_index('primary') + +test:is( + idx3.get, + idx4.get, + 'vinyl indexes share read methods' +) + +test:isnt( + idx1.get, + idx3.get, + 'memtx and vinyl indexes have separate read methods' +) + +function box.schema.space_mt.foo() end + +test:is( + sp1.foo, + box.schema.space_mt.foo, + 'box.schema.space_mt is mutable' +) + +function box.schema.index_mt.foo() end + +test:is( + idx1.foo, + box.schema.index_mt.foo, + 'box.schema.index_mt is mutable' +) + +test:check() + +sp1:drop() +sp2:drop() +sp3:drop() +sp4:drop() +os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH 2/2] schema: expose space_mt and index_mt on `box.schema` table 2018-04-01 11:02 ` [PATCH 2/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy @ 2018-04-02 11:28 ` Vladimir Davydov 2018-04-03 16:50 ` [PATCH v2 0/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 0 siblings, 1 reply; 20+ messages in thread From: Vladimir Davydov @ 2018-04-02 11:28 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches On Sun, Apr 01, 2018 at 02:02:46PM +0300, Vladislav Shpilevoy wrote: > function box.schema.space.bless(space) > - local index_mt = table.deepcopy(index_mt) > - local space_mt = table.deepcopy(space_mt) > + -- At first, reference all global index functions. At second, > + -- choose an implementation for read operations. They are not > + -- in a global index_mt, because they are engine specific. > + -- All common index_mt functions must be referenced here to > + -- be able to get them using getmetatable(index_object) - if > + -- they are only in index_mt, then getmetatable() returns only > + -- read ops. > + local local_index_mt = table.deepcopy(index_mt) > + local_index_mt.__index = function(index, key) > + return local_index_mt[key] or index_mt[key] > + end I don't really like that you use the same metatable for all spaces, but a separate metatable for each index. Besides, because of this one can't overwrite a built-in method (e.g. 'get') as it will be masked by local_index_mt. AFAIU you do this, because vinyl and memtx use different methods (luac vs ffi) for certain operations. I see the following alternatives: 1. Leave as is. This isn't all so bad. After all, I doubt anybody's actually willing to overwrite built-in methods. 2. Use two metatables, one for all memtx indexes, another for vinyl, and push all changes from the public index_mt to both of these metatables. Not quite sure it's technically possible in Lua. 3. Same as #2, but proscribe modification of built-in methods. 4. Add a separate engine-specific metatable for the method that differ between memtx and vinyl, then define __index as: local engine_index_mt = (engine == 'memtx' and memtx_index_mt or vinyl_index_mt) index_mt.__index = function(index, key) return index_mt[key] or engine_index_mt[key] Personally, I like #4 most, but it may affect performance. Anyway, I guess this is up to Kostja to decide which one to choose. I don't see any other technical problems in the patch, nor in the test. > -- true if reading operations may yield > local read_yields = space.engine == 'vinyl' > local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} > for _, op in ipairs(read_ops) do > if read_yields then > -- use Lua/C implmenetation > - index_mt[op] = index_mt[op .. "_luac"] > + local_index_mt[op] = index_mt[op .. "_luac"] > else > -- use FFI implementation > - index_mt[op] = index_mt[op .. "_ffi"] > + local_index_mt[op] = index_mt[op .. "_ffi"] > end > end > index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility > @@ -1430,7 +1441,7 @@ function box.schema.space.bless(space) > if type(space.index) == 'table' and space.enabled then > for j, index in pairs(space.index) do > if type(j) == 'number' then > - setmetatable(index, index_mt) > + setmetatable(index, local_index_mt) > end > end > end ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 0/3] schema: expose space_mt and index_mt on box.schema table 2018-04-02 11:28 ` Vladimir Davydov @ 2018-04-03 16:50 ` Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 1/3] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy ` (2 more replies) 0 siblings, 3 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-04-03 16:50 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Branch: http://github.com/tarantool/tarantool/tree/pr-3204-expose-space-index-mt Issue: https://github.com/tarantool/tarantool/issues/3204 Space_mt and index_mt are created individually for each space inside a giant space.bless function. It consumes lua memory by duplicating these metatables (which is very limited), and does not allow to add new functions or redefine existing ones, which would be visible in all spaces and indexes. Lets solve the problems one by one. At first, move space_mt and index_mt definitions out of space.bless function. Now space.bless just copy space_mt and inherit a needed index_mt depending on egine. At second, instead of customization of index_mt copy on each space.bless lets once create separate index metatables for memtx and for vinyl, and just copy needed in space.bless. With no selecting yielding/non-yielding methods inside the function. At third, just stop copying in bless, start reference metatables instead, and open them to users via box.schema: box.schema.space_mt - metatable of all spaces; box.schema.index_mt - base metatable of all indexes - replicated into the vinyl and memtx. See below how. box.schema.vinyl_index_mt - metatable of all vinyl indexes; box.schema.memtx_index_mt - metatable of all memtx indexes. Once metatables become global and shared, they can be easy extended. However there is a non-trival task how to make index metatable extension be transparent for a user, if a user modifies not vinyl or memtx metatable, but box.schema.index_mt. It would be good to replicate all changes of a base metatable over engine specific ones. This is solved using "metatable for a metatable". When a user modifies box.schema.index_mt, these changes via __newindex method of box.schema.index_mt are replicated both into memtx and vinyl index metatables. Vladislav Shpilevoy (3): schema: move space_mt and index_mt definition out of space bless schema: inherit vinyl/memtx_index_mt from base index mt schema: expose space_mt and index_mt on box.schema table src/box/lua/schema.lua | 663 +++++++++++++++++++++------------------- test/box-tap/schema_mt.test.lua | 80 +++++ 2 files changed, 424 insertions(+), 319 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] schema: move space_mt and index_mt definition out of space bless 2018-04-03 16:50 ` [PATCH v2 0/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy @ 2018-04-03 16:50 ` Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 2/3] schema: inherit vinyl/memtx_index_mt from base index mt Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2 siblings, 0 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-04-03 16:50 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Space_mt and index_mt are created individually for each space inside a giant space.bless function. It makes impossible to implement #3204: exposing space and index metatables to a user, because for this they must be global and shared between all spaces and indexes. Lets move their definition out of space.bless function, and do their duplicate inside. Needed #3204 --- src/box/lua/schema.lua | 619 +++++++++++++++++++++++++------------------------ 1 file changed, 311 insertions(+), 308 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 5a0f71559..8b6f69ad8 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1074,220 +1074,343 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box -function box.schema.space.bless(space) - local index_mt = {} - -- __len and __index - index_mt.len = function(index) - check_index_arg(index, 'len') - local ret = builtin.box_index_len(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - -- index.bsize - index_mt.bsize = function(index) - check_index_arg(index, 'bsize') - local ret = builtin.box_index_bsize(index.space_id, index.id) - if ret == -1 then - box.error() - end - return tonumber(ret) - end - index_mt.__len = index_mt.len -- Lua 5.2 compatibility - index_mt.__newindex = function(table, index) - return error('Attempt to modify a read-only table') end - index_mt.__index = index_mt - -- min and max - index_mt.min_ffi = function(index, key) - check_index_arg(index, 'min') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_min(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end +local index_mt = {} +-- __len and __index +index_mt.len = function(index) + check_index_arg(index, 'len') + local ret = builtin.box_index_len(index.space_id, index.id) + if ret == -1 then + box.error() end - index_mt.min_luac = function(index, key) - check_index_arg(index, 'min') - key = keify(key) - return internal.min(index.space_id, index.id, key); + return tonumber(ret) +end +-- index.bsize +index_mt.bsize = function(index) + check_index_arg(index, 'bsize') + local ret = builtin.box_index_bsize(index.space_id, index.id) + if ret == -1 then + box.error() end - index_mt.max_ffi = function(index, key) - check_index_arg(index, 'max') - local pkey, pkey_end = tuple_encode(key) - if builtin.box_index_max(index.space_id, index.id, - pkey, pkey_end, ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end + return tonumber(ret) +end +index_mt.__len = index_mt.len -- Lua 5.2 compatibility +index_mt.__newindex = function(table, index) + return error('Attempt to modify a read-only table') +end +index_mt.__index = index_mt +-- min and max +index_mt.min_ffi = function(index, key) + check_index_arg(index, 'min') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_min(index.space_id, index.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return end - index_mt.max_luac = function(index, key) - check_index_arg(index, 'max') - key = keify(key) - return internal.max(index.space_id, index.id, key); +end +index_mt.min_luac = function(index, key) + check_index_arg(index, 'min') + key = keify(key) + return internal.min(index.space_id, index.id, key); +end +index_mt.max_ffi = function(index, key) + check_index_arg(index, 'max') + local pkey, pkey_end = tuple_encode(key) + if builtin.box_index_max(index.space_id, index.id, + pkey, pkey_end, ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return end - index_mt.random_ffi = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - if builtin.box_index_random(index.space_id, index.id, rnd, - ptuple) ~= 0 then - box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end +end +index_mt.max_luac = function(index, key) + check_index_arg(index, 'max') + key = keify(key) + return internal.max(index.space_id, index.id, key); +end +index_mt.random_ffi = function(index, rnd) + check_index_arg(index, 'random') + rnd = rnd or math.random() + if builtin.box_index_random(index.space_id, index.id, rnd, + ptuple) ~= 0 then + box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return end - index_mt.random_luac = function(index, rnd) - check_index_arg(index, 'random') - rnd = rnd or math.random() - return internal.random(index.space_id, index.id, rnd); - end - -- iteration - index_mt.pairs_ffi = function(index, key, opts) - check_index_arg(index, 'pairs') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - - local keybuf = ffi.string(pkey, pkey_end - pkey) - local pkeybuf = ffi.cast('const char *', keybuf) - local cdata = builtin.box_index_iterator(index.space_id, index.id, - itype, pkeybuf, pkeybuf + #keybuf); - if cdata == nil then - box.error() - end - return fun.wrap(iterator_gen, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - index_mt.pairs_luac = function(index, key, opts) - check_index_arg(index, 'pairs') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - local keymp = msgpack.encode(key) - local keybuf = ffi.string(keymp, #keymp) - local cdata = internal.iterator(index.space_id, index.id, itype, keymp); - return fun.wrap(iterator_gen_luac, keybuf, - ffi.gc(cdata, builtin.box_iterator_free)) - end - - -- index subtree size - index_mt.count_ffi = function(index, key, opts) - check_index_arg(index, 'count') - local pkey, pkey_end = tuple_encode(key) - local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); - local count = builtin.box_index_count(index.space_id, index.id, - itype, pkey, pkey_end); - if count == -1 then - box.error() - end - return tonumber(count) - end - index_mt.count_luac = function(index, key, opts) - check_index_arg(index, 'count') - key = keify(key) - local itype = check_iterator_type(opts, #key == 0); - return internal.count(index.space_id, index.id, itype, key); - end - - index_mt.get_ffi = function(index, key) - check_index_arg(index, 'get') - local key, key_end = tuple_encode(key) - if builtin.box_index_get(index.space_id, index.id, - key, key_end, ptuple) ~= 0 then - return box.error() -- error - elseif ptuple[0] ~= nil then - return tuple_bless(ptuple[0]) - else - return - end +end +index_mt.random_luac = function(index, rnd) + check_index_arg(index, 'random') + rnd = rnd or math.random() + return internal.random(index.space_id, index.id, rnd); +end +-- iteration +index_mt.pairs_ffi = function(index, key, opts) + check_index_arg(index, 'pairs') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); + + local keybuf = ffi.string(pkey, pkey_end - pkey) + local pkeybuf = ffi.cast('const char *', keybuf) + local cdata = builtin.box_index_iterator(index.space_id, index.id, + itype, pkeybuf, pkeybuf + #keybuf); + if cdata == nil then + box.error() end - index_mt.get_luac = function(index, key) - check_index_arg(index, 'get') - key = keify(key) - return internal.get(index.space_id, index.id, key) + return fun.wrap(iterator_gen, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end +index_mt.pairs_luac = function(index, key, opts) + check_index_arg(index, 'pairs') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0); + local keymp = msgpack.encode(key) + local keybuf = ffi.string(keymp, #keymp) + local cdata = internal.iterator(index.space_id, index.id, itype, keymp); + return fun.wrap(iterator_gen_luac, keybuf, + ffi.gc(cdata, builtin.box_iterator_free)) +end + +-- index subtree size +index_mt.count_ffi = function(index, key, opts) + check_index_arg(index, 'count') + local pkey, pkey_end = tuple_encode(key) + local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); + local count = builtin.box_index_count(index.space_id, index.id, + itype, pkey, pkey_end); + if count == -1 then + box.error() end + return tonumber(count) +end +index_mt.count_luac = function(index, key, opts) + check_index_arg(index, 'count') + key = keify(key) + local itype = check_iterator_type(opts, #key == 0); + return internal.count(index.space_id, index.id, itype, key); +end - local function check_select_opts(opts, key_is_nil) - local offset = 0 - local limit = 4294967295 - local iterator = check_iterator_type(opts, key_is_nil) - if opts ~= nil then - if opts.offset ~= nil then - offset = opts.offset - end - if opts.limit ~= nil then - limit = opts.limit - end +index_mt.get_ffi = function(index, key) + check_index_arg(index, 'get') + local key, key_end = tuple_encode(key) + if builtin.box_index_get(index.space_id, index.id, + key, key_end, ptuple) ~= 0 then + return box.error() -- error + elseif ptuple[0] ~= nil then + return tuple_bless(ptuple[0]) + else + return + end +end +index_mt.get_luac = function(index, key) + check_index_arg(index, 'get') + key = keify(key) + return internal.get(index.space_id, index.id, key) +end + +local function check_select_opts(opts, key_is_nil) + local offset = 0 + local limit = 4294967295 + local iterator = check_iterator_type(opts, key_is_nil) + if opts ~= nil then + if opts.offset ~= nil then + offset = opts.offset + end + if opts.limit ~= nil then + limit = opts.limit end - return iterator, offset, limit end + return iterator, offset, limit +end - index_mt.select_ffi = function(index, key, opts) - check_index_arg(index, 'select') - local key, key_end = tuple_encode(key) - local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) +index_mt.select_ffi = function(index, key, opts) + check_index_arg(index, 'select') + local key, key_end = tuple_encode(key) + local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) - local port = ffi.cast('struct port *', port_tuple) + local port = ffi.cast('struct port *', port_tuple) - if builtin.box_select(index.space_id, index.id, - iterator, offset, limit, key, key_end, port) ~= 0 then - return box.error() - end + if builtin.box_select(index.space_id, index.id, + iterator, offset, limit, key, key_end, port) ~= 0 then + return box.error() + end - local ret = {} - local entry = port_tuple.first - for i=1,tonumber(port_tuple.size),1 do - ret[i] = tuple_bless(entry.tuple) - entry = entry.next - end - builtin.port_destroy(port); - return ret + local ret = {} + local entry = port_tuple.first + for i=1,tonumber(port_tuple.size),1 do + ret[i] = tuple_bless(entry.tuple) + entry = entry.next end + builtin.port_destroy(port); + return ret +end + +index_mt.select_luac = function(index, key, opts) + check_index_arg(index, 'select') + local key = keify(key) + local iterator, offset, limit = check_select_opts(opts, #key == 0) + return internal.select(index.space_id, index.id, iterator, + offset, limit, key) +end - index_mt.select_luac = function(index, key, opts) - check_index_arg(index, 'select') - local key = keify(key) - local iterator, offset, limit = check_select_opts(opts, #key == 0) - return internal.select(index.space_id, index.id, iterator, - offset, limit, key) +index_mt.update = function(index, key, ops) + check_index_arg(index, 'update') + return internal.update(index.space_id, index.id, keify(key), ops); +end +index_mt.delete = function(index, key) + check_index_arg(index, 'delete') + return internal.delete(index.space_id, index.id, keify(key)); +end + +index_mt.info = function(index) + return internal.info(index.space_id, index.id); +end + +index_mt.drop = function(index) + check_index_arg(index, 'drop') + return box.schema.index.drop(index.space_id, index.id) +end +index_mt.rename = function(index, name) + check_index_arg(index, 'rename') + return box.schema.index.rename(index.space_id, index.id, name) +end +index_mt.alter = function(index, options) + check_index_arg(index, 'alter') + if index.id == nil or index.space_id == nil then + box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") end + return box.schema.index.alter(index.space_id, index.id, options) +end - index_mt.update = function(index, key, ops) - check_index_arg(index, 'update') - return internal.update(index.space_id, index.id, keify(key), ops); +local space_mt = {} +space_mt.len = function(space) + check_space_arg(space, 'len') + local pk = space.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - index_mt.delete = function(index, key) - check_index_arg(index, 'delete') - return internal.delete(index.space_id, index.id, keify(key)); + return space.index[0]:len() +end +space_mt.count = function(space, key, opts) + check_space_arg(space, 'count') + local pk = space.index[0] + if pk == nil then + return 0 -- empty space without indexes, return 0 end - - index_mt.info = function(index) - return internal.info(index.space_id, index.id); + return pk:count(key, opts) +end +space_mt.bsize = function(space) + check_space_arg(space, 'bsize') + local s = builtin.space_by_id(space.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, space.name) end + return builtin.space_bsize(s) +end +space_mt.__newindex = index_mt.__newindex - index_mt.drop = function(index) - check_index_arg(index, 'drop') - return box.schema.index.drop(index.space_id, index.id) +space_mt.get = function(space, key) + check_space_arg(space, 'get') + return check_primary_index(space):get(key) +end +space_mt.select = function(space, key, opts) + check_space_arg(space, 'select') + return check_primary_index(space):select(key, opts) +end +space_mt.insert = function(space, tuple) + check_space_arg(space, 'insert') + return internal.insert(space.id, tuple); +end +space_mt.replace = function(space, tuple) + check_space_arg(space, 'replace') + return internal.replace(space.id, tuple); +end +space_mt.put = space_mt.replace; -- put is an alias for replace +space_mt.update = function(space, key, ops) + check_space_arg(space, 'update') + return check_primary_index(space):update(key, ops) +end +space_mt.upsert = function(space, tuple_key, ops, deprecated) + check_space_arg(space, 'upsert') + if deprecated ~= nil then + local msg = "Error: extra argument in upsert call: " + msg = msg .. tostring(deprecated) + msg = msg .. ". Usage :upsert(tuple, operations)" + box.error(box.error.PROC_LUA, msg) end - index_mt.rename = function(index, name) - check_index_arg(index, 'rename') - return box.schema.index.rename(index.space_id, index.id, name) + return internal.upsert(space.id, tuple_key, ops); +end +space_mt.delete = function(space, key) + check_space_arg(space, 'delete') + return check_primary_index(space):delete(key) +end +-- Assumes that spaceno has a TREE (NUM) primary key +-- inserts a tuple after getting the next value of the +-- primary key and returns it back to the user +space_mt.auto_increment = function(space, tuple) + check_space_arg(space, 'auto_increment') + local max_tuple = check_primary_index(space):max() + local max = 0 + if max_tuple ~= nil then + max = max_tuple[1] end - index_mt.alter = function(index, options) - check_index_arg(index, 'alter') - if index.id == nil or index.space_id == nil then - box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") - end - return box.schema.index.alter(index.space_id, index.id, options) + table.insert(tuple, 1, max + 1) + return space:insert(tuple) +end + +space_mt.pairs = function(space, key, opts) + check_space_arg(space, 'pairs') + local pk = space.index[0] + if pk == nil then + -- empty space without indexes, return empty iterator + return fun.iter({}) + end + return pk:pairs(key, opts) +end +space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility +space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility +space_mt.truncate = function(space) + check_space_arg(space, 'truncate') + return internal.truncate(space.id) +end +space_mt.format = function(space, format) + check_space_arg(space, 'format') + return box.schema.space.format(space.id, format) +end +space_mt.drop = function(space) + check_space_arg(space, 'drop') + check_space_exists(space) + return box.schema.space.drop(space.id, space.name) +end +space_mt.rename = function(space, name) + check_space_arg(space, 'rename') + check_space_exists(space) + return box.schema.space.rename(space.id, name) +end +space_mt.create_index = function(space, name, options) + check_space_arg(space, 'create_index') + check_space_exists(space) + return box.schema.index.create(space.id, name, options) +end +space_mt.run_triggers = function(space, yesno) + check_space_arg(space, 'run_triggers') + local s = builtin.space_by_id(space.id) + if s == nil then + box.error(box.error.NO_SUCH_SPACE, space.name) end + builtin.space_run_triggers(s, yesno) +end +space_mt.__index = space_mt +function box.schema.space.bless(space) + local index_mt = table.deepcopy(index_mt) + local space_mt = table.deepcopy(space_mt) -- true if reading operations may yield local read_yields = space.engine == 'vinyl' local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} @@ -1302,126 +1425,6 @@ function box.schema.space.bless(space) end index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility - -- - local space_mt = {} - space_mt.len = function(space) - check_space_arg(space, 'len') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return space.index[0]:len() - end - space_mt.count = function(space, key, opts) - check_space_arg(space, 'count') - local pk = space.index[0] - if pk == nil then - return 0 -- empty space without indexes, return 0 - end - return pk:count(key, opts) - end - space_mt.bsize = function(space) - check_space_arg(space, 'bsize') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - return builtin.space_bsize(s) - end - space_mt.__newindex = index_mt.__newindex - - space_mt.get = function(space, key) - check_space_arg(space, 'get') - return check_primary_index(space):get(key) - end - space_mt.select = function(space, key, opts) - check_space_arg(space, 'select') - return check_primary_index(space):select(key, opts) - end - space_mt.insert = function(space, tuple) - check_space_arg(space, 'insert') - return internal.insert(space.id, tuple); - end - space_mt.replace = function(space, tuple) - check_space_arg(space, 'replace') - return internal.replace(space.id, tuple); - end - space_mt.put = space_mt.replace; -- put is an alias for replace - space_mt.update = function(space, key, ops) - check_space_arg(space, 'update') - return check_primary_index(space):update(key, ops) - end - space_mt.upsert = function(space, tuple_key, ops, deprecated) - check_space_arg(space, 'upsert') - if deprecated ~= nil then - local msg = "Error: extra argument in upsert call: " - msg = msg .. tostring(deprecated) - msg = msg .. ". Usage :upsert(tuple, operations)" - box.error(box.error.PROC_LUA, msg) - end - return internal.upsert(space.id, tuple_key, ops); - end - space_mt.delete = function(space, key) - check_space_arg(space, 'delete') - return check_primary_index(space):delete(key) - end --- Assumes that spaceno has a TREE (NUM) primary key --- inserts a tuple after getting the next value of the --- primary key and returns it back to the user - space_mt.auto_increment = function(space, tuple) - check_space_arg(space, 'auto_increment') - local max_tuple = check_primary_index(space):max() - local max = 0 - if max_tuple ~= nil then - max = max_tuple[1] - end - table.insert(tuple, 1, max + 1) - return space:insert(tuple) - end - - space_mt.pairs = function(space, key, opts) - check_space_arg(space, 'pairs') - local pk = space.index[0] - if pk == nil then - -- empty space without indexes, return empty iterator - return fun.iter({}) - end - return pk:pairs(key, opts) - end - space_mt.__pairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.__ipairs = space_mt.pairs -- Lua 5.2 compatibility - space_mt.truncate = function(space) - check_space_arg(space, 'truncate') - return internal.truncate(space.id) - end - space_mt.format = function(space, format) - check_space_arg(space, 'format') - return box.schema.space.format(space.id, format) - end - space_mt.drop = function(space) - check_space_arg(space, 'drop') - check_space_exists(space) - return box.schema.space.drop(space.id, space.name) - end - space_mt.rename = function(space, name) - check_space_arg(space, 'rename') - check_space_exists(space) - return box.schema.space.rename(space.id, name) - end - space_mt.create_index = function(space, name, options) - check_space_arg(space, 'create_index') - check_space_exists(space) - return box.schema.index.create(space.id, name, options) - end - space_mt.run_triggers = function(space, yesno) - check_space_arg(space, 'run_triggers') - local s = builtin.space_by_id(space.id) - if s == nil then - box.error(box.error.NO_SUCH_SPACE, space.name) - end - builtin.space_run_triggers(s, yesno) - end - space_mt.__index = space_mt setmetatable(space, space_mt) if type(space.index) == 'table' and space.enabled then -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] schema: inherit vinyl/memtx_index_mt from base index mt 2018-04-03 16:50 ` [PATCH v2 0/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 1/3] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy @ 2018-04-03 16:50 ` Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2 siblings, 0 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-04-03 16:50 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy Now space.bless() in Lua serves to choose a correct index metatable that depends on a space engine. Vinyl index methods must not use FFI since yield breaks it. Lets do not choose correct methods one by one in space.bless, create them only once on start, and then just do copy of needed table. --- src/box/lua/schema.lua | 109 ++++++++++++++++++++++++++++++------------------- 1 file changed, 66 insertions(+), 43 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 8b6f69ad8..4b5eeeda9 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1074,9 +1074,32 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box -local index_mt = {} +local function forbid_new_index(t, k, v) + return error('Attempt to modify a read-only table') +end + +local base_index_mt = {} +base_index_mt.__index = base_index_mt +-- +-- Inherit engine specific index metatables from a base one. +-- +local vinyl_index_mt = {} +vinyl_index_mt.__index = vinyl_index_mt +local memtx_index_mt = {} +memtx_index_mt.__index = memtx_index_mt +-- +-- When a new method is added below to base index mt, the same +-- method is added both to vinyl and memtx index mt. +-- +setmetatable(base_index_mt, { + __newindex = function(t, k, v) + vinyl_index_mt[k] = v + memtx_index_mt[k] = v + rawset(t, k, v) + end +}) -- __len and __index -index_mt.len = function(index) +base_index_mt.len = function(index) check_index_arg(index, 'len') local ret = builtin.box_index_len(index.space_id, index.id) if ret == -1 then @@ -1085,7 +1108,7 @@ index_mt.len = function(index) return tonumber(ret) end -- index.bsize -index_mt.bsize = function(index) +base_index_mt.bsize = function(index) check_index_arg(index, 'bsize') local ret = builtin.box_index_bsize(index.space_id, index.id) if ret == -1 then @@ -1093,13 +1116,11 @@ index_mt.bsize = function(index) end return tonumber(ret) end -index_mt.__len = index_mt.len -- Lua 5.2 compatibility -index_mt.__newindex = function(table, index) - return error('Attempt to modify a read-only table') -end -index_mt.__index = index_mt +-- Lua 5.2 compatibility +base_index_mt.__len = base_index_mt.len +base_index_mt.__newindex = forbid_new_index -- min and max -index_mt.min_ffi = function(index, key) +base_index_mt.min_ffi = function(index, key) check_index_arg(index, 'min') local pkey, pkey_end = tuple_encode(key) if builtin.box_index_min(index.space_id, index.id, @@ -1111,12 +1132,12 @@ index_mt.min_ffi = function(index, key) return end end -index_mt.min_luac = function(index, key) +base_index_mt.min_luac = function(index, key) check_index_arg(index, 'min') key = keify(key) return internal.min(index.space_id, index.id, key); end -index_mt.max_ffi = function(index, key) +base_index_mt.max_ffi = function(index, key) check_index_arg(index, 'max') local pkey, pkey_end = tuple_encode(key) if builtin.box_index_max(index.space_id, index.id, @@ -1128,12 +1149,12 @@ index_mt.max_ffi = function(index, key) return end end -index_mt.max_luac = function(index, key) +base_index_mt.max_luac = function(index, key) check_index_arg(index, 'max') key = keify(key) return internal.max(index.space_id, index.id, key); end -index_mt.random_ffi = function(index, rnd) +base_index_mt.random_ffi = function(index, rnd) check_index_arg(index, 'random') rnd = rnd or math.random() if builtin.box_index_random(index.space_id, index.id, rnd, @@ -1145,13 +1166,13 @@ index_mt.random_ffi = function(index, rnd) return end end -index_mt.random_luac = function(index, rnd) +base_index_mt.random_luac = function(index, rnd) check_index_arg(index, 'random') rnd = rnd or math.random() return internal.random(index.space_id, index.id, rnd); end -- iteration -index_mt.pairs_ffi = function(index, key, opts) +base_index_mt.pairs_ffi = function(index, key, opts) check_index_arg(index, 'pairs') local pkey, pkey_end = tuple_encode(key) local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); @@ -1166,7 +1187,7 @@ index_mt.pairs_ffi = function(index, key, opts) return fun.wrap(iterator_gen, keybuf, ffi.gc(cdata, builtin.box_iterator_free)) end -index_mt.pairs_luac = function(index, key, opts) +base_index_mt.pairs_luac = function(index, key, opts) check_index_arg(index, 'pairs') key = keify(key) local itype = check_iterator_type(opts, #key == 0); @@ -1178,7 +1199,7 @@ index_mt.pairs_luac = function(index, key, opts) end -- index subtree size -index_mt.count_ffi = function(index, key, opts) +base_index_mt.count_ffi = function(index, key, opts) check_index_arg(index, 'count') local pkey, pkey_end = tuple_encode(key) local itype = check_iterator_type(opts, pkey + 1 >= pkey_end); @@ -1189,14 +1210,14 @@ index_mt.count_ffi = function(index, key, opts) end return tonumber(count) end -index_mt.count_luac = function(index, key, opts) +base_index_mt.count_luac = function(index, key, opts) check_index_arg(index, 'count') key = keify(key) local itype = check_iterator_type(opts, #key == 0); return internal.count(index.space_id, index.id, itype, key); end -index_mt.get_ffi = function(index, key) +base_index_mt.get_ffi = function(index, key) check_index_arg(index, 'get') local key, key_end = tuple_encode(key) if builtin.box_index_get(index.space_id, index.id, @@ -1208,7 +1229,7 @@ index_mt.get_ffi = function(index, key) return end end -index_mt.get_luac = function(index, key) +base_index_mt.get_luac = function(index, key) check_index_arg(index, 'get') key = keify(key) return internal.get(index.space_id, index.id, key) @@ -1229,7 +1250,7 @@ local function check_select_opts(opts, key_is_nil) return iterator, offset, limit end -index_mt.select_ffi = function(index, key, opts) +base_index_mt.select_ffi = function(index, key, opts) check_index_arg(index, 'select') local key, key_end = tuple_encode(key) local iterator, offset, limit = check_select_opts(opts, key + 1 >= key_end) @@ -1251,7 +1272,7 @@ index_mt.select_ffi = function(index, key, opts) return ret end -index_mt.select_luac = function(index, key, opts) +base_index_mt.select_luac = function(index, key, opts) check_index_arg(index, 'select') local key = keify(key) local iterator, offset, limit = check_select_opts(opts, #key == 0) @@ -1259,28 +1280,28 @@ index_mt.select_luac = function(index, key, opts) offset, limit, key) end -index_mt.update = function(index, key, ops) +base_index_mt.update = function(index, key, ops) check_index_arg(index, 'update') return internal.update(index.space_id, index.id, keify(key), ops); end -index_mt.delete = function(index, key) +base_index_mt.delete = function(index, key) check_index_arg(index, 'delete') return internal.delete(index.space_id, index.id, keify(key)); end -index_mt.info = function(index) +base_index_mt.info = function(index) return internal.info(index.space_id, index.id); end -index_mt.drop = function(index) +base_index_mt.drop = function(index) check_index_arg(index, 'drop') return box.schema.index.drop(index.space_id, index.id) end -index_mt.rename = function(index, name) +base_index_mt.rename = function(index, name) check_index_arg(index, 'rename') return box.schema.index.rename(index.space_id, index.id, name) end -index_mt.alter = function(index, options) +base_index_mt.alter = function(index, options) check_index_arg(index, 'alter') if index.id == nil or index.space_id == nil then box.error(box.error.PROC_LUA, "Usage: index:alter{opts}") @@ -1288,6 +1309,17 @@ index_mt.alter = function(index, options) return box.schema.index.alter(index.space_id, index.id, options) end +local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} +for _, op in ipairs(read_ops) do + vinyl_index_mt[op] = base_index_mt[op..'_luac'] + memtx_index_mt[op] = base_index_mt[op..'_ffi'] +end +-- Lua 5.2 compatibility +vinyl_index_mt.__pairs = vinyl_index_mt.pairs +vinyl_index_mt.__ipairs = vinyl_index_mt.pairs +memtx_index_mt.__pairs = memtx_index_mt.pairs +memtx_index_mt.__ipairs = memtx_index_mt.pairs + local space_mt = {} space_mt.len = function(space) check_space_arg(space, 'len') @@ -1313,7 +1345,7 @@ space_mt.bsize = function(space) end return builtin.space_bsize(s) end -space_mt.__newindex = index_mt.__newindex +space_mt.__newindex = forbid_new_index space_mt.get = function(space, key) check_space_arg(space, 'get') @@ -1409,22 +1441,13 @@ end space_mt.__index = space_mt function box.schema.space.bless(space) - local index_mt = table.deepcopy(index_mt) local space_mt = table.deepcopy(space_mt) - -- true if reading operations may yield - local read_yields = space.engine == 'vinyl' - local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} - for _, op in ipairs(read_ops) do - if read_yields then - -- use Lua/C implmenetation - index_mt[op] = index_mt[op .. "_luac"] - else - -- use FFI implementation - index_mt[op] = index_mt[op .. "_ffi"] - end + local index_mt + if space.engine == 'vinyl' then + index_mt = table.deepcopy(vinyl_index_mt) + else + index_mt = table.deepcopy(memtx_index_mt) end - index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility - index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility setmetatable(space, space_mt) if type(space.index) == 'table' and space.enabled then -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table 2018-04-03 16:50 ` [PATCH v2 0/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 1/3] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 2/3] schema: inherit vinyl/memtx_index_mt from base index mt Vladislav Shpilevoy @ 2018-04-03 16:50 ` Vladislav Shpilevoy 2018-05-05 12:47 ` [tarantool-patches] " Vladislav Shpilevoy 2 siblings, 1 reply; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-04-03 16:50 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy This commit allows userland to extend the space and index metatables with their own functions or even metamethods. Reducing barriers for this kind of experimentation is vital for user contribution toward the improvement of Tarantool's API. There are 4 metatables available for extending: box.schema.space_mt - metatable of all spaces; box.schema.index_mt - base metatable of all indexes - replicated into the vinyl and memtx. See below how. box.schema.vinyl_index_mt - metatable of all vinyl indexes; box.schema.memtx_index_mt - metatable of all memtx indexes. Closes #3204 --- src/box/lua/schema.lua | 17 +++++---- test/box-tap/schema_mt.test.lua | 80 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 9 deletions(-) create mode 100755 test/box-tap/schema_mt.test.lua diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 4b5eeeda9..5d5b0dc49 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1074,10 +1074,6 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box -local function forbid_new_index(t, k, v) - return error('Attempt to modify a read-only table') -end - local base_index_mt = {} base_index_mt.__index = base_index_mt -- @@ -1098,6 +1094,7 @@ setmetatable(base_index_mt, { rawset(t, k, v) end }) + -- __len and __index base_index_mt.len = function(index) check_index_arg(index, 'len') @@ -1118,7 +1115,6 @@ base_index_mt.bsize = function(index) end -- Lua 5.2 compatibility base_index_mt.__len = base_index_mt.len -base_index_mt.__newindex = forbid_new_index -- min and max base_index_mt.min_ffi = function(index, key) check_index_arg(index, 'min') @@ -1345,7 +1341,6 @@ space_mt.bsize = function(space) end return builtin.space_bsize(s) end -space_mt.__newindex = forbid_new_index space_mt.get = function(space, key) check_space_arg(space, 'get') @@ -1440,13 +1435,17 @@ space_mt.run_triggers = function(space, yesno) end space_mt.__index = space_mt +box.schema.index_mt = base_index_mt +box.schema.memtx_index_mt = memtx_index_mt +box.schema.vinyl_index_mt = vinyl_index_mt +box.schema.space_mt = space_mt + function box.schema.space.bless(space) - local space_mt = table.deepcopy(space_mt) local index_mt if space.engine == 'vinyl' then - index_mt = table.deepcopy(vinyl_index_mt) + index_mt = vinyl_index_mt else - index_mt = table.deepcopy(memtx_index_mt) + index_mt = memtx_index_mt end setmetatable(space, space_mt) diff --git a/test/box-tap/schema_mt.test.lua b/test/box-tap/schema_mt.test.lua new file mode 100755 index 000000000..b785b8e21 --- /dev/null +++ b/test/box-tap/schema_mt.test.lua @@ -0,0 +1,80 @@ +#!/usr/bin/env tarantool +-- +-- pr-3204: expose space_mt, index_mt into box.schema. +-- + +local tap = require('tap') +local test = tap.test('schema_mt') + +test:plan(18) + +box.cfg{ + log="tarantool.log", +} + +-- +-- Check that space metatable is shared between all spaces, +-- regardless of engine. +-- +local sp1 = box.schema.create_space('test1', {engine = 'memtx'}) +local sp2 = box.schema.create_space('test2', {engine = 'vinyl'}) +test:is(getmetatable(sp1), getmetatable(sp2), 'spaces share metatables') + +function box.schema.space_mt.myfunc(space, args) + return args +end +test:is(sp1:myfunc(123), 123, 'space_mt can be extended') +test:is(sp1.myfunc, sp2.myfunc, 'space_mt can be extended') + +-- +-- Check that index metatable is shared in a scope of an engine. +-- +local sp1_pk = sp1:create_index('pk') +local sp1_sk = sp1:create_index('sk') +local sp2_pk = sp2:create_index('pk') +local sp2_sk = sp2:create_index('sk') +test:is(getmetatable(sp1_pk), getmetatable(sp1_sk), 'memtx indexes share metatables') +test:is(getmetatable(sp2_pk), getmetatable(sp2_sk), 'vinyl indexes share metatables') +test:isnt(getmetatable(sp1_pk), getmetatable(sp2_pk), 'engines do not share metatables') + +-- +-- Check that there are two ways to extend index metatable: +-- extend base index metatable, or extend engine specific. +-- +function box.schema.index_mt.common_func(index, args) + return args +end +function box.schema.vinyl_index_mt.vinyl_func(index, args) + return args +end +function box.schema.memtx_index_mt.memtx_func(index, args) + return args +end +test:is(box.schema.index_mt.common_func, box.schema.vinyl_index_mt.common_func, + 'base index_mt is replicated into vinyl index_mt') +test:is(box.schema.index_mt.common_func, box.schema.memtx_index_mt.common_func, + 'base index_mt is replicated into memtx index_mt') +test:is(box.schema.index_mt.vinyl_func, nil, 'vinyl index_mt is not replicated') +test:is(box.schema.index_mt.memtx_func, nil, 'memtx index_mt is not replicated') + +test:is(sp1_pk.common_func, box.schema.index_mt.common_func, + 'new common methods are visible in memtx index') +test:is(sp2_pk.common_func, box.schema.index_mt.common_func, + 'new common methods are visible in vinyl index') + +test:is(sp1_pk.memtx_func, box.schema.memtx_index_mt.memtx_func, + 'new memtx methods are visible in memtx index') +test:is(sp2_pk.vinyl_func, box.schema.vinyl_index_mt.vinyl_func, + 'new vinyl methods are visible in vinyl index') + +test:is(sp1_pk:memtx_func(100), 100, 'memtx local methods work') +test:is(sp1_sk:common_func(200), 200, 'memtx common methods work') +test:is(sp2_pk:vinyl_func(300), 300, 'vinyl local methods work') +test:is(sp2_sk:common_func(400), 400, 'vinyl common methods work') + +sp1:drop() +sp2:drop() + +test:check() + +os.exit(0) -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table 2018-04-03 16:50 ` [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy @ 2018-05-05 12:47 ` Vladislav Shpilevoy 2018-05-08 16:48 ` Konstantin Osipov 0 siblings, 1 reply; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-05-05 12:47 UTC (permalink / raw) To: tarantool-patches; +Cc: Konstantin Osipov Users requested to be able to extend spaces individually. The patch was reworked to allow this. On 03/04/2018 19:50, Vladislav Shpilevoy wrote: > This commit allows userland to extend the space and index > metatables with their own functions or even metamethods. Reducing > barriers for this kind of experimentation is vital for user > contribution toward the improvement of Tarantool's API. > > There are 4 metatables available for extending: > box.schema.space_mt - metatable of all spaces; > box.schema.index_mt - base metatable of all indexes - replicated > into the vinyl and memtx. See below how. > box.schema.vinyl_index_mt - metatable of all vinyl indexes; > box.schema.memtx_index_mt - metatable of all memtx indexes. > > Closes #3204 > --- > src/box/lua/schema.lua | 17 +++++---- > test/box-tap/schema_mt.test.lua | 80 +++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 88 insertions(+), 9 deletions(-) > create mode 100755 test/box-tap/schema_mt.test.lua > schema: expose space_mt and index_mt on box.schema table This commit allows userland to extend the space and index metatables with their own functions or even metamethods. Reducing barriers for this kind of experimentation is vital for user contribution toward the improvement of Tarantool's API. There are 4 metatables available for extending: box.schema.space_mt - metatable of all spaces; box.schema.index_mt - base metatable of all indexes - replicated into the vinyl and memtx. See below how. box.schema.vinyl_index_mt - metatable of all vinyl indexes; box.schema.memtx_index_mt - metatable of all memtx indexes. On the other hand local space/index metatables still can be extended individually to save compatibility with existing modules. Routinely space/index metatable is just a proxy for a global mt. When a user attempts to extend a space or index methods via local space/index metatable instead of from box.schema mt, the local metatable is transformed. Its __index metamethod starts looking up at first in self, and only then into the global mt. Closes #3204 diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index 8a1d23b00..6a3da4897 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1074,10 +1074,6 @@ end internal.check_iterator_type = check_iterator_type -- export for net.box -local function forbid_new_index(t, k, v) - return error('Attempt to modify a read-only table') -end - local base_index_mt = {} base_index_mt.__index = base_index_mt -- @@ -1118,7 +1114,6 @@ base_index_mt.bsize = function(index) end -- Lua 5.2 compatibility base_index_mt.__len = base_index_mt.len -base_index_mt.__newindex = forbid_new_index -- min and max base_index_mt.min_ffi = function(index, key) check_index_arg(index, 'min') @@ -1345,7 +1340,6 @@ space_mt.bsize = function(space) end return builtin.space_bsize(s) end -space_mt.__newindex = forbid_new_index space_mt.get = function(space, key) check_space_arg(space, 'get') @@ -1441,14 +1435,41 @@ end space_mt.frommap = box.internal.space.frommap space_mt.__index = space_mt +box.schema.index_mt = base_index_mt +box.schema.memtx_index_mt = memtx_index_mt +box.schema.vinyl_index_mt = vinyl_index_mt +box.schema.space_mt = space_mt + +-- +-- Wrap a global space/index metatable into a space/index local +-- one. Routinely this metatable just indexes the global one. When +-- a user attempts to extend a space or index methods via local +-- space/index metatable instead of from box.schema mt, the local +-- metatable is transformed. Its __index metamethod starts looking +-- up at first in self, and only then into the global mt. +-- +local function wrap_schema_object_mt(name) + local mt = { __index = box.schema[name] } + local mt_mt = {} + mt_mt.__newindex = function(t, k, v) + mt_mt.__newindex = nil + mt.__index = function(t, k) + return mt[k] or box.schema[name][k] + end + rawset(mt, k, v) + end + setmetatable(mt, mt_mt) + return mt +end + function box.schema.space.bless(space) - local space_mt = table.deepcopy(space_mt) local index_mt if space.engine == 'vinyl' then - index_mt = table.deepcopy(vinyl_index_mt) + index_mt = wrap_schema_object_mt('vinyl_index_mt') else - index_mt = table.deepcopy(memtx_index_mt) + index_mt = wrap_schema_object_mt('memtx_index_mt') end + local space_mt = wrap_schema_object_mt('space_mt') setmetatable(space, space_mt) if type(space.index) == 'table' and space.enabled then diff --git a/test/box-tap/schema_mt.test.lua b/test/box-tap/schema_mt.test.lua new file mode 100755 index 000000000..5a635262f --- /dev/null +++ b/test/box-tap/schema_mt.test.lua @@ -0,0 +1,106 @@ +#!/usr/bin/env tarantool +-- +-- pr-3204: expose space_mt, index_mt into box.schema. +-- + +local tap = require('tap') +local test = tap.test('schema_mt') + +test:plan(26) + +box.cfg{ + log="tarantool.log", +} + +-- +-- Check that space metatable is shared between all spaces, +-- regardless of engine. +-- +local sp1 = box.schema.create_space('test1', {engine = 'memtx'}) +local sp2 = box.schema.create_space('test2', {engine = 'vinyl'}) +test:is(getmetatable(sp1).__index, getmetatable(sp2).__index, 'spaces share metatables __index') + +function box.schema.space_mt.myfunc(space, args) + return args +end +test:is(sp1:myfunc(123), 123, 'space_mt can be extended') +test:is(sp1.myfunc, sp2.myfunc, 'space_mt can be extended') + +-- +-- Check that index metatable is shared in a scope of an engine. +-- +local sp1_pk = sp1:create_index('pk') +local sp1_sk = sp1:create_index('sk') +local sp2_pk = sp2:create_index('pk') +local sp2_sk = sp2:create_index('sk') +test:is(getmetatable(sp1_pk).__index, getmetatable(sp1_sk).__index, 'memtx indexes share metatables __index') +test:is(getmetatable(sp2_pk).__index, getmetatable(sp2_sk).__index, 'vinyl indexes share metatables __index') +test:isnt(getmetatable(sp1_pk).__index, getmetatable(sp2_pk).__index, 'engines do not share metatables __index') + +-- +-- Check that there are two ways to extend index metatable: +-- extend base index metatable, or extend engine specific. +-- +function box.schema.index_mt.common_func(index, args) + return args +end +function box.schema.vinyl_index_mt.vinyl_func(index, args) + return args +end +function box.schema.memtx_index_mt.memtx_func(index, args) + return args +end +test:is(box.schema.index_mt.common_func, box.schema.vinyl_index_mt.common_func, + 'base index_mt is replicated into vinyl index_mt') +test:is(box.schema.index_mt.common_func, box.schema.memtx_index_mt.common_func, + 'base index_mt is replicated into memtx index_mt') +test:is(box.schema.index_mt.vinyl_func, nil, 'vinyl index_mt is not replicated') +test:is(box.schema.index_mt.memtx_func, nil, 'memtx index_mt is not replicated') + +test:is(sp1_pk.common_func, box.schema.index_mt.common_func, + 'new common methods are visible in memtx index') +test:is(sp2_pk.common_func, box.schema.index_mt.common_func, + 'new common methods are visible in vinyl index') + +test:is(sp1_pk.memtx_func, box.schema.memtx_index_mt.memtx_func, + 'new memtx methods are visible in memtx index') +test:is(sp2_pk.vinyl_func, box.schema.vinyl_index_mt.vinyl_func, + 'new vinyl methods are visible in vinyl index') + +test:is(sp1_pk:memtx_func(100), 100, 'memtx local methods work') +test:is(sp1_sk:common_func(200), 200, 'memtx common methods work') +test:is(sp2_pk:vinyl_func(300), 300, 'vinyl local methods work') +test:is(sp2_sk:common_func(400), 400, 'vinyl common methods work') + +-- +-- Test space/index-local methods. +-- A space local metatable can extended so it does not affect +-- other spaces. Same about index. +-- +sp3 = box.schema.create_space('test3', {engine = 'memtx'}) +sp3_pk = sp3:create_index('pk') +sp3_sk = sp3:create_index('sk') +mt1 = getmetatable(sp1) +mt2 = getmetatable(sp2) +test:isnt(mt1, mt2, 'spaces do not share metatables') +index_mt1 = getmetatable(sp3_pk) +index_mt2 = getmetatable(sp3_sk) +test:isnt(index_mt1, index_mt2, 'indexes do not share metatables') + +mt1.my_func = function(a) return a end +test:isnil(mt2.my_func, 'extend local space metatable') +test:is(sp1.my_func(100), 100, 'extend local space metatable') +test:isnil(sp2.my_func, 'extend local space metatable') + +index_mt1.my_func = function(a) return a + 100 end +test:isnil(index_mt2.my_func, 'extend local index metatable') +test:is(sp3_pk.my_func(100), 200, 'extend local index metatable') +test:isnil(sp3_sk.my_func, 'extend local index metatable') + +sp1:drop() +sp2:drop() +sp3:drop() + +test:check() + +os.exit(0) ^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table 2018-05-05 12:47 ` [tarantool-patches] " Vladislav Shpilevoy @ 2018-05-08 16:48 ` Konstantin Osipov 2018-05-08 17:33 ` Vladislav Shpilevoy 0 siblings, 1 reply; 20+ messages in thread From: Konstantin Osipov @ 2018-05-08 16:48 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/05/05 23:23]: This branch breaks app/luafun.test.lua > Users requested to be able to extend spaces individually. The > patch was reworked to allow this. > > On 03/04/2018 19:50, Vladislav Shpilevoy wrote: > >This commit allows userland to extend the space and index > >metatables with their own functions or even metamethods. Reducing > >barriers for this kind of experimentation is vital for user > >contribution toward the improvement of Tarantool's API. > > > >There are 4 metatables available for extending: > >box.schema.space_mt - metatable of all spaces; > >box.schema.index_mt - base metatable of all indexes - replicated > > into the vinyl and memtx. See below how. > >box.schema.vinyl_index_mt - metatable of all vinyl indexes; > >box.schema.memtx_index_mt - metatable of all memtx indexes. > > > >Closes #3204 -- Konstantin Osipov, Moscow, Russia, +7 903 626 22 32 http://tarantool.io - www.twitter.com/kostja_osipov ^ permalink raw reply [flat|nested] 20+ messages in thread
* [tarantool-patches] Re: [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table 2018-05-08 16:48 ` Konstantin Osipov @ 2018-05-08 17:33 ` Vladislav Shpilevoy 0 siblings, 0 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-05-08 17:33 UTC (permalink / raw) To: Konstantin Osipov; +Cc: tarantool-patches Hello. Thanks for review! On 08/05/2018 19:48, Konstantin Osipov wrote: > * Vladislav Shpilevoy <v.shpilevoy@tarantool.org> [18/05/05 23:23]: > > This branch breaks app/luafun.test.lua Sorry, just noticed that my auto test-runner script skips app/ directory. Fixed on the branch. The problem was that luafun expects that space/index metatable contains __ipairs/__pairs metamethods with not proxy via __index. @@ -1449,7 +1449,12 @@ box.schema.space_mt = space_mt -- up at first in self, and only then into the global mt. -- local function wrap_schema_object_mt(name) - local mt = { __index = box.schema[name] } + local global_mt = box.schema[name] + local mt = { + __index = global_mt, + __ipairs = global_mt.__ipairs, + __pairs = global_mt.__pairs + } local mt_mt = {} mt_mt.__newindex = function(t, k, v) mt_mt.__newindex = nil > >> Users requested to be able to extend spaces individually. The >> patch was reworked to allow this. >> >> On 03/04/2018 19:50, Vladislav Shpilevoy wrote: >>> This commit allows userland to extend the space and index >>> metatables with their own functions or even metamethods. Reducing >>> barriers for this kind of experimentation is vital for user >>> contribution toward the improvement of Tarantool's API. >>> >>> There are 4 metatables available for extending: >>> box.schema.space_mt - metatable of all spaces; >>> box.schema.index_mt - base metatable of all indexes - replicated >>> into the vinyl and memtx. See below how. >>> box.schema.vinyl_index_mt - metatable of all vinyl indexes; >>> box.schema.memtx_index_mt - metatable of all memtx indexes. >>> >>> Closes #3204 > ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 2/2] schema: expose space_mt and index_mt on `box.schema` table 2018-03-29 14:31 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-03-29 14:31 ` [PATCH v2 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy @ 2018-03-29 14:31 ` Vladislav Shpilevoy 2018-04-01 9:37 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladimir Davydov 2 siblings, 0 replies; 20+ messages in thread From: Vladislav Shpilevoy @ 2018-03-29 14:31 UTC (permalink / raw) To: tarantool-patches; +Cc: vdavydov.dev, Vladislav Shpilevoy This commit allows userland to extend the space and index metatables with their own functions or even metamethods. Reducing barriers for this kind of experimentation is vital for user contribution toward the improvement of Tarantool's API. Closes #3204 --- src/box/lua/schema.lua | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua index ff89c8e58..2f85bc541 100644 --- a/src/box/lua/schema.lua +++ b/src/box/lua/schema.lua @@ -1387,27 +1387,40 @@ function index_mt:alter(options) end function box.schema.space.bless(space) - local space_mt = table.deepcopy(space_mt) - local index_mt = table.deepcopy(index_mt) + local local_index_mt = {} + -- At first, reference all global index functions. At second, + -- choose an implementation for read operations. They are not + -- in a global index_mt, because they are engine specific. + -- All common index_mt functions must be referenced here to + -- be able to get them using getmetatable(index_object) - if + -- they are only in index_mt, then getmetatable() returns only + -- read ops. + for k, v in pairs(index_mt) do + local_index_mt[k] = v + end + local_index_mt.__index = function(index, key) + return local_index_mt[key] or index_mt[key] + end local read_yields = space.engine == 'vinyl' local read_ops = {'select', 'get', 'min', 'max', 'count', 'random', 'pairs'} for _, op in ipairs(read_ops) do if read_yields then -- use Lua/C implmenetation - index_mt[op] = index_mt[op .. "_luac"] + local_index_mt[op] = local_index_mt[op .. "_luac"] else -- use FFI implementation - index_mt[op] = index_mt[op .. "_ffi"] + local_index_mt[op] = local_index_mt[op .. "_ffi"] end end - index_mt.__pairs = index_mt.pairs -- Lua 5.2 compatibility - index_mt.__ipairs = index_mt.pairs -- Lua 5.2 compatibility + -- Lua 5.2 compatibility + local_index_mt.__pairs = local_index_mt.pairs + local_index_mt.__ipairs = local_index_mt.pairs setmetatable(space, space_mt) if type(space.index) == 'table' and space.enabled then for j, index in pairs(space.index) do if type(j) == 'number' then - setmetatable(index, index_mt) + setmetatable(index, local_index_mt) end end end -- 2.14.3 (Apple Git-98) ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table 2018-03-29 14:31 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-03-29 14:31 ` [PATCH v2 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy 2018-03-29 14:31 ` [PATCH v2 2/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy @ 2018-04-01 9:37 ` Vladimir Davydov 2 siblings, 0 replies; 20+ messages in thread From: Vladimir Davydov @ 2018-04-01 9:37 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches On Thu, Mar 29, 2018 at 05:31:43PM +0300, Vladislav Shpilevoy wrote: > Branch: http://github.com/tarantool/tarantool/tree/pr-3204-expose-space-index-mt > Issue: https://github.com/tarantool/tarantool/issues/3204 Nit-picking: change log is missing, see https://tarantool.io/en/doc/1.9/dev_guide/developer_guidelines.html#how-to-submit-a-patch-for-review > > Vladislav Shpilevoy (1): > schema: expose space_mt and index_mt on `box.schema` table > > aleclarson (1): > schema: move space_mt and index_mt definition out of space bless Please remove Alec from here. > > src/box/lua/schema.lua | 631 ++++++++++++++++++++-------------------- > test/box-tap/schema_mt.test.lua | 79 +++++ > 2 files changed, 391 insertions(+), 319 deletions(-) > create mode 100755 test/box-tap/schema_mt.test.lua ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2018-05-08 17:33 UTC | newest] Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-03-23 13:07 [PATCH 0/2] schema: expose space_mt and index_mt on table Vladislav Shpilevoy 2018-03-23 13:07 ` [PATCH 1/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy 2018-03-23 13:07 ` [PATCH 2/2] schema: review fixes for box.schema.space/index metatables Vladislav Shpilevoy 2018-03-29 10:54 ` [PATCH 0/2] schema: expose space_mt and index_mt on table Vladimir Davydov 2018-03-29 14:31 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-03-29 14:31 ` [PATCH v2 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy 2018-04-01 9:33 ` Vladimir Davydov 2018-04-01 11:02 ` [PATCH 0/2] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-04-01 11:02 ` [PATCH 1/2] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy 2018-04-01 11:02 ` [PATCH 2/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy 2018-04-02 11:28 ` Vladimir Davydov 2018-04-03 16:50 ` [PATCH v2 0/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 1/3] schema: move space_mt and index_mt definition out of space bless Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 2/3] schema: inherit vinyl/memtx_index_mt from base index mt Vladislav Shpilevoy 2018-04-03 16:50 ` [PATCH v2 3/3] schema: expose space_mt and index_mt on box.schema table Vladislav Shpilevoy 2018-05-05 12:47 ` [tarantool-patches] " Vladislav Shpilevoy 2018-05-08 16:48 ` Konstantin Osipov 2018-05-08 17:33 ` Vladislav Shpilevoy 2018-03-29 14:31 ` [PATCH v2 2/2] schema: expose space_mt and index_mt on `box.schema` table Vladislav Shpilevoy 2018-04-01 9:37 ` [PATCH v2 0/2] schema: expose space_mt and index_mt on box.schema table Vladimir Davydov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox