Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH v2 0/3] netbox: define formats for tuple from netbox
@ 2019-06-21 15:25 imeevma
  2019-06-21 15:25 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: imeevma @ 2019-06-21 15:25 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: vdavydov.dev, tarantool-patches

This patch defines field names for the tuples obtained through
the netbox.

https://github.com/tarantool/tarantool/issues/2978
https://github.com/tarantool/tarantool/tree/imeevma/gh-2978-formats-for-tuples-from-net-box

Changes in v2 compared to the original v1:
1) Added patch that moves format parsing function to internal.
2) Added destructor to returned cdata.
3) Removed unnecessary functions, that were added in v1.

Mergen Imeev (3):
  netbox: store method_encoder args in request
  lua: internal function to parse space format
  netbox: define formats for tuple from netbox

 src/box/lua/misc.cc       | 120 ++++++++++++++++++++++++++
 src/box/lua/misc.h        |   3 +
 src/box/lua/net_box.c     |  17 ++--
 src/box/lua/net_box.lua   | 209 ++++++++++++++++++++++++++++++----------------
 test/box/access.result    |  15 +++-
 test/box/access.test.lua  |   9 +-
 test/box/misc.result      |  88 +++++++++++++++++++
 test/box/misc.test.lua    |  50 +++++++++++
 test/box/net.box.result   | 104 ++++++++++++++++++++++-
 test/box/net.box.test.lua |  34 +++++++-
 10 files changed, 562 insertions(+), 87 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] netbox: store method_encoder args in request
  2019-06-21 15:25 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
