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 D0E3920A9C for ; Thu, 12 Jul 2018 07:16:58 -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 xPILs3oJ0ABG for ; Thu, 12 Jul 2018 07:16:58 -0400 (EDT) Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (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 5ECC126DCB for ; Thu, 12 Jul 2018 07:16:52 -0400 (EDT) From: Kirill Shcherbatov Subject: [tarantool-patches] [PATCH v1 1/1] sql: disallow division by zero Date: Thu, 12 Jul 2018 14:16:49 +0300 Message-Id: <2635b57f7537de386915fcac4d3fa6e10255f87a.1531394137.git.kshcherbatov@tarantool.org> 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: tarantool-patches@freelists.org Cc: v.shpilevoy@tarantool.org, Kirill Shcherbatov Dissallowed division by zero in SQL to follow ANSI standard. 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 src/box/sql/vdbe.c | 19 ++++++++++---- test/sql-tap/e_expr.test.lua | 55 ++++++++++++++++++++++++++++------------- test/sql-tap/randexpr1.test.lua | 11 +++------ 3 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index 7a4d376..8a200b0 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1590,13 +1590,15 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ case OP_Subtract: if (sqlite3SubInt64(&iB,iA)) goto fp_math; break; case OP_Multiply: if (sqlite3MulInt64(&iB,iA)) goto fp_math; break; case OP_Divide: { - if (iA==0) goto arithmetic_result_is_null; + if (iA == 0) + goto division_by_zero; if (iA==-1 && iB==SMALLEST_INT64) goto fp_math; iB /= iA; break; } default: { - if (iA==0) goto arithmetic_result_is_null; + if (iA == 0) + goto division_by_zero; if (iA==-1) iA = 1; iB %= iA; break; @@ -1615,14 +1617,16 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ case OP_Multiply: rB *= rA; break; case OP_Divide: { /* (double)0 In case of SQLITE_OMIT_FLOATING_POINT... */ - if (rA==(double)0) goto arithmetic_result_is_null; + if (rA == (double)0) + goto division_by_zero; rB /= rA; break; } default: { iA = (i64)rA; iB = (i64)rB; - if (iA==0) goto arithmetic_result_is_null; + if (iA == 0) + goto division_by_zero; if (iA==-1) iA = 1; rB = (double)(iB % iA); break; @@ -1644,9 +1648,14 @@ case OP_Remainder: { /* same as TK_REM, in1, in2, out3 */ } break; - arithmetic_result_is_null: +arithmetic_result_is_null: sqlite3VdbeMemSetNull(pOut); break; + +division_by_zero: + diag_set(ClientError, ER_SQL_EXECUTE, "division by zero"); + rc = SQL_TARANTOOL_ERROR; + goto abort_due_to_error; } /* Opcode: CollSeq P1 * * P4 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 @@ -165,11 +165,20 @@ for _, op1 in ipairs(oplist) do local sql1 = string.format("SELECT %s %s %s %s %s", A, op1, B, op2, C) local sql2 = string.format("SELECT (%s %s %s) %s %s", A, op1, B, op2, C) local sql3 = string.format("SELECT %s %s (%s %s %s)", A, op1, B, op2, C) - local a2 = test:execsql(sql2) - local a3 = test:execsql(sql3) - test:do_execsql_test( - testname, - sql1, (opprec[op2] < opprec[op1]) and a3 or a2) + local a2 = test:catchsql(sql2) + local a3 = test:catchsql(sql3) + local res = opprec[op2] < opprec[op1] and a3 or a2 + + if res[1] ~= 0 then + -- + -- gh-2135: Division by zero is forbiden. + -- + test:do_catchsql_test( + testname, sql1, + {1, "Failed to execute SQL statement: division by zero"}) + else + test:do_execsql_test(testname, sql1, res[2]) + end if (a2 ~= a3) then untested[op1..","..op2] = nil @@ -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) + 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"}) test:do_test( "randexpr-2.1651", -- 2.7.4