From: Timur Safin via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: <imeevma@tarantool.org> Cc: <tarantool-patches@dev.tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 1/2] sql: introduce UUID field type Date: Wed, 2 Jun 2021 23:13:00 +0300 [thread overview] Message-ID: <04e101d757eb$af9f1b10$0edd5130$@tarantool.org> (raw) In-Reply-To: <e39897ab529959457ad8370e3098870b9b69901b.1622620858.git.imeevma@gmail.com> There are few minor complains... : -----Original Message----- : From: imeevma@tarantool.org <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) : +} : + : 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? : + 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?) : @@ -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
next prev parent reply other threads:[~2021-06-02 20:13 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-06-02 8:02 [Tarantool-patches] [PATCH v2 0/2] sql: introduce UUID Mergen Imeev via Tarantool-patches 2021-06-02 8:02 ` [Tarantool-patches] [PATCH v2 1/2] sql: introduce UUID field type Mergen Imeev via Tarantool-patches 2021-06-02 20:13 ` Timur Safin via Tarantool-patches [this message] 2021-06-03 9:04 ` Mergen Imeev via Tarantool-patches 2021-06-03 9:40 ` Timur Safin via Tarantool-patches 2021-06-02 8:02 ` [Tarantool-patches] [PATCH v2 2/2] sql: introduce SQL built-in function UUID() Mergen Imeev via Tarantool-patches 2021-06-02 20:13 ` Timur Safin via Tarantool-patches 2021-06-02 20:13 ` [Tarantool-patches] [PATCH v2 0/2] sql: introduce UUID Timur Safin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to='04e101d757eb$af9f1b10$0edd5130$@tarantool.org' \ --to=tarantool-patches@dev.tarantool.org \ --cc=imeevma@tarantool.org \ --cc=tsafin@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 1/2] sql: introduce UUID field type' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox