Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts
@ 2021-07-27 11:31 Mergen Imeev via Tarantool-patches
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 1/3] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-27 11:31 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch fixes the explicit cast to comply with the new rules. According to
these rules, BOOLEAN and VARBINARY can no longer be cast to numeric types.

https://github.com/tarantool/tarantool/issues/4470
https://github.com/tarantool/tarantool/tree/imeevma/gh-4470-fix-explicit-cast

Changes in v2:
 - Change of str_to_bool() were moved in new patch.

Mergen Imeev (3):
  sql: disallow explicit cast of BOOLEAN to number
  sql: disallow explicit cast of VARBINARY to number
  sql: fix STRING to BOOLEAN explicit cast

 .../unreleased/gh-4470-explicit-cast.md       |   3 +
 src/box/sql/mem.c                             |  98 +++------
 test/sql-tap/cast.test.lua                    | 186 +++++++++++++++---
 test/sql-tap/cse.test.lua                     |  18 +-
 test/sql-tap/e_select1.test.lua               |   3 +-
 ...-4766-wrong-cast-from-blob-to-int.test.lua |  40 ----
 test/sql-tap/in1.test.lua                     |  17 +-
 test/sql-tap/misc3.test.lua                   |   2 +-
 test/sql-tap/numcast.test.lua                 |  18 +-
 test/sql/boolean.result                       |  71 +++----
 test/sql/boolean.test.sql                     |  13 +-
 test/sql/types.result                         |  14 +-
 12 files changed, 266 insertions(+), 217 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4470-explicit-cast.md
 delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v2 1/3] sql: disallow explicit cast of BOOLEAN to number
  2021-07-27 11:31 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches
@ 2021-07-27 11:31 ` Mergen Imeev via Tarantool-patches
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 3/3] sql: fix STRING to BOOLEAN explicit cast Mergen Imeev via Tarantool-patches
  2 siblings, 0 replies; 11+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-27 11:31 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

Thank you for the review! My answers, diff and new patch below.

On 26.07.2021 23:11, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> See 2 comments below.
>
>> diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
>> index 6b95e41d3..5c44bfdfc 100644
>> --- a/src/box/sql/mem.c
>> +++ b/src/box/sql/mem.c
>> @@ -683,24 +674,23 @@ str_to_bool(struct Mem *mem)
>>  {
>>      assert(mem->type == MEM_TYPE_STR);
>>      char *str = mem->z;
>> +    uint32_t len = mem->n;
>>      bool b;
>>      const char *str_true = "TRUE";
>>      const char *str_false = "FALSE";
>>      uint32_t len_true = strlen(str_true);
>>      uint32_t len_false = strlen(str_false);
>>  
>> -    for (; str[0] == ' '; str++);
>> -    if (strncasecmp(str, str_true, len_true) == 0) {
>> +    for (; isspace(str[0]); str++, len--);
>> +    for (; isspace(str[len - 1]); len--);
>> +    if (len != len_true && len != len_false)
>> +        return -1;
>> +
>> +    if (len == len_true && strncasecmp(str, str_true, len) == 0)
>>          b = true;
>> -        str += len_true;
>> -    } else if (strncasecmp(str, str_false, len_false) == 0) {
>> +    else if (len == len_false && strncasecmp(str, str_false, len) == 0)
>>          b = false;
>> -        str += len_false;
>> -    } else {
>> -        return -1;
>> -    }
>> -    for (; str[0] == ' '; str++);
>> -    if (str[0] != '\0')
>> +    else
>>          return -1;
>
> 1. Why did you change str_to_bool() if the patch is only about
> numbers <-> bool?
>
Fixed, moved to new patch. Also, added a test.

>>      mem_set_bool(mem, b);
>>      return 0;
>> @@ -1074,19 +1036,11 @@ mem_cast_explicit(struct Mem *mem, enum field_type type)
>>      case FIELD_TYPE_INTEGER:
>>          return mem_to_int(mem);
>>      case FIELD_TYPE_BOOLEAN:
>> -        switch (mem->type) {
>> -        case MEM_TYPE_BOOL:
>> +        if (mem->type == MEM_TYPE_BOOL)
>>              return 0;
>> -        case MEM_TYPE_INT:
>> -        case MEM_TYPE_UINT:
>> -            return int_to_bool(mem);
>> -        case MEM_TYPE_STR:
>> +        if (mem->type == MEM_TYPE_STR)
>>              return str_to_bool(mem);
>> -        case MEM_TYPE_DOUBLE:
>> -            return double_to_bool(mem);
>> -        default:
>> -            return -1;
>
> 2. I would propose to keep the switch-case. Otherwise you are
> going to jump back and forth between if and switch when these
> places will be changed again.
Fixed.

Diff:

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index ccd368170..aeaa01d94 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -28,8 +28,6 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
-#include <ctype.h>
-
 #include "sqlInt.h"
 #include "mem.h"
 #include "vdbeInt.h"
@@ -674,23 +672,24 @@ str_to_bool(struct Mem *mem)
 {
    assert(mem->type == MEM_TYPE_STR);
    char *str = mem->z;
-   uint32_t len = mem->n;
    bool b;
    const char *str_true = "TRUE";
    const char *str_false = "FALSE";
    uint32_t len_true = strlen(str_true);
    uint32_t len_false = strlen(str_false);
 
-   for (; isspace(str[0]); str++, len--);
-   for (; isspace(str[len - 1]); len--);
-   if (len != len_true && len != len_false)
-       return -1;
-
-   if (len == len_true && strncasecmp(str, str_true, len) == 0)
+   for (; str[0] == ' '; str++);
+   if (strncasecmp(str, str_true, len_true) == 0) {
        b = true;
-   else if (len == len_false && strncasecmp(str, str_false, len) == 0)
+       str += len_true;
+   } else if (strncasecmp(str, str_false, len_false) == 0) {
        b = false;
-   else
+       str += len_false;
+   } else {
+       return -1;
+   }
+   for (; str[0] == ' '; str++);
+   if (str[0] != '\0')
        return -1;
    mem_set_bool(mem, b);
    return 0;
@@ -1036,11 +1035,14 @@ mem_cast_explicit(struct Mem *mem, enum field_type type)
    case FIELD_TYPE_INTEGER:
        return mem_to_int(mem);
    case FIELD_TYPE_BOOLEAN:
-       if (mem->type == MEM_TYPE_BOOL)
+       switch (mem->type) {
+       case MEM_TYPE_BOOL:
            return 0;
-       if (mem->type == MEM_TYPE_STR)
+       case MEM_TYPE_STR:
            return str_to_bool(mem);
-       return -1;
+       default:
+           return -1;
+       }
    case FIELD_TYPE_VARBINARY:
        if (mem->type == MEM_TYPE_STR)
            return str_to_bin(mem);


New patch:

commit dda7ad8134ef47d21006fdbc7cc5378de8220bd5
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Fri Jul 16 11:28:39 2021 +0300

    sql: disallow explicit cast of BOOLEAN to number
    
    This patch removes explicit cast of BOOLEAN values to numeric types and
    explicit cast of numeric values to BOOLEAN.
    
    Part of #4470

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index fa80f6d5a..aeaa01d94 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -634,17 +634,6 @@ int_to_str0(struct Mem *mem)
    return mem_copy_str0(mem, str);
 }
 
-static inline int
-int_to_bool(struct Mem *mem)
-{
-   assert((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0);
-   mem->u.b = mem->u.i != 0;
-   mem->type = MEM_TYPE_BOOL;
-   assert(mem->flags == 0);
-   mem->field_type = FIELD_TYPE_BOOLEAN;
-   return 0;
-}
-
 static inline int
 str_to_str0(struct Mem *mem)
 {
@@ -870,28 +859,6 @@ double_to_str0(struct Mem *mem)
    return 0;
 }
 
-static inline int
-double_to_bool(struct Mem *mem)
-{
-   assert(mem->type == MEM_TYPE_DOUBLE);
-   mem->u.b = mem->u.r != 0.;
-   mem->type = MEM_TYPE_BOOL;
-   assert(mem->flags == 0);
-   mem->field_type = FIELD_TYPE_BOOLEAN;
-   return 0;
-}
-
-static inline int
-bool_to_int(struct Mem *mem)
-{
-   assert(mem->type == MEM_TYPE_BOOL);
-   mem->u.u = (uint64_t)mem->u.b;
-   mem->type = MEM_TYPE_UINT;
-   assert(mem->flags == 0);
-   mem->field_type = FIELD_TYPE_UNSIGNED;
-   return 0;
-}
-
 static inline int
 bool_to_str0(struct Mem *mem)
 {
@@ -942,8 +909,6 @@ mem_to_int(struct Mem *mem)
        return bytes_to_int(mem);
    if (mem->type == MEM_TYPE_DOUBLE)
        return double_to_int(mem);
-   if (mem->type == MEM_TYPE_BOOL)
-       return bool_to_int(mem);
    return -1;
 }
 
@@ -979,8 +944,6 @@ mem_to_number(struct Mem *mem)
    assert(mem->type < MEM_TYPE_INVALID);
    if (mem_is_num(mem))
        return 0;
-   if (mem->type == MEM_TYPE_BOOL)
-       return bool_to_int(mem);
    if ((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0) {
        if (bytes_to_int(mem) == 0)
            return 0;
@@ -1062,8 +1025,6 @@ mem_cast_explicit(struct Mem *mem, enum field_type type)
            return bytes_to_uint(mem);
        case MEM_TYPE_DOUBLE:
            return double_to_int(mem);
-       case MEM_TYPE_BOOL:
-           return bool_to_int(mem);
        default:
            return -1;
        }
@@ -1077,13 +1038,8 @@ mem_cast_explicit(struct Mem *mem, enum field_type type)
        switch (mem->type) {
        case MEM_TYPE_BOOL:
            return 0;
-       case MEM_TYPE_INT:
-       case MEM_TYPE_UINT:
-           return int_to_bool(mem);
        case MEM_TYPE_STR:
            return str_to_bool(mem);
-       case MEM_TYPE_DOUBLE:
-           return double_to_bool(mem);
        default:
            return -1;
        }
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 8e7b6640e..3dc49c38e 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(79)
+test:plan(91)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -877,4 +877,104 @@ test:do_execsql_test(
         -- </cast-5.1>
     })
 
+-- gh-4470: Make explicit casts work according to our rules.
+
+-- Make sure that explicit cast from BOOLEAN to numeric types throws an error.
+test:do_catchsql_test(
+    "cast-6.1.1",
+    [[
+        SELECT CAST(TRUE AS UNSIGNED);
+    ]], {
+        1, "Type mismatch: can not convert boolean(TRUE) to unsigned"
+    })
+
+test:do_catchsql_test(
+    "cast-6.1.2",
+    [[
+        SELECT CAST(FALSE AS UNSIGNED);
+    ]], {
+        1, "Type mismatch: can not convert boolean(FALSE) to unsigned"
+    })
+
+test:do_catchsql_test(
+    "cast-6.1.3",
+    [[
+        SELECT CAST(TRUE AS INTEGER);
+    ]], {
+        1, "Type mismatch: can not convert boolean(TRUE) to integer"
+    })
+
+test:do_catchsql_test(
+    "cast-6.1.4",
+    [[
+        SELECT CAST(FALSE AS INTEGER);
+    ]], {
+        1, "Type mismatch: can not convert boolean(FALSE) to integer"
+    })
+
+test:do_catchsql_test(
+    "cast-6.1.5",
+    [[
+        SELECT CAST(TRUE AS DOUBLE);
+    ]], {
+        1, "Type mismatch: can not convert boolean(TRUE) to double"
+    })
+
+test:do_catchsql_test(
+    "cast-6.1.6",
+    [[
+        SELECT CAST(FALSE AS DOUBLE);
+    ]], {
+        1, "Type mismatch: can not convert boolean(FALSE) to double"
+    })
+
+test:do_catchsql_test(
+    "cast-6.1.7",
+    [[
+        SELECT CAST(TRUE AS NUMBER);
+    ]], {
+        1, "Type mismatch: can not convert boolean(TRUE) to number"
+    })
+
+test:do_catchsql_test(
+    "cast-6.1.8",
+    [[
+        SELECT CAST(FALSE AS NUMBER);
+    ]], {
+        1, "Type mismatch: can not convert boolean(FALSE) to number"
+    })
+
+-- Make sure that explicit cast numeric value to BOOLEAN throws an error.
+test:do_catchsql_test(
+    "cast-6.2.1",
+    [[
+        SELECT CAST(0 AS BOOLEAN);
+    ]], {
+        1, "Type mismatch: can not convert integer(0) to boolean"
+    })
+
+test:do_catchsql_test(
+    "cast-6.2.2",
+    [[
+        SELECT CAST(-1 AS BOOLEAN);
+    ]], {
+        1, "Type mismatch: can not convert integer(-1) to boolean"
+    })
+
+test:do_catchsql_test(
+    "cast-6.2.3",
+    [[
+        SELECT CAST(1.5 AS BOOLEAN);
+    ]], {
+        1, "Type mismatch: can not convert double(1.5) to boolean"
+    })
+
+test:do_catchsql_test(
+    "cast-6.2.4",
+    [[
+        SELECT CAST(CAST(1 AS NUMBER) AS BOOLEAN);
+    ]], {
+        1, "Type mismatch: can not convert integer(1) to boolean"
+    })
+
 test:finish_test()
diff --git a/test/sql-tap/cse.test.lua b/test/sql-tap/cse.test.lua
index e09f955a0..665633aed 100755
--- a/test/sql-tap/cse.test.lua
+++ b/test/sql-tap/cse.test.lua
@@ -31,11 +31,11 @@ test:do_test(
             INSERT INTO t1 VALUES(2,21,22,23,24,25);
         ]]
         return test:execsql [[
-            SELECT b, -b, ~b, NOT CAST(b AS BOOLEAN), NOT NOT CAST(b AS BOOLEAN), b-b, b+b, b*b, b/b, b FROM t1
+            SELECT b, -b, ~b, b - b, b + b, b * b, b / b, b FROM t1;
         ]]
     end, {
         -- <cse-1.1>
-        11, -11, -12, false, true, 0, 22, 121, 1, 11, 21, -21, -22, false, true, 0, 42, 441, 1, 21
+        11, -11, -12, 0, 22, 121, 1, 11, 21, -21, -22, 0, 42, 441, 1, 21
         -- </cse-1.1>
     })
 
@@ -102,7 +102,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cse-1.6.3",
     [[
-        SELECT CASE WHEN CAST(b AS BOOLEAN) THEN d WHEN CAST(e AS BOOLEAN)  THEN f ELSE 999 END, b, c, d FROM t1
+        SELECT CASE WHEN b != 0 THEN d WHEN e != 0 THEN f ELSE 999 END, b, c, d FROM t1;
     ]], {
         -- <cse-1.6.3>
         13, 11, 12, 13, 23, 21, 22, 23
@@ -112,7 +112,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cse-1.6.4",
     [[
-        SELECT b, c, d, CASE WHEN CAST(b AS BOOLEAN) THEN d WHEN CAST(e AS BOOLEAN) THEN f ELSE 999 END FROM t1
+        SELECT b, c, d, CASE WHEN b != 0 THEN d WHEN e != 0 THEN f ELSE 999 END FROM t1;
     ]], {
         -- <cse-1.6.4>
         11, 12, 13, 13, 21, 22, 23, 23
@@ -122,7 +122,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cse-1.6.5",
     [[
-        SELECT b, c, d, CASE WHEN false THEN d WHEN CAST(e AS BOOLEAN) THEN f ELSE 999 END FROM t1
+        SELECT b, c, d, CASE WHEN false THEN d WHEN e != 0 THEN f ELSE 999 END FROM t1;
     ]], {
         -- <cse-1.6.5>
         11, 12, 13, 15, 21, 22, 23, 25
@@ -132,10 +132,10 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cse-1.7",
     [[
-        SELECT a, -a, ~a, NOT CAST(a AS BOOLEAN), NOT NOT CAST(a AS BOOLEAN), a-a, a+a, a*a, a/a, a FROM t1
+        SELECT a, -a, ~a, a - a, a + a, a * a, a / a, a FROM t1
     ]], {
         -- <cse-1.7>
-        1, -1 ,-2, false, true, 0, 2, 1, 1, 1, 2, -2, -3, false, true, 0, 4, 4, 1, 2
+        1, -1 ,-2, 0, 2, 1, 1, 1, 2, -2, -3, 0, 4, 4, 1, 2
         -- </cse-1.7>
     })
 
@@ -152,10 +152,10 @@ test:do_execsql_test(
 test:do_execsql_test(
     "cse-1.9",
     [[
-        SELECT NOT CAST(b AS BOOLEAN), ~b, NOT NOT CAST(b AS BOOLEAN), b FROM t1
+        SELECT ~b, b FROM t1
     ]], {
         -- <cse-1.9>
-        false, -12, true, 11, false, -22, true, 21
+        -12, 11, -22, 21
         -- </cse-1.9>
     })
 
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index ab0faa376..8293965b8 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -910,7 +910,8 @@ test:do_select_tests(
 
         {"3", "SELECT sum(b+1) FROM z1 NATURAL LEFT JOIN z3", {-43.06}},
         {"4", "SELECT sum(b+2) FROM z1 NATURAL LEFT JOIN z3", {-38.06}},
-        {"5", "SELECT sum(CAST(b IS NOT NULL AS INTEGER)) FROM z1 NATURAL LEFT JOIN z3", {5}},
+        {"5", "SELECT sum(CASE WHEN b IS NOT NULL THEN 1 ELSE 0 END) "..
+              "FROM z1 NATURAL LEFT JOIN z3", {5}},
     })
 
 -- EVIDENCE-OF: R-26684-40576 Each non-aggregate expression in the
diff --git a/test/sql-tap/in1.test.lua b/test/sql-tap/in1.test.lua
index 4b51da6e8..bdf1f207a 100755
--- a/test/sql-tap/in1.test.lua
+++ b/test/sql-tap/in1.test.lua
@@ -100,10 +100,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "in-1.7",
     [[
-        SELECT a+ 100*CAST((a BETWEEN 1 and 3) AS INTEGER) FROM t1 ORDER BY b
+        SELECT a, a BETWEEN 1 AND 3 FROM t1 ORDER BY b;
     ]], {
         -- <in-1.7>
-        101, 102, 103, 4, 5, 6, 7, 8, 9, 10
+        1, true, 2, true, 3, true, 4, false, 5, false,
+        6, false, 7, false, 8, false, 9, false, 10, false
         -- </in-1.7>
     })
 
@@ -157,10 +158,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "in-2.5",
     [[
-        SELECT a+100*(CAST(b IN (8,16,24) AS INTEGER)) FROM t1 ORDER BY b
+        SELECT b, b IN (8,16,24) FROM t1 ORDER BY b;
     ]], {
         -- <in-2.5>
-        1, 2, 103, 104, 5, 6, 7, 8, 9, 10
+        2, false, 4, false, 8, true, 16, true, 32, false, 64, false,
+        128, false, 256, false, 512, false, 1024, false
         -- </in-2.5>
     })
 
@@ -207,7 +209,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "in-2.10",
     [[
-        SELECT a FROM t1 WHERE LEAST(0, CAST(b IN (a,30) AS INT)) <> 0
+        SELECT a, b FROM t1 WHERE LEAST(TRUE, b IN (a, 30));
     ]], {
         -- <in-2.10>
 
@@ -253,10 +255,11 @@ test:do_execsql_test(
 test:do_execsql_test(
     "in-3.3",
     [[
-        SELECT a + 100*(CAST(b IN (SELECT b FROM t1 WHERE a<5) AS INTEGER)) FROM t1 ORDER BY b
+        SELECT a, b, b IN (SELECT b FROM t1 WHERE a < 5) FROM t1 ORDER BY b;
     ]], {
         -- <in-3.3>
-        101, 102, 103, 104, 5, 6, 7, 8, 9, 10
+        1, 2, true, 2, 4, true, 3, 8, true, 4, 16, true, 5, 32, false, 6, 64,
+        false, 7, 128, false, 8, 256, false, 9, 512, false, 10, 1024, false
         -- </in-3.3>
     })
 
diff --git a/test/sql-tap/misc3.test.lua b/test/sql-tap/misc3.test.lua
index 313484b5d..9add3dd9e 100755
--- a/test/sql-tap/misc3.test.lua
+++ b/test/sql-tap/misc3.test.lua
@@ -510,7 +510,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "misc-8.2",
     [[
-        SELECT count(*) FROM t3 WHERE 1+CAST((b IN ('abc','xyz')) AS INTEGER)==2
+        SELECT count(*) FROM t3 WHERE b IN ('abc','xyz');
     ]], {
         -- <misc-8.2>
         2
diff --git a/test/sql/boolean.result b/test/sql/boolean.result
index 5594d92cb..d54de8fe7 100644
--- a/test/sql/boolean.result
+++ b/test/sql/boolean.result
@@ -500,25 +500,25 @@ INSERT INTO t3 VALUES (4, false)
  | ...
 
 -- Check CAST from BOOLEAN to the other types.
-SELECT cast(true AS INTEGER), cast(false AS INTEGER);
+SELECT cast(true AS INTEGER);
  | ---
- | - metadata:
- |   - name: COLUMN_1
- |     type: integer
- |   - name: COLUMN_2
- |     type: integer
- |   rows:
- |   - [1, 0]
+ | - null
+ | - 'Type mismatch: can not convert boolean(TRUE) to integer'
  | ...
-SELECT cast(true AS NUMBER), cast(false AS NUMBER);
+SELECT cast(false AS INTEGER);
  | ---
- | - metadata:
- |   - name: COLUMN_1
- |     type: number
- |   - name: COLUMN_2
- |     type: number
- |   rows:
- |   - [1, 0]
+ | - null
+ | - 'Type mismatch: can not convert boolean(FALSE) to integer'
+ | ...
+SELECT cast(true AS NUMBER);
+ | ---
+ | - null
+ | - 'Type mismatch: can not convert boolean(TRUE) to number'
+ | ...
+SELECT cast(false AS NUMBER);
+ | ---
+ | - null
+ | - 'Type mismatch: can not convert boolean(FALSE) to number'
  | ...
 -- gh-4462: ensure that text representation is uppercase.
 SELECT cast(true AS TEXT), cast(false AS TEXT);
@@ -543,27 +543,30 @@ SELECT cast(true AS BOOLEAN), cast(false AS BOOLEAN);
  | ...
 
 -- Check CAST to BOOLEAN from the other types.
-SELECT cast(100 AS BOOLEAN), cast(1 AS BOOLEAN), cast(0 AS BOOLEAN);
+SELECT cast(100 AS BOOLEAN);
  | ---
- | - metadata:
- |   - name: COLUMN_1
- |     type: boolean
- |   - name: COLUMN_2
- |     type: boolean
- |   - name: COLUMN_3
- |     type: boolean
- |   rows:
- |   - [true, true, false]
+ | - null
+ | - 'Type mismatch: can not convert integer(100) to boolean'
  | ...
-SELECT cast(0.123 AS BOOLEAN), cast(0.0 AS BOOLEAN);
+SELECT cast(1 AS BOOLEAN);
  | ---
- | - metadata:
- |   - name: COLUMN_1
- |     type: boolean
- |   - name: COLUMN_2
- |     type: boolean
- |   rows:
- |   - [true, false]
+ | - null
+ | - 'Type mismatch: can not convert integer(1) to boolean'
+ | ...
+SELECT cast(0 AS BOOLEAN);
+ | ---
+ | - null
+ | - 'Type mismatch: can not convert integer(0) to boolean'
+ | ...
+SELECT cast(0.123 AS BOOLEAN);
+ | ---
+ | - null
+ | - 'Type mismatch: can not convert double(0.123) to boolean'
+ | ...
+SELECT cast(0.0 AS BOOLEAN);
+ | ---
+ | - null
+ | - 'Type mismatch: can not convert double(0.0) to boolean'
  | ...
 SELECT cast('true' AS BOOLEAN), cast('false' AS BOOLEAN);
  | ---
diff --git a/test/sql/boolean.test.sql b/test/sql/boolean.test.sql
index 65a830349..13991ca0c 100644
--- a/test/sql/boolean.test.sql
+++ b/test/sql/boolean.test.sql
@@ -127,15 +127,20 @@ INSERT INTO t3 VALUES (3, true)
 INSERT INTO t3 VALUES (4, false)
 
 -- Check CAST from BOOLEAN to the other types.
-SELECT cast(true AS INTEGER), cast(false AS INTEGER);
-SELECT cast(true AS NUMBER), cast(false AS NUMBER);
+SELECT cast(true AS INTEGER);
+SELECT cast(false AS INTEGER);
+SELECT cast(true AS NUMBER);
+SELECT cast(false AS NUMBER);
 -- gh-4462: ensure that text representation is uppercase.
 SELECT cast(true AS TEXT), cast(false AS TEXT);
 SELECT cast(true AS BOOLEAN), cast(false AS BOOLEAN);
 
 -- Check CAST to BOOLEAN from the other types.
-SELECT cast(100 AS BOOLEAN), cast(1 AS BOOLEAN), cast(0 AS BOOLEAN);
-SELECT cast(0.123 AS BOOLEAN), cast(0.0 AS BOOLEAN);
+SELECT cast(100 AS BOOLEAN);
+SELECT cast(1 AS BOOLEAN);
+SELECT cast(0 AS BOOLEAN);
+SELECT cast(0.123 AS BOOLEAN);
+SELECT cast(0.0 AS BOOLEAN);
 SELECT cast('true' AS BOOLEAN), cast('false' AS BOOLEAN);
 SELECT cast('TRUE' AS BOOLEAN), cast('FALSE' AS BOOLEAN);
 
diff --git a/test/sql/types.result b/test/sql/types.result
index a0b8668be..f8e9781f2 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -1035,11 +1035,8 @@ box.execute("SELECT CAST(18446744073709551615 AS SCALAR);")
 ...
 box.execute("SELECT CAST(18446744073709551615 AS BOOLEAN);")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: boolean
-  rows:
-  - [true]
+- null
+- 'Type mismatch: can not convert integer(18446744073709551615) to boolean'
 ...
 box.execute("SELECT CAST('18446744073709551615' AS INTEGER);")
 ---
@@ -1105,11 +1102,8 @@ box.execute("SELECT CAST(-1.5 AS UNSIGNED);")
 ...
 box.execute("SELECT CAST(true AS UNSIGNED);")
 ---
-- metadata:
-  - name: COLUMN_1
-    type: unsigned
-  rows:
-  - [1]
+- null
+- 'Type mismatch: can not convert boolean(TRUE) to unsigned'
 ...
 box.execute("SELECT CAST('123' AS UNSIGNED);")
 ---

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number
  2021-07-27 11:31 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 1/3] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
@ 2021-07-27 11:31 ` Mergen Imeev via Tarantool-patches
  2021-07-29 20:58   ` Vladislav Shpilevoy via Tarantool-patches
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 3/3] sql: fix STRING to BOOLEAN explicit cast Mergen Imeev via Tarantool-patches
  2 siblings, 1 reply; 11+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-27 11:31 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

Thank you for the review! My answers, diff and new patch below.

On 26.07.2021 23:12, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> See 3 comments below.
>
> On 21.07.2021 17:10, Mergen Imeev via Tarantool-patches wrote:
>> This patch removes explicit cast of VARBINARY values to numeric types.
>>
>> Part of #4470
>> Closes #4772
>> Closes #5852
>
> 1. You might need to add a changelog file.
>
I added in next commit, however, if you find it too general please let me know.
I will make it more specific.

>> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
>> index 3dc49c38e..379fbf09e 100755
>> --- a/test/sql-tap/cast.test.lua
>> +++ b/test/sql-tap/cast.test.lua
>> @@ -867,13 +879,14 @@ test:do_test(
>>  -- gh-4356: Check that result of blob to number cast if of type
>>  -- number.
>
> 2. This comment could mean the test becomes just wrong and can
> be deleted.
>
Removed.

>> diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
>> index 56b11da25..a2877167f 100755
>> --- a/test/sql-tap/numcast.test.lua
>> +++ b/test/sql-tap/numcast.test.lua
>> @@ -149,20 +149,22 @@ test:do_execsql_test(
>>  -- gh-4233: Make sure that NUMBER can contain UNSIGNED, INTEGER
>>  -- and DOUBLE and is not automatically converted to DOUBLE.
>>  --
>
> 3. These tests seem to be quite far from this comment. Perhaps they
> are broken for some time already. Might worth deleting them.
>
I looked at the commit that added these tests and the tests are the same. Still,
I removed ones with VARBINARY since they are useless now.

>> -test:do_execsql_test(
>> +test:do_catchsql_test(
>>      "numcast-3.1",
>>      [[
>>          SELECT CAST(x'3131313131313131313131313131313131313131' AS NUMBER);
>>      ]], {
>> -        11111111111111111111ULL
>> +        1, "Type mismatch: can not convert "..
>> +           "varbinary(x'3131313131313131313131313131313131313131') to number"
>>      })
>>  
>> -test:do_execsql_test(
>> +test:do_catchsql_test(
>>      "numcast-3.2",
>>      [[
>>          SELECT CAST(x'31313131313131313131313131313131313131312E' AS NUMBER);
>>      ]], {
>> -        11111111111111110656
>> +        1, "Type mismatch: can not convert "..
>> +           "varbinary(x'31313131313131313131313131313131313131312E') to number"
>>      })
>>  
>>  test:do_execsql_test(
>>

Diff:

diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 379fbf09e..997298693 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(95)
+test:plan(94)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -875,21 +875,6 @@ test:do_test(
         -- </cast-4.4>
     })
 
-
--- gh-4356: Check that result of blob to number cast if of type
--- number.
---
-test:do_catchsql_test(
-    "cast-5.1",
-    [[
-        SELECT CAST(x'3138343436373434303733372e33' AS NUMBER)
-    ]], {
-        -- <cast-5.1>
-        1, "Type mismatch: can not convert "..
-           "varbinary(x'3138343436373434303733372E33') to number"
-        -- </cast-5.1>
-    })
-
 -- gh-4470: Make explicit casts work according to our rules.
 
 -- Make sure that explicit cast from BOOLEAN to numeric types throws an error.
diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
index 8f738c760..73d0de642 100755
--- a/test/sql-tap/numcast.test.lua
+++ b/test/sql-tap/numcast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(31)
+test:plan(29)
 
 --!./tcltestrunner.lua
 -- 2013 March 20
@@ -149,24 +149,6 @@ test:do_execsql_test(
 -- gh-4233: Make sure that NUMBER can contain UNSIGNED, INTEGER
 -- and DOUBLE and is not automatically converted to DOUBLE.
 --
-test:do_catchsql_test(
-    "numcast-3.1",
-    [[
-        SELECT CAST(x'3131313131313131313131313131313131313131' AS NUMBER);
-    ]], {
-        1, "Type mismatch: can not convert "..
-           "varbinary(x'3131313131313131313131313131313131313131') to number"
-    })
-
-test:do_catchsql_test(
-    "numcast-3.2",
-    [[
-        SELECT CAST(x'31313131313131313131313131313131313131312E' AS NUMBER);
-    ]], {
-        1, "Type mismatch: can not convert "..
-           "varbinary(x'31313131313131313131313131313131313131312E') to number"
-    })
-
 test:do_execsql_test(
     "numcast-3.3",
     [[


New patch:

commit 8d3502b3cf8b1acbda0ecb76c03284c9edf2f76a
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Mon Jul 19 15:31:21 2021 +0300

    sql: disallow explicit cast of VARBINARY to number
    
    This patch removes explicit cast of VARBINARY values to numeric types.
    
    Part of #4470
    Closes #4772
    Closes #5852

diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index aeaa01d94..d8296faa5 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -735,9 +735,9 @@ bin_to_uuid(struct Mem *mem)
 }
 
 static inline int
-bytes_to_int(struct Mem *mem)
+str_to_int(struct Mem *mem)
 {
-   assert((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0);
+   assert(mem->type == MEM_TYPE_STR);
    bool is_neg;
    int64_t i;
    if (sql_atoi64(mem->z, &i, &is_neg, mem->n) != 0)
@@ -747,9 +747,9 @@ bytes_to_int(struct Mem *mem)
 }
 
 static inline int
-bytes_to_uint(struct Mem *mem)
+str_to_uint(struct Mem *mem)
 {
-   assert((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0);
+   assert(mem->type == MEM_TYPE_STR);
    bool is_neg;
    int64_t i;
    if (sql_atoi64(mem->z, &i, &is_neg, mem->n) != 0)
@@ -761,9 +761,9 @@ bytes_to_uint(struct Mem *mem)
 }
 
 static inline int
-bytes_to_double(struct Mem *mem)
+str_to_double(struct Mem *mem)
 {
-   assert((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0);
+   assert(mem->type == MEM_TYPE_STR);
    double d;
    if (sqlAtoF(mem->z, &d, mem->n) == 0)
        return -1;
@@ -905,8 +905,8 @@ mem_to_int(struct Mem *mem)
    assert(mem->type < MEM_TYPE_INVALID);
    if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
        return 0;
-   if ((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0)
-       return bytes_to_int(mem);
+   if (mem->type == MEM_TYPE_STR)
+       return str_to_int(mem);
    if (mem->type == MEM_TYPE_DOUBLE)
        return double_to_int(mem);
    return -1;
@@ -919,7 +919,7 @@ mem_to_int_precise(struct Mem *mem)
    if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
        return 0;
    if (mem->type == MEM_TYPE_STR)
-       return bytes_to_int(mem);
+       return str_to_int(mem);
    if (mem->type == MEM_TYPE_DOUBLE)
        return double_to_int_precise(mem);
    return -1;
@@ -934,7 +934,7 @@ mem_to_double(struct Mem *mem)
    if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
        return int_to_double(mem);
    if (mem->type == MEM_TYPE_STR)
-       return bytes_to_double(mem);
+       return str_to_double(mem);
    return -1;
 }
 
@@ -944,10 +944,10 @@ mem_to_number(struct Mem *mem)
    assert(mem->type < MEM_TYPE_INVALID);
    if (mem_is_num(mem))
        return 0;
-   if ((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0) {
-       if (bytes_to_int(mem) == 0)
+   if (mem->type == MEM_TYPE_STR) {
+       if (str_to_int(mem) == 0)
            return 0;
-       return bytes_to_double(mem);
+       return str_to_double(mem);
    }
    return -1;
 }
@@ -1021,8 +1021,7 @@ mem_cast_explicit(struct Mem *mem, enum field_type type)
        case MEM_TYPE_UINT:
            return 0;
        case MEM_TYPE_STR:
-       case MEM_TYPE_BIN:
-           return bytes_to_uint(mem);
+           return str_to_uint(mem);
        case MEM_TYPE_DOUBLE:
            return double_to_int(mem);
        default:
@@ -1158,7 +1157,7 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type type)
        if (mem->type == MEM_TYPE_DOUBLE)
            return double_to_uint_precise(mem);
        if (mem->type == MEM_TYPE_STR)
-           return bytes_to_uint(mem);
+           return str_to_uint(mem);
        return -1;
    case FIELD_TYPE_STRING:
        if ((mem->type & (MEM_TYPE_STR | MEM_TYPE_BIN)) != 0)
@@ -1182,7 +1181,7 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type type)
        if ((mem->type & (MEM_TYPE_INT | MEM_TYPE_UINT)) != 0)
            return 0;
        if (mem->type == MEM_TYPE_STR)
-           return bytes_to_int(mem);
+           return str_to_int(mem);
        if (mem->type == MEM_TYPE_DOUBLE)
            return double_to_int_precise(mem);
        return -1;
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 3dc49c38e..997298693 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(91)
+test:plan(94)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -565,23 +565,23 @@ test:do_catchsql_test(
         -- </case-1.66>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "case-1.68",
     [[
         SELECT CAST(x'31' AS NUMBER)
     ]], {
         -- <case-1.68>
-        1.0
+        1, "Type mismatch: can not convert varbinary(x'31') to number"
         -- </case-1.68>
     })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "case-1.69",
     [[
         SELECT typeof(CAST(x'31' AS NUMBER))
     ]], {
         -- <case-1.69>
-        "number"
+        1, "Type mismatch: can not convert varbinary(x'31') to number"
         -- </case-1.69>
     })
 
@@ -727,49 +727,61 @@ test:do_execsql_test(
 
 
 if true then --test:execsql("PRAGMA encoding")[1][1]=="UTF-8" then
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "cast-3.21",
         [[
             SELECT CAST(x'39323233333732303336383534373734383030' AS integer)
         ]], {
             -- <cast-3.21>
-            9223372036854774800LL
+            1, "Type mismatch: can not convert "..
+               "varbinary(x'39323233333732303336383534373734383030') to integer"
             -- </cast-3.21>
         })
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "cast-3.22",
         [[
             SELECT CAST(x'393232333337323033363835343737343830302E' AS NUMBER)
         ]], {
             -- <cast-3.22>
-            9223372036854774784
+            1, "Type mismatch: can not convert "..
+               "varbinary(x'393232333337323033363835343737343830302E') "..
+               "to number"
             -- </cast-3.22>
         })
 
-    test:do_execsql_test(
+    test:do_catchsql_test(
         "cast-3.24",
         [[
             SELECT CAST(CAST(x'39323233333732303336383534373734383030' AS NUMBER)
                         AS integer)
         ]], {
             -- <cast-3.24>
-            9223372036854774800LL
+            1, "Type mismatch: can not convert "..
+               "varbinary(x'39323233333732303336383534373734383030') to number"
             -- </cast-3.24>
         })
 end
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "case-3.25",
     [[
         SELECT CAST(x'31383434363734343037333730393535313631352E' AS NUMBER);
-    ]], { 1.844674407371e+19 } )
+    ]], {
+        1, "Type mismatch: can not convert "..
+           "varbinary(x'31383434363734343037333730393535313631352E') to number"
+    })
 
-test:do_execsql_test(
+test:do_catchsql_test(
     "case-3.26",
     [[
         SELECT CAST(x'3138343436373434303733373039353531363135' AS INT);
-    ]], { 18446744073709551615LL } )
+    ]], {
+        -- <cast-3.21>
+        1, "Type mismatch: can not convert "..
+           "varbinary(x'3138343436373434303733373039353531363135') to integer"
+        -- </cast-3.21>
+    })
 
 test:do_execsql_test(
     "case-3.31",
@@ -863,20 +875,6 @@ test:do_test(
         -- </cast-4.4>
     })
 
-
--- gh-4356: Check that result of blob to number cast if of type
--- number.
---
-test:do_execsql_test(
-    "cast-5.1",
-    [[
-        SELECT CAST(x'3138343436373434303733372e33' AS NUMBER)
-    ]], {
-        -- <cast-5.1>
-        184467440737.3
-        -- </cast-5.1>
-    })
-
 -- gh-4470: Make explicit casts work according to our rules.
 
 -- Make sure that explicit cast from BOOLEAN to numeric types throws an error.
@@ -977,4 +975,37 @@ test:do_catchsql_test(
         1, "Type mismatch: can not convert integer(1) to boolean"
     })
 
+-- Make sure that explicit cast from VARBINARY to numeric types throws an error.
+test:do_catchsql_test(
+    "cast-7.1.1",
+    [[
+        SELECT CAST(x'31' AS UNSIGNED);
+    ]], {
+        1, "Type mismatch: can not convert varbinary(x'31') to unsigned"
+    })
+
+test:do_catchsql_test(
+    "cast-7.1.2",
+    [[
+        SELECT CAST(x'31' AS INTEGER);
+    ]], {
+        1, "Type mismatch: can not convert varbinary(x'31') to integer"
+    })
+
+test:do_catchsql_test(
+    "cast-7.1.3",
+    [[
+        SELECT CAST(x'31' AS DOUBLE);
+    ]], {
+        1, "Type mismatch: can not convert varbinary(x'31') to double"
+    })
+
+test:do_catchsql_test(
+    "cast-7.1.4",
+    [[
+        SELECT CAST(x'31' AS NUMBER);
+    ]], {
+        1, "Type mismatch: can not convert varbinary(x'31') to number"
+    })
+
 test:finish_test()
diff --git a/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua b/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua
deleted file mode 100755
index a8cc0e770..000000000
--- a/test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua
+++ /dev/null
@@ -1,40 +0,0 @@
-#!/usr/bin/env tarantool
-local test = require("sqltester")
-test:plan(3)
-
---
--- Make sure that a blob as part of a tuple can be cast to NUMBER,
--- INTEGER and UNSIGNED. Prior to this patch, an error could
--- appear due to the absence of '\0' at the end of the BLOB.
---
-test:do_execsql_test(
-    "gh-4766-1",
-    [[
-        CREATE TABLE t1 (a VARBINARY PRIMARY KEY);
-        INSERT INTO t1 VALUES (X'33'), (X'372020202020');
-        SELECT a, CAST(a AS NUMBER), CAST(a AS INTEGER), CAST(a AS UNSIGNED) FROM t1;
-    ]], {
-        '3', 3, 3, 3, '7     ', 7, 7, 7
-    })
-
---
--- Make sure that BLOB longer than 12287 bytes cannot be cast to
--- INTEGER.
---
-local long_str = string.rep('0', 12284)
-test:do_execsql_test(
-    "gh-4766-2",
-    "SELECT CAST('" .. long_str .. "123'" .. " AS INTEGER);", {
-        123
-    })
-
-
-test:do_catchsql_test(
-    "gh-4766-3",
-    "SELECT CAST('" .. long_str .. "1234'" .. " AS INTEGER);", {
-        1, "Type mismatch: can not convert string('0000000000000000000000000" ..
-        "0000000000000000000000000000000000000000000000000000000000000000000" ..
-        "000000000000000000000000000000000000...) to integer"
-    })
-
-test:finish_test()
diff --git a/test/sql-tap/numcast.test.lua b/test/sql-tap/numcast.test.lua
index b3aa64e76..73d0de642 100755
--- a/test/sql-tap/numcast.test.lua
+++ b/test/sql-tap/numcast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(31)
+test:plan(29)
 
 --!./tcltestrunner.lua
 -- 2013 March 20
@@ -149,22 +149,6 @@ test:do_execsql_test(
 -- gh-4233: Make sure that NUMBER can contain UNSIGNED, INTEGER
 -- and DOUBLE and is not automatically converted to DOUBLE.
 --
-test:do_execsql_test(
-    "numcast-3.1",
-    [[
-        SELECT CAST(x'3131313131313131313131313131313131313131' AS NUMBER);
-    ]], {
-        11111111111111111111ULL
-    })
-
-test:do_execsql_test(
-    "numcast-3.2",
-    [[
-        SELECT CAST(x'31313131313131313131313131313131313131312E' AS NUMBER);
-    ]], {
-        11111111111111110656
-    })
-
 test:do_execsql_test(
     "numcast-3.3",
     [[

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v2 3/3] sql: fix STRING to BOOLEAN explicit cast
  2021-07-27 11:31 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 1/3] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
@ 2021-07-27 11:31 ` Mergen Imeev via Tarantool-patches
  2 siblings, 0 replies; 11+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-27 11:31 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

