[Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations

[imeevma@tarantool.org]

This patch removes the implicit conversion from STRING to NUMBER from
arithmetic operations. However, INTEGER can still be implicitly
converted to DOUBLE if the second operand is of type DOUBLE.

Follow-up #3809
---
 src/box/sql/vdbe.c                   |  71 ++++---------
 test/sql-tap/misc1.test.lua          |   6 +-
 test/sql-tap/misc3.test.lua          |  42 +-------
 test/sql-tap/tkt-a8a0d2996a.test.lua | 146 ---------------------------
 test/sql/types.result                |  61 ++++++++++-
 test/sql/types.test.lua              |  15 +++
 6 files changed, 97 insertions(+), 244 deletions(-)
 delete mode 100755 test/sql-tap/tkt-a8a0d2996a.test.lua

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 14ddb5160..c0143a6b1 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -522,41 +522,6 @@ mem_convert_to_numeric(struct Mem *mem, enum field_type type)
 	return mem_convert_to_integer(mem);
 }
 
-/*
- * pMem currently only holds a string type (or maybe a BLOB that we can
- * interpret as a string if we want to).  Compute its corresponding
- * numeric type, if has one.  Set the pMem->u.r and pMem->u.i fields
- * accordingly.
- */
-static u16 SQL_NOINLINE computeNumericType(Mem *pMem)
-{
-	assert((pMem->flags & (MEM_Int | MEM_UInt | MEM_Real)) == 0);
-	assert((pMem->flags & (MEM_Str|MEM_Blob))!=0);
-	if (sqlAtoF(pMem->z, &pMem->u.r, pMem->n)==0)
-		return 0;
-	bool is_neg;
-	if (sql_atoi64(pMem->z, (int64_t *) &pMem->u.i, &is_neg, pMem->n) == 0)
-		return is_neg ? MEM_Int : MEM_UInt;
-	return MEM_Real;
-}
-
-/*
- * Return the numeric type for pMem, either MEM_Int or MEM_Real or both or
- * none.
- *
- * Unlike mem_apply_numeric_type(), this routine does not modify pMem->flags.
- * But it does set pMem->u.r and pMem->u.i appropriately.
- */
-static u16 numericType(Mem *pMem)
-{
-	if ((pMem->flags & (MEM_Int | MEM_UInt | MEM_Real)) != 0)
-		return pMem->flags & (MEM_Int | MEM_UInt | MEM_Real);
-	if (pMem->flags & (MEM_Str|MEM_Blob)) {
-		return computeNumericType(pMem);
-	}
-	return 0;
-}
-
 #ifdef SQL_DEBUG
 /*
  * Write a nice string representation of the contents of cell pMem
@@ -1681,27 +1646,24 @@ case OP_Subtract:              /* same as TK_MINUS, in1, in2, out3 */
 case OP_Multiply:              /* same as TK_STAR, in1, in2, out3 */
 case OP_Divide:                /* same as TK_SLASH, in1, in2, out3 */
 case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
-	u32 flags;      /* Combined MEM_* flags from both inputs */
-	u16 type1;      /* Numeric type of left operand */
-	u16 type2;      /* Numeric type of right operand */
 	i64 iA;         /* Integer value of left operand */
 	i64 iB;         /* Integer value of right operand */
 	double rA;      /* Real value of left operand */
 	double rB;      /* Real value of right operand */
 
 	pIn1 = &aMem[pOp->p1];
-	type1 = numericType(pIn1);
 	pIn2 = &aMem[pOp->p2];
-	type2 = numericType(pIn2);
+	enum mp_type type1 = mem_mp_type(pIn1);
+	enum mp_type type2 = mem_mp_type(pIn2);
 	pOut = vdbe_prepare_null_out(p, pOp->p3);
-	flags = pIn1->flags | pIn2->flags;
-	if ((flags & MEM_Null)!=0) goto arithmetic_result_is_null;
-	if ((type1 & (MEM_Int | MEM_UInt)) != 0 &&
-	    (type2 & (MEM_Int | MEM_UInt)) != 0) {
+	if (type1 == MP_NIL || type2 == MP_NIL)
+		goto arithmetic_result_is_null;
+	if ((type1 == MP_INT || type1 == MP_UINT) &&
+	    (type2 == MP_INT || type2 == MP_UINT)) {
 		iA = pIn1->u.i;
 		iB = pIn2->u.i;
-		bool is_lhs_neg = pIn1->flags & MEM_Int;
-		bool is_rhs_neg = pIn2->flags & MEM_Int;
+		bool is_lhs_neg = type1 == MP_INT;
+		bool is_rhs_neg = type2 == MP_INT;
 		bool is_res_neg;
 		switch( pOp->opcode) {
 		case OP_Add: {
@@ -1742,17 +1704,28 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		}
 		mem_set_int(pOut, iB, is_res_neg);
 	} else {
-		if (sqlVdbeRealValue(pIn1, &rA) != 0) {
+		if (!mp_type_is_numeric(type1)) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_to_diag_str(pIn1), "numeric");
 			goto abort_due_to_error;
 		}
-		if (sqlVdbeRealValue(pIn2, &rB) != 0) {
+		if (!mp_type_is_numeric(type2)) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
 				 sql_value_to_diag_str(pIn2), "numeric");
 			goto abort_due_to_error;
 		}
-		assert(((type1 | type2) & MEM_Real) != 0);
+		if (type1 == MP_DOUBLE)
+			rA = pIn1->u.r;
+		else if (type1 == MP_INT)
+			rA = (double) pIn1->u.i;
+		else
+			rA = (double) pIn1->u.u;
+		if (type2 == MP_DOUBLE)
+			rB = pIn2->u.r;
+		else if (type2 == MP_INT)
+			rB = (double) pIn2->u.i;
+		else
+			rB = (double) pIn2->u.u;
 		switch( pOp->opcode) {
 		case OP_Add:         rB += rA;       break;
 		case OP_Subtract:    rB -= rA;       break;
diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua
index c0136d04c..005dc0cb6 100755
--- a/test/sql-tap/misc1.test.lua
+++ b/test/sql-tap/misc1.test.lua
@@ -68,7 +68,7 @@ test:do_test(
             cmd = cmd .. ")"
             test:execsql(cmd)
         end
-        return test:execsql("SELECT x50 FROM manycol ORDER BY x80+0")
+        return test:execsql("SELECT x50 FROM manycol ORDER BY CAST(x80 AS NUMBER)+0")
     end, {
         -- <misc1-1.3.1>
         "50", "150", "250", "350", "450", "550", "650", "750", "850", "950", "1050"
@@ -531,7 +531,7 @@ test:do_test(
     "misc1-10.7",
     function()
         where = string.gsub(where, "x0=0", "x0=100")
-        return test:catchsql("UPDATE manycol SET x1=CAST(x1+1 AS STRING) "..where.."")
+        return test:catchsql("UPDATE manycol SET x1=CAST(CAST(x1 AS NUMBER)+1 AS STRING) "..where.."")
     end, {
         -- <misc1-10.7>
         0
@@ -553,7 +553,7 @@ test:do_execsql_test(
 -- } {0 {}}
 test:do_execsql_test(
     "misc1-10.9",
-    "UPDATE manycol SET x1=CAST(x1+1 AS STRING) "..where
+    "UPDATE manycol SET x1=CAST(CAST(x1 AS NUMBER)+1 AS STRING) "..where
         --"UPDATE manycol SET x1=x1+1 $::where AND rowid>0"
     , {})
 
diff --git a/test/sql-tap/misc3.test.lua b/test/sql-tap/misc3.test.lua
index d0e45e872..7ee1a9e36 100755
--- a/test/sql-tap/misc3.test.lua
+++ b/test/sql-tap/misc3.test.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
 local json = require("json")
-test:plan(34)
+test:plan(30)
 
 --!./tcltestrunner.lua
 -- 2003 December 17
@@ -141,46 +141,6 @@ test:do_execsql_test(
         -- </misc3-2.5>
     })
 
-test:do_execsql_test(
-    "misc3-2.6",
-    [[
-        SELECT '-2.0e-127' * '-0.5e27'
-    ]], {
-        -- <misc3-2.6>
-        1e-100
-        -- </misc3-2.6>
-    })
-
-test:do_execsql_test(
-    "misc3-2.7",
-    [[
-        SELECT '+2.0e-127' * '-0.5e27'
-    ]], {
-        -- <misc3-2.7>
-        -1e-100
-        -- </misc3-2.7>
-    })
-
-test:do_execsql_test(
-    "misc3-2.8",
-    [[
-        SELECT 2.0e-27 * '+0.5e+127'
-    ]], {
-        -- <misc3-2.8>
-        1e+100
-        -- </misc3-2.8>
-    })
-
-test:do_execsql_test(
-    "misc3-2.9",
-    [[
-        SELECT 2.0e-27 * '+0.000005e+132'
-    ]], {
-        -- <misc3-2.9>
-        1e+100
-        -- </misc3-2.9>
-    })
-
 -- Ticket #522.  Make sure integer overflow is handled properly in
 -- indices.
 --
diff --git a/test/sql-tap/tkt-a8a0d2996a.test.lua b/test/sql-tap/tkt-a8a0d2996a.test.lua
deleted file mode 100755
index ddaeb60de..000000000
--- a/test/sql-tap/tkt-a8a0d2996a.test.lua
+++ /dev/null
@@ -1,146 +0,0 @@
-#!/usr/bin/env tarantool
-test = require("sqltester")
-test:plan(12)
-
---!./tcltestrunner.lua
--- 2014-03-24
---
--- The author disclaims copyright to this source code.  In place of
--- a legal notice, here is a blessing:
---
---    May you do good and not evil.
---    May you find forgiveness for yourself and forgive others.
---    May you share freely, never taking more than you give.
---
--------------------------------------------------------------------------
--- 
--- Tests to verify that arithmetic operators do not change the type of
--- input operands.  Ticket [a8a0d2996a]
---
--- ["set","testdir",[["file","dirname",["argv0"]]]]
--- ["source",[["testdir"],"\/tester.tcl"]]
-testprefix = "tkt-a8a0d2996a"
-test:do_execsql_test(
-    1.0,
-    [[
-        CREATE TABLE t(id INT PRIMARY KEY, x TEXT UNIQUE, y TEXT);
-        INSERT INTO t VALUES(1, '1','1');
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x+0 AND y=='1';
-    ]], {
-        -- <1.0>
-        "string", "string"
-        -- </1.0>
-    })
-
-test:do_execsql_test(
-    1.1,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x-0 AND y=='1';
-    ]], {
-        -- <1.1>
-        "string", "string"
-        -- </1.1>
-    })
-
-test:do_execsql_test(
-    1.2,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x*1 AND y=='1';
-    ]], {
-        -- <1.2>
-        "string", "string"
-        -- </1.2>
-    })
-
-test:do_execsql_test(
-    1.3,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x/1 AND y=='1';
-    ]], {
-        -- <1.3>
-        "string", "string"
-        -- </1.3>
-    })
-
-test:do_execsql_test(
-    1.4,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x%4 AND y=='1';
-    ]], {
-        -- <1.4>
-        "string", "string"
-        -- </1.4>
-    })
-
-test:do_execsql_test(
-    3.0,
-    [[
-        UPDATE t SET x='1.0';
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x+0 AND y=='1';
-    ]], {
-        -- <3.0>
-        "string", "string"
-        -- </3.0>
-    })
-
-test:do_execsql_test(
-    3.1,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x-0 AND y=='1';
-    ]], {
-        -- <3.1>
-        "string", "string"
-        -- </3.1>
-    })
-
-test:do_execsql_test(
-    3.2,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x*1 AND y=='1';
-    ]], {
-        -- <3.2>
-        "string", "string"
-        -- </3.2>
-    })
-
-test:do_execsql_test(
-    3.3,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x/1 AND y=='1';
-    ]], {
-        -- <3.3>
-        "string", "string"
-        -- </3.3>
-    })
-
-test:do_execsql_test(
-    3.4,
-    [[
-        SELECT typeof(x), typeof(y) FROM t WHERE 1=x%4 AND y=='1';
-    ]], {
-        -- <3.4>
-        "string", "string"
-        -- </3.4>
-    })
-
-test:do_execsql_test(
-    4.0,
-    [[
-        SELECT 1+1.;
-    ]], {
-        -- <4.0>
-        2.0
-        -- </4.0>
-    })
-
-test:do_execsql_test(
-    4.1,
-    [[
-        SELECT '1.23e64'/'1.0000e+62';
-    ]], {
-        -- <4.1>
-        123.0
-        -- </4.1>
-    })
-
-test:finish_test()
diff --git a/test/sql/types.result b/test/sql/types.result
index 442245186..caedbf409 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -310,11 +310,8 @@ box.execute('SELECT 1 + 1.1;')
 ...
 box.execute('SELECT \'9223372036854\' + 1;')
 ---
