[Tarantool-patches] [PATCH v5 46/52] sql: introduce mem_get_uint()

imeevma at tarantool.org imeevma at tarantool.org
Sat Apr 10 00:08:17 MSK 2021


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


On 30.03.2021 02:08, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> On 23.03.2021 10:36, Mergen Imeev via Tarantool-patches wrote:
>> This patch introduces mem_get_unsigned() function which is used to
>> receive unsigned value from MEM.
>>
>> Part of #5818
>> ---
>>  src/box/sql/func.c    | 16 +++++++++++-----
>>  src/box/sql/mem.c     | 37 +++++++++++++++++++++++++++----------
>>  src/box/sql/mem.h     |  6 +++---
>>  src/box/sql/sqlInt.h  |  3 ---
>>  src/box/sql/vdbeapi.c |  6 ------
>>  5 files changed, 41 insertions(+), 27 deletions(-)
>>
>> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
>> index 0fa0f6ac7..a851d98f2 100644
>> --- a/src/box/sql/func.c
>> +++ b/src/box/sql/func.c
>> @@ -118,9 +118,12 @@ port_vdbemem_dump_lua(struct port *base, struct lua_State *L, bool is_flat)
>>  			luaL_pushint64(L, n);
>>  			break;
>>  		}
>> -		case MP_UINT:
>> -			luaL_pushuint64(L, sql_value_uint64(param));
>> +		case MP_UINT: {
>> +			uint64_t u;
>> +			mem_get_unsigned(param, &u);
>> +			luaL_pushuint64(L, u);
> Maybe we could make 2 functions? One to get the value and ignore
> the errors, and the other to get as an out parameter + return an
> error?
>
> For instance, mem_to_uint() - returns uint64_t and internally asserts
> that the value is correct. And mem_get_uint() works like your version.
>
> The same for the other get functions whose result is often ignored.
For some functions I created a "proxy" functions in func.c the way you
described, but not for this function since it is only used in a few places of
sql/func.c. Should I do this for all functions? In func.c I mean. I see this as
temporary measure, since I hope we will rework built-in functions one day.


New patch:

commit bdd3107a42b82a0e1f964819fa0bf4798fd87a4e
Author: Mergen Imeev <imeevma at gmail.com>
Date:   Wed Mar 17 13:55:37 2021 +0300

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

diff --git a/src/box/sql/func.c b/src/box/sql/func.c
index 701e77d49..f2254fb29 100644
--- a/src/box/sql/func.c
+++ b/src/box/sql/func.c
@@ -201,7 +201,9 @@ absFunc(sql_context * context, int argc, sql_value ** argv)
 	UNUSED_PARAMETER(argc);
 	switch (sql_value_type(argv[0])) {
 	case MP_UINT: {
-		sql_result_uint(context, sql_value_uint64(argv[0]));
+		uint64_t u;
+		mem_get_uint(argv[0], &u);
+		sql_result_uint(context, u);
 		break;
 	}
 	case MP_INT: {
@@ -1174,7 +1176,7 @@ charFunc(sql_context * context, int argc, sql_value ** argv)
 		if (sql_value_type(argv[i]) == MP_INT)
 			x = 0xfffd;
 		else
-			x = sql_value_uint64(argv[i]);
+			mem_get_uint(argv[i], &x);
 		if (x > 0x10ffff)
 			x = 0xfffd;
 		c = (unsigned)(x & 0x1fffff);
@@ -1478,7 +1480,8 @@ trim_func_two_args(struct sql_context *context, sql_value *arg1,
 	int input_str_sz = sql_value_bytes(arg2);
 	if (sql_value_type(arg1) == MP_INT || sql_value_type(arg1) == MP_UINT) {
 		uint8_t len_one = 1;
-		uint64_t n = sql_value_uint64(arg1);
+		uint64_t n;
+		mem_get_uint(arg1, &n);
 		trim_procedure(context, n, (const unsigned char *) " ",
 			       &len_one, 1, input_str, input_str_sz);
 	} else if ((trim_set = sql_value_text(arg1)) != NULL) {
@@ -1518,7 +1521,8 @@ trim_func_three_args(struct sql_context *context, sql_value *arg1,
 					     &char_len);
 	if (char_cnt == -1)
 		return;
-	uint64_t n = sql_value_uint64(arg1);
+	uint64_t n;
+	mem_get_uint(arg1, &n);
 	trim_procedure(context, n, trim_set, char_len,
 		       char_cnt, input_str, input_str_sz);
 	sql_free(char_len);
diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
index 259bb7d2c..136da9d5d 100644
--- a/src/box/sql/mem.c
+++ b/src/box/sql/mem.c
@@ -1263,6 +1263,33 @@ mem_get_int(const struct Mem *mem, int64_t *i, bool *is_neg)
 	return -1;
 }
 
+int
+mem_get_uint(const struct Mem *mem, uint64_t *u)
+{
+	if ((mem->flags & MEM_Int) != 0)
+		return -1;
+	if ((mem->flags & MEM_UInt) != 0) {
+		*u = mem->u.u;
+		return 0;
+	}
+	if ((mem->flags & (MEM_Str | MEM_Blob)) != 0) {
+		bool is_neg;
+		if (sql_atoi64(mem->z, (int64_t *)u, &is_neg, mem->n) != 0 ||
+		    is_neg)
+			return -1;
+		return 0;
+	}
+	if ((mem->flags & MEM_Real) != 0) {
+		double d = mem->u.r;
+		if (d >= 0 && d < (double)UINT64_MAX) {
+			*u = (uint64_t)d;
+			return 0;
+		}
+		return -1;
+	}
+	return -1;
+}
+
 int
 mem_copy(struct Mem *to, const struct Mem *from)
 {
@@ -2507,16 +2534,6 @@ sql_value_boolean(sql_value *val)
 	return b;
 }
 
-uint64_t
-sql_value_uint64(sql_value *val)
-{
-	int64_t i = 0;
-	bool is_neg;
-	mem_get_int((struct Mem *) val, &i, &is_neg);
-	assert(!is_neg);
-	return i;
-}
-
 const unsigned char *
 sql_value_text(sql_value * pVal)
 {
diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h
index 2b5385c55..87aee0e9b 100644
--- a/src/box/sql/mem.h
+++ b/src/box/sql/mem.h
@@ -544,6 +544,14 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type type);
 int
 mem_get_int(const struct Mem *mem, int64_t *i, bool *is_neg);
 
+/**
+ * Return value for MEM of UNSIGNED type. For MEM of all other types convert
+ * value of the MEM to UNSIGNED if possible and return converted value. Original
+ * MEM is not changed.
+ */
+int
+mem_get_uint(const struct Mem *mem, uint64_t *u);
+
 /**
  * Simple type to str convertor. It is used to simplify
  * error reporting.
@@ -611,9 +619,6 @@ sql_value_double(struct Mem *);
 bool
 sql_value_boolean(struct Mem *val);
 
-uint64_t
-sql_value_uint64(struct Mem *val);
-
 const unsigned char *
 sql_value_text(struct Mem *);
 
diff --git a/src/box/sql/sqlInt.h b/src/box/sql/sqlInt.h
index 0af247ebf..6ead9b261 100644
--- a/src/box/sql/sqlInt.h
+++ b/src/box/sql/sqlInt.h
@@ -448,9 +448,6 @@ sql_column_double(sql_stmt *, int iCol);
 bool
 sql_column_boolean(struct sql_stmt *stmt, int column);
 
-uint64_t
-sql_column_uint64(struct sql_stmt *stmt, int column);
-
 const unsigned char *
 sql_column_text(sql_stmt *,
 		    int iCol);
diff --git a/src/box/sql/vdbeapi.c b/src/box/sql/vdbeapi.c
index 40404a3b7..85bbbc420 100644
--- a/src/box/sql/vdbeapi.c
+++ b/src/box/sql/vdbeapi.c
@@ -496,12 +496,6 @@ sql_column_boolean(struct sql_stmt *stmt, int i)
 	return sql_value_boolean(columnMem(stmt, i));
 }
 
-uint64_t
-sql_column_uint64(sql_stmt * pStmt, int i)
-{
-	return sql_value_uint64(columnMem(pStmt, i));
-}
-
 const unsigned char *
 sql_column_text(sql_stmt * pStmt, int i)
 {


More information about the Tarantool-patches mailing list