[Tarantool-patches] [PATCH v5 50/52] sql: introduce mem_get_bin()

[Mergen Imeev via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thank you for the review! My answer and new patch below.


On 30.03.2021 02:09, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index 78f4ec3b5..199f3abef 100644
>> --- a/src/box/sql/func.c
>> +++ b/src/box/sql/func.c
>> @@ -72,6 +72,19 @@ mem_get_ustr(struct Mem *mem)
>>  	return (const unsigned char *)str;
>>  }
>>  
>> +static const void *
>> +mem_get_blob(struct Mem *mem)
>
> The same comments as in the previous patch with strings. Also you
> use 'binary' term in the other places, not 'blob'. I would propose to
> keep it consistent if we want to switch to 'binary' everywhere.
>
Thanks! Fixed.

>> +{
>> +	const char *s;
>> +	if (!mem_is_varstring(mem) && mem_convert_to_string(mem) != 0)
>> +		return NULL;
>> +	if (ExpandBlob(mem) != 0)
>> +		return NULL;
>> +	if (mem_get_binary(mem, &s) != 0)
>> +		return NULL;
>> +	return (const void *)s;
>> +}


New patch:

commit a3e0cc2e89663b816c476578dce24d05da6fb66b
Author: Mergen Imeev <imeevma@gmail.com>
Date:   Mon Mar 22 11:54:01 2021 +0300

    sql: introduce mem_get_bin()
    
    This patch introduces mem_get_bin(). This function is used to receive
    binary value from MEM. If value of MEM is not binary value, it is
    converted to binary value if possible. MEM is not changed.
    
    Part of #5818

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index c1db6f78a..2896a5c31 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -54,6 +54,17 @@ mem_as_ustr(struct Mem *mem)
 	return (const unsigned char *)mem_as_str0(mem);
 }
 
+static const void *
+mem_as_bin(struct Mem *mem)
+{
+	const char *s;
+	if (!mem_is_bytes(mem) && mem_to_str(mem) != 0)
+		return NULL;
+	if (mem_get_bin(mem, &s) != 0)
+		return NULL;
+	return s;
+}
+
 /*
  * Return the collating function associated with a function.
  */
@@ -306,8 +317,8 @@ position_func(struct sql_context *context, int argc, struct Mem **argv)
 		const unsigned char *haystack_str;
 		const unsigned char *needle_str;
 		if (haystack_type == MP_BIN) {
-			needle_str = sql_value_blob(needle);
-			haystack_str = sql_value_blob(haystack);
+			needle_str = mem_as_bin(needle);
+			haystack_str = mem_as_bin(haystack);
 			assert(needle_str != NULL);
 			assert(haystack_str != NULL || n_haystack_bytes == 0);
 			/*
@@ -447,7 +458,7 @@ substrFunc(sql_context * context, int argc, sql_value ** argv)
 	mem_get_int(argv[1], &p1, &unused);
 	if (p0type == MP_BIN) {
 		len = sql_value_bytes(argv[0]);
-		z = sql_value_blob(argv[0]);
+		z = mem_as_bin(argv[0]);
 		if (z == 0)
 			return;
 		assert(len == sql_value_bytes(argv[0]));
@@ -1082,9 +1093,9 @@ quoteFunc(sql_context * context, int argc, sql_value ** argv)
 	case MP_ARRAY:
 	case MP_MAP: {
 			char *zText = 0;
-			char const *zBlob = sql_value_blob(argv[0]);
+			char const *zBlob = mem_as_bin(argv[0]);
 			int nBlob = sql_value_bytes(argv[0]);
-			assert(zBlob == sql_value_blob(argv[0]));	/* No encoding change */
+			assert(zBlob == mem_as_bin(argv[0]));	/* No encoding change */
 			zText =
 			    (char *)contextMalloc(context,
 						  (2 * (i64) nBlob) + 4);
@@ -1218,9 +1229,9 @@ hexFunc(sql_context * context, int argc, sql_value ** argv)
 	char *zHex, *z;
 	assert(argc == 1);
 	UNUSED_PARAMETER(argc);
-	pBlob = sql_value_blob(argv[0]);
+	pBlob = mem_as_bin(argv[0]);
 	n = sql_value_bytes(argv[0]);
-	assert(pBlob == sql_value_blob(argv[0]));	/* No encoding change */
+	assert(pBlob == mem_as_bin(argv[0]));	/* No encoding change */
 	z = zHex = contextMalloc(context, ((i64) n) * 2 + 1);
 	if (zHex) {
 		for (i = 0; i < n; i++, pBlob++) {
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 8c2d09168..2e69f4a80 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -1332,6 +1332,19 @@ mem_get_str0(const struct Mem *mem, const char **s)
 	return 0;
 }
 
+int
+mem_get_bin(const struct Mem *mem, const char **s)
+{
+	if ((mem->flags & MEM_Str) != 0) {
+		*s = mem->n > 0 ? mem->z : NULL;
+		return 0;
+	}
+	if ((mem->flags & MEM_Blob) == 0 || (mem->flags & MEM_Zero) != 0)
+		return -1;
+	*s = mem->z;
+	return 0;
+}
+
 int
 mem_copy(struct Mem *to, const struct Mem *from)
 {
@@ -2444,28 +2457,6 @@ releaseMemArray(Mem * p, int N)
 	}
 }
 
-/**************************** sql_value_  ******************************
- * The following routines extract information from a Mem or sql_value
- * structure.
- */
-const void *
-sql_value_blob(sql_value * pVal)
-{
-	Mem *p = (Mem *) pVal;
-	if (p->flags & (MEM_Blob | MEM_Str)) {
-		if (ExpandBlob(p) != 0) {
-			assert(p->flags == MEM_Null && p->z == 0);
-			return 0;
-		}
-		p->flags |= MEM_Blob;
-		return p->n ? p->z : 0;
-	} else {
-		if (mem_to_str(pVal) != 0)
-			return NULL;
-		return pVal->z;
-	}
-}
-
 int
 sql_value_bytes(sql_value * pVal)
 {
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 5848ae729..e2d0f343e 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -590,6 +590,14 @@ mem_as_str0(struct Mem *mem)
 	return str;
 }
 
+/**
+ * Return value for MEM of VARBINARY type. For MEM of all other types convert
+ * value of the MEM to VARBINARY if possible and return converted value.
+ * Original MEM is not changed.
+ */
+int
+mem_get_bin(const struct Mem *mem, const char **s);
+
 /**
  * Simple type to str convertor. It is used to simplify
  * error reporting.
@@ -641,9 +649,6 @@ releaseMemArray(Mem * p, int N);
 
 /** Getters. */
 
-const void *
-sql_value_blob(struct Mem *);
-
 int
 sql_value_bytes(struct Mem *);
 
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 51613aea3..8a7a87b49 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -433,9 +433,6 @@ sql_stmt_compile(const char *sql, int bytes_count, struct Vdbe *re_prepared,
 int
 sql_step(sql_stmt *);
 
-const void *
-sql_column_blob(sql_stmt *, int iCol);
-
 int
 sql_column_bytes(sql_stmt *, int iCol);
 
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index c229160b6..19b70a55b 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -470,13 +470,6 @@ columnMem(sql_stmt * pStmt, int i)
  * The following routines are used to access elements of the current row
  * in the result set.
  */
-const void *
-sql_column_blob(sql_stmt * pStmt, int i)
-{
-	const void *val;
-	val = sql_value_blob(columnMem(pStmt, i));
-	return val;
-}
 
 int
 sql_column_bytes(sql_stmt * pStmt, int i)