-- metadata:
-  - name: COLUMN_1
-    type: integer
-  rows:
-  - [9223372036855]
+- null
+- 'Type mismatch: can not convert 9223372036854 to numeric'
 ...
 -- Fix BOOLEAN bindings.
 box.execute('SELECT ?', {true})
@@ -2795,3 +2792,57 @@ box.execute([[DROP TABLE ts;]])
 ---
 - row_count: 1
 ...
+--
+-- Make sure there is no implicit string-to-number conversion in arithmetic
+-- operations.
+--
+box.execute([[SELECT '1' + 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to numeric'
+...
+box.execute([[SELECT '1' % 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to numeric'
+...
+box.execute([[SELECT '1' * 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to numeric'
+...
+box.execute([[SELECT '1' / 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to numeric'
+...
+box.execute([[SELECT '1' - 2;]])
+---
+- null
+- 'Type mismatch: can not convert 1 to numeric'
+...
+box.execute([[SELECT 1 + '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to numeric'
+...
+box.execute([[SELECT 1 % '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to numeric'
+...
+box.execute([[SELECT 1 * '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to numeric'
+...
+box.execute([[SELECT 1 / '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to numeric'
+...
+box.execute([[SELECT 1 - '2';]])
+---
+- null
+- 'Type mismatch: can not convert 2 to numeric'
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index 0270d9f8a..844a6b670 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -623,3 +623,18 @@ box.execute([[DROP TABLE tb;]])
 box.execute([[DROP TABLE tt;]])
 box.execute([[DROP TABLE tv;]])
 box.execute([[DROP TABLE ts;]])
+
+--
+-- Make sure there is no implicit string-to-number conversion in arithmetic
+-- operations.
+--
+box.execute([[SELECT '1' + 2;]])
+box.execute([[SELECT '1' % 2;]])
+box.execute([[SELECT '1' * 2;]])
+box.execute([[SELECT '1' / 2;]])
+box.execute([[SELECT '1' - 2;]])
+box.execute([[SELECT 1 + '2';]])
+box.execute([[SELECT 1 % '2';]])
+box.execute([[SELECT 1 * '2';]])
+box.execute([[SELECT 1 / '2';]])
+box.execute([[SELECT 1 - '2';]])
-- 
2.25.1