Re: [Tarantool-patches] [PATCH v4 2/3] sql: fix implicit cast from STRING to INTEGER

[Mergen Imeev <imeevma@tarantool.org>]

On Fri, Mar 27, 2020 at 04:54:17PM +0000, Nikita Pettik wrote:
> On 27 Mar 14:33, imeevma@tarantool.org wrote:
> > Prior to this patch, STRING, which contains the DOUBLE value,
> > could be implicitly 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.
> > 
> > Example:
> > box.execute("CREATE TABLE t(i INT PRIMARY KEY);")
> > 
> > Before patch:
> > box.execute("INSERT INTO t VALUES ('111.1');")
> > box.execute("SELECT * FROM t;")
> > Result: 111
> > 
> > After patch:
> > box.execute("INSERT INTO t VALUES ('1.1');")
> > Result: 'Type mismatch: can not convert 1.1 to integer'
> > 
> > box.execute("INSERT INTO t VALUES ('1.0');")
> > Result: 'Type mismatch: can not convert 1.0 to integer'
> > 
> > box.execute("INSERT INTO t VALUES ('1.');")
> > Result: 'Type mismatch: can not convert 1. to integer'
> 
> Is comparison predicat affected?
> 
No, since all implicit casts in case of comparison executed
inside of comparisons opcodes.

> > Part of #4766
> > 
> > @TarantoolBot document
> > Title: disallow cast from STRING contais DOUBLE to INTEGER
> 
> -> contains
>  
Fixed.

> > 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
> > index 0865c4e..559e33d 100755
> > --- 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
> 
> Same nit concerning test file as in previous patch: personally I'd
> not introduce it.
> 
Removed.


New patch:


From 18aed8fc1b1c566a90888c2835e4210ef3386009 Mon Sep 17 00:00:00 2001
From: Mergen Imeev <imeevma@gmail.com>
Date: Thu, 26 Mar 2020 15:30:02 +0300
Subject: [PATCH] sql: fix implicit cast from STRING to INTEGER

Prior to this patch, STRING, which contains the DOUBLE value,
could be implicitly 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.

Example:
box.execute("CREATE TABLE t(i INT PRIMARY KEY);")

Before patch:
box.execute("INSERT INTO t VALUES ('111.1');")
box.execute("SELECT * FROM t;")
Result: 111

After patch:
box.execute("INSERT INTO t VALUES ('1.1');")
Result: 'Type mismatch: can not convert 1.1 to integer'

box.execute("INSERT INTO t VALUES ('1.0');")
Result: 'Type mismatch: can not convert 1.0 to integer'

box.execute("INSERT INTO t VALUES ('1.');")
Result: 'Type mismatch: can not convert 1. to integer'

@TarantoolBot document
Title: disallow cast from STRING contains DOUBLE to INTEGER

After the last two patches, explicit and implicit casting from the
string containing DOUBLE to INTEGER directly will be prohibited.
The user must use the explicit cast to DOUBLE before the explicit
or implicit cast to INTEGER. The reason for this is that before
these patches, such STRINGs were implicitly cast to DOUBLE, and
then this DOUBLE was implicitly or explicitly cast to INTEGER.
Because of this, the result of such a cast may differ from what
the user expects, and the user may not know why.

Example for implicit cast:

box.execute("CREATE TABLE t(i INT PRIMARY KEY);")
-- Does not work anymore:
box.execute("INSERT INTO t VALUES ('1.1');")
-- Right way:
box.execute("INSERT INTO t VALUES (CAST('1.1' AS DOUBLE));")

Example for explicit cast:

-- Does not work anymore:
box.execute("SELECT CAST('1.1' AS INTEGER);")
-- Right way:
box.execute("SELECT CAST(CAST('1.1' AS DOUBLE) AS INTEGER);")

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 de1d9c3..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.
  */
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