[Tarantool-patches] [PATCH 0/4] introduce indices over UUID

[Tarantool-patches] [PATCH 0/4] introduce indices over UUID

[Serge Petrenko]

https://github.com/tarantool/tarantool/issues/4268
https://github.com/tarantool/tarantool/tree/sp/gh-4268-uuid-type

Serge Petrenko (4):
  decimal: fix comment typo
  uuid: expose additional from_string constructors
  box: add MsgPack encoding/decoding for UUID
  box: introduce indices by UUID

 extra/exports                        |   3 +
 src/box/field_def.c                  |  66 +++++++++++---
 src/box/field_def.h                  |  16 ++++
 src/box/key_def.h                    |   3 +-
 src/box/tuple_compare.cc             | 123 +++++++++++++++++++++++++++
 src/box/tuple_format.c               |   3 +-
 src/lib/core/CMakeLists.txt          |   1 +
 src/lib/core/mp_decimal.h            |   2 +-
 src/lib/core/mp_extension_types.h    |   2 +
 src/lib/core/mp_uuid.c               |  75 ++++++++++++++++
 src/lib/core/mp_uuid.h               |  90 ++++++++++++++++++++
 src/lib/core/mpstream.c              |  11 +++
 src/lib/core/mpstream.h              |   5 ++
 src/lib/msgpuck                      |   2 +-
 src/lib/uuid/tt_uuid.c               |   9 ++
 src/lib/uuid/tt_uuid.h               |  53 +++++++++---
 src/lua/msgpack.c                    |  27 ++++--
 src/lua/msgpackffi.lua               |  14 +++
 src/lua/utils.c                      |  21 ++++-
 src/lua/utils.h                      |   5 ++
 src/lua/uuid.lua                     |   9 --
 test/app-tap/lua/serializer_test.lua |   8 ++
 test/app-tap/msgpackffi.test.lua     |   3 +-
 test/app/msgpack.result              |  21 +++++
 test/app/msgpack.test.lua            |  13 +++
 test/app/uuid.result                 |   2 +-
 test/box/tuple.result                |  81 ++++++++++++++++++
 test/box/tuple.test.lua              |  25 ++++++
 test/engine/ddl.result               |  97 ++++++++++++++++++++-
 test/engine/ddl.test.lua             |  42 ++++++++-
 test/engine/gh-4268-uuid.result      |  58 +++++++++++++
 test/engine/gh-4268-uuid.test.lua    |  30 +++++++
 test/unit/uuid.c                     |  24 +++++-
 test/unit/uuid.result                |   8 +-
 third_party/lua-cjson/lua_cjson.c    |  27 ++++--
 third_party/lua-yaml/lyaml.cc        |  17 +++-
 36 files changed, 933 insertions(+), 63 deletions(-)
 create mode 100644 src/lib/core/mp_uuid.c
 create mode 100644 src/lib/core/mp_uuid.h
 create mode 100644 test/engine/gh-4268-uuid.result
 create mode 100644 test/engine/gh-4268-uuid.test.lua

-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 1/4] decimal: fix comment typo

[Serge Petrenko]

---
 src/lib/core/mp_decimal.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/lib/core/mp_decimal.h b/src/lib/core/mp_decimal.h
index 778529068..3c708a8e8 100644
--- a/src/lib/core/mp_decimal.h
+++ b/src/lib/core/mp_decimal.h
@@ -56,7 +56,7 @@ mp_decode_decimal(const char **data, decimal_t *dec);
 
 /**
  * \brief Encode a decimal pointed to by \a dec.
- * \parad dec - decimal pointer
+ * \param dec - decimal pointer
  * \param data - a buffer
  * \return \a data + mp_sizeof_decimal(\a dec)
  */
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors

[Serge Petrenko]

tt_uuid_from_string() checks string length via strlen to determine
whether the string is valid. When a string is decoded from arbitrary
msgpack data, it may not be null-terminated, so strlen() will return
incorrect results.
When a string is decoded from msgpack, its length is returned from the
decoder, so add tt_uuid_from_lstring(), which accepts string length and
validates it.
Allow tt_uuid_from_lstring() construct uuids from both 'normal'
36-byte strings, like '88ebbb9a-8c18-480f-bd04-dd5345f1573c',
and 32-byte stripped strings, like '88ebbb9a8c18480fbd04dd5345f1573c'.
Add a function tt_uuid_validate(), which checks that uuid conforms to
one of the variants.

Prerequisite #4268
---
 src/lib/uuid/tt_uuid.c |  9 +++++++
 src/lib/uuid/tt_uuid.h | 53 ++++++++++++++++++++++++++++++++----------
 2 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/src/lib/uuid/tt_uuid.c b/src/lib/uuid/tt_uuid.c
index 1bd2e2cfe..94a0b15bb 100644
--- a/src/lib/uuid/tt_uuid.c
+++ b/src/lib/uuid/tt_uuid.c
@@ -65,6 +65,15 @@ tt_uuid_create(struct tt_uuid *uu)
 }
 #endif
 
+extern inline int
+tt_uuid_validate(struct tt_uuid *uu);
+
+extern inline int
+tt_uuid_from_fmt_string(const char *in, struct tt_uuid *uu, const char *fmt);
+
+extern inline int
+tt_uuid_from_lstring(const char *in, uint32_t len, struct tt_uuid *uu);
+
 extern inline int
 tt_uuid_from_string(const char *in, struct tt_uuid *uu);
 
diff --git a/src/lib/uuid/tt_uuid.h b/src/lib/uuid/tt_uuid.h
index 48cd68e69..8f293a1e0 100644
--- a/src/lib/uuid/tt_uuid.h
+++ b/src/lib/uuid/tt_uuid.h
@@ -41,7 +41,7 @@
 extern "C" {
 #endif
 
-enum { UUID_LEN = 16, UUID_STR_LEN = 36 };
+enum { UUID_LEN = 16, UUID_STR_PLAIN_LEN = 32, UUID_STR_LEN = 36 };
 
 /**
  * \brief UUID structure struct
@@ -62,6 +62,28 @@ struct tt_uuid {
 void
 tt_uuid_create(struct tt_uuid *uu);
 
+inline int
+tt_uuid_validate(struct tt_uuid *uu)
+{
+	/* Check variant (NCS, RFC4122, MSFT) */
+	uint8_t n = uu->clock_seq_hi_and_reserved;
+	if ((n & 0x80) != 0x00 && (n & 0xc0) != 0x80 &&	(n & 0xe0) != 0xc0)
+		return 1;
+	return 0;
+}
+
+inline int
+tt_uuid_from_fmt_string(const char *in, struct tt_uuid *uu,
+			const char *fmt)
+{
+	if (sscanf(in, fmt, &uu->time_low, &uu->time_mid,
+		   &uu->time_hi_and_version, &uu->clock_seq_hi_and_reserved,
+		   &uu->clock_seq_low, &uu->node[0], &uu->node[1], &uu->node[2],
+		   &uu->node[3], &uu->node[4], &uu->node[5]) != 11)
+		return 1;
+	return tt_uuid_validate(uu);
+}
+
 /**
  * \brief Parse UUID from string.
  * \param in string
@@ -71,19 +93,26 @@ tt_uuid_create(struct tt_uuid *uu);
 inline int
 tt_uuid_from_string(const char *in, struct tt_uuid *uu)
 {
-	if (strlen(in) != UUID_STR_LEN ||
-	    sscanf(in, "%8x-%4hx-%4hx-%2hhx%2hhx-%2hhx%2hhx%2hhx%2hhx%2hhx%2hhx",
-		   &uu->time_low, &uu->time_mid, &uu->time_hi_and_version,
-		   &uu->clock_seq_hi_and_reserved, &uu->clock_seq_low,
-		   &uu->node[0], &uu->node[1], &uu->node[2], &uu->node[3],
-		   &uu->node[4], &uu->node[5]) != 11)
+	if (strlen(in) != UUID_STR_LEN)
 		return 1;
+	return tt_uuid_from_fmt_string(in, uu,
+				       "%8x-%4hx-%4hx-%2hhx%2hhx-"
+				       "%2hhx%2hhx%2hhx%2hhx%2hhx%2hhx");
+}
 
-	/* Check variant (NCS, RFC4122, MSFT) */
-	uint8_t n = uu->clock_seq_hi_and_reserved;
-	if ((n & 0x80) != 0x00 && (n & 0xc0) != 0x80 &&	(n & 0xe0) != 0xc0)
-		return 1;
-	return 0;
+inline int
+tt_uuid_from_lstring(const char *in, uint32_t len, struct tt_uuid *uu)
+{
+	if (len == UUID_STR_PLAIN_LEN) {
+		return tt_uuid_from_fmt_string(in, uu,
+					       "%8x%4hx%4hx%2hhx%2hhx%2hhx%2hhx"
+					       "%2hhx%2hhx%2hhx%2hhx");
+	} else if (len == UUID_STR_LEN) {
+		return tt_uuid_from_fmt_string(in, uu,
+					       "%8x-%4hx-%4hx-%2hhx%2hhx-"
+					       "%2hhx%2hhx%2hhx%2hhx%2hhx%2hhx");
+	}
+	return 1;
 }
 
 /**
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID

[Serge Petrenko]

A special format for encoding UUIDs to MsgPack is introduced.
It is supported by both lua and C encoders/decoders, and it is now
possible to insert UUIDs into spaces, but only into unindexed fields
without format for now.

Prerequisite #4268

@TarantoolBot document
Title: Internals: msgpack format for UUID

UUID values share the MessagePack type with decimals:
both use MP_EXT. A new subtype is introduced for UUIDs,
MP_UUID = `0x02`
UUID is encoded as follows:
```
    +--------+---------+-----------+
    | MP_EXT | MP_UUID | UuidValue |
    +--------+---------+-----------+
```
Since UUID is 16 bytes in size, the header, MP_EXT, is always the same:
`0xd8`. MP_UUID = `0x02` follows. The header is followed by the 16
bytes of the UuidValue.
The total size of such a representation is 18 bytes, whereas storing
uuids as strings requires from 34 (when '-'s are ommitted) to 38 bytes
per UUID, giving a 2x space usage improvement.
---
 extra/exports                        |  3 +
 src/lib/core/CMakeLists.txt          |  1 +
 src/lib/core/mp_extension_types.h    |  2 +
 src/lib/core/mp_uuid.c               | 75 +++++++++++++++++++++++
 src/lib/core/mp_uuid.h               | 90 ++++++++++++++++++++++++++++
 src/lib/core/mpstream.c              | 11 ++++
 src/lib/core/mpstream.h              |  5 ++
 src/lib/msgpuck                      |  2 +-
 src/lua/msgpack.c                    | 27 +++++++--
 src/lua/msgpackffi.lua               | 14 +++++
 src/lua/utils.c                      | 21 ++++++-
 src/lua/utils.h                      |  5 ++
 src/lua/uuid.lua                     |  9 ---
 test/app-tap/lua/serializer_test.lua |  8 +++
 test/app-tap/msgpackffi.test.lua     |  3 +-
 test/app/msgpack.result              | 21 +++++++
 test/app/msgpack.test.lua            | 13 ++++
 test/app/uuid.result                 |  2 +-
 test/box/tuple.result                | 81 +++++++++++++++++++++++++
 test/box/tuple.test.lua              | 25 ++++++++
 test/unit/uuid.c                     | 24 +++++++-
 test/unit/uuid.result                |  8 ++-
 third_party/lua-cjson/lua_cjson.c    | 27 ++++++---
 third_party/lua-yaml/lyaml.cc        | 17 +++++-
 24 files changed, 461 insertions(+), 33 deletions(-)
 create mode 100644 src/lib/core/mp_uuid.c
 create mode 100644 src/lib/core/mp_uuid.h

diff --git a/extra/exports b/extra/exports
index cbb5adcf4..9dcb6bdcb 100644
--- a/extra/exports
+++ b/extra/exports
@@ -63,11 +63,14 @@ crc32_calc
 mp_encode_double
 mp_encode_float
 mp_encode_decimal
+mp_encode_uuid
 mp_decode_double
 mp_decode_float
 mp_decode_extl
 mp_sizeof_decimal
+mp_sizeof_uuid
 decimal_unpack
+uuid_unpack
 
 log_type
 say_set_log_level
diff --git a/src/lib/core/CMakeLists.txt b/src/lib/core/CMakeLists.txt
index 3f13ff904..44968c2c9 100644
--- a/src/lib/core/CMakeLists.txt
+++ b/src/lib/core/CMakeLists.txt
@@ -29,6 +29,7 @@ set(core_sources
     port.c
     decimal.c
     mp_decimal.c
+    mp_uuid.c
 )
 
 if (TARGET_OS_NETBSD)
diff --git a/src/lib/core/mp_extension_types.h b/src/lib/core/mp_extension_types.h
index bc9873f68..7d42f212b 100644
--- a/src/lib/core/mp_extension_types.h
+++ b/src/lib/core/mp_extension_types.h
@@ -42,6 +42,8 @@
 enum mp_extension_type {
     MP_UNKNOWN_EXTENSION = 0,
     MP_DECIMAL = 1,
+    MP_UUID = 2,
+    mp_extension_type_MAX,
 };
 
 #endif
diff --git a/src/lib/core/mp_uuid.c b/src/lib/core/mp_uuid.c
new file mode 100644
index 000000000..5b0cc3f1a
--- /dev/null
+++ b/src/lib/core/mp_uuid.c
@@ -0,0 +1,75 @@
+/*
+ * Copyright 2020, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include "mp_uuid.h"
+#include "msgpuck.h"
+#include "mp_extension_types.h"
+#include "lib/uuid/tt_uuid.h"
+
+inline uint32_t
+mp_sizeof_uuid()
+{
+	return mp_sizeof_ext(sizeof(struct tt_uuid));
+}
+
+struct tt_uuid *
+uuid_unpack(const char **data, uint32_t len, struct tt_uuid *uuid)
+{
+	if (len != sizeof(*uuid))
+		return NULL;
+	memcpy(uuid, *data, sizeof(*uuid));
+	if (tt_uuid_validate(uuid) != 0)
+		return NULL;
+	*data += sizeof(*uuid);
+	return uuid;
+}
+
+struct tt_uuid *
+mp_decode_uuid(const char **data, struct tt_uuid *uuid)
+{
+	if (mp_typeof(**data) != MP_EXT)
+		return NULL;
+	int8_t type;
+	const char *const svp = *data;
+
+	uint32_t len = mp_decode_extl(data, &type);
+	if (type != MP_UUID || uuid_unpack(data, len, uuid) == NULL) {
+		*data = svp;
+		return NULL;
+	}
+	return uuid;
+}
+
+char *
+mp_encode_uuid(char *data, const struct tt_uuid *uuid)
+{
+	return mp_encode_ext(data, MP_UUID, (char *)uuid, sizeof(*uuid));
+}
diff --git a/src/lib/core/mp_uuid.h b/src/lib/core/mp_uuid.h
new file mode 100644
index 000000000..fda0f3aed
--- /dev/null
+++ b/src/lib/core/mp_uuid.h
@@ -0,0 +1,90 @@
+#ifndef TARANTOOL_LIB_CORE_MP_UUID_INCLUDED
+#define TARANTOOL_LIB_CORE_MP_UUID_INCLUDED
+/*
+ * Copyright 2020, Tarantool AUTHORS, please see AUTHORS file.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above
+ *    copyright notice, this list of conditions and the
+ *    following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above
+ *    copyright notice, this list of conditions and the following
+ *    disclaimer in the documentation and/or other materials
+ *    provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY <COPYRIGHT HOLDER> ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
+ * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
+ * <COPYRIGHT HOLDER> OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT,
+ * INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF
+ * THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#include <stdint.h>
+
+#if defined(__cplusplus)
+extern "C" {
+#endif /* defined(__cplusplus) */
+
+struct tt_uuid;
+
+/**
+ * \brief Return the number of bytes an encoded uuid value takes.
+ */
+uint32_t
+mp_sizeof_uuid();
+
+/**
+ * Copy a uuid value from a buffer. Can be used in a combination
+ * with mp_decode_extl() instead of mp_decode_uuid() when multiple
+ * extension types are possible.
+ *
+ * \param data A buffer.
+ * \param len Length returned by mp_decode_extl, has to be equal
+ *            to sizeof(struct tt_uuid), otherwise an error is
+ *            returned.
+ * \param[out] uuid Uuid to be decoded.
+ * \return A pointer to the decoded uuid.
+ *         NULL in case of an error.
+ * \post *data = *data + sizeof(struct tt_uuid).
+ */
+struct tt_uuid *
+uuid_unpack(const char **data, uint32_t len, struct tt_uuid *uuid);
+
+/**
+ * \brief Decode a uuid from MsgPack \a data.
+ * \param data A buffer.
+ * \param[out] uuid Uuid to be decoded.
+ * \return A pointer to the decoded uuid.
+ *         NULL in case of an error.
+ * \post *data = *data + mp_sizeof_uuid().
+ */
+struct tt_uuid *
+mp_decode_uuid(const char **data, struct tt_uuid *uuid);
+
+/**
+ * \brief Encode a uuid.
+ * \param data A buffer.
+ * \param uuid A uuid to encode.
+ *
+ * \return \a data + mp_sizeof_uuid()
+ */
+char *
+mp_encode_uuid(char *data, const struct tt_uuid *uuid);
+
+#if defined(__cplusplus)
+} /* extern "C" */
+#endif /* defined(__cplusplus) */
+
+#endif /* TARANTOOL_LIB_CORE_MP_UUID_INCLUDED */
diff --git a/src/lib/core/mpstream.c b/src/lib/core/mpstream.c
index 2be1797d0..758bf5e55 100644
--- a/src/lib/core/mpstream.c
+++ b/src/lib/core/mpstream.c
@@ -34,6 +34,7 @@
 #include <stdint.h>
 #include "msgpuck.h"
 #include "mp_decimal.h"
+#include "mp_uuid.h"
 
 void
 mpstream_reserve_slow(struct mpstream *stream, size_t size)
@@ -197,6 +198,16 @@ mpstream_encode_decimal(struct mpstream *stream, const decimal_t *val)
 	mpstream_advance(stream, pos - data);
 }
 
+void
+mpstream_encode_uuid(struct mpstream *stream, const struct tt_uuid *uuid)
+{
+	char *data = mpstream_reserve(stream, mp_sizeof_uuid());
+	if (data == NULL)
+		return;
+	char *pos = mp_encode_uuid(data, uuid);
+	mpstream_advance(stream, pos - data);
+}
+
 void
 mpstream_memcpy(struct mpstream *stream, const void *src, uint32_t n)
 {
diff --git a/src/lib/core/mpstream.h b/src/lib/core/mpstream.h
index 3a022daa0..a60add143 100644
--- a/src/lib/core/mpstream.h
+++ b/src/lib/core/mpstream.h
@@ -38,6 +38,8 @@
 extern "C" {
 #endif /* defined(__cplusplus) */
 
+struct tt_uuid;
+
 /**
 * Ask the allocator to reserve at least size bytes. It can reserve
 * more, and update *size with the new size.
@@ -140,6 +142,9 @@ mpstream_encode_binl(struct mpstream *stream, uint32_t len);
 void
 mpstream_encode_decimal(struct mpstream *stream, const decimal_t *val);
 
+void
+mpstream_encode_uuid(struct mpstream *stream, const struct tt_uuid *uuid);
+
 /** Copies n bytes from memory area src to stream. */
 void
 mpstream_memcpy(struct mpstream *stream, const void *src, uint32_t n);
diff --git a/src/lib/msgpuck b/src/lib/msgpuck
index 8ae606a16..0d273f95f 160000
--- a/src/lib/msgpuck
+++ b/src/lib/msgpuck
@@ -1 +1 @@
-Subproject commit 8ae606a1636dd89b2d61b154e5a1db03dce91657
+Subproject commit 0d273f95f8de64aeed698c354ca3bc7cb5ea461a
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index edbc15b72..fb49b1547 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -43,6 +43,7 @@
 
 #include "lua/decimal.h" /* lua_pushdecimal() */
 #include "lib/core/decimal.h" /* decimal_unpack() */
+#include "lib/core/mp_uuid.h" /* mp_decode_uuid() */
 #include "lib/core/mp_extension_types.h"
 
 #include <fiber.h>
@@ -114,7 +115,7 @@ luamp_encode_r(struct lua_State *L, struct luaL_serializer *cfg,
 	int top = lua_gettop(L);
 	enum mp_type type;
 
-restart: /* used by MP_EXT */
+restart: /* used by MP_EXT of unidentified subtype */
 	switch (field->type) {
 	case MP_UINT:
 		mpstream_encode_uint(stream, field->ival);
@@ -190,7 +191,10 @@ restart: /* used by MP_EXT */
 		switch (field->ext_type) {
 		case MP_DECIMAL:
 			mpstream_encode_decimal(stream, field->decval);
-			return MP_EXT;
+			break;
+		case MP_UUID:
+			mpstream_encode_uuid(stream, field->uuidval);
+			break;
 		default:
 			/* Run trigger if type can't be encoded */
 			type = luamp_encode_extension(L, top, stream);
@@ -310,10 +314,17 @@ luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
 		{
 			decimal_t *dec = lua_pushdecimal(L);
 			dec = decimal_unpack(data, len, dec);
-			if (dec == NULL) {
-				lua_pop(L, -1);
-				luaL_error(L, "msgpack.decode: invalid MsgPack");
-			}
+			if (dec == NULL)
+				goto ext_decode_err;
+			return;
+		}
+		case MP_UUID:
+		{
+			struct tt_uuid *uuid = luaL_pushuuid(L);
+			*data = svp;
+			uuid = mp_decode_uuid(data, uuid);
+			if (uuid == NULL)
+				goto ext_decode_err;
 			return;
 		}
 		default:
@@ -325,6 +336,10 @@ luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
 		break;
 	}
 	}
+return;
+ext_decode_err:
+	lua_pop(L, -1);
+	luaL_error(L, "msgpack.decode: invalid MsgPack");
 }
 
 
diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua
index f775f2d41..f01ffaef0 100644
--- a/src/lua/msgpackffi.lua
+++ b/src/lua/msgpackffi.lua
@@ -20,6 +20,10 @@ char *
 mp_encode_decimal(char *data, decimal_t *dec);
 uint32_t
 mp_sizeof_decimal(const decimal_t *dec);
+char *
+mp_encode_uuid(char *data, const struct tt_uuid *uuid);
+uint32_t
+mp_sizeof_uuid();
 float
 mp_decode_float(const char **data);
 double
@@ -28,6 +32,8 @@ uint32_t
 mp_decode_extl(const char **data, int8_t *type);
 decimal_t *
 decimal_unpack(const char **data, uint32_t len, decimal_t *dec);
+struct tt_uuid *
+uuid_unpack(const char **data, uint32_t len, struct tt_uuid *uuid);
 ]])
 
 local strict_alignment = (jit.arch == 'arm')
@@ -129,6 +135,11 @@ local function encode_decimal(buf, num)
     builtin.mp_encode_decimal(p, num)
 end
 
+local function encode_uuid(buf, uuid)
+    local p = buf:alloc(builtin.mp_sizeof_uuid())
+    builtin.mp_encode_uuid(p, uuid)
+end
+
 local function encode_int(buf, num)
     if num >= 0 then
         if num <= 0x7f then
@@ -311,6 +322,7 @@ on_encode(ffi.typeof('bool'), encode_bool_cdata)
 on_encode(ffi.typeof('float'), encode_float)
 on_encode(ffi.typeof('double'), encode_double)
 on_encode(ffi.typeof('decimal_t'), encode_decimal)
+on_encode(ffi.typeof('struct tt_uuid'), encode_uuid)
 
 --------------------------------------------------------------------------------
 -- Decoder
@@ -495,6 +507,8 @@ local ext_decoder = {
     [0] = function(data, len) error("unsupported extension type") end,
     -- MP_DECIMAL
     [1] = function(data, len) local num = ffi.new("decimal_t") builtin.decimal_unpack(data, len, num) return num end,
+    -- MP_UUID
+    [2] = function(data, len) local uuid = ffi.new("struct tt_uuid") builtin.uuid_unpack(data, len, uuid) return uuid end,
 }
 
 local function decode_ext(data)
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 54d18ac89..82f092cbf 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -45,9 +45,9 @@ static uint32_t CTID_STRUCT_IBUF;
 static uint32_t CTID_STRUCT_IBUF_PTR;
 static uint32_t CTID_CHAR_PTR;
 static uint32_t CTID_CONST_CHAR_PTR;
+static uint32_t CTID_UUID;
 uint32_t CTID_DECIMAL;
 
-
 void *
 luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
 {
@@ -101,6 +101,12 @@ luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
 	return cdataptr(cd);
 }
 
+struct tt_uuid *
+luaL_pushuuid(struct lua_State *L)
+{
+	return luaL_pushcdata(L, CTID_UUID);
+}
+
 void *
 luaL_checkcdata(struct lua_State *L, int idx, uint32_t *ctypeid)
 {
@@ -746,6 +752,9 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg, int index,
 			if (cd->ctypeid == CTID_DECIMAL) {
 				field->ext_type = MP_DECIMAL;
 				field->decval = (decimal_t *) cdata;
+			} else if (cd->ctypeid == CTID_UUID) {
+				field->ext_type = MP_UUID;
+				field->uuidval = (struct tt_uuid *) cdata;
 			} else {
 				field->ext_type = MP_UNKNOWN_EXTENSION;
 			}
@@ -1286,5 +1295,15 @@ tarantool_lua_utils_init(struct lua_State *L)
 	assert(CTID_CHAR_PTR != 0);
 	CTID_CONST_CHAR_PTR = luaL_ctypeid(L, "const char *");
 	assert(CTID_CONST_CHAR_PTR != 0);
+	rc = luaL_cdef(L, "struct tt_uuid {"
+				  "uint32_t time_low;"
+				  "uint16_t time_mid;"
+				  "uint16_t time_hi_and_version;"
+				  "uint8_t clock_seq_hi_and_reserved;"
+				  "uint8_t clock_seq_low;"
+				  "uint8_t node[6];"
+			  "};");
+	CTID_UUID = luaL_ctypeid(L, "struct tt_uuid");
+	assert(CTID_UUID != 0);
 	return 0;
 }
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 0b3672769..4bc041796 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -61,6 +61,7 @@ extern "C" {
 
 struct lua_State;
 struct ibuf;
+struct tt_uuid;
 
 /**
  * Single global lua_State shared by core and modules.
@@ -71,6 +72,9 @@ struct ibuf;
 extern struct lua_State *tarantool_L;
 extern struct ibuf *tarantool_lua_ibuf;
 
+struct tt_uuid *
+luaL_pushuuid(struct lua_State *L);
+
 /** \cond public */
 
 /**
@@ -320,6 +324,7 @@ struct luaL_field {
 		/* Array or map. */
 		uint32_t size;
 		decimal_t *decval;
+		struct tt_uuid *uuidval;
 	};
 	enum mp_type type;
 	/* subtypes of MP_EXT */
diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua
index f8418cf4d..5d19a4408 100644
--- a/src/lua/uuid.lua
+++ b/src/lua/uuid.lua
@@ -5,15 +5,6 @@ local static_alloc = require('buffer').static_alloc
 local builtin = ffi.C
 
 ffi.cdef[[
-struct tt_uuid {
-    uint32_t time_low;
-    uint16_t time_mid;
-    uint16_t time_hi_and_version;
-    uint8_t clock_seq_hi_and_reserved;
-    uint8_t clock_seq_low;
-    uint8_t node[6];
-};
-
 void
 tt_uuid_create(struct tt_uuid *uu);
 int
diff --git a/test/app-tap/lua/serializer_test.lua b/test/app-tap/lua/serializer_test.lua
index 47edac621..2a668f898 100644
--- a/test/app-tap/lua/serializer_test.lua
+++ b/test/app-tap/lua/serializer_test.lua
@@ -204,6 +204,13 @@ local function test_decimal(test, s)
     rt(test, s, decimal.new('-1234567891234567890.0987654321987654321'), 'cdata')
 end
 
+local function test_uuid(test, s)
+    local uuid = require('uuid')
+    test:plan(2)
+
+    rt(test, s, uuid.new(), 'cdata')
+end
+
 local function test_boolean(test, s)
     test:plan(4)
 
@@ -505,6 +512,7 @@ return {
     test_table = test_table;
     test_ucdata = test_ucdata;
     test_decimal = test_decimal;
+    test_uuid = test_uuid;
     test_depth = test_depth;
     test_decode_buffer = test_decode_buffer;
 }
diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua
index 994402d15..0ee5f5edc 100755
--- a/test/app-tap/msgpackffi.test.lua
+++ b/test/app-tap/msgpackffi.test.lua
@@ -118,11 +118,12 @@ end
 
 tap.test("msgpackffi", function(test)
     local serializer = require('msgpackffi')
-    test:plan(11)
+    test:plan(12)
     test:test("unsigned", common.test_unsigned, serializer)
     test:test("signed", common.test_signed, serializer)
     test:test("double", common.test_double, serializer)
     test:test("decimal", common.test_decimal, serializer)
+    test:test("uuid", common.test_uuid, serializer)
     test:test("boolean", common.test_boolean, serializer)
     test:test("string", common.test_string, serializer)
     test:test("nil", common.test_nil, serializer)
diff --git a/test/app/msgpack.result b/test/app/msgpack.result
index 4b5aec784..ddf06fc9d 100644
--- a/test/app/msgpack.result
+++ b/test/app/msgpack.result
@@ -293,3 +293,24 @@ msgpack.decode(msgpack.encode(e)) == e
 ---
 - true
 ...
+--
+-- gh-4268: msgpack encode/decode UUID
+--
+uuid = require('uuid')
+---
+...
+fail = nil
+---
+...
+for i = 1,10 do\
+    local a = uuid.new()\
+    if msgpack.decode(msgpack.encode(a)) ~= a then\
+        fail = a\
+    end\
+end
+---
+...
+fail
+---
+- null
+...
diff --git a/test/app/msgpack.test.lua b/test/app/msgpack.test.lua
index 9224d870a..17e05df5c 100644
--- a/test/app/msgpack.test.lua
+++ b/test/app/msgpack.test.lua
@@ -99,3 +99,16 @@ msgpack.decode(msgpack.encode(b)) == b
 msgpack.decode(msgpack.encode(c)) == c
 msgpack.decode(msgpack.encode(d)) == d
 msgpack.decode(msgpack.encode(e)) == e
+
+--
+-- gh-4268: msgpack encode/decode UUID
+--
+uuid = require('uuid')
+fail = nil
+for i = 1,10 do\
+    local a = uuid.new()\
+    if msgpack.decode(msgpack.encode(a)) ~= a then\
+        fail = a\
+    end\
+end
+fail
diff --git a/test/app/uuid.result b/test/app/uuid.result
index 0713614c6..013c51282 100644
--- a/test/app/uuid.result
+++ b/test/app/uuid.result
@@ -106,7 +106,7 @@ uu.node[5]
 -- invalid values
 uuid.fromstr(nil)
 ---
-- error: 'builtin/uuid.lua:47: fromstr(str)'
+- error: 'builtin/uuid.lua:38: fromstr(str)'
 ...
 uuid.fromstr('')
 ---
diff --git a/test/box/tuple.result b/test/box/tuple.result
index a499aa43a..eb60a5645 100644
--- a/test/box/tuple.result
+++ b/test/box/tuple.result
@@ -1490,6 +1490,87 @@ box.tuple.is(box.tuple.new({1}))
 ---
 - true
 ...
+--
+-- gh-4268 UUID in tuple
+--
+uuid = require("uuid")
+---
+...
+-- Fixed randomly generated uuids to avoid problems with test
+-- output comparison.
+a = uuid.fromstr("c8f0fa1f-da29-438c-a040-393f1126ad39")
+---
+...
+b = uuid.fromstr("83eb4959-3de6-49fb-8890-6fb4423dd186")
+---
+...
+t = box.tuple.new(a, 2, b, "string")
+---
+...
+state, val = t:next()
+---
+...
+state
+---
+- 1
+...
+val == a
+---
+- true
+...
+state, val = t:next(state)
+---
+...
+state
+---
+- 2
+...
+val
+---
+- 2
+...
+state, val = t:next(state)
+---
+...
+state
+---
+- 3
+...
+val == b
+---
+- true
+...
+t:slice(1)
+---
+- 2
+- 83eb4959-3de6-49fb-8890-6fb4423dd186
+- string
+...
+t:slice(-1)
+---
+- string
+...
+t:slice(-2)
+---
+- 83eb4959-3de6-49fb-8890-6fb4423dd186
+- string
+...
+msgpack.decode(msgpack.encode(t))
+---
+- [c8f0fa1f-da29-438c-a040-393f1126ad39, 2, 83eb4959-3de6-49fb-8890-6fb4423dd186,
+  'string']
+- 46
+...
+msgpackffi.decode(msgpackffi.encode(t))
+---
+- [c8f0fa1f-da29-438c-a040-393f1126ad39, 2, 83eb4959-3de6-49fb-8890-6fb4423dd186,
+  'string']
+- 46
+...
+t:bsize()
+---
+- 45
+...
 msgpack.cfg({encode_max_depth = max_depth, encode_deep_as_nil = deep_as_nil})
 ---
 ...
diff --git a/test/box/tuple.test.lua b/test/box/tuple.test.lua
index b83fca5cd..4201e9860 100644
--- a/test/box/tuple.test.lua
+++ b/test/box/tuple.test.lua
@@ -510,4 +510,29 @@ box.tuple.is('1')
 box.tuple.is(box.tuple.new())
 box.tuple.is(box.tuple.new({1}))
 
+--
+-- gh-4268 UUID in tuple
+--
+uuid = require("uuid")
+-- Fixed randomly generated uuids to avoid problems with test
+-- output comparison.
+a = uuid.fromstr("c8f0fa1f-da29-438c-a040-393f1126ad39")
+b = uuid.fromstr("83eb4959-3de6-49fb-8890-6fb4423dd186")
+t = box.tuple.new(a, 2, b, "string")
+state, val = t:next()
+state
+val == a
+state, val = t:next(state)
+state
+val
+state, val = t:next(state)
+state
+val == b
+t:slice(1)
+t:slice(-1)
+t:slice(-2)
+msgpack.decode(msgpack.encode(t))
+msgpackffi.decode(msgpackffi.encode(t))
+t:bsize()
+
 msgpack.cfg({encode_max_depth = max_depth, encode_deep_as_nil = deep_as_nil})
diff --git a/test/unit/uuid.c b/test/unit/uuid.c
index c43d93b4f..4f6b963ba 100644
--- a/test/unit/uuid.c
+++ b/test/unit/uuid.c
@@ -1,5 +1,7 @@
 #include "unit.h"
 #include "uuid/tt_uuid.h"
+#include "core/mp_uuid.h"
+#include "core/random.h"
 #include <string.h>
 
 static void
@@ -27,10 +29,28 @@ uuid_test(struct tt_uuid a, struct tt_uuid b, int expected_result)
            "%s %s %s", a_str, sign, b_str);
 }
 
+static void
+mp_uuid_test()
+{
+        plan(4);
+        char buf[18];
+        char *data = buf;
+        const char *data1 = buf;
+        struct tt_uuid uu, ret;
+        random_init();
+        tt_uuid_create(&uu);
+        char *end = mp_encode_uuid(data, &uu);
+        is(end - data, mp_sizeof_uuid(), "mp_sizeof_uuid() == encoded length");
+        struct tt_uuid *rc = mp_decode_uuid(&data1, &ret);
+        is(rc, &ret, "mp_decode_uuid() return code");
+        is(data1, end, "mp_sizeof_uuid() == decoded length");
+        is(tt_uuid_compare(&uu, &ret), 0, "mp_decode_uuid(mp_encode_uuid(uu)) == uu");
+}
+
 int
 main(void)
 {
-        plan(2);
+        plan(3);
 
         uuid_test(
                 (struct tt_uuid){.time_low = 1712399963,
@@ -63,5 +83,7 @@ main(void)
                                 .node = "v\025Oo9I"},
                 -1);
 
+        mp_uuid_test();
+
         return check_plan();
 }
diff --git a/test/unit/uuid.result b/test/unit/uuid.result
index 40ccce759..50a1140c5 100644
--- a/test/unit/uuid.result
+++ b/test/unit/uuid.result
@@ -1,3 +1,9 @@
-1..2
+1..3
 ok 1 - 6611265b-8852-4832-af8b-4164d52c62eb > 186ebbf7-cf97-4e2e-8b1b-76154f6f3949
 ok 2 - 075b4148-8fb0-2e7f-af50-4164d52c62eb < 1fbc929f-5da8-28c5-8b36-76154f6f3949
+    1..4
+    ok 1 - mp_sizeof_uuid() == encoded length
+    ok 2 - mp_decode_uuid() return code
+    ok 3 - mp_sizeof_uuid() == decoded length
+    ok 4 - mp_decode_uuid(mp_encode_uuid(uu)) == uu
+ok 3 - subtests
diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index 26e566a6f..6e1793a59 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -48,6 +48,9 @@
 #include "strbuf.h"
 
 #include "lua/utils.h"
+#include "lib/core/mp_extension_types.h" /* MP_DECIMAL, MP_UUID */
+#include "lib/core/tt_static.h"
+#include "lib/uuid/tt_uuid.h" /* tt_uuid_to_string(), UUID_STR_LEN */
 
 #define DEFAULT_ENCODE_KEEP_BUFFER 1
 
@@ -421,15 +424,21 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg,
     json_append_array(l, cfg, current_depth + 1, json, field.size);
     return;
     case MP_EXT:
-	switch (field.ext_type) {
-	case MP_DECIMAL:
-	{
-	    const char *str = decimal_to_string(field.decval);
-	    return json_append_string(cfg, json, str, strlen(str));
-	}
-	default:
-	    assert(false);
-	}
+        switch (field.ext_type) {
+        case MP_DECIMAL:
+        {
+            const char *str = decimal_to_string(field.decval);
+            return json_append_string(cfg, json, str, strlen(str));
+        }
+        case MP_UUID:
+        {
+            char *str = tt_static_buf();
+            tt_uuid_to_string(field.uuidval, str);
+            return json_append_string(cfg, json, str, UUID_STR_LEN);
+        }
+        default:
+            assert(false);
+        }
     }
 }
 
diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
index af4f2f5d5..411c56f71 100644
--- a/third_party/lua-yaml/lyaml.cc
+++ b/third_party/lua-yaml/lyaml.cc
@@ -50,6 +50,9 @@ extern "C" {
 } /* extern "C" */
 #include "lua/utils.h"
 #include "lib/core/decimal.h"
+#include "lib/core/tt_static.h"
+#include "lib/core/mp_extension_types.h" /* MP_DECIMAL, MP_UUID */
+#include "lib/uuid/tt_uuid.h" /* tt_uuid_to_string(), UUID_STR_LEN */
 
 #define LUAYAML_TAG_PREFIX "tag:yaml.org,2002:"
 
@@ -697,10 +700,18 @@ static int dump_node(struct lua_yaml_dumper *dumper)
       switch (field.ext_type) {
       case MP_DECIMAL:
          str = decimal_to_string(field.decval);
-	 len = strlen(str);
-	 break;
+         len = strlen(str);
+         break;
+      case MP_UUID:
+      {
+         char *buf = tt_static_buf();
+         tt_uuid_to_string(field.uuidval, buf);
+         str = buf;
+         len = UUID_STR_LEN;
+         break;
+      }
       default:
-	 assert(0); /* checked by luaL_checkfield() */
+         assert(0); /* checked by luaL_checkfield() */
       }
       break;
     }
-- 
2.21.1 (Apple Git-122.3)

[Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID

[Serge Petrenko]

It is now possible to create an index over UUID values, either returned
by something like `uuid.new()` or 36- and 32- byte strings in format
"88ebbb9a-8c18-480f-bd04-dd5345f1573c" or "88ebbb9a8c18480fbd04dd5345f1573c".
16-byte binary strings are also supported.

Closes #4268
Closes #2916

@TarantoolBot document
Title: Document uuid field type.

There's a new field type -- UUID, it accepts values returned by
`uuid.new()`, as well as strings representing uuids, like
`88ebbb9a-8c18-480f-bd04-dd5345f1573c` and
`88ebbb9a8c18480fbd04dd5345f1573c` and 16 byte binary values, encoded in
msgpack.

The index may be either unique or non-unique, nullable or non-nullable,
and may be a primary key.

To create an index over a uuid field for space `test`, say:
```
box.space.test:create_index("pk", {parts={1, 'uuid'}})
```
Now you may insert uuids into the space:
```
tarantool> box.space.test:insert{uuid.new()}
---
- [e631fdcc-0e8a-4d2f-83fd-b0ce6762b13f]
...

tarantool> box.space.test:insert{"64d22e4d-ac92-4a23-899a-e59f34af5479"}
---
- ['64d22e4d-ac92-4a23-899a-e59f34af5479']
...

tarantool> box.space.test:insert{"856dc1177dcc49969f1407f8c6c8a371"}
---
- ['856dc1177dcc49969f1407f8c6c8a371']
...

tarantool> box.space.test:select{}
---
- - ['64d22e4d-ac92-4a23-899a-e59f34af5479']
  - ['856dc1177dcc49969f1407f8c6c8a371']
  - [e631fdcc-0e8a-4d2f-83fd-b0ce6762b13f]
...

```
---
 src/box/field_def.c               |  66 ++++++++++++----
 src/box/field_def.h               |  16 ++++
 src/box/key_def.h                 |   3 +-
 src/box/tuple_compare.cc          | 123 ++++++++++++++++++++++++++++++
 src/box/tuple_format.c            |   3 +-
 test/engine/ddl.result            |  97 ++++++++++++++++++++++-
 test/engine/ddl.test.lua          |  42 +++++++++-
 test/engine/gh-4268-uuid.result   |  58 ++++++++++++++
 test/engine/gh-4268-uuid.test.lua |  30 ++++++++
 9 files changed, 421 insertions(+), 17 deletions(-)
 create mode 100644 test/engine/gh-4268-uuid.result
 create mode 100644 test/engine/gh-4268-uuid.test.lua

diff --git a/src/box/field_def.c b/src/box/field_def.c
index fde4e5a00..c03f26a47 100644
--- a/src/box/field_def.c
+++ b/src/box/field_def.c
@@ -33,6 +33,8 @@
 #include "trivia/util.h"
 #include "key_def.h"
 #include "mp_extension_types.h"
+#include "mp_uuid.h"
+#include "uuid/tt_uuid.h"
 
 const char *mp_type_strs[] = {
 	/* .MP_NIL    = */ "nil",
@@ -67,6 +69,7 @@ const uint32_t field_mp_type[] = {
 		(1U << MP_FLOAT) | (1U << MP_DOUBLE) | (1U << MP_STR) |
 		(1U << MP_BIN) | (1U << MP_BOOL),
 	/* [FIELD_TYPE_DECIMAL]  =  */ 0, /* only MP_DECIMAL is supported */
+	/* [FIELD_TYPE_UUID]     =  */ (1U << MP_STR) | (1U << MP_BIN),
 	/* [FIELD_TYPE_ARRAY]    =  */ 1U << MP_ARRAY,
 	/* [FIELD_TYPE_MAP]      =  */ (1U << MP_MAP),
 };
@@ -82,6 +85,7 @@ const uint32_t field_ext_type[] = {
 	/* [FIELD_TYPE_VARBINARY] = */ 0,
 	/* [FIELD_TYPE_SCALAR]    = */ 1U << MP_DECIMAL,
 	/* [FIELD_TYPE_DECIMAL]   = */ 1U << MP_DECIMAL,
+	/* [FIELD_TYPE_UUID]      = */ 1U << MP_UUID,
 	/* [FIELD_TYPE_ARRAY]     = */ 0,
 	/* [FIELD_TYPE_MAP]       = */ 0,
 };
@@ -97,6 +101,7 @@ const char *field_type_strs[] = {
 	/* [FIELD_TYPE_VARBINARY] = */"varbinary",
 	/* [FIELD_TYPE_SCALAR]   = */ "scalar",
 	/* [FIELD_TYPE_DECIMAL]  = */ "decimal",
+	/* [FIELD_TYPE_UUID]     = */ "uuid",
 	/* [FIELD_TYPE_ARRAY]    = */ "array",
 	/* [FIELD_TYPE_MAP]      = */ "map",
 };
@@ -123,19 +128,20 @@ field_type_by_name_wrapper(const char *str, uint32_t len)
  * values can be stored in the j type.
  */
 static const bool field_type_compatibility[] = {
-	   /*   ANY   UNSIGNED  STRING   NUMBER  DOUBLE  INTEGER  BOOLEAN VARBINARY SCALAR  DECIMAL  ARRAY    MAP  */
-/*   ANY    */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  false,   false,
-/* UNSIGNED */ true,   true,    false,   true,    false,   true,    false,   false,  true,   false,  false,   false,
-/*  STRING  */ true,   false,   true,    false,   false,   false,   false,   false,  true,   false,  false,   false,
-/*  NUMBER  */ true,   false,   false,   true,    false,   false,   false,   false,  true,   false,  false,   false,
-/*  DOUBLE  */ true,   false,   false,   true,    true,    false,   false,   false,  true,   false,  false,   false,
-/*  INTEGER */ true,   false,   false,   true,    false,   true,    false,   false,  true,   false,  false,   false,
-/*  BOOLEAN */ true,   false,   false,   false,   false,   false,   true,    false,  true,   false,  false,   false,
-/* VARBINARY*/ true,   false,   false,   false,   false,   false,   false,   true,   true,   false,  false,   false,
-/*  SCALAR  */ true,   false,   false,   false,   false,   false,   false,   false,  true,   false,  false,   false,
-/*  DECIMAL */ true,   false,   false,   true,    false,   false,   false,   false,  true,   true,   false,   false,
-/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  true,    false,
-/*    MAP   */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  false,   true,
+	   /*   ANY   UNSIGNED  STRING   NUMBER  DOUBLE  INTEGER  BOOLEAN VARBINARY SCALAR  DECIMAL   UUID    ARRAY    MAP  */
+/*   ANY    */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  false,   false,   false,
+/* UNSIGNED */ true,   true,    false,   true,    false,   true,    false,   false,  true,   false,  false,   false,   false,
+/*  STRING  */ true,   false,   true,    false,   false,   false,   false,   false,  true,   false,  false,   false,   false,
+/*  NUMBER  */ true,   false,   false,   true,    false,   false,   false,   false,  true,   false,  false,   false,   false,
+/*  DOUBLE  */ true,   false,   false,   true,    true,    false,   false,   false,  true,   false,  false,   false,   false,
+/*  INTEGER */ true,   false,   false,   true,    false,   true,    false,   false,  true,   false,  false,   false,   false,
+/*  BOOLEAN */ true,   false,   false,   false,   false,   false,   true,    false,  true,   false,  false,   false,   false,
+/* VARBINARY*/ true,   false,   false,   false,   false,   false,   false,   true,   true,   false,  false,   false,   false,
+/*  SCALAR  */ true,   false,   false,   false,   false,   false,   false,   false,  true,   false,  false,   false,   false,
+/*  DECIMAL */ true,   false,   false,   true,    false,   false,   false,   false,  true,   true,   false,   false,   false,
+/*   UUID   */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  true,    false,   false,
+/*   ARRAY  */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  false,   true,    false,
+/*    MAP   */ true,   false,   false,   false,   false,   false,   false,   false,  false,  false,  false,   false,   true,
 };
 
 bool
@@ -183,3 +189,37 @@ field_type_by_name(const char *name, size_t len)
 		return FIELD_TYPE_ANY;
 	return field_type_MAX;
 }
+
+/*
+ * UUID field only accepts Msgpack-encoded UUID,
+ * 36 byte strings in format 0a998917-06f4-4b66-ad12-22c701dc8c91,
+ * 32 byte hex strings and 16 byte binstrings, containing valid
+ * UUIDs.
+ */
+bool
+field_uuid_value_is_compatible(const char *data)
+{
+	const char *str;
+	uint32_t len;
+	struct tt_uuid uuid;
+	switch (mp_typeof(*data)) {
+	case MP_STR:
+		str = mp_decode_str(&data, &len);
+		return tt_uuid_from_lstring(str, len, &uuid) == 0;
+	case MP_BIN:
+		str = mp_decode_bin(&data, &len);
+		return uuid_unpack(&str, len, &uuid) != NULL;
+	case MP_EXT:
+		/*
+		 * Only MP_UUID is allowed. All values are
+		 * correct.
+		 */
+		return true;
+	default:
+		/*
+		 * Must be checked earlier in
+		 * field_mp_type_is_compatible()
+		 */
+		unreachable();
+	}
+}
diff --git a/src/box/field_def.h b/src/box/field_def.h
index 8e82369f1..94bfbf5f8 100644
--- a/src/box/field_def.h
+++ b/src/box/field_def.h
@@ -60,6 +60,7 @@ enum field_type {
 	FIELD_TYPE_VARBINARY,
 	FIELD_TYPE_SCALAR,
 	FIELD_TYPE_DECIMAL,
+	FIELD_TYPE_UUID,
 	FIELD_TYPE_ARRAY,
 	FIELD_TYPE_MAP,
 	field_type_MAX
@@ -182,6 +183,21 @@ field_mp_type_is_compatible(enum field_type type, const char *data,
 	}
 }
 
+bool
+field_uuid_value_is_compatible(const char *data);
+
+/** Check whether a value may be stored in field of given type. */
+static inline bool
+field_value_is_compatible(enum field_type type, const char *data)
+{
+	switch (type) {
+	case FIELD_TYPE_UUID:
+		return field_uuid_value_is_compatible(data);
+	default:
+		return true;
+	}
+}
+
 static inline bool
 action_is_nullable(enum on_conflict_action nullable_action)
 {
diff --git a/src/box/key_def.h b/src/box/key_def.h
index f4d9e76f2..6a84f1885 100644
--- a/src/box/key_def.h
+++ b/src/box/key_def.h
@@ -528,7 +528,8 @@ static inline int
 key_part_validate(enum field_type key_type, const char *key,
 		  uint32_t field_no, bool is_nullable)
 {
-	if (unlikely(!field_mp_type_is_compatible(key_type, key, is_nullable))) {
+	if (unlikely(!field_mp_type_is_compatible(key_type, key, is_nullable) ||
+		     !field_value_is_compatible(key_type, key))) {
 		diag_set(ClientError, ER_KEY_PART_TYPE, field_no,
 			 field_type_strs[key_type]);
 		return -1;
diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
index 3f8a0ce24..aad69340d 100644
--- a/src/box/tuple_compare.cc
+++ b/src/box/tuple_compare.cc
@@ -35,6 +35,7 @@
 #include <math.h>
 #include "lib/core/decimal.h"
 #include "lib/core/mp_decimal.h"
+#include "lib/core/mp_uuid.h"
 #include "lib/core/mp_extension_types.h"
 
 /* {{{ tuple_compare */
@@ -74,6 +75,7 @@ enum mp_class {
 	MP_CLASS_NUMBER,
 	MP_CLASS_STR,
 	MP_CLASS_BIN,
+	MP_CLASS_UUID,
 	MP_CLASS_ARRAY,
 	MP_CLASS_MAP,
 	mp_class_max,
@@ -96,6 +98,7 @@ static enum mp_class mp_classes[] = {
 static enum mp_class mp_ext_classes[] = {
 	/* .MP_UNKNOWN_EXTENSION = */ mp_class_max, /* unsupported */
 	/* .MP_DECIMAL		 = */ MP_CLASS_NUMBER,
+	/* .MP_UUID		 = */ MP_CLASS_UUID,
 };
 
 static enum mp_class
@@ -110,6 +113,7 @@ mp_extension_class(const char *data)
 	assert(mp_typeof(*data) == MP_EXT);
 	int8_t type;
 	mp_decode_extl(&data, &type);
+	assert(type >= 0 && type < mp_extension_type_MAX);
 	return mp_ext_classes[type];
 }
 
@@ -378,6 +382,61 @@ mp_compare_bin(const char *field_a, const char *field_b)
 	return COMPARE_RESULT(size_a, size_b);
 }
 
+static int
+mp_compare_uuid_with_type(const char *field_a, enum mp_type type_a,
+			  const char *field_b, enum mp_type type_b)
+{
+	struct tt_uuid uuid_a, uuid_b;
+	struct tt_uuid *ret;
+	const char *str;
+	uint32_t len;
+	int rc;
+	switch (type_a) {
+	case MP_STR:
+		str = mp_decode_str(&field_a, &len);
+		rc = tt_uuid_from_lstring(str, len, &uuid_a);
+		assert(rc == 0);
+		break;
+	case MP_BIN:
+		str = mp_decode_bin(&field_a, &len);
+		ret = uuid_unpack(&str, len, &uuid_a);
+		assert(ret != NULL);
+		break;
+	case MP_EXT:
+		ret = mp_decode_uuid(&field_a, &uuid_a);
+		assert(ret != NULL);
+		break;
+	default:
+		unreachable();
+	}
+	switch (type_b) {
+	case MP_STR:
+		str = mp_decode_str(&field_b, &len);
+		rc = tt_uuid_from_lstring(str, len, &uuid_b);
+		assert(rc == 0);
+		break;
+	case MP_BIN:
+		str = mp_decode_bin(&field_b, &len);
+		ret = uuid_unpack(&str, len, &uuid_b);
+		assert(ret != NULL);
+		break;
+	case MP_EXT:
+		ret = mp_decode_uuid(&field_b, &uuid_b);
+		assert(ret != NULL);
+		break;
+	default:
+		unreachable();
+	}
+	return tt_uuid_compare(&uuid_a, &uuid_b);
+}
+
+static inline int
+mp_compare_uuid(const char *field_a, const char *field_b)
+{
+	return mp_compare_uuid_with_type(field_a, mp_typeof(*field_a),
+					 field_b, mp_typeof(*field_b));
+}
+
 typedef int (*mp_compare_f)(const char *, const char *);
 static mp_compare_f mp_class_comparators[] = {
 	/* .MP_CLASS_NIL    = */ NULL,
@@ -463,6 +522,8 @@ tuple_compare_field(const char *field_a, const char *field_b,
 		       mp_compare_scalar(field_a, field_b);
 	case FIELD_TYPE_DECIMAL:
 		return mp_compare_decimal(field_a, field_b);
+	case FIELD_TYPE_UUID:
+		return mp_compare_uuid(field_a, field_b);
 	default:
 		unreachable();
 		return 0;
@@ -501,6 +562,9 @@ tuple_compare_field_with_type(const char *field_a, enum mp_type a_type,
 	case FIELD_TYPE_DECIMAL:
 		return mp_compare_number_with_type(field_a, a_type,
 						   field_b, b_type);
+	case FIELD_TYPE_UUID:
+		return mp_compare_uuid_with_type(field_a, a_type,
+						 field_b, b_type);
 	default:
 		unreachable();
 		return 0;
@@ -1578,6 +1642,21 @@ hint_decimal(decimal_t *dec)
 	return hint_create(MP_CLASS_NUMBER, val);
 }
 
+static inline hint_t
+hint_uuid(struct tt_uuid *uuid)
+{
+	/* Simply take the first part of the UUID as hint. */
+	uint64_t val = 0;
+	val |= uuid->time_low;
+	val <<= sizeof(uuid->time_mid) * CHAR_BIT;
+	val |= uuid->time_mid;
+	val <<= sizeof(uuid->time_hi_and_version) * CHAR_BIT;
+	val |= uuid->time_hi_and_version;
+	/* Make space for class representation. */
+	val >>= HINT_CLASS_BITS;
+	return hint_create(MP_CLASS_UUID, val);
+}
+
 static inline uint64_t
 hint_str_raw(const char *s, uint32_t len)
 {
@@ -1698,6 +1777,45 @@ field_hint_decimal(const char *field)
 	}
 }
 
+static inline hint_t
+field_hint_uuid(const char *field)
+{
+	const char *str;
+	uint32_t len;
+	struct tt_uuid uuid;
+	struct tt_uuid *ret;
+	switch(mp_typeof(*field)) {
+	case MP_STR:
+	{
+		str = mp_decode_str(&field, &len);
+		int rc = tt_uuid_from_lstring(str, len, &uuid);
+		assert(rc == 0);
+		(void) rc;
+		break;
+	}
+	case MP_BIN:
+		str = mp_decode_bin(&field, &len);
+		/*
+		 * Binary UUID representation shares the same
+		 * format with UUID data after MP_EXT header.
+		 */
+		ret = uuid_unpack(&str, len, &uuid);
+		assert(ret != NULL);
+		break;
+	case MP_EXT:
+		/*
+		 * Must be a UUID. Otherwise mp_decode_uuid() will
+		 * return NULL.
+		 */
+		ret = mp_decode_uuid(&field, &uuid);
+		assert(ret != NULL);
+		break;
+	default:
+		unreachable();
+	}
+	return hint_uuid(&uuid);
+}
+
 static inline hint_t
 field_hint_string(const char *field, struct coll *coll)
 {
@@ -1782,6 +1900,8 @@ field_hint(const char *field, struct coll *coll)
 		return field_hint_scalar(field, coll);
 	case FIELD_TYPE_DECIMAL:
 		return field_hint_decimal(field);
+	case FIELD_TYPE_UUID:
+		return field_hint_uuid(field);
 	default:
 		unreachable();
 	}
@@ -1893,6 +2013,9 @@ key_def_set_hint_func(struct key_def *def)
 	case FIELD_TYPE_DECIMAL:
 		key_def_set_hint_func<FIELD_TYPE_DECIMAL>(def);
 		break;
+	case FIELD_TYPE_UUID:
+		key_def_set_hint_func<FIELD_TYPE_UUID>(def);
+		break;
 	default:
 		/* Invalid key definition. */
 		def->key_hint = NULL;
diff --git a/src/box/tuple_format.c b/src/box/tuple_format.c
index 312c96621..b8595d579 100644
--- a/src/box/tuple_format.c
+++ b/src/box/tuple_format.c
@@ -1170,7 +1170,8 @@ tuple_format_iterator_next(struct tuple_format_iterator *it,
 	 * defined in format.
 	 */
 	bool is_nullable = tuple_field_is_nullable(field);
-	if (!field_mp_type_is_compatible(field->type, entry->data, is_nullable) != 0) {
+	if (!field_mp_type_is_compatible(field->type, entry->data, is_nullable) != 0 ||
+	    !field_value_is_compatible(field->type, entry->data)) {
 		diag_set(ClientError, ER_FIELD_TYPE,
 			 tuple_field_path(field),
 			 field_type_strs[field->type]);
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index 67b22ed9e..b7c04aafe 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -1037,6 +1037,9 @@ s:drop()
 decimal = require('decimal')
 ---
 ...
+uuid = require('uuid')
+---
+...
 -- Ensure that vinyl correctly process field count change.
 s = box.schema.space.create('test', {engine = engine, field_count = 2})
 ---
@@ -1098,13 +1101,16 @@ format[10] = {name = 'field10', type = 'map'}
 format[11] = {name = 'field11', type = 'decimal'}
 ---
 ...
+format[12] = {name = 'field12', type = 'uuid'}
+---
+...
 s = box.schema.space.create('test', {engine = engine, format = format})
 ---
 ...
 pk = s:create_index('pk')
 ---
 ...
-t = s:replace{1, {2}, 3, '4', 5.5, -6, true, -8, {9, 9}, {val = 10}, decimal.new(-11.11)}
+t = s:replace{1, {2}, 3, '4', 5.5, -6, true, -8, {9, 9}, {val = 10}, decimal.new(-11.11), uuid.new()}
 ---
 ...
 inspector:cmd("setopt delimiter ';'")
@@ -1171,6 +1177,11 @@ fail_format_change(3, 'decimal')
 ---
 - 'Tuple field 3 type does not match one required by operation: expected decimal'
 ...
+-- unsigned --X--> uuid
+fail_format_change(3, 'uuid')
+---
+- 'Tuple field 3 type does not match one required by operation: expected uuid'
+...
 -- string -----> any
 ok_format_change(4, 'any')
 ---
@@ -1189,6 +1200,11 @@ fail_format_change(4, 'decimal')
 ---
 - 'Tuple field 4 type does not match one required by operation: expected decimal'
 ...
+-- string --X--> uuid
+fail_format_change(4, 'uuid')
+---
+- 'Tuple field 4 type does not match one required by operation: expected uuid'
+...
 -- number -----> any
 ok_format_change(5, 'any')
 ---
@@ -1207,6 +1223,11 @@ fail_format_change(5, 'decimal')
 ---
 - 'Tuple field 5 type does not match one required by operation: expected decimal'
 ...
+-- number --X--> uuid
+fail_format_change(5, 'uuid')
+---
+- 'Tuple field 5 type does not match one required by operation: expected uuid'
+...
 -- integer -----> any
 ok_format_change(6, 'any')
 ---
@@ -1229,6 +1250,11 @@ fail_format_change(6, 'decimal')
 ---
 - 'Tuple field 6 type does not match one required by operation: expected decimal'
 ...
+-- integer --X--> uuid
+fail_format_change(6, 'uuid')
+---
+- 'Tuple field 6 type does not match one required by operation: expected uuid'
+...
 -- boolean -----> any
 ok_format_change(7, 'any')
 ---
@@ -1247,6 +1273,11 @@ fail_format_change(7, 'decimal')
 ---
 - 'Tuple field 7 type does not match one required by operation: expected decimal'
 ...
+-- boolean --X--> uuid
+fail_format_change(7, 'uuid')
+---
+- 'Tuple field 7 type does not match one required by operation: expected uuid'
+...
 -- scalar -----> any
 ok_format_change(8, 'any')
 ---
@@ -1261,6 +1292,11 @@ fail_format_change(8, 'decimal')
 ---
 - 'Tuple field 8 type does not match one required by operation: expected decimal'
 ...
+-- scalar --X--> uuid
+fail_format_change(8, 'uuid')
+---
+- 'Tuple field 8 type does not match one required by operation: expected uuid'
+...
 -- array -----> any
 ok_format_change(9, 'any')
 ---
@@ -1275,6 +1311,11 @@ fail_format_change(9, 'decimal')
 ---
 - 'Tuple field 9 type does not match one required by operation: expected decimal'
 ...
+-- array --X--> uuid
+fail_format_change(9, 'uuid')
+---
+- 'Tuple field 9 type does not match one required by operation: expected uuid'
+...
 -- map -----> any
 ok_format_change(10, 'any')
 ---
@@ -1289,6 +1330,11 @@ fail_format_change(10, 'decimal')
 ---
 - 'Tuple field 10 type does not match one required by operation: expected decimal'
 ...
+-- map --X--> uuid
+fail_format_change(10, 'uuid')
+---
+- 'Tuple field 10 type does not match one required by operation: expected uuid'
+...
 -- decimal ----> any
 ok_format_change(11, 'any')
 ---
@@ -1326,6 +1372,55 @@ fail_format_change(11, 'array')
 ---
 - 'Tuple field 11 type does not match one required by operation: expected array'
 ...
+-- decimal --X--> uuid
+fail_format_change(11, 'uuid')
+---
+- 'Tuple field 11 type does not match one required by operation: expected uuid'
+...
+-- uuid ----> any
+ok_format_change(12, 'any')
+---
+...
+-- uuid --X--> number
+fail_format_change(12, 'number')
+---
+- 'Tuple field 12 type does not match one required by operation: expected number'
+...
+-- uuid --X--> scalar
+fail_format_change(12, 'scalar')
+---
+- 'Tuple field 12 type does not match one required by operation: expected scalar'
+...
+-- uuid --X--> string
+fail_format_change(12, 'string')
+---
+- 'Tuple field 12 type does not match one required by operation: expected string'
+...
+-- uuid --X--> integer
+fail_format_change(12, 'integer')
+---
+- 'Tuple field 12 type does not match one required by operation: expected integer'
+...
+-- uuid --X--> unsigned
+fail_format_change(12, 'unsigned')
+---
+- 'Tuple field 12 type does not match one required by operation: expected unsigned'
+...
+-- uuid --X--> map
+fail_format_change(12, 'map')
+---
+- 'Tuple field 12 type does not match one required by operation: expected map'
+...
+-- uuid --X--> array
+fail_format_change(12, 'array')
+---
+- 'Tuple field 12 type does not match one required by operation: expected array'
+...
+-- uuid --X--> decimal
+fail_format_change(12, 'decimal')
+---
+- 'Tuple field 12 type does not match one required by operation: expected decimal'
+...
 s:drop()
 ---
 ...
diff --git a/test/engine/ddl.test.lua b/test/engine/ddl.test.lua
index e761966d7..7d408807f 100644
--- a/test/engine/ddl.test.lua
+++ b/test/engine/ddl.test.lua
@@ -356,6 +356,7 @@ s:drop()
 --
 
 decimal = require('decimal')
+uuid = require('uuid')
 
 -- Ensure that vinyl correctly process field count change.
 s = box.schema.space.create('test', {engine = engine, field_count = 2})
@@ -379,10 +380,11 @@ format[8] = {name = 'field8', type = 'scalar'}
 format[9] = {name = 'field9', type = 'array'}
 format[10] = {name = 'field10', type = 'map'}
 format[11] = {name = 'field11', type = 'decimal'}
+format[12] = {name = 'field12', type = 'uuid'}
 
 s = box.schema.space.create('test', {engine = engine, format = format})
 pk = s:create_index('pk')
-t = s:replace{1, {2}, 3, '4', 5.5, -6, true, -8, {9, 9}, {val = 10}, decimal.new(-11.11)}
+t = s:replace{1, {2}, 3, '4', 5.5, -6, true, -8, {9, 9}, {val = 10}, decimal.new(-11.11), uuid.new()}
 
 inspector:cmd("setopt delimiter ';'")
 function fail_format_change(fieldno, new_type)
@@ -421,6 +423,8 @@ ok_format_change(3, 'scalar')
 fail_format_change(3, 'map')
 -- unsigned --X--> decimal
 fail_format_change(3, 'decimal')
+-- unsigned --X--> uuid
+fail_format_change(3, 'uuid')
 
 -- string -----> any
 ok_format_change(4, 'any')
@@ -430,6 +434,8 @@ ok_format_change(4, 'scalar')
 fail_format_change(4, 'boolean')
 -- string --X--> decimal
 fail_format_change(4, 'decimal')
+-- string --X--> uuid
+fail_format_change(4, 'uuid')
 
 -- number -----> any
 ok_format_change(5, 'any')
@@ -439,6 +445,8 @@ ok_format_change(5, 'scalar')
 fail_format_change(5, 'integer')
 -- number --X--> decimal
 fail_format_change(5, 'decimal')
+-- number --X--> uuid
+fail_format_change(5, 'uuid')
 
 -- integer -----> any
 ok_format_change(6, 'any')
@@ -450,6 +458,8 @@ ok_format_change(6, 'scalar')
 fail_format_change(6, 'unsigned')
 -- integer --X--> decimal
 fail_format_change(6, 'decimal')
+-- integer --X--> uuid
+fail_format_change(6, 'uuid')
 
 -- boolean -----> any
 ok_format_change(7, 'any')
@@ -459,6 +469,8 @@ ok_format_change(7, 'scalar')
 fail_format_change(7, 'string')
 -- boolead --X--> decimal
 fail_format_change(7, 'decimal')
+-- boolean --X--> uuid
+fail_format_change(7, 'uuid')
 
 -- scalar -----> any
 ok_format_change(8, 'any')
@@ -466,6 +478,8 @@ ok_format_change(8, 'any')
 fail_format_change(8, 'unsigned')
 -- scalar --X--> decimal
 fail_format_change(8, 'decimal')
+-- scalar --X--> uuid
+fail_format_change(8, 'uuid')
 
 -- array -----> any
 ok_format_change(9, 'any')
@@ -473,6 +487,8 @@ ok_format_change(9, 'any')
 fail_format_change(9, 'scalar')
 -- arary --X--> decimal
 fail_format_change(9, 'decimal')
+-- array --X--> uuid
+fail_format_change(9, 'uuid')
 
 -- map -----> any
 ok_format_change(10, 'any')
@@ -480,6 +496,8 @@ ok_format_change(10, 'any')
 fail_format_change(10, 'scalar')
 -- map --X--> decimal
 fail_format_change(10, 'decimal')
+-- map --X--> uuid
+fail_format_change(10, 'uuid')
 
 -- decimal ----> any
 ok_format_change(11, 'any')
@@ -497,6 +515,28 @@ fail_format_change(11, 'unsigned')
 fail_format_change(11, 'map')
 -- decimal --X--> array
 fail_format_change(11, 'array')
+-- decimal --X--> uuid
+fail_format_change(11, 'uuid')
+
+-- uuid ----> any
+ok_format_change(12, 'any')
+-- uuid --X--> number
+fail_format_change(12, 'number')
+-- uuid --X--> scalar
+fail_format_change(12, 'scalar')
+-- uuid --X--> string
+fail_format_change(12, 'string')
+-- uuid --X--> integer
+fail_format_change(12, 'integer')
+-- uuid --X--> unsigned
+fail_format_change(12, 'unsigned')
+-- uuid --X--> map
+fail_format_change(12, 'map')
+-- uuid --X--> array
+fail_format_change(12, 'array')
+-- uuid --X--> decimal
+fail_format_change(12, 'decimal')
+
 s:drop()
 
 -- Check new fields adding.
diff --git a/test/engine/gh-4268-uuid.result b/test/engine/gh-4268-uuid.result
new file mode 100644
index 000000000..928204507
--- /dev/null
+++ b/test/engine/gh-4268-uuid.result
@@ -0,0 +1,58 @@
+-- test-run result file version 2
+env = require('test_run')
+ | ---
+ | ...
+test_run = env.new()
+ | ---
+ | ...
+engine = test_run:get_cfg('engine')
+ | ---
+ | ...
+
+uuid = require('uuid')
+ | ---
+ | ...
+ffi = require('ffi')
+ | ---
+ | ...
+
+-- check uuid indices
+_ = box.schema.space.create('test', {engine=engine})
+ | ---
+ | ...
+_ = box.space.test:create_index('pk', {parts={1,'uuid'}})
+ | ---
+ | ...
+
+-- uuid indices support cdata uuids, 36- and 32- byte strings
+-- and 16 byte binary strings
+for i = 1,16 do\
+    box.space.test:insert{uuid.new()}\
+    box.space.test:insert{tostring(uuid.new())}\
+    box.space.test:insert{tostring(uuid.new()):gsub('-', '')}\
+end
+ | ---
+ | ...
+
+a = box.space.test:select{}
+ | ---
+ | ...
+err = nil
+ | ---
+ | ...
+for i = 1, #a - 1 do\
+    if tostring(a[i][1]):gsub('-', '') >= tostring(a[i+1][1]):gsub('-', '')\
+            then err = {a[i][1], a[i+1][1]}\
+    end\
+end
+ | ---
+ | ...
+
+err
+ | ---
+ | - null
+ | ...
+
+box.space.test:drop()
+ | ---
+ | ...
diff --git a/test/engine/gh-4268-uuid.test.lua b/test/engine/gh-4268-uuid.test.lua
new file mode 100644
index 000000000..66c928553
--- /dev/null
+++ b/test/engine/gh-4268-uuid.test.lua
@@ -0,0 +1,30 @@
+env = require('test_run')
+test_run = env.new()
+engine = test_run:get_cfg('engine')
+
+uuid = require('uuid')
+ffi = require('ffi')
+
+-- check uuid indices
+_ = box.schema.space.create('test', {engine=engine})
+_ = box.space.test:create_index('pk', {parts={1,'uuid'}})
+
+-- uuid indices support cdata uuids, 36- and 32- byte strings
+-- and 16 byte binary strings
+for i = 1,16 do\
+    box.space.test:insert{uuid.new()}\
+    box.space.test:insert{tostring(uuid.new())}\
+    box.space.test:insert{tostring(uuid.new()):gsub('-', '')}\
+end
+
+a = box.space.test:select{}
+err = nil
+for i = 1, #a - 1 do\
+    if tostring(a[i][1]):gsub('-', '') >= tostring(a[i+1][1]):gsub('-', '')\
+            then err = {a[i][1], a[i+1][1]}\
+    end\
+end
+
+err
+
+box.space.test:drop()
-- 
2.21.1 (Apple Git-122.3)

Re: [Tarantool-patches] [PATCH 0/4] introduce indices over UUID

[Vladislav Shpilevoy]

Hi! Thanks for the patchset!

The build fails on your branch in Travis, on Linux with gcc:
https://travis-ci.org/github/tarantool/tarantool/builds/670792910?utm_source=github_status&utm_medium=notification

On 04/04/2020 01:02, Serge Petrenko wrote:
> https://github.com/tarantool/tarantool/issues/4268
> https://github.com/tarantool/tarantool/tree/sp/gh-4268-uuid-type
> 
> Serge Petrenko (4):
>   decimal: fix comment typo
>   uuid: expose additional from_string constructors
>   box: add MsgPack encoding/decoding for UUID
>   box: introduce indices by UUID
> 
>  extra/exports                        |   3 +
>  src/box/field_def.c                  |  66 +++++++++++---
>  src/box/field_def.h                  |  16 ++++
>  src/box/key_def.h                    |   3 +-
>  src/box/tuple_compare.cc             | 123 +++++++++++++++++++++++++++
>  src/box/tuple_format.c               |   3 +-
>  src/lib/core/CMakeLists.txt          |   1 +
>  src/lib/core/mp_decimal.h            |   2 +-
>  src/lib/core/mp_extension_types.h    |   2 +
>  src/lib/core/mp_uuid.c               |  75 ++++++++++++++++
>  src/lib/core/mp_uuid.h               |  90 ++++++++++++++++++++
>  src/lib/core/mpstream.c              |  11 +++
>  src/lib/core/mpstream.h              |   5 ++
>  src/lib/msgpuck                      |   2 +-
>  src/lib/uuid/tt_uuid.c               |   9 ++
>  src/lib/uuid/tt_uuid.h               |  53 +++++++++---
>  src/lua/msgpack.c                    |  27 ++++--
>  src/lua/msgpackffi.lua               |  14 +++
>  src/lua/utils.c                      |  21 ++++-
>  src/lua/utils.h                      |   5 ++
>  src/lua/uuid.lua                     |   9 --
>  test/app-tap/lua/serializer_test.lua |   8 ++
>  test/app-tap/msgpackffi.test.lua     |   3 +-
>  test/app/msgpack.result              |  21 +++++
>  test/app/msgpack.test.lua            |  13 +++
>  test/app/uuid.result                 |   2 +-
>  test/box/tuple.result                |  81 ++++++++++++++++++
>  test/box/tuple.test.lua              |  25 ++++++
>  test/engine/ddl.result               |  97 ++++++++++++++++++++-
>  test/engine/ddl.test.lua             |  42 ++++++++-
>  test/engine/gh-4268-uuid.result      |  58 +++++++++++++
>  test/engine/gh-4268-uuid.test.lua    |  30 +++++++
>  test/unit/uuid.c                     |  24 +++++-
>  test/unit/uuid.result                |   8 +-
>  third_party/lua-cjson/lua_cjson.c    |  27 ++++--
>  third_party/lua-yaml/lyaml.cc        |  17 +++-
>  36 files changed, 933 insertions(+), 63 deletions(-)
>  create mode 100644 src/lib/core/mp_uuid.c
>  create mode 100644 src/lib/core/mp_uuid.h
>  create mode 100644 test/engine/gh-4268-uuid.result
>  create mode 100644 test/engine/gh-4268-uuid.test.lua
> 

Re: [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo

[Vladislav Shpilevoy]

This commit LGTM.

On 04/04/2020 01:02, Serge Petrenko wrote:
> ---
>  src/lib/core/mp_decimal.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/lib/core/mp_decimal.h b/src/lib/core/mp_decimal.h
> index 778529068..3c708a8e8 100644
> --- a/src/lib/core/mp_decimal.h
> +++ b/src/lib/core/mp_decimal.h
> @@ -56,7 +56,7 @@ mp_decode_decimal(const char **data, decimal_t *dec);
>  
>  /**
>   * \brief Encode a decimal pointed to by \a dec.
> - * \parad dec - decimal pointer
> + * \param dec - decimal pointer
>   * \param data - a buffer
>   * \return \a data + mp_sizeof_decimal(\a dec)
>   */
> 

Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors

[Vladislav Shpilevoy]

Thanks for the patch!

> diff --git a/src/lib/uuid/tt_uuid.c b/src/lib/uuid/tt_uuid.c
> index 1bd2e2cfe..94a0b15bb 100644
> --- a/src/lib/uuid/tt_uuid.c
> +++ b/src/lib/uuid/tt_uuid.c
> @@ -65,6 +65,15 @@ tt_uuid_create(struct tt_uuid *uu)
>  }
>  #endif
>  
> +extern inline int

'inline' modifier won't change anything. The function body is
not visible anyway.

> +tt_uuid_validate(struct tt_uuid *uu);
> +
> +extern inline int
> +tt_uuid_from_fmt_string(const char *in, struct tt_uuid *uu, const char *fmt);
> +
> +extern inline int
> +tt_uuid_from_lstring(const char *in, uint32_t len, struct tt_uuid *uu);
> +
>  extern inline int
>  tt_uuid_from_string(const char *in, struct tt_uuid *uu);
>  

There are unit tests for uuid in unit/uuid.c. You may need to
add tests for the new functions.

Re: [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID

[Vladislav Shpilevoy]

Thanks for the patch!

See 10 comments below.

> diff --git a/src/lib/core/mp_uuid.c b/src/lib/core/mp_uuid.c
> new file mode 100644
> index 000000000..5b0cc3f1a
> --- /dev/null
> +++ b/src/lib/core/mp_uuid.c
> @@ -0,0 +1,75 @@
> +
> +#include "mp_uuid.h"
> +#include "msgpuck.h"
> +#include "mp_extension_types.h"
> +#include "lib/uuid/tt_uuid.h"

1. uuid is a separate library depending on core. Now core
depends on uuid too. This is a cyclic dependency. It does not
explode probably only because all needed functions are in the
tt_uuid.h header. So mp_uuid.c/.h should be moved to lib/uuid.
Is it possible?

I see we add MP_UUID to src/lib/core/mp_extension_types.h.
That is also a part of core lib. Maybe we should just merge core
and uuid libs. I remember we split them only because Kostja
insisted, but personally I don't see any reason in keeping then
separated.

However I am afraid this won't help forever. In future we may
want to add new mp_ext for something in box/ such as MP_TUPLE
or something. We already want something like that for custom
errors. Not related to this patch anyway.

> +
> +inline uint32_t
> +mp_sizeof_uuid()
> +{
> +	return mp_sizeof_ext(sizeof(struct tt_uuid));
> +}
> +
> +struct tt_uuid *
> +uuid_unpack(const char **data, uint32_t len, struct tt_uuid *uuid)
> +{
> +	if (len != sizeof(*uuid))
> +		return NULL;
> +	memcpy(uuid, *data, sizeof(*uuid));
> +	if (tt_uuid_validate(uuid) != 0)
> +		return NULL;
> +	*data += sizeof(*uuid);
> +	return uuid;
> +}
> +
> +struct tt_uuid *
> +mp_decode_uuid(const char **data, struct tt_uuid *uuid)
> +{
> +	if (mp_typeof(**data) != MP_EXT)
> +		return NULL;
> +	int8_t type;
> +	const char *const svp = *data;
> +
> +	uint32_t len = mp_decode_extl(data, &type);
> +	if (type != MP_UUID || uuid_unpack(data, len, uuid) == NULL) {
> +		*data = svp;
> +		return NULL;
> +	}
> +	return uuid;
> +}
> +
> +char *
> +mp_encode_uuid(char *data, const struct tt_uuid *uuid)
> +{
> +	return mp_encode_ext(data, MP_UUID, (char *)uuid, sizeof(*uuid));

2. Can we just copy it as is? Shouldn't it be big endian or
little endian or whatever the standard says? If the standard
says nothing, we should use big-endian (because all other msgpack
types use it), and we need to either convert every field of
uuid to big endian before encoding, or ensure it is already
big endian always, and copy as is.

In case tt_uuid fields are little endian now, I think no big
problem in storing them as big endian. Just keep tostring/fromstring
and other public functions doing the same as before. If we store
uuid in big-endian, we can use memcmp() to compare two 16byte binaries.
Without conversions.

> +}
> diff --git a/src/lib/core/mp_uuid.h b/src/lib/core/mp_uuid.h
> new file mode 100644
> index 000000000..fda0f3aed
> --- /dev/null
> +++ b/src/lib/core/mp_uuid.h
> @@ -0,0 +1,90 @@
> +#ifndef TARANTOOL_LIB_CORE_MP_UUID_INCLUDED
> +#define TARANTOOL_LIB_CORE_MP_UUID_INCLUDED

3. We use #pragma once now, no?

> +#include <stdint.h>
> +
> +#if defined(__cplusplus)
> +extern "C" {
> +#endif /* defined(__cplusplus) */
> +
> +struct tt_uuid;
> +
> +/**
> + * \brief Return the number of bytes an encoded uuid value takes.
> + */
> +uint32_t
> +mp_sizeof_uuid();

4. Please, use 'void' when a function does not take any arguments.
Cyrill was saing that it is bad to leave empty (), because then
the compiler considers it a function having any number of arguments,
and generates some code for that.

> +
> +/**
> + * Copy a uuid value from a buffer. Can be used in a combination
> + * with mp_decode_extl() instead of mp_decode_uuid() when multiple
> + * extension types are possible.
> + *
> + * \param data A buffer.
> + * \param len Length returned by mp_decode_extl, has to be equal
> + *            to sizeof(struct tt_uuid), otherwise an error is
> + *            returned.
> + * \param[out] uuid Uuid to be decoded.
> + * \return A pointer to the decoded uuid.
> + *         NULL in case of an error.
> + * \post *data = *data + sizeof(struct tt_uuid).
> + */
> +struct tt_uuid *
> +uuid_unpack(const char **data, uint32_t len, struct tt_uuid *uuid);

5. Why is it in mp_uuid.h/.c? It does not depend on msgpack at all.
The similar function decimal_unpack(), for example, is in decimal.c.
Not in mp_decimal.c.

> +
> +/**
> + * \brief Decode a uuid from MsgPack \a data.
> + * \param data A buffer.
> + * \param[out] uuid Uuid to be decoded.
> + * \return A pointer to the decoded uuid.
> + *         NULL in case of an error.
> + * \post *data = *data + mp_sizeof_uuid().
> + */
> +struct tt_uuid *
> +mp_decode_uuid(const char **data, struct tt_uuid *uuid);
> +
> diff --git a/src/lib/msgpuck b/src/lib/msgpuck
> index 8ae606a16..0d273f95f 160000
> --- a/src/lib/msgpuck
> +++ b/src/lib/msgpuck
> @@ -1 +1 @@
> -Subproject commit 8ae606a1636dd89b2d61b154e5a1db03dce91657
> +Subproject commit 0d273f95f8de64aeed698c354ca3bc7cb5ea461a

6. I didn't see a patch for msgpuck repo. Did you send it?

> diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
> index edbc15b72..fb49b1547 100644
> --- a/src/lua/msgpack.c
> +++ b/src/lua/msgpack.c
> @@ -325,6 +336,10 @@ luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
>  		break;
>  	}
>  	}
> +return;

7. Bad indentation for 'return;'.

> +ext_decode_err:
> +	lua_pop(L, -1);
> +	luaL_error(L, "msgpack.decode: invalid MsgPack");
>  }
>  > diff --git a/test/app/uuid.result b/test/app/uuid.result
> index 0713614c6..013c51282 100644
> --- a/test/app/uuid.result
> +++ b/test/app/uuid.result
> @@ -106,7 +106,7 @@ uu.node[5]
>  -- invalid values
>  uuid.fromstr(nil)
>  ---
> -- error: 'builtin/uuid.lua:47: fromstr(str)'
> +- error: 'builtin/uuid.lua:38: fromstr(str)'

8. Better add a test_run filter to avoid that change in
future when anything will be updated in uuid.lua again.

>  ...
>  uuid.fromstr('')
>  ---
> diff --git a/test/unit/uuid.c b/test/unit/uuid.c
> index c43d93b4f..4f6b963ba 100644
> --- a/test/unit/uuid.c
> +++ b/test/unit/uuid.c
> @@ -27,10 +29,28 @@ uuid_test(struct tt_uuid a, struct tt_uuid b, int expected_result)
>             "%s %s %s", a_str, sign, b_str);
>  }
>  
> +static void
> +mp_uuid_test()
> +{
> +        plan(4);
> +        char buf[18];
> +        char *data = buf;
> +        const char *data1 = buf;
> +        struct tt_uuid uu, ret;
> +        random_init();
> +        tt_uuid_create(&uu);
> +        char *end = mp_encode_uuid(data, &uu);
> +        is(end - data, mp_sizeof_uuid(), "mp_sizeof_uuid() == encoded length");
> +        struct tt_uuid *rc = mp_decode_uuid(&data1, &ret);
> +        is(rc, &ret, "mp_decode_uuid() return code");
> +        is(data1, end, "mp_sizeof_uuid() == decoded length");
> +        is(tt_uuid_compare(&uu, &ret), 0, "mp_decode_uuid(mp_encode_uuid(uu)) == uu");

9. When you create a subset of tests, it should have at least
plan() + check_plan(). Also we usually add header() footer(), but
seems like it is too late for this test file.

> +}
> +
>  int
>  main(void)
>  {
> -        plan(2);
> +        plan(3);
> diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
> index af4f2f5d5..411c56f71 100644
> --- a/third_party/lua-yaml/lyaml.cc
> +++ b/third_party/lua-yaml/lyaml.cc
> @@ -697,10 +700,18 @@ static int dump_node(struct lua_yaml_dumper *dumper)
>        switch (field.ext_type) {
>        case MP_DECIMAL:
>           str = decimal_to_string(field.decval);
> -	 len = strlen(str);
> -	 break;
> +         len = strlen(str);
> +         break;
> +      case MP_UUID:
> +      {
> +         char *buf = tt_static_buf();
> +         tt_uuid_to_string(field.uuidval, buf);
> +         str = buf;
> +         len = UUID_STR_LEN;
> +         break;
> +      }
>        default:
> -	 assert(0); /* checked by luaL_checkfield() */
> +         assert(0); /* checked by luaL_checkfield() */

10. Better avoid the unnecessary diff.

>        }
>        break;
>      }
> 

Re: [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID

[Vladislav Shpilevoy]

Thanks for the patch!

See 6 comments below.

On 04/04/2020 01:02, Serge Petrenko wrote:
> It is now possible to create an index over UUID values, either returned
> by something like `uuid.new()` or 36- and 32- byte strings in format
> "88ebbb9a-8c18-480f-bd04-dd5345f1573c" or "88ebbb9a8c18480fbd04dd5345f1573c".
> 16-byte binary strings are also supported.
> 
> Closes #4268
> Closes #2916
> 
> @TarantoolBot document
> Title: Document uuid field type.
> 
> There's a new field type -- UUID, it accepts values returned by
> `uuid.new()`, as well as strings representing uuids, like
> `88ebbb9a-8c18-480f-bd04-dd5345f1573c` and
> `88ebbb9a8c18480fbd04dd5345f1573c` and 16 byte binary values, encoded in
> msgpack.
> 
> The index may be either unique or non-unique, nullable or non-nullable,
> and may be a primary key.
> 
> To create an index over a uuid field for space `test`, say:
> ```
> box.space.test:create_index("pk", {parts={1, 'uuid'}})
> ```
> Now you may insert uuids into the space:
> ```
> tarantool> box.space.test:insert{uuid.new()}
> ---
> - [e631fdcc-0e8a-4d2f-83fd-b0ce6762b13f]
> ...
> 
> tarantool> box.space.test:insert{"64d22e4d-ac92-4a23-899a-e59f34af5479"}
> ---
> - ['64d22e4d-ac92-4a23-899a-e59f34af5479']
> ...
> 
> tarantool> box.space.test:insert{"856dc1177dcc49969f1407f8c6c8a371"}
> ---
> - ['856dc1177dcc49969f1407f8c6c8a371']
> ...
> 
> tarantool> box.space.test:select{}
> ---
> - - ['64d22e4d-ac92-4a23-899a-e59f34af5479']
>   - ['856dc1177dcc49969f1407f8c6c8a371']
>   - [e631fdcc-0e8a-4d2f-83fd-b0ce6762b13f]

1. Wait. Why are they printed differently? They should be all the
same 36 strings when you print them, no?

Seems like you understood Mons to the letter. I believe he meant
we should provide a way to insert strings to simplify moving data
to a new space, but it does not mean we should store them as strings.
It makes comparators slower. sscanf() to extract uuid from a string
on *each* comparison is a huge performance hole. From what I measured
recently (not in Tarantool), sscanf() is a *really* slow thing. Its
usage in comparators is dangerous.

On the other hand ability to insert strings means we need to convert
them to MP_EXT after insertion, which means full scan of each tuple
by field types and possibly its recreation. We had a similar problem
with 'double' type. After all it was decided to keep as is and let
users enforce the needed type when they want using ffi.cast('double').

IMO, we should just disallow both ability to store strings, and to
insert strings. I mean, you can't insert a string into a decimal field,
for example. So a user anyway will never be able to insert only Lua
simple types. Cdata is enforced for decimal and double right now.

Better keep it simple. If someone wants to convert their existing spaces
to mp uuid, they easily can write a trivial function which would replace
string uuid with cdata uuid before insertion into the new space. We
should not sacrifice perf for that.

This is only my opinion, you may want to ask Mons and Kirill.


2. It is worth mentioning comparison order. Are they compared as
strings, or individual uuid fields are compared? In the latter case
in what order the fields are compared?

Also probably string comparison vs by-field comparison vs memcmp
comparison will be the same in case fields are stored in msgpack
in the same order as in the string, and in big-endian. I didn't check
but it is worth investigation. It would be good to be able to use
memcmp.

> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
> index 3f8a0ce24..aad69340d 100644
> --- a/src/box/tuple_compare.cc
> +++ b/src/box/tuple_compare.cc
> @@ -378,6 +382,61 @@ mp_compare_bin(const char *field_a, const char *field_b)
>  	return COMPARE_RESULT(size_a, size_b);
>  }
>  
> +static int
> +mp_compare_uuid_with_type(const char *field_a, enum mp_type type_a,
> +			  const char *field_b, enum mp_type type_b)
> +{
> +	struct tt_uuid uuid_a, uuid_b;
> +	struct tt_uuid *ret;
> +	const char *str;
> +	uint32_t len;
> +	int rc;
> +	switch (type_a) {
> +	case MP_STR:
> +		str = mp_decode_str(&field_a, &len);
> +		rc = tt_uuid_from_lstring(str, len, &uuid_a);

3. This is perf killer. Basically it would be faster to store
uuids as a string, or just as 16 bytes binary. Because memcmp
could be used directly then.

> +		assert(rc == 0);
> +		break;
> +	case MP_BIN:
> +		str = mp_decode_bin(&field_a, &len);
> +		ret = uuid_unpack(&str, len, &uuid_a);
> +		assert(ret != NULL);
> +		break;
> +	case MP_EXT:
> +		ret = mp_decode_uuid(&field_a, &uuid_a);
> +		assert(ret != NULL);
> +		break;
> +	default:
> +		unreachable();
> +	}
> +	switch (type_b) {
> +	case MP_STR:
> +		str = mp_decode_str(&field_b, &len);
> +		rc = tt_uuid_from_lstring(str, len, &uuid_b);
> +		assert(rc == 0);
> +		break;
> +	case MP_BIN:
> +		str = mp_decode_bin(&field_b, &len);
> +		ret = uuid_unpack(&str, len, &uuid_b);
> +		assert(ret != NULL);
> +		break;
> +	case MP_EXT:
> +		ret = mp_decode_uuid(&field_b, &uuid_b);
> +		assert(ret != NULL);
> +		break;
> +	default:
> +		unreachable();
> +	}
> +	return tt_uuid_compare(&uuid_a, &uuid_b);

4. If we will decide to store uuids as MP_EXT, it may be
useful to introduce a function mp_compare_uuid, by analogue
with other mp_compare_* functions. To compare them directly
using memcmp, without unpacking. But see above my comment
about order. memcmp validness depends on how uuid is stored
in MessagePack.

> +}
> +
> +static inline int
> +mp_compare_uuid(const char *field_a, const char *field_b)
> +{
> +	return mp_compare_uuid_with_type(field_a, mp_typeof(*field_a),
> +					 field_b, mp_typeof(*field_b));
> +}
> @@ -1578,6 +1642,21 @@ hint_decimal(decimal_t *dec)
>  	return hint_create(MP_CLASS_NUMBER, val);
>  }
>  
> +static inline hint_t
> +hint_uuid(struct tt_uuid *uuid)
> +{
> +	/* Simply take the first part of the UUID as hint. */

5. Why only first?

> +	uint64_t val = 0;
> +	val |= uuid->time_low;
> +	val <<= sizeof(uuid->time_mid) * CHAR_BIT;
> +	val |= uuid->time_mid;
> +	val <<= sizeof(uuid->time_hi_and_version) * CHAR_BIT;
> +	val |= uuid->time_hi_and_version;
> +	/* Make space for class representation. */
> +	val >>= HINT_CLASS_BITS;
> +	return hint_create(MP_CLASS_UUID, val);
> +}
> diff --git a/test/engine/gh-4268-uuid.result b/test/engine/gh-4268-uuid.result
> new file mode 100644
> index 000000000..928204507
> --- /dev/null
> +++ b/test/engine/gh-4268-uuid.result

6. Since this is a feature, we probably should not call the
test file using name pattern used for bugs. Just uuid.test.lua.

Re: [Tarantool-patches] [PATCH 0/4] introduce indices over UUID

[Serge Petrenko]

> 6 апр. 2020 г., в 00:21, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
> 
> Hi! Thanks for the patchset!
> 


Hi! Thanks for the review!
Please see my letters with answers to your comments, as well as v2 with all
the changes.

> The build fails on your branch in Travis, on Linux with gcc:
> https://travis-ci.org/github/tarantool/tarantool/builds/670792910?utm_source=github_status&utm_medium=notification

I couldn’t fix the build yet, but I’m still sending out the updated patchset
so that you have more time to review it.
I guess the build fix will be small enough, and I’ll update the patchset
again as soon as I manage to find what goes wrong.

> 
> On 04/04/2020 01:02, Serge Petrenko wrote:
>> https://github.com/tarantool/tarantool/issues/4268
>> https://github.com/tarantool/tarantool/tree/sp/gh-4268-uuid-type
>> 
>> Serge Petrenko (4):
>>  decimal: fix comment typo
>>  uuid: expose additional from_string constructors
>>  box: add MsgPack encoding/decoding for UUID
>>  box: introduce indices by UUID
>> 
>> extra/exports                        |   3 +
>> src/box/field_def.c                  |  66 +++++++++++---
>> src/box/field_def.h                  |  16 ++++
>> src/box/key_def.h                    |   3 +-
>> src/box/tuple_compare.cc             | 123 +++++++++++++++++++++++++++
>> src/box/tuple_format.c               |   3 +-
>> src/lib/core/CMakeLists.txt          |   1 +
>> src/lib/core/mp_decimal.h            |   2 +-
>> src/lib/core/mp_extension_types.h    |   2 +
>> src/lib/core/mp_uuid.c               |  75 ++++++++++++++++
>> src/lib/core/mp_uuid.h               |  90 ++++++++++++++++++++
>> src/lib/core/mpstream.c              |  11 +++
>> src/lib/core/mpstream.h              |   5 ++
>> src/lib/msgpuck                      |   2 +-
>> src/lib/uuid/tt_uuid.c               |   9 ++
>> src/lib/uuid/tt_uuid.h               |  53 +++++++++---
>> src/lua/msgpack.c                    |  27 ++++--
>> src/lua/msgpackffi.lua               |  14 +++
>> src/lua/utils.c                      |  21 ++++-
>> src/lua/utils.h                      |   5 ++
>> src/lua/uuid.lua                     |   9 --
>> test/app-tap/lua/serializer_test.lua |   8 ++
>> test/app-tap/msgpackffi.test.lua     |   3 +-
>> test/app/msgpack.result              |  21 +++++
>> test/app/msgpack.test.lua            |  13 +++
>> test/app/uuid.result                 |   2 +-
>> test/box/tuple.result                |  81 ++++++++++++++++++
>> test/box/tuple.test.lua              |  25 ++++++
>> test/engine/ddl.result               |  97 ++++++++++++++++++++-
>> test/engine/ddl.test.lua             |  42 ++++++++-
>> test/engine/gh-4268-uuid.result      |  58 +++++++++++++
>> test/engine/gh-4268-uuid.test.lua    |  30 +++++++
>> test/unit/uuid.c                     |  24 +++++-
>> test/unit/uuid.result                |   8 +-
>> third_party/lua-cjson/lua_cjson.c    |  27 ++++--
>> third_party/lua-yaml/lyaml.cc        |  17 +++-
>> 36 files changed, 933 insertions(+), 63 deletions(-)
>> create mode 100644 src/lib/core/mp_uuid.c
>> create mode 100644 src/lib/core/mp_uuid.h
>> create mode 100644 test/engine/gh-4268-uuid.result
>> create mode 100644 test/engine/gh-4268-uuid.test.lua
>> 



--
Serge Petrenko
sergepetrenko@tarantool.org

Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors

[Serge Petrenko]

Hi! Thanks for the review!


> 6 апр. 2020 г., в 00:22, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
> 
> Thanks for the patch!
> 
>> diff --git a/src/lib/uuid/tt_uuid.c b/src/lib/uuid/tt_uuid.c
>> index 1bd2e2cfe..94a0b15bb 100644
>> --- a/src/lib/uuid/tt_uuid.c
>> +++ b/src/lib/uuid/tt_uuid.c
>> @@ -65,6 +65,15 @@ tt_uuid_create(struct tt_uuid *uu)
>> }
>> #endif
>> 
>> +extern inline int
> 
> 'inline' modifier won't change anything. The function body is
> not visible anyway.

Not sure about that.
What about other functions?

```
extern inline int                                                                   
tt_uuid_from_string(const char *in, struct tt_uuid *uu);
```

I’m still not sure what ‘extern inline’ does, but I googled a stackoverflow
question discussing it:
https://stackoverflow.com/questions/216510/what-does-extern-inline-do

> 
>> +tt_uuid
>> _validate(struct tt_uuid *uu);
>> +
>> +extern inline int
>> +tt_uuid_from_fmt_string(const char *in, struct tt_uuid *uu, const char *fmt);
>> +
>> +extern inline int
>> +tt_uuid_from_lstring(const char *in, uint32_t len, struct tt_uuid *uu);
>> +
>> extern inline int
>> tt_uuid_from_string(const char *in, struct tt_uuid *uu);
>> 
> 
> There are unit tests for uuid in unit/uuid.c. You may need to
> add tests for the new functions.

I removed all the excess tt_uuid_from_* helpers in the new version after discussing
the patch with Mons. He is okay with allowing only cdata in indices, so I don’t need
the from string constructors anymore.

--
Serge Petrenko
sergepetrenko@tarantool.org

Re: [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID

[Serge Petrenko]

> 6 апр. 2020 г., в 00:26, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
> 
> Thanks for the patch!
> 

Hi! Thanks for the review!

> See 10 comments below.
> 
>> diff --git a/src/lib/core/mp_uuid.c b/src/lib/core/mp_uuid.c
>> new file mode 100644
>> index 000000000..5b0cc3f1a
>> --- /dev/null
>> +++ b/src/lib/core/mp_uuid.c
>> @@ -0,0 +1,75 @@
>> +
>> +#include "mp_uuid.h"
>> +#include "msgpuck.h"
>> +#include "mp_extension_types.h"
>> +#include "lib/uuid/tt_uuid.h"
> 
> 1. uuid is a separate library depending on core. Now core
> depends on uuid too. This is a cyclic dependency. It does not
> explode probably only because all needed functions are in the
> tt_uuid.h header. So mp_uuid.c/.h should be moved to lib/uuid.
> Is it possible?
> 
> I see we add MP_UUID to src/lib/core/mp_extension_types.h.
> That is also a part of core lib. Maybe we should just merge core
> and uuid libs. I remember we split them only because Kostja
> insisted, but personally I don't see any reason in keeping then
> separated.
> 
> However I am afraid this won't help forever. In future we may
> want to add new mp_ext for something in box/ such as MP_TUPLE
> or something. We already want something like that for custom
> errors. Not related to this patch anyway.

Thanks for pointing this out. I merged mp_uuid.h/.c into uuid lib,
and also extracted mpstream, which depends on mp_uuid now, into a
separate library. So now the dependencies are
                  
mpstream —> uuid -> core
    \________->_______/

> 
>> +
>> +inline uint32_t
>> +mp_sizeof_uuid()
>> +{
>> +	return mp_sizeof_ext(sizeof(struct tt_uuid));
>> +}
>> +
>> +struct tt_uuid *
>> +uuid_unpack(const char **data, uint32_t len, struct tt_uuid *uuid)
>> +{
>> +	if (len != sizeof(*uuid))
>> +		return NULL;
>> +	memcpy(uuid, *data, sizeof(*uuid));
>> +	if (tt_uuid_validate(uuid) != 0)
>> +		return NULL;
>> +	*data += sizeof(*uuid);
>> +	return uuid;
>> +}
>> +
>> +struct tt_uuid *
>> +mp_decode_uuid(const char **data, struct tt_uuid *uuid)
>> +{
>> +	if (mp_typeof(**data) != MP_EXT)
>> +		return NULL;
>> +	int8_t type;
>> +	const char *const svp = *data;
>> +
>> +	uint32_t len = mp_decode_extl(data, &type);
>> +	if (type != MP_UUID || uuid_unpack(data, len, uuid) == NULL) {
>> +		*data = svp;
>> +		return NULL;
>> +	}
>> +	return uuid;
>> +}
>> +
>> +char *
>> +mp_encode_uuid(char *data, const struct tt_uuid *uuid)
>> +{
>> +	return mp_encode_ext(data, MP_UUID, (char *)uuid, sizeof(*uuid));
> 
> 2. Can we just copy it as is? Shouldn't it be big endian or
> little endian or whatever the standard says? If the standard
> says nothing, we should use big-endian (because all other msgpack
> types use it), and we need to either convert every field of
> uuid to big endian before encoding, or ensure it is already
> big endian always, and copy as is.
> 
> In case tt_uuid fields are little endian now, I think no big
> problem in storing them as big endian. Just keep tostring/fromstring
> and other public functions doing the same as before. If we store
> uuid in big-endian, we can use memcmp() to compare two 16byte binaries.
> Without conversions.
> 

Good point, thanks! It isn’t specified whether uuid fields should be big or
little endian. The standard does specify though that the fields must be
encoded as big endian. So instead of memcpy I do mp_store_u32, mp_store_u16, …
and so on. (mp_load_u32(), … for unpack).
mp_load/store_8/16/32 convert the integers to big endian for us, if needed.

>> +}
>> diff --git a/src/lib/core/mp_uuid.h b/src/lib/core/mp_uuid.h
>> new file mode 100644
>> index 000000000..fda0f3aed
>> --- /dev/null
>> +++ b/src/lib/core/mp_uuid.h
>> @@ -0,0 +1,90 @@
>> +#ifndef TARANTOOL_LIB_CORE_MP_UUID_INCLUDED
>> +#define TARANTOOL_LIB_CORE_MP_UUID_INCLUDED
> 
> 3. We use #pragma once now, no?
> 

Ok, fixed.

>> +#include <stdint.h>
>> +
>> +#if defined(__cplusplus)
>> +extern "C" {
>> +#endif /* defined(__cplusplus) */
>> +
>> +struct tt_uuid;
>> +
>> +/**
>> + * \brief Return the number of bytes an encoded uuid value takes.
>> + */
>> +uint32_t
>> +mp_sizeof_uuid();
> 
> 4. Please, use 'void' when a function does not take any arguments.
> Cyrill was saing that it is bad to leave empty (), because then
> the compiler considers it a function having any number of arguments,
> and generates some code for that.

Done.

> 
>> +
>> +/**
>> + * Copy a uuid value from a buffer. Can be used in a combination
>> + * with mp_decode_extl() instead of mp_decode_uuid() when multiple
>> + * extension types are possible.
>> + *
>> + * \param data A buffer.
>> + * \param len Length returned by mp_decode_extl, has to be equal
>> + *            to sizeof(struct tt_uuid), otherwise an error is
>> + *            returned.
>> + * \param[out] uuid Uuid to be decoded.
>> + * \return A pointer to the decoded uuid.
>> + *         NULL in case of an error.
>> + * \post *data = *data + sizeof(struct tt_uuid).
>> + */
>> +struct tt_uuid *
>> +uuid_unpack(const char **data, uint32_t len, struct tt_uuid *uuid);
> 
> 5. Why is it in mp_uuid.h/.c? It does not depend on msgpack at all.
> The similar function decimal_unpack(), for example, is in decimal.c.
> Not in mp_decimal.c.

After considering your comments re uuid_unpack
I pack and unpack uuid fields in big endian style,
using mp_store/load_u32/u16/u8. So let it be in mp_uuid.h/.c now.

> 
>> +
>> +/**
>> + * \brief Decode a uuid from MsgPack \a data.
>> + * \param data A buffer.
>> + * \param[out] uuid Uuid to be decoded.
>> + * \return A pointer to the decoded uuid.
>> + *         NULL in case of an error.
>> + * \post *data = *data + mp_sizeof_uuid().
>> + */
>> +struct tt_uuid *
>> +mp_decode_uuid(const char **data, struct tt_uuid *uuid);
>> +
>> diff --git a/src/lib/msgpuck b/src/lib/msgpuck
>> index 8ae606a16..0d273f95f 160000
>> --- a/src/lib/msgpuck
>> +++ b/src/lib/msgpuck
>> @@ -1 +1 @@
>> -Subproject commit 8ae606a1636dd89b2d61b154e5a1db03dce91657
>> +Subproject commit 0d273f95f8de64aeed698c354ca3bc7cb5ea461a
> 
> 6. I didn't see a patch for msgpuck repo. Did you send it?

Yeah, you even LGTM’ed it ([PATCH] Add const qualifier to mp_encode_ext argument)
Alexander Turenko pushed it and I updated the reference in v2 of this patchset.

> 
>> diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
>> index edbc15b72..fb49b1547 100644
>> --- a/src/lua/msgpack.c
>> +++ b/src/lua/msgpack.c
>> @@ -325,6 +336,10 @@ luamp_decode(struct lua_State *L, struct luaL_serializer *cfg,
>> 		break;
>> 	}
>> 	}
>> +return;
> 
> 7. Bad indentation for 'return;'.
> 

Yep, fixed, ty.

>> +ext_decode_err:
>> +	lua_pop(L, -1);
>> +	luaL_error(L, "msgpack.decode: invalid MsgPack");
>> }
>>> diff --git a/test/app/uuid.result b/test/app/uuid.result
>> index 0713614c6..013c51282 100644
>> --- a/test/app/uuid.result
>> +++ b/test/app/uuid.result
>> @@ -106,7 +106,7 @@ uu.node[5]
>> -- invalid values
>> uuid.fromstr(nil)
>> ---
>> -- error: 'builtin/uuid.lua:47: fromstr(str)'
>> +- error: 'builtin/uuid.lua:38: fromstr(str)'
> 
> 8. Better add a test_run filter to avoid that change in
> future when anything will be updated in uuid.lua again.
> 

Ok, done.

>> ...
>> uuid.fromstr('')
>> ---
>> diff --git a/test/unit/uuid.c b/test/unit/uuid.c
>> index c43d93b4f..4f6b963ba 100644
>> --- a/test/unit/uuid.c
>> +++ b/test/unit/uuid.c
>> @@ -27,10 +29,28 @@ uuid_test(struct tt_uuid a, struct tt_uuid b, int expected_result)
>>            "%s %s %s", a_str, sign, b_str);
>> }
>> 
>> +static void
>> +mp_uuid_test()
>> +{
>> +        plan(4);
>> +        char buf[18];
>> +        char *data = buf;
>> +        const char *data1 = buf;
>> +        struct tt_uuid uu, ret;
>> +        random_init();
>> +        tt_uuid_create(&uu);
>> +        char *end = mp_encode_uuid(data, &uu);
>> +        is(end - data, mp_sizeof_uuid(), "mp_sizeof_uuid() == encoded length");
>> +        struct tt_uuid *rc = mp_decode_uuid(&data1, &ret);
>> +        is(rc, &ret, "mp_decode_uuid() return code");
>> +        is(data1, end, "mp_sizeof_uuid() == decoded length");
>> +        is(tt_uuid_compare(&uu, &ret), 0, "mp_decode_uuid(mp_encode_uuid(uu)) == uu");
> 
> 9. When you create a subset of tests, it should have at least
> plan() + check_plan(). Also we usually add header() footer(), but
> seems like it is too late for this test file.

Fixed, thanks.

> 
>> +}
>> +
>> int
>> main(void)
>> {
>> -        plan(2);
>> +        plan(3);
>> diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
>> index af4f2f5d5..411c56f71 100644
>> --- a/third_party/lua-yaml/lyaml.cc
>> +++ b/third_party/lua-yaml/lyaml.cc
>> @@ -697,10 +700,18 @@ static int dump_node(struct lua_yaml_dumper *dumper)
>>       switch (field.ext_type) {
>>       case MP_DECIMAL:
>>          str = decimal_to_string(field.decval);
>> -	 len = strlen(str);
>> -	 break;
>> +         len = strlen(str);
>> +         break;
>> +      case MP_UUID:
>> +      {
>> +         char *buf = tt_static_buf();
>> +         tt_uuid_to_string(field.uuidval, buf);
>> +         str = buf;
>> +         len = UUID_STR_LEN;
>> +         break;
>> +      }
>>       default:
>> -	 assert(0); /* checked by luaL_checkfield() */
>> +         assert(0); /* checked by luaL_checkfield() */
> 
> 10. Better avoid the unnecessary diff.
> 
>>       }
>>       break;
>>     }
>> 


--
Serge Petrenko
sergepetrenko@tarantool.org

Re: [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID

[Serge Petrenko]

> 6 апр. 2020 г., в 00:29, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
> 
> Thanks for the patch!


Hi! Thanks for the review!

> 
> See 6 comments below.
> 
> On 04/04/2020 01:02, Serge Petrenko wrote:
>> It is now possible to create an index over UUID values, either returned
>> by something like `uuid.new()` or 36- and 32- byte strings in format
>> "88ebbb9a-8c18-480f-bd04-dd5345f1573c" or "88ebbb9a8c18480fbd04dd5345f1573c".
>> 16-byte binary strings are also supported.
>> 
>> Closes #4268
>> Closes #2916
>> 
>> @TarantoolBot document
>> Title: Document uuid field type.
>> 
>> There's a new field type -- UUID, it accepts values returned by
>> `uuid.new()`, as well as strings representing uuids, like
>> `88ebbb9a-8c18-480f-bd04-dd5345f1573c` and
>> `88ebbb9a8c18480fbd04dd5345f1573c` and 16 byte binary values, encoded in
>> msgpack.
>> 
>> The index may be either unique or non-unique, nullable or non-nullable,
>> and may be a primary key.
>> 
>> To create an index over a uuid field for space `test`, say:
>> ```
>> box.space.test:create_index("pk", {parts={1, 'uuid'}})
>> ```
>> Now you may insert uuids into the space:
>> ```
>> tarantool> box.space.test:insert{uuid.new()}
>> ---
>> - [e631fdcc-0e8a-4d2f-83fd-b0ce6762b13f]
>> ...
>> 
>> tarantool> box.space.test:insert{"64d22e4d-ac92-4a23-899a-e59f34af5479"}
>> ---
>> - ['64d22e4d-ac92-4a23-899a-e59f34af5479']
>> ...
>> 
>> tarantool> box.space.test:insert{"856dc1177dcc49969f1407f8c6c8a371"}
>> ---
>> - ['856dc1177dcc49969f1407f8c6c8a371']
>> ...
>> 
>> tarantool> box.space.test:select{}
>> ---
>> - - ['64d22e4d-ac92-4a23-899a-e59f34af5479']
>>  - ['856dc1177dcc49969f1407f8c6c8a371']
>>  - [e631fdcc-0e8a-4d2f-83fd-b0ce6762b13f]
> 
> 1. Wait. Why are they printed differently? They should be all the
> same 36 strings when you print them, no?
> 
> Seems like you understood Mons to the letter. I believe he meant
> we should provide a way to insert strings to simplify moving data
> to a new space, but it does not mean we should store them as strings.
> It makes comparators slower. sscanf() to extract uuid from a string
> on *each* comparison is a huge performance hole. From what I measured
> recently (not in Tarantool), sscanf() is a *really* slow thing. Its
> usage in comparators is dangerous.
> 
> On the other hand ability to insert strings means we need to convert
> them to MP_EXT after insertion, which means full scan of each tuple
> by field types and possibly its recreation. We had a similar problem
> with 'double' type. After all it was decided to keep as is and let
> users enforce the needed type when they want using ffi.cast('double').
> 
> IMO, we should just disallow both ability to store strings, and to
> insert strings. I mean, you can't insert a string into a decimal field,
> for example. So a user anyway will never be able to insert only Lua
> simple types. Cdata is enforced for decimal and double right now.
> 
> Better keep it simple. If someone wants to convert their existing spaces
> to mp uuid, they easily can write a trivial function which would replace
> string uuid with cdata uuid before insertion into the new space. We
> should not sacrifice perf for that.
> 
> This is only my opinion, you may want to ask Mons and Kirill.

So, I consulted. Mons, he’s ok with allowing only cdata.

> 
> 
> 2. It is worth mentioning comparison order. Are they compared as
> strings, or individual uuid fields are compared? In the latter case
> in what order the fields are compared?

Ok, mentioned in commit message. The order’s lexicographical.
> 
> Also probably string comparison vs by-field comparison vs memcmp
> comparison will be the same in case fields are stored in msgpack
> in the same order as in the string, and in big-endian. I didn't check
> but it is worth investigation. It would be good to be able to use
> memcmp.
> 
>> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc
>> index 3f8a0ce24..aad69340d 100644
>> --- a/src/box/tuple_compare.cc
>> +++ b/src/box/tuple_compare.cc
>> @@ -378,6 +382,61 @@ mp_compare_bin(const char *field_a, const char *field_b)
>> 	return COMPARE_RESULT(size_a, size_b);
>> }
>> 
>> +static int
>> +mp_compare_uuid_with_type(const char *field_a, enum mp_type type_a,
>> +			  const char *field_b, enum mp_type type_b)
>> +{
>> +	struct tt_uuid uuid_a, uuid_b;
>> +	struct tt_uuid *ret;
>> +	const char *str;
>> +	uint32_t len;
>> +	int rc;
>> +	switch (type_a) {
>> +	case MP_STR:
>> +		str = mp_decode_str(&field_a, &len);
>> +		rc = tt_uuid_from_lstring(str, len, &uuid_a);
> 
> 3. This is perf killer. Basically it would be faster to store
> uuids as a string, or just as 16 bytes binary. Because memcmp
> could be used directly then.

Switched to memcmp.

> 
>> +		assert(rc == 0);
>> +		break;
>> +	case MP_BIN:
>> +		str = mp_decode_bin(&field_a, &len);
>> +		ret = uuid_unpack(&str, len, &uuid_a);
>> +		assert(ret != NULL);
>> +		break;
>> +	case MP_EXT:
>> +		ret = mp_decode_uuid(&field_a, &uuid_a);
>> +		assert(ret != NULL);
>> +		break;
>> +	default:
>> +		unreachable();
>> +	}
>> +	switch (type_b) {
>> +	case MP_STR:
>> +		str = mp_decode_str(&field_b, &len);
>> +		rc = tt_uuid_from_lstring(str, len, &uuid_b);
>> +		assert(rc == 0);
>> +		break;
>> +	case MP_BIN:
>> +		str = mp_decode_bin(&field_b, &len);
>> +		ret = uuid_unpack(&str, len, &uuid_b);
>> +		assert(ret != NULL);
>> +		break;
>> +	case MP_EXT:
>> +		ret = mp_decode_uuid(&field_b, &uuid_b);
>> +		assert(ret != NULL);
>> +		break;
>> +	default:
>> +		unreachable();
>> +	}
>> +	return tt_uuid_compare(&uuid_a, &uuid_b);
> 
> 4. If we will decide to store uuids as MP_EXT, it may be
> useful to introduce a function mp_compare_uuid, by analogue
> with other mp_compare_* functions. To compare them directly
> using memcmp, without unpacking. But see above my comment
> about order. memcmp validness depends on how uuid is stored
> in MessagePack.
> 

Done. uuid fields are stored in big-endian manner and in the same
order they’re compared, so memcmp is valid.

>> +}
>> +
>> +static inline int
>> +mp_compare_uuid(const char *field_a, const char *field_b)
>> +{
>> +	return mp_compare_uuid_with_type(field_a, mp_typeof(*field_a),
>> +					 field_b, mp_typeof(*field_b));
>> +}
>> @@ -1578,6 +1642,21 @@ hint_decimal(decimal_t *dec)
>> 	return hint_create(MP_CLASS_NUMBER, val);
>> }
>> 
>> +static inline hint_t
>> +hint_uuid(struct tt_uuid *uuid)
>> +{
>> +	/* Simply take the first part of the UUID as hint. */
> 
> 5. Why only first?

I meant, take the ‘high’ part of the UUID. The one which’s compared first.

> 
>> +	uint64_t val = 0;
>> +	val |= uuid->time_low;
>> +	val <<= sizeof(uuid->time_mid) * CHAR_BIT;
>> +	val |= uuid->time_mid;
>> +	val <<= sizeof(uuid->time_hi_and_version) * CHAR_BIT;
>> +	val |= uuid->time_hi_and_version;
>> +	/* Make space for class representation. */
>> +	val >>= HINT_CLASS_BITS;
>> +	return hint_create(MP_CLASS_UUID, val);
>> +}
>> diff --git a/test/engine/gh-4268-uuid.result b/test/engine/gh-4268-uuid.result
>> new file mode 100644
>> index 000000000..928204507
>> --- /dev/null
>> +++ b/test/engine/gh-4268-uuid.result
> 
> 6. Since this is a feature, we probably should not call the
> test file using name pattern used for bugs. Just uuid.test.lua.

Done.


--
Serge Petrenko
sergepetrenko@tarantool.org

Re: [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID

[Vladislav Shpilevoy]

>>> +}
>>> +
>>> +static inline int
>>> +mp_compare_uuid(const char *field_a, const char *field_b)
>>> +{
>>> +	return mp_compare_uuid_with_type(field_a, mp_typeof(*field_a),
>>> +					 field_b, mp_typeof(*field_b));
>>> +}
>>> @@ -1578,6 +1642,21 @@ hint_decimal(decimal_t *dec)
>>> 	return hint_create(MP_CLASS_NUMBER, val);
>>> }
>>>
>>> +static inline hint_t
>>> +hint_uuid(struct tt_uuid *uuid)
>>> +{
>>> +	/* Simply take the first part of the UUID as hint. */
>>
>> 5. Why only first?
> 
> I meant, take the ‘high’ part of the UUID. The one which’s compared first.

The current hint is good. I forgot hints should be ordered.

Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors

[Vladislav Shpilevoy]

Hi! Thanks for the fixes!

>> 6 апр. 2020 г., в 00:22, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
>>
>> Thanks for the patch!
>>
>>> diff --git a/src/lib/uuid/tt_uuid.c b/src/lib/uuid/tt_uuid.c
>>> index 1bd2e2cfe..94a0b15bb 100644
>>> --- a/src/lib/uuid/tt_uuid.c
>>> +++ b/src/lib/uuid/tt_uuid.c
>>> @@ -65,6 +65,15 @@ tt_uuid_create(struct tt_uuid *uu)
>>> }
>>> #endif
>>>
>>> +extern inline int
>>
>> 'inline' modifier won't change anything. The function body is
>> not visible anyway.
> 
> Not sure about that.
> What about other functions?
> 
> ```
> extern inline int                                                                   
> tt_uuid_from_string(const char *in, struct tt_uuid *uu);
> ```
> 
> I’m still not sure what ‘extern inline’ does, but I googled a stackoverflow
> question discussing it:
> https://stackoverflow.com/questions/216510/what-does-extern-inline-do

From what I understood by the link above and here: http://m68hc11.serveftp.org/inline-1.php
it looks like 'extern inline' does not make any sense for function
declaration. It should be used only for function definition in a
header file.

Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors

[Serge Petrenko]

> 10 апр. 2020 г., в 19:56, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
> 
> Hi! Thanks for the fixes!

Thanks for the reivew!

> 
>>> 6 апр. 2020 г., в 00:22, Vladislav Shpilevoy <v.shpilevoy@tarantool.org> написал(а):
>>> 
>>> Thanks for the patch!
>>> 
>>>> diff --git a/src/lib/uuid/tt_uuid.c b/src/lib/uuid/tt_uuid.c
>>>> index 1bd2e2cfe..94a0b15bb 100644
>>>> --- a/src/lib/uuid/tt_uuid.c
>>>> +++ b/src/lib/uuid/tt_uuid.c
>>>> @@ -65,6 +65,15 @@ tt_uuid_create(struct tt_uuid *uu)
>>>> }
>>>> #endif
>>>> 
>>>> +extern inline int
>>> 
>>> 'inline' modifier won't change anything. The function body is
>>> not visible anyway.
>> 
>> Not sure about that.
>> What about other functions?
>> 
>> ```
>> extern inline int                                                                   
>> tt_uuid_from_string(const char *in, struct tt_uuid *uu);
>> ```
>> 
>> I’m still not sure what ‘extern inline’ does, but I googled a stackoverflow
>> question discussing it:
>> https://stackoverflow.com/questions/216510/what-does-extern-inline-do
> 
> From what I understood by the link above and here: http://m68hc11.serveftp.org/inline-1.php
> it looks like 'extern inline' does not make any sense for function
> declaration. It should be used only for function definition in a
> header file.

Here’s what I’ve found:
https://www.greenend.org.uk/rjk/tech/inline.html

```
A C99 model. Use inline in a common header, and provide definitions in a .c file somewhere, via extern declarations. For instance, in the header file:

inline int max(int a, int b) {
  return a > b ? a : b;
}

...and in exactly one source file:

#include "header.h"
extern int max(int a, int b);

```
So, AFAIU, if the compiler chooses to not inline the function, `extern inline`
will make it generate the object code for the function only in `tt_uuid.c.o`.
As I understood, the same effect can be reached by omitting `extern inline`,
and leaving only.
```
int
tt_uuid_validate(...
``` 

But let’s stick with `extern inline` for consistency with other declarations in tt_uuid.c

--
Serge Petrenko
sergepetrenko@tarantool.org