[Tarantool-patches] [PATCH v1 0/2] sql: remove implicit cast from operations

[Tarantool-patches] [PATCH v1 0/2] sql: remove implicit cast from operations

[imeevma]

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

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

[imeevma]

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

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

[imeevma]

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

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

[Nikita Pettik]

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

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

[Nikita Pettik]

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
> 

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

[Mergen Imeev]

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;]])

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

[Nikita Pettik]

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
> 

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

[Mergen Imeev]

Привет! Спасибо за ревью. Мои ответы и новый патч ниже.

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';]])

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

[Nikita Pettik]

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';]])

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

[Mergen Imeev]

Привет! Спасибо за ревью. Мои ответы ниже.

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';]])