[Tarantool-patches] [PATCH 1/1] sql: introduce UUID field type

[Timur Safin]

There is one major issue I'd like to discuss, please see below...

: From: Mergen Imeev
: Subject: Re: [Tarantool-patches] [PATCH 1/1] sql: introduce UUID field type
: 
...

: +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)

Here lies the limitation I've specifically wanted to avoid and put
below to RFC:

"The most complete implementation seemingly is in PostgreSQL, 
which allows various relaxed formats for string literals 
which may be accepted as UUID - the plan is to be as close
as possible to PostgreSQL here."

Checking for UUID_STR_LEN restricts only to 1 specific form with
dashes in specific positions, but we would rather want to allow
relaxed formats also, with some dashes omitted. Also with optional
braces around. i.e. putting this as regexp

  \{?[0-9a-f]{8}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{4}-?[0-9a-f]{12}\}?

And yes, I mean we need to reimplement tt_uuid_from_string() differently.
(If you were asking me then I'd use RE2C for that purpose, but that up
to you)

: +		return -1;
: +	mem_set_uuid(mem, &uuid);
: +	return 0;
: +}
: +
:  static inline int
:  str_to_bool(struct Mem *mem)
:  {
...
: 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,

I guess there should be no incompatibility problems here, but just
in case I ask - why did you insert new constant to the middle of prior list?

Also I see there is another omission from RFC - there is nothing added 
for SQL built-in support to generate UUID from within SQL mode.

  "Introduce UUID([version#]) functions which would allow generating
   any particular type of GUID. If version argument is omitted then
   we generate UUID v4 (as used by default in box);"

Regards,
Timur