[tarantool-patches] Re: [PATCH v1 1/1] sql: disallow division by zero

  • From: Vladislav Shpilevoy <v.shpilevoy@xxxxxxxxxxxxx>
  • To: Kirill Shcherbatov <kshcherbatov@xxxxxxxxxxxxx>, tarantool-patches@xxxxxxxxxxxxx
  • Date: Fri, 13 Jul 2018 12:50:23 +0300

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",


Other related posts: