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 0A8CD22522 for ; Wed, 16 May 2018 09:10:38 -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 d51B2ByY3o5o for ; Wed, 16 May 2018 09:10:37 -0400 (EDT) Received: from smtp51.i.mail.ru (smtp51.i.mail.ru [94.100.177.111]) (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 B418520297 for ; Wed, 16 May 2018 09:10:37 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] box: ability to omit box.cfg() call in sql References: <6056f62646b1aee1ce8c99b7d3fe5cc3d78a993d.1526475084.git.kshcherbatov@tarantool.org> From: Vladislav Shpilevoy Message-ID: <52ef9ff2-8b64-d68d-23d4-757303680135@tarantool.org> Date: Wed, 16 May 2018 16:10:35 +0300 MIME-Version: 1.0 In-Reply-To: <6056f62646b1aee1ce8c99b7d3fe5cc3d78a993d.1526475084.git.kshcherbatov@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed 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, Kirill Shcherbatov Hello. Thanks for the short and simple patch! See my 6 minor comments below. On 16/05/2018 15:51, Kirill Shcherbatov wrote: > This makes possible to call box.sql.execute without pre-calling box.cfg. > Introduced metatable __index function for box.sql that called when no > appropriate box.sql methods (execute, debug) is defined. It calls > box.cfg() itself that redefines box.sql table. > > Closes #3266 > --- > src/box/lua/load_cfg.lua | 15 +++++++++++++++ > test/box/cfg.result | 17 +++++++++++++++++ > test/box/cfg.test.lua | 10 ++++++++++ > 3 files changed, 42 insertions(+) > > diff --git a/src/box/lua/load_cfg.lua b/src/box/lua/load_cfg.lua > index 4ace1e1..781de3e 100644 > --- a/src/box/lua/load_cfg.lua > +++ b/src/box/lua/load_cfg.lua > @@ -418,6 +418,21 @@ local function load_cfg(cfg) > end > box.cfg = load_cfg > > +-- > +-- gh-3266: box.cfg{} still not optional on 2.0 brach 1. Please, do not reference github issues in the code. Only in tests. I see, that it is ignored few lines above, but it was wrong too. Unfortunately, the author of this ' -- gh-810:' line sometimes ignores its own rules. > +-- > +-- This makes possible do box.sql.execute without calling box.cfg > +-- manually. The load_cfg call would overwrite following table and > +-- metatable. 2. Why 'would'? It actually overrides (it is ok, only the comment is strange). > diff --git a/test/box/cfg.result b/test/box/cfg.result > index 14dceac..42a0a89 100644 > --- a/test/box/cfg.result > +++ b/test/box/cfg.result > @@ -434,6 +434,23 @@ box.cfg{net_msg_max = old + 1000} > box.cfg{net_msg_max = old} > --- > ... > +-- > +-- gh-3266: box.cfg{} still not optional on 2.0 brach > +-- > +-- box.sql defined with __index function in metatable overriten with first 3. overridden. > +-- sql.cfg() call 4. sql.cfg -> box.cfg. > +-- > +box.cfg() > +--- > +... > +assert(next(box.sql) ~= box.NULL) 5. Why box.NULL? NULL is cdata, and used mostly to insert NULL into spaces. Lets use nil. > +--- > +- true > +... > +assert(getmetatable(box.sql) == box.NULL) 6. Same.