From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 4EAA76EC40; Thu, 3 Jun 2021 12:04:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4EAA76EC40 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1622711098; bh=WqlAt9jxzUETcVIbFkz2J2VdIb9N9ioLROflqaIwlxk=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=BHRtt0Djhi+GXnb0wIGBVcHQgY/GTL+atqxzALFJ9Cpy3O9e5m5kNrpU3wRp78k8x Y2PpfUKM+sHXeUQYI3ZE8jmEczEbge7XIW6MSGYNiI16585tay2nJZRzIxEfuw/TW5 wqyeWBkbgpAvXjCkNpfUfjTV9RR9KvO57Xb/ZWi0= Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 56BA96EC40 for ; Thu, 3 Jun 2021 12:04:57 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 56BA96EC40 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lojHc-0002Ju-AH; Thu, 03 Jun 2021 12:04:56 +0300 Date: Thu, 3 Jun 2021 12:04:55 +0300 To: Timur Safin Cc: tarantool-patches@dev.tarantool.org Message-ID: <20210603090455.GA48615@tarantool.org> References: <04e101d757eb$af9f1b10$0edd5130$@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <04e101d757eb$af9f1b10$0edd5130$@tarantool.org> X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D5B0DA836B685C543EF5F9E25E4001B3518B676B8BE4A4C7182A05F538085040CD290FE715881230B8C3090C29281151AFC7179A8CCF0DE4DC437F5C0942976B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE728F774C865CF4B07EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637993F4D0876F024C68638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D86BC663F18C34C0697E88C5EAE33513ED117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAA867293B0326636D2E47CDBA5A96583BD4B6F7A4D31EC0BC014FD901B82EE079FA2833FD35BB23D27C277FBC8AE2E8BAA867293B0326636D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B62CFFCC7B69C47339089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5BCA8278F6D0883A3BA5969B332FDC942CF13493846AAAF45D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75FBC5FED0552DA851410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34FDC9529E4578B99C0D1F80B5CB868833448A648B0F0042C577F67F896BE2D032459F17470D7D41501D7E09C32AA3244C1947B02AB64C774D0DD76418BC156DCB3A92A9747B6CC886729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojNbQUdF9mq4FpAyzq8IRHeg== X-Mailru-Sender: 689FA8AB762F73936BC43F508A0638220983B34804A77BDCB20ED310585B66AA83D72C36FC87018B9F80AB2734326CD2FB559BB5D741EB96352A0ABBE4FDA4210A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v2 1/2] sql: introduce UUID field type X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Mergen Imeev via Tarantool-patches Reply-To: Mergen Imeev Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thank you for the review! My answers and diff below. On Wed, Jun 02, 2021 at 11:13:00PM +0300, Timur Safin wrote: > There are few minor complains... > : -----Original Message----- > : From: imeevma@tarantool.org > : Sent: Wednesday, June 2, 2021 11:03 AM > : To: tsafin@tarantool.org > : Cc: tarantool-patches@dev.tarantool.org > : Subject: [PATCH v2 1/2] sql: introduce UUID field type > : > : This patch introduces UUID to SQL. UUID is now available as a new field > : type. > : > : Part of #5886 > : > : @TarantoolBot document > : Title: Field type UUID is now available in SQL > : > : The UUID field type is now available in SQL. This means that we can > : create spaces and indexes with UUID, use it in SELECT, UPDATE and > : DELETE. UUID can be accepted and returned by built-in functions and > : user-defined functions. > : > : According to the comparison rules, there will be no implicit casting in > : the comparison. This rule also applies to UUID values: if a value is not > : part of a SCALAR field, it cannot be compared to a value of any other > : type. If the value is in a SCALAR field, it can be compared to any other > : scalar value according to the comparison rules for a SCALAR field. > : > : In case a UUID value is used in an operation that is not a comparison, > : it can be implicitly converted to STRING or VARBINARY. > : > : If a STRING or VARBINARY value is used in an operation that is not a > : comparison, it can be implicitly converted to a UUID. > : > : UUID value can always be explicitly converted to STRING or VARBINARY. > : > : A STRING or VARBINARY value can be explicitly converted to a UUID if it > : conforms to the UUID standard. > : --- > : extra/mkkeywordhash.c | 1 + > : src/box/sql/func.c | 30 +- > : src/box/sql/mem.c | 203 ++- > : src/box/sql/mem.h | 29 +- > : src/box/sql/parse.y | 1 + > : src/box/sql/vdbe.c | 15 +- > : test/sql-tap/CMakeLists.txt | 1 + > : .../gh-5913-segfault-on-select-uuid.test.lua | 42 +- > : .../sql-tap/gh-6024-funcs-return-bin.test.lua | 8 +- > : test/sql-tap/sql_uuid.c | 46 + > : test/sql-tap/uuid.test.lua | 1294 +++++++++++++++++ > : 11 files changed, 1602 insertions(+), 68 deletions(-) > : create mode 100644 test/sql-tap/sql_uuid.c > : create mode 100755 test/sql-tap/uuid.test.lua > : > : diff --git a/extra/mkkeywordhash.c b/extra/mkkeywordhash.c > : index 7480c0211..0d998506c 100644 > : --- a/extra/mkkeywordhash.c > : +++ b/extra/mkkeywordhash.c > : @@ -172,6 +172,7 @@ static Keyword aKeywordTable[] = { > : { "UNSIGNED", "TK_UNSIGNED", true }, > : { "UPDATE", "TK_UPDATE", true }, > : { "USING", "TK_USING", true }, > : + { "UUID" , "TK_UUID" , true }, > : { "VALUES", "TK_VALUES", true }, > : { "VARBINARY", "TK_VARBINARY", true }, > : { "VIEW", "TK_VIEW", true }, > : diff --git a/src/box/sql/func.c b/src/box/sql/func.c > : index 90e8e152f..9c4480a92 100644 > : --- a/src/box/sql/func.c > : +++ b/src/box/sql/func.c > : @@ -58,7 +58,8 @@ static const void * > : mem_as_bin(struct Mem *mem) > : { > : const char *s; > : - if (!mem_is_bytes(mem) && mem_to_str(mem) != 0) > : + if (mem_cast_implicit(mem, FIELD_TYPE_VARBINARY) != 0 && > : + mem_to_str(mem) != 0) > : return NULL; > : if (mem_get_bin(mem, &s) != 0) > : return NULL; > : @@ -142,26 +143,29 @@ typeofFunc(sql_context * context, int NotUsed, > : sql_value ** argv) > : -1, SQL_STATIC); > : return; > : } > : - switch (sql_value_type(argv[0])) { > : - case MP_INT: > : - case MP_UINT: > : + switch (argv[0]->type) { > : + case MEM_TYPE_INT: > : + case MEM_TYPE_UINT: > : z = "integer"; > : break; > : - case MP_STR: > : + case MEM_TYPE_STR: > : z = "string"; > : break; > : - case MP_DOUBLE: > : + case MEM_TYPE_DOUBLE: > : z = "double"; > : break; > : - case MP_BIN: > : - case MP_ARRAY: > : - case MP_MAP: > : + case MEM_TYPE_BIN: > : + case MEM_TYPE_ARRAY: > : + case MEM_TYPE_MAP: > : z = "varbinary"; > : break; > : - case MP_BOOL: > : - case MP_NIL: > : + case MEM_TYPE_BOOL: > : + case MEM_TYPE_NULL: > : z = "boolean"; > : break; > : + case MEM_TYPE_UUID: > : + z = "uuid"; > : + break; > : default: > : unreachable(); > : break; > : @@ -191,6 +195,7 @@ lengthFunc(sql_context * context, int argc, sql_value ** > : argv) > : sql_result_uint(context, mem_len_unsafe(argv[0])); > : break; > : } > : + case MP_EXT: > : case MP_STR:{ > : const unsigned char *z = mem_as_ustr(argv[0]); > : if (z == 0) > : @@ -235,6 +240,7 @@ absFunc(sql_context * context, int argc, sql_value ** > : argv) > : } > : case MP_BOOL: > : case MP_BIN: > : + case MP_EXT: > : case MP_ARRAY: > : case MP_MAP: { > : diag_set(ClientError, ER_INCONSISTENT_TYPES, "number", > : @@ -1461,8 +1467,8 @@ trim_func_one_arg(struct sql_context *context, > : sql_value *arg) > : default_trim = (const unsigned char *) "\0"; > : else > : default_trim = (const unsigned char *) " "; > : - int input_str_sz = mem_len_unsafe(arg); > : const unsigned char *input_str = mem_as_ustr(arg); > : + int input_str_sz = mem_len_unsafe(arg); > : 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/src/box/sql/mem.c b/src/box/sql/mem.c > : index 9894c09af..f0dfce395 100644 > : --- a/src/box/sql/mem.c > : +++ b/src/box/sql/mem.c > : @@ -58,6 +58,16 @@ enum { > : BUF_SIZE = 32, > : }; > : > : +bool > : +mem_is_field_compatible(const struct Mem *mem, enum field_type type) > : +{ > : + if (mem->type == MEM_TYPE_UUID) > : + return (field_ext_type[type] & (1U << MP_UUID)) != 0; > : + enum mp_type mp_type = mem_mp_type(mem); > : + assert(mp_type != MP_EXT); > : + return field_mp_plain_type_is_compatible(type, mp_type, true); > : +} > : + > : const char * > : mem_str(const struct Mem *mem) > : { > : @@ -81,6 +91,8 @@ mem_str(const struct Mem *mem) > : case MEM_TYPE_MAP: > : case MEM_TYPE_ARRAY: > : return mp_str(mem->z); > : + case MEM_TYPE_UUID: > : + return tt_uuid_str(&mem->u.uuid); > : case MEM_TYPE_BOOL: > : return mem->u.b ? "TRUE" : "FALSE"; > : default: > : @@ -190,6 +202,16 @@ mem_set_double(struct Mem *mem, double value) > : mem->type = MEM_TYPE_DOUBLE; > : } > : > : +void > : +mem_set_uuid(struct Mem *mem, const struct tt_uuid *uuid) > : +{ > : + mem_clear(mem); > : + mem->field_type = FIELD_TYPE_UUID; > : + mem->u.uuid = *uuid; > : + mem->type = MEM_TYPE_UUID; > : + assert(mem->flags == 0); > > What's the point of this assertion? It looks redundant, because flags > supposed to be nulled at the end of mem_clear()? > (Not critical problem, but simply confusing) > For me it is something that can be used as comment that says that flags should be 0, also it checks flag in debug. > > : +} > : + > : static inline void > : set_str_const(struct Mem *mem, char *value, uint32_t len, int alloc_type) > : { > : @@ -585,6 +607,18 @@ str_to_bin(struct Mem *mem) > : return 0; > : } > : > : +static inline int > : +str_to_uuid(struct Mem *mem) > : +{ > : + assert(mem->type == MEM_TYPE_STR); > : + struct tt_uuid uuid; > : + if (mem->n != UUID_STR_LEN || > : + tt_uuid_from_string(tt_cstr(mem->z, mem->n), &uuid) != 0) > > Eventually, when we would be able to parse more relaxed formats in tt_uuid_from_string() > (e.g. with dashes omitted or braces around uuid) we would have to remove this check for > particular length UUID_STR_LEN, offloading any correctness check entirely to the function. > I'm not insisting that this check should be modified right away. Though, from future > Compatible point of view it would be ideal... But not required. Just saying. > > BTW, while we are here - could we avoid this static_alloc() call in tt_cstr() as a > prerequisite to tt_uuid_from_string(), and pass view of this data with mem->z, mem->n > directly there? > Actually, I can and I did. Fixed. Diff below. > : + return -1; > : + mem_set_uuid(mem, &uuid); > : + return 0; > : +} > : + > : static inline int > : str_to_bool(struct Mem *mem) > : { > : @@ -639,6 +673,19 @@ bin_to_str0(struct Mem *mem) > : return 0; > : } > : > : +static inline int > : +bin_to_uuid(struct Mem *mem) > : +{ > : + assert(mem->type == MEM_TYPE_BIN); > : + if (ExpandBlob(mem) != 0) > : + return -1; > : + if (mem->n != UUID_LEN || > : + tt_uuid_validate((struct tt_uuid *)mem->z) != 0) > : + return -1; > : + mem_set_uuid(mem, (struct tt_uuid *)mem->z); > : + return 0; > : +} > : + > : static inline int > : bytes_to_int(struct Mem *mem) > : { > : @@ -810,6 +857,22 @@ map_to_str0(struct Mem *mem) > : return mem_copy_str0(mem, str); > : } > : > : +static inline int > : +uuid_to_str0(struct Mem *mem) > : +{ > : + assert(mem->type == MEM_TYPE_UUID); > : + char buf[UUID_STR_LEN + 1]; > : + tt_uuid_to_string(&mem->u.uuid, &buf[0]); > : + return mem_copy_str0(mem, &buf[0]); > : +} > : + > : +static inline int > : +uuid_to_bin(struct Mem *mem) > : +{ > : + assert(mem->type == MEM_TYPE_UUID); > : + return mem_copy_bin(mem, (char *)&mem->u.uuid, UUID_LEN); > : +} > : + > : int > : mem_to_int(struct Mem *mem) > : { > : @@ -889,6 +952,8 @@ mem_to_str0(struct Mem *mem) > : return map_to_str0(mem); > : case MEM_TYPE_ARRAY: > : return array_to_str0(mem); > : + case MEM_TYPE_UUID: > : + return uuid_to_str0(mem); > : default: > : return -1; > : } > : @@ -914,6 +979,8 @@ mem_to_str(struct Mem *mem) > : return map_to_str0(mem); > : case MEM_TYPE_ARRAY: > : return array_to_str0(mem); > : + case MEM_TYPE_UUID: > : + return uuid_to_str0(mem); > : default: > : return -1; > : } > : @@ -966,9 +1033,19 @@ mem_cast_explicit(struct Mem *mem, enum field_type > : type) > : return str_to_bin(mem); > : if (mem_is_bytes(mem)) > : return 0; > : + if (mem->type == MEM_TYPE_UUID) > : + return uuid_to_bin(mem); > : return -1; > : case FIELD_TYPE_NUMBER: > : return mem_to_number(mem); > : + case FIELD_TYPE_UUID: > : + if (mem->type == MEM_TYPE_UUID) > : + return 0; > : + if (mem->type == MEM_TYPE_STR) > : + return str_to_uuid(mem); > : + if (mem->type == MEM_TYPE_BIN) > : + return bin_to_uuid(mem); > : + return -1; > : case FIELD_TYPE_SCALAR: > : if ((mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0) > : return -1; > : @@ -996,6 +1073,8 @@ mem_cast_implicit(struct Mem *mem, enum field_type > : type) > : case FIELD_TYPE_STRING: > : if (mem->type == MEM_TYPE_STR) > : return 0; > : + if (mem->type == MEM_TYPE_UUID) > : + return uuid_to_str0(mem); > : return -1; > : case FIELD_TYPE_DOUBLE: > : if (mem->type == MEM_TYPE_DOUBLE) > : @@ -1017,6 +1096,8 @@ mem_cast_implicit(struct Mem *mem, enum field_type > : type) > : if ((mem->type & (MEM_TYPE_BIN | MEM_TYPE_MAP | > : MEM_TYPE_ARRAY)) != 0) > : return 0; > : + if (mem->type == MEM_TYPE_UUID) > : + return uuid_to_bin(mem); > : return -1; > : case FIELD_TYPE_NUMBER: > : if (mem_is_num(mem)) > : @@ -1034,6 +1115,14 @@ mem_cast_implicit(struct Mem *mem, enum field_type > : type) > : if ((mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0) > : return -1; > : return 0; > : + case FIELD_TYPE_UUID: > : + if (mem->type == MEM_TYPE_UUID) > : + return 0; > : + if (mem->type == MEM_TYPE_STR) > : + return str_to_uuid(mem); > : + if (mem->type == MEM_TYPE_BIN) > : + return bin_to_uuid(mem); > : + return -1; > : case FIELD_TYPE_ANY: > : return 0; > : default: > : @@ -1063,6 +1152,8 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type > : type) > : return int_to_str0(mem); > : if (mem->type == MEM_TYPE_DOUBLE) > : return double_to_str0(mem); > : + if (mem->type == MEM_TYPE_UUID) > : + return uuid_to_str0(mem); > : return -1; > : case FIELD_TYPE_DOUBLE: > : if (mem->type == MEM_TYPE_DOUBLE) > : @@ -1087,6 +1178,8 @@ mem_cast_implicit_old(struct Mem *mem, enum field_type > : type) > : case FIELD_TYPE_VARBINARY: > : if (mem->type == MEM_TYPE_BIN) > : return 0; > : + if (mem->type == MEM_TYPE_UUID) > : + return uuid_to_bin(mem); > : return -1; > : case FIELD_TYPE_NUMBER: > : if (mem_is_num(mem)) > : @@ -1106,6 +1199,14 @@ mem_cast_implicit_old(struct Mem *mem, enum > : field_type type) > : if ((mem->type & (MEM_TYPE_MAP | MEM_TYPE_ARRAY)) != 0) > : return -1; > : return 0; > : + case FIELD_TYPE_UUID: > : + if (mem->type == MEM_TYPE_UUID) > : + return 0; > : + if (mem->type == MEM_TYPE_STR) > : + return str_to_uuid(mem); > : + if (mem->type == MEM_TYPE_BIN) > : + return bin_to_uuid(mem); > : + return -1; > : default: > : break; > : } > > Uh-oh, those changes in mem_cast_implicit_old() will conflict with my > cleanup here.But I'll handle it. > (OTOH, could we not touch mem_cast_implicit_old as it will be deleted soon?) > I believe we can't. It is easier to remove function during merge than to fix possible inconsistency. > : @@ -1899,6 +2000,15 @@ mem_cmp_str(const struct Mem *left, const struct Mem > : *right, int *result, > : return 0; > : } > : > : +int > : +mem_cmp_uuid(const struct Mem *a, const struct Mem *b, int *result) > : +{ > : + if ((a->type & b->type & MEM_TYPE_UUID) == 0) > : + return -1; > : + *result = memcmp(&a->u.uuid, &b->u.uuid, UUID_LEN); > : + return 0; > : +} > : + > : /* > : * Both *pMem1 and *pMem2 contain string values. Compare the two values > : * using the collation sequence pColl. As usual, return a negative , zero > : @@ -1951,13 +2061,15 @@ mem_type_to_str(const struct Mem *p) > : return "varbinary"; > : case MEM_TYPE_BOOL: > : return "boolean"; > : + case MEM_TYPE_UUID: > : + return "uuid"; > : default: > : unreachable(); > : } > : } > : > : enum mp_type > : -mem_mp_type(struct Mem *mem) > : +mem_mp_type(const struct Mem *mem) > : { > : assert(mem->type < MEM_TYPE_INVALID); > : switch (mem->type) { > : @@ -1979,6 +2091,8 @@ mem_mp_type(struct Mem *mem) > : return MP_BOOL; > : case MEM_TYPE_DOUBLE: > : return MP_DOUBLE; > : + case MEM_TYPE_UUID: > : + return MP_EXT; > : default: > : unreachable(); > : } > : @@ -2144,6 +2258,9 @@ memTracePrint(Mem *p) > : case MEM_TYPE_BOOL: > : printf(" bool:%s", SQL_TOKEN_BOOLEAN(p->u.b)); > : return; > : + case MEM_TYPE_UUID: > : + printf(" uuid:%s", tt_uuid_str(&p->u.uuid)); > : + return; > : default: { > : char zBuf[200]; > : sqlVdbeMemPrettyPrint(p, zBuf); > : @@ -2360,6 +2477,14 @@ sqlMemCompare(const Mem * pMem1, const Mem * pMem2, > : const struct coll * pColl) > : return -1; > : } > : > : + if (((type1 | type2) & MEM_TYPE_UUID) != 0) { > : + if (mem_cmp_uuid(pMem1, pMem2, &res) == 0) > : + return res; > : + if (type1 != MEM_TYPE_UUID) > : + return +1; > : + return -1; > : + } > : + > : /* At least one of the two values is a number > : */ > : if (((type1 | type2) & > : @@ -2565,13 +2690,30 @@ sqlVdbeCompareMsgpack(const char **key1, > : break; > : } > : case MP_ARRAY: > : - case MP_MAP: > : - case MP_EXT:{ > : + case MP_MAP: { > : mem1.z = (char *)aKey1; > : mp_next(&aKey1); > : mem1.n = aKey1 - (char *)mem1.z; > : goto do_blob; > : } > : + case MP_EXT: { > : + int8_t type; > : + const char *buf = aKey1; > : + uint32_t len = mp_decode_extl(&aKey1, &type); > : + if (type == MP_UUID) { > : + assert(len == UUID_LEN); > : + mem1.type = MEM_TYPE_UUID; > : + aKey1 = buf; > : + if (mp_decode_uuid(&aKey1, &mem1.u.uuid) == NULL || > : + mem_cmp_uuid(&mem1, pKey2, &rc) != 0) > : + rc = 1; > : + break; > : + } > : + aKey1 += len; > : + mem1.z = (char *)buf; > : + mem1.n = aKey1 - buf; > : + goto do_blob; > : + } > : } > : *key1 = aKey1; > : return rc; > : @@ -2625,9 +2767,25 @@ mem_from_mp_ephemeral(struct Mem *mem, const char > : *buf, uint32_t *len) > : break; > : } > : case MP_EXT: { > : - mem->z = (char *)buf; > : - mp_next(&buf); > : - mem->n = buf - mem->z; > : + int8_t type; > : + const char *svp = buf; > : + uint32_t size = mp_decode_extl(&buf, &type); > : + if (type == MP_UUID) { > : + assert(size == UUID_LEN); > : + buf = svp; > : + if (mp_decode_uuid(&buf, &mem->u.uuid) == NULL) { > : + diag_set(ClientError, ER_INVALID_MSGPACK, > : + "Invalid MP_UUID MsgPack format"); > : + return -1; > : + } > : + mem->type = MEM_TYPE_UUID; > : + mem->flags = 0; > : + mem->field_type = FIELD_TYPE_UUID; > : + break; > : + } > : + buf += size; > : + mem->z = (char *)svp; > : + mem->n = buf - svp; > : mem->type = MEM_TYPE_BIN; > : mem->flags = MEM_Ephem; > : mem->field_type = FIELD_TYPE_VARBINARY; > : @@ -2764,6 +2922,9 @@ mpstream_encode_vdbe_mem(struct mpstream *stream, > : struct Mem *var) > : case MEM_TYPE_BOOL: > : mpstream_encode_bool(stream, var->u.b); > : return; > : + case MEM_TYPE_UUID: > : + mpstream_encode_uuid(stream, &var->u.uuid); > : + return; > : default: > : unreachable(); > : } > : @@ -2850,6 +3011,9 @@ port_vdbemem_dump_lua(struct port *base, struct > : lua_State *L, bool is_flat) > : case MEM_TYPE_BOOL: > : lua_pushboolean(L, mem->u.b); > : break; > : + case MEM_TYPE_UUID: > : + *luaL_pushuuid(L) = mem->u.uuid; > : + break; > : default: > : unreachable(); > : } > : @@ -2976,14 +3140,8 @@ port_lua_get_vdbemem(struct port *base, uint32_t > : *size) > : uint32_t size; > : uint32_t svp = region_used(&fiber()->gc); > : if (field.ext_type == MP_UUID) { > : - size = mp_sizeof_uuid(); > : - buf = region_alloc(&fiber()->gc, size); > : - if (buf == NULL) { > : - diag_set(OutOfMemory, size, > : - "region_alloc", "buf"); > : - goto error; > : - } > : - mp_encode_uuid(buf, field.uuidval); > : + mem_set_uuid(&val[i], field.uuidval); > : + break; > : } else { > : size = mp_sizeof_decimal(field.decval); > : buf = region_alloc(&fiber()->gc, size); > : @@ -3087,7 +3245,22 @@ port_c_get_vdbemem(struct port *base, uint32_t *size) > : break; > : case MP_EXT: > : str = data; > : - mp_next(&data); > : + int8_t type; > : + len = mp_decode_extl(&data, &type); > : + if (type == MP_UUID) { > : + assert(len == UUID_LEN); > : + struct tt_uuid *uuid = &val[i].u.uuid; > : + data = str; > : + if (mp_decode_uuid(&data, uuid) == NULL) { > : + diag_set(ClientError, > : + ER_INVALID_MSGPACK, "Invalid " > : + "MP_UUID MsgPack format"); > : + goto error; > : + } > : + val[i].type = MEM_TYPE_UUID; > : + break; > : + } > : + data += len; > : if (mem_copy_bin(&val[i], str, data - str) != 0) > : goto error; > : break; > : diff --git a/src/box/sql/mem.h b/src/box/sql/mem.h > : index 15d97da0e..b3cd5c545 100644 > : --- a/src/box/sql/mem.h > : +++ b/src/box/sql/mem.h > : @@ -30,6 +30,7 @@ > : * SUCH DAMAGE. > : */ > : #include "box/field_def.h" > : +#include "uuid/tt_uuid.h" > : > : struct sql; > : struct Vdbe; > : @@ -47,10 +48,11 @@ enum mem_type { > : MEM_TYPE_MAP = 1 << 6, > : MEM_TYPE_BOOL = 1 << 7, > : MEM_TYPE_DOUBLE = 1 << 8, > : - MEM_TYPE_INVALID = 1 << 9, > : - MEM_TYPE_FRAME = 1 << 10, > : - MEM_TYPE_PTR = 1 << 11, > : - MEM_TYPE_AGG = 1 << 12, > : + MEM_TYPE_UUID = 1 << 9, > : + MEM_TYPE_INVALID = 1 << 10, > : + MEM_TYPE_FRAME = 1 << 11, > : + MEM_TYPE_PTR = 1 << 12, > : + MEM_TYPE_AGG = 1 << 13, > : }; > : > : /* > : @@ -72,6 +74,7 @@ struct Mem { > : */ > : struct func *func; > : struct VdbeFrame *pFrame; /* Used when flags==MEM_Frame */ > : + struct tt_uuid uuid; > : } u; > : /** Type of the value this MEM contains. */ > : enum mem_type type; > : @@ -255,6 +258,10 @@ mem_is_any_null(const struct Mem *mem1, const struct > : Mem *mem2) > : return ((mem1->type| mem2->type) & MEM_TYPE_NULL) != 0; > : } > : > : +/** Check if MEM is compatible with field type. */ > : +bool > : +mem_is_field_compatible(const struct Mem *mem, enum field_type type); > : + > : /** > : * Return a string that represent content of MEM. String is either > : allocated > : * using static_alloc() of just a static variable. > : @@ -290,6 +297,10 @@ mem_set_bool(struct Mem *mem, bool value); > : void > : mem_set_double(struct Mem *mem, double value); > : > : +/** Clear MEM and set it to UUID. */ > : +void > : +mem_set_uuid(struct Mem *mem, const struct tt_uuid *uuid); > : + > : /** Clear MEM and set it to STRING. The string belongs to another object. > : */ > : void > : mem_set_str_ephemeral(struct Mem *mem, char *value, uint32_t len); > : @@ -677,6 +688,14 @@ mem_cmp_str(const struct Mem *left, const struct Mem > : *right, int *result, > : int > : mem_cmp_num(const struct Mem *a, const struct Mem *b, int *result); > : > : +/** > : + * Compare two MEMs and return the result of comparison. MEMs should be of > : + * UUID type or their values are converted to UUID according to > : + * implicit cast rules. Original MEMs are not changed. > : + */ > : +int > : +mem_cmp_uuid(const struct Mem *left, const struct Mem *right, int *result); > : + > : /** > : * Convert the given MEM to INTEGER. This function and the function below > : define > : * the rules that are used to convert values of all other types to INTEGER. > : In > : @@ -898,7 +917,7 @@ mem_type_to_str(const struct Mem *p); > : * transparent memory cell. > : */ > : enum mp_type > : -mem_mp_type(struct Mem *mem); > : +mem_mp_type(const struct Mem *mem); > : > : enum mp_type > : sql_value_type(struct Mem *); > : diff --git a/src/box/sql/parse.y b/src/box/sql/parse.y > : index abc363951..4c9cf475e 100644 > : --- a/src/box/sql/parse.y > : +++ b/src/box/sql/parse.y > : @@ -1834,6 +1834,7 @@ typedef(A) ::= SCALAR . { A.type = FIELD_TYPE_SCALAR; > : } > : typedef(A) ::= BOOL . { A.type = FIELD_TYPE_BOOLEAN; } > : typedef(A) ::= BOOLEAN . { A.type = FIELD_TYPE_BOOLEAN; } > : typedef(A) ::= VARBINARY . { A.type = FIELD_TYPE_VARBINARY; } > : +typedef(A) ::= UUID . { A.type = FIELD_TYPE_UUID; } > : > : /** > : * Time-like types are temporary disabled, until they are > : diff --git a/src/box/sql/vdbe.c b/src/box/sql/vdbe.c > : index 12ec703a2..32d02d96e 100644 > : --- a/src/box/sql/vdbe.c > : +++ b/src/box/sql/vdbe.c > : @@ -1322,10 +1322,10 @@ case OP_FunctionByName: { > : region_truncate(region, region_svp); > : if (mem == NULL) > : goto abort_due_to_error; > : - enum mp_type type = sql_value_type((sql_value *)pOut); > : - if (!field_mp_plain_type_is_compatible(returns, type, true)) { > : + if (!mem_is_field_compatible(pOut, returns)) { > : diag_set(ClientError, ER_FUNC_INVALID_RETURN_TYPE, pOp->p4.z, > : - field_type_strs[returns], mp_type_strs[type]); > : + field_type_strs[returns], > : + mp_type_strs[mem_mp_type(pOut)]); > : goto abort_due_to_error; > : } > : > : @@ -1634,6 +1634,15 @@ case OP_Ge: { /* same as TK_GE, jump, > : in1, in3 */ > : "boolean"); > : goto abort_due_to_error; > : } > : + } else if (((pIn3->type | pIn1->type) & MEM_TYPE_UUID) != 0) { > : + if (mem_cmp_uuid(pIn3, pIn1, &res) != 0) { > : + char *str = pIn3->type != MEM_TYPE_UUID ? > : + mem_type_to_str(pIn3) : > : + mem_type_to_str(pIn1); > : + diag_set(ClientError, ER_SQL_TYPE_MISMATCH, str, > : + "uuid"); > : + goto abort_due_to_error; > : + } > : } else if (mem_is_bin(pIn3) || mem_is_bin(pIn1)) { > : if (mem_cmp_bin(pIn3, pIn1, &res) != 0) { > : char *str = !mem_is_bin(pIn3) ? > > In general, despite minor complains, it all looks good to me, and ok to commit. > Those complains we will address later. > > LGTM > > Timur > Diff: diff --git a/src/box/sql/mem.c b/src/box/sql/mem.c index f0dfce395..6f3bf52e5 100644 --- a/src/box/sql/mem.c +++ b/src/box/sql/mem.c @@ -612,8 +612,7 @@ str_to_uuid(struct Mem *mem) { assert(mem->type == MEM_TYPE_STR); struct tt_uuid uuid; - if (mem->n != UUID_STR_LEN || - tt_uuid_from_string(tt_cstr(mem->z, mem->n), &uuid) != 0) + if (tt_uuid_from_strl(mem->z, mem->n, &uuid) != 0) return -1; mem_set_uuid(mem, &uuid); return 0;