From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id C48452D6A1 for ; Thu, 5 Apr 2018 15:28:13 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id vglUfuX2eJ68 for ; Thu, 5 Apr 2018 15:28:13 -0400 (EDT) Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [217.69.128.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 4F0BA2D635 for ; Thu, 5 Apr 2018 15:28:13 -0400 (EDT) From: Vladislav Shpilevoy Subject: [tarantool-patches] [PATCH 2/2] netbox: reuse _request() to do SQL execute() Date: Thu, 5 Apr 2018 22:28:10 +0300 Message-Id: <9401c4f91662037dca337c6d320c5952c61863fb.1522956408.git.v.shpilevoy@tarantool.org> In-Reply-To: References: In-Reply-To: References: Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: tarantool-patches@freelists.org Cc: kostja@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...\"]:: '") 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)