Tarantool development patches archive
 help / color / mirror / Atom feed
* [PATCH v2 0/3] netbox: define formats for tuple from netbox
@ 2019-06-19 10:34 imeevma
  2019-06-19 10:34 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: imeevma @ 2019-06-19 10:34 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: 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] 6+ messages in thread

* [PATCH v2 1/3] netbox: store method_encoder args in request
  2019-06-19 10:34 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
@ 2019-06-19 10:34 ` imeevma
  2019-06-19 10:34 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: imeevma @ 2019-06-19 10:34 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: 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] 6+ messages in thread

* [PATCH v2 2/3] lua: internal function to parse space format
  2019-06-19 10:34 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
  2019-06-19 10:34 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
@ 2019-06-19 10:34 ` imeevma
  2019-06-19 10:34 ` [PATCH v2 3/3] netbox: define formats for tuple from netbox imeevma
  2019-06-21 14:27 ` [PATCH v2 0/3] " Vladimir Davydov
  3 siblings, 0 replies; 6+ 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] 6+ messages in thread

* [PATCH v2 3/3] netbox: define formats for tuple from netbox
  2019-06-19 10:34 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
  2019-06-19 10:34 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
  2019-06-19 10:34 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma
@ 2019-06-19 10:34 ` imeevma
  2019-06-21 14:27 ` [PATCH v2 0/3] " Vladimir Davydov
  3 siblings, 0 replies; 6+ messages in thread
From: imeevma @ 2019-06-19 10:34 UTC (permalink / raw)
  To: vdavydov.dev; +Cc: tarantool-patches

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

On 6/18/19 12:00 PM, Vladimir Davydov wrote:
> On Fri, Jun 14, 2019 at 03:29:21PM +0300, Mergen Imeev wrote:
>> Second new patch:
>>
>> From f9e959e3f0f38e8f6b8f1294ba29c28d41f30968 Mon Sep 17 00:00:00 2001
>> Date: Tue, 11 Jun 2019 16:36:39 +0300
>> Subject: [PATCH] netbox: define formats for tuple from netbox
>>
>> 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
>> ...
>>
>> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
>> index 7484a86..946d397 100644
>> --- a/src/box/lua/net_box.c
>> +++ b/src/box/lua/net_box.c
>> @@ -590,12 +590,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 +617,15 @@ 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 = *(struct tuple_format **)luaL_checkcdata(L, 2,
>> +								  &ctypeid);
>
> I think we should use lbox_check_tuple_format helper from the previous
> patch here.
>
Done.

> Other than that, this patch looks good to me.
>
>> +	} 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);


Diff:

From 31ccda48f4f01b89ab9d2818e79cba38699d3386 Mon Sep 17 00:00:00 2001
Date: Wed, 19 Jun 2019 12:51:37 +0300
Subject: [PATCH] Review fix


diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 946d397..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
 
@@ -620,12 +621,10 @@ netbox_decode_select(struct lua_State *L)
 	int top = lua_gettop(L);
 	assert(top == 1 || top == 2);
 	struct tuple_format *format;
-	if (top == 2 && lua_type(L, 2) == LUA_TCDATA) {
-		format = *(struct tuple_format **)luaL_checkcdata(L, 2,
-								  &ctypeid);
-	} else {
+	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);
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 30cf227..8f31712 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -1274,7 +1274,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
         s.index = {}
         s.temporary = false
         s._format = format
-        s._format_cdata = box.internal.format_new(format)
+        s._format_cdata = box.internal.new_tuple_format(format)
         s.connection = self
         if #space > 5 then
             local opts = space[6]



New version:

From 83981bbb325345e0115941f587a8c31050e94ebb Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Tue, 11 Jun 2019 16:36:39 +0300
Subject: [PATCH] netbox: define formats for tuple from netbox

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
...

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.
 --

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

* Re: [PATCH v2 0/3] netbox: define formats for tuple from netbox
  2019-06-19 10:34 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
                   ` (2 preceding siblings ...)
  2019-06-19 10:34 ` [PATCH v2 3/3] netbox: define formats for tuple from netbox imeevma
@ 2019-06-21 14:27 ` Vladimir Davydov
  3 siblings, 0 replies; 6+ messages in thread
From: Vladimir Davydov @ 2019-06-21 14:27 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

On Wed, Jun 19, 2019 at 01:34:20PM +0300, imeevma@tarantool.org wrote:
> Mergen Imeev (3):
>   netbox: store method_encoder args in request
>   lua: internal function to parse space format
>   netbox: define formats for tuple from netbox

Pushed to master, thanks!

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

* [PATCH v2 0/3] netbox: define formats for tuple from netbox
@ 2019-06-21 15:25 imeevma
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

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

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-19 10:34 [PATCH v2 0/3] netbox: define formats for tuple from netbox imeevma
2019-06-19 10:34 ` [PATCH v2 1/3] netbox: store method_encoder args in request imeevma
2019-06-19 10:34 ` [PATCH v2 2/3] lua: internal function to parse space format imeevma
2019-06-19 10:34 ` [PATCH v2 3/3] netbox: define formats for tuple from netbox imeevma
2019-06-21 14:27 ` [PATCH v2 0/3] " Vladimir Davydov
2019-06-21 15:25 imeevma

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