[tarantool-patches] Re: [PATCH v1 1/1] sql: disallow division by zero
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Fri Jul 13 12:50:23 MSK 2018
Hello. Thanks for the patch!
See 3 comments below.
On 12/07/2018 14:16, Kirill Shcherbatov wrote:
> Dissallowed division by zero in SQL to follow ANSI standard.
1. Typo.
> This also affects some unobvious scenarios when divider indirectly
> casts to zero like
> SELECT 'word1' / 'word2';
>
> Closes #2135.
> ---
> https://github.com/tarantool/tarantool/compare/kshch/gh-2135-dissallow-div-by-zero
> https://github.com/tarantool/tarantool/issues/2135
>
> diff --git a/test/sql-tap/e_expr.test.lua b/test/sql-tap/e_expr.test.lua
> index d617f09..d7b8820 100755
> --- a/test/sql-tap/e_expr.test.lua
> +++ b/test/sql-tap/e_expr.test.lua
> @@ -460,18 +469,30 @@ literals = {
> for _, op in ipairs(oplist) do
> for n1, rhs in ipairs(literals) do
> for n2, lhs in ipairs(literals) do
> - local t = test:execsql(string.format(" SELECT typeof(%s %s %s) ", lhs, op, rhs))[1]
> - test:do_test(
> - string.format("e_expr-7.%s.%s.%s", opname[op], n1, n2),
> - function()
> - --print("\n op "..op.." t "..t)
> - return (((op == "||") and ((t == "text") or
> - (t == "null"))) or
> - ((op ~= "||") and (((t == "integer") or
> - (t == "real")) or
> - (t == "null")))) and 1 or 0
> - end, 1)
> -
> + local res = test:catchsql(string.format(" SELECT typeof(%s %s %s) ", lhs, op, rhs))
> + local testname = string.format("e_expr-7.%s.%s.%s", opname[op], n1, n2)
> + if res[1] ~= 0 then
> + --
> + -- gh-2135: Division by zero is forbiden.
> + --
> + test:do_test(
> + testname,
> + function()
> + return res[2] == "Failed to execute SQL statement: division by zero"
> + end, true)
> + else
> + local t = res[2][1]
> + test:do_test(
> + testname,
> + function()
> + --print("\n op "..op.." t "..t)
2. Garbage comment.
> + return (((op == "||") and ((t == "text") or
> + (t == "null"))) or
> + ((op ~= "||") and (((t == "integer") or
> + (t == "real")) or
> + (t == "null")))) and 1 or 0
> + end, 1)
> + end
> end
> end
> end
> diff --git a/test/sql-tap/randexpr1.test.lua b/test/sql-tap/randexpr1.test.lua
> index 5665fff..7074b18 100755
> --- a/test/sql-tap/randexpr1.test.lua
> +++ b/test/sql-tap/randexpr1.test.lua
> @@ -16534,15 +16534,10 @@ test:do_test(
> -- </randexpr-2.1649>
> })
>
> -test:do_test(
> +test:do_catchsql_test(
> "randexpr-2.1650",
> - function()
> - return test:execsql "SELECT f+case when 17-t1.f in (select ~count(distinct (abs((abs(c)/abs(c)))/abs((abs(11)/abs(+case -t1.a when t1.d then -13 else 17 end))))-t1.c) from t1 union select ~case cast(avg(t1.c) AS integer) when -~ -count(*)*max((t1.f))+count(*) then (min(d)) else max(a) end-count(distinct t1.a)+ -count(*)-count(*) from t1) then d when b not between t1.f and t1.e then a else 11 end+t1.a FROM t1 WHERE NOT ((exists(select 1 from t1 where case when exists(select 1 from t1 where case when (t1.e+case (t1.b) when f then case t1.c when 11 | -13-coalesce((select max(d) from t1 where c not in ( -t1.e,t1.c,17) and t1.c<17),e)*17+t1.a then t1.b else 11 end else d end)+t1.f not in (d,13,d) then f else t1.d end not between (c) and f) then t1.e else f end-t1.c in (t1.b,t1.a,f))))"
> - end, {
> - -- <randexpr-2.1650>
> - 800
> - -- </randexpr-2.1650>
> - })
> + "SELECT f+case when 17-t1.f in (select ~count(distinct (abs((abs(c)/abs(c)))/abs((abs(11)/abs(+case -t1.a when t1.d then -13 else 17 end))))-t1.c) from t1 union select ~case cast(avg(t1.c) AS integer) when -~ -count(*)*max((t1.f))+count(*) then (min(d)) else max(a) end-count(distinct t1.a)+ -count(*)-count(*) from t1) then d when b not between t1.f and t1.e then a else 11 end+t1.a FROM t1 WHERE NOT ((exists(select 1 from t1 where case when exists(select 1 from t1 where case when (t1.e+case (t1.b) when f then case t1.c when 11 | -13-coalesce((select max(d) from t1 where c not in ( -t1.e,t1.c,17) and t1.c<17),e)*17+t1.a then t1.b else 11 end else d end)+t1.f not in (d,13,d) then f else t1.d end not between (c) and f) then t1.e else f end-t1.c in (t1.b,t1.a,f))))",
> + {1, "Failed to execute SQL statement: division by zero"})
3. Such huge test for division by zero is overkill. Please, fix the test to
avoid division by zero. Obviously it was not about divisions.
>
> test:do_test(
> "randexpr-2.1651",
>
More information about the Tarantool-patches
mailing list