From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTP id 20BC82726E for ; Fri, 13 Jul 2018 05:50:30 -0400 (EDT) Received: from turing.freelists.org ([127.0.0.1]) by localhost (turing.freelists.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id tloQ0_FDv1gA for ; Fri, 13 Jul 2018 05:50:30 -0400 (EDT) Received: from smtp35.i.mail.ru (smtp35.i.mail.ru [94.100.177.95]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by turing.freelists.org (Avenir Technologies Mail Multiplex) with ESMTPS id 6B6BE27261 for ; Fri, 13 Jul 2018 05:50:29 -0400 (EDT) Subject: [tarantool-patches] Re: [PATCH v1 1/1] sql: disallow division by zero References: <2635b57f7537de386915fcac4d3fa6e10255f87a.1531394137.git.kshcherbatov@tarantool.org> From: Vladislav Shpilevoy Message-ID: <50125917-283f-11a9-5849-5026efd17cef@tarantool.org> Date: Fri, 13 Jul 2018 12:50:23 +0300 MIME-Version: 1.0 In-Reply-To: <2635b57f7537de386915fcac4d3fa6e10255f87a.1531394137.git.kshcherbatov@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: tarantool-patches-bounce@freelists.org Errors-to: tarantool-patches-bounce@freelists.org Reply-To: tarantool-patches@freelists.org List-help: List-unsubscribe: List-software: Ecartis version 1.0.0 List-Id: tarantool-patches List-subscribe: List-owner: List-post: List-archive: To: Kirill Shcherbatov , tarantool-patches@freelists.org 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( > -- > }) > > -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, { > - -- > - 800 > - -- > - }) > + "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", >