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 25AFA2A8DD for ; Tue, 26 Mar 2019 17:48:21 -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 kbRDhVBt8yuX for ; Tue, 26 Mar 2019 17:48:21 -0400 (EDT) Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [94.100.177.98]) (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 D3E522A8CA for ; Tue, 26 Mar 2019 17:48:20 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v9 7/7] sql: remove box.sql.execute() References: <40ed2682703f6c62298587dd74d997f71c124411.1553251042.git.imeevma@gmail.com> From: Vladislav Shpilevoy Message-ID: Date: Wed, 27 Mar 2019 00:48:17 +0300 MIME-Version: 1.0 In-Reply-To: <40ed2682703f6c62298587dd74d997f71c124411.1553251042.git.imeevma@gmail.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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, imeevma@tarantool.org Thanks for the patch! See 5 comments below. > diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua > index 6c9a820..5530b2c 100644 > --- a/src/box/lua/load_cfg.lua > +++ b/src/box/lua/load_cfg.lua > @@ -505,17 +505,14 @@ end > box.cfg = locked(load_cfg) > > -- > --- This makes possible do box.sql.execute without calling box.cfg > +-- This makes possible do box.execute without calling box.cfg > -- manually. The load_cfg call overwrites following table and 1. There are no metatable anymore as I see. At least an explicit. Do you really call load_cfg on every single box.execute() call? If you do - please, do not. It is too slow. Use metatables. > -- metatable. > -- > -box.sql = {} > -setmetatable(box.sql, { > - __index = function(table, index) > - load_cfg() > - return box.sql[index] > - end, > -}) > +function box.execute(...) > + load_cfg() > + return box.execute(...) > +end > > -- gh-810: > -- hack luajit default cpath > diff --git a/test/sql-tap/gh2140-trans.test.lua b/test/sql-tap/gh2140-trans.test.lua > index fe978d1..3843c97 100755 > --- a/test/sql-tap/gh2140-trans.test.lua > +++ b/test/sql-tap/gh2140-trans.test.lua > @@ -28,8 +28,8 @@ test:do_execsql_test('rollback1_check', > for _, verb in ipairs({'ROLLBACK', 'ABORT'}) do > - box.sql.execute('DELETE FROM t2') > - answer = "Duplicate key exists in unique index 'unique_unnamed_T1_2' in space 'T1'" > + box.execute('DELETE FROM t2') > + answer = "/Duplicate key exists in unique index 'unique_unnamed_T1_2' in space 'T1'/" 2. Why are '/' added? > test:do_catchsql_test('insert1_'..verb, > [[START TRANSACTION; > INSERT INTO t2 VALUES (20, 2, 2); > diff --git a/test/sql-tap/lua/sqltester.lua b/test/sql-tap/lua/sqltester.lua > index 63c5d9b..cb77bc4 100644 > --- a/test/sql-tap/lua/sqltester.lua > +++ b/test/sql-tap/lua/sqltester.lua > @@ -403,7 +415,16 @@ test.do_eqp_test = function (self, label, sql, result) > test:do_test( > label, > function() > - return execsql_one_by_one("EXPLAIN QUERY PLAN "..sql) > + local result = execsql_one_by_one("EXPLAIN QUERY PLAN "..sql) > + local res = {} > + for k,v in pairs(result) do > + res[k] = {} > + res[k][1] = v[1] > + res[k][2] = v[2] > + res[k][3] = v[3] > + res[k][4] = tostring(v[4]) 3. What is happening here? > + end > + return res > end, > result) > end > diff --git a/test/sql-tap/orderby9.test.lua b/test/sql-tap/orderby9.test.lua > index 191c21b..13f9ec6 100755 > --- a/test/sql-tap/orderby9.test.lua > +++ b/test/sql-tap/orderby9.test.lua > @@ -41,6 +41,7 @@ test:do_test( > -- separately for the result set and the ORDER BY clause, then the output > -- order will be random. > local l1 = test:execsql("SELECT random() AS y FROM t1 ORDER BY 1;") > + for k,_ in pairs(l1) do l1[k] = tonumber(l1[k]) end > local l2 = table.deepcopy(l1) > table.sort(l1) > return test.is_deeply_regex(l1, l2) > @@ -50,6 +51,7 @@ test:do_test( > 1.1, > function() > local l1 = test:execsql("SELECT random() AS y FROM t1 ORDER BY random();") > + for k,_ in pairs(l1) do l1[k] = tonumber(l1[k]) end 4. And in these two hunks? > local l2 = table.deepcopy(l1) > table.sort(l1) > return test.is_deeply_regex(l1, l2) > diff --git a/test/sql/min-on-index.result b/test/sql/min-on-index.result > deleted file mode 100644 > index 1b2aadf..0000000 > --- a/test/sql/min-on-index.result > +++ /dev/null 5. Why removed?