From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 7854C6EC71; Sat, 27 Feb 2021 20:53:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7854C6EC71 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1614448391; bh=YPCmiNIp8/Kgi3APrpegDnmVq+bob+q/gBnbYpRDeAo=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=FEB+/Gq6EXQLRg71s6QwF37xpu9plh0+xIfv1M+oXiDreGHPZ43adDmL653eLXPfD h1s3uEbR0wqasd1j+DDl8wyvOiXKZuia5ara1Vv7VjyF137g2+zSWHqXQoeuaaxUse nsEQiWmd6+viKzcG196MzEz9L3EeeEWY/No/EJgg= Received: from smtp31.i.mail.ru (smtp31.i.mail.ru [94.100.177.91]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E4DC06EC71 for ; Sat, 27 Feb 2021 20:53:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E4DC06EC71 Received: by smtp31.i.mail.ru with esmtpa (envelope-from ) id 1lG3mA-0003IP-2j; Sat, 27 Feb 2021 20:53:10 +0300 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org References: <3a5360daf42cd920e3ce536020a6ff2fbccc095e.1611232655.git.sergeyb@tarantool.org> <9c57efe9-3b45-40ee-ab94-a55b08bb27d1@tarantool.org> Message-ID: <6bfcded5-da78-92a7-f641-679bd3e1ea7f@tarantool.org> Date: Sat, 27 Feb 2021 20:53:09 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <9c57efe9-3b45-40ee-ab94-a55b08bb27d1@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9795828B892398B720FF7727D08DA7A588E6C33AE9CCC9B44182A05F5380850406A769DEF36B40D872DA9F7483B6731A6E5542E63F2C2A01539D3DC48CB253092 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7DCDABBCBEAF682B1EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006374D78D7F7271F09E9EA1F7E6F0F101C674E70A05D1297E1BBC6CDE5D1141D2B1C2F0524362B26FE72613420FFE594ADFAE60E0FF19F47D35E9FA2833FD35BB23D9E625A9149C048EE9ECD01F8117BC8BEA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735209ECD01F8117BC8BEA471835C12D1D977C4224003CC836476EB9C4185024447017B076A6E789B0E975F5C1EE8F4F765FC94C40504FA40089E3AA81AA40904B5D9CF19DD082D7633A078D18283394535A93AA81AA40904B5D98AA50765F790063789FE560007CBA68CD81D268191BDAD3D698AB9A7B718F8C442539A7722CA490C13377AFFFEAFD26923F8577A6DFFEA7C5B09DCDDD98ABCD293EC92FD9297F6715571747095F342E857739F23D657EF2BD5E8D9A59859A8B6505CD8581A3881E7089D37D7C0E48F6C5571747095F342E857739F23D657EF2B6825BDBE14D8E7028C9DFF55498CEFB0BD9CCCA9EDD067B1EDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A523C4D2E8BC0B79B8A84E34D6A46558D4405A963557078FE6D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75968C9853642EB7C3410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34806D3522FB05EB3922BA44182A4631EF55BCBB1A848AE624FA439F72532F02E3C9BFF6F84EFD86A41D7E09C32AA3244C489EFF76418332312D01639C9B81BB961E098CBE561D6343729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojzT24cXffn6zVrvWidZFuvA== X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F25884588979E22292E5B26A1D6501CF0EC1F387E9438CDA4132709F282EC151BADDC1D3523A6D01B4765B2DFB59E2DDD9FE06B14FA522850F29BC30112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v8 03/14] test: fix luacheck warnings W113 in test/sql-tap X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hello! On 27.02.2021 02:42, Vladislav Shpilevoy wrote: > Hi again! Thanks for the fixes! > >>>> -if 0>0 then >>>> -db("nullvalue", "null") >>>> test:do_select_tests( >>>> "e_select-7.9", >>>> { >>>> - {"1", "SELECT NULL UNION ALL SELECT NULL", {null, null}}, >>>> - {"2", "SELECT NULL UNION SELECT NULL", {null}}, >>>> - {"3", "SELECT NULL INTERSECT SELECT NULL", {null}}, >>>> + {"1", "SELECT NULL UNION ALL SELECT NULL", {"", ""}}, >>>> + {"2", "SELECT NULL UNION SELECT NULL", {""}}, >>>> + {"3", "SELECT NULL INTERSECT SELECT NULL", {""}}, >>>> {"4", "SELECT NULL EXCEPT SELECT NULL", {}}, >>> 2. But why? What was the effect of `db("nullvalue", "null")` in the >>> original tests? If you don't know then maybe it is worth leaving it >>> as is + adding `local null = box.NULL` or `local null = nil` so at >>> least we could preserve the test body unchanged for future >>> investigations. >> Well, reverted hunks back and disabled testcase in the test. >> >> I found a source code of original test [1] and it is not clear >> >> why "nullvalue" redefined there. >> >> 1. https://www.sqlite.org/src/file?ci=trunk&name=test%2Fe_select.test&ln=1504 > It is not 'redefinition' it seems. > https://www.oreilly.com/library/view/using-sqlite/9781449394592/re64.html > > It is a marker for output. From what I understood, sqlite prints NULLs as > empty string, and here in the test they wanted to look for NULLs in the > result. But you can't look for N empty strings. So NULL's string representation > was replaced with "null" temporary. In our case it won't happen because we > know number of returned columns and their exact values. Somebody will find > the same in the future and will either fix or delete the test. > > See 4 more comments below and diff in the end of the email. > >> diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua >> index b22f195ca..4704970c8 100755 >> --- a/test/sql-tap/lua_sql.test.lua >> +++ b/test/sql-tap/lua_sql.test.lua >> @@ -75,9 +75,10 @@ box.schema.func.create('CHECK_FROM_SQL_TO_LUA', {language = 'Lua', >> exports = {'LUA', 'SQL'}}) >> >> -- check for different types >> -for i = 1, #from_sql_to_lua, 1 do >> +for i = 1, #from_sql_to_lua, 1 do -- luacheck: ignore from_sql_to_lua >> test:do_execsql_test( >> "lua_sql-2.2."..i, >> + -- luacheck: ignore from_sql_to_lua > 1. You can just declare local from_sql_to_lua and also save it to _G. No > need to ignore it. Thanks for solution, it's definitely better than suppression. >> "select check_from_sql_to_lua("..i..","..from_sql_to_lua[i][1]..")", >> {1}) >> end >> @@ -102,9 +103,10 @@ box.schema.func.create('CHECK_FROM_LUA_TO_SQL', {language = 'Lua', >> exports = {'LUA', 'SQL'}}) >> >> -- check for different types >> -for i = 1, #from_lua_to_sql, 1 do >> +for i = 1, #from_lua_to_sql, 1 do -- luacheck: ignore from_lua_to_sql >> test:do_execsql_test( >> "lua_sql-2.3."..i, >> + -- luacheck: ignore from_lua_to_sql >> "select "..tostring(from_lua_to_sql[i][1]).." = check_from_lua_to_sql("..i..")", >> {true}) >> end >> diff --git a/test/sql-tap/minmax2.test.lua b/test/sql-tap/minmax2.test.lua >> index f3d608aab..b6c02a4cc 100755 >> --- a/test/sql-tap/minmax2.test.lua >> +++ b/test/sql-tap/minmax2.test.lua >> @@ -72,6 +72,7 @@ test:do_test( >> test:do_test( >> "minmax2-1.2", >> function() >> + -- luacheck: ignore sql_search_count >> return box.stat.sql().sql_search_count - sql_search_count > 2. There is no reason for sql_search_count to be global. > made it local >> end, 19) >> >> @@ -89,6 +90,7 @@ test:do_test( >> test:do_test( >> "minmax2-1.4", >> function() >> + -- luacheck: ignore sql_search_count >> return box.stat.sql().sql_search_count - sql_search_count >> end, 19) >> >> diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua >> index f57a22624..bb03f25ab 100755 >> --- a/test/sql-tap/subquery.test.lua >> +++ b/test/sql-tap/subquery.test.lua >> @@ -684,6 +684,8 @@ test:do_test( >> -- This is the key test. The subquery should have only run once. If >> -- The double-quoted identifier "two" were causing the subquery to be >> -- processed as a correlated subquery, then it would have run 4 times. >> + >> + -- luacheck: ignore callcnt >> return callcnt > 3. This can be fixed, lets not ignore it. One way would be > to store callcnt in a table which is saved in a local variable > and into _G. > > local callcnt = {value = 0} > _G.callcnt = callcnt > > Then you can use callcnt.value from anywhere to reference the > same counter. > > Another way is to have it local and update it via a function > stored in _G. > > Also can simply write _G.callcnt in 3 more places instead of > ignoring it. Done > >> end, 1) >> >> @@ -706,7 +708,9 @@ test:do_test( >> test:do_test( >> "subquery-6.2", >> function() >> + -- luacheck: ignore callcnt >> return callcnt >> + >> end, 4) >> >> test:do_test( >> @@ -725,6 +729,7 @@ test:do_test( >> test:do_test( >> "subquery-6.4", >> function() >> + -- luacheck: ignore callcnt >> return callcnt >> end, 1) >> >> diff --git a/test/sql-tap/triggerC.test.lua b/test/sql-tap/triggerC.test.lua >> index e95641938..7d496bc4d 100755 >> --- a/test/sql-tap/triggerC.test.lua >> +++ b/test/sql-tap/triggerC.test.lua >> @@ -791,7 +791,6 @@ for testno, v in ipairs(tests11) do >> SELECT a,b FROM log; >> ]] >> end, { >> - defaults > 4. The variable is supposed to be filled with something, from > what I see in the commented code. > > You can declare it with nil somewhere above to keep the not > working tests intact. luacheck doesn't warn about it and tests passed successfully. Although from code style point of view it's better to define it... --- a/test/sql-tap/triggerC.test.lua +++ b/test/sql-tap/triggerC.test.lua @@ -779,6 +779,8 @@ for testno, v in ipairs(tests11) do      --         -- X(891, "X!cmd", [=[["concat",["defaults"],["defaults"]]]=])      --     }) +    -- Legacy from the original code. Must be replaced with valid value. +    local defaults = nil      test:do_test(          "triggerC-11."..testno..".3",          function() @@ -790,6 +792,7 @@ for testno, v in ipairs(tests11) do                  SELECT a,b FROM log;              ]]          end, { +            defaults          })      -- >> }) >> >> -- >> diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua >> index 2f1af29b0..40fc71998 100755 >> --- a/test/sql-tap/view.test.lua >> +++ b/test/sql-tap/view.test.lua >> @@ -1119,6 +1093,9 @@ if (0 > 0) >> -- >> }) >> >> + -- Legacy from the original code. Must be replaced with analogue >> + -- functions from box. >> + local X = nil >> test:do_test( >> "view-22.2", >> function() > I tried to fix some of my comments: > > ==================== > diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua > index 4704970c8..d073ffe4f 100755 > --- a/test/sql-tap/lua_sql.test.lua > +++ b/test/sql-tap/lua_sql.test.lua > @@ -51,7 +51,7 @@ for _, val in ipairs({ > {result}) > end > > -_G.from_sql_to_lua = { > +local from_sql_to_lua = { > [1] = {1, 1}, > [2] = {"1", 1}, > [3] = {"1.5", 1.5}, > @@ -60,6 +60,7 @@ _G.from_sql_to_lua = { > [6] = {"x'0500'", "\u{0005}\u{0000}"}, > [7] = {"123123123123123", 123123123123123LL}, > } > +_G.from_sql_to_lua = from_sql_to_lua > > box.schema.func.create('CHECK_FROM_SQL_TO_LUA', {language = 'Lua', > is_deterministic = true, > @@ -75,10 +76,9 @@ box.schema.func.create('CHECK_FROM_SQL_TO_LUA', {language = 'Lua', > exports = {'LUA', 'SQL'}}) > > -- check for different types > -for i = 1, #from_sql_to_lua, 1 do -- luacheck: ignore from_sql_to_lua > +for i = 1, #from_sql_to_lua, 1 do > test:do_execsql_test( > "lua_sql-2.2."..i, > - -- luacheck: ignore from_sql_to_lua > "select check_from_sql_to_lua("..i..","..from_sql_to_lua[i][1]..")", > {1}) > end > diff --git a/test/sql-tap/minmax2.test.lua b/test/sql-tap/minmax2.test.lua > index b6c02a4cc..6a7859d42 100755 > --- a/test/sql-tap/minmax2.test.lua > +++ b/test/sql-tap/minmax2.test.lua > @@ -56,12 +56,12 @@ test:do_execsql_test( > -- > }) > > -_G.sql_search_count = 0 > +local sql_search_count = 0 > > test:do_test( > "minmax2-1.1", > function() > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT min(x) FROM t1" > end, { > -- > @@ -72,14 +72,13 @@ test:do_test( > test:do_test( > "minmax2-1.2", > function() > - -- luacheck: ignore sql_search_count > return box.stat.sql().sql_search_count - sql_search_count > end, 19) > > test:do_test( > "minmax2-1.3", > function() > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT max(x) FROM t1" > end, { > -- > @@ -90,7 +89,6 @@ test:do_test( > test:do_test( > "minmax2-1.4", > function() > - -- luacheck: ignore sql_search_count > return box.stat.sql().sql_search_count - sql_search_count > end, 19) > > @@ -98,7 +96,7 @@ test:do_test( > "minmax2-1.5", > function() > test:execsql "CREATE INDEX t1i1 ON t1(x DESC)" > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT min(x) FROM t1" > end, { > -- > @@ -109,13 +107,13 @@ test:do_test( > test:do_test( > "minmax2-1.6", > function() > - return box.stat.sql().sql_search_count - _G.sql_search_count > + return box.stat.sql().sql_search_count - sql_search_count > end, 1) > > test:do_test( > "minmax2-1.7", > function() > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT max(x) FROM t1" > end, { > -- > @@ -126,13 +124,13 @@ test:do_test( > test:do_test( > "minmax2-1.8", > function() > - return box.stat.sql().sql_search_count - _G.sql_search_count > + return box.stat.sql().sql_search_count - sql_search_count > end, 0) > > test:do_test( > "minmax2-1.9", > function() > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT max(y) FROM t1" > end, { > -- > @@ -143,7 +141,7 @@ test:do_test( > test:do_test( > "minmax2-1.10", > function() > - return box.stat.sql().sql_search_count - _G.sql_search_count > + return box.stat.sql().sql_search_count - sql_search_count > end, 19) > > test:do_test( > @@ -153,7 +151,7 @@ test:do_test( > CREATE TABLE t2(a INTEGER PRIMARY KEY, b INT ); > INSERT INTO t2 SELECT x, y FROM t1; > ]] > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT min(a) FROM t2" > end, { > -- > @@ -164,13 +162,13 @@ test:do_test( > test:do_test( > "minmax2-2.1", > function() > - return box.stat.sql().sql_search_count - _G.sql_search_count > + return box.stat.sql().sql_search_count - sql_search_count > end, 0) > > test:do_test( > "minmax2-2.2", > function() > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT max(a) FROM t2" > end, { > -- > @@ -181,7 +179,7 @@ test:do_test( > test:do_test( > "minmax2-2.3", > function() > - return box.stat.sql().sql_search_count - _G.sql_search_count > + return box.stat.sql().sql_search_count - sql_search_count > end, 0) > > test:do_test( > @@ -190,7 +188,7 @@ test:do_test( > test:execsql "INSERT INTO t2 VALUES((SELECT max(a) FROM t2)+1,999)" > > > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql "SELECT max(a) FROM t2" > end, { > -- > @@ -201,7 +199,7 @@ test:do_test( > test:do_test( > "minmax2-3.1", > function() > - return box.stat.sql().sql_search_count - _G.sql_search_count > + return box.stat.sql().sql_search_count - sql_search_count > end, 0) > > test:do_test( > @@ -210,7 +208,7 @@ test:do_test( > test:execsql "INSERT INTO t2 VALUES((SELECT max(a) FROM t2)+1,999)" > > > - _G.sql_search_count = box.stat.sql().sql_search_count > + sql_search_count = box.stat.sql().sql_search_count > return test:execsql " SELECT b FROM t2 WHERE a=(SELECT max(a) FROM t2) " > > > @@ -224,7 +222,7 @@ test:do_test( > test:do_test( > "minmax2-3.3", > function() > - return box.stat.sql().sql_search_count - _G.sql_search_count > + return box.stat.sql().sql_search_count - sql_search_count > end, 1) > > test:do_execsql_test( > diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua > index bb03f25ab..df3075ee3 100755 > --- a/test/sql-tap/subquery.test.lua > +++ b/test/sql-tap/subquery.test.lua > @@ -685,8 +685,7 @@ test:do_test( > -- The double-quoted identifier "two" were causing the subquery to be > -- processed as a correlated subquery, then it would have run 4 times. > > - -- luacheck: ignore callcnt > - return callcnt > + return _G.callcnt > end, 1) > > -- Ticket #1380. Make sure correlated subqueries on an IN clause work > @@ -708,8 +707,7 @@ test:do_test( > test:do_test( > "subquery-6.2", > function() > - -- luacheck: ignore callcnt > - return callcnt > + return _G.callcnt > > end, 4) > > @@ -729,8 +727,7 @@ test:do_test( > test:do_test( > "subquery-6.4", > function() > - -- luacheck: ignore callcnt > - return callcnt > + return _G.callcnt > end, 1) > > box.func.CALLCNT:drop() >