Prior to this patch, if a non-NULL-terminated string was cast to
BOOLEAN, the conversion always failed. Casting to BOOLEAN is now
independent of NULL termination.

Part of #4470
---
 .../unreleased/gh-4470-explicit-cast.md       |  3 +++
 src/box/sql/mem.c                             | 21 ++++++++++---------
 test/sql-tap/cast.test.lua                    | 11 +++++++++-
 3 files changed, 24 insertions(+), 11 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4470-explicit-cast.md

diff --git a/changelogs/unreleased/gh-4470-explicit-cast.md b/changelogs/unreleased/gh-4470-explicit-cast.md
new file mode 100644
index 000000000..8d291fa9b
--- /dev/null
+++ b/changelogs/unreleased/gh-4470-explicit-cast.md
@@ -0,0 +1,3 @@
+## feature/sql
+
+* Explicit cast now works according to defined rules (gh-4470).
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index d8296faa5..9b160cbc5 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -28,6 +28,8 @@
  * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
  * SUCH DAMAGE.
  */
+#include <ctype.h>
+
 #include "sqlInt.h"
 #include "mem.h"
 #include "vdbeInt.h"
@@ -672,24 +674,23 @@ str_to_bool(struct Mem *mem)
 {
 	assert(mem->type == MEM_TYPE_STR);
 	char *str = mem->z;
+	uint32_t len = mem->n;
 	bool b;
 	const char *str_true = "TRUE";
 	const char *str_false = "FALSE";
 	uint32_t len_true = strlen(str_true);
 	uint32_t len_false = strlen(str_false);
 
-	for (; str[0] == ' '; str++);
-	if (strncasecmp(str, str_true, len_true) == 0) {
+	for (; isspace(str[0]); str++, len--);
+	for (; isspace(str[len - 1]); len--);
+	if (len != len_true && len != len_false)
+		return -1;
+
+	if (len == len_true && strncasecmp(str, str_true, len) == 0)
 		b = true;
-		str += len_true;
-	} else if (strncasecmp(str, str_false, len_false) == 0) {
+	else if (len == len_false && strncasecmp(str, str_false, len) == 0)
 		b = false;
-		str += len_false;
-	} else {
-		return -1;
-	}
-	for (; str[0] == ' '; str++);
-	if (str[0] != '\0')
+	else
 		return -1;
 	mem_set_bool(mem, b);
 	return 0;
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 997298693..799bcc1a8 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 local test = require("sqltester")
-test:plan(94)
+test:plan(95)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -1008,4 +1008,13 @@ test:do_catchsql_test(
         1, "Type mismatch: can not convert varbinary(x'31') to number"
     })
 