@ 2019-06-21 15:25 ` imeevma
  2019-06-21 20:39   ` [tarantool-patches] " Vladislav Shpilevoy
  2019-06-21 15:25 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma
  2019-06-21 15:25 ` [PATCH v2 3/3] netbox: define formats for tuple from netbox imeevma
  2 siblings, 1 reply; 8+ messages in thread
From: imeevma @ 2019-06-21 15:25 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: vdavydov.dev, tarantool-patches

This patch changes the way arguments are passed to functions in
the array method_encoder, and saves these arguments in REQUEST.
This is necessary to establish a connection between netbox space
objects and the tuples obtained through the netbox

Needed for #2978
---
 src/box/lua/net_box.lua   | 181 ++++++++++++++++++++++++++++++----------------
 test/box/access.result    |  15 +++-
 test/box/access.test.lua  |   9 ++-
 test/box/net.box.result   |   6 +-
 test/box/net.box.test.lua |   6 +-
 5 files changed, 142 insertions(+), 75 deletions(-)

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 251ad40..d9838f8 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -25,7 +25,6 @@ local check_primary_index = box.internal.check_primary_index
 
 local communicate     = internal.communicate
 local encode_auth     = internal.encode_auth
-local encode_select   = internal.encode_select
 local decode_greeting = internal.decode_greeting
 
 local TIMEOUT_INFINITY = 500 * 365 * 86400
@@ -84,22 +83,63 @@ local function decode_push(raw_data)
     return response[IPROTO_DATA_KEY][1], raw_end
 end
 
+local function encode_call(send_buf, id, args)
+    return internal.encode_call(send_buf, id, args.func_name, args.args)
+end
+local function encode_call_16(send_buf, id, args)
+    return internal.encode_call_16(send_buf, id, args.func_name, args.args)
+end
+local function encode_ping(send_buf, id, ...)
+    return internal.encode_ping(send_buf, id, ...)
+end
+local function encode_eval(send_buf, id, args)
+    return internal.encode_eval(send_buf, id, args.code, args.args)
+end
+local function encode_insert(send_buf, id, args)
+    return internal.encode_insert(send_buf, id, args.space_id, args.tuple)
+end
+local function encode_replace(send_buf, id, args)
+    return internal.encode_replace(send_buf, id, args.space_id, args.tuple)
+end
+local function encode_delete(send_buf, id, args)
+    return internal.encode_delete(send_buf, id, args.space_id, args.index_id,
+                                  args.key)
+end
+local function encode_update(send_buf, id, args)
+    return internal.encode_update(send_buf, id, args.space_id, args.index_id,
+                                  args.key, args.oplist)
+end
+local function encode_upsert(send_buf, id, args)
+    return internal.encode_upsert(send_buf, id, args.space_id, args.key,
+                                  args.oplist)
+end
+local function encode_select(send_buf, id, args)
+    return internal.encode_select(send_buf, id, args.space_id, args.index_id,
+                                  args.iterator, args.offset, args.limit,
+                                  args.key)
+end
+local function encode_execute(send_buf, id, args)
+    return internal.encode_execute(send_buf, id, args.query, args.parameters,
+                                   args.sql_opts)
+end
+
+
 local method_encoder = {
-    ping    = internal.encode_ping,
-    call_16 = internal.encode_call_16,
-    call_17 = internal.encode_call,
-    eval    = internal.encode_eval,
-    insert  = internal.encode_insert,
-    replace = internal.encode_replace,
-    delete  = internal.encode_delete,
-    update  = internal.encode_update,
-    upsert  = internal.encode_upsert,
-    select  = internal.encode_select,
-    execute = internal.encode_execute,
-    get     = internal.encode_select,
-    min     = internal.encode_select,
-    max     = internal.encode_select,
-    count   = internal.encode_call,
+    ping    = encode_ping,
+    call_16 = encode_call_16,
+    call_17 = encode_call,
+    eval    = encode_eval,
+    insert  = encode_insert,
+    replace = encode_replace,
+    delete  = encode_delete,
+    update  = encode_update,
+    upsert  = encode_upsert,
+    select  = encode_select,
+    execute = encode_execute,
+    get     = encode_select,
+    min     = encode_select,
+    max     = encode_select,
+    count   = encode_call,
     -- inject raw data into connection, used by console and tests
     inject = function(buf, id, bytes)
         local ptr = buf:reserve(#bytes)
@@ -486,7 +526,7 @@ local function create_transport(host, port, user, password, callback,
     -- @retval not nil Future object.
     --
     local function perform_async_request(buffer, skip_header, method, on_push,
-                                         on_push_ctx, ...)
+                                         on_push_ctx, args)
         if state ~= 'active' and state ~= 'fetch_schema' then
             return nil, box.error.new({code = last_errno or E_NO_CONNECTION,
                                        reason = last_error})
@@ -497,12 +537,12 @@ local function create_transport(host, port, user, password, callback,
             worker_fiber:wakeup()
         end
         local id = next_request_id
-        method_encoder[method](send_buf, id, ...)
+        method_encoder[method](send_buf, id, args)
         next_request_id = next_id(id)
-        -- Request in most cases has maximum 9 members:
+        -- Request in most cases has maximum 10 members:
         -- method, buffer, skip_header, id, cond, errno, response,
-        -- on_push, on_push_ctx.
-        local request = setmetatable(table_new(0, 9), request_mt)
+        -- on_push, on_push_ctx and args.
+        local request = setmetatable(table_new(0, 10), request_mt)
         request.method = method
         request.buffer = buffer
         request.skip_header = skip_header
@@ -511,6 +551,7 @@ local function create_transport(host, port, user, password, callback,
         requests[id] = request
         request.on_push = on_push
         request.on_push_ctx = on_push_ctx
+        request.args = args
         return request
     end
 
@@ -520,10 +561,10 @@ local function create_transport(host, port, user, password, callback,
     -- @retval not nil Response object.
     --
     local function perform_request(timeout, buffer, skip_header, method,
-                                   on_push, on_push_ctx, ...)
+                                   on_push, on_push_ctx, args)
         local request, err =
             perform_async_request(buffer, skip_header, method, on_push,
-                                  on_push_ctx, ...)
+                                  on_push_ctx, args)
         if not request then
             return nil, err
         end
@@ -739,13 +780,14 @@ local function create_transport(host, port, user, password, callback,
         local select3_id = new_request_id()
         local response = {}
         -- fetch everything from space _vspace, 2 = ITER_ALL
-        encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0, 0xFFFFFFFF, nil)
+        internal.encode_select(send_buf, select1_id, VSPACE_ID, 0, 2, 0,
+                               0xFFFFFFFF, nil)
         -- fetch everything from space _vindex, 2 = ITER_ALL
-        encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0, 0xFFFFFFFF, nil)
+        internal.encode_select(send_buf, select2_id, VINDEX_ID, 0, 2, 0,
+                               0xFFFFFFFF, nil)
         -- fetch everything from space _vcollation, 2 = ITER_ALL
-        encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0, 0xFFFFFFFF,
-                      nil)
-
+        internal.encode_select(send_buf, select3_id, VCOLLATION_ID, 0, 2, 0,
+                               0xFFFFFFFF, nil)
         schema_version = nil -- any schema_version will do provided that
                              -- it is consistent across responses
         repeat
@@ -1064,7 +1106,7 @@ function remote_methods:wait_connected(timeout)
     return self._transport.wait_state('active', timeout)
 end
 
-function remote_methods:_request(method, opts, ...)
+function remote_methods:_request(method, opts, args)
     local transport = self._transport
     local on_push, on_push_ctx, buffer, skip_header, deadline
     -- Extract options, set defaults, check if the request is
@@ -1078,7 +1120,7 @@ function remote_methods:_request(method, opts, ...)
             end
             local res, err =
                 transport.perform_async_request(buffer, skip_header, method,
-                                                table.insert, {}, ...)
+                                                table.insert, {}, args)
             if err then
                 box.error(err)
             end
@@ -1106,7 +1148,7 @@ function remote_methods:_request(method, opts, ...)
     end
     local res, err = transport.perform_request(timeout, buffer, skip_header,
                                                method, on_push, on_push_ctx,
-                                               ...)
+                                               args)
     if err then
         box.error(err)
     end
@@ -1133,14 +1175,16 @@ end
 -- @deprecated since 1.7.4
 function remote_methods:call_16(func_name, ...)
     check_remote_arg(self, 'call')
-    return (self:_request('call_16', nil, tostring(func_name), {...}))
+    local args = {func_name = tostring(func_name), args = {...}}
+    return (self:_request('call_16', nil, args))
 end
 
-function remote_methods:call(func_name, args, opts)
+function remote_methods:call(func_name, call_args, opts)
     check_remote_arg(self, 'call')
-    check_call_args(args)
-    args = args or {}
-    local res = self:_request('call_17', opts, tostring(func_name), args)
+    check_call_args(call_args)
+    call_args = call_args or {}
+    local args = {func_name = tostring(func_name), args = call_args}
+    local res = self:_request('call_17', opts, args)
     if type(res) ~= 'table' or opts and opts.is_async then
         return res
     end
@@ -1150,14 +1194,16 @@ end
 -- @deprecated since 1.7.4
 function remote_methods:eval_16(code, ...)
     check_remote_arg(self, 'eval')
-    return unpack((self:_request('eval', nil, code, {...})))
+    local args = {code = code, args = {...}}
+    return unpack((self:_request('eval', nil, args)))
 end
 
-function remote_methods:eval(code, args, opts)
+function remote_methods:eval(code, eval_args, opts)
     check_remote_arg(self, 'eval')
-    check_eval_args(args)
-    args = args or {}
-    local res = self:_request('eval', opts, code, args)
+    check_eval_args(eval_args)
+    eval_args = eval_args or {}
+    local args = {code = code, args = eval_args}
+    local res = self:_request('eval', opts, args)
     if type(res) ~= 'table' or opts and opts.is_async then
         return res
     end
@@ -1169,8 +1215,9 @@ function remote_methods:execute(query, parameters, sql_opts, netbox_opts)
     if sql_opts ~= nil then
         box.error(box.error.UNSUPPORTED, "execute", "options")
     end
-    return self:_request('execute', netbox_opts, query, parameters or {},
-                         sql_opts or {})
+    local args = {query =query, parameters = parameters or {},
+                  sql_opts = sql_opts or {}}
+    return self:_request('execute', netbox_opts, args)
 end
 
 function remote_methods:wait_state(state, timeout)
@@ -1314,7 +1361,8 @@ function console_methods:eval(line, timeout)
     end
     if self.protocol == 'Binary' then
         local loader = 'return require("console").eval(...)'
-        res, err = pr(timeout, nil, false, 'eval', nil, nil, loader, {line})
+        local args = {code = loader, args = {line}}
+        res, err = pr(timeout, nil, false, 'eval', nil, nil, args)
     else
         assert(self.protocol == 'Lua console')
         res, err = pr(timeout, nil, false, 'inject', nil, nil, line..'$EOF$\n')
@@ -1336,12 +1384,14 @@ space_metatable = function(remote)
 
     function methods:insert(tuple, opts)
         check_space_arg(self, 'insert')
-        return remote:_request('insert', opts, self.id, tuple)
+        local args = {space_id = self.id, tuple = tuple}
+        return remote:_request('insert', opts, args)
     end
 
     function methods:replace(tuple, opts)
         check_space_arg(self, 'replace')
-        return remote:_request('replace', opts, self.id, tuple)
+        local args = {space_id = self.id, tuple = tuple}
+        return remote:_request('replace', opts, args)
     end
 
     function methods:select(key, opts)
@@ -1361,8 +1411,8 @@ space_metatable = function(remote)
 
     function methods:upsert(key, oplist, opts)
         check_space_arg(self, 'upsert')
-        return nothing_or_data(remote:_request('upsert', opts, self.id, key,
-                                               oplist))
+        local args = {space_id = self.id, key = key, oplist = oplist}
+        return nothing_or_data(remote:_request('upsert', opts, args))
     end
 
     function methods:get(key, opts)
@@ -1391,8 +1441,10 @@ index_metatable = function(remote)
         local iterator = check_iterator_type(opts, key_is_nil)
         local offset = tonumber(opts and opts.offset) or 0
         local limit = tonumber(opts and opts.limit) or 0xFFFFFFFF
-        return (remote:_request('select', opts, self.space.id, self.id,
-                                iterator, offset, limit, key))
+        local args = {space_id = self.space.id, index_id = self.id,
+                      iterator = iterator, offset = offset, limit = limit,
+                      key = key}
+        return (remote:_request('select', opts, args))
     end
 
     function methods:get(key, opts)
@@ -1400,8 +1452,9 @@ index_metatable = function(remote)
         if opts and opts.buffer then
             error("index:get() doesn't support `buffer` argument")
         end
-        return nothing_or_data(remote:_request('get', opts, self.space.id,
-                               self.id, box.index.EQ, 0, 2, key))
+        local args = {space_id = self.space.id, index_id = self.id,
+                      iterator = box.index.EQ, offset = 0, limit = 2, key = key}
+        return nothing_or_data(remote:_request('get', opts, args))
     end
 
     function methods:min(key, opts)
@@ -1409,9 +1462,9 @@ index_metatable = function(remote)
         if opts and opts.buffer then
             error("index:min() doesn't support `buffer` argument")
         end
-        return nothing_or_data(remote:_request('min', opts, self.space.id,
-                                               self.id, box.index.GE, 0, 1,
-                                               key))
+        local args = {space_id = self.space.id, index_id = self.id,
+                      iterator = box.index.GE, offset = 0, limit = 1, key = key}
+        return nothing_or_data(remote:_request('min', opts, args))
     end
 
     function methods:max(key, opts)
@@ -1419,9 +1472,9 @@ index_metatable = function(remote)
         if opts and opts.buffer then
             error("index:max() doesn't support `buffer` argument")
         end
-        return nothing_or_data(remote:_request('max', opts, self.space.id,
-                                               self.id, box.index.LE, 0, 1,
-                                               key))
+        local args = {space_id = self.space.id, index_id = self.id,
+                      iterator = box.index.LE, offset = 0, limit = 1, key = key}
+        return nothing_or_data(remote:_request('max', opts, args))
     end
 
     function methods:count(key, opts)
@@ -1431,19 +1484,21 @@ index_metatable = function(remote)
         end
         local code = string.format('box.space.%s.index.%s:count',
                                    self.space.name, self.name)
-        return remote:_request('count', opts, code, { key, opts })
+        local args = {func_name = code, args = { key, opts }}
+        return remote:_request('count', opts, args)
     end
 
     function methods:delete(key, opts)
         check_index_arg(self, 'delete')
-        return nothing_or_data(remote:_request('delete', opts, self.space.id,
-                                               self.id, key))
+        local args = {space_id = self.space.id, index_id = self.id, key = key}
+        return nothing_or_data(remote:_request('delete', opts, args))
     end
 
     function methods:update(key, oplist, opts)
         check_index_arg(self, 'update')
-        return nothing_or_data(remote:_request('update', opts, self.space.id,
-                                               self.id, key, oplist))
+        local args = {space_id = self.space.id, index_id = self.id, key = key,
+                      oplist = oplist}
+        return nothing_or_data(remote:_request('update', opts, args))
     end
 
     return { __index = methods, __metatable = false }
diff --git a/test/box/access.result b/test/box/access.result
index 0ebc6a3..e3d4036 100644
--- a/test/box/access.result
+++ b/test/box/access.result
@@ -890,15 +890,24 @@ LISTEN = require('uri').parse(box.cfg.listen)
 c = (require 'net.box').connect(LISTEN.host, LISTEN.service)
 ---
 ...
-c:_request("select", nil, 1, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args = {space_id=1, index_id=0, iterator=box.index.EQ, offset=0, limit=0xFFFFFFFF, key={}}
+---
+...
+c:_request("select", nil, args)
 ---
 - error: Space '1' does not exist
 ...
-c:_request("select", nil, 65537, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args.space_id = 65537
+---
+...
+c:_request("select", nil, args)
 ---
 - error: Space '65537' does not exist
 ...
-c:_request("select", nil, 4294967295, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args.space_id = 4294967295
+---
+...
+c:_request("select", nil, args)
 ---
 - error: Space '4294967295' does not exist
 ...
diff --git a/test/box/access.test.lua b/test/box/access.test.lua
index ad63a10..c63152b 100644
--- a/test/box/access.test.lua
+++ b/test/box/access.test.lua
@@ -343,9 +343,12 @@ box.schema.func.drop(name)
 -- very large space id, no crash occurs.
 LISTEN = require('uri').parse(box.cfg.listen)
 c = (require 'net.box').connect(LISTEN.host, LISTEN.service)
-c:_request("select", nil, 1, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
-c:_request("select", nil, 65537, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
-c:_request("select", nil, 4294967295, box.index.EQ, 0, 0, 0xFFFFFFFF, {})
+args = {space_id=1, index_id=0, iterator=box.index.EQ, offset=0, limit=0xFFFFFFFF, key={}}
+c:_request("select", nil, args)
+args.space_id = 65537
+c:_request("select", nil, args)
+args.space_id = 4294967295
+c:_request("select", nil, args)
 c:close()
 
 session = box.session
diff --git a/test/box/net.box.result b/test/box/net.box.result
index 3652762..269df0d 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -25,9 +25,9 @@ test_run:cmd("setopt delimiter ';'")
 - true
 ...
 function x_select(cn, space_id, index_id, iterator, offset, limit, key, opts)
-    local ret = cn:_request('select', opts, space_id, index_id, iterator,
-                            offset, limit, key)
-    return ret
+    local args = {space_id = space_id, index_id = index_id, iterator = iterator,
+                  offset = offset, limit = limit, key = key}
+    return cn:_request('select', opts, args)
 end
 function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80') end
 test_run:cmd("setopt delimiter ''");
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 96b15c7..026f380 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -8,9 +8,9 @@ test_run:cmd("push filter ".."'\\.lua.*:[0-9]+: ' to '.lua...\"]:<line>: '")
 
 test_run:cmd("setopt delimiter ';'")
 function x_select(cn, space_id, index_id, iterator, offset, limit, key, opts)
-    local ret = cn:_request('select', opts, space_id, index_id, iterator,
-                            offset, limit, key)
-    return ret
+    local args = {space_id = space_id, index_id = index_id, iterator = iterator,
+                  offset = offset, limit = limit, key = key}
+    return cn:_request('select', opts, args)
 end
 function x_fatal(cn) cn._transport.perform_request(nil, nil, false, 'inject', nil, nil, '\x80') end
 test_run:cmd("setopt delimiter ''");
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] lua: internal function to parse space format
  2019-06-21 15:25 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
  2019-06-21 15:25 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
@ 2019-06-21 15:25 ` imeevma
  2019-06-21 20:39   ` [tarantool-patches] " Vladislav Shpilevoy
  2019-06-21 15:25 ` [PATCH v2 3/3] netbox: define formats for tuple from netbox imeevma
  2 siblings, 1 reply; 8+ messages in thread
From: imeevma @ 2019-06-21 15:25 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: vdavydov.dev, tarantool-patches

This patch defines a new function that parses space format and
returns it to Lua as cdata. For this cdata destructor is included.

Needed for #2978
---
 src/box/lua/misc.cc    | 120 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/box/lua/misc.h     |   3 ++
 test/box/misc.result   |  88 ++++++++++++++++++++++++++++++++++++
 test/box/misc.test.lua |  50 +++++++++++++++++++++
 4 files changed, 261 insertions(+)

diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index 8de7401..a835d3d 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -37,9 +37,13 @@
 
 #include "box/box.h"
 #include "box/port.h"
+#include "box/tuple.h"
+#include "box/tuple_format.h"
 #include "box/lua/tuple.h"
 #include "mpstream.h"
 
+static uint32_t CTID_STRUCT_TUPLE_FORMAT_PTR;
+
 /** {{{ Miscellaneous utils **/
 
 char *
@@ -116,14 +120,130 @@ lbox_select(lua_State *L)
 
 /* }}} */
 
+/** {{{ Utils to work with tuple_format. **/
+
+struct tuple_format *
+lbox_check_tuple_format(struct lua_State *L, int narg)
+{
+	uint32_t ctypeid;
+	struct tuple_format *format =
+		*(struct tuple_format **)luaL_checkcdata(L, narg, &ctypeid);
+	if (ctypeid != CTID_STRUCT_TUPLE_FORMAT_PTR) {
+		luaL_error(L, "Invalid argument: 'struct tuple_format *' "
+			   "expected, got %s)",
+			   lua_typename(L, lua_type(L, narg)));
+	}
+	return format;
+}
+
+static int
+lbox_tuple_format_gc(struct lua_State *L)
+{
+	struct tuple_format *format =  lbox_check_tuple_format(L, 1);
+	tuple_format_unref(format);
+	return 0;
+}
+
+static int
+lbox_push_tuple_format(struct lua_State *L, struct tuple_format *format)
+{
+	struct tuple_format **ptr = (struct tuple_format **)
+		luaL_pushcdata(L, CTID_STRUCT_TUPLE_FORMAT_PTR);
+	*ptr = format;
+	tuple_format_ref(format);
+	lua_pushcfunction(L, lbox_tuple_format_gc);
+	luaL_setcdatagc(L, -2);
+	return 1;
+}
+
+static int
+lbox_tuple_format_new(struct lua_State *L)
+{
+	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
+	if (lua_gettop(L) == 0)
+		return lbox_push_tuple_format(L, tuple_format_runtime);
+	if (lua_gettop(L) > 1 || ! lua_istable(L, 1))
+		return luaL_error(L, "Usage: box.internal.format_new(format)");
+	uint32_t count = lua_objlen(L, 1);
+	if (count == 0)
+		return lbox_push_tuple_format(L, tuple_format_runtime);
+	size_t size = count * sizeof(struct field_def);
+	struct region *region = &fiber()->gc;
+	size_t region_svp = region_used(region);
+	struct field_def *fields =
+		(struct field_def *)region_alloc(region, size);
+	if (fields == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc", "fields");
+		return luaT_error(L);
+	}
+	for (uint32_t i = 0; i < count; ++i) {
+		size_t len;
+
+		fields[i] = field_def_default;
+
+		lua_pushinteger(L, i + 1);
+		lua_gettable(L, 1);
+
+		lua_pushstring(L, "type");
+		lua_gettable(L, -2);
+		if (! lua_isnil(L, -1)) {
+			const char *type_name = lua_tolstring(L, -1, &len);
+			fields[i].type = field_type_by_name(type_name, len);
+			if (fields[i].type == field_type_MAX) {
+				const char *err =
+					"field %d has unknown field type";
+				return luaL_error(L, tt_sprintf(err, i + 1));
+			}
+		}
+		lua_pop(L, 1);
+
+		lua_pushstring(L, "name");
+		lua_gettable(L, -2);
+		if (lua_isnil(L, -1)) {
+			return luaL_error(L, tt_sprintf("field %d name is not "
+							"specified", i + 1));
+		}
+		const char *name = lua_tolstring(L, -1, &len);
+		fields[i].name = (char *)region_alloc(region, len + 1);
+		if (fields == NULL) {
+			diag_set(OutOfMemory, size, "region_alloc",
+				 "fields[i].name");
+			return luaT_error(L);
+		}
+		memcpy(fields[i].name, name, len);
+		fields[i].name[len] = '\0';
+		lua_pop(L, 1);
+		lua_pop(L, 1);
+	}
+	struct tuple_dictionary *dict = tuple_dictionary_new(fields, count);
+	region_truncate(region, region_svp);
+	if (dict == NULL)
+		return luaT_error(L);
+	struct tuple_format *format =
+		tuple_format_new(&tuple_format_runtime->vtab, NULL, NULL, 0,
+				 NULL, 0, 0, dict, false, false);
+	if (format == NULL)
+		return luaT_error(L);
+	return lbox_push_tuple_format(L, format);
+}
+
+/* }}} */
+
 void
 box_lua_misc_init(struct lua_State *L)
 {
 	static const struct luaL_Reg boxlib_internal[] = {
 		{"select", lbox_select},
+		{"new_tuple_format", lbox_tuple_format_new},
 		{NULL, NULL}
 	};
 
 	luaL_register(L, "box.internal", boxlib_internal);
 	lua_pop(L, 1);
+
+	int rc = luaL_cdef(L, "struct tuple_format;");
+	assert(rc == 0);
+	(void) rc;
+	CTID_STRUCT_TUPLE_FORMAT_PTR = luaL_ctypeid(L, "struct tuple_format *");
+	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
 }
diff --git a/src/box/lua/misc.h b/src/box/lua/misc.h
index dfedfe3..1c6f1f6 100644
--- a/src/box/lua/misc.h
+++ b/src/box/lua/misc.h
@@ -45,6 +45,9 @@ lbox_encode_tuple_on_gc(struct lua_State *L, int idx, size_t *p_len);
 void
 box_lua_misc_init(struct lua_State *L);
 
+struct tuple_format *
+lbox_check_tuple_format(struct lua_State *L, int narg);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/test/box/misc.result b/test/box/misc.result
index 43b5a4a..5c42e33 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1271,3 +1271,91 @@ box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
 ---
 ...
+--
+-- gh-2978: Function to parse space format.
+--
+-- Error: no field name:
+format = {}
+---
+...
+format[1] = {}
+---
+...
+format[1].type = 'unsigned'
+---
+...
+box.internal.new_tuple_format(format)
+---
+- error: field 1 name is not specified
+...
+-- Error: duplicate field name:
+format[1].name = 'aaa'
+---
+...
+format[2] = {}
+---
+...
+format[2].name = 'aaa'
+---
+...
+format[2].type = 'any'
+---
+...
+box.internal.new_tuple_format(format)
+---
+- error: Space field 'aaa' is duplicate
+...
+-- Error: wrong field type:
+format[2].name = 'bbb'
+---
+...
+format[3] = {}
+---
+...
+format[3].name = 'ccc'
+---
+...
+format[3].type = 'something'
+---
+...
+box.internal.new_tuple_format(format)
+---
+- error: field 3 has unknown field type
+...
+-- Error: too many arguments:
+box.internal.new_tuple_format(format, format)
+---
+- error: 'Usage: box.internal.format_new(format)'
+...
+-- Error: wrong argument:
+box.internal.new_tuple_format(1)
+---
+- error: 'Usage: box.internal.format_new(format)'
+...
+--
+-- In next tests we should receive cdata("struct tuple_format *").
+-- We do not have a way to check cdata in Lua, but there should be
+-- no errors.
+--
+-- Without argument it is equivalent to new_tuple_format({})
+tuple_format = box.internal.new_tuple_format()
+---
+...
+-- If no type that type == "any":
+format[3].type = nil
+---
+...
+tuple_format = box.internal.new_tuple_format(format)
+---
+...
+-- Function space:format() without arguments returns valid format:
+tuple_format = box.internal.new_tuple_format(box.space._space:format())
+---
+...
+-- Check is_nullable option fo field
+format[2].is_nullable = true
+---
+...
+tuple_format = box.internal.new_tuple_format(format)
+---
+...
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 75d937e..94a6450 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -353,3 +353,53 @@ lsn == expected_lsn
 box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
 
+
+--
+-- gh-2978: Function to parse space format.
+--
+
+-- Error: no field name:
+format = {}
+format[1] = {}
+format[1].type = 'unsigned'
+box.internal.new_tuple_format(format)
+
+-- Error: duplicate field name:
+format[1].name = 'aaa'
+format[2] = {}
+format[2].name = 'aaa'
+format[2].type = 'any'
+box.internal.new_tuple_format(format)
+
+-- Error: wrong field type:
+format[2].name = 'bbb'
+format[3] = {}
+format[3].name = 'ccc'
+format[3].type = 'something'
+box.internal.new_tuple_format(format)
+
+-- Error: too many arguments:
+box.internal.new_tuple_format(format, format)
+
+-- Error: wrong argument:
+box.internal.new_tuple_format(1)
+
+--
+-- In next tests we should receive cdata("struct tuple_format *").
+-- We do not have a way to check cdata in Lua, but there should be
+-- no errors.
+--
+
+-- Without argument it is equivalent to new_tuple_format({})
+tuple_format = box.internal.new_tuple_format()
+
+-- If no type that type == "any":
+format[3].type = nil
+tuple_format = box.internal.new_tuple_format(format)
+
+-- Function space:format() without arguments returns valid format:
+tuple_format = box.internal.new_tuple_format(box.space._space:format())
+
+-- Check is_nullable option fo field
+format[2].is_nullable = true
+tuple_format = box.internal.new_tuple_format(format)
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] netbox: define formats for tuple from netbox
  2019-06-21 15:25 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
  2019-06-21 15:25 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
  2019-06-21 15:25 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma
@ 2019-06-21 15:25 ` imeevma
  2019-06-21 20:39   ` [tarantool-patches] " Vladislav Shpilevoy
  2 siblings, 1 reply; 8+ messages in thread
From: imeevma @ 2019-06-21 15:25 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: vdavydov.dev, tarantool-patches

This patch creates tuple_formats for the tuples obtained through
the netbox.

Closes #2978

@TarantoolBot document
Title: Field names for tuples received from net.box

It is possible now to access by field name for tuples received
from net.box. For example:

box.cfg{listen = 3302}
box.schema.user.grant('guest','read, write, execute', 'space')
box.schema.user.grant('guest', 'create', 'space')

box.schema.create_space("named", {format = {{name = "id"}}})
box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
box.space.named:insert({1})
require('net.box').connect('localhost', 3302).space.named:get(1).id

Result:

tarantool> require('net.box').connect('localhost', 3302).space.named:get(1).id
---
- 1
...
---
 src/box/lua/net_box.c     | 17 +++++---
 src/box/lua/net_box.lua   | 44 +++++++++++++--------
 test/box/net.box.result   | 98 +++++++++++++++++++++++++++++++++++++++++++++++
 test/box/net.box.test.lua | 28 ++++++++++++++
 4 files changed, 167 insertions(+), 20 deletions(-)

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 7484a86..ad7bc6a 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -48,6 +48,7 @@
 #include "box/errcode.h"
 #include "lua/fiber.h"
 #include "mpstream.h"
+#include "misc.h" /* lbox_check_tuple_format() */
 
 #define cfg luaL_msgpack_default
 
@@ -590,12 +591,11 @@ netbox_encode_execute(lua_State *L)
  * @param data MessagePack.
  */
 static void
