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

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

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. ???

>   -- 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.

> +        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.

> +            test:execsql(string.format([[DROP VIEW "%s";]], v[3]))
> +            entry_count = entry_count + 1
> +        end
>       end
> -    return views
> +    return entry_count
>   end
>   
>   function test.do_select_tests(self, label, tests)
>