From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 63C4E43040C for ; Fri, 21 Aug 2020 11:40:52 +0300 (MSK) From: imeevma@tarantool.org Date: Fri, 21 Aug 2020 11:40:50 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: korablev@tarantool.org, tsafin@tarantool.org Cc: tarantool-patches@dev.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, { -- "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, { -- 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( -- }) -test:do_execsql_test( - "misc3-2.6", - [[ - SELECT '-2.0e-127' * '-0.5e27' - ]], { - -- - 1e-100 - -- - }) - -test:do_execsql_test( - "misc3-2.7", - [[ - SELECT '+2.0e-127' * '-0.5e27' - ]], { - -- - -1e-100 - -- - }) - -test:do_execsql_test( - "misc3-2.8", - [[ - SELECT 2.0e-27 * '+0.5e+127' - ]], { - -- - 1e+100 - -- - }) - -test:do_execsql_test( - "misc3-2.9", - [[ - SELECT 2.0e-27 * '+0.000005e+132' - ]], { - -- - 1e+100 - -- - }) - -- 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" - -- - }) - -test:do_execsql_test( - 1.1, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x-0 AND y=='1'; - ]], { - -- <1.1> - "string", "string" - -- - }) - -test:do_execsql_test( - 1.2, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x*1 AND y=='1'; - ]], { - -- <1.2> - "string", "string" - -- - }) - -test:do_execsql_test( - 1.3, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x/1 AND y=='1'; - ]], { - -- <1.3> - "string", "string" - -- - }) - -test:do_execsql_test( - 1.4, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x%4 AND y=='1'; - ]], { - -- <1.4> - "string", "string" - -- - }) - -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" - -- - }) - -test:do_execsql_test( - 3.1, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x-0 AND y=='1'; - ]], { - -- <3.1> - "string", "string" - -- - }) - -test:do_execsql_test( - 3.2, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x*1 AND y=='1'; - ]], { - -- <3.2> - "string", "string" - -- - }) - -test:do_execsql_test( - 3.3, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x/1 AND y=='1'; - ]], { - -- <3.3> - "string", "string" - -- - }) - -test:do_execsql_test( - 3.4, - [[ - SELECT typeof(x), typeof(y) FROM t WHERE 1=x%4 AND y=='1'; - ]], { - -- <3.4> - "string", "string" - -- - }) - -test:do_execsql_test( - 4.0, - [[ - SELECT 1+1.; - ]], { - -- <4.0> - 2.0 - -- - }) - -test:do_execsql_test( - 4.1, - [[ - SELECT '1.23e64'/'1.0000e+62'; - ]], { - -- <4.1> - 123.0 - -- - }) - -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