Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v1 0/2] Fix explicit casts
@ 2021-07-21 15:10 Mergen Imeev via Tarantool-patches
  2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
  2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
  0 siblings, 2 replies; 5+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-21 15:10 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

Mergen Imeev (2):
  sql: disallow explicit cast of BOOLEAN to number
  sql: disallow explicit cast of VARBINARY to number

 src/box/sql/mem.c                             | 107 +++--------
 test/sql-tap/cast.test.lua                    | 180 ++++++++++++++++--
 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                 |  10 +-
 test/sql/boolean.result                       |  71 +++----
 test/sql/boolean.test.sql                     |  13 +-
 test/sql/types.result                         |  14 +-
 11 files changed, 271 insertions(+), 204 deletions(-)
 delete mode 100755 test/sql-tap/gh-4766-wrong-cast-from-blob-to-int.test.lua

-- 
2.25.1


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

* [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number
  2021-07-21 15:10 [Tarantool-patches] [PATCH v1 0/2] Fix explicit casts Mergen Imeev via Tarantool-patches
@ 2021-07-21 15:10 ` Mergen Imeev via Tarantool-patches
  2021-07-26 20:11   ` Vladislav Shpilevoy via Tarantool-patches
  2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
  1 sibling, 1 reply; 5+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-21 15:10 UTC (permalink / raw)
  To: v.shpilevoy; +Cc: tarantool-patches

This patch removes explicit cast of BOOLEAN values to numeric types and
explicit cast of numeric values to BOOLEAN.

Part of #4470
---
 src/box/sql/mem.c               |  74 +++++------------------
 test/sql-tap/cast.test.lua      | 102 +++++++++++++++++++++++++++++++-
 test/sql-tap/cse.test.lua       |  18 +++---
 test/sql-tap/e_select1.test.lua |   3 +-
 test/sql-tap/in1.test.lua       |  17 +++---
 test/sql-tap/misc3.test.lua     |   2 +-
 test/sql/boolean.result         |  71 +++++++++++-----------
 test/sql/boolean.test.sql       |  13 ++--
 test/sql/types.result           |  14 ++---
 9 files changed, 187 insertions(+), 127 deletions(-)

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
@@ -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"
@@ -634,17 +636,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)
 {
@@ -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;
 	mem_set_bool(mem, b);
 	return 0;
@@ -870,28 +860,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 +910,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 +945,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 +1026,6 @@ mem_cast_explicit(struct Mem *mem, enum field_type type)
 			return bytes_to_uint(mem);
 		case MEM_TYPE_DOUBLE:
 			return double_to_uint(mem);
-		case MEM_TYPE_BOOL:
-			return bool_to_int(mem);
 		default:
 			return -1;
 		}
@@ -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;
-		}
+		return -1;
 	case FIELD_TYPE_VARBINARY:
 		if (mem->type == MEM_TYPE_STR)
 			return str_to_bin(mem);
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 c1e1a8ef1..8da94d126 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);")
 ---
@@ -1102,11 +1099,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);")
 ---
-- 
2.25.1


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

* [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY to number
  2021-07-21 15:10 [Tarantool-patches] [PATCH v1 0/2] Fix explicit casts Mergen Imeev via Tarantool-patches
  2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
@ 2021-07-21 15:10 ` Mergen Imeev via Tarantool-patches
  2021-07-26 20:12   ` Vladislav Shpilevoy via Tarantool-patches
  1 sibling, 1 reply; 5+ messages in thread
From: Mergen Imeev via Tarantool-patches @ 2021-07-21 15:10 UTC (permalink / raw)
  To: v.shpilevoy; +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                    | 80 +++++++++++++++----
 ...-4766-wrong-cast-from-blob-to-int.test.lua | 40 ----------
 test/sql-tap/numcast.test.lua                 | 10 ++-
 4 files changed, 85 insertions(+), 78 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 5c44bfdfc..e75392834 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -736,9 +736,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)
@@ -748,9 +748,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)
@@ -762,9 +762,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;
@@ -906,8 +906,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;
@@ -920,7 +920,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;
@@ -935,7 +935,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;
 }
 
@@ -945,10 +945,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;
 }
@@ -1022,8 +1022,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:
@@ -1156,7 +1155,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)
@@ -1180,7 +1179,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..379fbf09e 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(95)
 
 --!./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",
@@ -867,13 +879,14 @@ test:do_test(
 -- gh-4356: Check that result of blob to number cast if of type
 -- number.
 --
-test:do_execsql_test(
+test:do_catchsql_test(
     "cast-5.1",
     [[
         SELECT CAST(x'3138343436373434303733372e33' AS NUMBER)
     ]], {
         -- <cast-5.1>
-        184467440737.3
+        1, "Type mismatch: can not convert "..
+           "varbinary(x'3138343436373434303733372E33') to number"
         -- </cast-5.1>
     })
 
@@ -977,4 +990,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 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.
 --
-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(
-- 
2.25.1


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

* Re: [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number
  2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
@ 2021-07-26 20:11   ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 0 replies; 5+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-26 20:11 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

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?

>  	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.

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

* Re: [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY to number
  2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
@ 2021-07-26 20:12   ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 0 replies; 5+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-07-26 20:12 UTC (permalink / raw)
  To: imeevma; +Cc: tarantool-patches

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.

> 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.

> 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.

> -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(
> 

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

end of thread, other threads:[~2021-07-26 20:13 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-21 15:10 [Tarantool-patches] [PATCH v1 0/2] Fix explicit casts Mergen Imeev via Tarantool-patches
2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 1/2] sql: disallow explicit cast of BOOLEAN to number Mergen Imeev via Tarantool-patches
2021-07-26 20:11   ` Vladislav Shpilevoy via Tarantool-patches
2021-07-21 15:10 ` [Tarantool-patches] [PATCH v1 2/2] sql: disallow explicit cast of VARBINARY " Mergen Imeev via Tarantool-patches
2021-07-26 20:12   ` Vladislav Shpilevoy via Tarantool-patches

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git