[PATCH 2/2] schema: review fixes for box.schema.space/index metatables

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

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)