Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] Netbox connection and self are interchangable
@ 2019-10-29 11:46 Maria
  2019-10-30  8:59 ` Kirill Yukhin
  0 siblings, 1 reply; 7+ messages in thread
From: Maria @ 2019-10-29 11:46 UTC (permalink / raw)
  To: tarantool-patches

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.

Closes #4513

Issue:
https://github.com/tarantool/tarantool/issues/4513
Branch:
https://github.com/tarantool/tarantool/tree/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type
---
 src/box/lua/net_box.lua   | 11 ++++++++++-
 test/box/net.box.result   | 24 ++++++++++++++++++++++++
 test/box/net.box.test.lua | 11 +++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 31a8c16b7..31cb9cb12 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -1518,7 +1518,16 @@ local function handle_eval_result(status, ...)
         rollback()
         return box.error(E_PROC_LUA, (...))
     end
-    return ...
+    if not ... then
+        return ...
+    end
+    local results = {...}
+    for n, res in pairs(results) do
+        if type(res) == 'cdata' and ffi.istype('struct tuple', res) then
+            results[n] = res:totable()
+        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..e0332a5f3 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3930,3 +3930,27 @@ test_run:grep_log('default', '00000040:.*')
 box.cfg{log_level=log_level}
 ---
 ...
+--
+-- gh-4513 netbox.connect returns table but netbox.self does not
+--
+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
+...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 8e65ff470..bcc41c84e 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1583,3 +1583,14 @@ test_run:wait_log('default', '00000030:.*', nil, 10)
 test_run:grep_log('default', '00000040:.*')
 
 box.cfg{log_level=log_level}
+
+--
+-- gh-4513 netbox.connect returns table but netbox.self does not
+--
+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}"))
-- 
2.20.1 (Apple Git-117)

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

* Re: [Tarantool-patches] [PATCH] Netbox connection and self are interchangable
  2019-10-29 11:46 [Tarantool-patches] [PATCH] Netbox connection and self are interchangable Maria
@ 2019-10-30  8:59 ` Kirill Yukhin
  0 siblings, 0 replies; 7+ messages in thread
From: Kirill Yukhin @ 2019-10-30  8:59 UTC (permalink / raw)
  To: Maria; +Cc: tarantool-patches

Hello,

On 29 окт 14:46, 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.
> 
> Closes #4513
> 
> Issue:
> https://github.com/tarantool/tarantool/issues/4513
> Branch:
> https://github.com/tarantool/tarantool/tree/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type

[This is not a review] Could you please extract
test case for the bug into separatye file?

--
Regards, Kirill Yukhin

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

* Re: [Tarantool-patches] [PATCH] Netbox connection and self are interchangable
  2019-11-07 14:27   ` Maria Khaydich
@ 2019-11-07 18:40     ` Бабин Олег
  0 siblings, 0 replies; 7+ messages in thread
From: Бабин Олег @ 2019-11-07 18:40 UTC (permalink / raw)
  To: Maria Khaydich; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 1751 bytes --]


I remembered one more think.
Remote net.box convents nils to box.NULLs but your approach with {...} ignores it.
Please, change pairs({...}) to select(). Also it's interesting to see following cases: uuid and decimal (but only for 2.3+)
Notice that net.box:call can't send and receive functions.

`function() return uuid() end` returns an error  in this implementation:
```
- error: '[string "return require(''uuid'')():__serialize()"]:1: ''struct tt_uuid''
     has no member named ''__serialize'''
```

But remote net.box returns:
```
- error: unsupported Lua type 'cdata'

``` Also please see  https://github.com/tarantool/tarantool/issues/4602

It seems that correct way is to use msgpack.decode(msgpack.encode()), but it will lead to performance degradation
so the problem should be discussed more closely.

>Четверг,  7 ноября 2019, 17:27 +03:00 от Maria Khaydich <maria.khaydich@tarantool.org>:
>
>Thank you for the comment!
>I’ve checked the behavior and previous version does not indeed cover the case with box.NULL. Suitable fix and tests were added. 
> 
>>Четверг, 7 ноября 2019, 17:08 +03:00 от Бабин Олег < olegrok@tarantool.org >:
>> 
>>Maria, thanks for your patch!
>>
>>But as I see you don't consider the case of box.NULL that has 'cdata' type, but doesn't have
>>__serialize metamethod.
>>
>>It will lead to an error:
>>```
>>- error: '[string "return x:__serialize()"]:1: ''void *'' has no member named ''__serialize'''
>>...
>>```
>>>Четверг, 7 ноября 2019, 16:47 +03:00 от Maria < maria.khaydich@tarantool.org >:
>>>
>>>+ if type(res) == 'cdata' then
>>>+ results[n] = res:__serialize()
>>>+ end
>>
>>-- 
>>Oleg Babin
> 
> 
>--
>Maria Khaydich
> 
-- 
Oleg Babin

[-- Attachment #2: Type: text/html, Size: 4061 bytes --]

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

* Re: [Tarantool-patches] [PATCH] Netbox connection and self are interchangable
  2019-11-07 14:08 ` Бабин Олег
@ 2019-11-07 14:27   ` Maria Khaydich
  2019-11-07 18:40     ` Бабин Олег
  0 siblings, 1 reply; 7+ messages in thread
From: Maria Khaydich @ 2019-11-07 14:27 UTC (permalink / raw)
  To: Бабин Олег
  Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]


Thank you for the comment!
I’ve checked the behavior and previous version does not indeed cover the case with box.NULL. Suitable fix and tests were added. 
  
>Четверг, 7 ноября 2019, 17:08 +03:00 от Бабин Олег <olegrok@tarantool.org>:
> 
>Maria, thanks for your patch!
>
>But as I see you don't consider the case of box.NULL that has 'cdata' type, but doesn't have
>__serialize metamethod.
>
>It will lead to an error:
>```
>- error: '[string "return x:__serialize()"]:1: ''void *'' has no member named ''__serialize'''
>...
>```
>>Четверг, 7 ноября 2019, 16:47 +03:00 от Maria < maria.khaydich@tarantool.org >:
>>
>>+ if type(res) == 'cdata' then
>>+ results[n] = res:__serialize()
>>+ end
>
>-- 
>Oleg Babin
 
 
--
Maria Khaydich
 

[-- Attachment #2: Type: text/html, Size: 2214 bytes --]

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

* Re: [Tarantool-patches] [PATCH] Netbox connection and self are interchangable
  2019-11-07 13:47 Maria
@ 2019-11-07 14:08 ` Бабин Олег
  2019-11-07 14:27   ` Maria Khaydich
  0 siblings, 1 reply; 7+ messages in thread
From: Бабин Олег @ 2019-11-07 14:08 UTC (permalink / raw)
  To: Maria; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 469 bytes --]

Maria, thanks for your patch!

But as I see you don't consider the case of box.NULL that has 'cdata' type, but doesn't have
__serialize metamethod.

It will lead to an error:
```
- error: '[string "return x:__serialize()"]:1: ''void *'' has no member named ''__serialize'''
...
``` 
>Четверг, 7 ноября 2019, 16:47 +03:00 от Maria <maria.khaydich@tarantool.org>:
>
>+ if type(res) == 'cdata' then
>+ results[n] = res:__serialize()
>+ end

-- 
Oleg Babin

[-- Attachment #2: Type: text/html, Size: 877 bytes --]

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

* [Tarantool-patches] [PATCH] Netbox connection and self are interchangable
@ 2019-11-07 13:47 Maria
  2019-11-07 14:08 ` Бабин Олег
  0 siblings, 1 reply; 7+ messages in thread
From: Maria @ 2019-11-07 13:47 UTC (permalink / raw)
  To: tarantool-patches, georgy

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.
The patch is fixed so that it now covers all
cases where netbox.connect and netbox.self
perform conversion of types - e.g., box.error.

Closes #4513
Issue:
https://github.com/tarantool/tarantool/issues/4513
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   | 32 ++++++++++++++++++++++++++++++++
 test/box/net.box.test.lua | 14 ++++++++++++++
 3 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 31a8c16b7..c30e6bced 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -1518,7 +1518,16 @@ local function handle_eval_result(status, ...)
         rollback()
         return box.error(E_PROC_LUA, (...))
     end
-    return ...
+    if not ... then
+        return ...
+    end
+    local results = {...}
+    for n, res in pairs(results) do
+        if type(res) == 'cdata' then
+            results[n] = res:__serialize()
+        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..805492bb1 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3930,3 +3930,35 @@ test_run:grep_log('default', '00000040:.*')
 box.cfg{log_level=log_level}
 ---
 ...
+--
+-- gh-4513 netbox.connect returns table but netbox.self does not
+--
+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
+...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 8e65ff470..b84e75e3d 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1583,3 +1583,17 @@ test_run:wait_log('default', '00000030:.*', nil, 10)
 test_run:grep_log('default', '00000040:.*')
 
 box.cfg{log_level=log_level}
+
+--
+-- gh-4513 netbox.connect returns table but netbox.self does not
+--
+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")'))
-- 
2.20.1 (Apple Git-117)

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

* [Tarantool-patches] [PATCH] Netbox connection and self are interchangable
@ 2019-10-24  8:45 Maria
  0 siblings, 0 replies; 7+ messages in thread
From: Maria @ 2019-10-24  8:45 UTC (permalink / raw)
  To: tarantool-patches

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.

Closes #4513

Issue:
https://github.com/tarantool/tarantool/issues/4513
Branch:
https://github.com/tarantool/tarantool/tree/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type
---
 src/box/lua/net_box.lua   | 11 ++++++++++-
 test/box/net.box.result   | 24 ++++++++++++++++++++++++
 test/box/net.box.test.lua | 11 +++++++++++
 3 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 31a8c16b7..31cb9cb12 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -1518,7 +1518,16 @@ local function handle_eval_result(status, ...)
         rollback()
         return box.error(E_PROC_LUA, (...))
     end
-    return ...
+    if not ... then
+        return ...
+    end
+    local results = {...}
+    for n, res in pairs(results) do
+        if type(res) == 'cdata' and ffi.istype('struct tuple', res) then
+            results[n] = res:totable()
+        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..e0332a5f3 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3930,3 +3930,27 @@ test_run:grep_log('default', '00000040:.*')
 box.cfg{log_level=log_level}
 ---
 ...
+--
+-- gh-4513 netbox.connect returns table but netbox.self does not
+--
+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
+...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 8e65ff470..bcc41c84e 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1583,3 +1583,14 @@ test_run:wait_log('default', '00000030:.*', nil, 10)
 test_run:grep_log('default', '00000040:.*')
 
 box.cfg{log_level=log_level}
+
+--
+-- gh-4513 netbox.connect returns table but netbox.self does not
+--
+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}"))
-- 
2.20.1 (Apple Git-117)

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

end of thread, other threads:[~2019-11-07 18:40 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-29 11:46 [Tarantool-patches] [PATCH] Netbox connection and self are interchangable Maria
2019-10-30  8:59 ` Kirill Yukhin
  -- strict thread matches above, loose matches on Subject: below --
2019-11-07 13:47 Maria
2019-11-07 14:08 ` Бабин Олег
2019-11-07 14:27   ` Maria Khaydich
2019-11-07 18:40     ` Бабин Олег
2019-10-24  8:45 Maria

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