[Tarantool-patches] [PATCH] box: netbox.self and connect should work interchangeably
Oleg Babin
olegrok at tarantool.org
Fri Dec 13 16:53:38 MSK 2019
Hello, thanks for your patch!
> + for i = 1, select('#', ...) do
> + if type(results[i] == 'cdata') then
> + results[i] = msgpack.decode(msgpack.encode(results[i]))
I suppose that it should be "type(results[i]) == 'cdata'". Now this
condition works always.
> + if not ... then
> + return ...
> + end
What do you want to achieve with it?
This condition can't distinguish what it's a function without arguments
or has nil as first argument.
I think this check should be dropped or replaced to something as
"select('#', ...) == 0".
> + local results = {...}
This transformation drops trailing nils that should be evaluated to
box.NULL.
Please, add tests to check that all nils you passed will be converted to
box.NULLs after transformation.
--
Oleg Babin
On 13/12/2019 00:28, Maria wrote:
> Despite what was stated in the documentation, netbox.connect was not always
> equivalent to netbox.self. In particular, they converted tuple to different
> types - table and cdata respectively.
> Netbox.self also allowed to modify source objects after transfer, which was
> not an example of expected behavior either.
>
> The patch fixes both those issues and covers all cases where netbox.self and
> connect perform conversion of types - e.g., for box.error.
>
> Closes #4513, #4602
> ---
> Issues:
> https://github.com/tarantool/tarantool/issues/4513
> https://github.com/tarantool/tarantool/issues/4602
> Branch:
> https://github.com/tarantool/tarantool/compare/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type
>
> src/box/lua/net_box.lua | 11 +++-
> test/box/net.box.result | 4 +-
> test/box/net_connect_self.result | 81 ++++++++++++++++++++++++++++++
> test/box/net_connect_self.test.lua | 33 ++++++++++++
> 4 files changed, 125 insertions(+), 4 deletions(-)
> create mode 100644 test/box/net_connect_self.result
> create mode 100644 test/box/net_connect_self.test.lua
>
> diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
> index c2e1bb9c4..aa36ba7d1 100644
> --- a/src/box/lua/net_box.lua
> +++ b/src/box/lua/net_box.lua
> @@ -1523,7 +1523,16 @@ local function handle_eval_result(status, ...)
> rollback()
> return box.error(E_PROC_LUA, (...))
> end
> - return ...
> + if not ... then
> + return ...
> + end
> + local results = {...}
> + for i = 1, select('#', ...) do
> + if type(results[i] == 'cdata') then
> + results[i] = msgpack.decode(msgpack.encode(results[i]))
> + end
> + end
> + return unpack(results)
> end
>
> this_module.self = {
> diff --git a/test/box/net.box.result b/test/box/net.box.result
> index e3dabf7d9..836a4fe8c 100644
> --- a/test/box/net.box.result
> +++ b/test/box/net.box.result
> @@ -626,9 +626,7 @@ box.schema.func.drop('pause')
> -- call
> remote.self:call('test_foo', {'a', 'b', 'c'})
> ---
> -- - - a: 1
> - - - b: 2
> - - c
> +- [[{'a': 1}], [{'b': 2}], 'c']
> ...
> cn:call('test_foo', {'a', 'b', 'c'})
> ---
> diff --git a/test/box/net_connect_self.result b/test/box/net_connect_self.result
> new file mode 100644
> index 000000000..b6ec38fb8
> --- /dev/null
> +++ b/test/box/net_connect_self.result
> @@ -0,0 +1,81 @@
> +-- test-run result file version 2
> +remote = require('net.box')
> + | ---
> + | ...
> +--
> +-- gh-4513 netbox.connect and netbox.self should be interchangeable
> +--
> +space = box.schema.space.create('gh4513')
> + | ---
> + | ...
> +box.schema.user.grant('guest','read, write, execute','universe')
> + | ---
> + | ...
> +idx = box.space.gh4513:create_index('primary')
> + | ---
> + | ...
> +box.space.gh4513:insert({1})
> + | ---
> + | - [1]
> + | ...
> +
> +type(remote.connect(box.cfg.listen):eval("return box.space.gh4513:get({1})"))
> + | ---
> + | - table
> + | ...
> +type(remote.self:eval("return box.space.gh4513:get{1}"))
> + | ---
> + | - table
> + | ...
> +
> +type(remote.connect(box.cfg.listen):eval('return box.error.new(1, "test error")'))
> + | ---
> + | - string
> + | ...
> +type(remote.self:eval('return box.error.new(1, "test error")'))
> + | ---
> + | - string
> + | ...
> +
> +type(remote.self:eval("return box.NULL"))
> + | ---
> + | - cdata
> + | ...
> +type(remote.connect(box.cfg.listen):eval("return box.NULL"))
> + | ---
> + | - cdata
> + | ...
> +
> +--
> +-- gh-4602 net.box:self allows to modify source object after transfer
> +--
> +x = {}
> + | ---
> + | ...
> +function test() return x end
> + | ---
> + | ...
> +box.schema.func.create('test')
> + | ---
> + | ...
> +box.schema.user.grant('guest', 'execute', 'function', 'test')
> + | ---
> + | ...
> +
> +y = remote.connect(box.cfg.listen):call('test')
> + | ---
> + | ...
> +-- should be false
> +y == x
> + | ---
> + | - false
> + | ...
> +
> +z = remote.self:call('test')
> + | ---
> + | ...
> +-- should be false as well
> +z == x
> + | ---
> + | - false
> + | ...
> diff --git a/test/box/net_connect_self.test.lua b/test/box/net_connect_self.test.lua
> new file mode 100644
> index 000000000..76b16704f
> --- /dev/null
> +++ b/test/box/net_connect_self.test.lua
> @@ -0,0 +1,33 @@
> +remote = require('net.box')
> +--
> +-- gh-4513 netbox.connect and netbox.self should be interchangeable
> +--
> +space = box.schema.space.create('gh4513')
> +box.schema.user.grant('guest','read, write, execute','universe')
> +idx = box.space.gh4513:create_index('primary')
> +box.space.gh4513:insert({1})
> +
> +type(remote.connect(box.cfg.listen):eval("return box.space.gh4513:get({1})"))
> +type(remote.self:eval("return box.space.gh4513:get{1}"))
> +
> +type(remote.connect(box.cfg.listen):eval('return box.error.new(1, "test error")'))
> +type(remote.self:eval('return box.error.new(1, "test error")'))
> +
> +type(remote.self:eval("return box.NULL"))
> +type(remote.connect(box.cfg.listen):eval("return box.NULL"))
> +
> +--
> +-- gh-4602 net.box:self allows to modify source object after transfer
> +--
> +x = {}
> +function test() return x end
> +box.schema.func.create('test')
> +box.schema.user.grant('guest', 'execute', 'function', 'test')
> +
> +y = remote.connect(box.cfg.listen):call('test')
> +-- should be false
> +y == x
> +
> +z = remote.self:call('test')
> +-- should be false as well
> +z == x
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20191213/d8db5633/attachment.html>
More information about the Tarantool-patches
mailing list