+-- Make sure that not NULL-terminated can be cast to BOOLEAN.
+test:do_execsql_test(
+    "cast-8",
+    [[
+        SELECT CAST(substr('true       ', 0, 6) AS BOOLEAN);
+    ]], {
+        true
+    })
+
 test:finish_test()
-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number
  2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
@ 2021-07-29 20:58   ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 0 replies; 11+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-29 20:58 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Good job on the fixes!

On 27.07.2021 13:31, Mergen Imeev via Tarantool-patches wrote:
> Thank you for the review! My answers, diff and new patch below.
> 
> On 26.07.2021 23:12, Vladislav Shpilevoy wrote:
>> Thanks for the patch!
>>
>> See 3 comments below.
>>
>> On 21.07.2021 17:10, Mergen Imeev via Tarantool-patches wrote:
>>> This patch removes explicit cast of VARBINARY values to numeric types.
>>>
>>> Part of #4470
>>> Closes #4772
>>> Closes #5852
>>
>> 1. You might need to add a changelog file.
>>
> I added in next commit, however, if you find it too general please let me know.
> I will make it more specific.

Yes, it looks too vague, a user won't be able to understand anything when
see it in the changelog. Lets add more specifics. That BOOL <-> numbers
was banned, and VARBINARY <-> numbers. And mention the bug that "sometimes
a string couldn't be cast to boolean even if it was a correct boolean".

After that LGTM and you can send it to a next reviewer.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts
  2021-08-04  8:32 Mergen Imeev via Tarantool-patches
  2021-08-04 14:16 ` Kirill Yukhin via Tarantool-patches
@ 2021-08-05  9:48 ` Kirill Yukhin via Tarantool-patches
  1 sibling, 0 replies; 11+ messages in thread
From: Kirill Yukhin via Tarantool-patches @ 2021-08-05  9:48 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Hello,

On 04 авг 11:32, imeevma@tarantool.org wrote:
> This patch fixes the explicit cast to comply with the new rules. According to
> these rules, BOOLEAN and VARBINARY can no longer be cast to numeric types.
> 
> https://github.com/tarantool/tarantool/issues/4470
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4470-fix-explicit-cast

I've checked your patchset into 2.7, 2.8 and master.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches]  [PATCH v2 0/3] Fix explicit casts
  2021-08-04 14:16 ` Kirill Yukhin via Tarantool-patches
@ 2021-08-04 15:21   ` Vitaliia Ioffe via Tarantool-patches
  0 siblings, 0 replies; 11+ messages in thread
From: Vitaliia Ioffe via Tarantool-patches @ 2021-08-04 15:21 UTC (permalink / raw)
  To: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 846 bytes --]


 
Hi,  
 
QA LGTM
 
--
Vitaliia Ioffe
 
  
>Среда, 4 августа 2021, 17:16 +03:00 от Kirill Yukhin via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
> 
>Hello,
>
>On 04 авг 11:32,  imeevma@tarantool.org wrote:
>> This patch fixes the explicit cast to comply with the new rules. According to
>> these rules, BOOLEAN and VARBINARY can no longer be cast to numeric types.
>>
>>  https://github.com/tarantool/tarantool/issues/4470
>>  https://github.com/tarantool/tarantool/tree/imeevma/gh-4470-fix-explicit-cast
>>
>> Changes in v2:
>> - Change of str_to_bool() were moved in new patch.
>>
>> Mergen Imeev (3):
>> sql: disallow explicit cast of BOOLEAN to number
>> sql: disallow explicit cast of VARBINARY to number
>> sql: fix STRING to BOOLEAN explicit cast
>The patch set LGTM.
>
>--
>Regards, Kirill Yukhin
 

[-- Attachment #2: Type: text/html, Size: 1657 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts
  2021-08-04  8:32 Mergen Imeev via Tarantool-patches
@ 2021-08-04 14:16 ` Kirill Yukhin via Tarantool-patches
  2021-08-04 15:21   ` Vitaliia Ioffe via Tarantool-patches
  2021-08-05  9:48 ` Kirill Yukhin via Tarantool-patches
  1 sibling, 1 reply; 11+ messages in thread
From: Kirill Yukhin via Tarantool-patches @ 2021-08-04 14:16 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

Hello,

On 04 авг 11:32, imeevma@tarantool.org wrote:
> This patch fixes the explicit cast to comply with the new rules. According to
> these rules, BOOLEAN and VARBINARY can no longer be cast to numeric types.
> 
> https://github.com/tarantool/tarantool/issues/4470
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4470-fix-explicit-cast
> 
> Changes in v2:
>  - Change of str_to_bool() were moved in new patch.
> 
> Mergen Imeev (3):
>   sql: disallow explicit cast of BOOLEAN to number
>   sql: disallow explicit cast of VARBINARY to number
>   sql: fix STRING to BOOLEAN explicit cast

The patch set LGTM.

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts
@ 2021-08-04  8:32 Mergen Imeev via Tarantool-patches
  2021-08-04 14:16 ` Kirill Yukhin via Tarantool-patches
  2021-08-05  9:48 ` Kirill Yukhin via Tarantool-patches
  0 siblings, 2 replies; 11+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-04  8:32 UTC (permalink / raw)
  To: kyukhin; +Cc: tarantool-patches

This patch fixes the explicit cast to comply with the new rules. According to
these rules, BOOLEAN and VARBINARY can no longer be cast to numeric types.

https://github.com/tarantool/tarantool/issues/4470
https://github.com/tarantool/tarantool/tree/imeevma/gh-4470-fix-explicit-cast

Changes in v2:
 - Change of str_to_bool() were moved in new patch.

Mergen Imeev (3):
  sql: disallow explicit cast of BOOLEAN to number
  sql: disallow explicit cast of VARBINARY to number
  sql: fix STRING to BOOLEAN explicit cast

 .../unreleased/gh-4470-explicit-cast.md       |   8 +
 src/box/sql/mem.c                             |  98 +++------
 test/sql-tap/cast.test.lua                    | 186 +++++++++++++++---
 test/sql-tap/cse.test.lua                     |  18 +-
 test/sql-tap/e_select1.test.lua               |   3 +-
 ...-4766-wrong-cast-from-blob-to-int.test.lua |  40 ----
 test/sql-tap/in1.test.lua                     |  17 +-
 test/sql-tap/misc3.test.lua                   |  12 +-
 test/sql-tap/numcast.test.lua                 |  18 +-
 test/sql/boolean.result                       |  71 +++----
 test/sql/boolean.test.sql                     |  13 +-
 test/sql/types.result                         |  14 +-
 12 files changed, 271 insertions(+), 227 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4470-explicit-cast.md
 delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts
@ 2021-08-02 17:25 Mergen Imeev via Tarantool-patches
  0 siblings, 0 replies; 11+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-08-02 17:25 UTC (permalink / raw)
  To: imun; +Cc: tarantool-patches

This patch fixes the explicit cast to comply with the new rules. According to
these rules, BOOLEAN and VARBINARY can no longer be cast to numeric types.

https://github.com/tarantool/tarantool/issues/4470
https://github.com/tarantool/tarantool/tree/imeevma/gh-4470-fix-explicit-cast

Changes in v2:
 - Change of str_to_bool() were moved in new patch.

Mergen Imeev (3):
  sql: disallow explicit cast of BOOLEAN to number
  sql: disallow explicit cast of VARBINARY to number
  sql: fix STRING to BOOLEAN explicit cast

 .../unreleased/gh-4470-explicit-cast.md       |   8 +
 src/box/sql/mem.c                             |  98 +++------
 test/sql-tap/cast.test.lua                    | 186 +++++++++++++++---
 test/sql-tap/cse.test.lua                     |  18 +-
 test/sql-tap/e_select1.test.lua               |   3 +-
 ...-4766-wrong-cast-from-blob-to-int.test.lua |  40 ----
 test/sql-tap/in1.test.lua                     |  17 +-
 test/sql-tap/misc3.test.lua                   |  12 +-
 test/sql-tap/numcast.test.lua                 |  18 +-
 test/sql/boolean.result                       |  71 +++----
 test/sql/boolean.test.sql                     |  13 +-
 test/sql/types.result                         |  14 +-
 12 files changed, 271 insertions(+), 227 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4470-explicit-cast.md
 delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts
@ 2021-07-30  7:11 Mergen Imeev via Tarantool-patches
  0 siblings, 0 replies; 11+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-30  7:11 UTC (permalink / raw)
  To: tsafin; +Cc: tarantool-patches

This patch fixes the explicit cast to comply with the new rules. According to
these rules, BOOLEAN and VARBINARY values can no longer be cast to numeric
types, and numeric values can no longer be cast to BOOLEAN.

https://github.com/tarantool/tarantool/issues/4470
https://github.com/tarantool/tarantool/tree/imeevma/gh-4470-fix-explicit-cast

Changes in v2:
 - Change of str_to_bool() were moved in new patch.

Mergen Imeev (3):
  sql: disallow explicit cast of BOOLEAN to number
  sql: disallow explicit cast of VARBINARY to number
  sql: fix STRING to BOOLEAN explicit cast

 .../unreleased/gh-4470-explicit-cast.md       |   8 +
 src/box/sql/mem.c                             |  98 +++------
 test/sql-tap/cast.test.lua                    | 186 +++++++++++++++---
 test/sql-tap/cse.test.lua                     |  18 +-
 test/sql-tap/e_select1.test.lua               |   3 +-
 ...-4766-wrong-cast-from-blob-to-int.test.lua |  40 ----
 test/sql-tap/in1.test.lua                     |  17 +-
 test/sql-tap/misc3.test.lua                   |   2 +-
 test/sql-tap/numcast.test.lua                 |  18 +-
 test/sql/boolean.result                       |  71 +++----
 test/sql/boolean.test.sql                     |  13 +-
 test/sql/types.result                         |  14 +-
 12 files changed, 271 insertions(+), 217 deletions(-)
 create mode 100644 changelogs/unreleased/gh-4470-explicit-cast.md
 delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua

-- 
2.25.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-08-05  9:48 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-27 11:31 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches
2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 1/3] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
2021-07-29 20:58   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-27 11:31 ` [Tarantool-patches] [PATCH v2 3/3] sql: fix STRING to BOOLEAN explicit cast Mergen Imeev via Tarantool-patches
2021-07-30  7:11 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches
2021-08-02 17:25 Mergen Imeev via Tarantool-patches
2021-08-04  8:32 Mergen Imeev via Tarantool-patches
2021-08-04 14:16 ` Kirill Yukhin via Tarantool-patches
2021-08-04 15:21   ` Vitaliia Ioffe via Tarantool-patches
2021-08-05  9:48 ` Kirill Yukhin via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox