[tarantool-patches] [PATCH 0/5] Introduce VARBINARY in SQL

[tarantool-patches] [PATCH 0/5] Introduce VARBINARY in SQL

[Nikita Pettik]

Branch: https://github.com/tarantool/tarantool/tree/np/sql-introduce-blob
Issue: https://github.com/tarantool/tarantool/issues/4206

This patch-set introduces new column type available in SQL - VARBINARY.
All values of VARBINARY type are stored as MP_BIN format type in
msgpack. Basically, prior to the current patch all literals starting
with x'...' format were assumed to be encoded with MP_BIN type when
were inserted into SCALAR field. This rule has been remained.
According to ANSI, values of VARBINARY type can't be converted to
any other type.

Nikita Pettik (5):
  sql: always erase numeric flag after stringifying
  sql: fix resulting type calculation for CASE-WHEN stmt
  sql: use 'varbinary' as a name of type instead of 'blob'
  sql: make built-ins raise errors for varbin args
  sql: introduce VARBINARY column type

 extra/mkkeywordhash.c                      |   1 +
 src/box/lua/lua_sql.c                      |   2 +-
 src/box/sql/expr.c                         |  29 ++-
 src/box/sql/func.c                         |  32 ++-
 src/box/sql/parse.y                        |   3 +-
 src/box/sql/vdbe.c                         |  38 ++--
 src/box/sql/vdbeInt.h                      |   2 +-
 src/box/sql/vdbeapi.c                      |   4 +-
 src/box/sql/vdbemem.c                      |  14 +-
 test/sql-tap/cast.test.lua                 |   4 +-
 test/sql-tap/func.test.lua                 |   2 +-
 test/sql-tap/keyword1.test.lua             |   3 +-
 test/sql-tap/lua_sql.test.lua              |   4 +-
 test/sql-tap/position.test.lua             |  16 +-
 test/sql/gh-3888-values-blob-assert.result |   4 +-
 test/sql/iproto.result                     |   4 +-
 test/sql/misc.result                       |   2 +-
 test/sql/types.result                      | 315 +++++++++++++++++++++++++++--
 test/sql/types.test.lua                    |  68 +++++++
 19 files changed, 473 insertions(+), 74 deletions(-)

-- 
2.15.1

[tarantool-patches] [PATCH 1/5] sql: always erase numeric flag after stringifying

[Nikita Pettik]

Function which converts values to string representation
(sqlVdbeMemStringify()) erase MEM_Int/MEM_Real/MEM_Bool flags only when
it is specified by 'force' parameter. Hence, when 'force' argument is
false, memory cell after conversion will contain string value, but flag
indicating its type will be equal to combination of MEM_Str and one of
mentioned flags. It seems to be remains of affinity routines, since in
current state memory cell must have only one type.  What is more, it can
lead to unpredicted consequences, for instance assertion fault
(sql_value_type() assumes that value has one specific type). Let's fix
it removing 'force' argument from sqlVdbeMemStringify() and always clean
up type flag.
---
 src/box/sql/vdbe.c    | 8 ++++----
 src/box/sql/vdbeInt.h | 2 +-
 src/box/sql/vdbemem.c | 9 +++++----
 3 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index c8887f9b7..9f3879910 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -183,7 +183,7 @@ vdbeTakeBranch(int iSrcLine, u8 I, u8 M)
  * already. Return non-zero if a malloc() fails.
  */
 #define Stringify(P)						\
-	if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P,0)) \
+	if(((P)->flags&(MEM_Str|MEM_Blob))==0 && sqlVdbeMemStringify(P)) \
 	{ goto no_mem; }
 
 /*
@@ -333,7 +333,7 @@ mem_apply_type(struct Mem *record, enum field_type type)
 		 */
 		if ((record->flags & MEM_Str) == 0) {
 			if ((record->flags & (MEM_Real | MEM_Int)))
-				sqlVdbeMemStringify(record, 1);
+				sqlVdbeMemStringify(record);
 		}
 		record->flags &= ~(MEM_Real | MEM_Int);
 		return 0;
@@ -2144,7 +2144,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			if ((flags1 & MEM_Str)==0 && (flags1 & (MEM_Int|MEM_Real))!=0) {
 				testcase( pIn1->flags & MEM_Int);
 				testcase( pIn1->flags & MEM_Real);
-				sqlVdbeMemStringify(pIn1, 1);
+				sqlVdbeMemStringify(pIn1);
 				testcase( (flags1&MEM_Dyn) != (pIn1->flags&MEM_Dyn));
 				flags1 = (pIn1->flags & ~MEM_TypeMask) | (flags1 & MEM_TypeMask);
 				assert(pIn1!=pIn3);
@@ -2152,7 +2152,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			if ((flags3 & MEM_Str)==0 && (flags3 & (MEM_Int|MEM_Real))!=0) {
 				testcase( pIn3->flags & MEM_Int);
 				testcase( pIn3->flags & MEM_Real);
-				sqlVdbeMemStringify(pIn3, 1);
+				sqlVdbeMemStringify(pIn3);
 				testcase( (flags3&MEM_Dyn) != (pIn3->flags&MEM_Dyn));
 				flags3 = (pIn3->flags & ~MEM_TypeMask) | (flags3 & MEM_TypeMask);
 			}
diff --git a/src/box/sql/vdbeInt.h b/src/box/sql/vdbeInt.h
index 6bfeecc85..7d2470b07 100644
--- a/src/box/sql/vdbeInt.h
+++ b/src/box/sql/vdbeInt.h
@@ -469,7 +469,7 @@ void sqlVdbeMemInit(Mem *, sql *, u32);
 void sqlVdbeMemSetNull(Mem *);
 void sqlVdbeMemSetZeroBlob(Mem *, int);
 int sqlVdbeMemMakeWriteable(Mem *);
-int sqlVdbeMemStringify(Mem *, u8);
+int sqlVdbeMemStringify(Mem *);
 int sqlVdbeIntValue(Mem *, int64_t *);
 int sqlVdbeMemIntegerify(Mem *, bool is_forced);
 int sqlVdbeRealValue(Mem *, double *);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index 4e4bd597d..d85148bc3 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -275,7 +275,7 @@ sqlVdbeMemNulTerminate(Mem * pMem)
  * user and the latter is an internal programming error.
  */
 int
-sqlVdbeMemStringify(Mem * pMem, u8 bForce)
+sqlVdbeMemStringify(Mem * pMem)
 {
 	int fg = pMem->flags;
 	const int nByte = 32;
@@ -292,16 +292,17 @@ sqlVdbeMemStringify(Mem * pMem, u8 bForce)
 	}
 	if (fg & MEM_Int) {
 		sql_snprintf(nByte, pMem->z, "%lld", pMem->u.i);
+		pMem->flags &= ~MEM_Int;
 	} else if ((fg & MEM_Bool) != 0) {
 		sql_snprintf(nByte, pMem->z, "%s", pMem->u.b ? "true" : "false");
+		pMem->flags &= ~MEM_Bool;
 	} else {
 		assert(fg & MEM_Real);
 		sql_snprintf(nByte, pMem->z, "%!.15g", pMem->u.r);
+		pMem->flags &= ~MEM_Real;
 	}
 	pMem->n = sqlStrlen30(pMem->z);
 	pMem->flags |= MEM_Str | MEM_Term;
-	if (bForce)
-		pMem->flags &= ~(MEM_Int | MEM_Real);
 	return 0;
 }
 
@@ -1107,7 +1108,7 @@ valueToText(sql_value * pVal)
 		pVal->flags |= MEM_Str;
 		sqlVdbeMemNulTerminate(pVal);	/* IMP: R-31275-44060 */
 	} else {
-		sqlVdbeMemStringify(pVal, 0);
+		sqlVdbeMemStringify(pVal);
 		assert(0 == (1 & SQL_PTR_TO_INT(pVal->z)));
 	}
 	return pVal->z;
-- 
2.15.1

[tarantool-patches] [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt

[Nikita Pettik]

Before this patch, resulting type for CASE-WHEN statement was assumed to
be the same as type of argument of first THEN clause. Obviously, it is
wrong and could lead to sad consequence (e.g. creating ephemeral table
with inconsistent format). To deal with this, we check all THEN
arguments: if all of them have the same type, then such type will be
resulting of the whole statement; if at least two types are different,
we can't determine actual resulting type during compilation stage and
assign SCALAR as a most general type in SQL now.

Need for #4206
---
 src/box/sql/expr.c      | 27 +++++++++++++++++++++------
 test/sql/types.result   | 35 +++++++++++++++++++++++++++++++++++
 test/sql/types.test.lua |  8 ++++++++
 3 files changed, 64 insertions(+), 6 deletions(-)

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index d7104d8a0..97f5bd180 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -85,18 +85,33 @@ sql_expr_type(struct Expr *pExpr)
 		return sql_type_result(rhs_type, lhs_type);
 	case TK_CONCAT:
 		return FIELD_TYPE_STRING;
-	case TK_CASE:
-		assert(pExpr->x.pList->nExpr >= 2);
+	case TK_CASE: {
+		struct ExprList *cs = pExpr->x.pList;
+		assert(cs->nExpr >= 2);
 		/*
 		 * CASE expression comes at least with one
 		 * WHEN and one THEN clauses. So, first
 		 * expression always represents WHEN
 		 * argument, and the second one - THEN.
-		 *
-		 * TODO: We must ensure that all THEN clauses
-		 * have arguments of the same type.
+		 * In case at least one type of THEN argument
+		 * is different from others then we can't
+		 * determine type of returning value at compiling
+		 * stage and set SCALAR (i.e. most general) type.
 		 */
-		return sql_expr_type(pExpr->x.pList->a[1].pExpr);
+		enum field_type ref_type = sql_expr_type(cs->a[1].pExpr);
+		for (int i = 1; i <= cs->nExpr / 2; i = i * 2) {
+			if (ref_type != sql_expr_type(cs->a[i + 1].pExpr))
+				return FIELD_TYPE_SCALAR;
+		}
+		/*
+		 * ELSE clause is optional but we should check
+		 * its type as well.
+		 */
+		if (cs->nExpr % 2 == 1 &&
+		    ref_type != sql_expr_type(cs->a[cs->nExpr - 1].pExpr))
+			return FIELD_TYPE_SCALAR;
+		return ref_type;
+	}
 	case TK_LT:
 	case TK_GT:
 	case TK_EQ:
diff --git a/test/sql/types.result b/test/sql/types.result
index cdfb1e783..1db4b980d 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -966,3 +966,38 @@ box.execute('SELECT ?', {true})
   rows:
   - [true]
 ...
+-- Make sure that CASE-THEN statement return type is SCALAR in
+-- case two THEN clauses feature different types.
+--
+box.execute("SELECT CASE 1 WHEN 1 THEN x'0000000000' WHEN 2 THEN 'str' END")
+---
+- metadata:
+  - name: CASE 1 WHEN 1 THEN x'0000000000' WHEN 2 THEN 'str' END
+    type: scalar
+  rows:
+  - ["\0\0\0\0\0"]
+...
+box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 END")
+---
+- metadata:
+  - name: CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 END
+    type: integer
+  rows:
+  - [666]
+...
+box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 321 END")
+---
+- metadata:
+  - name: CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 321 END
+    type: integer
+  rows:
+  - [666]
+...
+box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' END")
+---
+- metadata:
+  - name: CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' END
+    type: scalar
+  rows:
+  - [666]
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index ae1a0ab72..b66a3e068 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -234,3 +234,11 @@ box.execute('SELECT \'9223372036854\' + 1;')
 
 -- Fix BOOLEAN bindings.
 box.execute('SELECT ?', {true})
+
+-- Make sure that CASE-THEN statement return type is SCALAR in
+-- case two THEN clauses feature different types.
+--
+box.execute("SELECT CASE 1 WHEN 1 THEN x'0000000000' WHEN 2 THEN 'str' END")
+box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 END")
+box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 321 END")
+box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' END")
-- 
2.15.1

[tarantool-patches] [PATCH 3/5] sql: use 'varbinary' as a name of type instead of 'blob'

[Nikita Pettik]

We are going to introduce new column type 'VARBINARY', which allows to
store values with MP_BIN msgpack format. On the other hand, now it is also
possible to meet this type: all literals in form of x'...' are
supposed to be inserted into SCALAR column type exactly with MP_BIN
encoding. Prior to this moment type of such values (encoded as MP_BIN)
was called 'blob'. Thus, let's fix all visible to user messages using
'varbinary' name of type instead of 'blob'.
---
 src/box/lua/lua_sql.c          |  2 +-
 src/box/sql/func.c             |  4 ++--
 src/box/sql/vdbe.c             |  4 ++--
 src/box/sql/vdbeapi.c          |  4 ++--
 test/sql-tap/cast.test.lua     |  4 ++--
 test/sql-tap/func.test.lua     |  2 +-
 test/sql-tap/lua_sql.test.lua  |  4 ++--
 test/sql-tap/position.test.lua | 16 ++++++++--------
 test/sql/types.result          | 24 ++++++++++++------------
 9 files changed, 32 insertions(+), 32 deletions(-)

diff --git a/src/box/lua/lua_sql.c b/src/box/lua/lua_sql.c
index 4d90a53de..c3bddf735 100644
--- a/src/box/lua/lua_sql.c
+++ b/src/box/lua/lua_sql.c
@@ -165,7 +165,7 @@ lbox_sql_create_function(struct lua_State *L)
 		type = FIELD_TYPE_NUMBER;
 	else if (strcmp(type_arg, "NUM") == 0)
 		type = FIELD_TYPE_NUMBER;
-	else if (strcmp(type_arg, "BLOB") == 0)
+	else if (strcmp(type_arg, "VARBINARY") == 0)
 		type = FIELD_TYPE_SCALAR;
 	else if (strcmp(type_arg, "BOOL") == 0 ||
 		 strcmp(type_arg, "BOOLEAN") == 0)
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 761a3abae..4ec591eee 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -120,7 +120,7 @@ typeofFunc(sql_context * context, int NotUsed, sql_value ** argv)
 		z = "number";
 		break;
 	case MP_BIN:
-		z = "scalar";
+		z = "varbinary";
 		break;
 	case MP_BOOL:
 		z = "boolean";
@@ -249,7 +249,7 @@ position_func(struct sql_context *context, int argc, struct Mem **argv)
 	if (haystack_type != MP_STR && haystack_type != MP_BIN)
 		inconsistent_type_arg = haystack;
 	if (inconsistent_type_arg != NULL) {
-		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or BLOB",
+		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or VARBINARY",
 			 mem_type_to_str(inconsistent_type_arg));
 		context->is_aborted = true;
 		return;
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 9f3879910..826232f99 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -589,7 +589,7 @@ mem_type_to_str(const struct Mem *p)
 	case MEM_Real:
 		return "REAL";
 	case MEM_Blob:
-		return "BLOB";
+		return "VARBINARY";
 	case MEM_Bool:
 		return "BOOLEAN";
 	default:
@@ -1485,7 +1485,7 @@ case OP_Concat: {           /* same as TK_CONCAT, in1, in2, out3 */
 		char *inconsistent_type = str_type_p1 == 0 ?
 					  mem_type_to_str(pIn1) :
 					  mem_type_to_str(pIn2);
-		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or BLOB",
+		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT or VARBINARY",
 			 inconsistent_type);
 		goto abort_due_to_error;
 	}
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index f470ac6b1..3bda7d145 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -942,7 +942,7 @@ sql_bind_blob(sql_stmt * pStmt,
 	struct Mem *var = &p->aVar[i - 1];
 	if (sqlVdbeMemSetStr(var, zData, nData, 0, xDel) != 0)
 		return -1;
-	return sql_bind_type(p, i, "BLOB");
+	return sql_bind_type(p, i, "VARBINARY");
 }
 
 int
@@ -1014,7 +1014,7 @@ sql_bind_ptr(struct sql_stmt *stmt, int i, void *ptr)
 	struct Vdbe *p = (struct Vdbe *) stmt;
 	int rc = vdbeUnbind(p, i);
 	if (rc == 0) {
-		rc = sql_bind_type(p, i, "BLOB");
+		rc = sql_bind_type(p, i, "VARBINARY");
 		mem_set_ptr(&p->aVar[i - 1], ptr);
 	}
 	return rc;
diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
index 531c310d2..47784d273 100755
--- a/test/sql-tap/cast.test.lua
+++ b/test/sql-tap/cast.test.lua
@@ -40,7 +40,7 @@ test:do_execsql_test(
         SELECT typeof(x'616263')
     ]], {
         -- <cast-1.2>
-        "scalar"
+        "varbinary"
         -- </cast-1.2>
     })
 
@@ -90,7 +90,7 @@ test:do_execsql_test(
         SELECT typeof(CAST(x'616263' AS SCALAR))
     ]], {
         -- <cast-1.8>
-        "scalar"
+        "varbinary"
         -- </cast-1.8>
     })
 
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index f9044ad01..50a40b777 100755
--- a/test/sql-tap/func.test.lua
+++ b/test/sql-tap/func.test.lua
@@ -978,7 +978,7 @@ test:do_execsql_test(
         SELECT typeof(randomblob(32));
     ]], {
         -- <func-9.4>
-        "scalar"
+        "varbinary"
         -- </func-9.4>
     })
 
diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua
index b0913e7f4..676b105ae 100755
--- a/test/sql-tap/lua_sql.test.lua
+++ b/test/sql-tap/lua_sql.test.lua
@@ -108,7 +108,7 @@ local from_lua_to_sql = {
 local function check_from_lua_to_sql(i)
     return from_lua_to_sql[i][2]
 end
-box.internal.sql_create_function("check_from_lua_to_sql", "BLOB", check_from_lua_to_sql)
+box.internal.sql_create_function("check_from_lua_to_sql", "VARBINARY", check_from_lua_to_sql)
 
 -- check for different types
 for i = 1, #from_lua_to_sql, 1 do
@@ -125,7 +125,7 @@ local from_lua_to_sql_bad = {
 local function check_from_lua_to_sql_bad(i)
     return from_lua_to_sql_bad[i]
 end
-box.internal.sql_create_function("check_from_lua_to_sql_bad", "BLOB", check_from_lua_to_sql_bad)
+box.internal.sql_create_function("check_from_lua_to_sql_bad", "VARBINARY", check_from_lua_to_sql_bad)
 
 for i = 1, #from_lua_to_sql_bad, 1 do
     test:do_catchsql_test(
diff --git a/test/sql-tap/position.test.lua b/test/sql-tap/position.test.lua
index 8c46d7b9e..8ffe3ae2f 100755
--- a/test/sql-tap/position.test.lua
+++ b/test/sql-tap/position.test.lua
@@ -228,7 +228,7 @@ test:do_test(
         return test:catchsql "SELECT position(34, 12345);"
     end, {
         -- <position-1.23>
-        1, "Inconsistent types: expected TEXT or BLOB got INTEGER"
+        1, "Inconsistent types: expected TEXT or VARBINARY got INTEGER"
         -- </position-1.23>
     })
 
@@ -238,7 +238,7 @@ test:do_test(
         return test:catchsql "SELECT position(34, 123456.78);"
     end, {
         -- <position-1.24>
-        1, "Inconsistent types: expected TEXT or BLOB got REAL"
+        1, "Inconsistent types: expected TEXT or VARBINARY got REAL"
         -- </position-1.24>
     })
 
@@ -248,7 +248,7 @@ test:do_test(
         return test:catchsql "SELECT position(x'3334', 123456.78);"
     end, {
         -- <position-1.25>
-        1, "Inconsistent types: expected TEXT or BLOB got REAL"
+        1, "Inconsistent types: expected TEXT or VARBINARY got REAL"
         -- </position-1.25>
     })
 
@@ -554,7 +554,7 @@ test:do_test(
         return test:catchsql("SELECT position('x', x'78c3a4e282ac79');")
     end, {
         -- <position-1.54>
-        1, "Inconsistent types: expected TEXT got BLOB"
+        1, "Inconsistent types: expected TEXT got VARBINARY"
         -- </position-1.54>
     })
 
@@ -564,7 +564,7 @@ test:do_test(
         return test:catchsql "SELECT position('y', x'78c3a4e282ac79');"
     end, {
         -- <position-1.55>
-        1, "Inconsistent types: expected TEXT got BLOB"
+        1, "Inconsistent types: expected TEXT got VARBINARY"
         -- </position-1.55>
     })
 
@@ -614,7 +614,7 @@ test:do_test(
         return test:catchsql "SELECT position(x'79', 'xä€y');"
     end, {
         -- <position-1.57.1>
-        1, "Inconsistent types: expected BLOB got TEXT"
+        1, "Inconsistent types: expected VARBINARY got TEXT"
         -- </position-1.57.1>
     })
 
@@ -624,7 +624,7 @@ test:do_test(
         return test:catchsql "SELECT position(x'a4', 'xä€y');"
     end, {
         -- <position-1.57.2>
-        1, "Inconsistent types: expected BLOB got TEXT"
+        1, "Inconsistent types: expected VARBINARY got TEXT"
         -- </position-1.57.2>
     })
 
@@ -634,7 +634,7 @@ test:do_test(
         return test:catchsql "SELECT position('y', x'78c3a4e282ac79');"
     end, {
         -- <position-1.57.3>
-        1, "Inconsistent types: expected TEXT got BLOB"
+        1, "Inconsistent types: expected TEXT got VARBINARY"
         -- </position-1.57.3>
     })
 
diff --git a/test/sql/types.result b/test/sql/types.result
index 1db4b980d..332ebd43b 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -152,33 +152,33 @@ sp:drop()
 --
 box.execute("SELECT 'abc' || 1;")
 ---
-- error: 'Inconsistent types: expected TEXT or BLOB got INTEGER'
+- error: 'Inconsistent types: expected TEXT or VARBINARY got INTEGER'
 ...
 box.execute("SELECT 'abc' || 1.123;")
 ---
-- error: 'Inconsistent types: expected TEXT or BLOB got REAL'
+- error: 'Inconsistent types: expected TEXT or VARBINARY got REAL'
 ...
 box.execute("SELECT 1 || 'abc';")
 ---
-- error: 'Inconsistent types: expected TEXT or BLOB got INTEGER'
+- error: 'Inconsistent types: expected TEXT or VARBINARY got INTEGER'
 ...
 box.execute("SELECT 1.123 || 'abc';")
 ---
-- error: 'Inconsistent types: expected TEXT or BLOB got REAL'
+- error: 'Inconsistent types: expected TEXT or VARBINARY got REAL'
 ...
 box.execute("SELECt 'a' || 'b' || 1;")
 ---
-- error: 'Inconsistent types: expected TEXT or BLOB got INTEGER'
+- error: 'Inconsistent types: expected TEXT or VARBINARY got INTEGER'
 ...
 -- What is more, they must be of the same type.
 --
 box.execute("SELECT 'abc' || randomblob(5);")
 ---
-- error: 'Inconsistent types: expected TEXT got BLOB'
+- error: 'Inconsistent types: expected TEXT got VARBINARY'
 ...
 box.execute("SELECT randomblob(5) || 'x';")
 ---
-- error: 'Inconsistent types: expected BLOB got TEXT'
+- error: 'Inconsistent types: expected VARBINARY got TEXT'
 ...
 -- Result of BLOBs concatenation must be BLOB.
 --
@@ -188,7 +188,7 @@ box.execute("VALUES (TYPEOF(randomblob(5) || zeroblob(5)));")
   - name: column1
     type: string
   rows:
-  - ['scalar']
+  - ['varbinary']
 ...
 -- gh-3954: LIKE accepts only arguments of type TEXT and NULLs.
 --
@@ -202,15 +202,15 @@ box.execute("INSERT INTO t1 VALUES (randomblob(5));")
 ...
 box.execute("SELECT * FROM t1 WHERE s LIKE 'blob';")
 ---
-- error: 'Inconsistent types: expected TEXT got BLOB'
+- error: 'Inconsistent types: expected TEXT got VARBINARY'
 ...
 box.execute("SELECT * FROM t1 WHERE 'blob' LIKE s;")
 ---
-- error: 'Inconsistent types: expected TEXT got BLOB'
+- error: 'Inconsistent types: expected TEXT got VARBINARY'
 ...
 box.execute("SELECT * FROM t1 WHERE 'blob' LIKE x'0000';")
 ---
-- error: 'Inconsistent types: expected TEXT got BLOB'
+- error: 'Inconsistent types: expected TEXT got VARBINARY'
 ...
 box.execute("SELECT s LIKE NULL FROM t1;")
 ---
@@ -393,7 +393,7 @@ box.execute("SELECT 1 LIMIT 1 OFFSET true;")
 ...
 box.execute("SELECT 'abc' || true;")
 ---
-- error: 'Inconsistent types: expected TEXT or BLOB got BOOLEAN'
+- error: 'Inconsistent types: expected TEXT or VARBINARY got BOOLEAN'
 ...
 -- Boolean can take part in arithmetic operations.
 --
-- 
2.15.1

[tarantool-patches] [PATCH 4/5] sql: make built-ins raise errors for varbin args

[Nikita Pettik]

Since values of type 'varbinary' can't be cast to any other type, let's
patch built-in functions which are not assumed to accept arguments of
this type to raise an error in case argument turn out to be of type
varbinary.

Part of #4206
---
 src/box/sql/func.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 4ec591eee..5fd1496fd 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -196,9 +196,10 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
 			sql_result_null(context);
 			break;
 		}
-	case MP_BOOL: {
+	case MP_BOOL:
+	case MP_BIN: {
 		diag_set(ClientError, ER_INCONSISTENT_TYPES, "number",
-			 "boolean");
+			 mem_type_to_str(argv[0]));
 		context->is_aborted = true;
 		return;
 	}
@@ -506,6 +507,12 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
 	}
 	if (sql_value_is_null(argv[0]))
 		return;
+	if (sql_value_type(argv[0]) == MP_BIN) {
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 sql_value_text(argv[0]), "numeric");
+		context->is_aborted = true;
+		return;
+	}
 	r = sql_value_double(argv[0]);
 	/* If Y==0 and X will fit in a 64-bit int,
 	 * handle the rounding directly,
@@ -560,6 +567,13 @@ case_type##ICUFunc(sql_context *context, int argc, sql_value **argv)   \
 	const char *z2;                                                        \
 	int n;                                                                 \
 	UNUSED_PARAMETER(argc);                                                \
+	int arg_type = sql_value_type(argv[0]);                                \
+	if (arg_type == MP_BIN) {                                              \
+		diag_set(ClientError, ER_INCONSISTENT_TYPES, "TEXT",           \
+			 "VARBINARY");                                         \
+		context->is_aborted = true;                                    \
+		return;                                                        \
+	}                                                                      \
 	z2 = (char *)sql_value_text(argv[0]);                              \
 	n = sql_value_bytes(argv[0]);                                      \
 	/*                                                                     \
@@ -646,6 +660,12 @@ randomBlob(sql_context * context, int argc, sql_value ** argv)
 	unsigned char *p;
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
+	if (sql_value_type(argv[0]) == MP_BIN) {
+		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
+			 sql_value_text(argv[0]), "numeric");
+		context->is_aborted = true;
+		return;
+	}
 	n = sql_value_int(argv[0]);
 	if (n < 1)
 		return;
-- 
2.15.1

[tarantool-patches] [PATCH 5/5] sql: introduce VARBINARY column type

[Nikita Pettik]

Current patch introduces new type available in SQL:
 - VARBINARY now is reserved keyword;
 - Allow to specify VARBINARY column and CAST type;
 - All literals which start from 'x' are assumed to be of this type;
 - There's no available implicit or explicit conversions between
   VARBINARY and other types;
 - Under the hood all values of VARBINARY type are stored as MP_BIN
   msgpack format type.

Closes #4206
---
 extra/mkkeywordhash.c                      |   1 +
 src/box/sql/expr.c                         |   2 +-
 src/box/sql/func.c                         |   4 +-
 src/box/sql/parse.y                        |   3 +-
 src/box/sql/vdbe.c                         |  26 ++-
 src/box/sql/vdbemem.c                      |   5 +
 test/sql-tap/keyword1.test.lua             |   3 +-
 test/sql/gh-3888-values-blob-assert.result |   4 +-
 test/sql/iproto.result                     |   4 +-
 test/sql/misc.result                       |   2 +-
 test/sql/types.result                      | 256 ++++++++++++++++++++++++++++-
 test/sql/types.test.lua                    |  60 +++++++
 12 files changed, 345 insertions(+), 25 deletions(-)

diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c
index b83294cb3..49c9565c7 100644
--- a/extra/mkkeywordhash.c
+++ b/extra/mkkeywordhash.c
@@ -188,6 +188,7 @@ static Keyword aKeywordTable[] = {
   { "UPDATE",                 "TK_UPDATE",      ALWAYS,           true  },
   { "USING",                  "TK_USING",       ALWAYS,           true  },
   { "VALUES",                 "TK_VALUES",      ALWAYS,           true  },
+  { "VARBINARY",              "TK_VARBINARY",   ALWAYS,           true  },
   { "VIEW",                   "TK_VIEW",        VIEW,             true  },
   { "WITH",                   "TK_WITH",        CTE,              true  },
   { "WHEN",                   "TK_WHEN",        ALWAYS,           true  },
diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 97f5bd180..4448f01cb 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -2235,7 +2235,7 @@ sql_expr_needs_no_type_change(const struct Expr *p, enum field_type type)
 	case TK_STRING:
 		return type == FIELD_TYPE_STRING;
 	case TK_BLOB:
-		return true;
+		return type == FIELD_TYPE_VARBINARY;
 	case TK_COLUMN:
 		/* p cannot be part of a CHECK constraint. */
 		assert(p->iTable >= 0);
diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 5fd1496fd..34e80cd32 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1926,13 +1926,13 @@ sqlRegisterBuiltinFunctions(void)
 		FUNCTION2(ifnull, 2, 0, 0, noopFunc, SQL_FUNC_COALESCE,
 			  FIELD_TYPE_INTEGER),
 		VFUNCTION(random, 0, 0, 0, randomFunc, FIELD_TYPE_INTEGER),
-		VFUNCTION(randomblob, 1, 0, 0, randomBlob, FIELD_TYPE_SCALAR),
+		VFUNCTION(randomblob, 1, 0, 0, randomBlob, FIELD_TYPE_VARBINARY),
 		FUNCTION(nullif, 2, 0, 1, nullifFunc, FIELD_TYPE_SCALAR),
 		FUNCTION(version, 0, 0, 0, sql_func_version, FIELD_TYPE_STRING),
 		FUNCTION(quote, 1, 0, 0, quoteFunc, FIELD_TYPE_STRING),
 		VFUNCTION(row_count, 0, 0, 0, sql_row_count, FIELD_TYPE_INTEGER),
 		FUNCTION_COLL(replace, 3, 0, 0, replaceFunc),
-		FUNCTION(zeroblob, 1, 0, 0, zeroblobFunc, FIELD_TYPE_SCALAR),
+		FUNCTION(zeroblob, 1, 0, 0, zeroblobFunc, FIELD_TYPE_VARBINARY),
 		FUNCTION_COLL(substr, 2, 0, 0, substrFunc),
 		FUNCTION_COLL(substr, 3, 0, 0, substrFunc),
 		AGGREGATE(sum, 1, 0, 0, sum_step, sumFinalize,
diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y
index 010feffd4..1512b37d7 100644
--- a/src/box/sql/parse.y
+++ b/src/box/sql/parse.y
@@ -897,7 +897,7 @@ idlist(A) ::= nm(Y). {
         p->type = FIELD_TYPE_STRING;
         break;
       case TK_BLOB:
-        p->type = FIELD_TYPE_SCALAR;
+        p->type = FIELD_TYPE_VARBINARY;
         break;
       case TK_INTEGER:
         p->type = FIELD_TYPE_INTEGER;
@@ -1724,6 +1724,7 @@ typedef(A) ::= SCALAR . { A.type = FIELD_TYPE_SCALAR; }
 /** BOOL | BOOLEAN is not used due to possible bug in Lemon. */
 typedef(A) ::= BOOL . { A.type = FIELD_TYPE_BOOLEAN; }
 typedef(A) ::= BOOLEAN . { A.type = FIELD_TYPE_BOOLEAN; }
+typedef(A) ::= VARBINARY . { A.type = FIELD_TYPE_VARBINARY; }
 
 /**
  * Time-like types are temporary disabled, until they are
diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index 826232f99..d0f0cb4f5 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -337,6 +337,10 @@ mem_apply_type(struct Mem *record, enum field_type type)
 		}
 		record->flags &= ~(MEM_Real | MEM_Int);
 		return 0;
+	case FIELD_TYPE_VARBINARY:
+		if ((record->flags & MEM_Blob) == 0)
+			return -1;
+		return 0;
 	case FIELD_TYPE_SCALAR:
 		return 0;
 	default:
@@ -2094,20 +2098,24 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			}
 			break;
 		}
-	} else if ((flags1 | flags3) & MEM_Bool) {
+	} else if (((flags1 | flags3) & MEM_Bool) != 0 ||
+		   ((flags1 | flags3) & MEM_Blob) != 0) {
 		/*
-		 * If one of values is of type BOOLEAN, then the
-		 * second one must be BOOLEAN as well. Otherwise
-		 * an error is raised.
+		 * If one of values is of type BOOLEAN or VARBINARY,
+		 * then the second one must be of the same type as
+		 * well. Otherwise an error is raised.
 		 */
-		bool is_bool_type_arg1 = flags1 & MEM_Bool;
-		bool is_bool_type_arg3 = flags3 & MEM_Bool;
-		if (! is_bool_type_arg1 || ! is_bool_type_arg3) {
-			char *inconsistent_type = ! is_bool_type_arg1 ?
+		int type_arg1 = flags1 & (MEM_Bool | MEM_Blob);
+		int type_arg3 = flags3 & (MEM_Bool | MEM_Blob);
+		if (type_arg1 != type_arg3) {
+			char *inconsistent_type = type_arg1 != 0 ?
+						  mem_type_to_str(pIn3) :
+						  mem_type_to_str(pIn1);
+			char *expected_type     = type_arg1 != 0 ?
 						  mem_type_to_str(pIn1) :
 						  mem_type_to_str(pIn3);
 			diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
-				 inconsistent_type, "boolean");
+				 inconsistent_type, expected_type);
 			goto abort_due_to_error;
 		}
 		res = sqlMemCompare(pIn3, pIn1, NULL);
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index d85148bc3..3aaf3e04f 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -691,6 +691,11 @@ sqlVdbeMemCast(Mem * pMem, enum field_type type)
 		return sqlVdbeMemIntegerify(pMem, true);
 	case FIELD_TYPE_NUMBER:
 		return sqlVdbeMemRealify(pMem);
+	case FIELD_TYPE_VARBINARY:
+		/* VARIBNARY can't be converted to any other type. */
+		if ((pMem->flags & MEM_Blob) != 0)
+			return 0;
+		return -1;
 	default:
 		assert(type == FIELD_TYPE_STRING);
 		assert(MEM_Str == (MEM_Blob >> 3));
diff --git a/test/sql-tap/keyword1.test.lua b/test/sql-tap/keyword1.test.lua
index 9c524d607..4266f93d7 100755
--- a/test/sql-tap/keyword1.test.lua
+++ b/test/sql-tap/keyword1.test.lua
@@ -1,6 +1,6 @@
 #!/usr/bin/env tarantool
 test = require("sqltester")
-test:plan(176)
+test:plan(177)
 
 --!./tcltestrunner.lua
 -- 2009 January 29
@@ -186,6 +186,7 @@ local bannedkws = {
 	"sql",
 	"user",
 	"varchar",
+	"varbinary",
 	"whenever",
 	"while"
 }
diff --git a/test/sql/gh-3888-values-blob-assert.result b/test/sql/gh-3888-values-blob-assert.result
index 81b0f52fd..bb396a0df 100644
--- a/test/sql/gh-3888-values-blob-assert.result
+++ b/test/sql/gh-3888-values-blob-assert.result
@@ -55,7 +55,7 @@ box.execute('SELECT X\'507265766564\'')
 ---
 - metadata:
   - name: X'507265766564'
-    type: scalar
+    type: varbinary
   rows:
   - ['Preved']
 ...
@@ -72,7 +72,7 @@ box.execute('SELECT X\'4D6564766564\'')
 ---
 - metadata:
   - name: X'4D6564766564'
-    type: scalar
+    type: varbinary
   rows:
   - ['Medved']
 ...
diff --git a/test/sql/iproto.result b/test/sql/iproto.result
index 9639ba7a6..7efc31355 100644
--- a/test/sql/iproto.result
+++ b/test/sql/iproto.result
@@ -771,7 +771,7 @@ cn:execute("SELECT zeroblob(1);")
 ---
 - metadata:
   - name: zeroblob(1)
-    type: scalar
+    type: varbinary
   rows:
   - ["\0"]
 ...
@@ -784,7 +784,7 @@ res = cn:execute("SELECT randomblob(1);")
 res.metadata
 ---
 - - name: randomblob(1)
-    type: scalar
+    type: varbinary
 ...
 -- Type set during compilation stage, and since min/max are accept
 -- arguments of all scalar type, we can't say nothing more than
diff --git a/test/sql/misc.result b/test/sql/misc.result
index bc8b10e87..96cfb3e84 100644
--- a/test/sql/misc.result
+++ b/test/sql/misc.result
@@ -102,7 +102,7 @@ box.execute('SELECT X\'4D6564766564\'')
 ---
 - metadata:
   - name: X'4D6564766564'
-    type: scalar
+    type: varbinary
   rows:
   - ['Medved']
 ...
diff --git a/test/sql/types.result b/test/sql/types.result
index 332ebd43b..5a7d42e5a 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -355,15 +355,15 @@ box.execute("SELECT unknown = true;")
 ...
 box.execute("SELECT 1 = true;")
 ---
-- error: 'Type mismatch: can not convert INTEGER to boolean'
+- error: 'Type mismatch: can not convert INTEGER to BOOLEAN'
 ...
 box.execute("SELECT 'abc' = true;")
 ---
-- error: 'Type mismatch: can not convert TEXT to boolean'
+- error: 'Type mismatch: can not convert TEXT to BOOLEAN'
 ...
 box.execute("SELECT 1.123 > true;")
 ---
-- error: 'Type mismatch: can not convert REAL to boolean'
+- error: 'Type mismatch: can not convert REAL to BOOLEAN'
 ...
 box.execute("SELECT true IN (1, 'abc', true)")
 ---
@@ -626,7 +626,7 @@ box.execute("SELECT upper(b) FROM t;")
 ...
 box.execute("SELECT abs(b) FROM t;")
 ---
-- error: 'Inconsistent types: expected number got boolean'
+- error: 'Inconsistent types: expected number got BOOLEAN'
 ...
 box.execute("SELECT typeof(b) FROM t;")
 ---
@@ -895,11 +895,11 @@ box.execute("INSERT INTO t1 VALUES (3, 'abc'), (4, 12.5);")
 ...
 box.execute("SELECT s FROM t1 WHERE s = true;")
 ---
-- error: 'Type mismatch: can not convert TEXT to boolean'
+- error: 'Type mismatch: can not convert TEXT to BOOLEAN'
 ...
 box.execute("SELECT s FROM t1 WHERE s < true;")
 ---
-- error: 'Type mismatch: can not convert TEXT to boolean'
+- error: 'Type mismatch: can not convert TEXT to BOOLEAN'
 ...
 box.execute("SELECT s FROM t1 WHERE s IN (true, 1, 'abcd')")
 ---
@@ -1001,3 +1001,247 @@ box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' END")
   rows:
   - [666]
 ...
+-- Test basic capabilities of VARBINARY type.
+--
+box.execute("CREATE TABLE t (id INT PRIMARY KEY, v VARBINARY);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t VALUES(1, 1);")
+---
+- error: 'Type mismatch: can not convert 1 to varbinary'
+...
+box.execute("INSERT INTO t VALUES(1, 1.123);")
+---
+- error: 'Type mismatch: can not convert 1.123 to varbinary'
+...
+box.execute("INSERT INTO t VALUES(1, true);")
+---
+- error: 'Type mismatch: can not convert true to varbinary'
+...
+box.execute("INSERT INTO t VALUES(1, 'asd');")
+---
+- error: 'Type mismatch: can not convert asd to varbinary'
+...
+box.execute("INSERT INTO t VALUES(1, x'616263');")
+---
+- row_count: 1
+...
+box.execute("SELECT * FROM t WHERE v = 1")
+---
+- error: 'Type mismatch: can not convert INTEGER to VARBINARY'
+...
+box.execute("SELECT * FROM t WHERE v = 1.123")
+---
+- error: 'Type mismatch: can not convert REAL to VARBINARY'
+...
+box.execute("SELECT * FROM t WHERE v = 'str'")
+---
+- error: 'Type mismatch: can not convert TEXT to VARBINARY'
+...
+box.execute("SELECT * FROM t WHERE v = x'616263'")
+---
+- metadata:
+  - name: ID
+    type: integer
+  - name: V
+    type: varbinary
+  rows:
+  - [1, 'abc']
+...
+box.execute("SELECT sum(v) FROM t;")
+---
+- error: 'Type mismatch: can not convert abc to number'
+...
+box.execute("SELECT avg(v) FROM t;")
+---
+- error: 'Type mismatch: can not convert abc to number'
+...
+box.execute("SELECT total(v) FROM t;")
+---
+- error: 'Type mismatch: can not convert abc to number'
+...
+box.execute("SELECT min(v) FROM t;")
+---
+- metadata:
+  - name: min(v)
+    type: scalar
+  rows:
+  - ['abc']
+...
+box.execute("SELECT max(v) FROM t;")
+---
+- metadata:
+  - name: max(v)
+    type: scalar
+  rows:
+  - ['abc']
+...
+box.execute("SELECT count(v) FROM t;")
+---
+- metadata:
+  - name: count(v)
+    type: integer
+  rows:
+  - [1]
+...
+box.execute("SELECT group_concat(v) FROM t;")
+---
+- metadata:
+  - name: group_concat(v)
+    type: string
+  rows:
+  - ['abc']
+...
+box.execute("SELECT lower(v) FROM t;")
+---
+- error: 'Inconsistent types: expected TEXT got VARBINARY'
+...
+box.execute("SELECT upper(v) FROM t;")
+---
+- error: 'Inconsistent types: expected TEXT got VARBINARY'
+...
+box.execute("SELECT abs(v) FROM t;")
+---
+- error: 'Inconsistent types: expected number got VARBINARY'
+...
+box.execute("SELECT typeof(v) FROM t;")
+---
+- metadata:
+  - name: typeof(v)
+    type: string
+  rows:
+  - ['varbinary']
+...
+box.execute("SELECT quote(v) FROM t;")
+---
+- metadata:
+  - name: quote(v)
+    type: string
+  rows:
+  - ['X''616263''']
+...
+box.execute("SELECT min(v, x'') FROM t;")
+---
+- metadata:
+  - name: min(v, x'')
+    type: scalar
+  rows:
+  - ['']
+...
+box.execute("CREATE INDEX iv ON t(v);")
+---
+- row_count: 1
+...
+box.execute("SELECT v FROM t WHERE v = x'616263';")
+---
+- metadata:
+  - name: V
+    type: varbinary
+  rows:
+  - ['abc']
+...
+box.execute("SELECT v FROM t ORDER BY v;")
+---
+- metadata:
+  - name: V
+    type: varbinary
+  rows:
+  - ['abc']
+...
+box.execute("UPDATE t SET v = x'636261' WHERE v = x'616263';")
+---
+- row_count: 1
+...
+box.execute("SELECT v FROM t;")
+---
+- metadata:
+  - name: V
+    type: varbinary
+  rows:
+  - ['cba']
+...
+box.execute("CREATE TABLE parent (id INT PRIMARY KEY, a VARBINARY UNIQUE);")
+---
+- row_count: 1
+...
+box.space.T:truncate()
+---
+...
+box.execute("ALTER TABLE t ADD CONSTRAINT fk1 FOREIGN KEY (v) REFERENCES parent (a);")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t VALUES (1, x'616263');")
+---
+- error: 'Failed to execute SQL statement: FOREIGN KEY constraint failed'
+...
+box.execute("INSERT INTO parent VALUES (1, x'616263');")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t VALUES (1, x'616263');")
+---
+- row_count: 1
+...
+box.execute("ALTER TABLE t DROP CONSTRAINT fk1;")
+---
+- row_count: 1
+...
+box.space.PARENT:drop()
+---
+...
+box.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a VARBINARY CHECK (a = x'616263'));")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t1 VALUES (1, x'006162');")
+---
+- error: 'Check constraint failed ''CK_CONSTRAINT_1_T1'': a = x''616263'''
+...
+box.execute("INSERT INTO t1 VALUES (1, x'616263');")
+---
+- row_count: 1
+...
+box.space.T1:drop()
+---
+...
+box.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a VARBINARY DEFAULT x'616263');")
+---
+- row_count: 1
+...
+box.execute("INSERT INTO t1 (id) VALUES (1);")
+---
+- row_count: 1
+...
+box.space.T1:select()
+---
+- - [1, 'abc']
+...
+box.space.T1:drop()
+---
+...
+box.execute("SELECT CAST(1 AS VARBINARY);")
+---
+- error: 'Type mismatch: can not convert 1 to varbinary'
+...
+box.execute("SELECT CAST(1.123 AS VARBINARY);")
+---
+- error: 'Type mismatch: can not convert 1.123 to varbinary'
+...
+box.execute("SELECT CAST(true AS VARBINARY);")
+---
+- error: 'Type mismatch: can not convert true to varbinary'
+...
+box.execute("SELECT CAST('asd' AS VARBINARY);")
+---
+- error: 'Type mismatch: can not convert asd to varbinary'
+...
+box.execute("SELECT CAST(x'' AS VARBINARY);")
+---
+- metadata:
+  - name: CAST(x'' AS VARBINARY)
+    type: varbinary
+  rows:
+  - ['']
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index b66a3e068..fb2e824d4 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -242,3 +242,63 @@ box.execute("SELECT CASE 1 WHEN 1 THEN x'0000000000' WHEN 2 THEN 'str' END")
 box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 END")
 box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 321 END")
 box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' END")
+
+-- Test basic capabilities of VARBINARY type.
+--
+box.execute("CREATE TABLE t (id INT PRIMARY KEY, v VARBINARY);")
+box.execute("INSERT INTO t VALUES(1, 1);")
+box.execute("INSERT INTO t VALUES(1, 1.123);")
+box.execute("INSERT INTO t VALUES(1, true);")
+box.execute("INSERT INTO t VALUES(1, 'asd');")
+box.execute("INSERT INTO t VALUES(1, x'616263');")
+box.execute("SELECT * FROM t WHERE v = 1")
+box.execute("SELECT * FROM t WHERE v = 1.123")
+box.execute("SELECT * FROM t WHERE v = 'str'")
+box.execute("SELECT * FROM t WHERE v = x'616263'")
+
+box.execute("SELECT sum(v) FROM t;")
+box.execute("SELECT avg(v) FROM t;")
+box.execute("SELECT total(v) FROM t;")
+box.execute("SELECT min(v) FROM t;")
+box.execute("SELECT max(v) FROM t;")
+box.execute("SELECT count(v) FROM t;")
+box.execute("SELECT group_concat(v) FROM t;")
+
+box.execute("SELECT lower(v) FROM t;")
+box.execute("SELECT upper(v) FROM t;")
+box.execute("SELECT abs(v) FROM t;")
+box.execute("SELECT typeof(v) FROM t;")
+box.execute("SELECT quote(v) FROM t;")
+box.execute("SELECT min(v, x'') FROM t;")
+
+box.execute("CREATE INDEX iv ON t(v);")
+box.execute("SELECT v FROM t WHERE v = x'616263';")
+box.execute("SELECT v FROM t ORDER BY v;")
+
+box.execute("UPDATE t SET v = x'636261' WHERE v = x'616263';")
+box.execute("SELECT v FROM t;")
+
+box.execute("CREATE TABLE parent (id INT PRIMARY KEY, a VARBINARY UNIQUE);")
+box.space.T:truncate()
+box.execute("ALTER TABLE t ADD CONSTRAINT fk1 FOREIGN KEY (v) REFERENCES parent (a);")
+box.execute("INSERT INTO t VALUES (1, x'616263');")
+box.execute("INSERT INTO parent VALUES (1, x'616263');")
+box.execute("INSERT INTO t VALUES (1, x'616263');")
+box.execute("ALTER TABLE t DROP CONSTRAINT fk1;")
+box.space.PARENT:drop()
+
+box.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a VARBINARY CHECK (a = x'616263'));")
+box.execute("INSERT INTO t1 VALUES (1, x'006162');")
+box.execute("INSERT INTO t1 VALUES (1, x'616263');")
+box.space.T1:drop()
+
+box.execute("CREATE TABLE t1 (id INT PRIMARY KEY, a VARBINARY DEFAULT x'616263');")
+box.execute("INSERT INTO t1 (id) VALUES (1);")
+box.space.T1:select()
+box.space.T1:drop()
+
+box.execute("SELECT CAST(1 AS VARBINARY);")
+box.execute("SELECT CAST(1.123 AS VARBINARY);")
+box.execute("SELECT CAST(true AS VARBINARY);")
+box.execute("SELECT CAST('asd' AS VARBINARY);")
+box.execute("SELECT CAST(x'' AS VARBINARY);")
-- 
2.15.1

[tarantool-patches] Re: [PATCH 3/5] sql: use 'varbinary' as a name of type instead of 'blob'

[Vladislav Shpilevoy]

On 24/07/2019 13:42, Nikita Pettik wrote:
> We are going to introduce new column type 'VARBINARY', which allows to
> store values with MP_BIN msgpack format. On the other hand, now it is also
> possible to meet this type: all literals in form of x'...' are
> supposed to be inserted into SCALAR column type exactly with MP_BIN
> encoding. Prior to this moment type of such values (encoded as MP_BIN)
> was called 'blob'. Thus, let's fix all visible to user messages using
> 'varbinary' name of type instead of 'blob'.
> ---
>  src/box/lua/lua_sql.c          |  2 +-
>  src/box/sql/func.c             |  4 ++--
>  src/box/sql/vdbe.c             |  4 ++--
>  src/box/sql/vdbeapi.c          |  4 ++--
>  test/sql-tap/cast.test.lua     |  4 ++--
>  test/sql-tap/func.test.lua     |  2 +-
>  test/sql-tap/lua_sql.test.lua  |  4 ++--
>  test/sql-tap/position.test.lua | 16 ++++++++--------
>  test/sql/types.result          | 24 ++++++++++++------------
>  9 files changed, 32 insertions(+), 32 deletions(-)

More mentionings of blob in messages visible to user.

func.c   :540, :1185, :1254, :1753
vdbeapi.c:283, :382,  :1061
vdbemem.c:996, :1022
vdbeaux.c:1192

[tarantool-patches] Re: [PATCH 4/5] sql: make built-ins raise errors for varbin args

[Vladislav Shpilevoy]

On 24/07/2019 13:42, Nikita Pettik wrote:
> Since values of type 'varbinary' can't be cast to any other type, let's
> patch built-in functions which are not assumed to accept arguments of
> this type to raise an error in case argument turn out to be of type
> varbinary.
>
> Part of #4206
> ---
>  src/box/sql/func.c | 24 ++++++++++++++++++++++--
>  1 file changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index 4ec591eee..5fd1496fd 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -506,6 +507,12 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
>  	}
>  	if (sql_value_is_null(argv[0]))
>  		return;
> +	if (sql_value_type(argv[0]) == MP_BIN) {
> +		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,

The patch is ok, but now I see, that we allow 'string' values to functions,
taking numbers, such as round(). Is it ok?

[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type

[Vladislav Shpilevoy]

Thanks for the patch! See 4 comments below.

On 24/07/2019 13:42, Nikita Pettik wrote:
> Current patch introduces new type available in SQL:
>  - VARBINARY now is reserved keyword;
>  - Allow to specify VARBINARY column and CAST type;
>  - All literals which start from 'x' are assumed to be of this type;
>  - There's no available implicit or explicit conversions between
>    VARBINARY and other types;
>  - Under the hood all values of VARBINARY type are stored as MP_BIN
>    msgpack format type.
>
> Closes #4206
> ---
>  extra/mkkeywordhash.c                      |   1 +
>  src/box/sql/expr.c                         |   2 +-
>  src/box/sql/func.c                         |   4 +-
>  src/box/sql/parse.y                        |   3 +-
>  src/box/sql/vdbe.c                         |  26 ++-
>  src/box/sql/vdbemem.c                      |   5 +
>  test/sql-tap/keyword1.test.lua             |   3 +-
>  test/sql/gh-3888-values-blob-assert.result |   4 +-
>  test/sql/iproto.result                     |   4 +-
>  test/sql/misc.result                       |   2 +-
>  test/sql/types.result                      | 256 ++++++++++++++++++++++++++++-
>  test/sql/types.test.lua                    |  60 +++++++
>  12 files changed, 345 insertions(+), 25 deletions(-)
>
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index 826232f99..d0f0cb4f5 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2094,20 +2098,24 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
>  			}
>  			break;
>  		}
> -	} else if ((flags1 | flags3) & MEM_Bool) {
> +	} else if (((flags1 | flags3) & MEM_Bool) != 0 ||
> +		   ((flags1 | flags3) & MEM_Blob) != 0) {

1. Nit:

diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
index d0f0cb4f5..ea7c9f25f 100644
--- a/src/box/sql/vdbe.c
+++ b/src/box/sql/vdbe.c
@@ -2098,8 +2098,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
 			}
 			break;
 		}
-	} else if (((flags1 | flags3) & MEM_Bool) != 0 ||
-		   ((flags1 | flags3) & MEM_Blob) != 0) {
+	} else if (((flags1 | flags3) & (MEM_Bool | MEM_Blob)) != 0) {
 		/*
 		 * If one of values is of type BOOLEAN or VARBINARY,
 		 * then the second one must be of the same type as



2. Please, add tests on varbinary values binding.

3. BLOB keyword is reserved, but also it is used in parse.y:980.
Should not it be deleted from all the rules, and be just
reserved?

4. Additionally (I bet, you knew I would ask), what are we going
to do with all mentionings of blob in the code? We have word 'blob'
mentioned 368 times in all the SQL sources, including comments,
parts of names.

In wherecode.c:644 we have a CREATE TABLE example, using BLOB -
this one definitely should be fixed. What about other places?
We could replace 'blob' -> 'bin','binary', 'vbin', ... . It
should not take much time.

[tarantool-patches] Re: [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt

[Vladislav Shpilevoy]

Hi! Thanks for the patch!

On 24/07/2019 13:42, Nikita Pettik wrote:
> Before this patch, resulting type for CASE-WHEN statement was assumed to
> be the same as type of argument of first THEN clause. Obviously, it is
> wrong and could lead to sad consequence (e.g. creating ephemeral table
> with inconsistent format). To deal with this, we check all THEN
> arguments: if all of them have the same type, then such type will be
> resulting of the whole statement; if at least two types are different,
> we can't determine actual resulting type during compilation stage and
> assign SCALAR as a most general type in SQL now.
>
> Need for #4206
> ---
>  src/box/sql/expr.c      | 27 +++++++++++++++++++++------
>  test/sql/types.result   | 35 +++++++++++++++++++++++++++++++++++
>  test/sql/types.test.lua |  8 ++++++++
>  3 files changed, 64 insertions(+), 6 deletions(-)
>
> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
> index d7104d8a0..97f5bd180 100644
> --- a/src/box/sql/expr.c
> +++ b/src/box/sql/expr.c
> @@ -85,18 +85,33 @@ sql_expr_type(struct Expr *pExpr)
>  		return sql_type_result(rhs_type, lhs_type);
>  	case TK_CONCAT:
>  		return FIELD_TYPE_STRING;
> -	case TK_CASE:
> -		assert(pExpr->x.pList->nExpr >= 2);
> +	case TK_CASE: {
> +		struct ExprList *cs = pExpr->x.pList;
> +		assert(cs->nExpr >= 2);
>  		/*
>  		 * CASE expression comes at least with one
>  		 * WHEN and one THEN clauses. So, first
>  		 * expression always represents WHEN
>  		 * argument, and the second one - THEN.
> -		 *
> -		 * TODO: We must ensure that all THEN clauses
> -		 * have arguments of the same type.
> +		 * In case at least one type of THEN argument
> +		 * is different from others then we can't
> +		 * determine type of returning value at compiling
> +		 * stage and set SCALAR (i.e. most general) type.
>  		 */
> -		return sql_expr_type(pExpr->x.pList->a[1].pExpr);
> +		enum field_type ref_type = sql_expr_type(cs->a[1].pExpr);
> +		for (int i = 1; i <= cs->nExpr / 2; i = i * 2) {
> +			if (ref_type != sql_expr_type(cs->a[i + 1].pExpr))
> +				return FIELD_TYPE_SCALAR;
> +		}

Something is wrong with that cycle.

box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 END")
---
- metadata:
  - name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4
      WHEN 'e' THEN 5 WHEN 'f' THEN 6 END
    type: scalar
  rows:
  - [1]
...

But it is not scalar, it is integer - all 'then's
here return an integer. Perhaps this is because in
the cycle you do i*=2 instead of i+=2, so you compare
not 1 vs 3, 1 vs 5, 1 vs 7, etc, (all 'then' are by
odd indexes), but 1 vs 2, 1 vs 3, 1 vs 5, 1 vs 9, etc.

[tarantool-patches] Re: [PATCH 2/5] sql: fix resulting type calculation for CASE-WHEN stmt

[n.pettik]

> On 26 Jul 2019, at 01:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Hi! Thanks for the patch!
> 
> On 24/07/2019 13:42, Nikita Pettik wrote:
>> Before this patch, resulting type for CASE-WHEN statement was assumed to
>> be the same as type of argument of first THEN clause. Obviously, it is
>> wrong and could lead to sad consequence (e.g. creating ephemeral table
>> with inconsistent format). To deal with this, we check all THEN
>> arguments: if all of them have the same type, then such type will be
>> resulting of the whole statement; if at least two types are different,
>> we can't determine actual resulting type during compilation stage and
>> assign SCALAR as a most general type in SQL now.
>> 
>> Need for #4206
>> ---
>> src/box/sql/expr.c      | 27 +++++++++++++++++++++------
>> test/sql/types.result   | 35 +++++++++++++++++++++++++++++++++++
>> test/sql/types.test.lua |  8 ++++++++
>> 3 files changed, 64 insertions(+), 6 deletions(-)
>> 
>> diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
>> index d7104d8a0..97f5bd180 100644
>> --- a/src/box/sql/expr.c
>> +++ b/src/box/sql/expr.c
>> @@ -85,18 +85,33 @@ sql_expr_type(struct Expr *pExpr)
>> 		return sql_type_result(rhs_type, lhs_type);
>> 	case TK_CONCAT:
>> 		return FIELD_TYPE_STRING;
>> -	case TK_CASE:
>> -		assert(pExpr->x.pList->nExpr >= 2);
>> +	case TK_CASE: {
>> +		struct ExprList *cs = pExpr->x.pList;
>> +		assert(cs->nExpr >= 2);
>> 		/*
>> 		 * CASE expression comes at least with one
>> 		 * WHEN and one THEN clauses. So, first
>> 		 * expression always represents WHEN
>> 		 * argument, and the second one - THEN.
>> -		 *
>> -		 * TODO: We must ensure that all THEN clauses
>> -		 * have arguments of the same type.
>> +		 * In case at least one type of THEN argument
>> +		 * is different from others then we can't
>> +		 * determine type of returning value at compiling
>> +		 * stage and set SCALAR (i.e. most general) type.
>> 		 */
>> -		return sql_expr_type(pExpr->x.pList->a[1].pExpr);
>> +		enum field_type ref_type = sql_expr_type(cs->a[1].pExpr);
>> +		for (int i = 1; i <= cs->nExpr / 2; i = i * 2) {
>> +			if (ref_type != sql_expr_type(cs->a[i + 1].pExpr))
>> +				return FIELD_TYPE_SCALAR;
>> +		}
> 
> Something is wrong with that cycle.
> 
> box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 END")
> ---
> - metadata:
>  - name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4
>      WHEN 'e' THEN 5 WHEN 'f' THEN 6 END
>    type: scalar
>  rows:
>  - [1]
> ...
> 
> But it is not scalar, it is integer - all 'then's
> here return an integer. Perhaps this is because in
> the cycle you do i*=2 instead of i+=2, so you compare
> not 1 vs 3, 1 vs 5, 1 vs 7, etc, (all 'then' are by
> odd indexes), but 1 vs 2, 1 vs 3, 1 vs 5, 1 vs 9, etc.

Yep, this is obvious bug. Fixed:

diff --git a/src/box/sql/expr.c b/src/box/sql/expr.c
index 97f5bd180..7b3a41c46 100644
--- a/src/box/sql/expr.c
+++ b/src/box/sql/expr.c
@@ -99,8 +99,8 @@ sql_expr_type(struct Expr *pExpr)
                 * stage and set SCALAR (i.e. most general) type.
                 */
                enum field_type ref_type = sql_expr_type(cs->a[1].pExpr);
-               for (int i = 1; i <= cs->nExpr / 2; i = i * 2) {
-                       if (ref_type != sql_expr_type(cs->a[i + 1].pExpr))
+               for (int i = 3; i < cs->nExpr; i += 2) {
+                       if (ref_type != sql_expr_type(cs->a[i].pExpr))
                                return FIELD_TYPE_SCALAR;
                }
                /*
diff --git a/test/sql/types.result b/test/sql/types.result
index 1db4b980d..d05fa9dfa 100644
--- a/test/sql/types.result
+++ b/test/sql/types.result
@@ -1001,3 +1001,39 @@ box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' END")
   rows:
   - [666]
 ...
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 END;")
+---
+- metadata:
+  - name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4
+      WHEN 'e' THEN 5 WHEN 'f' THEN 6 END
+    type: integer
+  rows:
+  - [1]
+...
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 'asd' END;")
+---
+- metadata:
+  - name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4
+      WHEN 'e' THEN 5 WHEN 'f' THEN 'asd' END
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 ELSE 'asd' END;")
+---
+- metadata:
+  - name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4
+      WHEN 'e' THEN 5 WHEN 'f' THEN 6 ELSE 'asd' END
+    type: scalar
+  rows:
+  - [1]
+...
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 ELSE 7 END;")
+---
+- metadata:
+  - name: CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4
+      WHEN 'e' THEN 5 WHEN 'f' THEN 6 ELSE 7 END
+    type: integer
+  rows:
+  - [1]
+...
diff --git a/test/sql/types.test.lua b/test/sql/types.test.lua
index b66a3e068..8fd20f892 100644
--- a/test/sql/types.test.lua
+++ b/test/sql/types.test.lua
@@ -242,3 +242,7 @@ box.execute("SELECT CASE 1 WHEN 1 THEN x'0000000000' WHEN 2 THEN 'str' END")
 box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 END")
 box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 321 END")
 box.execute("SELECT CASE 1 WHEN 1 THEN 666 WHEN 2 THEN 123 ELSE 'asd' END")
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 END;")
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 'asd' END;")
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 ELSE 'asd' END;")
+box.execute("SELECT CASE 'a' WHEN 'a' THEN 1 WHEN 'b' THEN 2 WHEN 'c' THEN 3 WHEN 'd' THEN 4 WHEN 'e' THEN 5 WHEN 'f' THEN 6 ELSE 7 END;")

[tarantool-patches] Re: [PATCH 3/5] sql: use 'varbinary' as a name of type instead of 'blob'

[n.pettik]

> On 26 Jul 2019, at 01:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> On 24/07/2019 13:42, Nikita Pettik wrote:
>> We are going to introduce new column type 'VARBINARY', which allows to
>> store values with MP_BIN msgpack format. On the other hand, now it is also
>> possible to meet this type: all literals in form of x'...' are
>> supposed to be inserted into SCALAR column type exactly with MP_BIN
>> encoding. Prior to this moment type of such values (encoded as MP_BIN)
>> was called 'blob'. Thus, let's fix all visible to user messages using
>> 'varbinary' name of type instead of 'blob'.
>> ---
>> src/box/lua/lua_sql.c          |  2 +-
>> src/box/sql/func.c             |  4 ++--
>> src/box/sql/vdbe.c             |  4 ++--
>> src/box/sql/vdbeapi.c          |  4 ++--
>> test/sql-tap/cast.test.lua     |  4 ++--
>> test/sql-tap/func.test.lua     |  2 +-
>> test/sql-tap/lua_sql.test.lua  |  4 ++--
>> test/sql-tap/position.test.lua | 16 ++++++++--------
>> test/sql/types.result          | 24 ++++++++++++------------
>> 9 files changed, 32 insertions(+), 32 deletions(-)
> 
> More mentionings of blob in messages visible to user.
> 
> func.c   :540, :1185, :1254, :1753
> vdbeapi.c:283, :382,  :1061
> vdbemem.c:996, :1022
> vdbeaux.c:1192

All these usages are in form of:
"string or blob is too big”

I think that blob is a good name for entity of varbinary
type. Another possible names are “binary sequence”,
“binary data” or “binary string”. The latter variant is used
 in the ANSI specification. Now I’ve changed to “binary string”,
but if you can suggest better name, let’s discuss it.

Diff:

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 4ec591eee..1a5671187 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1182,7 +1182,8 @@ zeroblobFunc(sql_context * context, int argc, sql_value ** argv)
        if (n < 0)
                n = 0;
        if (sql_result_zeroblob64(context, n) != 0) {
-               diag_set(ClientError, ER_SQL_EXECUTE, "string or blob too big");
+               diag_set(ClientError, ER_SQL_EXECUTE, "string or binary string"\
+                        "is too big");
                context->is_aborted = true;
        }
 }
@@ -1251,7 +1252,7 @@ replaceFunc(sql_context * context, int argc, sql_value ** argv)
                        testcase(nOut - 2 == db->aLimit[SQL_LIMIT_LENGTH]);
                        if (nOut - 1 > db->aLimit[SQL_LIMIT_LENGTH]) {
                                diag_set(ClientError, ER_SQL_EXECUTE, "string "\
-                                        "or blob too big");
+                                        "or binary string is too big");
                                context->is_aborted = true;
                                sql_free(zOut);
                                return;
@@ -1750,8 +1751,8 @@ groupConcatFinalize(sql_context * context)
        pAccum = sql_aggregate_context(context, 0);
        if (pAccum) {
                if (pAccum->accError == STRACCUM_TOOBIG) {
-                       diag_set(ClientError, ER_SQL_EXECUTE, "string or blob "\
-                                "too big");
+                       diag_set(ClientError, ER_SQL_EXECUTE, "string or binary"\
+                                "string is too big");
                        context->is_aborted = true;
                } else if (pAccum->accError == STRACCUM_NOMEM) {
                        context->is_aborted = true;
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 3bda7d145..217a9f22d 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -280,8 +280,8 @@ invokeValueDestructor(const void *p,        /* Value to destroy */
                xDel((void *)p);
        }
        if (pCtx) {
-               diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
-                        "big");
+               diag_set(ClientError, ER_SQL_EXECUTE, "string or binary string"\
+                        "is too big");
                pCtx->is_aborted = true;
        }
        return -1;
@@ -379,8 +379,8 @@ sql_result_zeroblob64(sql_context * pCtx, u64 n)
 {
        Mem *pOut = pCtx->pOut;
        if (n > (u64) pOut->db->aLimit[SQL_LIMIT_LENGTH]) {
-               diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
-                        "big");
+               diag_set(ClientError, ER_SQL_EXECUTE, "string or binary string"\
+                        "is too big");
                return -1;
        }
        sqlVdbeMemSetZeroBlob(pCtx->pOut, (int)n);
@@ -1058,8 +1058,8 @@ sql_bind_zeroblob64(sql_stmt * pStmt, int i, sql_uint64 n)
 {
        Vdbe *p = (Vdbe *) pStmt;
        if (n > (u64) p->db->aLimit[SQL_LIMIT_LENGTH]) {
-               diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
-                        "big");
+               diag_set(ClientError, ER_SQL_EXECUTE, "string or binary string"\
+                        "is too big");
                return -1;
        }
        assert((n & 0x7FFFFFFF) == n);
diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c
index baeeb4624..de50860bb 100644
--- a/src/box/sql/vdbeaux.c
+++ b/src/box/sql/vdbeaux.c
@@ -1189,7 +1189,7 @@ displayP4(Op * pOp, char *zTemp, int nTemp)
                                zP4 = "NULL";
                        } else {
                                assert(pMem->flags & MEM_Blob);
-                               zP4 = "(blob)";
+                               zP4 = "(binary string)";
                        }
                        break;
                }
