* [PATCH v1 0/2] netbox: define formats for tuple from netbox
@ 2019-06-10 10:02 imeevma
2019-06-10 10:02 ` [PATCH v1 1/2] netbox: store method_encoder args in request imeevma
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: imeevma @ 2019-06-10 10:02 UTC (permalink / raw)
To: vdavydov.dev; +Cc: tarantool-patches
This patch defines field names for the tuples obtained through
the netbox.
Mergen Imeev (2):
netbox: store method_encoder args in request
netbox: define formats for tuple from netbox
src/box/lua/net_box.c | 87 ++++++++++++++++-
src/box/lua/net_box.lua | 232 +++++++++++++++++++++++++++++++++-------------
test/box/access.result | 15 ++-
test/box/access.test.lua | 9 +-
test/box/net.box.result | 83 ++++++++++++++++-
test/box/net.box.test.lua | 26 +++++-
6 files changed, 369 insertions(+), 83 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v1 1/2] netbox: store method_encoder args in request
2019-06-10 10:02 [PATCH v1 0/2] netbox: define formats for tuple from netbox imeevma
@ 2019-06-10 10:02 ` imeevma
2019-06-11 8:15 ` Vladimir Davydov
2019-06-10 10:02 ` [PATCH v1 2/2] netbox: define formats for tuple from netbox imeevma
2019-06-11 8:55 ` [PATCH v1 0/2] " Vladimir Davydov
2 siblings, 1 reply; 12+ messages in thread
From: imeevma @ 2019-06-10 10:02 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 | 179 +++++++++++++++++++++++++++++++---------------
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, 146 insertions(+), 69 deletions(-)
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 251ad40..8d42fb4 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,70 @@ local function decode_push(raw_data)
return response[IPROTO_DATA_KEY][1], raw_end
end
+local function encode_call(send_buf, id, method_args)
+ return internal.encode_call(send_buf, id, method_args.func_name,
+ method_args.args)
+end
+local function encode_call_16(send_buf, id, method_args)
+ return internal.encode_call_16(send_buf, id, method_args.func_name,
+ method_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, method_args)
+ return internal.encode_eval(send_buf, id, method_args.code,
+ method_args.args)
+end
+local function encode_insert(send_buf, id, method_args)
+ return internal.encode_insert(send_buf, id, method_args.space_id,
+ method_args.tuple)
+end
+local function encode_replace(send_buf, id, method_args)
+ return internal.encode_replace(send_buf, id, method_args.space_id,
+ method_args.tuple)
+end
+local function encode_delete(send_buf, id, method_args)
+ return internal.encode_delete(send_buf, id, method_args.space_id,
+ method_args.index_id, method_args.key)
+end
+local function encode_update(send_buf, id, method_args)
+ return internal.encode_update(send_buf, id, method_args.space_id,
+ method_args.index_id, method_args.key,
+ method_args.oplist)
+end
+local function encode_upsert(send_buf, id, method_args)
+ return internal.encode_upsert(send_buf, id, method_args.space_id,
+ method_args.key, method_args.oplist)
+end
+local function encode_select(send_buf, id, method_args)
+ return internal.encode_select(send_buf, id, method_args.space_id,
+ method_args.index_id, method_args.iterator,
+ method_args.offset, method_args.limit,
+ method_args.key)
+end
+local function encode_execute(send_buf, id, method_args)
+ return internal.encode_execute(send_buf, id, method_args.query,
+ method_args.parameters, method_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 +533,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, method_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 +544,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, method_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 method_args.
+ local request = setmetatable(table_new(0, 10), request_mt)
request.method = method
request.buffer = buffer
request.skip_header = skip_header
@@ -511,6 +558,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.method_args = method_args
return request
end
@@ -520,10 +568,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, method_args)
local request, err =
perform_async_request(buffer, skip_header, method, on_push,
- on_push_ctx, ...)
+ on_push_ctx, method_args)
if not request then
return nil, err
end
@@ -739,13 +787,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 +1113,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, method_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 +1127,7 @@ function remote_methods:_request(method, opts, ...)
end
local res, err =
transport.perform_async_request(buffer, skip_header, method,
- table.insert, {}, ...)
+ table.insert, {}, method_args)
if err then
box.error(err)
end
@@ -1106,7 +1155,7 @@ function remote_methods:_request(method, opts, ...)
end
local res, err = transport.perform_request(timeout, buffer, skip_header,
method, on_push, on_push_ctx,
- ...)
+ method_args)
if err then
box.error(err)
end
@@ -1133,14 +1182,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 method_args = {func_name=tostring(func_name), args={...}}
+ return (self:_request('call_16', nil, method_args))
end
function remote_methods:call(func_name, 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)
+ local method_args = {func_name=tostring(func_name), args=args}
+ local res = self:_request('call_17', opts, method_args)
if type(res) ~= 'table' or opts and opts.is_async then
return res
end
@@ -1150,14 +1201,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 method_args = {code=code, args={...}}
+ return unpack((self:_request('eval', nil, method_args)))
end
function remote_methods:eval(code, args, opts)
check_remote_arg(self, 'eval')
check_eval_args(args)
args = args or {}
- local res = self:_request('eval', opts, code, args)
+ local method_args = {code=code, args=args}
+ local res = self:_request('eval', opts, method_args)
if type(res) ~= 'table' or opts and opts.is_async then
return res
end
@@ -1169,8 +1222,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 method_args = {query=query, parameters=parameters or {},
+ sql_opts=sql_opts or {}}
+ return self:_request('execute', netbox_opts, method_args)
end
function remote_methods:wait_state(state, timeout)
@@ -1314,7 +1368,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 method_args = {code=loader, args={line}}
+ res, err = pr(timeout, nil, false, 'eval', nil, nil, method_args)
else
assert(self.protocol == 'Lua console')
res, err = pr(timeout, nil, false, 'inject', nil, nil, line..'$EOF$\n')
@@ -1336,33 +1391,38 @@ space_metatable = function(remote)
function methods:insert(tuple, opts)
check_space_arg(self, 'insert')
- return remote:_request('insert', opts, self.id, tuple)
+ local method_args = {space_id=self.id, tuple=tuple}
+ return remote:_request('insert', opts, method_args)
end
function methods:replace(tuple, opts)
check_space_arg(self, 'replace')
- return remote:_request('replace', opts, self.id, tuple)
+ local method_args = {space_id=self.id, tuple=tuple}
+ return remote:_request('replace', opts, method_args)
end
function methods:select(key, opts)
check_space_arg(self, 'select')
+ local method_args = {key=key, opts=opts}
return check_primary_index(self):select(key, opts)
end
function methods:delete(key, opts)
check_space_arg(self, 'delete')
+ local method_args = {key=key, opts=opts}
return check_primary_index(self):delete(key, opts)
end
function methods:update(key, oplist, opts)
check_space_arg(self, 'update')
+ local method_args = {oplist=oplist, opts=opts}
return check_primary_index(self):update(key, oplist, opts)
end
function methods:upsert(key, oplist, opts)
check_space_arg(self, 'upsert')
- return nothing_or_data(remote:_request('upsert', opts, self.id, key,
- oplist))
+ local method_args = {space_id=self.id, key=key, oplist=oplist}
+ return nothing_or_data(remote:_request('upsert', opts, method_args))
end
function methods:get(key, opts)
@@ -1391,8 +1451,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 method_args = {space_id=self.space.id, index_id=self.id,
+ iterator=iterator, offset=offset, limit=limit,
+ key=key}
+ return (remote:_request('select', opts, method_args))
end
function methods:get(key, opts)
@@ -1400,8 +1462,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 method_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, method_args))
end
function methods:min(key, opts)
@@ -1409,9 +1472,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 method_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, method_args))
end
function methods:max(key, opts)
@@ -1419,9 +1482,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 method_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, method_args))
end
function methods:count(key, opts)
@@ -1431,19 +1494,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 method_args = {func_name=code, args={ key, opts }}
+ return remote:_request('count', opts, method_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 method_args = {space_id=self.space.id, index_id=self.id, key=key}
+ return nothing_or_data(remote:_request('delete', opts, method_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 method_args = {space_id=self.space.id, index_id=self.id, key=key,
+ oplist=oplist}
+ return nothing_or_data(remote:_request('update', opts, method_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 474297a..451c31d 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 bea4300..6651b58 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] 12+ messages in thread
* [PATCH v1 2/2] netbox: define formats for tuple from netbox
2019-06-10 10:02 [PATCH v1 0/2] netbox: define formats for tuple from netbox imeevma
2019-06-10 10:02 ` [PATCH v1 1/2] netbox: store method_encoder args in request imeevma
@ 2019-06-10 10:02 ` imeevma
2019-06-11 8:55 ` Vladimir Davydov
2019-06-11 8:55 ` [PATCH v1 0/2] " Vladimir Davydov
2 siblings, 1 reply; 12+ messages in thread
From: imeevma @ 2019-06-10 10:02 UTC (permalink / raw)
To: vdavydov.dev; +Cc: tarantool-patches
This patch creates tuple_formats for the tuples obtained through
the netbox.
Closes #2978
---
src/box/lua/net_box.c | 87 ++++++++++++++++++++++++++++++++++++++++++++---
src/box/lua/net_box.lua | 69 ++++++++++++++++++++++++++++---------
test/box/net.box.result | 77 +++++++++++++++++++++++++++++++++++++++++
test/box/net.box.test.lua | 20 +++++++++++
4 files changed, 231 insertions(+), 22 deletions(-)
diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index 7484a86..fab4a8b 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -40,6 +40,7 @@
#include "box/xrow.h"
#include "box/tuple.h"
#include "box/execute.h"
+#include "box/schema.h"
#include "lua/msgpack.h"
#include "third_party/base64.h"
@@ -590,12 +591,12 @@ 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, uint32_t format_id)
{
+ (void)format_id;
uint32_t count = mp_decode_array(data);
lua_createtable(L, count, 0);
- struct tuple_format *format =
- box_tuple_format_default();
+ struct tuple_format *format = tuple_format_by_id(format_id);
for (uint32_t j = 0; j < count; ++j) {
const char *begin = *data;
mp_next(data);
@@ -608,6 +609,74 @@ netbox_decode_data(struct lua_State *L, const char **data)
}
}
+static int
+netbox_format_new(struct lua_State *L)
+{
+ if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1)
+ return luaL_error(L, "Bad params!");
+
+ uint32_t count = lua_objlen(L, 1);
+ if (count == 0) {
+ lua_pushinteger(L, box_tuple_format_default()->id);
+ return 1;
+ }
+ size_t size = count * sizeof(struct field_def);
+ struct region *region = &fiber()->gc;
+ size_t region_svp = region_used(region);
+ struct field_def *fields = region_alloc(region, size);
+ if (fields == NULL) {
+ diag_set(OutOfMemory, size, "region_alloc", "fields");
+ return luaT_error(L);
+ }
+ memset(fields, 0, size);
+ for (uint32_t i = 0; i < count; ++i) {
+ lua_pushinteger(L, i + 1);
+ lua_gettable(L, 1);
+
+ lua_pushstring(L, "type");
+ lua_gettable(L, -2);
+ size_t len;
+ const char *type_name = lua_tolstring(L, -1, &len);
+ lua_pop(L, 1);
+ fields[i].type = field_type_by_name(type_name, len);
+
+ lua_pushstring(L, "name");
+ lua_gettable(L, -2);
+ const char *name = lua_tolstring(L, -1, &len);
+ fields[i].name = region_alloc(region, len + 1);
+ 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);
+ if (dict == NULL) {
+ region_truncate(region, region_svp);
+ return luaT_error(L);
+ }
+
+ struct tuple_format *format =
+ tuple_format_new(&box_tuple_format_default()->vtab, NULL, NULL,
+ 0, fields, count, 0, dict, false, false);
+ assert(format != NULL);
+ tuple_format_ref(format);
+ lua_pushinteger(L, format->id);
+ region_truncate(region, region_svp);
+ return 1;
+}
+
+static int
+netbox_format_delete(struct lua_State *L)
+{
+ int32_t format_id = luaL_checkinteger(L, 1);
+ if (format_id == box_tuple_format_default()->id)
+ return 0;
+ struct tuple_format *format = tuple_format_by_id(format_id);
+ assert(format != NULL);
+ tuple_format_unref(format);
+ return 0;
+}
+
/**
* Decode Tarantool response body consisting of single
* IPROTO_DATA key into array of tuples.
@@ -619,6 +688,11 @@ netbox_decode_select(struct lua_State *L)
{
uint32_t ctypeid;
const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
+ uint32_t format_id;
+ if (lua_gettop(L) == 2)
+ format_id = luaL_checkinteger(L, 2);
+ else
+ format_id = box_tuple_format_default()->id;
assert(mp_typeof(*data) == MP_MAP);
uint32_t map_size = mp_decode_map(&data);
/* Until 2.0 body has no keys except DATA. */
@@ -627,7 +701,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_id);
*(const char **)luaL_pushcdata(L, ctypeid) = data;
return 2;
}
@@ -716,7 +790,8 @@ 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,
+ box_tuple_format_default()->id);
rows_index = i - map_size;
break;
case IPROTO_METADATA:
@@ -766,6 +841,8 @@ luaopen_net_box(struct lua_State *L)
{ "communicate", netbox_communicate },
{ "decode_select", netbox_decode_select },
{ "decode_execute", netbox_decode_execute },
+ { "_format_new", netbox_format_new },
+ { "_format_delete", netbox_format_delete },
{ NULL, NULL}
};
/* luaL_register_module polutes _G */
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 8d42fb4..26ff7ff 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -63,12 +63,22 @@ 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, opts)
+ local response, raw_end
+ if opts ~= nil and opts.format_id ~= nil then
+ response, raw_end = internal.decode_select(raw_data, opts.format_id)
+ else
+ response, raw_end = internal.decode_select(raw_data)
+ end
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, opts)
+ local body, raw_end
+ if opts ~= nil and opts.format_id ~= nil then
+ body, raw_end = internal.decode_select(raw_data, opts.format_id)
+ else
+ body, raw_end = internal.decode_select(raw_data)
+ end
if body[2] then
return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
end
@@ -82,6 +92,15 @@ 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_select(raw_data, raw_data_end, opts)
+ if opts ~= nil and opts.format_id ~= nil then
+ return internal.decode_select(raw_data, opts.format_id)
+ end
+ return internal.decode_select(raw_data)
+end
+local function decode_execute(raw_data, raw_data_end)
+ return internal.decode_execute(raw_data)
+end
local function encode_call(send_buf, id, method_args)
return internal.encode_call(send_buf, id, method_args.func_name,
@@ -157,7 +176,7 @@ local method_encoder = {
local method_decoder = {
ping = decode_nil,
- call_16 = internal.decode_select,
+ call_16 = decode_select,
call_17 = decode_data,
eval = decode_data,
insert = decode_tuple,
@@ -165,8 +184,8 @@ local method_decoder = {
delete = decode_tuple,
update = decode_tuple,
upsert = decode_nil,
- select = internal.decode_select,
- execute = internal.decode_execute,
+ select = decode_select,
+ execute = decode_execute,
get = decode_get,
min = decode_get,
max = decode_get,
@@ -630,14 +649,15 @@ 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.method_args)
assert(real_end == body_end, "invalid body length")
requests[id] = nil
request.id = nil
else
local msg
msg, real_end, request.errno =
- method_decoder.push(body_rpos, body_end)
+ method_decoder.push(body_rpos, body_end, request.method_args)
assert(real_end == body_end, "invalid body length")
request.on_push(request.on_push_ctx, msg)
end
@@ -1085,6 +1105,14 @@ end
function remote_methods:close()
check_remote_arg(self, 'close')
+ if (self.space ~= nil and type(self.space) == 'table') then
+ for _,space in pairs(self.space) do
+ if space.format_id ~= nil then
+ internal._format_delete(space._format_id)
+ space.format_id = nil
+ end
+ end
+ end
self._transport.stop()
end
@@ -1274,6 +1302,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
s.index = {}
s.temporary = false
s._format = format
+ s._format_id = internal._format_new(format)
s.connection = self
if #space > 5 then
local opts = space[6]
@@ -1391,13 +1420,15 @@ space_metatable = function(remote)
function methods:insert(tuple, opts)
check_space_arg(self, 'insert')
- local method_args = {space_id=self.id, tuple=tuple}
+ local method_args = {space_id=self.id, tuple=tuple,
+ format_id=self._format_id}
return remote:_request('insert', opts, method_args)
end
function methods:replace(tuple, opts)
check_space_arg(self, 'replace')
- local method_args = {space_id=self.id, tuple=tuple}
+ local method_args = {space_id=self.id, tuple=tuple,
+ format_id=self._format_id}
return remote:_request('replace', opts, method_args)
end
@@ -1453,7 +1484,7 @@ index_metatable = function(remote)
local limit = tonumber(opts and opts.limit) or 0xFFFFFFFF
local method_args = {space_id=self.space.id, index_id=self.id,
iterator=iterator, offset=offset, limit=limit,
- key=key}
+ key=key, format_id=self.space._format_id}
return (remote:_request('select', opts, method_args))
end
@@ -1463,7 +1494,8 @@ index_metatable = function(remote)
error("index:get() doesn't support `buffer` argument")
end
local method_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_id=self.space._format_id}
return nothing_or_data(remote:_request('get', opts, method_args))
end
@@ -1473,7 +1505,8 @@ index_metatable = function(remote)
error("index:min() doesn't support `buffer` argument")
end
local method_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_id=self.space._format_id}
return nothing_or_data(remote:_request('min', opts, method_args))
end
@@ -1483,7 +1516,8 @@ index_metatable = function(remote)
error("index:max() doesn't support `buffer` argument")
end
local method_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_id=self.space._format_id}
return nothing_or_data(remote:_request('max', opts, method_args))
end
@@ -1500,14 +1534,15 @@ index_metatable = function(remote)
function methods:delete(key, opts)
check_index_arg(self, 'delete')
- local method_args = {space_id=self.space.id, index_id=self.id, key=key}
+ local method_args = {space_id=self.space.id, index_id=self.id, key=key,
+ format_id=self.space._format_id}
return nothing_or_data(remote:_request('delete', opts, method_args))
end
function methods:update(key, oplist, opts)
check_index_arg(self, 'update')
local method_args = {space_id=self.space.id, index_id=self.id, key=key,
- oplist=oplist}
+ oplist=oplist, format_id=self.space._format_id}
return nothing_or_data(remote:_request('update', opts, method_args))
end
diff --git a/test/box/net.box.result b/test/box/net.box.result
index 451c31d..da40a3d 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3572,6 +3572,83 @@ 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', 'universe')
+---
+...
+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
+...
+cn:close()
+---
+...
+box.space.named:drop()
+---
+...
+box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+---
+...
+--
-- 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 6651b58..bba4eb5 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1433,6 +1433,26 @@ 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', 'universe')
+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()
+cn:close()
+box.space.named:drop()
+box.schema.user.revoke('guest', 'read, write, execute', 'universe')
+
+--
-- gh-3400: long-poll input discard must not touch event loop of
-- a closed connection.
--
--
2.7.4
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] netbox: store method_encoder args in request
2019-06-10 10:02 ` [PATCH v1 1/2] netbox: store method_encoder args in request imeevma
@ 2019-06-11 8:15 ` Vladimir Davydov
2019-06-14 11:50 ` Mergen Imeev
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Davydov @ 2019-06-11 8:15 UTC (permalink / raw)
To: imeevma; +Cc: tarantool-patches
On Mon, Jun 10, 2019 at 01:02:23PM +0300, imeevma@tarantool.org wrote:
> This patch changes the way arguments are passed to functions in
> the array method_encoder, and saves these arguments in REQUEST.
> This is necessary to establish a connection between netbox space
> objects and the tuples obtained through the netbox
>
> Needed for #2978
> ---
> src/box/lua/net_box.lua | 179 +++++++++++++++++++++++++++++++---------------
> 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, 146 insertions(+), 69 deletions(-)
>
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index 251ad40..8d42fb4 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,70 @@ local function decode_push(raw_data)
> return response[IPROTO_DATA_KEY][1], raw_end
> end
>
> +local function encode_call(send_buf, id, method_args)
> + return internal.encode_call(send_buf, id, method_args.func_name,
> + method_args.args)
> +end
> +local function encode_call_16(send_buf, id, method_args)
> + return internal.encode_call_16(send_buf, id, method_args.func_name,
> + method_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, method_args)
> + return internal.encode_eval(send_buf, id, method_args.code,
> + method_args.args)
> +end
> +local function encode_insert(send_buf, id, method_args)
> + return internal.encode_insert(send_buf, id, method_args.space_id,
> + method_args.tuple)
> +end
> +local function encode_replace(send_buf, id, method_args)
> + return internal.encode_replace(send_buf, id, method_args.space_id,
> + method_args.tuple)
> +end
> +local function encode_delete(send_buf, id, method_args)
> + return internal.encode_delete(send_buf, id, method_args.space_id,
> + method_args.index_id, method_args.key)
> +end
> +local function encode_update(send_buf, id, method_args)
> + return internal.encode_update(send_buf, id, method_args.space_id,
> + method_args.index_id, method_args.key,
> + method_args.oplist)
> +end
> +local function encode_upsert(send_buf, id, method_args)
> + return internal.encode_upsert(send_buf, id, method_args.space_id,
> + method_args.key, method_args.oplist)
> +end
> +local function encode_select(send_buf, id, method_args)
> + return internal.encode_select(send_buf, id, method_args.space_id,
> + method_args.index_id, method_args.iterator,
> + method_args.offset, method_args.limit,
> + method_args.key)
> +end
> +local function encode_execute(send_buf, id, method_args)
> + return internal.encode_execute(send_buf, id, method_args.query,
> + method_args.parameters, method_args.sql_opts)
> +end
Why not call it simply 'args'?
> @@ -1150,14 +1201,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 method_args = {code=code, args={...}}
> + return unpack((self:_request('eval', nil, method_args)))
> end
>
> function remote_methods:eval(code, args, opts)
> check_remote_arg(self, 'eval')
> check_eval_args(args)
> args = args or {}
> - local res = self:_request('eval', opts, code, args)
> + local method_args = {code=code, args=args}
Coding style: we add a space before and after '=':
local method_args = {code = code, args = args}
Please fix here and everywhere else.
Other than that this patch looks okay to me.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] netbox: define formats for tuple from netbox
2019-06-10 10:02 ` [PATCH v1 2/2] netbox: define formats for tuple from netbox imeevma
@ 2019-06-11 8:55 ` Vladimir Davydov
2019-06-14 12:29 ` Mergen Imeev
0 siblings, 1 reply; 12+ messages in thread
From: Vladimir Davydov @ 2019-06-11 8:55 UTC (permalink / raw)
To: imeevma; +Cc: tarantool-patches
On Mon, Jun 10, 2019 at 01:02:24PM +0300, imeevma@tarantool.org wrote:
> This patch creates tuple_formats for the tuples obtained through
> the netbox.
>
> Closes #2978
Please write a docbot request.
> ---
> src/box/lua/net_box.c | 87 ++++++++++++++++++++++++++++++++++++++++++++---
> src/box/lua/net_box.lua | 69 ++++++++++++++++++++++++++++---------
> test/box/net.box.result | 77 +++++++++++++++++++++++++++++++++++++++++
> test/box/net.box.test.lua | 20 +++++++++++
> 4 files changed, 231 insertions(+), 22 deletions(-)
>
> diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
> index 7484a86..fab4a8b 100644
> --- a/src/box/lua/net_box.c
> +++ b/src/box/lua/net_box.c
> @@ -40,6 +40,7 @@
> #include "box/xrow.h"
> #include "box/tuple.h"
> #include "box/execute.h"
> +#include "box/schema.h"
>
> #include "lua/msgpack.h"
> #include "third_party/base64.h"
> @@ -590,12 +591,12 @@ 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, uint32_t format_id)
I'd pass a pointer to tuple_format struct instead of format_id to
this function.
> {
> + (void)format_id;
This is clearly useless.
> uint32_t count = mp_decode_array(data);
> lua_createtable(L, count, 0);
> - struct tuple_format *format =
> - box_tuple_format_default();
> + struct tuple_format *format = tuple_format_by_id(format_id);
> for (uint32_t j = 0; j < count; ++j) {
> const char *begin = *data;
> mp_next(data);
> @@ -608,6 +609,74 @@ netbox_decode_data(struct lua_State *L, const char **data)
> }
> }
>
> +static int
> +netbox_format_new(struct lua_State *L)
> +{
> + if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1)
> + return luaL_error(L, "Bad params!");
Please print a "Usage: ..." message, like other functions do.
> +
> + uint32_t count = lua_objlen(L, 1);
> + if (count == 0) {
> + lua_pushinteger(L, box_tuple_format_default()->id);
Please use tuple_format_runtime instead of box_tuple_format_default(),
here and everywhere else. The latter is, after all, for modules.
> + return 1;
> + }
> + size_t size = count * sizeof(struct field_def);
> + struct region *region = &fiber()->gc;
> + size_t region_svp = region_used(region);
> + struct field_def *fields = region_alloc(region, size);
> + if (fields == NULL) {
> + diag_set(OutOfMemory, size, "region_alloc", "fields");
> + return luaT_error(L);
> + }
> + memset(fields, 0, size);
Please init each field with field_def_default in the loop below
instead of zeroing it.
> + for (uint32_t i = 0; i < count; ++i) {
> + lua_pushinteger(L, i + 1);
> + lua_gettable(L, 1);
> +
> + lua_pushstring(L, "type");
> + lua_gettable(L, -2);
> + size_t len;
> + const char *type_name = lua_tolstring(L, -1, &len);
> + lua_pop(L, 1);
> + fields[i].type = field_type_by_name(type_name, len);
> +
> + lua_pushstring(L, "name");
> + lua_gettable(L, -2);
> + const char *name = lua_tolstring(L, -1, &len);
Strictly speaking, the name or type can be absent. The type can also be
invalid. We should handle those situations gracefully.
> + fields[i].name = region_alloc(region, len + 1);
> + 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);
> + if (dict == NULL) {
> + region_truncate(region, region_svp);
> + return luaT_error(L);
> + }
> +
> + struct tuple_format *format =
> + tuple_format_new(&box_tuple_format_default()->vtab, NULL, NULL,
> + 0, fields, count, 0, dict, false, false);
Do we need really need to pass fields to this function? AFAIU tuple
dictionary would be enough since we only need names. What happens if
a field is nullable?
> + assert(format != NULL);
Strictly speaking tuple_format_new() may fail with OOM.
Please handle it.
> + tuple_format_ref(format);
> + lua_pushinteger(L, format->id);
Returning a cdata with gc set would look robuster to me - with an id
it's pretty easy to leak an object.
> + region_truncate(region, region_svp);
> + return 1;
> +}
> +
> +static int
> +netbox_format_delete(struct lua_State *L)
> +{
> + int32_t format_id = luaL_checkinteger(L, 1);
> + if (format_id == box_tuple_format_default()->id)
> + return 0;
> + struct tuple_format *format = tuple_format_by_id(format_id);
> + assert(format != NULL);
> + tuple_format_unref(format);
> + return 0;
> +}
> +
> /**
> * Decode Tarantool response body consisting of single
> * IPROTO_DATA key into array of tuples.
> @@ -619,6 +688,11 @@ netbox_decode_select(struct lua_State *L)
> {
> uint32_t ctypeid;
> const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
> + uint32_t format_id;
> + if (lua_gettop(L) == 2)
> + format_id = luaL_checkinteger(L, 2);
> + else
> + format_id = box_tuple_format_default()->id;
> assert(mp_typeof(*data) == MP_MAP);
> uint32_t map_size = mp_decode_map(&data);
> /* Until 2.0 body has no keys except DATA. */
> @@ -627,7 +701,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_id);
> *(const char **)luaL_pushcdata(L, ctypeid) = data;
> return 2;
> }
> @@ -716,7 +790,8 @@ 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,
> + box_tuple_format_default()->id);
> rows_index = i - map_size;
> break;
> case IPROTO_METADATA:
> @@ -766,6 +841,8 @@ luaopen_net_box(struct lua_State *L)
> { "communicate", netbox_communicate },
> { "decode_select", netbox_decode_select },
> { "decode_execute", netbox_decode_execute },
> + { "_format_new", netbox_format_new },
> + { "_format_delete", netbox_format_delete },
I think we better place these helpers in src/box/lua/misc.cc - we might
need them somewhere else. Also, this is 'internal' namespace so no need
to prefix function names with an underscore.
Actually, I think we should add these functions in a separate patch and
cover them with some basic tests involving argument checking (like
passing an invalid field type or a field without a name).
> { NULL, NULL}
> };
> /* luaL_register_module polutes _G */
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index 8d42fb4..26ff7ff 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -63,12 +63,22 @@ 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, opts)
You use name 'args' when passing 'opts' to the encoder. Let's use name
'args' everywhere to avoid confusion.
> + local response, raw_end
> + if opts ~= nil and opts.format_id ~= nil then
> + response, raw_end = internal.decode_select(raw_data, opts.format_id)
> + else
> + response, raw_end = internal.decode_select(raw_data)
> + end
> 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, opts)
> + local body, raw_end
> + if opts ~= nil and opts.format_id ~= nil then
> + body, raw_end = internal.decode_select(raw_data, opts.format_id)
> + else
> + body, raw_end = internal.decode_select(raw_data)
> + end
Can the 'else' branch be executed at all? Shouldn't 'format' always be
available for decode_tuple and decode_get?
> if body[2] then
> return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
> end
> @@ -82,6 +92,15 @@ 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_select(raw_data, raw_data_end, opts)
> + if opts ~= nil and opts.format_id ~= nil then
> + return internal.decode_select(raw_data, opts.format_id)
> + end
> + return internal.decode_select(raw_data)
> +end
> +local function decode_execute(raw_data, raw_data_end)
> + return internal.decode_execute(raw_data)
> +end
>
> local function encode_call(send_buf, id, method_args)
> return internal.encode_call(send_buf, id, method_args.func_name,
> @@ -157,7 +176,7 @@ local method_encoder = {
>
> local method_decoder = {
> ping = decode_nil,
> - call_16 = internal.decode_select,
> + call_16 = decode_select,
I'd rather add decode_call_16 to avoid branching in decode_select
(branching in a virtual method looks ugly IMO).
> call_17 = decode_data,
> eval = decode_data,
> insert = decode_tuple,
> @@ -165,8 +184,8 @@ local method_decoder = {
> delete = decode_tuple,
> update = decode_tuple,
> upsert = decode_nil,
> - select = internal.decode_select,
> - execute = internal.decode_execute,
> + select = decode_select,
> + execute = decode_execute,
> get = decode_get,
> min = decode_get,
> max = decode_get,
> @@ -630,14 +649,15 @@ 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.method_args)
> assert(real_end == body_end, "invalid body length")
> requests[id] = nil
> request.id = nil
> else
> local msg
> msg, real_end, request.errno =
> - method_decoder.push(body_rpos, body_end)
> + method_decoder.push(body_rpos, body_end, request.method_args)
Looks unnecessary.
> assert(real_end == body_end, "invalid body length")
> request.on_push(request.on_push_ctx, msg)
> end
> @@ -1085,6 +1105,14 @@ end
>
> function remote_methods:close()
> check_remote_arg(self, 'close')
> + if (self.space ~= nil and type(self.space) == 'table') then
> + for _,space in pairs(self.space) do
> + if space.format_id ~= nil then
> + internal._format_delete(space._format_id)
> + space.format_id = nil
> + end
> + end
> + end
> self._transport.stop()
> end
>
> @@ -1274,6 +1302,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
> s.index = {}
> s.temporary = false
> s._format = format
> + s._format_id = internal._format_new(format)
Shouldn't you unref the old format before setting the new one?
> s.connection = self
> if #space > 5 then
> local opts = space[6]
> @@ -1391,13 +1420,15 @@ space_metatable = function(remote)
>
> function methods:insert(tuple, opts)
> check_space_arg(self, 'insert')
> - local method_args = {space_id=self.id, tuple=tuple}
> + local method_args = {space_id=self.id, tuple=tuple,
> + format_id=self._format_id}
> return remote:_request('insert', opts, method_args)
> end
>
> function methods:replace(tuple, opts)
> check_space_arg(self, 'replace')
> - local method_args = {space_id=self.id, tuple=tuple}
> + local method_args = {space_id=self.id, tuple=tuple,
> + format_id=self._format_id}
> return remote:_request('replace', opts, method_args)
> end
>
> @@ -1453,7 +1484,7 @@ index_metatable = function(remote)
> local limit = tonumber(opts and opts.limit) or 0xFFFFFFFF
> local method_args = {space_id=self.space.id, index_id=self.id,
> iterator=iterator, offset=offset, limit=limit,
> - key=key}
> + key=key, format_id=self.space._format_id}
> return (remote:_request('select', opts, method_args))
> end
>
> @@ -1463,7 +1494,8 @@ index_metatable = function(remote)
> error("index:get() doesn't support `buffer` argument")
> end
> local method_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_id=self.space._format_id}
> return nothing_or_data(remote:_request('get', opts, method_args))
> end
>
> @@ -1473,7 +1505,8 @@ index_metatable = function(remote)
> error("index:min() doesn't support `buffer` argument")
> end
> local method_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_id=self.space._format_id}
> return nothing_or_data(remote:_request('min', opts, method_args))
> end
>
> @@ -1483,7 +1516,8 @@ index_metatable = function(remote)
> error("index:max() doesn't support `buffer` argument")
> end
> local method_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_id=self.space._format_id}
> return nothing_or_data(remote:_request('max', opts, method_args))
> end
>
> @@ -1500,14 +1534,15 @@ index_metatable = function(remote)
>
> function methods:delete(key, opts)
> check_index_arg(self, 'delete')
> - local method_args = {space_id=self.space.id, index_id=self.id, key=key}
> + local method_args = {space_id=self.space.id, index_id=self.id, key=key,
> + format_id=self.space._format_id}
> return nothing_or_data(remote:_request('delete', opts, method_args))
> end
>
> function methods:update(key, oplist, opts)
> check_index_arg(self, 'update')
> local method_args = {space_id=self.space.id, index_id=self.id, key=key,
> - oplist=oplist}
> + oplist=oplist, format_id=self.space._format_id}
> return nothing_or_data(remote:_request('update', opts, method_args))
> end
>
> diff --git a/test/box/net.box.result b/test/box/net.box.result
> index 451c31d..da40a3d 100644
> --- a/test/box/net.box.result
> +++ b/test/box/net.box.result
> @@ -3572,6 +3572,83 @@ 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', 'universe')
Please don't use 'universe' in tests - we're trying to deprecate it
AFAIR. Grant specific permissions instead.
It would be nice to check that schema reload does update the format,
i.e. update the space format, call remote methods again and check
that they do use new names.
> +---
> +...
> +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
> +...
> +cn:close()
> +---
> +...
> +box.space.named:drop()
> +---
> +...
> +box.schema.user.revoke('guest', 'read, write, execute', 'universe')
> +---
> +...
> +--
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 0/2] netbox: define formats for tuple from netbox
2019-06-10 10:02 [PATCH v1 0/2] netbox: define formats for tuple from netbox imeevma
2019-06-10 10:02 ` [PATCH v1 1/2] netbox: store method_encoder args in request imeevma
2019-06-10 10:02 ` [PATCH v1 2/2] netbox: define formats for tuple from netbox imeevma
@ 2019-06-11 8:55 ` Vladimir Davydov
2019-06-14 12:32 ` Mergen Imeev
2 siblings, 1 reply; 12+ messages in thread
From: Vladimir Davydov @ 2019-06-11 8:55 UTC (permalink / raw)
To: imeevma; +Cc: tarantool-patches
On Mon, Jun 10, 2019 at 01:02:21PM +0300, imeevma@tarantool.org wrote:
> This patch defines field names for the tuples obtained through
> the netbox.
Links to the branch and the ticket are missing.
>
> Mergen Imeev (2):
> netbox: store method_encoder args in request
> netbox: define formats for tuple from netbox
>
> src/box/lua/net_box.c | 87 ++++++++++++++++-
> src/box/lua/net_box.lua | 232 +++++++++++++++++++++++++++++++++-------------
> test/box/access.result | 15 ++-
> test/box/access.test.lua | 9 +-
> test/box/net.box.result | 83 ++++++++++++++++-
> test/box/net.box.test.lua | 26 +++++-
> 6 files changed, 369 insertions(+), 83 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] netbox: store method_encoder args in request
2019-06-11 8:15 ` Vladimir Davydov
@ 2019-06-14 11:50 ` Mergen Imeev
2019-06-18 8:38 ` Vladimir Davydov
0 siblings, 1 reply; 12+ messages in thread
From: Mergen Imeev @ 2019-06-14 11:50 UTC (permalink / raw)
To: Vladimir Davydov; +Cc: tarantool-patches
On Tue, Jun 11, 2019 at 11:15:06AM +0300, Vladimir Davydov wrote:
> On Mon, Jun 10, 2019 at 01:02:23PM +0300, imeevma@tarantool.org wrote:
> > This patch changes the way arguments are passed to functions in
> > the array method_encoder, and saves these arguments in REQUEST.
> > This is necessary to establish a connection between netbox space
> > objects and the tuples obtained through the netbox
> >
> > Needed for #2978
> > ---
> > src/box/lua/net_box.lua | 179 +++++++++++++++++++++++++++++++---------------
> > 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, 146 insertions(+), 69 deletions(-)
> >
> > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> > index 251ad40..8d42fb4 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,70 @@ local function decode_push(raw_data)
> > return response[IPROTO_DATA_KEY][1], raw_end
> > end
> >
> > +local function encode_call(send_buf, id, method_args)
> > + return internal.encode_call(send_buf, id, method_args.func_name,
> > + method_args.args)
> > +end
> > +local function encode_call_16(send_buf, id, method_args)
> > + return internal.encode_call_16(send_buf, id, method_args.func_name,
> > + method_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, method_args)
> > + return internal.encode_eval(send_buf, id, method_args.code,
> > + method_args.args)
> > +end
> > +local function encode_insert(send_buf, id, method_args)
> > + return internal.encode_insert(send_buf, id, method_args.space_id,
> > + method_args.tuple)
> > +end
> > +local function encode_replace(send_buf, id, method_args)
> > + return internal.encode_replace(send_buf, id, method_args.space_id,
> > + method_args.tuple)
> > +end
> > +local function encode_delete(send_buf, id, method_args)
> > + return internal.encode_delete(send_buf, id, method_args.space_id,
> > + method_args.index_id, method_args.key)
> > +end
> > +local function encode_update(send_buf, id, method_args)
> > + return internal.encode_update(send_buf, id, method_args.space_id,
> > + method_args.index_id, method_args.key,
> > + method_args.oplist)
> > +end
> > +local function encode_upsert(send_buf, id, method_args)
> > + return internal.encode_upsert(send_buf, id, method_args.space_id,
> > + method_args.key, method_args.oplist)
> > +end
> > +local function encode_select(send_buf, id, method_args)
> > + return internal.encode_select(send_buf, id, method_args.space_id,
> > + method_args.index_id, method_args.iterator,
> > + method_args.offset, method_args.limit,
> > + method_args.key)
> > +end
> > +local function encode_execute(send_buf, id, method_args)
> > + return internal.encode_execute(send_buf, id, method_args.query,
> > + method_args.parameters, method_args.sql_opts)
> > +end
>
> Why not call it simply 'args'?
>
Fixed. I used 'method_args' since 'args' was used in call
and eval.
> > @@ -1150,14 +1201,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 method_args = {code=code, args={...}}
> > + return unpack((self:_request('eval', nil, method_args)))
> > end
> >
> > function remote_methods:eval(code, args, opts)
> > check_remote_arg(self, 'eval')
> > check_eval_args(args)
> > args = args or {}
> > - local res = self:_request('eval', opts, code, args)
> > + local method_args = {code=code, args=args}
>
> Coding style: we add a space before and after '=':
>
> local method_args = {code = code, args = args}
>
> Please fix here and everywhere else.
>
> Other than that this patch looks okay to me.
Fixed.
New patch:
From 8be1560eb5ed57d397c5f2da97112b448505080c Mon Sep 17 00:00:00 2001
Date: Wed, 5 Jun 2019 13:31:07 +0300
Subject: [PATCH] netbox: store method_encoder args in request
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
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 474297a..e87a60b 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 bea4300..6e58da8 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 ''");
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] netbox: define formats for tuple from netbox
2019-06-11 8:55 ` Vladimir Davydov
@ 2019-06-14 12:29 ` Mergen Imeev
2019-06-18 8:55 ` Vladimir Davydov
2019-06-18 9:00 ` Vladimir Davydov
0 siblings, 2 replies; 12+ messages in thread
From: Mergen Imeev @ 2019-06-14 12:29 UTC (permalink / raw)
To: Vladimir Davydov; +Cc: tarantool-patches
Thank you for review! There will be two new patches below.
On Tue, Jun 11, 2019 at 11:55:11AM +0300, Vladimir Davydov wrote:
> On Mon, Jun 10, 2019 at 01:02:24PM +0300, imeevma@tarantool.org wrote:
> > This patch creates tuple_formats for the tuples obtained through
> > the netbox.
> >
> > Closes #2978
>
> Please write a docbot request.
>
Done:
@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
...
> > static void
> > -netbox_decode_data(struct lua_State *L, const char **data)
> > +netbox_decode_data(struct lua_State *L, const char **data, uint32_t format_id)
>
> I'd pass a pointer to tuple_format struct instead of format_id to
> this function.
>
Done.
> > {
> > + (void)format_id;
>
> This is clearly useless.
>
Fixed.
> > + if (lua_gettop(L) != 1 || lua_istable(L, 1) != 1)
> > + return luaL_error(L, "Bad params!");
>
> Please print a "Usage: ..." message, like other functions do.
>
Fixed.
> > +
> > + uint32_t count = lua_objlen(L, 1);
> > + if (count == 0) {
> > + lua_pushinteger(L, box_tuple_format_default()->id);
>
> Please use tuple_format_runtime instead of box_tuple_format_default(),
> here and everywhere else. The latter is, after all, for modules.
>
Fixed.
> > + struct field_def *fields = region_alloc(region, size);
> > + if (fields == NULL) {
> > + diag_set(OutOfMemory, size, "region_alloc", "fields");
> > + return luaT_error(L);
> > + }
> > + memset(fields, 0, size);
>
> Please init each field with field_def_default in the loop below
> instead of zeroing it.
>
Fixed.
> > +
> > + lua_pushstring(L, "name");
> > + lua_gettable(L, -2);
> > + const char *name = lua_tolstring(L, -1, &len);
>
> Strictly speaking, the name or type can be absent. The type can also be
> invalid. We should handle those situations gracefully.
>
Fixed.
> > + fields[i].name = region_alloc(region, len + 1);
> > + 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);
> > + if (dict == NULL) {
> > + region_truncate(region, region_svp);
> > + return luaT_error(L);
> > + }
> > +
> > + struct tuple_format *format =
> > + tuple_format_new(&box_tuple_format_default()->vtab, NULL, NULL,
> > + 0, fields, count, 0, dict, false, false);
>
> Do we need really need to pass fields to this function? AFAIU tuple
> dictionary would be enough since we only need names.
You are right.
> What happens if a field is nullable?
>
It works fine, I think:
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"}, {name = "nf", is_nullable = true}}})
box.space.named:create_index('id', {parts = {{1, 'unsigned'}}})
box.space.named:insert({1})
box.space.named:insert({2,3})
box.space.named:get(1):tomap()
box.space.named:get(2):tomap()
require('net.box').connect('localhost', 3302).space.named:get(1):tomap()
require('net.box').connect('localhost', 3302).space.named:get(2):tomap()
tarantool> box.space.named:get(1):tomap()
---
- 1: 1
id: 1
...
tarantool> box.space.named:get(2):tomap()
---
- 1: 2
2: 3
nf: 3
id: 2
...
tarantool> require('net.box').connect('localhost', 3302).space.named:get(1):tomap()
---
- 1: 1
id: 1
...
tarantool> require('net.box').connect('localhost', 3302).space.named:get(2):tomap()
---
- 1: 2
2: 3
nf: 3
id: 2
...
> > + assert(format != NULL);
>
> Strictly speaking tuple_format_new() may fail with OOM.
> Please handle it.
>
Fixed.
> > + tuple_format_ref(format);
> > + lua_pushinteger(L, format->id);
>
> Returning a cdata with gc set would look robuster to me - with an id
> it's pretty easy to leak an object.
>
Fixed.
> > + region_truncate(region, region_svp);
> > + return 1;
> > +}
> > +
> > +static int
> > +netbox_format_delete(struct lua_State *L)
> > +{
> > + int32_t format_id = luaL_checkinteger(L, 1);
> > + if (format_id == box_tuple_format_default()->id)
> > + return 0;
> > + struct tuple_format *format = tuple_format_by_id(format_id);
> > + assert(format != NULL);
> > + tuple_format_unref(format);
> > + return 0;
> > +}
> > +
> > /**
> > * Decode Tarantool response body consisting of single
> > * IPROTO_DATA key into array of tuples.
> > @@ -619,6 +688,11 @@ netbox_decode_select(struct lua_State *L)
> > {
> > uint32_t ctypeid;
> > const char *data = *(const char **)luaL_checkcdata(L, 1, &ctypeid);
> > + uint32_t format_id;
> > + if (lua_gettop(L) == 2)
> > + format_id = luaL_checkinteger(L, 2);
> > + else
> > + format_id = box_tuple_format_default()->id;
> > assert(mp_typeof(*data) == MP_MAP);
> > uint32_t map_size = mp_decode_map(&data);
> > /* Until 2.0 body has no keys except DATA. */
> > @@ -627,7 +701,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_id);
> > *(const char **)luaL_pushcdata(L, ctypeid) = data;
> > return 2;
> > }
> > @@ -716,7 +790,8 @@ 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,
> > + box_tuple_format_default()->id);
> > rows_index = i - map_size;
> > break;
> > case IPROTO_METADATA:
> > @@ -766,6 +841,8 @@ luaopen_net_box(struct lua_State *L)
> > { "communicate", netbox_communicate },
> > { "decode_select", netbox_decode_select },
> > { "decode_execute", netbox_decode_execute },
> > + { "_format_new", netbox_format_new },
> > + { "_format_delete", netbox_format_delete },
>
> I think we better place these helpers in src/box/lua/misc.cc - we might
> need them somewhere else. Also, this is 'internal' namespace so no need
> to prefix function names with an underscore.
>
> Actually, I think we should add these functions in a separate patch and
> cover them with some basic tests involving argument checking (like
> passing an invalid field type or a field without a name).
>
Done. Patch below.
> > { NULL, NULL}
> > };
> > /* luaL_register_module polutes _G */
> > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> > index 8d42fb4..26ff7ff 100644
> > --- a/src/box/lua/net_box.lua
> > +++ b/src/box/lua/net_box.lua
> > @@ -63,12 +63,22 @@ 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, opts)
>
> You use name 'args' when passing 'opts' to the encoder. Let's use name
> 'args' everywhere to avoid confusion.
>
Fixed.
> > + local response, raw_end
> > + if opts ~= nil and opts.format_id ~= nil then
> > + response, raw_end = internal.decode_select(raw_data, opts.format_id)
> > + else
> > + response, raw_end = internal.decode_select(raw_data)
> > + end
> > 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, opts)
> > + local body, raw_end
> > + if opts ~= nil and opts.format_id ~= nil then
> > + body, raw_end = internal.decode_select(raw_data, opts.format_id)
> > + else
> > + body, raw_end = internal.decode_select(raw_data)
> > + end
>
> Can the 'else' branch be executed at all? Shouldn't 'format' always be
> available for decode_tuple and decode_get?
>
You are right, fixed.
> > if body[2] then
> > return nil, raw_end, box.error.MORE_THAN_ONE_TUPLE
> > end
> > @@ -82,6 +92,15 @@ 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_select(raw_data, raw_data_end, opts)
> > + if opts ~= nil and opts.format_id ~= nil then
> > + return internal.decode_select(raw_data, opts.format_id)
> > + end
> > + return internal.decode_select(raw_data)
> > +end
> > +local function decode_execute(raw_data, raw_data_end)
> > + return internal.decode_execute(raw_data)
> > +end
> >
> > local function encode_call(send_buf, id, method_args)
> > return internal.encode_call(send_buf, id, method_args.func_name,
> > @@ -157,7 +176,7 @@ local method_encoder = {
> >
> > local method_decoder = {
> > ping = decode_nil,
> > - call_16 = internal.decode_select,
> > + call_16 = decode_select,
>
> I'd rather add decode_call_16 to avoid branching in decode_select
> (branching in a virtual method looks ugly IMO).
>
Fixed.
> > call_17 = decode_data,
> > eval = decode_data,
> > insert = decode_tuple,
> > @@ -165,8 +184,8 @@ local method_decoder = {
> > delete = decode_tuple,
> > update = decode_tuple,
> > upsert = decode_nil,
> > - select = internal.decode_select,
> > - execute = internal.decode_execute,
> > + select = decode_select,
> > + execute = decode_execute,
> > get = decode_get,
> > min = decode_get,
> > max = decode_get,
> > @@ -630,14 +649,15 @@ 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.method_args)
> > assert(real_end == body_end, "invalid body length")
> > requests[id] = nil
> > request.id = nil
> > else
> > local msg
> > msg, real_end, request.errno =
> > - method_decoder.push(body_rpos, body_end)
> > + method_decoder.push(body_rpos, body_end, request.method_args)
>
> Looks unnecessary.
>
Fixed.
> > assert(real_end == body_end, "invalid body length")
> > request.on_push(request.on_push_ctx, msg)
> > end
> > @@ -1085,6 +1105,14 @@ end
> >
> > function remote_methods:close()
> > check_remote_arg(self, 'close')
> > + if (self.space ~= nil and type(self.space) == 'table') then
> > + for _,space in pairs(self.space) do
> > + if space.format_id ~= nil then
> > + internal._format_delete(space._format_id)
> > + space.format_id = nil
> > + end
> > + end
> > + end
> > self._transport.stop()
> > end
> >
> > @@ -1274,6 +1302,7 @@ function remote_methods:_install_schema(schema_version, spaces, indices,
> > s.index = {}
> > s.temporary = false
> > s._format = format
> > + s._format_id = internal._format_new(format)
>
> Shouldn't you unref the old format before setting the new one?
>
Fixed since __gc is set now.
> > +-- 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', 'universe')
>
> Please don't use 'universe' in tests - we're trying to deprecate it
> AFAIR. Grant specific permissions instead.
>
Fixed.
> It would be nice to check that schema reload does update the format,
> i.e. update the space format, call remote methods again and check
> that they do use new names.
>
Done.
First new patch:
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;
+
/** {{{ 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 *' "\
+ "expected, got %s)",
+ lua_typename(L, lua_type(L, 1)));
+ }
+ box_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;
+ 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)
+{
+ 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)");
+ 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},
+ {"format_new", lbox_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/test/box/misc.result b/test/box/misc.result
index 43b5a4a..ce39bbb 100644
--- a/test/box/misc.result
+++ b/test/box/misc.result
@@ -1271,3 +1271,51 @@ box.cfg{too_long_threshold = too_long_threshold}
s:drop()
---
...
+--
+-- gh-2978: Function to parse space format.
+--
+format = {}
+---
+...
+format[1] = {}
+---
+...
+format[1].type = 'unsigned'
+---
+...
+box.internal.format_new(format)
+---
+- error: field 1 name is not specified
+...
+format[1].name = 'aaa'
+---
+...
+format[2] = {}
+---
+...
+format[2].name = 'aaa'
+---
+...
+format[2].type = 'any'
+---
+...
+box.internal.format_new(format)
+---
+- error: Space field 'aaa' is duplicate
+...
+format[2].name = 'bbb'
+---
+...
+format[3] = {}
+---
+...
+format[3].name = 'ccc'
+---
+...
+format[3].type = 'something'
+---
+...
+box.internal.format_new(format)
+---
+- error: field 3 has unknown field type
+...
diff --git a/test/box/misc.test.lua b/test/box/misc.test.lua
index 75d937e..2d13b99 100644
--- a/test/box/misc.test.lua
+++ b/test/box/misc.test.lua
@@ -353,3 +353,23 @@ lsn == expected_lsn
box.cfg{too_long_threshold = too_long_threshold}
s:drop()
+
+--
+-- gh-2978: Function to parse space format.
+--
+format = {}
+format[1] = {}
+format[1].type = 'unsigned'
+box.internal.format_new(format)
+
+format[1].name = 'aaa'
+format[2] = {}
+format[2].name = 'aaa'
+format[2].type = 'any'
+box.internal.format_new(format)
+
+format[2].name = 'bbb'
+format[3] = {}
+format[3].name = 'ccc'
+format[3].type = 'something'
+box.internal.format_new(format)
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);
+ } 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 +635,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 +724,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..30cf227 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.format_new(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 e87a60b..413bdec 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 6e58da8..759cc4f 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] 12+ messages in thread
* Re: [PATCH v1 0/2] netbox: define formats for tuple from netbox
2019-06-11 8:55 ` [PATCH v1 0/2] " Vladimir Davydov
@ 2019-06-14 12:32 ` Mergen Imeev
0 siblings, 0 replies; 12+ messages in thread
From: Mergen Imeev @ 2019-06-14 12:32 UTC (permalink / raw)
To: Vladimir Davydov; +Cc: tarantool-patches
Hi! Thank you for review. I'm sorry, I haven't looked at my
patch properly before sending it last time.
https://github.com/tarantool/tarantool/issues/2978
https://github.com/tarantool/tarantool/tree/imeevma/gh-2978-formats-for-tuples-from-net-box
On Tue, Jun 11, 2019 at 11:55:52AM +0300, Vladimir Davydov wrote:
> On Mon, Jun 10, 2019 at 01:02:21PM +0300, imeevma@tarantool.org wrote:
> > This patch defines field names for the tuples obtained through
> > the netbox.
>
> Links to the branch and the ticket are missing.
>
> >
> > Mergen Imeev (2):
> > netbox: store method_encoder args in request
> > netbox: define formats for tuple from netbox
> >
> > src/box/lua/net_box.c | 87 ++++++++++++++++-
> > src/box/lua/net_box.lua | 232 +++++++++++++++++++++++++++++++++-------------
> > test/box/access.result | 15 ++-
> > test/box/access.test.lua | 9 +-
> > test/box/net.box.result | 83 ++++++++++++++++-
> > test/box/net.box.test.lua | 26 +++++-
> > 6 files changed, 369 insertions(+), 83 deletions(-)
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 1/2] netbox: store method_encoder args in request
2019-06-14 11:50 ` Mergen Imeev
@ 2019-06-18 8:38 ` Vladimir Davydov
0 siblings, 0 replies; 12+ messages in thread
From: Vladimir Davydov @ 2019-06-18 8:38 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
On Fri, Jun 14, 2019 at 02:50:23PM +0300, Mergen Imeev wrote:
> New patch:
>
> From 8be1560eb5ed57d397c5f2da97112b448505080c Mon Sep 17 00:00:00 2001
> Date: Wed, 5 Jun 2019 13:31:07 +0300
> Subject: [PATCH] netbox: store method_encoder args in request
>
> 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
Looks good to me now, thanks.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] netbox: define formats for tuple from netbox
2019-06-14 12:29 ` Mergen Imeev
@ 2019-06-18 8:55 ` Vladimir Davydov
2019-06-18 9:00 ` Vladimir Davydov
1 sibling, 0 replies; 12+ messages in thread
From: Vladimir Davydov @ 2019-06-18 8:55 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
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.
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.
> /** {{{ 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.
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.
> + 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.
> +{
> + 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.
Also, I think we should allow calling it without any arguments, in which
case it should be equivalent to box.internal.new_tuple_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 "\
Useless backslash (\).
> + "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},
> + {"format_new", lbox_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/test/box/misc.result b/test/box/misc.result
> index 43b5a4a..ce39bbb 100644
> --- a/test/box/misc.result
> +++ b/test/box/misc.result
> @@ -1271,3 +1271,51 @@ box.cfg{too_long_threshold = too_long_threshold}
> s:drop()
> ---
> ...
> +--
> +-- gh-2978: Function to parse space format.
> +--
> +format = {}
> +---
> +...
> +format[1] = {}
> +---
> +...
> +format[1].type = 'unsigned'
> +---
> +...
> +box.internal.format_new(format)
> +---
> +- error: field 1 name is not specified
> +...
> +format[1].name = 'aaa'
> +---
> +...
> +format[2] = {}
> +---
> +...
> +format[2].name = 'aaa'
> +---
> +...
> +format[2].type = 'any'
> +---
> +...
> +box.internal.format_new(format)
> +---
> +- error: Space field 'aaa' is duplicate
> +...
> +format[2].name = 'bbb'
> +---
> +...
> +format[3] = {}
> +---
> +...
> +format[3].name = 'ccc'
> +---
> +...
> +format[3].type = 'something'
> +---
> +...
> +box.internal.format_new(format)
> +---
> +- error: field 3 has unknown field type
> +...
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v1 2/2] netbox: define formats for tuple from netbox
2019-06-14 12:29 ` Mergen Imeev
2019-06-18 8:55 ` Vladimir Davydov
@ 2019-06-18 9:00 ` Vladimir Davydov
1 sibling, 0 replies; 12+ messages in thread
From: Vladimir Davydov @ 2019-06-18 9:00 UTC (permalink / raw)
To: Mergen Imeev; +Cc: tarantool-patches
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.
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);
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-06-18 9:00 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-10 10:02 [PATCH v1 0/2] netbox: define formats for tuple from netbox imeevma
2019-06-10 10:02 ` [PATCH v1 1/2] netbox: store method_encoder args in request imeevma
2019-06-11 8:15 ` Vladimir Davydov
2019-06-14 11:50 ` Mergen Imeev
2019-06-18 8:38 ` Vladimir Davydov
2019-06-10 10:02 ` [PATCH v1 2/2] netbox: define formats for tuple from netbox imeevma
2019-06-11 8:55 ` Vladimir Davydov
2019-06-14 12:29 ` Mergen Imeev
2019-06-18 8:55 ` Vladimir Davydov
2019-06-18 9:00 ` Vladimir Davydov
2019-06-11 8:55 ` [PATCH v1 0/2] " Vladimir Davydov
2019-06-14 12:32 ` Mergen Imeev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox