[tarantool-patches] [PATCH 1/2] sql: do not print binary data in diag_set()

Roman Khabibov roman.habibov at tarantool.org
Sun Sep 15 22:39:10 MSK 2019


Print the data type instead of the data itself in diag_set() in
the case of binary data. The reason of this patch is that LibYAML
converts whole error message to base64 in the case of
non-printable symbols.

Closes #4356
---
 src/box/sql/func.c                   |  8 +--
 src/box/sql/sqlInt.h                 | 13 ++++
 src/box/sql/vdbe.c                   | 28 ++++-----
 src/box/sql/vdbeapi.c                |  7 +++
 test/sql-tap/cast.test.lua           |  4 +-
 test/sql-tap/func.test.lua           | 45 ++++++++++++-
 test/sql-tap/sql-errors.test.lua     | 94 +++++++++++++++++++++++++++-
 test/sql-tap/tkt-80e031a00f.test.lua |  4 +-
 test/sql/types.result                |  8 +--
 9 files changed, 183 insertions(+), 28 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 1d8f28a51..631003672 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -812,7 +812,7 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
 		return;
 	if (sql_value_type(argv[0]) == MP_BIN) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(argv[0]), "numeric");
+			 sql_diag_text(argv[0]), "numeric");
 		context->is_aborted = true;
 		return;
 	}
@@ -954,7 +954,7 @@ randomBlob(sql_context * context, int argc, sql_value ** argv)
 	UNUSED_PARAMETER(argc);
 	if (sql_value_type(argv[0]) == MP_BIN) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(argv[0]), "numeric");
+			 sql_diag_text(argv[0]), "numeric");
 		context->is_aborted = true;
 		return;
 	}
@@ -1842,7 +1842,7 @@ soundexFunc(sql_context * context, int argc, sql_value ** argv)
 	assert(argc == 1);
 	if (sql_value_type(argv[0]) == MP_BIN) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(argv[0]), "TEXT");
+			 sql_diag_text(argv[0]), "TEXT");
 		context->is_aborted = true;
 		return;
 	}
@@ -1915,7 +1915,7 @@ sum_step(struct sql_context *context, int argc, sql_value **argv)
 	if (type != MP_DOUBLE && type != MP_INT && type != MP_UINT) {
 		if (mem_apply_numeric_type(argv[0]) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(argv[0]), "number");
+				 sql_diag_text(argv[0]), "number");
 			context->is_aborted = true;
 			return;
 		}
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index e617edd79..05727b572 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -375,6 +375,19 @@ sql_value_uint64(sql_value *val);
 const unsigned char *
 sql_value_text(sql_value *);
 
+/**
+ * Return pointer to string with the data type in the case of
+ * binary data stored in @a value. Otherwise, return the result
+ * of sql_value_text().
+ *
+ * Wrapper for sql_value_text().
+ *
+ * retval Ptr to constant string "binary".
+ * retval Ptr to the result of sql_value_text().
+ */
+const unsigned char *
+sql_diag_text(sql_value *);
+
 enum mp_type
 sql_value_type(sql_value *);
 
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 02e16da19..a154e34e3 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -1619,12 +1619,12 @@ case OP_Remainder: {           /* same as TK_REM, in1, in2, out3 */
 		bIntint = 0;
 		if (sqlVdbeRealValue(pIn1, &rA) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn1), "numeric");
+				 sql_diag_text(pIn1), "numeric");
 			goto abort_due_to_error;
 		}
 		if (sqlVdbeRealValue(pIn2, &rB) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn2), "numeric");
+				 sql_diag_text(pIn2), "numeric");
 			goto abort_due_to_error;
 		}
 		switch( pOp->opcode) {
@@ -1887,12 +1887,12 @@ case OP_ShiftRight: {           /* same as TK_RSHIFT, in1, in2, out3 */
 	bool unused;
 	if (sqlVdbeIntValue(pIn2, (int64_t *) &iA, &unused) != 0) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(pIn2), "integer");
+			 sql_diag_text(pIn2), "integer");
 		goto abort_due_to_error;
 	}
 	if (sqlVdbeIntValue(pIn1, (int64_t *) &iB, &unused) != 0) {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(pIn1), "integer");
+			 sql_diag_text(pIn1), "integer");
 		goto abort_due_to_error;
 	}
 	op = pOp->opcode;
@@ -1957,7 +1957,7 @@ case OP_MustBeInt: {            /* jump, in1 */
 		if ((pIn1->flags & (MEM_Int | MEM_UInt)) == 0) {
 			if (pOp->p2==0) {
 				diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-					 sql_value_text(pIn1), "integer");
+					 sql_diag_text(pIn1), "integer");
 				goto abort_due_to_error;
 			} else {
 				goto jump_to_p2;
@@ -2014,7 +2014,7 @@ case OP_Cast: {                  /* in1 */
 	UPDATE_MAX_BLOBSIZE(pIn1);
 	if (rc == 0)
 		break;
-	diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_value_text(pIn1),
+	diag_set(ClientError, ER_SQL_TYPE_MISMATCH, sql_diag_text(pIn1),
 		 field_type_strs[pOp->p2]);
 	goto abort_due_to_error;
 }
@@ -2187,7 +2187,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 					if (mem_apply_numeric_type(pIn3) != 0) {
 						diag_set(ClientError,
 							 ER_SQL_TYPE_MISMATCH,
-							 sql_value_text(pIn3),
+							 sql_diag_text(pIn3),
 							 "numeric");
 						goto abort_due_to_error;
 					}
@@ -2453,7 +2453,7 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 		v1 = pIn1->u.b;
 	} else {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(pIn1), "boolean");
+			 sql_diag_text(pIn1), "boolean");
 		goto abort_due_to_error;
 	}
 	pIn2 = &aMem[pOp->p2];
@@ -2463,7 +2463,7 @@ case OP_Or: {             /* same as TK_OR, in1, in2, out3 */
 		v2 = pIn2->u.b;
 	} else {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(pIn2), "boolean");
+			 sql_diag_text(pIn2), "boolean");
 		goto abort_due_to_error;
 	}
 	if (pOp->opcode==OP_And) {
@@ -2493,7 +2493,7 @@ case OP_Not: {                /* same as TK_NOT, in1, out2 */
 	if ((pIn1->flags & MEM_Null)==0) {
 		if ((pIn1->flags & MEM_Bool) == 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn1), "boolean");
+				 sql_diag_text(pIn1), "boolean");
 			goto abort_due_to_error;
 		}
 		mem_set_bool(pOut, ! pIn1->u.b);
@@ -2518,7 +2518,7 @@ case OP_BitNot: {             /* same as TK_BITNOT, in1, out2 */
 		bool is_neg;
 		if (sqlVdbeIntValue(pIn1, &i, &is_neg) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn1), "integer");
+				 sql_diag_text(pIn1), "integer");
 			goto abort_due_to_error;
 		}
 		mem_set_i64(pOut, ~i);
@@ -2564,7 +2564,7 @@ case OP_IfNot: {            /* jump, in1 */
 		c = pOp->opcode == OP_IfNot ? ! pIn1->u.b : pIn1->u.b;
 	} else {
 		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-			 sql_value_text(pIn1), "boolean");
+			 sql_diag_text(pIn1), "boolean");
 		goto abort_due_to_error;
 	}
 	VdbeBranchTaken(c!=0, 2);
@@ -2752,7 +2752,7 @@ case OP_ApplyType: {
 		assert(memIsValid(pIn1));
 		if (mem_apply_type(pIn1, type) != 0) {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn1),
+				 sql_diag_text(pIn1),
 				 field_type_strs[type]);
 			goto abort_due_to_error;
 		}
@@ -3385,7 +3385,7 @@ case OP_SeekGT: {       /* jump, in3 */
 			is_neg = i < 0;
 		} else {
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 sql_value_text(pIn3), "integer");
+				 sql_diag_text(pIn3), "integer");
 			goto abort_due_to_error;
 		}
 		iKey = i;
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index bf40b44fa..c48be87a7 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -202,6 +202,13 @@ sql_value_text(sql_value * pVal)
 	return (const unsigned char *)sqlValueText(pVal);
 }
 
+const unsigned char *
+sql_diag_text(sql_value *value)
+{
+	return sql_value_type(value) == MP_BIN ?
+	       (const unsigned char *) "VARBINARY" : sql_value_text(value);
+}
+
 /* EVIDENCE-OF: R-12793-43283 Every value in sql has one of five
  * fundamental datatypes: 64-bit signed integer 64-bit IEEE floating
  * point number string BLOB NULL
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 553c72e44..1eba15b38 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -70,7 +70,7 @@ test:do_catchsql_test(
         SELECT CAST(x'616263' AS NUMBER)
     ]], {
         -- <cast-1.5>
-        1, 'Type mismatch: can not convert abc to number'
+        1, 'Type mismatch: can not convert VARBINARY to number'
         -- </cast-1.5>
     })
 
@@ -100,7 +100,7 @@ test:do_catchsql_test(
         SELECT CAST(x'616263' AS integer)
     ]], {
         -- <cast-1.9>
-        1, 'Type mismatch: can not convert abc to integer'
+        1, 'Type mismatch: can not convert VARBINARY to integer'
         -- </cast-1.9>
     })
 
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index a96fc611e..2b93fd83b 100755
--- a/test/sql-tap/func.test.lua
+++ b/test/sql-tap/func.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(14606)
+test:plan(14610)
 
 --!./tcltestrunner.lua
 -- 2001 September 15
@@ -2909,4 +2909,47 @@ test:do_execsql_test(
     "SELECT POSITION(CHAR(00), CHAR(65,66));",
     {0})
 
+-- gh-4356: Make sure that 'VARBINARY' is printed instead of the
+-- binary data itself.
+
+test:do_catchsql_test(
+    "func-76.1",
+    [[
+        SELECT ROUND(X'FF')
+    ]], {
+        -- <func-76.1>
+        1, "Type mismatch: can not convert VARBINARY to numeric"
+        -- </func-76.1>
+    })
+
+test:do_catchsql_test(
+    "func-76.2",
+    [[
+        SELECT RANDOMBLOB(X'FF')
+    ]], {
+        -- <func-76.2>
+        1, "Type mismatch: can not convert VARBINARY to numeric"
+        -- </func-76.2>
+    })
+
+test:do_catchsql_test(
+    "func-76.3",
+    [[
+        SELECT SOUNDEX(X'FF')
+    ]], {
+        -- <func-76.3>
+        1, "Type mismatch: can not convert VARBINARY to TEXT"
+        -- </func-76.3>
+    })
+
+test:do_catchsql_test(
+    "func-76.4",
+    [[
+        SELECT SUM(X'FF')
+    ]], {
+        -- <func-76.4>
+        1, "Type mismatch: can not convert VARBINARY to number"
+        -- </func-76.4>
+    })
+
 test:finish_test()
diff --git a/test/sql-tap/sql-errors.test.lua b/test/sql-tap/sql-errors.test.lua
index d11c5c597..ec9d53dd0 100755
--- a/test/sql-tap/sql-errors.test.lua
+++ b/test/sql-tap/sql-errors.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(63)
+test:plan(72)
 
 test:execsql([[
 	CREATE TABLE t0 (i INT PRIMARY KEY, a INT);
@@ -686,4 +686,96 @@ test:do_catchsql_test(
 		-- </sql-errors-1.63>
 	})
 
+-- gh-4356: Make sure that 'VARBINARY' is printed instead of the
+-- binary data itself.
+test:do_catchsql_test(
+	"sql-errors-2.1",
+	[[
+		SELECT X'ff' + 1;
+	]], {
+		-- <sql-errors-2.1>
+		1, "Type mismatch: can not convert VARBINARY to numeric"
+		-- </sql-errors-2.1>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.2",
+	[[
+		SELECT X'ff' - 1;
+	]], {
+		-- <sql-errors-2.2>
+		1, "Type mismatch: can not convert VARBINARY to numeric"
+		-- </sql-errors-2.2>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.3",
+	[[
+		SELECT X'ff' * 1;
+	]], {
+		-- <sql-errors-2.3>
+		1, "Type mismatch: can not convert VARBINARY to numeric"
+		-- </sql-errors-2.3>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.4",
+	[[
+		SELECT X'ff' / 1;
+	]], {
+		-- <sql-errors-2.4>
+		1, "Type mismatch: can not convert VARBINARY to numeric"
+		-- </sql-errors-2.4>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.5",
+	[[
+		SELECT X'ff' AND true;
+	]], {
+		-- <sql-errors-2.5>
+		1, "Type mismatch: can not convert VARBINARY to boolean"
+		-- </sql-errors-2.5>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.6",
+	[[
+		SELECT X'ff' OR false;
+	]], {
+		-- <sql-errors-2.6>
+		1, "Type mismatch: can not convert VARBINARY to boolean"
+		-- </sql-errors-2.6>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.7",
+	[[
+		SELECT false OR X'ff';
+	]], {
+		-- <sql-errors-2.7>
+		1, "Type mismatch: can not convert VARBINARY to boolean"
+		-- </sql-errors-2.7>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.8",
+	[[
+		SELECT X'ff' >= false;
+	]], {
+		-- <sql-errors-2.8>
+		1, "Type mismatch: can not convert VARBINARY to BOOLEAN"
+		-- </sql-errors-2.8>
+	})
+
+test:do_catchsql_test(
+	"sql-errors-2.9",
+	[[
+		SELECT X'ff' <= false;
+	]], {
+		-- <sql-errors-2.9>
+		1, "Type mismatch: can not convert VARBINARY to BOOLEAN"
+		-- </sql-errors-2.9>
+	})
+
 test:finish_test()
diff --git a/test/sql-tap/tkt-80e031a00f.test.lua b/test/sql-tap/tkt-80e031a00f.test.lua
index 75bae0ed2..6eedf367c 100755
--- a/test/sql-tap/tkt-80e031a00f.test.lua
+++ b/test/sql-tap/tkt-80e031a00f.test.lua
@@ -386,7 +386,7 @@ test:do_catchsql_test(
         SELECT x'303132' IN t1
     ]], {
         -- <tkt-80e031a00f.31>
-        1, 'Type mismatch: can not convert 012 to integer'
+        1, 'Type mismatch: can not convert VARBINARY to integer'
         -- </tkt-80e031a00f.31>
     })
 
@@ -396,7 +396,7 @@ test:do_catchsql_test(
         SELECT x'303132' NOT IN t1
     ]], {
         -- <tkt-80e031a00f.32>
-        1, 'Type mismatch: can not convert 012 to integer'
+        1, 'Type mismatch: can not convert VARBINARY to integer'
         -- </tkt-80e031a00f.32>
     })
 
diff --git a/test/sql/types.result b/test/sql/types.result
index 773586023..387a69500 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -344,7 +344,7 @@ box.execute("INSERT INTO tboolean VALUES (TRUE);")
 box.execute("SELECT * FROM tboolean WHERE s1 = x'44';")
 ---
 - null
-- 'Type mismatch: can not convert D to boolean'
+- 'Type mismatch: can not convert VARBINARY to boolean'
 ...
 box.execute("SELECT * FROM tboolean WHERE s1 = 'abc';")
 ---
@@ -1271,17 +1271,17 @@ box.execute("SELECT * FROM t WHERE v = x'616263'")
 box.execute("SELECT sum(v) FROM t;")
 ---
 - null
-- 'Type mismatch: can not convert abc to number'
+- 'Type mismatch: can not convert VARBINARY to number'
 ...
 box.execute("SELECT avg(v) FROM t;")
 ---
 - null
-- 'Type mismatch: can not convert abc to number'
+- 'Type mismatch: can not convert VARBINARY to number'
 ...
 box.execute("SELECT total(v) FROM t;")
 ---
 - null
-- 'Type mismatch: can not convert abc to number'
+- 'Type mismatch: can not convert VARBINARY to number'
 ...
 box.execute("SELECT min(v) FROM t;")
 ---
-- 
2.20.1 (Apple Git-117)





More information about the Tarantool-patches mailing list