* [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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ 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; 8+ 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] 8+ messages in thread
* [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts @ 2021-07-30 7:11 Mergen Imeev via Tarantool-patches 2021-07-30 7:11 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number Mergen Imeev via Tarantool-patches 0 siblings, 1 reply; 8+ 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] 8+ messages in thread
* [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number 2021-07-30 7:11 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches @ 2021-07-30 7:11 ` Mergen Imeev via Tarantool-patches 0 siblings, 0 replies; 8+ messages in thread From: Mergen Imeev via Tarantool-patches @ 2021-07-30 7:11 UTC (permalink / raw) To: tsafin; +Cc: tarantool-patches This patch removes explicit cast of VARBINARY values to numeric types. Part of #4470 Closes #4772 Closes #5852 --- src/box/sql/mem.c | 33 ++++--- test/sql-tap/cast.test.lua | 89 +++++++++++++------ ...-4766-wrong-cast-from-blob-to-int.test.lua | 40 --------- test/sql-tap/numcast.test.lua | 18 +--- 4 files changed, 77 insertions(+), 103 deletions(-) delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 8f56cd289..985438663 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_uint(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 20aea3c4b..802fe712c 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(32) +test:plan(30) --!./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", [[ -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts @ 2021-08-02 17:25 Mergen Imeev via Tarantool-patches 2021-08-02 17:25 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number Mergen Imeev via Tarantool-patches 0 siblings, 1 reply; 8+ 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] 8+ messages in thread
* [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number 2021-08-02 17:25 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches @ 2021-08-02 17:25 ` Mergen Imeev via Tarantool-patches 0 siblings, 0 replies; 8+ messages in thread From: Mergen Imeev via Tarantool-patches @ 2021-08-02 17:25 UTC (permalink / raw) To: imun; +Cc: tarantool-patches This patch removes explicit cast of VARBINARY values to numeric types. Part of #4470 Closes #4772 Closes #5852 --- src/box/sql/mem.c | 33 ++++--- test/sql-tap/cast.test.lua | 89 +++++++++++++------ ...-4766-wrong-cast-from-blob-to-int.test.lua | 40 --------- test/sql-tap/numcast.test.lua | 18 +--- 4 files changed, 77 insertions(+), 103 deletions(-) delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 8f56cd289..985438663 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_uint(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 20aea3c4b..802fe712c 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(32) +test:plan(30) --!./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", [[ -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ 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 8:32 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number Mergen Imeev via Tarantool-patches 0 siblings, 1 reply; 8+ 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] 8+ messages in thread
* [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number 2021-08-04 8:32 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches @ 2021-08-04 8:32 ` Mergen Imeev via Tarantool-patches 0 siblings, 0 replies; 8+ messages in thread From: Mergen Imeev via Tarantool-patches @ 2021-08-04 8:32 UTC (permalink / raw) To: kyukhin; +Cc: tarantool-patches This patch removes explicit cast of VARBINARY values to numeric types. Part of #4470 Closes #4772 Closes #5852 --- src/box/sql/mem.c | 33 ++++--- test/sql-tap/cast.test.lua | 89 +++++++++++++------ ...-4766-wrong-cast-from-blob-to-int.test.lua | 40 --------- test/sql-tap/numcast.test.lua | 18 +--- 4 files changed, 77 insertions(+), 103 deletions(-) delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index 8f56cd289..985438663 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_uint(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 20aea3c4b..802fe712c 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(32) +test:plan(30) --!./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", [[ -- 2.25.1 ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-04 8:33 UTC | newest] Thread overview: 8+ 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-07-30 7:11 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number Mergen Imeev via Tarantool-patches 2021-08-02 17:25 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches 2021-08-02 17:25 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number Mergen Imeev via Tarantool-patches 2021-08-04 8:32 [Tarantool-patches] [PATCH v2 0/3] Fix explicit casts Mergen Imeev via Tarantool-patches 2021-08-04 8:32 ` [Tarantool-patches] [PATCH v2 2/3] sql: disallow explicit cast of VARBINARY to number Mergen Imeev 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