* [Tarantool-patches] [PATCH v1 0/2] sql: remove implicit cast from operations @ 2020-08-21 8:40 imeevma 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations imeevma 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations imeevma 0 siblings, 2 replies; 10+ messages in thread From: imeevma @ 2020-08-21 8:40 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches This patch-set removes implicit string-to-number conversion from arithmetic and bitwise operations. https://github.com/tarantool/tarantool/issues/3809 https://github.com/tarantool/tarantool/tree/imeevma/gh-3809-follow-up @ChangeLog - Strings are no longer implicitly converted to numbers in arithmetic and bitwise operations. Mergen Imeev (2): sql: remove implicit cast in arithmetic operations sql: remove implicit cast in bitwise operations src/box/sql/vdbe.c | 130 +++++++++++++----------- 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 | 110 +++++++++++++++++++- test/sql/types.test.lua | 29 ++++++ 6 files changed, 207 insertions(+), 256 deletions(-) delete mode 100755 test/sql-tap/tkt-a8a0d2996a.test.lua -- 2.25.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations 2020-08-21 8:40 [Tarantool-patches] [PATCH v1 0/2] sql: remove implicit cast from operations imeevma @ 2020-08-21 8:40 ` imeevma 2020-08-21 8:59 ` Nikita Pettik 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations imeevma 1 sibling, 1 reply; 10+ messages in thread From: imeevma @ 2020-08-21 8:40 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations imeevma @ 2020-08-21 8:59 ` Nikita Pettik 0 siblings, 0 replies; 10+ messages in thread From: Nikita Pettik @ 2020-08-21 8:59 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches On 21 Aug 11:40, imeevma@tarantool.org wrote: > 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 LGTM ^ permalink raw reply [flat|nested] 10+ messages in thread
* [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations 2020-08-21 8:40 [Tarantool-patches] [PATCH v1 0/2] sql: remove implicit cast from operations imeevma 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations imeevma @ 2020-08-21 8:40 ` imeevma 2020-08-21 9:21 ` Nikita Pettik 1 sibling, 1 reply; 10+ messages in thread From: imeevma @ 2020-08-21 8:40 UTC (permalink / raw) To: korablev, tsafin; +Cc: tarantool-patches This patch removes the implicit conversion from STRING to INTEGER from bitwise operations. However, DOUBLE can still be implicitly converted to INTEGER. Follow-up #3809 --- src/box/sql/vdbe.c | 59 ++++++++++++++++++++++++++++++++--------- test/sql/types.result | 49 ++++++++++++++++++++++++++++++++++ test/sql/types.test.lua | 14 ++++++++++ 3 files changed, 110 insertions(+), 12 deletions(-) diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index c0143a6b1..a8db6d076 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1989,19 +1989,44 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ pIn1 = &aMem[pOp->p1]; pIn2 = &aMem[pOp->p2]; + enum mp_type type1 = mem_mp_type(pIn1); + enum mp_type type2 = mem_mp_type(pIn2); pOut = vdbe_prepare_null_out(p, pOp->p3); - if ((pIn1->flags | pIn2->flags) & MEM_Null) { + if (type1 == MP_NIL || type2 == MP_NIL) { /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; break; } - bool unused; - if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) { + if (type2 == MP_DOUBLE) { + double r = pIn2->u.r; + if (r >= 0 && r < (double)UINT64_MAX) { + iB = (int64_t)(uint64_t)r; + type2 = MP_UINT; + } else if (r >= (double)INT64_MIN && r < 0) { + iB = (int64_t)r; + type2 = MP_INT; + } + } + if (type2 == MP_INT || type2 == MP_UINT) { + iB = pIn2->u.i; + } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_to_diag_str(pIn2), "integer"); goto abort_due_to_error; } - if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) { + if (type1 == MP_DOUBLE) { + double r = pIn1->u.r; + if (r >= 0 && r < (double)UINT64_MAX) { + iA = (int64_t)(uint64_t)r; + type1 = MP_UINT; + } else if (r >= (double)INT64_MIN && r < 0) { + iA = (int64_t)r; + type1 = MP_INT; + } + } + if (type1 == MP_INT || type1 == MP_UINT) { + iA = pIn1->u.i; + } else { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_to_diag_str(pIn1), "integer"); goto abort_due_to_error; @@ -2621,19 +2646,29 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ */ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ pIn1 = &aMem[pOp->p1]; + enum mp_type type = mem_mp_type(pIn1); pOut = vdbe_prepare_null_out(p, pOp->p2); /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; - if ((pIn1->flags & MEM_Null)==0) { - int64_t i; - bool is_neg; - if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "integer"); - goto abort_due_to_error; + int64_t i; + if (type == MP_DOUBLE) { + double r = pIn1->u.r; + if (r >= 0 && r < (double)UINT64_MAX) { + i = (int64_t)(uint64_t)r; + type = MP_UINT; + } else if (r >= (double)INT64_MIN && r < 0) { + i = (int64_t)r; + type = MP_INT; } - mem_set_i64(pOut, ~i); } + if (type == MP_INT || type == MP_UINT) { + i = pIn1->u.i; + } else { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(pIn1), "integer"); + goto abort_due_to_error; + } + mem_set_i64(pOut, ~i); break; } diff --git a/test/sql/types.result b/test/sql/types.result index caedbf409..601e5beca 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -2846,3 +2846,52 @@ box.execute([[SELECT 1 - '2';]]) - null - 'Type mismatch: can not convert 2 to numeric' ... +-- +-- Make sure there is no implicit string-to-number conversion in bitwise +-- operations. +-- +box.execute([[SELECT '1' | 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' & 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' << 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' >> 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT ~'1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT 1 | '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 & '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 << '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 >> '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index 844a6b670..2d2f41da2 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -638,3 +638,17 @@ box.execute([[SELECT 1 % '2';]]) box.execute([[SELECT 1 * '2';]]) box.execute([[SELECT 1 / '2';]]) box.execute([[SELECT 1 - '2';]]) + +-- +-- Make sure there is no implicit string-to-number conversion in bitwise +-- 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';]]) +box.execute([[SELECT 1 | '2';]]) +box.execute([[SELECT 1 & '2';]]) +box.execute([[SELECT 1 << '2';]]) +box.execute([[SELECT 1 >> '2';]]) -- 2.25.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations imeevma @ 2020-08-21 9:21 ` Nikita Pettik 2020-08-21 12:34 ` Mergen Imeev 0 siblings, 1 reply; 10+ messages in thread From: Nikita Pettik @ 2020-08-21 9:21 UTC (permalink / raw) To: imeevma; +Cc: tarantool-patches On 21 Aug 11:40, imeevma@tarantool.org wrote: > This patch removes the implicit conversion from STRING to INTEGER from > bitwise operations. However, DOUBLE can still be implicitly converted to > INTEGER. I see no test involving doubles in bitwise operations. Does it make any sense at all? > Follow-up #3809 > --- > src/box/sql/vdbe.c | 59 ++++++++++++++++++++++++++++++++--------- > test/sql/types.result | 49 ++++++++++++++++++++++++++++++++++ > test/sql/types.test.lua | 14 ++++++++++ > 3 files changed, 110 insertions(+), 12 deletions(-) > > diff --git a/test/sql/types.result b/test/sql/types.result > index caedbf409..601e5beca 100644 > --- a/test/sql/types.result > +++ b/test/sql/types.result > @@ -2846,3 +2846,52 @@ box.execute([[SELECT 1 - '2';]]) > - null > - 'Type mismatch: can not convert 2 to numeric' > ... > +-- > +-- Make sure there is no implicit string-to-number conversion in bitwise > +-- operations. > +-- > +box.execute([[SELECT '1' | 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT '1' & 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT '1' << 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT '1' >> 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT ~'1';]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT 1 | '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > +box.execute([[SELECT 1 & '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > +box.execute([[SELECT 1 << '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > +box.execute([[SELECT 1 >> '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua > index 844a6b670..2d2f41da2 100644 > --- a/test/sql/types.test.lua > +++ b/test/sql/types.test.lua > @@ -638,3 +638,17 @@ box.execute([[SELECT 1 % '2';]]) > box.execute([[SELECT 1 * '2';]]) > box.execute([[SELECT 1 / '2';]]) > box.execute([[SELECT 1 - '2';]]) > + > +-- > +-- Make sure there is no implicit string-to-number conversion in bitwise > +-- 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';]]) > +box.execute([[SELECT 1 | '2';]]) > +box.execute([[SELECT 1 & '2';]]) > +box.execute([[SELECT 1 << '2';]]) > +box.execute([[SELECT 1 >> '2';]]) > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations 2020-08-21 9:21 ` Nikita Pettik @ 2020-08-21 12:34 ` Mergen Imeev 2020-09-17 13:36 ` Nikita Pettik 0 siblings, 1 reply; 10+ messages in thread From: Mergen Imeev @ 2020-08-21 12:34 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Hi! Thank you for the review. My answer and new patch below. On Fri, Aug 21, 2020 at 09:21:30AM +0000, Nikita Pettik wrote: > On 21 Aug 11:40, imeevma@tarantool.org wrote: > > This patch removes the implicit conversion from STRING to INTEGER from > > bitwise operations. However, DOUBLE can still be implicitly converted to > > INTEGER. > > I see no test involving doubles in bitwise operations. Does it make > any sense at all? Even if it doesn't, it is legal to implicitly convert DOUBLE to INTEGER. However, when I tried to add tests for this case, I found an error in my patch. I re-made patch. Now in these opcodes we convert MEM to INTEGER, which I tried to avoid in previous patch. I did this to fix a bug where result of the operation is DOUBLE if one of the operands is DOUBLE. It didn't help, the result still has DOUBLE type. I decided to left conversion since it looks right here because all operands must be INTEGERS. This wouldn't work for arithmetic operations though. New patch: From 3515ada4b363062cf9caa5d550ea40770e8a5e65 Mon Sep 17 00:00:00 2001 From: Mergen Imeev <imeevma@gmail.com> Date: Tue, 18 Aug 2020 18:18:59 +0300 Subject: [PATCH] sql: remove implicit cast in bitwise operations This patch removes the implicit conversion from STRING to INTEGER from bitwise operations. However, DOUBLE can still be implicitly converted to INTEGER. Follow-up #3809 diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index c0143a6b1..42228c435 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1989,23 +1989,26 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ pIn1 = &aMem[pOp->p1]; pIn2 = &aMem[pOp->p2]; + enum mp_type type1 = mem_mp_type(pIn1); + enum mp_type type2 = mem_mp_type(pIn2); pOut = vdbe_prepare_null_out(p, pOp->p3); - if ((pIn1->flags | pIn2->flags) & MEM_Null) { + if (type1 == MP_NIL || type2 == MP_NIL) { /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; break; } - bool unused; - if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) { + if (mem_convert_to_integer(pIn2) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_to_diag_str(pIn2), "integer"); goto abort_due_to_error; } - if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) { + if (mem_convert_to_integer(pIn1) != 0) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_to_diag_str(pIn1), "integer"); goto abort_due_to_error; } + iA = pIn2->u.i; + iB = pIn1->u.i; op = pOp->opcode; if (op==OP_BitAnd) { iA &= iB; @@ -2621,19 +2624,19 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ */ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ pIn1 = &aMem[pOp->p1]; + enum mp_type type = mem_mp_type(pIn1); pOut = vdbe_prepare_null_out(p, pOp->p2); /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; - if ((pIn1->flags & MEM_Null)==0) { - int64_t i; - bool is_neg; - if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "integer"); - goto abort_due_to_error; - } - mem_set_i64(pOut, ~i); + if (type == MP_NIL) { + break; + } + if (mem_convert_to_integer(pIn1) != 0) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(pIn1), "integer"); + goto abort_due_to_error; } + mem_set_i64(pOut, ~pIn1->u.i); break; } diff --git a/test/sql/types.result b/test/sql/types.result index caedbf409..70247471e 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -2846,3 +2846,93 @@ box.execute([[SELECT 1 - '2';]]) - null - 'Type mismatch: can not convert 2 to numeric' ... +-- +-- Make sure there is no implicit string-to-number conversion in bitwise +-- operations. +-- +box.execute([[SELECT '1' | 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' & 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' << 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' >> 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT ~'1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT 1 | '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 & '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 << '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 >> '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +-- Make sure that DOUBLE implicitly cast to INTEGER in bitwise operations. +box.execute([[SELECT 3.5 | 1.3;]]) +--- +- metadata: + - name: COLUMN_1 + type: double + rows: + - [3] +... +box.execute([[SELECT 3.5 & 1.3;]]) +--- +- metadata: + - name: COLUMN_1 + type: double + rows: + - [1] +... +box.execute([[SELECT 3.5 << 1.3;]]) +--- +- metadata: + - name: COLUMN_1 + type: double + rows: + - [6] +... +box.execute([[SELECT 3.5 >> 1.3;]]) +--- +- metadata: + - name: COLUMN_1 + type: double + rows: + - [1] +... +box.execute([[SELECT ~3.5;]]) +--- +- metadata: + - name: COLUMN_1 + type: double + rows: + - [-4] +... diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index 844a6b670..c000f4d13 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -638,3 +638,24 @@ box.execute([[SELECT 1 % '2';]]) box.execute([[SELECT 1 * '2';]]) box.execute([[SELECT 1 / '2';]]) box.execute([[SELECT 1 - '2';]]) + +-- +-- Make sure there is no implicit string-to-number conversion in bitwise +-- 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';]]) +box.execute([[SELECT 1 | '2';]]) +box.execute([[SELECT 1 & '2';]]) +box.execute([[SELECT 1 << '2';]]) +box.execute([[SELECT 1 >> '2';]]) + +-- Make sure that DOUBLE implicitly cast to INTEGER in bitwise operations. +box.execute([[SELECT 3.5 | 1.3;]]) +box.execute([[SELECT 3.5 & 1.3;]]) +box.execute([[SELECT 3.5 << 1.3;]]) +box.execute([[SELECT 3.5 >> 1.3;]]) +box.execute([[SELECT ~3.5;]]) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations 2020-08-21 12:34 ` Mergen Imeev @ 2020-09-17 13:36 ` Nikita Pettik 2020-09-27 9:11 ` Mergen Imeev 0 siblings, 1 reply; 10+ messages in thread From: Nikita Pettik @ 2020-09-17 13:36 UTC (permalink / raw) To: Mergen Imeev; +Cc: tarantool-patches On 21 Aug 15:34, Mergen Imeev wrote: > Hi! Thank you for the review. My answer and new patch below. > > On Fri, Aug 21, 2020 at 09:21:30AM +0000, Nikita Pettik wrote: > > On 21 Aug 11:40, imeevma@tarantool.org wrote: > > > This patch removes the implicit conversion from STRING to INTEGER from > > > bitwise operations. However, DOUBLE can still be implicitly converted to > > > INTEGER. > > > > I see no test involving doubles in bitwise operations. Does it make > > any sense at all? > Even if it doesn't, it is legal to implicitly convert DOUBLE to INTEGER. > > However, when I tried to add tests for this case, I found an error in my patch. > I re-made patch. Now in these opcodes we convert MEM to INTEGER, which I tried > to avoid in previous patch. I did this to fix a bug where result of the > operation is DOUBLE if one of the operands is DOUBLE. It didn't help, the > result still has DOUBLE type. I decided to left conversion since it looks right > here because all operands must be INTEGERS. This wouldn't work for arithmetic > operations though. Не понял ничего..Как эта штука должна работать?? box.execute([[SELECT 3.5 | 1.3;]]) metadata: name: COLUMN_1 type: double rows: [3] -- WTF Давай по-честному выдавать ошибку, когда в битовые операции суем не инты. > From 3515ada4b363062cf9caa5d550ea40770e8a5e65 Mon Sep 17 00:00:00 2001 > From: Mergen Imeev <imeevma@gmail.com> > Date: Tue, 18 Aug 2020 18:18:59 +0300 > Subject: [PATCH] sql: remove implicit cast in bitwise operations > > This patch removes the implicit conversion from STRING to INTEGER from > bitwise operations. However, DOUBLE can still be implicitly converted to > INTEGER. > > Follow-up #3809 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations 2020-09-17 13:36 ` Nikita Pettik @ 2020-09-27 9:11 ` Mergen Imeev 2020-09-28 17:13 ` Nikita Pettik 0 siblings, 1 reply; 10+ messages in thread From: Mergen Imeev @ 2020-09-27 9:11 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Привет! Спасибо за ревью. Мои ответы и новый патч ниже. On Thu, Sep 17, 2020 at 01:36:48PM +0000, Nikita Pettik wrote: > On 21 Aug 15:34, Mergen Imeev wrote: > > Hi! Thank you for the review. My answer and new patch below. > > > > On Fri, Aug 21, 2020 at 09:21:30AM +0000, Nikita Pettik wrote: > > > On 21 Aug 11:40, imeevma@tarantool.org wrote: > > > > This patch removes the implicit conversion from STRING to INTEGER from > > > > bitwise operations. However, DOUBLE can still be implicitly converted to > > > > INTEGER. > > > > > > I see no test involving doubles in bitwise operations. Does it make > > > any sense at all? > > Even if it doesn't, it is legal to implicitly convert DOUBLE to INTEGER. > > > > However, when I tried to add tests for this case, I found an error in my patch. > > I re-made patch. Now in these opcodes we convert MEM to INTEGER, which I tried > > to avoid in previous patch. I did this to fix a bug where result of the > > operation is DOUBLE if one of the operands is DOUBLE. It didn't help, the > > result still has DOUBLE type. I decided to left conversion since it looks right > > here because all operands must be INTEGERS. This wouldn't work for arithmetic > > operations though. > > Не понял ничего..Как эта штука должна работать?? > box.execute([[SELECT 3.5 | 1.3;]]) > metadata: > name: COLUMN_1 > type: double > rows: > [3] -- WTF > В соответствии с правилами implicit cast between numeric values. В любом случае, убрал возможность делать значения типа DOUBLE операндами битовых операций. > Давай по-честному выдавать ошибку, когда в битовые операции суем не инты. Сделал. > > > From 3515ada4b363062cf9caa5d550ea40770e8a5e65 Mon Sep 17 00:00:00 2001 > > From: Mergen Imeev <imeevma@gmail.com> > > Date: Tue, 18 Aug 2020 18:18:59 +0300 > > Subject: [PATCH] sql: remove implicit cast in bitwise operations > > > > This patch removes the implicit conversion from STRING to INTEGER from > > bitwise operations. However, DOUBLE can still be implicitly converted to > > INTEGER. > > > > Follow-up #3809 > > Новый патч: From a630c39d486769d1a684e56b15bbf5107e7bef66 Mon Sep 17 00:00:00 2001 From: Mergen Imeev <imeevma@gmail.com> Date: Tue, 18 Aug 2020 18:18:59 +0300 Subject: [PATCH] sql: remove implicit cast in bitwise operations This patch removes the implicit conversion from STRING to INTEGER from bitwise operations. Only integer values allowed in bitwise operations. Follow-up #3809 diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c index c0143a6b1..5d1ab8c94 100644 --- a/src/box/sql/vdbe.c +++ b/src/box/sql/vdbe.c @@ -1989,23 +1989,26 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ pIn1 = &aMem[pOp->p1]; pIn2 = &aMem[pOp->p2]; + enum mp_type type1 = mem_mp_type(pIn1); + enum mp_type type2 = mem_mp_type(pIn2); pOut = vdbe_prepare_null_out(p, pOp->p3); - if ((pIn1->flags | pIn2->flags) & MEM_Null) { + if (type1 == MP_NIL || type2 == MP_NIL) { /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; break; } - bool unused; - if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) { + if (type2 != MP_INT && type2 != MP_UINT) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_to_diag_str(pIn2), "integer"); goto abort_due_to_error; } - if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) { + if (type1 != MP_INT && type1 != MP_UINT) { diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_to_diag_str(pIn1), "integer"); goto abort_due_to_error; } + iA = pIn2->u.i; + iB = pIn1->u.i; op = pOp->opcode; if (op==OP_BitAnd) { iA &= iB; @@ -2621,19 +2624,18 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ */ case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ pIn1 = &aMem[pOp->p1]; + enum mp_type type = mem_mp_type(pIn1); pOut = vdbe_prepare_null_out(p, pOp->p2); /* Force NULL be of type INTEGER. */ pOut->field_type = FIELD_TYPE_INTEGER; - if ((pIn1->flags & MEM_Null)==0) { - int64_t i; - bool is_neg; - if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) { - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, - sql_value_to_diag_str(pIn1), "integer"); - goto abort_due_to_error; - } - mem_set_i64(pOut, ~i); + if (type == MP_NIL) + break; + if (type != MP_INT && type != MP_UINT) { + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, + sql_value_to_diag_str(pIn1), "integer"); + goto abort_due_to_error; } + mem_set_i64(pOut, ~pIn1->u.i); break; } diff --git a/test/sql/types.result b/test/sql/types.result index caedbf409..a79818b30 100644 --- a/test/sql/types.result +++ b/test/sql/types.result @@ -2846,3 +2846,284 @@ box.execute([[SELECT 1 - '2';]]) - null - 'Type mismatch: can not convert 2 to numeric' ... +-- +-- Make sure there is no implicit string-to-number conversion in bitwise +-- operations. +-- +box.execute([[SELECT '1' | 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' & 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' << 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT '1' >> 2;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT ~'1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT 1 | '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 & '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 << '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +box.execute([[SELECT 1 >> '2';]]) +--- +- null +- 'Type mismatch: can not convert 2 to integer' +... +-- +-- Make sure that only values of type INTEGER can be operands of bitwise +-- operations. +-- +box.execute([[SELECT 3 | 1;]]) +--- +- metadata: + - name: COLUMN_1 + type: integer + rows: + - [3] +... +box.execute([[SELECT 3 | 1.1;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT 3 | '1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT 3 | true;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT 3 | X'31';]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT 1 | 3;]]) +--- +- metadata: + - name: COLUMN_1 + type: integer + rows: + - [3] +... +box.execute([[SELECT 1.1 | 3;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT '1' | 3;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT true | 3;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT X'31' | 3;]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT 3 & 1;]]) +--- +- metadata: + - name: COLUMN_1 + type: integer + rows: + - [1] +... +box.execute([[SELECT 3 & 1.1;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT 3 & '1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT 3 & true;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT 3 & X'31';]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT 1.1 & 3;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT '1' & 3;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT true & 3;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT X'31' & 3;]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT 3 << 1;]]) +--- +- metadata: + - name: COLUMN_1 + type: integer + rows: + - [6] +... +box.execute([[SELECT 3 << 1.1;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT 3 << '1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT 3 << true;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT 3 << X'31';]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT 1.1 << 3;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT '1' << 3;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT true << 3;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT X'31' << 3;]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT 3 >> 1;]]) +--- +- metadata: + - name: COLUMN_1 + type: integer + rows: + - [1] +... +box.execute([[SELECT 3 >> 1.1;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT 3 >> '1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT 3 >> true;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT 3 >> X'31';]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT 1.1 >> 3;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT '1' >> 3;]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT true >> 3;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT X'31' >> 3;]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... +box.execute([[SELECT ~1;]]) +--- +- metadata: + - name: COLUMN_1 + type: integer + rows: + - [-2] +... +box.execute([[SELECT ~1.1;]]) +--- +- null +- 'Type mismatch: can not convert 1.1 to integer' +... +box.execute([[SELECT ~'1';]]) +--- +- null +- 'Type mismatch: can not convert 1 to integer' +... +box.execute([[SELECT ~true;]]) +--- +- null +- 'Type mismatch: can not convert TRUE to integer' +... +box.execute([[SELECT ~X'31';]]) +--- +- null +- 'Type mismatch: can not convert varbinary to integer' +... diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua index 844a6b670..bb922e78f 100644 --- a/test/sql/types.test.lua +++ b/test/sql/types.test.lua @@ -638,3 +638,68 @@ box.execute([[SELECT 1 % '2';]]) box.execute([[SELECT 1 * '2';]]) box.execute([[SELECT 1 / '2';]]) box.execute([[SELECT 1 - '2';]]) + +-- +-- Make sure there is no implicit string-to-number conversion in bitwise +-- 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';]]) +box.execute([[SELECT 1 | '2';]]) +box.execute([[SELECT 1 & '2';]]) +box.execute([[SELECT 1 << '2';]]) +box.execute([[SELECT 1 >> '2';]]) + +-- +-- Make sure that only values of type INTEGER can be operands of bitwise +-- operations. +-- +box.execute([[SELECT 3 | 1;]]) +box.execute([[SELECT 3 | 1.1;]]) +box.execute([[SELECT 3 | '1';]]) +box.execute([[SELECT 3 | true;]]) +box.execute([[SELECT 3 | X'31';]]) +box.execute([[SELECT 1 | 3;]]) +box.execute([[SELECT 1.1 | 3;]]) +box.execute([[SELECT '1' | 3;]]) +box.execute([[SELECT true | 3;]]) +box.execute([[SELECT X'31' | 3;]]) + +box.execute([[SELECT 3 & 1;]]) +box.execute([[SELECT 3 & 1.1;]]) +box.execute([[SELECT 3 & '1';]]) +box.execute([[SELECT 3 & true;]]) +box.execute([[SELECT 3 & X'31';]]) +box.execute([[SELECT 1.1 & 3;]]) +box.execute([[SELECT '1' & 3;]]) +box.execute([[SELECT true & 3;]]) +box.execute([[SELECT X'31' & 3;]]) + +box.execute([[SELECT 3 << 1;]]) +box.execute([[SELECT 3 << 1.1;]]) +box.execute([[SELECT 3 << '1';]]) +box.execute([[SELECT 3 << true;]]) +box.execute([[SELECT 3 << X'31';]]) +box.execute([[SELECT 1.1 << 3;]]) +box.execute([[SELECT '1' << 3;]]) +box.execute([[SELECT true << 3;]]) +box.execute([[SELECT X'31' << 3;]]) + +box.execute([[SELECT 3 >> 1;]]) +box.execute([[SELECT 3 >> 1.1;]]) +box.execute([[SELECT 3 >> '1';]]) +box.execute([[SELECT 3 >> true;]]) +box.execute([[SELECT 3 >> X'31';]]) +box.execute([[SELECT 1.1 >> 3;]]) +box.execute([[SELECT '1' >> 3;]]) +box.execute([[SELECT true >> 3;]]) +box.execute([[SELECT X'31' >> 3;]]) + +box.execute([[SELECT ~1;]]) +box.execute([[SELECT ~1.1;]]) +box.execute([[SELECT ~'1';]]) +box.execute([[SELECT ~true;]]) +box.execute([[SELECT ~X'31';]]) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations 2020-09-27 9:11 ` Mergen Imeev @ 2020-09-28 17:13 ` Nikita Pettik 2020-09-29 13:00 ` Mergen Imeev 0 siblings, 1 reply; 10+ messages in thread From: Nikita Pettik @ 2020-09-28 17:13 UTC (permalink / raw) To: Mergen Imeev; +Cc: tarantool-patches On 27 Sep 12:11, Mergen Imeev wrote: > Привет! Спасибо за ревью. Мои ответы и новый патч ниже. > > On Thu, Sep 17, 2020 at 01:36:48PM +0000, Nikita Pettik wrote: > > On 21 Aug 15:34, Mergen Imeev wrote: > > > Hi! Thank you for the review. My answer and new patch below. > > > > > > On Fri, Aug 21, 2020 at 09:21:30AM +0000, Nikita Pettik wrote: > > > > On 21 Aug 11:40, imeevma@tarantool.org wrote: > > > > > This patch removes the implicit conversion from STRING to INTEGER from > > > > > bitwise operations. However, DOUBLE can still be implicitly converted to > > > > > INTEGER. > > > > > > > > I see no test involving doubles in bitwise operations. Does it make > > > > any sense at all? > > > Even if it doesn't, it is legal to implicitly convert DOUBLE to INTEGER. > > > > > > However, when I tried to add tests for this case, I found an error in my patch. > > > I re-made patch. Now in these opcodes we convert MEM to INTEGER, which I tried > > > to avoid in previous patch. I did this to fix a bug where result of the > > > operation is DOUBLE if one of the operands is DOUBLE. It didn't help, the > > > result still has DOUBLE type. I decided to left conversion since it looks right > > > here because all operands must be INTEGERS. This wouldn't work for arithmetic > > > operations though. > > > > Не понял ничего..Как эта штука должна работать?? > > box.execute([[SELECT 3.5 | 1.3;]]) > > metadata: > > name: COLUMN_1 > > type: double > > rows: > > [3] -- WTF > > > В соответствии с правилами implicit cast between numeric values. В любом случае, > убрал возможность делать значения типа DOUBLE операндами битовых операций. > > > Давай по-честному выдавать ошибку, когда в битовые операции суем не инты. > > Сделал. > > > > > > From 3515ada4b363062cf9caa5d550ea40770e8a5e65 Mon Sep 17 00:00:00 2001 > > > From: Mergen Imeev <imeevma@gmail.com> > > > Date: Tue, 18 Aug 2020 18:18:59 +0300 > > > Subject: [PATCH] sql: remove implicit cast in bitwise operations > > > > > > This patch removes the implicit conversion from STRING to INTEGER from > > > bitwise operations. However, DOUBLE can still be implicitly converted to > > > INTEGER. > > > > > > Follow-up #3809 > > > > > > > Новый патч: > > > From a630c39d486769d1a684e56b15bbf5107e7bef66 Mon Sep 17 00:00:00 2001 > From: Mergen Imeev <imeevma@gmail.com> > Date: Tue, 18 Aug 2020 18:18:59 +0300 > Subject: [PATCH] sql: remove implicit cast in bitwise operations > > This patch removes the implicit conversion from STRING to INTEGER from > bitwise operations. Only integer values allowed in bitwise operations. > > Follow-up #3809 > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > index c0143a6b1..5d1ab8c94 100644 > --- a/src/box/sql/vdbe.c > +++ b/src/box/sql/vdbe.c > @@ -1989,23 +1989,26 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ > > pIn1 = &aMem[pOp->p1]; > pIn2 = &aMem[pOp->p2]; > + enum mp_type type1 = mem_mp_type(pIn1); > + enum mp_type type2 = mem_mp_type(pIn2); > pOut = vdbe_prepare_null_out(p, pOp->p3); > - if ((pIn1->flags | pIn2->flags) & MEM_Null) { > + if (type1 == MP_NIL || type2 == MP_NIL) { > /* Force NULL be of type INTEGER. */ > pOut->field_type = FIELD_TYPE_INTEGER; > break; > } > - bool unused; > - if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) { > + if (type2 != MP_INT && type2 != MP_UINT) { > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > sql_value_to_diag_str(pIn2), "integer"); > goto abort_due_to_error; > } > - if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) { > + if (type1 != MP_INT && type1 != MP_UINT) { > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > sql_value_to_diag_str(pIn1), "integer"); > goto abort_due_to_error; > } > + iA = pIn2->u.i; > + iB = pIn1->u.i; Это будет работать с интами > 2^32? В любом случае, я думаю, надо добавить тестов.. select 18446744073709551615 | 4 --- - metadata: - name: COLUMN_1 type: integer rows: - [-1] ... Выглядит неверно... > op = pOp->opcode; > if (op==OP_BitAnd) { > iA &= iB; > @@ -2621,19 +2624,18 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ > */ > case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ > pIn1 = &aMem[pOp->p1]; > + enum mp_type type = mem_mp_type(pIn1); > pOut = vdbe_prepare_null_out(p, pOp->p2); > /* Force NULL be of type INTEGER. */ > pOut->field_type = FIELD_TYPE_INTEGER; > - if ((pIn1->flags & MEM_Null)==0) { > - int64_t i; > - bool is_neg; > - if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) { > - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > - sql_value_to_diag_str(pIn1), "integer"); > - goto abort_due_to_error; > - } > - mem_set_i64(pOut, ~i); > + if (type == MP_NIL) > + break; > + if (type != MP_INT && type != MP_UINT) { > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > + sql_value_to_diag_str(pIn1), "integer"); > + goto abort_due_to_error; > } > + mem_set_i64(pOut, ~pIn1->u.i); > break; > } > > diff --git a/test/sql/types.result b/test/sql/types.result > index caedbf409..a79818b30 100644 > --- a/test/sql/types.result > +++ b/test/sql/types.result > @@ -2846,3 +2846,284 @@ box.execute([[SELECT 1 - '2';]]) > - null > - 'Type mismatch: can not convert 2 to numeric' > ... > +-- > +-- Make sure there is no implicit string-to-number conversion in bitwise > +-- operations. > +-- > +box.execute([[SELECT '1' | 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT '1' & 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT '1' << 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT '1' >> 2;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT ~'1';]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT 1 | '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > +box.execute([[SELECT 1 & '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > +box.execute([[SELECT 1 << '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > +box.execute([[SELECT 1 >> '2';]]) > +--- > +- null > +- 'Type mismatch: can not convert 2 to integer' > +... > +-- > +-- Make sure that only values of type INTEGER can be operands of bitwise > +-- operations. > +-- > +box.execute([[SELECT 3 | 1;]]) > +--- > +- metadata: > + - name: COLUMN_1 > + type: integer > + rows: > + - [3] > +... > +box.execute([[SELECT 3 | 1.1;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT 3 | '1';]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT 3 | true;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT 3 | X'31';]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT 1 | 3;]]) > +--- > +- metadata: > + - name: COLUMN_1 > + type: integer > + rows: > + - [3] > +... > +box.execute([[SELECT 1.1 | 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT '1' | 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT true | 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT X'31' | 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT 3 & 1;]]) > +--- > +- metadata: > + - name: COLUMN_1 > + type: integer > + rows: > + - [1] > +... > +box.execute([[SELECT 3 & 1.1;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT 3 & '1';]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT 3 & true;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT 3 & X'31';]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT 1.1 & 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT '1' & 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT true & 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT X'31' & 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT 3 << 1;]]) > +--- > +- metadata: > + - name: COLUMN_1 > + type: integer > + rows: > + - [6] > +... > +box.execute([[SELECT 3 << 1.1;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT 3 << '1';]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT 3 << true;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT 3 << X'31';]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT 1.1 << 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT '1' << 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT true << 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT X'31' << 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT 3 >> 1;]]) > +--- > +- metadata: > + - name: COLUMN_1 > + type: integer > + rows: > + - [1] > +... > +box.execute([[SELECT 3 >> 1.1;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT 3 >> '1';]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT 3 >> true;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT 3 >> X'31';]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT 1.1 >> 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT '1' >> 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT true >> 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT X'31' >> 3;]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > +box.execute([[SELECT ~1;]]) > +--- > +- metadata: > + - name: COLUMN_1 > + type: integer > + rows: > + - [-2] > +... > +box.execute([[SELECT ~1.1;]]) > +--- > +- null > +- 'Type mismatch: can not convert 1.1 to integer' > +... > +box.execute([[SELECT ~'1';]]) > +--- > +- null > +- 'Type mismatch: can not convert 1 to integer' > +... > +box.execute([[SELECT ~true;]]) > +--- > +- null > +- 'Type mismatch: can not convert TRUE to integer' > +... > +box.execute([[SELECT ~X'31';]]) > +--- > +- null > +- 'Type mismatch: can not convert varbinary to integer' > +... > diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua > index 844a6b670..bb922e78f 100644 > --- a/test/sql/types.test.lua > +++ b/test/sql/types.test.lua > @@ -638,3 +638,68 @@ box.execute([[SELECT 1 % '2';]]) > box.execute([[SELECT 1 * '2';]]) > box.execute([[SELECT 1 / '2';]]) > box.execute([[SELECT 1 - '2';]]) > + > +-- > +-- Make sure there is no implicit string-to-number conversion in bitwise > +-- 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';]]) > +box.execute([[SELECT 1 | '2';]]) > +box.execute([[SELECT 1 & '2';]]) > +box.execute([[SELECT 1 << '2';]]) > +box.execute([[SELECT 1 >> '2';]]) > + > +-- > +-- Make sure that only values of type INTEGER can be operands of bitwise > +-- operations. > +-- > +box.execute([[SELECT 3 | 1;]]) > +box.execute([[SELECT 3 | 1.1;]]) > +box.execute([[SELECT 3 | '1';]]) > +box.execute([[SELECT 3 | true;]]) > +box.execute([[SELECT 3 | X'31';]]) > +box.execute([[SELECT 1 | 3;]]) > +box.execute([[SELECT 1.1 | 3;]]) > +box.execute([[SELECT '1' | 3;]]) > +box.execute([[SELECT true | 3;]]) > +box.execute([[SELECT X'31' | 3;]]) > + > +box.execute([[SELECT 3 & 1;]]) > +box.execute([[SELECT 3 & 1.1;]]) > +box.execute([[SELECT 3 & '1';]]) > +box.execute([[SELECT 3 & true;]]) > +box.execute([[SELECT 3 & X'31';]]) > +box.execute([[SELECT 1.1 & 3;]]) > +box.execute([[SELECT '1' & 3;]]) > +box.execute([[SELECT true & 3;]]) > +box.execute([[SELECT X'31' & 3;]]) > + > +box.execute([[SELECT 3 << 1;]]) > +box.execute([[SELECT 3 << 1.1;]]) > +box.execute([[SELECT 3 << '1';]]) > +box.execute([[SELECT 3 << true;]]) > +box.execute([[SELECT 3 << X'31';]]) > +box.execute([[SELECT 1.1 << 3;]]) > +box.execute([[SELECT '1' << 3;]]) > +box.execute([[SELECT true << 3;]]) > +box.execute([[SELECT X'31' << 3;]]) > + > +box.execute([[SELECT 3 >> 1;]]) > +box.execute([[SELECT 3 >> 1.1;]]) > +box.execute([[SELECT 3 >> '1';]]) > +box.execute([[SELECT 3 >> true;]]) > +box.execute([[SELECT 3 >> X'31';]]) > +box.execute([[SELECT 1.1 >> 3;]]) > +box.execute([[SELECT '1' >> 3;]]) > +box.execute([[SELECT true >> 3;]]) > +box.execute([[SELECT X'31' >> 3;]]) > + > +box.execute([[SELECT ~1;]]) > +box.execute([[SELECT ~1.1;]]) > +box.execute([[SELECT ~'1';]]) > +box.execute([[SELECT ~true;]]) > +box.execute([[SELECT ~X'31';]]) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations 2020-09-28 17:13 ` Nikita Pettik @ 2020-09-29 13:00 ` Mergen Imeev 0 siblings, 0 replies; 10+ messages in thread From: Mergen Imeev @ 2020-09-29 13:00 UTC (permalink / raw) To: Nikita Pettik; +Cc: tarantool-patches Привет! Спасибо за ревью. Мои ответы ниже. On Mon, Sep 28, 2020 at 05:13:40PM +0000, Nikita Pettik wrote: > On 27 Sep 12:11, Mergen Imeev wrote: > > Привет! Спасибо за ревью. Мои ответы и новый патч ниже. > > > > On Thu, Sep 17, 2020 at 01:36:48PM +0000, Nikita Pettik wrote: > > > On 21 Aug 15:34, Mergen Imeev wrote: > > > > Hi! Thank you for the review. My answer and new patch below. > > > > > > > > On Fri, Aug 21, 2020 at 09:21:30AM +0000, Nikita Pettik wrote: > > > > > On 21 Aug 11:40, imeevma@tarantool.org wrote: > > > > > > This patch removes the implicit conversion from STRING to INTEGER from > > > > > > bitwise operations. However, DOUBLE can still be implicitly converted to > > > > > > INTEGER. > > > > > > > > > > I see no test involving doubles in bitwise operations. Does it make > > > > > any sense at all? > > > > Even if it doesn't, it is legal to implicitly convert DOUBLE to INTEGER. > > > > > > > > However, when I tried to add tests for this case, I found an error in my patch. > > > > I re-made patch. Now in these opcodes we convert MEM to INTEGER, which I tried > > > > to avoid in previous patch. I did this to fix a bug where result of the > > > > operation is DOUBLE if one of the operands is DOUBLE. It didn't help, the > > > > result still has DOUBLE type. I decided to left conversion since it looks right > > > > here because all operands must be INTEGERS. This wouldn't work for arithmetic > > > > operations though. > > > > > > Не понял ничего..Как эта штука должна работать?? > > > box.execute([[SELECT 3.5 | 1.3;]]) > > > metadata: > > > name: COLUMN_1 > > > type: double > > > rows: > > > [3] -- WTF > > > > > В соответствии с правилами implicit cast between numeric values. В любом случае, > > убрал возможность делать значения типа DOUBLE операндами битовых операций. > > > > > Давай по-честному выдавать ошибку, когда в битовые операции суем не инты. > > > > Сделал. > > > > > > > > > From 3515ada4b363062cf9caa5d550ea40770e8a5e65 Mon Sep 17 00:00:00 2001 > > > > From: Mergen Imeev <imeevma@gmail.com> > > > > Date: Tue, 18 Aug 2020 18:18:59 +0300 > > > > Subject: [PATCH] sql: remove implicit cast in bitwise operations > > > > > > > > This patch removes the implicit conversion from STRING to INTEGER from > > > > bitwise operations. However, DOUBLE can still be implicitly converted to > > > > INTEGER. > > > > > > > > Follow-up #3809 > > > > > > > > > > > > Новый патч: > > > > > > From a630c39d486769d1a684e56b15bbf5107e7bef66 Mon Sep 17 00:00:00 2001 > > From: Mergen Imeev <imeevma@gmail.com> > > Date: Tue, 18 Aug 2020 18:18:59 +0300 > > Subject: [PATCH] sql: remove implicit cast in bitwise operations > > > > This patch removes the implicit conversion from STRING to INTEGER from > > bitwise operations. Only integer values allowed in bitwise operations. > > > > Follow-up #3809 > > > > diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > > index c0143a6b1..5d1ab8c94 100644 > > --- a/src/box/sql/vdbe.c > > +++ b/src/box/sql/vdbe.c > > @@ -1989,23 +1989,26 @@ case OP_ShiftRight: { /* same as TK_RSHIFT, in1, in2, out3 */ > > > > pIn1 = &aMem[pOp->p1]; > > pIn2 = &aMem[pOp->p2]; > > + enum mp_type type1 = mem_mp_type(pIn1); > > + enum mp_type type2 = mem_mp_type(pIn2); > > pOut = vdbe_prepare_null_out(p, pOp->p3); > > - if ((pIn1->flags | pIn2->flags) & MEM_Null) { > > + if (type1 == MP_NIL || type2 == MP_NIL) { > > /* Force NULL be of type INTEGER. */ > > pOut->field_type = FIELD_TYPE_INTEGER; > > break; > > } > > - bool unused; > > - if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) { > > + if (type2 != MP_INT && type2 != MP_UINT) { > > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > > sql_value_to_diag_str(pIn2), "integer"); > > goto abort_due_to_error; > > } > > - if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) { > > + if (type1 != MP_INT && type1 != MP_UINT) { > > diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > > sql_value_to_diag_str(pIn1), "integer"); > > goto abort_due_to_error; > > } > > + iA = pIn2->u.i; > > + iB = pIn1->u.i; > > Это будет работать с интами > 2^32? В любом случае, я думаю, надо добавить > тестов.. > С 2^32 проблем не должно бить, но возможны проблемы с числами равными или больше 2^63. Тесты предлагаю делать в рамках тикета по этой теме (#5364). > select 18446744073709551615 | 4 > --- > - metadata: > - name: COLUMN_1 > type: integer > rows: > - [-1] > ... > > Выглядит неверно... > Создал на это отдельный тикет(#5364), предлагаю решать этот вопрос там. > > op = pOp->opcode; > > if (op==OP_BitAnd) { > > iA &= iB; > > @@ -2621,19 +2624,18 @@ case OP_Not: { /* same as TK_NOT, in1, out2 */ > > */ > > case OP_BitNot: { /* same as TK_BITNOT, in1, out2 */ > > pIn1 = &aMem[pOp->p1]; > > + enum mp_type type = mem_mp_type(pIn1); > > pOut = vdbe_prepare_null_out(p, pOp->p2); > > /* Force NULL be of type INTEGER. */ > > pOut->field_type = FIELD_TYPE_INTEGER; > > - if ((pIn1->flags & MEM_Null)==0) { > > - int64_t i; > > - bool is_neg; > > - if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) { > > - diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > > - sql_value_to_diag_str(pIn1), "integer"); > > - goto abort_due_to_error; > > - } > > - mem_set_i64(pOut, ~i); > > + if (type == MP_NIL) > > + break; > > + if (type != MP_INT && type != MP_UINT) { > > + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, > > + sql_value_to_diag_str(pIn1), "integer"); > > + goto abort_due_to_error; > > } > > + mem_set_i64(pOut, ~pIn1->u.i); > > break; > > } > > > > diff --git a/test/sql/types.result b/test/sql/types.result > > index caedbf409..a79818b30 100644 > > --- a/test/sql/types.result > > +++ b/test/sql/types.result > > @@ -2846,3 +2846,284 @@ box.execute([[SELECT 1 - '2';]]) > > - null > > - 'Type mismatch: can not convert 2 to numeric' > > ... > > +-- > > +-- Make sure there is no implicit string-to-number conversion in bitwise > > +-- operations. > > +-- > > +box.execute([[SELECT '1' | 2;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT '1' & 2;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT '1' << 2;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT '1' >> 2;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT ~'1';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT 1 | '2';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 2 to integer' > > +... > > +box.execute([[SELECT 1 & '2';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 2 to integer' > > +... > > +box.execute([[SELECT 1 << '2';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 2 to integer' > > +... > > +box.execute([[SELECT 1 >> '2';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 2 to integer' > > +... > > +-- > > +-- Make sure that only values of type INTEGER can be operands of bitwise > > +-- operations. > > +-- > > +box.execute([[SELECT 3 | 1;]]) > > +--- > > +- metadata: > > + - name: COLUMN_1 > > + type: integer > > + rows: > > + - [3] > > +... > > +box.execute([[SELECT 3 | 1.1;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT 3 | '1';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT 3 | true;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT 3 | X'31';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT 1 | 3;]]) > > +--- > > +- metadata: > > + - name: COLUMN_1 > > + type: integer > > + rows: > > + - [3] > > +... > > +box.execute([[SELECT 1.1 | 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT '1' | 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT true | 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT X'31' | 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT 3 & 1;]]) > > +--- > > +- metadata: > > + - name: COLUMN_1 > > + type: integer > > + rows: > > + - [1] > > +... > > +box.execute([[SELECT 3 & 1.1;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT 3 & '1';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT 3 & true;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT 3 & X'31';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT 1.1 & 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT '1' & 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT true & 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT X'31' & 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT 3 << 1;]]) > > +--- > > +- metadata: > > + - name: COLUMN_1 > > + type: integer > > + rows: > > + - [6] > > +... > > +box.execute([[SELECT 3 << 1.1;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT 3 << '1';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT 3 << true;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT 3 << X'31';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT 1.1 << 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT '1' << 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT true << 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT X'31' << 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT 3 >> 1;]]) > > +--- > > +- metadata: > > + - name: COLUMN_1 > > + type: integer > > + rows: > > + - [1] > > +... > > +box.execute([[SELECT 3 >> 1.1;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT 3 >> '1';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT 3 >> true;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT 3 >> X'31';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT 1.1 >> 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT '1' >> 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT true >> 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT X'31' >> 3;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > +box.execute([[SELECT ~1;]]) > > +--- > > +- metadata: > > + - name: COLUMN_1 > > + type: integer > > + rows: > > + - [-2] > > +... > > +box.execute([[SELECT ~1.1;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1.1 to integer' > > +... > > +box.execute([[SELECT ~'1';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert 1 to integer' > > +... > > +box.execute([[SELECT ~true;]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert TRUE to integer' > > +... > > +box.execute([[SELECT ~X'31';]]) > > +--- > > +- null > > +- 'Type mismatch: can not convert varbinary to integer' > > +... > > diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua > > index 844a6b670..bb922e78f 100644 > > --- a/test/sql/types.test.lua > > +++ b/test/sql/types.test.lua > > @@ -638,3 +638,68 @@ box.execute([[SELECT 1 % '2';]]) > > box.execute([[SELECT 1 * '2';]]) > > box.execute([[SELECT 1 / '2';]]) > > box.execute([[SELECT 1 - '2';]]) > > + > > +-- > > +-- Make sure there is no implicit string-to-number conversion in bitwise > > +-- 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';]]) > > +box.execute([[SELECT 1 | '2';]]) > > +box.execute([[SELECT 1 & '2';]]) > > +box.execute([[SELECT 1 << '2';]]) > > +box.execute([[SELECT 1 >> '2';]]) > > + > > +-- > > +-- Make sure that only values of type INTEGER can be operands of bitwise > > +-- operations. > > +-- > > +box.execute([[SELECT 3 | 1;]]) > > +box.execute([[SELECT 3 | 1.1;]]) > > +box.execute([[SELECT 3 | '1';]]) > > +box.execute([[SELECT 3 | true;]]) > > +box.execute([[SELECT 3 | X'31';]]) > > +box.execute([[SELECT 1 | 3;]]) > > +box.execute([[SELECT 1.1 | 3;]]) > > +box.execute([[SELECT '1' | 3;]]) > > +box.execute([[SELECT true | 3;]]) > > +box.execute([[SELECT X'31' | 3;]]) > > + > > +box.execute([[SELECT 3 & 1;]]) > > +box.execute([[SELECT 3 & 1.1;]]) > > +box.execute([[SELECT 3 & '1';]]) > > +box.execute([[SELECT 3 & true;]]) > > +box.execute([[SELECT 3 & X'31';]]) > > +box.execute([[SELECT 1.1 & 3;]]) > > +box.execute([[SELECT '1' & 3;]]) > > +box.execute([[SELECT true & 3;]]) > > +box.execute([[SELECT X'31' & 3;]]) > > + > > +box.execute([[SELECT 3 << 1;]]) > > +box.execute([[SELECT 3 << 1.1;]]) > > +box.execute([[SELECT 3 << '1';]]) > > +box.execute([[SELECT 3 << true;]]) > > +box.execute([[SELECT 3 << X'31';]]) > > +box.execute([[SELECT 1.1 << 3;]]) > > +box.execute([[SELECT '1' << 3;]]) > > +box.execute([[SELECT true << 3;]]) > > +box.execute([[SELECT X'31' << 3;]]) > > + > > +box.execute([[SELECT 3 >> 1;]]) > > +box.execute([[SELECT 3 >> 1.1;]]) > > +box.execute([[SELECT 3 >> '1';]]) > > +box.execute([[SELECT 3 >> true;]]) > > +box.execute([[SELECT 3 >> X'31';]]) > > +box.execute([[SELECT 1.1 >> 3;]]) > > +box.execute([[SELECT '1' >> 3;]]) > > +box.execute([[SELECT true >> 3;]]) > > +box.execute([[SELECT X'31' >> 3;]]) > > + > > +box.execute([[SELECT ~1;]]) > > +box.execute([[SELECT ~1.1;]]) > > +box.execute([[SELECT ~'1';]]) > > +box.execute([[SELECT ~true;]]) > > +box.execute([[SELECT ~X'31';]]) ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-09-29 13:00 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-08-21 8:40 [Tarantool-patches] [PATCH v1 0/2] sql: remove implicit cast from operations imeevma 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 1/2] sql: remove implicit cast in arithmetic operations imeevma 2020-08-21 8:59 ` Nikita Pettik 2020-08-21 8:40 ` [Tarantool-patches] [PATCH v1 2/2] sql: remove implicit cast in bitwise operations imeevma 2020-08-21 9:21 ` Nikita Pettik 2020-08-21 12:34 ` Mergen Imeev 2020-09-17 13:36 ` Nikita Pettik 2020-09-27 9:11 ` Mergen Imeev 2020-09-28 17:13 ` Nikita Pettik 2020-09-29 13:00 ` Mergen Imeev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox