[Tarantool-patches] [PATCH v3 0/2] sql: fix CAST() from BLOB to INTEGER

[Tarantool-patches] [PATCH v3 0/2] sql: fix CAST() from BLOB to INTEGER

[imeevma]

This patch-set fixes CAST() from BLOB to INTEGER in case a BLOB
does not have '\0'.

https://github.com/tarantool/tarantool/issues/4766
https://github.com/tarantool/tarantool/tree/imeevma/gh-4766-fix-blob-size-for-cast


Changelog:
 - CAST() from string contains DOUBLE value to INTEGER or UNSIGNED
   was disallowed.
 - Maximum length of a BLOB that is allowed to be cast to INTEGER
   or UNSIGNED was limited to 12287 bytes.
 - Fixed wrong behaviour of CAST() in case a BLOB does not have
   '\0'.


Mergen Imeev (2):
  sql: fix CAST() from STRING to INTEGER
  sql: add '\0' to the BLOB when it is cast to INTEGER

 src/box/sql/util.c                   | 17 +++++++++----
 src/box/sql/vdbe.c                   | 11 +++++++--
 src/box/sql/vdbeInt.h                |  1 -
 src/box/sql/vdbemem.c                | 45 +++++++++++----------------------
 test/sql-tap/cast.test.lua           | 48 +++++++++++++++++++++++++++++++++++-
 test/sql-tap/e_select1.test.lua      |  2 +-
 test/sql-tap/intpkey.test.lua        |  2 +-
 test/sql-tap/join.test.lua           |  4 +--
 test/sql-tap/subquery.test.lua       |  6 ++---
 test/sql-tap/tkt-9a8b09f8e6.test.lua |  4 +--
 test/sql/types.result                | 23 +++++++----------
 11 files changed, 100 insertions(+), 63 deletions(-)

-- 
2.7.4

[Tarantool-patches] [PATCH v3 1/2] sql: fix CAST() from STRING to INTEGER

[imeevma]

Prior to this patch, STRING, which contains the DOUBLE value,
could be cast to INTEGER. This was done by converting STRING to
DOUBLE and then converting this DOUBLE value to INTEGER. This may
affect the accuracy of CAST(), so it was forbidden.

Before patch:
tarantool> box.execute("SELECT CAST('1.1' as INTEGER);")
---
- metadata:
  - name: CAST('1.1' as INTEGER)
    type: integer
  rows:
  - [1]
...

After patch:
tarantool> box.execute("SELECT CAST('1.1' as INTEGER);")
---
- null
- 'Type mismatch: can not convert 1.1 to integer'
...

Part of #4766
---
 src/box/sql/vdbe.c                   | 11 +++++++--
 src/box/sql/vdbeInt.h                |  1 -
 src/box/sql/vdbemem.c                | 45 +++++++++++-------------------------
 test/sql-tap/e_select1.test.lua      |  2 +-
 test/sql-tap/intpkey.test.lua        |  2 +-
 test/sql-tap/join.test.lua           |  4 ++--
 test/sql-tap/subquery.test.lua       |  6 ++---
 test/sql-tap/tkt-9a8b09f8e6.test.lua |  4 ++--
 test/sql/types.result                | 23 ++++++++----------
 9 files changed, 41 insertions(+), 57 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index e8a029a..6c0e5bd 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -319,6 +319,8 @@ mem_apply_type(struct Mem *record, enum field_type type)
 	switch (type) {
 	case FIELD_TYPE_INTEGER:
 	case FIELD_TYPE_UNSIGNED:
+		if ((record->flags & (MEM_Bool | MEM_Blob)) != 0)
+			return -1;
 		if ((record->flags & MEM_UInt) == MEM_UInt)
 			return 0;
 		if ((record->flags & MEM_Real) == MEM_Real) {
@@ -331,8 +333,13 @@ mem_apply_type(struct Mem *record, enum field_type type)
 				mem_set_u64(record, u);
 			return 0;
 		}
-		if (sqlVdbeMemIntegerify(record) != 0)
-			return -1;
+		if ((record->flags & MEM_Str) != 0) {
+			bool is_neg;
+			int64_t i;
+			if (sql_atoi64(record->z, &i, &is_neg, record->n) != 0)
+				return -1;
+			mem_set_int(record, i, is_neg);
+		}
 		if ((record->flags & MEM_Int) == MEM_Int) {
 			if (type == FIELD_TYPE_UNSIGNED)
 				return -1;
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 38305ce..2c50b67 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -525,7 +525,6 @@ int sqlVdbeMemMakeWriteable(Mem *);
 int sqlVdbeMemStringify(Mem *);
 int sqlVdbeIntValue(Mem *, int64_t *, bool *is_neg);
 
-int sqlVdbeMemIntegerify(struct Mem *pMem);
 int sqlVdbeRealValue(Mem *, double *);
 
 int
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index aad030d..3bc7303 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -551,35 +551,6 @@ mem_apply_integer_type(Mem *pMem)
 }
 
 /*
- * Convert pMem to type integer.  Invalidate any prior representations.
- */
-int
-sqlVdbeMemIntegerify(struct Mem *pMem)
-{
-	assert(EIGHT_BYTE_ALIGNMENT(pMem));
-
-	int64_t i;
-	bool is_neg;
-	if (sqlVdbeIntValue(pMem, &i, &is_neg) == 0) {
-		mem_set_int(pMem, i, is_neg);
-		return 0;
-	}
-
-	double d;
-	if (sqlVdbeRealValue(pMem, &d) != 0)
-		return -1;
-	if (d < INT64_MAX && d >= INT64_MIN) {
-		mem_set_int(pMem, d, d <= -1);
-		return 0;
-	}
-	if (d >= INT64_MAX && d < UINT64_MAX) {
-		mem_set_u64(pMem, d);
-		return 0;
-	}
-	return -1;
-}
-
-/*
  * Convert pMem so that it is of type MEM_Real.
  * Invalidate any prior representations.
  */
@@ -696,7 +667,7 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 		return -1;
 	case FIELD_TYPE_INTEGER:
 	case FIELD_TYPE_UNSIGNED:
-		if ((pMem->flags & MEM_Blob) != 0) {
+		if ((pMem->flags & (MEM_Blob | MEM_Str)) != 0) {
 			bool is_neg;
 			int64_t val;
 			if (sql_atoi64(pMem->z, &val, &is_neg, pMem->n) != 0)
@@ -711,8 +682,20 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 			MemSetTypeFlag(pMem, MEM_UInt);
 			return 0;
 		}
-		if (sqlVdbeMemIntegerify(pMem) != 0)
+		if ((pMem->flags & MEM_Real) != 0) {
+			double d;
+			if (sqlVdbeRealValue(pMem, &d) != 0)
+				return -1;
+			if (d < INT64_MAX && d >= INT64_MIN) {
+				mem_set_int(pMem, d, d <= -1);
+				return 0;
+			}
+			if (d >= INT64_MAX && d < UINT64_MAX) {
+				mem_set_u64(pMem, d);
+				return 0;
+			}
 			return -1;
+		}
 		if (type == FIELD_TYPE_UNSIGNED &&
 		    (pMem->flags & MEM_UInt) == 0)
 			return -1;
diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua
index c1818dd..1d3b964 100755
--- a/test/sql-tap/e_select1.test.lua
+++ b/test/sql-tap/e_select1.test.lua
@@ -2195,7 +2195,7 @@ test:do_select_tests(
         {"1", "SELECT b FROM f1 ORDER BY a LIMIT 0 ", {}},
         {"2", "SELECT b FROM f1 ORDER BY a DESC LIMIT 4 ", {"z", "y", "x", "w"}},
         {"3", "SELECT b FROM f1 ORDER BY a DESC LIMIT 8 ", {"z", "y", "x", "w", "v", "u", "t", "s"}},
-        {"4", "SELECT b FROM f1 ORDER BY a DESC LIMIT '12.0' ", {"z", y, "x", "w", "v", "u", "t", "s", "r", "q", "p", "o"}},
+        {"4", "SELECT b FROM f1 ORDER BY a DESC LIMIT '12' ", {"z", y, "x", "w", "v", "u", "t", "s", "r", "q", "p", "o"}},
     })
 
 -- EVIDENCE-OF: R-54935-19057 Or, if the SELECT statement would return
diff --git a/test/sql-tap/intpkey.test.lua b/test/sql-tap/intpkey.test.lua
index bec2670..b6b1866 100755
--- a/test/sql-tap/intpkey.test.lua
+++ b/test/sql-tap/intpkey.test.lua
@@ -788,7 +788,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     "intpkey-13.2",
     [[
-        INSERT INTO t1 VALUES('1.0',2,3);
+        INSERT INTO t1 VALUES('1',2,3);
         SELECT * FROM t1 WHERE a=1;
     ]], {
         -- <intpkey-13.2>
diff --git a/test/sql-tap/join.test.lua b/test/sql-tap/join.test.lua
index 4f014e0..840b780 100755
--- a/test/sql-tap/join.test.lua
+++ b/test/sql-tap/join.test.lua
@@ -1017,7 +1017,7 @@ test:do_test(
         return test:execsql [[
             CREATE TABLE t1(a TEXT primary key, b TEXT);
             CREATE TABLE t2(b INTEGER primary key, a TEXT);
-            INSERT INTO t1 VALUES('one', '1.0');
+            INSERT INTO t1 VALUES('one', '1');
             INSERT INTO t1 VALUES('two', '2');
             INSERT INTO t2 VALUES(1, 'one');
             INSERT INTO t2 VALUES(2, 'two');
@@ -1034,7 +1034,7 @@ test:do_execsql_test(
         SELECT * FROM t1 NATURAL JOIN t2 
     ]], {
         -- <join-11.9>
-        "one", "1.0", "two", "2"
+        "one", "1", "two", "2"
         -- </join-11.9>
     })
 
diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua
index 6bedf58..15c4c82 100755
--- a/test/sql-tap/subquery.test.lua
+++ b/test/sql-tap/subquery.test.lua
@@ -342,7 +342,7 @@ test:do_execsql_test(
         INSERT INTO t3 VALUES(10);
 
         CREATE TABLE t4(x TEXT PRIMARY KEY);
-        INSERT INTO t4 VALUES('10.0');
+        INSERT INTO t4 VALUES('10');
     ]], {
         -- <subquery-2.5.1>
         
@@ -363,7 +363,7 @@ test:do_test(
         ]]
     end, {
         -- <subquery-2.5.2>
-        "10.0"
+        "10"
         -- </subquery-2.5.2>
     })
 
@@ -378,7 +378,7 @@ test:do_test(
         ]]
     end, {
         -- <subquery-2.5.3.1>
-        "10.0"
+        "10"
         -- </subquery-2.5.3.1>
     })
 