-netbox_decode_data(struct lua_State *L, const char **data)
+netbox_decode_data(struct lua_State *L, const char **data,
+		   struct tuple_format *format)
 {
 	uint32_t count = mp_decode_array(data);
 	lua_createtable(L, count, 0);
-	struct tuple_format *format =
-		box_tuple_format_default();
 	for (uint32_t j = 0; j < count; ++j) {
 		const char *begin = *data;
 		mp_next(data);
@@ -618,6 +618,13 @@ static int
 netbox_decode_select(struct lua_State *L)
 {
 	uint32_t ctypeid;
+	int top = lua_gettop(L);
+	assert(top == 1 || top == 2);
+	struct tuple_format *format;
+	if (top == 2 && lua_type(L, 2) == LUA_TCDATA)
+		format = lbox_check_tuple_format(L, 2);
+	else
+		format = tuple_format_runtime;
 	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
 	assert(mp_typeof(*data) == MP_MAP);
 	uint32_t map_size = mp_decode_map(&data);
@@ -627,7 +634,7 @@ netbox_decode_select(struct lua_State *L)
 	uint32_t key = mp_decode_uint(&data);
 	assert(key == IPROTO_DATA);
 	(void) key;
-	netbox_decode_data(L, &data);
+	netbox_decode_data(L, &data, format);
 	*(const char **)luaL_pushcdata(L, ctypeid) = data;
 	return 2;
 }
@@ -716,7 +723,7 @@ netbox_decode_execute(struct lua_State *L)
 		uint32_t key = mp_decode_uint(&data);
 		switch(key) {
 		case IPROTO_DATA:
-			netbox_decode_data(L, &data);
+			netbox_decode_data(L, &data, tuple_format_runtime);
 			rows_index = i - map_size;
 			break;
 		case IPROTO_METADATA:
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index d9838f8..8f31712 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -63,12 +63,12 @@ local function decode_data(raw_data)
     local response, raw_end = decode(raw_data)
     return response[IPROTO_DATA_KEY], raw_end
 end
-local function decode_tuple(raw_data)
-    local response, raw_end = internal.decode_select(raw_data)
+local function decode_tuple(raw_data, raw_data_end, args)
+    local response, raw_end = internal.decode_select(raw_data, args.format)
     return response[1], raw_end
 end
-local function decode_get(raw_data)
-    local body, raw_end = internal.decode_select(raw_data)
+local function decode_get(raw_data, raw_data_end, args)
+    local body, raw_end = internal.decode_select(raw_data, args.format)
     if body[2] then
         return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
     end
@@ -82,6 +82,12 @@ local function decode_push(raw_data)
     local response, raw_end = decode(raw_data)
     return response[IPROTO_DATA_KEY][1], raw_end
 end
+local function decode_call_16(raw_data)
+    return internal.decode_select(raw_data)
+end
+local function decode_select(raw_data, raw_data_end, args)
+    return internal.decode_select(raw_data, args.format)
+end
 
 local function encode_call(send_buf, id, args)
     return internal.encode_call(send_buf, id, args.func_name, args.args)
@@ -150,7 +156,7 @@ local method_encoder = {
 
 local method_decoder = {
     ping    = decode_nil,
-    call_16 = internal.decode_select,
+    call_16 = decode_call_16,
     call_17 = decode_data,
     eval    = decode_data,
     insert  = decode_tuple,
@@ -158,7 +164,7 @@ local method_decoder = {
     delete  = decode_tuple,
     update  = decode_tuple,
     upsert  = decode_nil,
-    select  = internal.decode_select,
+    select  = decode_select,
     execute = internal.decode_execute,
     get     = decode_get,
     min     = decode_get,
@@ -623,7 +629,8 @@ local function create_transport(host, port, user, password, callback,
         -- Decode xrow.body[DATA] to Lua objects
         if status == IPROTO_OK_KEY then
             request.response, real_end, request.errno =
-                method_decoder[request.method](body_rpos, body_end)
+                method_decoder[request.method](body_rpos, body_end,
+                                               request.args)
             assert(real_end == body_end, "invalid body length")
             requests[id] = nil
             request.id = nil
@@ -1267,6 +1274,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
         s.index = {}
         s.temporary = false
         s._format = format
+        s._format_cdata = box.internal.new_tuple_format(format)
         s.connection = self
         if #space > 5 then
             local opts = space[6]
@@ -1384,13 +1392,15 @@ space_metatable = function(remote)
 
     function methods:insert(tuple, opts)
         check_space_arg(self, 'insert')
-        local args = {space_id = self.id, tuple = tuple}
+        local args = {space_id = self.id, tuple = tuple,
+                      format = self._format_cdata}
         return remote:_request('insert', opts, args)
     end
 
     function methods:replace(tuple, opts)
         check_space_arg(self, 'replace')
-        local args = {space_id = self.id, tuple = tuple}
+        local args = {space_id = self.id, tuple = tuple,
+                      format = self._format_cdata}
         return remote:_request('replace', opts, args)
     end
 
@@ -1443,7 +1453,7 @@ index_metatable = function(remote)
         local limit = tonumber(opts and opts.limit) or 0xFFFFFFFF
         local args = {space_id = self.space.id, index_id = self.id,
                       iterator = iterator, offset = offset, limit = limit,
-                      key = key}
+                      key = key, format = self.space._format_cdata}
         return (remote:_request('select', opts, args))
     end
 
@@ -1453,7 +1463,8 @@ index_metatable = function(remote)
             error("index:get() doesn't support `buffer` argument")
         end
         local args = {space_id = self.space.id, index_id = self.id,
-                      iterator = box.index.EQ, offset = 0, limit = 2, key = key}
+                      iterator = box.index.EQ, offset = 0, limit = 2, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('get', opts, args))
     end
 
@@ -1463,7 +1474,8 @@ index_metatable = function(remote)
             error("index:min() doesn't support `buffer` argument")
         end
         local args = {space_id = self.space.id, index_id = self.id,
-                      iterator = box.index.GE, offset = 0, limit = 1, key = key}
+                      iterator = box.index.GE, offset = 0, limit = 1, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('min', opts, args))
     end
 
@@ -1473,7 +1485,8 @@ index_metatable = function(remote)
             error("index:max() doesn't support `buffer` argument")
         end
         local args = {space_id = self.space.id, index_id = self.id,
-                      iterator = box.index.LE, offset = 0, limit = 1, key = key}
+                      iterator = box.index.LE, offset = 0, limit = 1, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('max', opts, args))
     end
 
@@ -1490,14 +1503,15 @@ index_metatable = function(remote)
 
     function methods:delete(key, opts)
         check_index_arg(self, 'delete')
-        local args = {space_id = self.space.id, index_id = self.id, key = key}
+        local args = {space_id = self.space.id, index_id = self.id, key = key,
+                      format = self.space._format_cdata}
         return nothing_or_data(remote:_request('delete', opts, args))
     end
 
     function methods:update(key, oplist, opts)
         check_index_arg(self, 'update')
         local args = {space_id = self.space.id, index_id = self.id, key = key,
-                      oplist = oplist}
+                      oplist = oplist, format = self.space._format_cdata}
         return nothing_or_data(remote:_request('update', opts, args))
     end
 
diff --git a/test/box/net.box.result b/test/box/net.box.result
index 269df0d..1248c64 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3572,6 +3572,104 @@ box.schema.func.drop('change_schema')
 ---
 ...
 --
+-- gh-2978: field names for tuples received from netbox.
+--
+_ = box.schema.create_space("named", {format = {{name = "id"}, {name="abc"}}})
+---
+...
+_ = box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
+---
+...
+box.space.named:insert({1, 1})
+---
+- [1, 1]
+...
+box.schema.user.grant('guest', 'read, write, execute', 'space')
+---
+...
+cn = net.connect(box.cfg.listen)
+---
+...
+s = cn.space.named
+---
+...
+s:get{1}.id
+---
+- 1
+...
+s:get{1}:tomap()
+---
+- 1: 1
+  2: 1
+  abc: 1
+  id: 1
+...
+s:insert{2,3}:tomap()
+---
+- 1: 2
+  2: 3
+  abc: 3
+  id: 2
+...
+s:replace{2,14}:tomap()
+---
+- 1: 2
+  2: 14
+  abc: 14
+  id: 2
+...
+s:update(1, {{'+', 2, 10}}):tomap()
+---
+- 1: 1
+  2: 11
+  abc: 11
+  id: 1
+...
+s:select()[1]:tomap()
+---
+- 1: 1
+  2: 11
+  abc: 11
+  id: 1
+...
+s:delete({2}):tomap()
+---
+- 1: 2
+  2: 14
+  abc: 14
+  id: 2
+...
+-- Check that formats changes after reload.
+box.space.named:format({{name = "id2"}, {name="abc2"}})
+---
+...
+s:select()[1]:tomap()
+---
+- 1: 1
+  2: 11
+  abc: 11
+  id: 1
+...
+cn:reload_schema()
+---
+...
+s:select()[1]:tomap()
+---
+- 1: 1
+  2: 11
+  id2: 1
+  abc2: 11
+...
+cn:close()
+---
+...
+box.space.named:drop()
+---
+...
+box.schema.user.revoke('guest', 'read, write, execute', 'space')
+---
+...
+--
 -- gh-3400: long-poll input discard must not touch event loop of
 -- a closed connection.
 --
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 026f380..f222ad9 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1433,6 +1433,34 @@ box.space.test3:drop()
 box.schema.func.drop('change_schema')
 
 --
+-- gh-2978: field names for tuples received from netbox.
+--
+_ = box.schema.create_space("named", {format = {{name = "id"}, {name="abc"}}})
+_ = box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
+box.space.named:insert({1, 1})
+box.schema.user.grant('guest', 'read, write, execute', 'space')
+cn = net.connect(box.cfg.listen)
+
+s = cn.space.named
+s:get{1}.id
+s:get{1}:tomap()
+s:insert{2,3}:tomap()
+s:replace{2,14}:tomap()
+s:update(1, {{'+', 2, 10}}):tomap()
+s:select()[1]:tomap()
+s:delete({2}):tomap()
+
+-- Check that formats changes after reload.
+box.space.named:format({{name = "id2"}, {name="abc2"}})
+s:select()[1]:tomap()
+cn:reload_schema()
+s:select()[1]:tomap()
+
+cn:close()
+box.space.named:drop()
+box.schema.user.revoke('guest', 'read, write, execute', 'space')
+
+--
 -- gh-3400: long-poll input discard must not touch event loop of
 -- a closed connection.
 --
-- 
2.7.4

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tarantool-patches] [PATCH v2 3/3] netbox: define formats for tuple from netbox
  2019-06-21 15:25 ` [PATCH v2 3/3] netbox: define formats for tuple from netbox imeevma
@ 2019-06-21 20:39   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy @ 2019-06-21 20:39 UTC (permalink / raw)
  To: tarantool-patches, imeevma; +Cc: vdavydov.dev

Thanks for the patch!

> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
> @@ -618,6 +618,13 @@ static int
>  netbox_decode_select(struct lua_State *L)
>  {
>  	uint32_t ctypeid;
> +	int top = lua_gettop(L);
> +	assert(top == 1 || top == 2);
> +	struct tuple_format *format;
> +	if (top == 2 && lua_type(L, 2) == LUA_TCDATA)
> +		format = lbox_check_tuple_format(L, 2);

How is it possible, that we do not have a format here?

> +	else
> +		format = tuple_format_runtime;
>  	const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
>  	assert(mp_typeof(*data) == MP_MAP);
>  	uint32_t map_size = mp_decode_map(&data);

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tarantool-patches] [PATCH v2 2/3] lua: internal function to parse space format
  2019-06-21 15:25 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma
@ 2019-06-21 20:39   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy @ 2019-06-21 20:39 UTC (permalink / raw)
  To: tarantool-patches, imeevma; +Cc: vdavydov.dev

Thanks for the patch!

> +
> +static int
> +lbox_tuple_format_new(struct lua_State *L)
> +{
> +	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
> +	if (lua_gettop(L) == 0)
> +		return lbox_push_tuple_format(L, tuple_format_runtime);
> +	if (lua_gettop(L) > 1 || ! lua_istable(L, 1))
> +		return luaL_error(L, "Usage: box.internal.format_new(format)");
> +	uint32_t count = lua_objlen(L, 1);
> +	if (count == 0)
> +		return lbox_push_tuple_format(L, tuple_format_runtime);
> +	size_t size = count * sizeof(struct field_def);
> +	struct region *region = &fiber()->gc;
> +	size_t region_svp = region_used(region);
> +	struct field_def *fields =
> +		(struct field_def *)region_alloc(region, size);
> +	if (fields == NULL) {
> +		diag_set(OutOfMemory, size, "region_alloc", "fields");
> +		return luaT_error(L);
> +	}
> +	for (uint32_t i = 0; i < count; ++i) {
> +		size_t len;
> +
> +		fields[i] = field_def_default;
> +
> +		lua_pushinteger(L, i + 1);
> +		lua_gettable(L, 1);
> +
> +		lua_pushstring(L, "type");
> +		lua_gettable(L, -2);
> +		if (! lua_isnil(L, -1)) {
> +			const char *type_name = lua_tolstring(L, -1, &len);
> +			fields[i].type = field_type_by_name(type_name, len);
> +			if (fields[i].type == field_type_MAX) {
> +				const char *err =
> +					"field %d has unknown field type";
> +				return luaL_error(L, tt_sprintf(err, i + 1));

Here and in all other 'return on error' the region leaks. But there is
another point. How can there be errors, if the method is internal? If I
had implemented that patch, I would panic/assert on any error here, except
region OOM.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [tarantool-patches] [PATCH v2 1/3] netbox: store method_encoder args in request
  2019-06-21 15:25 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
@ 2019-06-21 20:39   ` Vladislav Shpilevoy
  0 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy @ 2019-06-21 20:39 UTC (permalink / raw)
  To: tarantool-patches, imeevma; +Cc: vdavydov.dev

Hi! Thanks for the patchset!

First of all I would like to say, that I impressed by its
simplicity, good work. I have only some minor comments except
one concerning internal netbox API and perf.

See 3 comments below.

On 21/06/2019 17:25, imeevma@tarantool.org wrote:
> This patch changes the way arguments are passed to functions in
> the array method_encoder, and saves these arguments in REQUEST.
> This is necessary to establish a connection between netbox space
> objects and the tuples obtained through the netbox
> 
> Needed for #2978
> ---
>  src/box/lua/net_box.lua   | 181 ++++++++++++++++++++++++++++++----------------
>  test/box/access.result    |  15 +++-
>  test/box/access.test.lua  |   9 ++-
>  test/box/net.box.result   |   6 +-
>  test/box/net.box.test.lua |   6 +-
>  5 files changed, 142 insertions(+), 75 deletions(-)
> 
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index 251ad40..d9838f8 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -25,7 +25,6 @@ local check_primary_index = box.internal.check_primary_index
>  
>  local communicate     = internal.communicate
>  local encode_auth     = internal.encode_auth
> -local encode_select   = internal.encode_select
>  local decode_greeting = internal.decode_greeting
>  
>  local TIMEOUT_INFINITY = 500 * 365 * 86400
> @@ -84,22 +83,63 @@ local function decode_push(raw_data)
>      return response[IPROTO_DATA_KEY][1], raw_end
>  end
>  
> +local function encode_call(send_buf, id, args)

1. It would be great to see here a comment on why does all
these methods need request args saved.

Secondly, why do you need the args for call/eval/ping/execute?
These function does not return tuples from a concrete space.

For DML/DQL you need format ptr only, so why do you store the whole
set of parameters? You could store just one scalar value. See below
how.

> +    return internal.encode_call(send_buf, id, args.func_name, args.args)
> +end
> +local function encode_call_16(send_buf, id, args)
> +    return internal.encode_call_16(send_buf, id, args.func_name, args.args)
> +end
> +local function encode_ping(send_buf, id, ...)
> +    return internal.encode_ping(send_buf, id, ...)
> +end
> +local function encode_eval(send_buf, id, args)
> +    return internal.encode_eval(send_buf, id, args.code, args.args)
> +end
> +local function encode_insert(send_buf, id, args)
> +    return internal.encode_insert(send_buf, id, args.space_id, args.tuple)
> +end
> +local function encode_replace(send_buf, id, args)
> +    return internal.encode_replace(send_buf, id, args.space_id, args.tuple)
> +end
> +local function encode_delete(send_buf, id, args)
> +    return internal.encode_delete(send_buf, id, args.space_id, args.index_id,
> +                                  args.key)
> +end
> +local function encode_update(send_buf, id, args)
> +    return internal.encode_update(send_buf, id, args.space_id, args.index_id,
> +                                  args.key, args.oplist)
> +end
> +local function encode_upsert(send_buf, id, args)
> +    return internal.encode_upsert(send_buf, id, args.space_id, args.key,
> +                                  args.oplist)
> +end
> +local function encode_select(send_buf, id, args)
> +    return internal.encode_select(send_buf, id, args.space_id, args.index_id,
> +                                  args.iterator, args.offset, args.limit,
> +                                  args.key)
> +end
> +local function encode_execute(send_buf, id, args)
> +    return internal.encode_execute(send_buf, id, args.query, args.parameters,
> +                                   args.sql_opts)
> +end
> +
> +
>  local method_encoder = {
> -    ping    = internal.encode_ping,
> -    call_16 = internal.encode_call_16,
> -    call_17 = internal.encode_call,
> -    eval    = internal.encode_eval,

2. You could keep these functions as is, couldn't you? The problem with current
version is that before your patch method_encoder.call would invoke C function
directly. After your patch each 'call' firstly lookups 'internal' table. This is
why me and Nick were trying to avoid calling any 'internal' functions via
accessing 'internal' table.

> -    insert  = internal.encode_insert,
> -    replace = internal.encode_replace,
> -    delete  = internal.encode_delete,
> -    update  = internal.encode_update,
> -    upsert  = internal.encode_upsert,
> -    select  = internal.encode_select,
> -    execute = internal.encode_execute,
> -    get     = internal.encode_select,
> -    min     = internal.encode_select,
> -    max     = internal.encode_select,
> -    count   = internal.encode_call,
> +    ping    = encode_ping,
> +    call_16 = encode_call_16,
> +    call_17 = encode_call,
> +    eval    = encode_eval,
> +    insert  = encode_insert,
> +    replace = encode_replace,
> +    delete  = encode_delete,
> +    update  = encode_update,
> +    upsert  = encode_upsert,
> +    select  = encode_select,
> +    execute = encode_execute,
> +    get     = encode_select,
> +    min     = encode_select,
> +    max     = encode_select,
> +    count   = encode_call,
>      -- inject raw data into connection, used by console and tests
>      inject = function(buf, id, bytes)
>          local ptr = buf:reserve(#bytes)
> @@ -486,7 +526,7 @@ local function create_transport(host, port, user, password, callback,
>      -- @retval not nil Future object.
>      --
>      local function perform_async_request(buffer, skip_header, method, on_push,
> -                                         on_push_ctx, ...)
> +                                         on_push_ctx, args)
3. Problem with wrapping each time the parameters into a table is
that table wrap is usually orders of magnitude slower than variable
args pass via stack. It was benched recently. If the patch had not
been pushed, my suggestion would be to make request() method as

    perform_async_request(buffer, skip_header, method, on_push, on_push_ctx,
                          *request_ctx*, ...)

This function internally would do

    request.ctx = request_ctx

That 'ctx' would be ignored by all decoders except DML/DQL. The latter
will store format pointer here. For other requests it would be nil, and
costs nothing. 'nil' on Lua stack is stored as a special value and does
not occupy memory except stack cell.

Motivation of my proposal is that netbox *is used* in highload, despite
we sometimes forget about it. In vshard netbox is used extremely hard. Each
new request table member costs perf and memory. For example, if we have
300k RPS from netbox connections on one instance (it is a real number), and
args table pack costs us +1us (in fact even more), then it is not hard to
calculate, that each second we will spend nearly 3ms on packing args.
Taking into account the fact, that network delays are 'ms' too, it looks
quite expensive.

https://github.com/tarantool/vshard/issues/186#issuecomment-500380508

>          if state ~= 'active' and state ~= 'fetch_schema' then
>              return nil, box.error.new({code = last_errno or E_NO_CONNECTION,
>                                         reason = last_error})

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] lua: internal function to parse space format
  2019-06-19 10:34 [PATCH v2 0/3] " imeevma
@ 2019-06-19 10:34 ` imeevma
  0 siblings, 0 replies; 8+ messages in thread
From: imeevma @ 2019-06-19 10:34 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches

Thank you for review! My answers, diff between versions and new
patch below.

On 6/18/19 11:55 AM, Vladimir Davydov wrote:
> On Fri, Jun 14, 2019 at 03:29:21PM +0300, Mergen Imeev wrote:
>> First new patch:
>
> It looks like it's time to send v2 in a separate thread.
>
Done.

> See a few minor comments inline.
>
>>
>> From 63075deb8411643d4af0cf27be2c024b5a20222c Mon Sep 17 00:00:00 2001
>> Date: Tue, 11 Jun 2019 15:21:39 +0300
>> Subject: [PATCH] lua: internal function to parse space format
>>
>> This patch defines a new function that parses space format and
>> returns it to Lua as cdata. For this cdata destructor is included.
>>
>> Needed for #2978
>>
>> diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
>> index 8de7401..e62e2ba 100644
>> --- a/src/box/lua/misc.cc
>> +++ b/src/box/lua/misc.cc
>> @@ -37,9 +37,13 @@
>>  
>>  #include "box/box.h"
>>  #include "box/port.h"
>> +#include "box/tuple.h"
>> +#include "box/tuple_format.h"
>>  #include "box/lua/tuple.h"
>>  #include "mpstream.h"
>>  
>> +uint32_t CTID_STRUCT_TUPLE_FORMAT_PTR;
>> +
>
> Should be static.
>
Fixed.

>>  /** {{{ Miscellaneous utils **/
>>  
>>  char *
>> @@ -116,14 +120,117 @@ lbox_select(lua_State *L)
>>  
>>  /* }}} */
>>  
>> +static int
>> +lbox_tuple_format_gc(struct lua_State *L)
>> +{
>> +	uint32_t ctypeid;
>> +	struct tuple_format *format =
>> +		*(struct tuple_format **)luaL_checkcdata(L, 1, &ctypeid);
>> +	if (ctypeid != CTID_STRUCT_TUPLE_FORMAT_PTR) {
>> +		luaL_error(L, "Invalid argument: 'struct tuple_format *' "\
>
> Backslash (\) is not necessary.
>
Fixed.

> Anyway, I think we better factor this check out into a separate
> function, like lboxk_check_tuple_format, so that we could easily
> reuse it in case we introduce some new functions taking a format.
>
>> +			   "expected, got %s)",
>> +			   lua_typename(L, lua_type(L, 1)));
>> +	}
>> +	box_tuple_format_unref(format);
>
> Please use tuple_format_ref/tuple_format_unref.
>
> box_* functions exist for external modules. No need to use them here.
>
Fixed.

>> +	return 0;
>> +}
>> +
>> +static int
>> +lbox_push_tuple_format(struct lua_State *L, struct tuple_format *format)
>> +{
>> +	struct tuple_format **ptr = (struct tuple_format **)
>> +		luaL_pushcdata(L, CTID_STRUCT_TUPLE_FORMAT_PTR);
>> +	*ptr = format;
>> +	box_tuple_format_ref(format);
>> +	lua_pushcfunction(L, lbox_tuple_format_gc);
>> +	luaL_setcdatagc(L, -2);
>> +	return 1;
>> +}
>> +
>> +static int
>> +lbox_format_new(struct lua_State *L)
>
> Please be consistent and use tuple_format prefix everywhere.
>
Fixed.

>> +{
>> +	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
>> +	if (lua_gettop(L) != 1 || ! lua_istable(L, 1))
>> +		return luaL_error(L, "Usage: box.internal.format_new(format)");
>
> I would rather name this function box.internal.new_tuple_format.
>
Fixed.

> Also, I think we should allow calling it without any arguments, in which
> case it should be equivalent to box.internal.new_tuple_format({}).
>
Fixed.

>> +	uint32_t count = lua_objlen(L, 1);
>> +	if (count == 0)
>> +		return lbox_push_tuple_format(L, tuple_format_runtime);
>> +	size_t size = count * sizeof(struct field_def);
>> +	struct region *region = &fiber()->gc;
>> +	size_t region_svp = region_used(region);
>> +	struct field_def *fields =
>> +		(struct field_def *)region_alloc(region, size);
>> +	if (fields == NULL) {
>> +		diag_set(OutOfMemory, size, "region_alloc", "fields");
>> +		return luaT_error(L);
>> +	}
>> +	for (uint32_t i = 0; i < count; ++i) {
>> +		size_t len;
>> +
>> +		fields[i] = field_def_default;
>> +
>> +		lua_pushinteger(L, i + 1);
>> +		lua_gettable(L, 1);
>> +
>> +		lua_pushstring(L, "type");
>> +		lua_gettable(L, -2);
>> +		if (! lua_isnil(L, -1)) {
>> +			const char *type_name = lua_tolstring(L, -1, &len);
>> +			fields[i].type = field_type_by_name(type_name, len);
>> +			if (fields[i].type == field_type_MAX) {
>> +				const char *err =
>> +					"field %d has unknown field type";
>> +				return luaL_error(L, tt_sprintf(err, i + 1));
>> +			}
>> +		}
>> +		lua_pop(L, 1);
>> +
>> +		lua_pushstring(L, "name");
>> +		lua_gettable(L, -2);
>> +		if (lua_isnil(L, -1)) {
>> +			return luaL_error(L, tt_sprintf("field %d name is not "\
>
> Useless backslash (\).
>
Fixed.

> Please write more tests:
>  - setting 'format' without 'type'
>  - passing something different from a table to the function
>  - passing the value returned by space:format() to this function
>  - setting is_nullable in the input table
Fixed.


Diff between versions:

From 9196d92c38772913e5ac0e078d6b9c1342e96bf3 Mon Sep 17 00:00:00 2001
Date: Wed, 19 Jun 2019 12:01:55 +0300
Subject: [PATCH] Review fix


diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index e62e2ba..a835d3d 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -42,7 +42,7 @@
 #include "box/lua/tuple.h"
 #include "mpstream.h"
 
-uint32_t CTID_STRUCT_TUPLE_FORMAT_PTR;
+static uint32_t CTID_STRUCT_TUPLE_FORMAT_PTR;
 
 /** {{{ Miscellaneous utils **/
 
@@ -120,18 +120,27 @@ lbox_select(lua_State *L)
 
 /* }}} */
 
-static int
-lbox_tuple_format_gc(struct lua_State *L)
+/** {{{ Utils to work with tuple_format. **/
+
+struct tuple_format *
+lbox_check_tuple_format(struct lua_State *L, int narg)
 {
 	uint32_t ctypeid;
 	struct tuple_format *format =
-		*(struct tuple_format **)luaL_checkcdata(L, 1, &ctypeid);
+		*(struct tuple_format **)luaL_checkcdata(L, narg, &ctypeid);
 	if (ctypeid != CTID_STRUCT_TUPLE_FORMAT_PTR) {
-		luaL_error(L, "Invalid argument: 'struct tuple_format *' "\
+		luaL_error(L, "Invalid argument: 'struct tuple_format *' "
 			   "expected, got %s)",
-			   lua_typename(L, lua_type(L, 1)));
+			   lua_typename(L, lua_type(L, narg)));
 	}
-	box_tuple_format_unref(format);
+	return format;
+}
+
+static int
+lbox_tuple_format_gc(struct lua_State *L)
+{
+	struct tuple_format *format =  lbox_check_tuple_format(L, 1);
+	tuple_format_unref(format);
 	return 0;
 }
 
@@ -141,17 +150,19 @@ lbox_push_tuple_format(struct lua_State *L, struct tuple_format *format)
 	struct tuple_format **ptr = (struct tuple_format **)
 		luaL_pushcdata(L, CTID_STRUCT_TUPLE_FORMAT_PTR);
 	*ptr = format;
-	box_tuple_format_ref(format);
+	tuple_format_ref(format);
 	lua_pushcfunction(L, lbox_tuple_format_gc);
 	luaL_setcdatagc(L, -2);
 	return 1;
 }
 
 static int
-lbox_format_new(struct lua_State *L)
+lbox_tuple_format_new(struct lua_State *L)
 {
 	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
-	if (lua_gettop(L) != 1 || ! lua_istable(L, 1))
+	if (lua_gettop(L) == 0)
+		return lbox_push_tuple_format(L, tuple_format_runtime);
+	if (lua_gettop(L) > 1 || ! lua_istable(L, 1))
 		return luaL_error(L, "Usage: box.internal.format_new(format)");
 	uint32_t count = lua_objlen(L, 1);
 	if (count == 0)
@@ -189,7 +200,7 @@ lbox_format_new(struct lua_State *L)
 		lua_pushstring(L, "name");
 		lua_gettable(L, -2);
 		if (lua_isnil(L, -1)) {
-			return luaL_error(L, tt_sprintf("field %d name is not "\
+			return luaL_error(L, tt_sprintf("field %d name is not "
 							"specified", i + 1));
 		}
 		const char *name = lua_tolstring(L, -1, &len);
@@ -216,12 +227,14 @@ lbox_format_new(struct lua_State *L)
 	return lbox_push_tuple_format(L, format);
 }
 
+/* }}} */
+
 void
 box_lua_misc_init(struct lua_State *L)
 {
 	static const struct luaL_Reg boxlib_internal[] = {
 		{"select", lbox_select},
-		{"format_new", lbox_format_new},
+		{"new_tuple_format", lbox_tuple_format_new},
 		{NULL, NULL}
 	};
 
diff --git a/src/box/lua/misc.h b/src/box/lua/misc.h
index dfedfe3..1c6f1f6 100644
--- a/src/box/lua/misc.h
+++ b/src/box/lua/misc.h
@@ -45,6 +45,9 @@ lbox_encode_tuple_on_gc(struct lua_State *L, int idx, size_t *p_len);
 void
 box_lua_misc_init(struct lua_State *L);
 
+struct tuple_format *
+lbox_check_tuple_format(struct lua_State *L, int narg);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/test/box/misc.result b/test/box/misc.result
index ce39bbb..5c42e33 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1274,6 +1274,7 @@ s:drop()
 --
 -- gh-2978: Function to parse space format.
 --
+-- Error: no field name:
 format = {}
 ---
 ...
@@ -1283,10 +1284,11 @@ format[1] = {}
 format[1].type = 'unsigned'
 ---
 ...
-box.internal.format_new(format)
+box.internal.new_tuple_format(format)
 ---
 - error: field 1 name is not specified
 ...
+-- Error: duplicate field name:
 format[1].name = 'aaa'
 ---
 ...
@@ -1299,10 +1301,11 @@ format[2].name = 'aaa'
 format[2].type = 'any'
 ---
 ...
-box.internal.format_new(format)
+box.internal.new_tuple_format(format)
 ---
 - error: Space field 'aaa' is duplicate
 ...
+-- Error: wrong field type:
 format[2].name = 'bbb'
 ---
 ...
@@ -1315,7 +1318,44 @@ format[3].name = 'ccc'
 format[3].type = 'something'
 ---
 ...
-box.internal.format_new(format)
+box.internal.new_tuple_format(format)
 ---
 - error: field 3 has unknown field type
 ...
+-- Error: too many arguments:
+box.internal.new_tuple_format(format, format)
+---
+- error: 'Usage: box.internal.format_new(format)'
+...
+-- Error: wrong argument:
+box.internal.new_tuple_format(1)
+---
+- error: 'Usage: box.internal.format_new(format)'
+...
+--
+-- In next tests we should receive cdata("struct tuple_format *").
+-- We do not have a way to check cdata in Lua, but there should be
+-- no errors.
+--
+-- Without argument it is equivalent to new_tuple_format({})
+tuple_format = box.internal.new_tuple_format()
+---
+...
+-- If no type that type == "any":
+format[3].type = nil
+---
+...
+tuple_format = box.internal.new_tuple_format(format)
+---
+...
+-- Function space:format() without arguments returns valid format:
+tuple_format = box.internal.new_tuple_format(box.space._space:format())
+---
+...
+-- Check is_nullable option fo field
+format[2].is_nullable = true
+---
+...
+tuple_format = box.internal.new_tuple_format(format)
+---
+...
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 2d13b99..94a6450 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -357,19 +357,49 @@ s:drop()
 --
 -- gh-2978: Function to parse space format.
 --
+
+-- Error: no field name:
 format = {}
 format[1] = {}
 format[1].type = 'unsigned'
-box.internal.format_new(format)
+box.internal.new_tuple_format(format)
 
+-- Error: duplicate field name:
 format[1].name = 'aaa'
 format[2] = {}
 format[2].name = 'aaa'
 format[2].type = 'any'
-box.internal.format_new(format)
+box.internal.new_tuple_format(format)
 
+-- Error: wrong field type:
 format[2].name = 'bbb'
 format[3] = {}
 format[3].name = 'ccc'
 format[3].type = 'something'
-box.internal.format_new(format)
+box.internal.new_tuple_format(format)
+
+-- Error: too many arguments:
+box.internal.new_tuple_format(format, format)
+
+-- Error: wrong argument:
+box.internal.new_tuple_format(1)
+
+--
+-- In next tests we should receive cdata("struct tuple_format *").
+-- We do not have a way to check cdata in Lua, but there should be
+-- no errors.
+--
+
+-- Without argument it is equivalent to new_tuple_format({})
+tuple_format = box.internal.new_tuple_format()
+
+-- If no type that type == "any":
+format[3].type = nil
+tuple_format = box.internal.new_tuple_format(format)
+
+-- Function space:format() without arguments returns valid format:
+tuple_format = box.internal.new_tuple_format(box.space._space:format())
+
+-- Check is_nullable option fo field
+format[2].is_nullable = true
+tuple_format = box.internal.new_tuple_format(format)



New version:

From a911d820ae46a4ac9151e649ee45f6df21e6120a Mon Sep 17 00:00:00 2001
Date: Tue, 11 Jun 2019 15:21:39 +0300
Subject: [PATCH] lua: internal function to parse space format

This patch defines a new function that parses space format and
returns it to Lua as cdata. For this cdata destructor is included.

Needed for #2978

diff --git a/src/box/lua/misc.cc b/src/box/lua/misc.cc
index 8de7401..a835d3d 100644
--- a/src/box/lua/misc.cc
+++ b/src/box/lua/misc.cc
@@ -37,9 +37,13 @@
 
 #include "box/box.h"
 #include "box/port.h"
+#include "box/tuple.h"
+#include "box/tuple_format.h"
 #include "box/lua/tuple.h"
 #include "mpstream.h"
 
+static uint32_t CTID_STRUCT_TUPLE_FORMAT_PTR;
+
 /** {{{ Miscellaneous utils **/
 
 char *
@@ -116,14 +120,130 @@ lbox_select(lua_State *L)
 
 /* }}} */
 
+/** {{{ Utils to work with tuple_format. **/
+
+struct tuple_format *
+lbox_check_tuple_format(struct lua_State *L, int narg)
+{
+	uint32_t ctypeid;
+	struct tuple_format *format =
+		*(struct tuple_format **)luaL_checkcdata(L, narg, &ctypeid);
+	if (ctypeid != CTID_STRUCT_TUPLE_FORMAT_PTR) {
+		luaL_error(L, "Invalid argument: 'struct tuple_format *' "
+			   "expected, got %s)",
+			   lua_typename(L, lua_type(L, narg)));
+	}
+	return format;
+}
+
+static int
+lbox_tuple_format_gc(struct lua_State *L)
+{
+	struct tuple_format *format =  lbox_check_tuple_format(L, 1);
+	tuple_format_unref(format);
+	return 0;
+}
+
+static int
+lbox_push_tuple_format(struct lua_State *L, struct tuple_format *format)
+{
+	struct tuple_format **ptr = (struct tuple_format **)
+		luaL_pushcdata(L, CTID_STRUCT_TUPLE_FORMAT_PTR);
+	*ptr = format;
+	tuple_format_ref(format);
+	lua_pushcfunction(L, lbox_tuple_format_gc);
+	luaL_setcdatagc(L, -2);
+	return 1;
+}
+
+static int
+lbox_tuple_format_new(struct lua_State *L)
+{
+	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
+	if (lua_gettop(L) == 0)
+		return lbox_push_tuple_format(L, tuple_format_runtime);
+	if (lua_gettop(L) > 1 || ! lua_istable(L, 1))
+		return luaL_error(L, "Usage: box.internal.format_new(format)");
+	uint32_t count = lua_objlen(L, 1);
+	if (count == 0)
+		return lbox_push_tuple_format(L, tuple_format_runtime);
+	size_t size = count * sizeof(struct field_def);
+	struct region *region = &fiber()->gc;
+	size_t region_svp = region_used(region);
+	struct field_def *fields =
+		(struct field_def *)region_alloc(region, size);
+	if (fields == NULL) {
+		diag_set(OutOfMemory, size, "region_alloc", "fields");
+		return luaT_error(L);
+	}
+	for (uint32_t i = 0; i < count; ++i) {
+		size_t len;
+
+		fields[i] = field_def_default;
+
+		lua_pushinteger(L, i + 1);
+		lua_gettable(L, 1);
+
+		lua_pushstring(L, "type");
+		lua_gettable(L, -2);
+		if (! lua_isnil(L, -1)) {
+			const char *type_name = lua_tolstring(L, -1, &len);
+			fields[i].type = field_type_by_name(type_name, len);
+			if (fields[i].type == field_type_MAX) {
+				const char *err =
+					"field %d has unknown field type";
+				return luaL_error(L, tt_sprintf(err, i + 1));
+			}
+		}
+		lua_pop(L, 1);
+
+		lua_pushstring(L, "name");
+		lua_gettable(L, -2);
+		if (lua_isnil(L, -1)) {
+			return luaL_error(L, tt_sprintf("field %d name is not "
+							"specified", i + 1));
+		}
+		const char *name = lua_tolstring(L, -1, &len);
+		fields[i].name = (char *)region_alloc(region, len + 1);
+		if (fields == NULL) {
+			diag_set(OutOfMemory, size, "region_alloc",
+				 "fields[i].name");
+			return luaT_error(L);
+		}
+		memcpy(fields[i].name, name, len);
+		fields[i].name[len] = '\0';
+		lua_pop(L, 1);
+		lua_pop(L, 1);
+	}
+	struct tuple_dictionary *dict = tuple_dictionary_new(fields, count);
+	region_truncate(region, region_svp);
+	if (dict == NULL)
+		return luaT_error(L);
+	struct tuple_format *format =
+		tuple_format_new(&tuple_format_runtime->vtab, NULL, NULL, 0,
+				 NULL, 0, 0, dict, false, false);
+	if (format == NULL)
+		return luaT_error(L);
+	return lbox_push_tuple_format(L, format);
+}
+
+/* }}} */
+
 void
 box_lua_misc_init(struct lua_State *L)
 {
 	static const struct luaL_Reg boxlib_internal[] = {
 		{"select", lbox_select},
+		{"new_tuple_format", lbox_tuple_format_new},
 		{NULL, NULL}
 	};
 
 	luaL_register(L, "box.internal", boxlib_internal);
 	lua_pop(L, 1);
+
+	int rc = luaL_cdef(L, "struct tuple_format;");
+	assert(rc == 0);
+	(void) rc;
+	CTID_STRUCT_TUPLE_FORMAT_PTR = luaL_ctypeid(L, "struct tuple_format *");
+	assert(CTID_STRUCT_TUPLE_FORMAT_PTR != 0);
 }
diff --git a/src/box/lua/misc.h b/src/box/lua/misc.h
index dfedfe3..1c6f1f6 100644
--- a/src/box/lua/misc.h
+++ b/src/box/lua/misc.h
@@ -45,6 +45,9 @@ lbox_encode_tuple_on_gc(struct lua_State *L, int idx, size_t *p_len);
 void
 box_lua_misc_init(struct lua_State *L);
 
+struct tuple_format *
+lbox_check_tuple_format(struct lua_State *L, int narg);
+
 #if defined(__cplusplus)
 } /* extern "C" */
 #endif /* defined(__cplusplus) */
diff --git a/test/box/misc.result b/test/box/misc.result
index 43b5a4a..5c42e33 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1271,3 +1271,91 @@ box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
 ---
 ...
+--
+-- gh-2978: Function to parse space format.
+--
+-- Error: no field name:
+format = {}
+---
+...
+format[1] = {}
+---
+...
+format[1].type = 'unsigned'
+---
+...
+box.internal.new_tuple_format(format)
+---
+- error: field 1 name is not specified
+...
+-- Error: duplicate field name:
+format[1].name = 'aaa'
+---
+...
+format[2] = {}
+---
+...
+format[2].name = 'aaa'
+---
+...
+format[2].type = 'any'
+---
+...
+box.internal.new_tuple_format(format)
+---
+- error: Space field 'aaa' is duplicate
+...
+-- Error: wrong field type:
+format[2].name = 'bbb'
+---
+...
+format[3] = {}
+---
+...
+format[3].name = 'ccc'
+---
+...
+format[3].type = 'something'
+---
+...
+box.internal.new_tuple_format(format)
+---
+- error: field 3 has unknown field type
+...
+-- Error: too many arguments:
+box.internal.new_tuple_format(format, format)
+---
+- error: 'Usage: box.internal.format_new(format)'
+...
+-- Error: wrong argument:
+box.internal.new_tuple_format(1)
+---
+- error: 'Usage: box.internal.format_new(format)'
+...
+--
+-- In next tests we should receive cdata("struct tuple_format *").
+-- We do not have a way to check cdata in Lua, but there should be
+-- no errors.
+--
+-- Without argument it is equivalent to new_tuple_format({})
+tuple_format = box.internal.new_tuple_format()
+---
+...
+-- If no type that type == "any":
+format[3].type = nil
+---
+...
+tuple_format = box.internal.new_tuple_format(format)
+---
+...
+-- Function space:format() without arguments returns valid format:
+tuple_format = box.internal.new_tuple_format(box.space._space:format())
+---
+...
+-- Check is_nullable option fo field
+format[2].is_nullable = true
+---
+...
+tuple_format = box.internal.new_tuple_format(format)
+---
+...
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 75d937e..94a6450 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -353,3 +353,53 @@ lsn == expected_lsn
 box.cfg{too_long_threshold = too_long_threshold}
 s:drop()
 
+
+--
+-- gh-2978: Function to parse space format.
+--
+
+-- Error: no field name:
+format = {}
+format[1] = {}
+format[1].type = 'unsigned'
+box.internal.new_tuple_format(format)
+
+-- Error: duplicate field name:
+format[1].name = 'aaa'
+format[2] = {}
+format[2].name = 'aaa'
+format[2].type = 'any'
+box.internal.new_tuple_format(format)
+
+-- Error: wrong field type:
+format[2].name = 'bbb'
+format[3] = {}
+format[3].name = 'ccc'
+format[3].type = 'something'
+box.internal.new_tuple_format(format)
+
+-- Error: too many arguments:
+box.internal.new_tuple_format(format, format)
+
+-- Error: wrong argument:
+box.internal.new_tuple_format(1)
+
+--
+-- In next tests we should receive cdata("struct tuple_format *").
+-- We do not have a way to check cdata in Lua, but there should be
+-- no errors.
+--
+
+-- Without argument it is equivalent to new_tuple_format({})
+tuple_format = box.internal.new_tuple_format()
+
+-- If no type that type == "any":
+format[3].type = nil
+tuple_format = box.internal.new_tuple_format(format)
+
+-- Function space:format() without arguments returns valid format:
+tuple_format = box.internal.new_tuple_format(box.space._space:format())
+
+-- Check is_nullable option fo field
+format[2].is_nullable = true
+tuple_format = box.internal.new_tuple_format(format)

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2019-06-21 20:39 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-21 15:25 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
2019-06-21 15:25 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
2019-06-21 20:39   ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-21 15:25 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma
2019-06-21 20:39   ` [tarantool-patches] " Vladislav Shpilevoy
2019-06-21 15:25 ` [PATCH v2 3/3] netbox: define formats for tuple from netbox imeevma
2019-06-21 20:39   ` [tarantool-patches] " Vladislav Shpilevoy
  -- strict thread matches above, loose matches on Subject: below --
2019-06-19 10:34 [PATCH v2 0/3] " imeevma
2019-06-19 10:34 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox