[tarantool-patches] Re: [PATCH 4/6] sql: enforce implicit type conversions

["n.pettik" <korablev@tarantool.org>]

> On 30 Oct 2018, at 00:32, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Thanks for the fixes! See 3 comments below.
> 
>> --- a/test/sql-tap/boundary1.test.lua
>> +++ b/test/sql-tap/boundary1.test.lua
>> @@ -7649,5 +7649,4 @@ test:do_execsql_test(
>>     "boundary1-2.66.le.5",
>>     "SELECT a FROM t1 WHERE rowid <= -9.22337303685477580800e+18 ORDER BY x",
>>     {})
>> -
>> test:finish_test(
> 1. Stray diff in boundary1.test.lua.

Removed.

>>>> diff --git a/test/sql-tap/boundary2.test.lua b/test/sql-tap/boundary2.test.lua
>>>> index 3eaef75dc..be4b8750d 100755
>>>> --- a/test/sql-tap/boundary2.test.lua
>>>> +++ b/test/sql-tap/boundary2.test.lua
>>>> @@ -1,6 +1,6 @@
>>>>  #!/usr/bin/env tarantool
>>>>  test = require("sqltester")
>>>> -test:plan(3021)
>>>> +test:plan(2965)
>>>>    --!./tcltestrunner.lua
>>>>  -- 2008 December 11
>>>> @@ -7462,6 +7462,7 @@ test:do_execsql_test(
>>>>      "SELECT a FROM t1 WHERE r > 9.22337303685477580800e+18 ORDER BY a DESC",
>>>>      {})
>>>>  +if false then
>>> 
>>> 6. I thought you have removed all these 'if false'. Please, do it.
>> I did it in boundary3 test and forgot about this one. Fixed:
> 
> 2. Sorry, still can be found in cast.test.lua.

Thx, I found another one bug with CAST operator.
Here is the fix. I also uncommented the rest of tests.
In a nutshell: CAST(x’100’ as INT) and CAST(123 as BLOB)
didn’t work since I forgot to set a flag to memory cell.
Moreover, sqlite3AtoF returns 1 (true) in case of success,
so we need to revert its return code.

+++ b/src/box/sql/vdbemem.c
@@ -609,7 +609,7 @@ sqlite3VdbeMemCast(Mem * pMem, u8 aff)
                        pMem->u.r = pMem->u.i;
                        return 0;
                }
-               return sqlite3AtoF(pMem->z, &pMem->u.r, pMem->n);
+               return ! sqlite3AtoF(pMem->z, &pMem->u.r, pMem->n);
        }
        switch (aff) {
        case AFFINITY_BLOB:
@@ -619,15 +619,22 @@ sqlite3VdbeMemCast(Mem * pMem, u8 aff)
                        MemSetTypeFlag(pMem, MEM_Blob);
                        return SQLITE_OK;
                }
-               if (pMem->flags & MEM_Int || pMem->flags & MEM_Real)
-                       return sqlite3VdbeMemStringify(pMem, 0);
+               if (pMem->flags & MEM_Int || pMem->flags & MEM_Real) {
+                       if (sqlite3VdbeMemStringify(pMem, 1) != 0)
+                               return -1;
+                       MemSetTypeFlag(pMem, MEM_Blob);
+                       return 0;
+               }
                return SQLITE_ERROR;
        case AFFINITY_NUMERIC:
                return sqlite3VdbeMemNumerify(pMem);
        case AFFINITY_INTEGER:
                if ((pMem->flags & MEM_Blob) != 0) {
-                       return sql_atoi64(pMem->z, (int64_t *) &pMem->u.i,
-                                         pMem->n);
+                       if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i,
+                                         pMem->n) != 0)
+                               return -1;
+                       MemSetTypeFlag(pMem, MEM_Int);
+                       return 0;
                }
                return sqlite3VdbeMemIntegerify(pMem, true);
        case AFFINITY_REAL:

diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index ee132e11d..e57bdaf1d 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(72)
+test:plan(82)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -64,28 +64,16 @@ test:do_execsql_test(
         -- </cast-1.4>
     })
 
-if false then
-test:do_execsql_test(
+test:do_catchsql_test(
     "cast-1.5",
     [[
         SELECT CAST(x'616263' AS numeric)
     ]], {
         -- <cast-1.5>
-        0
+        1, 'Type mismatch: can not convert abc to real'
         -- </cast-1.5>
     })
 
-test:do_execsql_test(
-    "cast-1.6",
-    [[
-        SELECT typeof(CAST(x'616263' AS numeric))
-    ]], {
-        -- <cast-1.6>
-        "real"
-        -- </cast-1.6>
-    })
-end
-
 test:do_execsql_test(
     "cast-1.7",
     [[
@@ -106,29 +94,16 @@ test:do_execsql_test(
         -- </cast-1.8>
     })
 
-if false then
-test:do_execsql_test(
+test:do_catchsql_test(
     "cast-1.9",
     [[
         SELECT CAST(x'616263' AS integer)
     ]], {
         -- <cast-1.9>
-        0
+        1, 'Type mismatch: can not convert abc to integer'
         -- </cast-1.9>
     })
 
-test:do_execsql_test(
-    "cast-1.10",
-    [[
-        SELECT typeof(CAST(x'616263' AS integer))
-    ]], {
-        -- <cast-1.10>
-if false then
-test:do_execsql_test(
+test:do_catchsql_test(
     "cast-1.5",
     [[
         SELECT CAST(x'616263' AS numeric)
     ]], {
         -- <cast-1.5>
-        0
+        1, 'Type mismatch: can not convert abc to real'
         -- </cast-1.5>
     })
 
-test:do_execsql_test(
-    "cast-1.6",
-    [[
-        SELECT typeof(CAST(x'616263' AS numeric))
-    ]], {
-        -- <cast-1.6>
-        "real"
-        -- </cast-1.6>
-    })
-end
-
 test:do_execsql_test(
     "cast-1.7",
     [[
@@ -106,29 +94,16 @@ test:do_execsql_test(
         -- </cast-1.8>
     })
 
-if false then
-test:do_execsql_test(
+test:do_catchsql_test(
     "cast-1.9",
     [[
         SELECT CAST(x'616263' AS integer)
     ]], {
         -- <cast-1.9>
-        0
+        1, 'Type mismatch: can not convert abc to integer'
         -- </cast-1.9>
     })
 
-test:do_execsql_test(
-    "cast-1.10",
-    [[
-        SELECT typeof(CAST(x'616263' AS integer))
-    ]], {
-        -- <cast-1.10>
-        "integer"
-        -- </cast-1.10>
-    })
-end
-
-
 test:do_execsql_test(
     "cast-1.11",
     [[
@@ -289,7 +264,6 @@ test:do_execsql_test(
         -- </cast-1.26>
     })
 
-if false then
 test:do_execsql_test(
     "cast-1.27",
     [[
@@ -309,7 +283,6 @@ test:do_execsql_test(
         "blob"
         -- </cast-1.28>
     })
-end
 
 test:do_execsql_test(
     "cast-1.29",
@@ -391,7 +364,6 @@ test:do_execsql_test(
         -- </cast-1.36>
     })
 
-if false then
 test:do_execsql_test(
     "cast-1.37",
     [[
@@ -411,7 +383,6 @@ test:do_execsql_test(
         "blob"
         -- </cast-1.38>
     })
-end
 
 test:do_execsql_test(
     "cast-1.39",
@@ -786,7 +757,7 @@ test:do_execsql_test(
 
 
 
-if false then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
+if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
     test:do_execsql_test(
         "cast-3.21",
         [[

> 
> Also stray empty lines on 526, 763.

Removed.

> 
> 3. if false in in3.test.lua.

Removed in previous commit.