diff --git a/test/sql-tap/tkt-9a8b09f8e6.test.lua b/test/sql-tap/tkt-9a8b09f8e6.test.lua
index db0881c..cb5348a 100755
--- a/test/sql-tap/tkt-9a8b09f8e6.test.lua
+++ b/test/sql-tap/tkt-9a8b09f8e6.test.lua
@@ -196,7 +196,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     3.4,
     [[
-        SELECT x FROM t2 WHERE x IN ('1.0');
+        SELECT x FROM t2 WHERE x IN ('1');
     ]], {
         -- <3.4>
         1
@@ -236,7 +236,7 @@ test:do_execsql_test(
 test:do_execsql_test(
     3.8,
     [[
-        SELECT x FROM t2 WHERE '1.0' IN (x);
+        SELECT x FROM t2 WHERE '1' IN (x);
     ]], {
         -- <3.8>
         1
diff --git a/test/sql/types.result b/test/sql/types.result
index 38e4385..54aff46 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -269,11 +269,8 @@ box.space.T1:drop()
 --
 box.execute("SELECT CAST('1.123' AS INTEGER);")
 ---
-- metadata:
-  - name: CAST('1.123' AS INTEGER)
-    type: integer
-  rows:
-  - [1]
+- null
+- 'Type mismatch: can not convert 1.123 to integer'
 ...
 box.execute("CREATE TABLE t1 (f TEXT PRIMARY KEY);")
 ---
@@ -285,13 +282,8 @@ box.execute("INSERT INTO t1 VALUES('0.0'), ('1.5'), ('3.9312453');")
 ...
 box.execute("SELECT CAST(f AS INTEGER) FROM t1;")
 ---
-- metadata:
-  - name: CAST(f AS INTEGER)
-    type: integer
-  rows:
-  - [0]
-  - [1]
-  - [3]
+- null
+- 'Type mismatch: can not convert 0.0 to integer'
 ...
 box.space.T1:drop()
 ---
@@ -1105,8 +1097,11 @@ box.execute("SELECT CAST(1.5 AS UNSIGNED);")
 ...
 box.execute("SELECT CAST(-1.5 AS UNSIGNED);")
 ---
-- null
-- 'Type mismatch: can not convert -1 to unsigned'
+- metadata:
+  - name: CAST(-1.5 AS UNSIGNED)
+    type: unsigned
+  rows:
+  - [-1]
 ...
 box.execute("SELECT CAST(true AS UNSIGNED);")
 ---
-- 
2.7.4

[Tarantool-patches] [PATCH v3 2/2] sql: add '\0' to the BLOB when it is cast to INTEGER

[imeevma]

Hi! Thank you for review. My answers and new patch below.

On 3/11/20 7:15 PM, Nikita Pettik wrote:
> On 22 Feb 11:27, Mergen Imeev wrote:
>> Hi! Thank you for review. I changed a test once again.
>> Diff below.
>>
>> On Thu, Feb 20, 2020 at 10:58:21PM +0300, Nikita Pettik wrote:
>>> On 13 Feb 11:16, imeevma@tarantool.org wrote:
>>> So now you insert 0x33 instead of 1 to integer field. But how does it
>>> affect test? I failed to understand. In both cases you fetch and operate
>>> on blob, meanwhile integer field doesn't seem to be involved.
>>>
>> As I wrote in the last letter, we have a way to make sure
>> that with the first case everything will be in order,
>> without creating a duplicate of this binary value.
>> Obviously, that method will definitely not affect
>> performance. But it can lead to the part of the value that
>> looks like X'333300' being decoded as 33. See the example
>> from the last letter.
>>
>>
>> Diff:
>>
>> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
>> index 86c0fee..74844e0 100755
>> --- a/test/sql-tap/cast.test.lua
>> +++ b/test/sql-tap/cast.test.lua
>> @@ -891,13 +891,15 @@ test:do_execsql_test(
>>  
>>  --
>>  -- In some cases, the absence of '\0' could lead to an incorrect
>> --- result. Make sure this does not happen now.
>> +-- result. For example, in this case, part of the value is as
>> +-- follows: X'333300', which can be decoded as the number 33. Make
>> +-- sure this does not happen now.
>>  --
>>  test:do_execsql_test(
>>      "cast-6.2",
>>      [[
>> -        CREATE TABLE t (a VARBINARY PRIMARY KEY, i INT);
>> -        INSERT INTO t VALUES (X'33', 0x33);
>> +        CREATE TABLE t (a VARBINARY PRIMARY KEY, i INT, u INT);
>> +        INSERT INTO t VALUES (X'33', 0x33, 0x00);
>
> Still don't understand the purpose of creating separate table and so on.
I removed this test, though I still do not think that there is
something wrong with it.

> Again: next/prev fields don't affect content of field 'A': blob is
> stored in msgpack alongside with its length, so OP_Column can't decode
> more/less bytes than indicated in msgpack.
In new commit-message you can see example (also in previous
letters), from which it can be seen, that second and third field
affects result of the CAST().

>
> What is more, found that your implementation relies on tt_cstr() which
> uses static buffer which in turn restricted by 3 * 4096 bytes. So users
> may get wrong results of cast with ease. Example:
>
> long_str = string.rep('0', 15000) 
> long_str = long_str..'123'
> box.execute(string.format("insert into test values(2, '%s')", long_str)) 
> box.execute("select cast(s as INTEGER) from test")
>
> Result is 0 meanwhile should lead to error.
>
Fixed.


New patch:

From 0450959d670b1a466f7bc2c3dccb6a5f6ca0a0b8 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Wed, 25 Mar 2020 13:34:20 +0300
Subject: [PATCH] sql: add '\0' to the BLOB when it is cast to INTEGER

Prior to this patch, due to the absence of the '\0' character at
the end of the BLOB, it was possible to get an error or incorrect
result when using CAST() from BLOB to INTEGER or UNSIGNED. This
has now been fixed, but the maximum length of a BLOB that could be
cast to INTEGER or UNSIGNED was limited to 12287 bytes.

Examples of wrong CAST() from BLOB to INTEGER:

Error during CAST():
tarantool> box.execute("CREATE TABLE t1 (a VARBINARY PRIMARY KEY);")
---
- row_count: 1
...

tarantool> box.execute("INSERT INTO t1 VALUES (X'33'), (X'372020202020');")
---
- row_count: 2
...

tarantool> box.execute("SELECT a, CAST(a AS INTEGER) FROM t1;")
---
- null
- 'Type mismatch: can not convert varbinary to integer'
...

Wrong result:
tarantool> box.execute("CREATE TABLE t2 (a VARBINARY PRIMARY KEY, i INT, u INT);")
---
- row_count: 1
...

tarantool> box.execute("INSERT INTO t2 VALUES (X'33', 0x33, 0x00);")
---
- row_count: 1
...

tarantool> box.execute("SELECT a, CAST(a AS INTEGER) FROM t2;")
---
- metadata:
  - name: A
    type: varbinary
  - name: CAST(a AS INTEGER)
    type: integer
  rows:
  - ['3', 33]
...

Closes #4766

diff --git a/src/box/sql/util.c b/src/box/sql/util.c
index f908e9c..c556b98 100644
--- a/src/box/sql/util.c
+++ b/src/box/sql/util.c
@@ -467,14 +467,21 @@ sql_atoi64(const char *z, int64_t *val, bool *is_neg, int length)
 	if (*z == '-')
 		*is_neg = true;
 
+	/*
+	 * BLOB data may not end with '\0'. Because of this, the
+	 * strtoll() and strtoull() functions may return an
+	 * incorrect result. To fix this, let's copy the value for
+	 * decoding into static memory and add '\0' to it.
+	 */
+	if (length > SMALL_STATIC_SIZE - 1)
+		return -1;
+	const char *str_value = tt_cstr(z, length);
 	char *end = NULL;
 	errno = 0;
-	if (*z == '-') {
-		*is_neg = true;
-		*val = strtoll(z, &end, 10);
+	if (*is_neg) {
+		*val = strtoll(str_value, &end, 10);
 	} else {
-		*is_neg = false;
-		uint64_t u_val = strtoull(z, &end, 10);
+		uint64_t u_val = strtoull(str_value, &end, 10);
 		*val = u_val;
 	}
 	/* Overflow and underflow errors. */
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index fb0790d..42fdf81 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(79)
+test:plan(82)
 
 --!./tcltestrunner.lua
 -- 2005 June 25
@@ -871,4 +871,50 @@ test:do_execsql_test(
         -- </cast-5.1>
     })
 
+--
+-- gh-4766: 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(
+    "cast-6.1",
+    [[
+        CREATE TABLE t (a VARBINARY PRIMARY KEY);
+        INSERT INTO t VALUES (X'33'), (X'372020202020');
+        SELECT a, CAST(a AS NUMBER), CAST(a AS INTEGER), CAST(a AS UNSIGNED) FROM t;
+        DROP TABLE t;
+    ]], {
+        -- <cast-6.1>
+        '3', 3, 3, 3, '7     ', 7, 7, 7
+        -- </cast-6.1>
+    })
+
+--
+-- Make sure that BLOB longer than 12287 bytes cannot be cast to
+-- INTEGER.
+--
+long_str = string.rep('0', 12284)
+test:do_execsql_test(
+    "cast-6.2",
+    "SELECT CAST('" .. long_str .. "123'" .. " AS INTEGER);", {
+        -- <cast-6.2>
+        123
+        -- </cast-6.2>
+    })
+
+test:do_catchsql_test(
+    "cast-6.3",
+    "SELECT CAST('" .. long_str .. "1234'" .. " AS INTEGER);", {
+        -- <cast-6.3>
+        1, "Type mismatch: can not convert 000000000000000000000000000000000" ..
+        "0000000000000000000000000000000000000000000000000000000000000000000" ..
+        "0000000000000000000000000000000000000000000000000000000000000000000" ..
+        "0000000000000000000000000000000000000000000000000000000000000000000" ..
+        "0000000000000000000000000000000000000000000000000000000000000000000" ..
+        "0000000000000000000000000000000000000000000000000000000000000000000" ..
+        "0000000000000000000000000000000000000000000000000000000000000000000" ..
+        "000000000000000000000000000000000000000000000"
+        -- </cast-6.3>
+    })
+
 test:finish_test()

Re: [Tarantool-patches] [PATCH v3 0/2] sql: fix CAST() from BLOB to INTEGER

[Nikita Pettik]

On 25 Mar 14:38, imeevma@tarantool.org wrote:
> This patch-set fixes CAST() from BLOB to INTEGER in case a BLOB
> does not have '\0'.
> 
> https://github.com/tarantool/tarantool/issues/4766
> https://github.com/tarantool/tarantool/tree/imeevma/gh-4766-fix-blob-size-for-cast
> 
> 
> Changelog:
>  - CAST() from string contains DOUBLE value to INTEGER or UNSIGNED
>    was disallowed.
>  - Maximum length of a BLOB that is allowed to be cast to INTEGER
>    or UNSIGNED was limited to 12287 bytes.
>  - Fixed wrong behaviour of CAST() in case a BLOB does not have
>    '\0'.

@Changelog entry to release notes is missing.
 
> Mergen Imeev (2):
>   sql: fix CAST() from STRING to INTEGER
>   sql: add '\0' to the BLOB when it is cast to INTEGER
> 
>  src/box/sql/util.c                   | 17 +++++++++----
>  src/box/sql/vdbe.c                   | 11 +++++++--
>  src/box/sql/vdbeInt.h                |  1 -
>  src/box/sql/vdbemem.c                | 45 +++++++++++----------------------
>  test/sql-tap/cast.test.lua           | 48 +++++++++++++++++++++++++++++++++++-
>  test/sql-tap/e_select1.test.lua      |  2 +-
>  test/sql-tap/intpkey.test.lua        |  2 +-
>  test/sql-tap/join.test.lua           |  4 +--
>  test/sql-tap/subquery.test.lua       |  6 ++---
>  test/sql-tap/tkt-9a8b09f8e6.test.lua |  4 +--
>  test/sql/types.result                | 23 +++++++----------
>  11 files changed, 100 insertions(+), 63 deletions(-)
> 
> -- 
> 2.7.4
> 

Re: [Tarantool-patches] [PATCH v3 1/2] sql: fix CAST() from STRING to INTEGER

[Nikita Pettik]

On 25 Mar 14:38, imeevma@tarantool.org wrote:
> Prior to this patch, STRING, which contains the DOUBLE value,
> could be cast to INTEGER. This was done by converting STRING to
> DOUBLE and then converting this DOUBLE value to INTEGER. This may
> affect the accuracy of CAST(), so it was forbidden.
> 
> Before patch:
> tarantool> box.execute("SELECT CAST('1.1' as INTEGER);")
> ---
> - metadata:
>   - name: CAST('1.1' as INTEGER)
>     type: integer
>   rows:
>   - [1]
> ...
> 
> After patch:
> tarantool> box.execute("SELECT CAST('1.1' as INTEGER);")
> ---
> - null
> - 'Type mismatch: can not convert 1.1 to integer'
> ...

What about CAST('1.0' AS INTEGER)? Should this conversion be banned?
Moreover, your patch affects not only explicit cast. Please, split
it into two commits.

Doc. request is missing.

Re: [Tarantool-patches] [PATCH v3 2/2] sql: add '\0' to the BLOB when it is cast to INTEGER

[Nikita Pettik]

On 25 Mar 14:38, imeevma@tarantool.org wrote:
> Hi! Thank you for review. My answers and new patch below.
> 
> New patch:
> 
> From 0450959d670b1a466f7bc2c3dccb6a5f6ca0a0b8 Mon Sep 17 00:00:00 2001
> From: Mergen Imeev <imeevma@gmail.com>
> Date: Wed, 25 Mar 2020 13:34:20 +0300
> Subject: [PATCH] sql: add '\0' to the BLOB when it is cast to INTEGER
> 
> Prior to this patch, due to the absence of the '\0' character at
> the end of the BLOB, it was possible to get an error or incorrect
> result when using CAST() from BLOB to INTEGER or UNSIGNED. This
> has now been fixed, but the maximum length of a BLOB that could be
> cast to INTEGER or UNSIGNED was limited to 12287 bytes.
> Examples of wrong CAST() from BLOB to INTEGER:
> 
> Error during CAST():
> tarantool> box.execute("CREATE TABLE t1 (a VARBINARY PRIMARY KEY);")
> ---
> - row_count: 1
> ...

Please strip all excess output (... --- etc).

Re: [Tarantool-patches] [PATCH v3 0/2] sql: fix CAST() from BLOB to INTEGER

[Mergen Imeev]

Hi! Thank you for review. I will fix all mentioned in
review problems in the next patch. I hope you do not mind
that I do not plan to include your comments and my answers
since I see no reason for this. All answers will be just
"Done" or "Fixed"

On Wed, Mar 25, 2020 at 05:46:04PM +0000, Nikita Pettik wrote:
> On 25 Mar 14:38, imeevma@tarantool.org wrote:
> > This patch-set fixes CAST() from BLOB to INTEGER in case a BLOB
> > does not have '\0'.
> > 
> > https://github.com/tarantool/tarantool/issues/4766
> > https://github.com/tarantool/tarantool/tree/imeevma/gh-4766-fix-blob-size-for-cast
> > 
> > 
> > Changelog:
> >  - CAST() from string contains DOUBLE value to INTEGER or UNSIGNED
> >    was disallowed.
> >  - Maximum length of a BLOB that is allowed to be cast to INTEGER
> >    or UNSIGNED was limited to 12287 bytes.
> >  - Fixed wrong behaviour of CAST() in case a BLOB does not have
> >    '\0'.
> 
> @Changelog entry to release notes is missing.
>  
> > Mergen Imeev (2):
> >   sql: fix CAST() from STRING to INTEGER
> >   sql: add '\0' to the BLOB when it is cast to INTEGER
> > 
> >  src/box/sql/util.c                   | 17 +++++++++----
> >  src/box/sql/vdbe.c                   | 11 +++++++--
> >  src/box/sql/vdbeInt.h                |  1 -
> >  src/box/sql/vdbemem.c                | 45 +++++++++++----------------------
> >  test/sql-tap/cast.test.lua           | 48 +++++++++++++++++++++++++++++++++++-
> >  test/sql-tap/e_select1.test.lua      |  2 +-
> >  test/sql-tap/intpkey.test.lua        |  2 +-
> >  test/sql-tap/join.test.lua           |  4 +--
> >  test/sql-tap/subquery.test.lua       |  6 ++---
> >  test/sql-tap/tkt-9a8b09f8e6.test.lua |  4 +--
> >  test/sql/types.result                | 23 +++++++----------
> >  11 files changed, 100 insertions(+), 63 deletions(-)
> > 
> > -- 
> > 2.7.4
> >