[tarantool-patches] [PATCH 2/2] netbox: reuse _request() to do SQL execute()

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

_request() is a wrapper for perform_request, that detects schema
changes, and waits until it is reloaded. Lets use _request()
instead of direct perform_request() for execute(). The reason why
the _request() was not used earlier was my attempt to avoid
multiple return values in _request(), that leads to minor fixes
in non-execute methods like index.select or eval/call_16, which
return the _request() directly.

But diplicating schema reloading logic for execute() is worse.

Closes #3323
Closes #3322
---
 src/box/lua/net_box.lua   | 57 +++++++++++++++++++++++++----------------------
 test/box/net.box.result   |  5 +++--
 test/box/net.box.test.lua |  5 +++--
 test/sql/iproto.result    | 45 -------------------------------------
 test/sql/iproto.test.lua  | 15 -------------
 5 files changed, 36 insertions(+), 91 deletions(-)

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 8d1955315..ccf40df06 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -848,33 +848,41 @@ function remote_methods:_request(method, opts, ...)
         deadline = self._deadlines[this_fiber]
     end
     local buffer = opts and opts.buffer
-    local err, res
+    local err, res, metadata, info
     repeat
         local timeout = deadline and max(0, deadline - fiber_clock())
         if self.state ~= 'active' then
             wait_state('active', timeout)
             timeout = deadline and max(0, deadline - fiber_clock())
         end
-        err, res = perform_request(timeout, buffer, method,
-                                   self.schema_version, ...)
+        err, res, metadata, info =
+            perform_request(timeout, buffer, method, self.schema_version, ...)
         if not err and buffer ~= nil then
             return res -- the length of xrow.body
         elseif not err then
-            setmetatable(res, sequence_mt)
-            local postproc = method ~= 'eval' and method ~= 'call_17'
-            if postproc then
-                local tnew = box.tuple.new
-                for i, v in pairs(res) do
-                    res[i] = tnew(v)
+            -- SQL execute can return info with no data.
+            if res then
+                setmetatable(res, sequence_mt)
+                local postproc = method ~= 'eval' and method ~= 'call_17'
+                if postproc then
+                    local tnew = box.tuple.new
+                    for i, v in pairs(res) do
+                        res[i] = tnew(v)
+                    end
                 end
             end
-            return res
+            return res, metadata, info
         elseif err == E_WRONG_SCHEMA_VERSION then
             err = nil
         end
     until err
     box.error({code = err, reason = res})
 end
+--
+-- Get multiple values and return the first only. Use to ignore
+-- tail consisting of nils.
+--
+local function first_value(arg) return arg end
 
 function remote_methods:ping(opts)
     check_remote_arg(self, 'ping')
@@ -900,7 +908,8 @@ 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), {...})
+    return first_value(self:_request('call_16', nil, tostring(func_name),
+                                     {...}))
 end
 
 function remote_methods:call(func_name, args, opts)
@@ -917,7 +926,7 @@ end
 -- @deprecated since 1.7.4
 function remote_methods:eval_16(code, ...)
     check_remote_arg(self, 'eval')
-    return unpack(self:_request('eval', nil, code, {...}))
+    return unpack(first_value(self:_request('eval', nil, code, {...})))
 end
 
 function remote_methods:eval(code, args, opts)
@@ -936,18 +945,10 @@ function remote_methods:execute(query, parameters, sql_opts, netbox_opts)
     if sql_opts ~= nil then
         box.error(box.error.UNSUPPORTED, "execute", "options")
     end
-    local timeout = netbox_opts and netbox_opts.timeout
-    local buffer = netbox_opts and netbox_opts.buffer
-    parameters = parameters or {}
-    sql_opts = sql_opts or {}
-    local err, res, metadata, info =
-        self._transport.perform_request(timeout, buffer, 'execute',
-                                        self.schema_version, query, parameters,
-                                        sql_opts)
-    if err then
-        box.error({code = err, reason = res})
-    end
-    if buffer ~= nil then
+    local res, metadata, info =
+        self:_request('execute', netbox_opts, query, parameters or {},
+                      sql_opts or {})
+    if netbox_opts and netbox_opts.buffer then
         return res -- body length. Body is written to the buffer.
     end
     assert((info == nil and metadata ~= nil and res ~= nil) or
@@ -1180,8 +1181,9 @@ 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)
+        return first_value(remote:_request('select', opts, self.space.id,
+                                           self.id, iterator, offset, limit,
+                                           key))
     end
 
     function methods:get(key, opts)
@@ -1222,7 +1224,8 @@ index_metatable = function(remote)
         end
         local code = string.format('box.space.%s.index.%s:count',
                                    self.space.name, self.name)
-        return remote:_request('call_16', opts, code, { key })[1][1]
+        return first_value(remote:_request('call_16', opts, code,
+                                           { key }))[1][1]
     end
 
     function methods:delete(key, opts)
diff --git a/test/box/net.box.result b/test/box/net.box.result
index cf7b27f0b..1c95fe0aa 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -25,8 +25,9 @@ test_run:cmd("setopt delimiter ';'")
 - true
 ...
 function x_select(cn, space_id, index_id, iterator, offset, limit, key, opts)
-    return cn:_request('select', opts, space_id, index_id, iterator,
-                       offset, limit, key)
+    local ret = cn:_request('select', opts, space_id, index_id, iterator,
+                            offset, limit, key)
+    return ret
 end
 function x_fatal(cn) cn._transport.perform_request(nil, nil, 'inject', 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 576b5cfea..03ca5cc0f 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -8,8 +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)
-    return cn:_request('select', opts, space_id, index_id, iterator,
-                       offset, limit, key)
+    local ret = cn:_request('select', opts, space_id, index_id, iterator,
+                            offset, limit, key)
+    return ret
 end
 function x_fatal(cn) cn._transport.perform_request(nil, nil, 'inject', nil, '\x80') end
 test_run:cmd("setopt delimiter ''");
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index f7749cb05..39e51d84c 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -286,9 +286,6 @@ cn:execute('create table test2(id primary key, a, b, c)')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 box.space.TEST2.name
 ---
 - TEST2
@@ -307,9 +304,6 @@ cn:execute('create index test2_a_b_index on test2(a, b)')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 #box.space.TEST2.index
 ---
 - 1
@@ -318,9 +312,6 @@ cn:execute('drop table test2')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 box.space.TEST2
 ---
 - null
@@ -333,9 +324,6 @@ cn:execute('create table test3(id primary key, a, b)')
 ...
 -- Rowcount = 1, although two tuples were created:
 -- for _space and for _index.
-cn:reload_schema()
----
-...
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 ---
 - rowcount: 3
@@ -350,9 +338,6 @@ cn:execute('create view test3_view(id) as select id from test3')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('create view if not exists test3_view(id) as select id from test3')
 ---
 - rowcount: 0
@@ -361,9 +346,6 @@ cn:execute('drop view test3_view')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('drop view if exists test3_view')
 ---
 - rowcount: 0
@@ -374,9 +356,6 @@ cn:execute('create index test3_sec on test3(a, b)')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('create index if not exists test3_sec on test3(a, b)')
 ---
 - rowcount: 0
@@ -385,9 +364,6 @@ cn:execute('drop index test3_sec on test3')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('drop index if exists test3_sec on test3')
 ---
 - rowcount: 0
@@ -398,9 +374,6 @@ cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;'
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('create trigger if not exists trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
 - rowcount: 0
@@ -409,9 +382,6 @@ cn:execute('drop trigger trig')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('drop trigger if exists trig')
 ---
 - rowcount: 0
@@ -422,29 +392,17 @@ cn:execute('create index idx1 on test3(a)')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('create index idx2 on test3(b)')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 box.space.TEST3:truncate()
 ---
 ...
-cn:reload_schema()
----
-...
 cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 ---
 - rowcount: 3
@@ -453,9 +411,6 @@ cn:execute('drop table test3')
 ---
 - rowcount: 1
 ...
-cn:reload_schema()
----
-...
 cn:execute('drop table if exists test3')
 ---
 - rowcount: 0
diff --git a/test/sql/iproto.test.lua b/test/sql/iproto.test.lua
index 64c0a56fe..faf844226 100644
--- a/test/sql/iproto.test.lua
+++ b/test/sql/iproto.test.lua
@@ -111,15 +111,12 @@ cn:execute('select :value', parameters)
 
 -- gh-2608 SQL iproto DDL
 cn:execute('create table test2(id primary key, a, b, c)')
-cn:reload_schema()
 box.space.TEST2.name
 cn:execute('insert into test2 values (1, 1, 1, 1)')
 cn:execute('select * from test2')
 cn:execute('create index test2_a_b_index on test2(a, b)')
-cn:reload_schema()
 #box.space.TEST2.index
 cn:execute('drop table test2')
-cn:reload_schema()
 box.space.TEST2
 
 -- gh-2617 DDL row_count either 0 or 1.
@@ -128,50 +125,38 @@ box.space.TEST2
 cn:execute('create table test3(id primary key, a, b)')
 -- Rowcount = 1, although two tuples were created:
 -- for _space and for _index.
-cn:reload_schema()
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 cn:execute('create table if not exists test3(id primary key)')
 
 -- Test CREATE VIEW [IF NOT EXISTS] and
 --      DROP   VIEW [IF EXISTS].
 cn:execute('create view test3_view(id) as select id from test3')
-cn:reload_schema()
 cn:execute('create view if not exists test3_view(id) as select id from test3')
 cn:execute('drop view test3_view')
-cn:reload_schema()
 cn:execute('drop view if exists test3_view')
 
 -- Test CREATE INDEX [IF NOT EXISTS] and
 --      DROP   INDEX [IF EXISTS].
 cn:execute('create index test3_sec on test3(a, b)')
-cn:reload_schema()
 cn:execute('create index if not exists test3_sec on test3(a, b)')
 cn:execute('drop index test3_sec on test3')
-cn:reload_schema()
 cn:execute('drop index if exists test3_sec on test3')
 
 -- Test CREATE TRIGGER [IF NOT EXISTS] and
 --      DROP   TRIGGER [IF EXISTS].
 cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
-cn:reload_schema()
 cn:execute('create trigger if not exists trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
 cn:execute('drop trigger trig')
-cn:reload_schema()
 cn:execute('drop trigger if exists trig')
 
 -- Test DROP TABLE [IF EXISTS].
 -- Create more indexes, triggers and _truncate tuple.
 cn:execute('create index idx1 on test3(a)')
-cn:reload_schema()
 cn:execute('create index idx2 on test3(b)')
-cn:reload_schema()
 box.space.TEST3:truncate()
-cn:reload_schema()
 cn:execute('create trigger trig INSERT ON test3 BEGIN SELECT * FROM test3; END;')
-cn:reload_schema()
 cn:execute('insert into test3 values (1, 1, 1), (2, 2, 2), (3, 3, 3)')
 cn:execute('drop table test3')
-cn:reload_schema()
 cn:execute('drop table if exists test3')
 
 -- gh-2602 obuf_alloc breaks the tuple in different slabs
-- 
2.14.3 (Apple Git-98)