Re: [Tarantool-patches] [PATCH v2 2/4] sql: properly show values in type mismatch error

[Timur Safin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

LGTM, with one minor note, which may be addressed later.

: From: imeevma@tarantool.org <imeevma@tarantool.org>
: Subject: [PATCH v2 2/4] sql: properly show values in type mismatch error
: 
...
: 
: diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c
: index 05f053c55..3bbff9897 100644
: --- a/src/box/sql/mem.c
: +++ b/src/box/sql/mem.c
: @@ -79,8 +79,8 @@ mem_str(const struct Mem *mem)
:  		return "NULL";
:  	case MEM_TYPE_STR:
:  		if (mem->n > STR_VALUE_MAX_LEN)
: -			return tt_sprintf("%.*s...", STR_VALUE_MAX_LEN, mem->z);
: -		return tt_cstr(mem->z, mem->n);
: +			return tt_sprintf("'%.*s...", STR_VALUE_MAX_LEN, mem->z);
: +		return tt_sprintf("'%.*s'", mem->n, mem->z);
:  	case MEM_TYPE_INT:
:  		return tt_sprintf("%lld", mem->u.i);
:  	case MEM_TYPE_UINT:
: @@ -88,8 +88,18 @@ mem_str(const struct Mem *mem)
:  	case MEM_TYPE_DOUBLE:
:  		sql_snprintf(STR_VALUE_MAX_LEN, buf, "%!.15g", mem->u.r);
:  		return tt_sprintf("%s", buf);
: -	case MEM_TYPE_BIN:
: -		return "varbinary";
: +	case MEM_TYPE_BIN: {
: +		int len = MIN(mem->n, STR_VALUE_MAX_LEN / 2);
: +		for (int i = 0; i < len; ++i) {
: +			int n = (mem->z[i] & 0xF0) >> 4;
: +			buf[2 * i] = n < 10 ? ('0' + n) : ('A' + n - 10);
: +			n = (mem->z[i] & 0x0F);
: +			buf[2 * i + 1] = n < 10 ? ('0' + n) : ('A' + n - 10);
: +		}
: +		if (mem->n > len)
: +			return tt_sprintf("x'%.*s...", len * 2, buf);
: +		return tt_sprintf("x'%.*s'", len * 2, buf);
: +	}

This looks mouthful, and improper abstraction level. I wanted to 
suggest to use cycle with tt_*snprintf for wrapped %0X format calls
but on the second thought this function became too fragile and even
more verbose
(static_reserve(len*2) .. static_alloc(..) series of vsnprintf("%02X",...)
Hell no, these 7 lines are simpler.

:  	case MEM_TYPE_MAP:
:  	case MEM_TYPE_ARRAY: {
:  		const char *str = mp_str(mem->z);
: @@ -99,7 +109,8 @@ mem_str(const struct Mem *mem)
:  		return tt_sprintf("%.*s...", STR_VALUE_MAX_LEN, buf);
:  	}
:  	case MEM_TYPE_UUID:
: -		return tt_uuid_str(&mem->u.uuid);
: +		tt_uuid_to_string(&mem->u.uuid, buf);
: +		return tt_sprintf("'%s'", buf);
:  	case MEM_TYPE_BOOL:
:  		return mem->u.b ? "TRUE" : "FALSE";
:  	default:

Regards,
Timur