[Tarantool-patches] [PATCH v2 1/2] sql: introduce UUID field type
Mergen Imeev
imeevma at tarantool.org
Thu Jun 3 12:04:55 MSK 2021
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 at tarantool.org <imeevma at tarantool.org>
> : Sent: Wednesday, June 2, 2021 11:03 AM
> : To: tsafin at tarantool.org
> : Cc: tarantool-patches at 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;
More information about the Tarantool-patches
mailing list