[tarantool-patches] Re: [PATCH v9 7/7] sql: remove box.sql.execute()
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Wed Mar 27 00:48:17 MSK 2019
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?
More information about the Tarantool-patches
mailing list