diff --git a/src/box/sql/vdbemem.c b/src/box/sql/vdbemem.c
index d85148bc3..274f80a18 100644
--- a/src/box/sql/vdbemem.c
+++ b/src/box/sql/vdbemem.c
@@ -993,8 +993,8 @@ sqlVdbeMemSetStr(Mem * pMem,        /* Memory cell to set to string value */
                        nAlloc += 1; //SQL_UTF8
                }
                if (nByte > iLimit) {
-                       diag_set(ClientError, ER_SQL_EXECUTE, "string or blob "\
-                                "is too big");
+                       diag_set(ClientError, ER_SQL_EXECUTE, "string or binary"\
+                                "string is too big");
                        return -1;
                }
                testcase(nAlloc == 0);
@@ -1019,8 +1019,8 @@ sqlVdbeMemSetStr(Mem * pMem,      /* Memory cell to set to string value */
        pMem->flags = flags;
 
        if (nByte > iLimit) {
-               diag_set(ClientError, ER_SQL_EXECUTE, "string or blob is too "\
-                        "big");
+               diag_set(ClientError, ER_SQL_EXECUTE, "string or binary string"\
+                        "is too big");
                return -1;
        }

[tarantool-patches] Re: [PATCH 4/5] sql: make built-ins raise errors for varbin args

[n.pettik]

> On 26 Jul 2019, at 01:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> On 24/07/2019 13:42, Nikita Pettik wrote:
>> Since values of type 'varbinary' can't be cast to any other type, let's
>> patch built-in functions which are not assumed to accept arguments of
>> this type to raise an error in case argument turn out to be of type
>> varbinary.
>> 
>> Part of #4206
>> ---
>> src/box/sql/func.c | 24 ++++++++++++++++++++++--
>> 1 file changed, 22 insertions(+), 2 deletions(-)
>> 
>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index 4ec591eee..5fd1496fd 100644
>> --- a/src/box/sql/func.c
>> +++ b/src/box/sql/func.c
>> @@ -506,6 +507,12 @@ roundFunc(sql_context * context, int argc, sql_value ** argv)
>> 	}
>> 	if (sql_value_is_null(argv[0]))
>> 		return;
>> +	if (sql_value_type(argv[0]) == MP_BIN) {
>> +		diag_set(ClientError, ER_SQL_TYPE_MISMATCH,
> 
> The patch is ok, but now I see, that we allow 'string' values to functions,
> taking numbers, such as round(). Is it ok?

Yep, there was a discussion (not sure whether it was private or in dev
mailing list) and it was decided that it is ok. Justification is following:
we allow implicit cast between strings and numeric types. In turn,
before passing arguments to functions they should be implicitly casted
if it is required. We don’t add explicit OP_ApplyType opcode, but handle
that conversion inside  each function separately (due to the sqlite legacy).
I guess it would be better to emit extra OP_ApplyType opcode before
each function call, but remove type dispatching from func implementations.

[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type

[n.pettik]

> 
> 1. Nit:
> 
> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
> index d0f0cb4f5..ea7c9f25f 100644
> --- a/src/box/sql/vdbe.c
> +++ b/src/box/sql/vdbe.c
> @@ -2098,8 +2098,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
> 			}
> 			break;
> 		}
> -	} else if (((flags1 | flags3) & MEM_Bool) != 0 ||
> -		   ((flags1 | flags3) & MEM_Blob) != 0) {
> +	} else if (((flags1 | flags3) & (MEM_Bool | MEM_Blob)) != 0) {
> 		/*
> 		 * If one of values is of type BOOLEAN or VARBINARY,
> 		 * then the second one must be of the same type as

Ok, applied.

> 2. Please, add tests on varbinary values binding.

Could you suggest the way to force MP_BIN format from Lua?
I see the only test on VARBINARY type (test/box/varbinary_type.test.lua),
but this approach is unlikely to be applicable for bindings.

> 3. BLOB keyword is reserved, but also it is used in parse.y:980.
> Should not it be deleted from all the rules, and be just
> reserved?

It’s rule for declaring blob (aka binary string) literals.
I can rename it, but TBO it looks OK to me.

> 4. Additionally (I bet, you knew I would ask), what are we going
> to do with all mentionings of blob in the code? We have word 'blob'
> mentioned 368 times in all the SQL sources, including comments,
> parts of names.

I’m OK with using BLOB as a synonym to entity of type varbinary.
Can manually clean-out all these places, but a bit later (after 2.2 release).

> In wherecode.c:644 we have a CREATE TABLE example, using BLOB -
> this one definitely should be fixed. What about other places?
> We could replace 'blob' -> 'bin','binary', 'vbin', ... . It
> should not take much time.

This path has been removed on master branch. So I simply
rebased current branch and it disappeared.

I’ve also extended patch-set setting correct default trim
octet for binary string arguments:

    sql: fix default trim octet for binary strings
    
    According to ANSI specification, if TRIM function accepts binary string
    and trim octet is not specified, then it is implicitly set to X'00'.
    Before this patch trim octet was set to ' ' both for string and binary
    string arguments. In turn, ' ' is equal to X'20' in hex representation.
    Hence, TRIM function cut wrong characters:
    
    TRIM(X'004420') -> X‘0044'
    
    This patch sets default trim octet to X'00' for binary string arguments.
    
    Part of #4206

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index e799f380f..9bea3eda0 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -1397,15 +1397,20 @@ trim_func_one_arg(struct sql_context *context, int argc, sql_value **argv)
 {
        assert(argc == 1);
        (void) argc;
-
-       const unsigned char *input_str;
-       if ((input_str = sql_value_text(argv[0])) == NULL)
+       /* In case of VARBINARY type default trim octet is X'00'. */
+       const unsigned char *default_trim;
+       enum mp_type val_type = sql_value_type(argv[0]);
+       if (val_type == MP_NIL)
                return;
-
+       if (val_type == MP_BIN)
+               default_trim = (const unsigned char *) "\0";
+       else
+               default_trim = (const unsigned char *) " ";
        int input_str_sz = sql_value_bytes(argv[0]);
-       uint8_t len_one = 1;
-       trim_procedure(context, TRIM_BOTH, (const unsigned char *) " ",
-                      &len_one, 1, input_str, input_str_sz);
+       const unsigned char *input_str = sql_value_text(argv[0]);
+       uint8_t trim_char_len[1] = { 1 };
+       trim_procedure(context, TRIM_BOTH, default_trim, trim_char_len, 1,
+                      input_str, input_str_sz);
 }
 
 /**
diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
index e073937b8..b81520e33 100755
--- a/test/sql-tap/func.test.lua
+++ b/test/sql-tap/func.test.lua
@@ -2232,6 +2232,24 @@ test:do_catchsql_test(
         -- </func-22.38>
     })
 
+test:do_execsql_test(
+    "func-22.39",
+    [[
+        SELECT HEX(TRIM(X'004420'))
+    ]], { "4420"  })
+
+test:do_execsql_test(
+    "func-22.40",
+    [[
+        SELECT HEX(TRIM(X'00442000'))
+    ]], { "4420"  })
+
+test:do_execsql_test(
+    "func-22.41",
+    [[
+        SELECT HEX(TRIM(X'442000'))
+    ]], { "4420"  })
+
 -- This is to test the deprecated sql_aggregate_count() API.
 --
 --test:do_test(

[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type

[Vladislav Shpilevoy]

Thanks for the fixes!

On 29/07/2019 02:03, n.pettik wrote:
> 
>>
>> 1. Nit:
>>
>> diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c
>> index d0f0cb4f5..ea7c9f25f 100644
>> --- a/src/box/sql/vdbe.c
>> +++ b/src/box/sql/vdbe.c
>> @@ -2098,8 +2098,7 @@ case OP_Ge: {             /* same as TK_GE, jump, in1, in3 */
>> 			}
>> 			break;
>> 		}
>> -	} else if (((flags1 | flags3) & MEM_Bool) != 0 ||
>> -		   ((flags1 | flags3) & MEM_Blob) != 0) {
>> +	} else if (((flags1 | flags3) & (MEM_Bool | MEM_Blob)) != 0) {
>> 		/*
>> 		 * If one of values is of type BOOLEAN or VARBINARY,
>> 		 * then the second one must be of the same type as
> 
> Ok, applied.
> 
>> 2. Please, add tests on varbinary values binding.
> 
> Could you suggest the way to force MP_BIN format from Lua?
> I see the only test on VARBINARY type (test/box/varbinary_type.test.lua),
> but this approach is unlikely to be applicable for bindings.

I can, but you won't like it.

	box.cfg{listen = 3313}
	netbox = require('net.box')
	buffer = require('buffer')
	ffi = require('ffi')

	ffi.cdef[[
	typedef struct box_tuple_format_t box_tuple_format_t;
	typedef struct box_tuple_t box_tuple_t;

	int
	box_insert(uint32_t space_id, const char *tuple, const char *tuple_end,
		   box_tuple_t **result);
	]]

	s = box.schema.create_space('test')
	pk = s:create_index('pk', {parts = {{1, 'scalar'}}})

	box.schema.user.grant('guest','read, write, execute', 'universe')
	box.schema.user.grant('guest', 'create', 'space')
	cn = netbox.connect(box.cfg.listen)

	data = buffer.static_alloc('char', 1 + 6)
	data[0] = 0x91
	data[1] = 0xc4
	data[2] = 3
	data[3] = string.byte('b')
	data[4] = string.byte('i')
	data[5] = string.byte('n')
	ffi.C.box_insert(s.id, data, data + 6, nil)

	cn:execute('SELECT typeof(?);', s:select()[1])

This returns:

	---
	- metadata:
	  - name: typeof(?)
	    type: string
	  rows:
	  - ['varbinary']
	...

I used the fact that netbox encodes 'execute' bind array as a
tuple. So I inserted MP_BIN into a tuple, and used it as a
bind array.

>> 3. BLOB keyword is reserved, but also it is used in parse.y:980.
>> Should not it be deleted from all the rules, and be just
>> reserved?
> 
> It’s rule for declaring blob (aka binary string) literals.
> I can rename it, but TBO it looks OK to me.

Could you please provide an example, how to use BLOB keyword to
declare a literal? I can't find any test, using BLOB in a query
string for anything.

Also, I've found, that 'blob' type is still mentioned in
sql-tap/e_expr.test.lua quite a lot. And in sql-tap/types2.test.lua,
where it is still used as a column type. These tests are disabled.
Why?

[tarantool-patches] Re: [PATCH 3/5] sql: use 'varbinary' as a name of type instead of 'blob'

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

On 29/07/2019 01:56, n.pettik wrote:
> 
> 
>> On 26 Jul 2019, at 01:03, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
>>
>> On 24/07/2019 13:42, Nikita Pettik wrote:
>>> We are going to introduce new column type 'VARBINARY', which allows to
>>> store values with MP_BIN msgpack format. On the other hand, now it is also
>>> possible to meet this type: all literals in form of x'...' are
>>> supposed to be inserted into SCALAR column type exactly with MP_BIN
>>> encoding. Prior to this moment type of such values (encoded as MP_BIN)
>>> was called 'blob'. Thus, let's fix all visible to user messages using
>>> 'varbinary' name of type instead of 'blob'.
>>> ---
>>> src/box/lua/lua_sql.c          |  2 +-
>>> src/box/sql/func.c             |  4 ++--
>>> src/box/sql/vdbe.c             |  4 ++--
>>> src/box/sql/vdbeapi.c          |  4 ++--
>>> test/sql-tap/cast.test.lua     |  4 ++--
>>> test/sql-tap/func.test.lua     |  2 +-
>>> test/sql-tap/lua_sql.test.lua  |  4 ++--
>>> test/sql-tap/position.test.lua | 16 ++++++++--------
>>> test/sql/types.result          | 24 ++++++++++++------------
>>> 9 files changed, 32 insertions(+), 32 deletions(-)
>>
>> More mentionings of blob in messages visible to user.
>>
>> func.c   :540, :1185, :1254, :1753
>> vdbeapi.c:283, :382,  :1061
>> vdbemem.c:996, :1022
>> vdbeaux.c:1192
> 
> All these usages are in form of:
> "string or blob is too big”
> 
> I think that blob is a good name for entity of varbinary
> type. Another possible names are “binary sequence”,
> “binary data” or “binary string”. The latter variant is used
>  in the ANSI specification. Now I’ve changed to “binary string”,
> but if you can suggest better name, let’s discuss it.

No, I don't mind any name. I saw, that in the commit message
you said

    "Thus, let's fix all visible to user messages using 'varbinary'
     name of type instead of 'blob'."

And I noticed, that there are still visible messages mentioning blob.
You can keep blob, or rename, or anything.

[tarantool-patches] Re: [PATCH 3/5] sql: use 'varbinary' as a name of type instead of 'blob'

[n.pettik]

>>> func.c   :540, :1185, :1254, :1753
>>> vdbeapi.c:283, :382,  :1061
>>> vdbemem.c:996, :1022
>>> vdbeaux.c:1192
>> 
>> All these usages are in form of:
>> "string or blob is too big”
>> 
>> I think that blob is a good name for entity of varbinary
>> type. Another possible names are “binary sequence”,
>> “binary data” or “binary string”. The latter variant is used
>> in the ANSI specification. Now I’ve changed to “binary string”,
>> but if you can suggest better name, let’s discuss it.
> 
> No, I don't mind any name. I saw, that in the commit message
> you said
> 
>    "Thus, let's fix all visible to user messages using 'varbinary'
>     name of type instead of 'blob'."
> 
> And I noticed, that there are still visible messages mentioning blob.
> You can keep blob, or rename, or anything.

I’m okay with "binary string” as well, so let’s keep it.

[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type

[n.pettik]

>>> 2. Please, add tests on varbinary values binding.
>> 
>> Could you suggest the way to force MP_BIN format from Lua?
>> I see the only test on VARBINARY type (test/box/varbinary_type.test.lua),
>> but this approach is unlikely to be applicable for bindings.
> 
> I can, but you won't like it.
> 
> 	box.cfg{listen = 3313}
> 	netbox = require('net.box')
> 	buffer = require('buffer')
> 	ffi = require('ffi')
> 
> 	ffi.cdef[[
> 	typedef struct box_tuple_format_t box_tuple_format_t;
> 	typedef struct box_tuple_t box_tuple_t;
> 
> 	int
> 	box_insert(uint32_t space_id, const char *tuple, const char *tuple_end,
> 		   box_tuple_t **result);
> 	]]
> 
> 	s = box.schema.create_space('test')
> 	pk = s:create_index('pk', {parts = {{1, 'scalar'}}})
> 
> 	box.schema.user.grant('guest','read, write, execute', 'universe')
> 	box.schema.user.grant('guest', 'create', 'space')
> 	cn = netbox.connect(box.cfg.listen)
> 
> 	data = buffer.static_alloc('char', 1 + 6)
> 	data[0] = 0x91
> 	data[1] = 0xc4
> 	data[2] = 3
> 	data[3] = string.byte('b')
> 	data[4] = string.byte('i')
> 	data[5] = string.byte('n')
> 	ffi.C.box_insert(s.id, data, data + 6, nil)
> 
> 	cn:execute('SELECT typeof(?);', s:select()[1])
> 
> This returns:
> 
> 	---
> 	- metadata:
> 	  - name: typeof(?)
> 	    type: string
> 	  rows:
> 	  - ['varbinary']
> 	...
> 
> I used the fact that netbox encodes 'execute' bind array as a
> tuple. So I inserted MP_BIN into a tuple, and used it as a
> bind array.

Ok, I can’t come up with better option, so I slightly
simplified your example: there’s no need to use FFI
to insert MP_BIN since we can use SQL facilities to do so.

diff --git a/test/sql/bind.test.lua b/test/sql/bind.test.lua
index 3e0eed29a..51a87b0ee 100644
--- a/test/sql/bind.test.lua
+++ b/test/sql/bind.test.lua
@@ -93,7 +93,20 @@ execute('SELECT :value', parameters)
 --
 execute('SELECT ? ', {18446744073709551615ULL})
 
+-- Make sure that VARBINARY values can be bound. Note that
+-- at the moment there's no direct way to encode value as MP_BIN,
+-- so we have to use workaround only with remote option.
+--
 test_run:cmd("setopt delimiter ';'")
+
+if remote then
+       execute("CREATE TABLE t(a VARBINARY PRIMARY KEY);")
+       execute("INSERT INTO t VALUES (X'00');")
+       res = execute("SELECT typeof(?);", box.space.T:select()[1])
+       assert(res['rows'][1][1] == "varbinary")
+       execute("DROP TABLE t;")
+end;
+
 if remote then
        cn:close()
        box.schema.user.revoke('guest', 'read, write, execute', 'universe')

> 
>>> 3. BLOB keyword is reserved, but also it is used in parse.y:980.
>>> Should not it be deleted from all the rules, and be just
>>> reserved?
>> 
>> It’s rule for declaring blob (aka binary string) literals.
>> I can rename it, but TBO it looks OK to me.
> 
> Could you please provide an example, how to use BLOB keyword to
> declare a literal? I can't find any test, using BLOB in a query
> string for anything.

I meant that it’s not the rule that allows BLOB keyword but
the rule to process X’…’ literals. When string is processed by
lexer (tokenize.c) its token type is assigned to TK_BLOB.
BLOB keyword itself is banned in mkkeywordhash.c since
it has token type TK_STANDARD.
In case you suggest to rename token type from TK_BLOB
to smth else - I see no reason to do so, it’s quite brief and
clear. If some day we decide to add BLOB keyword, its type
will be TK_BLOB_KW to avoid any confusions.

> Also, I've found, that 'blob' type is still mentioned in
> sql-tap/e_expr.test.lua quite a lot. And in sql-tap/types2.test.lua,
> where it is still used as a column type. These tests are disabled.
> Why?

See https://github.com/tarantool/tarantool/issues/3102#issuecomment-437274405

Seems that they contain a lot of type mess, I guess.
Nothing prevents us from resurrecting them, expect for time.

[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

LGTM.

>>>> 3. BLOB keyword is reserved, but also it is used in parse.y:980.
>>>> Should not it be deleted from all the rules, and be just
>>>> reserved?
>>>
>>> It’s rule for declaring blob (aka binary string) literals.
>>> I can rename it, but TBO it looks OK to me.
>>
>> Could you please provide an example, how to use BLOB keyword to
>> declare a literal? I can't find any test, using BLOB in a query
>> string for anything.
> 
> I meant that it’s not the rule that allows BLOB keyword but
> the rule to process X’…’ literals. When string is processed by
> lexer (tokenize.c) its token type is assigned to TK_BLOB.

Ahh, I got it now. BLOB(X) is not a 'BLOB' keyword, it is rather
like a rule 'BLOB := x...'. Then of course it is ok, nothing to
do here.

[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type

[Vladislav Shpilevoy]

Oh, but wait. Travis is fully red:

https://travis-ci.org/tarantool/tarantool/builds/565497371?utm_source=github_status&utm_medium=notification

Looks like you forgot to update plan in sql-tap/func.test.lua.

On 30/07/2019 21:41, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes!
> 
> LGTM.
> 
>>>>> 3. BLOB keyword is reserved, but also it is used in parse.y:980.
>>>>> Should not it be deleted from all the rules, and be just
>>>>> reserved?
>>>>
>>>> It’s rule for declaring blob (aka binary string) literals.
>>>> I can rename it, but TBO it looks OK to me.
>>>
>>> Could you please provide an example, how to use BLOB keyword to
>>> declare a literal? I can't find any test, using BLOB in a query
>>> string for anything.
>>
>> I meant that it’s not the rule that allows BLOB keyword but
>> the rule to process X’…’ literals. When string is processed by
>> lexer (tokenize.c) its token type is assigned to TK_BLOB.
> 
> Ahh, I got it now. BLOB(X) is not a 'BLOB' keyword, it is rather
> like a rule 'BLOB := x...'. Then of course it is ok, nothing to
> do here.
> 

[tarantool-patches] Re: [PATCH 5/5] sql: introduce VARBINARY column type

[n.pettik]

> On 30 Jul 2019, at 22:52, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> wrote:
> 
> Oh, but wait. Travis is fully red:
> 
> https://travis-ci.org/tarantool/tarantool/builds/565497371?utm_source=github_status&utm_medium=notification
> 
> Looks like you forgot to update plan in sql-tap/func.test.lua.

Yep, thanks, fixed. Now gitlab seems to be OK:

https://gitlab.com/tarantool/tarantool/pipelines/73961933

> On 30/07/2019 21:41, Vladislav Shpilevoy wrote:
>> Hi! Thanks for the fixes!
>> 
>> LGTM.
>> 
>>>>>> 3. BLOB keyword is reserved, but also it is used in parse.y:980.
>>>>>> Should not it be deleted from all the rules, and be just
>>>>>> reserved?
>>>>> 
>>>>> It’s rule for declaring blob (aka binary string) literals.
>>>>> I can rename it, but TBO it looks OK to me.
>>>> 
>>>> Could you please provide an example, how to use BLOB keyword to
>>>> declare a literal? I can't find any test, using BLOB in a query
>>>> string for anything.
>>> 
>>> I meant that it’s not the rule that allows BLOB keyword but
>>> the rule to process X’…’ literals. When string is processed by
>>> lexer (tokenize.c) its token type is assigned to TK_BLOB.
>> 
>> Ahh, I got it now. BLOB(X) is not a 'BLOB' keyword, it is rather
>> like a rule 'BLOB := x...'. Then of course it is ok, nothing to
>> do here.
>> 
> 

[tarantool-patches] Re: [PATCH 0/5] Introduce VARBINARY in SQL

[Kirill Yukhin]

Hello,

On 24 Jul 14:42, Nikita Pettik wrote:
> Branch: https://github.com/tarantool/tarantool/tree/np/sql-introduce-blob
> Issue: https://github.com/tarantool/tarantool/issues/4206
> 
> This patch-set introduces new column type available in SQL - VARBINARY.
> All values of VARBINARY type are stored as MP_BIN format type in
> msgpack. Basically, prior to the current patch all literals starting
> with x'...' format were assumed to be encoded with MP_BIN type when
> were inserted into SCALAR field. This rule has been remained.
> According to ANSI, values of VARBINARY type can't be converted to
> any other type.
> 
> Nikita Pettik (5):
>   sql: always erase numeric flag after stringifying
>   sql: fix resulting type calculation for CASE-WHEN stmt
>   sql: use 'varbinary' as a name of type instead of 'blob'
>   sql: make built-ins raise errors for varbin args
>   sql: introduce VARBINARY column type

I've checked the patch set into master.

--
Regards, Kirill Yukhin