[tarantool-patches] Re: [PATCH 3/6] test: fix sqltester methods to drop all tables/views

[n.pettik]

> On 10 Dec 2018, at 17:16, Vladislav Shpilevoy <v.shpilevoy at tarantool.org> wrote:
> 
> Thanks for the patch! See 3 comments below.
> 
> On 10/12/2018 00:30, Nikita Pettik wrote:
>> Since we are going to remove string of SQL "CREATE TABLE ..." statement
>> from space's opts, lets rework methods in sqltester to drop all tables
>> and views in order to avoid relying on this parameter.
>> Part of #2647
>> ---
>>  test/sql-tap/drop_all.test.lua        |  4 ++--
>>  test/sql-tap/identifier_case.test.lua |  6 +++---
>>  test/sql-tap/lua/sqltester.lua        | 38 ++++++++++++++---------------------
>>  3 files changed, 20 insertions(+), 28 deletions(-)
>> diff --git a/test/sql-tap/lua/sqltester.lua b/test/sql-tap/lua/sqltester.lua
>> index 8751ef820..672d2e67d 100644
>> --- a/test/sql-tap/lua/sqltester.lua
>> +++ b/test/sql-tap/lua/sqltester.lua
>> @@ -277,7 +277,7 @@ end
>>  test.catchsql2 = catchsql2
>>    -- Show the VDBE program for an SQL statement but omit the Trace
>> --- opcode at the beginning.  This procedure can be used to prove
>> +-- opcode at the beginning.  This procedure can be used to PROVE
> 
> 1. ???

Oops, fixed:

diff --git a/test/sql-tap/lua/sqltester.lua b/test/sql-tap/lua/sqltester.lua
index 672d2e67d..076510932 100644
--- a/test/sql-tap/lua/sqltester.lua
+++ b/test/sql-tap/lua/sqltester.lua
@@ -277,7 +277,7 @@ end
 test.catchsql2 = catchsql2
 
 -- Show the VDBE program for an SQL statement but omit the Trace
--- opcode at the beginning.  This procedure can be used to PROVE
+-- opcode at the beginning.  This procedure can be used to prove

> 
>>  -- that different SQL statements generate exactly the same VDBE code.
>>  local function explain_no_trace(self, sql)
>>      tr = execsql(self, "EXPLAIN "..sql)
>> @@ -288,35 +288,27 @@ local function explain_no_trace(self, sql)
>>  end
>>  test.explain_no_trace = explain_no_trace
>>  json = require("json")
>> -function test.find_spaces_by_sql_statement(self, statement)
>> -    local spaces = box.space._space:select{}
>> -    local res = {}
>> -    for _, space in ipairs(spaces) do
>> -        local name_num = 3-- [3] is space name
>> -        local options_num = 6-- [6] is space options
>> -        if not space[name_num]:startswith("_") and space[options_num]["sql"] then
>> -            if string.find(space[options_num]["sql"], statement) then
>> -                table.insert(res, space[name_num])
>> -            end
>> -        end
>> -    end
>> -    return res
>> -end
>>    function test.drop_all_tables(self)
>> -    local tables = test:find_spaces_by_sql_statement("CREATE TABLE")
>> -    for _, table in ipairs(tables) do
>> -        test:execsql(string.format([[DROP TABLE "%s";]], table))
>> +    local entry_count = 0
>> +    for _, v in box.space._space:pairs() do
>> +        if v[1] >= 512 then
>> +            test:execsql(string.format([[DROP TABLE "%s";]], v[3]))
>> +            entry_count = entry_count + 1
> 
> 2. Since DD integration is finished, can we do box.space[v3]:drop()
> instead of executing SQL? I think it can help us to find more bugs.
> And it is faster.

Seems like:

@@ -293,7 +293,7 @@ function test.drop_all_tables(self)
     local entry_count = 0
     for _, v in box.space._space:pairs() do
         if v[1] >= 512 then
-            test:execsql(string.format([[DROP TABLE "%s";]], v[3]))
+            box.space[v[3]]:drop()
             entry_count = entry_count + 1
         end
     end
@@ -303,8 +303,8 @@ end
 function test.drop_all_views(self)
     local entry_count = 0
     for _, v in box.space._space:pairs() do
-        if v[1] > 512 and v[6]["view"] == true then
-            test:execsql(string.format([[DROP VIEW "%s";]], v[3]))
+        if v[1] > 512 and v[6].view == true then
+            box.space[v[3]]:drop()
             entry_count = entry_count + 1
         end

> 
>> +        end
>>      end
>> -    return tables
>> +    return entry_count
>>  end
>>    function test.drop_all_views(self)
>> -    local views = test:find_spaces_by_sql_statement("CREATE VIEW")
>> -    for _, view in ipairs(views) do
>> -        test:execsql(string.format([[DROP VIEW "%s";]], view))
>> +    local entry_count = 0
>> +    for _, v in box.space._space:pairs() do
>> +        if v[1] > 512 and v[6]["view"] == true then
> 
> 3. IMO .view looks better than ["view"], but I do not insist. Up to
> you.

I don’t really care, so let's use your approach (see above).