* [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