* [Tarantool-patches] [PATCH 0/4] introduce indices over UUID
@ 2020-04-03 23:02 Serge Petrenko
2020-04-03 23:02 ` [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo Serge Petrenko
` (4 more replies)
0 siblings, 5 replies; 17+ messages in thread
From: Serge Petrenko @ 2020-04-03 23:02 UTC (permalink / raw)
To: v.shpilevoy; +Cc: tarantool-patches
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)
^ permalink raw reply [flat|nested] 17+ messages in thread* [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo 2020-04-03 23:02 [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Serge Petrenko @ 2020-04-03 23:02 ` Serge Petrenko 2020-04-05 21:22 ` Vladislav Shpilevoy 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors Serge Petrenko ` (3 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Serge Petrenko @ 2020-04-03 23:02 UTC (permalink / raw) To: v.shpilevoy; +Cc: tarantool-patches --- 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) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo Serge Petrenko @ 2020-04-05 21:22 ` Vladislav Shpilevoy 0 siblings, 0 replies; 17+ messages in thread From: Vladislav Shpilevoy @ 2020-04-05 21:22 UTC (permalink / raw) To: Serge Petrenko; +Cc: tarantool-patches 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) > */ > ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors 2020-04-03 23:02 [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Serge Petrenko 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo Serge Petrenko @ 2020-04-03 23:02 ` Serge Petrenko 2020-04-05 21:22 ` Vladislav Shpilevoy 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID Serge Petrenko ` (2 subsequent siblings) 4 siblings, 1 reply; 17+ messages in thread From: Serge Petrenko @ 2020-04-03 23:02 UTC (permalink / raw) To: v.shpilevoy; +Cc: tarantool-patches 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) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors Serge Petrenko @ 2020-04-05 21:22 ` Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko 0 siblings, 1 reply; 17+ messages in thread From: Vladislav Shpilevoy @ 2020-04-05 21:22 UTC (permalink / raw) To: Serge Petrenko; +Cc: tarantool-patches 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors 2020-04-05 21:22 ` Vladislav Shpilevoy @ 2020-04-09 23:46 ` Serge Petrenko 2020-04-10 16:56 ` Vladislav Shpilevoy 0 siblings, 1 reply; 17+ messages in thread From: Serge Petrenko @ 2020-04-09 23:46 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors 2020-04-09 23:46 ` Serge Petrenko @ 2020-04-10 16:56 ` Vladislav Shpilevoy 2020-04-11 13:35 ` Serge Petrenko 0 siblings, 1 reply; 17+ messages in thread From: Vladislav Shpilevoy @ 2020-04-10 16:56 UTC (permalink / raw) To: Serge Petrenko; +Cc: tarantool-patches 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors 2020-04-10 16:56 ` Vladislav Shpilevoy @ 2020-04-11 13:35 ` Serge Petrenko 0 siblings, 0 replies; 17+ messages in thread From: Serge Petrenko @ 2020-04-11 13:35 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tml > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID 2020-04-03 23:02 [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Serge Petrenko 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo Serge Petrenko 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors Serge Petrenko @ 2020-04-03 23:02 ` Serge Petrenko 2020-04-05 21:26 ` Vladislav Shpilevoy 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID Serge Petrenko 2020-04-05 21:21 ` [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Vladislav Shpilevoy 4 siblings, 1 reply; 17+ messages in thread From: Serge Petrenko @ 2020-04-03 23:02 UTC (permalink / raw) To: v.shpilevoy; +Cc: tarantool-patches 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) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID Serge Petrenko @ 2020-04-05 21:26 ` Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko 0 siblings, 1 reply; 17+ messages in thread From: Vladislav Shpilevoy @ 2020-04-05 21:26 UTC (permalink / raw) To: Serge Petrenko; +Cc: tarantool-patches 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; > } > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID 2020-04-05 21:26 ` Vladislav Shpilevoy @ 2020-04-09 23:46 ` Serge Petrenko 0 siblings, 0 replies; 17+ messages in thread From: Serge Petrenko @ 2020-04-09 23:46 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID 2020-04-03 23:02 [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Serge Petrenko ` (2 preceding siblings ...) 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID Serge Petrenko @ 2020-04-03 23:02 ` Serge Petrenko 2020-04-05 21:29 ` Vladislav Shpilevoy 2020-04-05 21:21 ` [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Vladislav Shpilevoy 4 siblings, 1 reply; 17+ messages in thread From: Serge Petrenko @ 2020-04-03 23:02 UTC (permalink / raw) To: v.shpilevoy; +Cc: tarantool-patches 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) ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID Serge Petrenko @ 2020-04-05 21:29 ` Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko 0 siblings, 1 reply; 17+ messages in thread From: Vladislav Shpilevoy @ 2020-04-05 21:29 UTC (permalink / raw) To: Serge Petrenko; +Cc: tarantool-patches 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID 2020-04-05 21:29 ` Vladislav Shpilevoy @ 2020-04-09 23:46 ` Serge Petrenko 2020-04-10 16:56 ` Vladislav Shpilevoy 0 siblings, 1 reply; 17+ messages in thread From: Serge Petrenko @ 2020-04-09 23:46 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tml > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID 2020-04-09 23:46 ` Serge Petrenko @ 2020-04-10 16:56 ` Vladislav Shpilevoy 0 siblings, 0 replies; 17+ messages in thread From: Vladislav Shpilevoy @ 2020-04-10 16:56 UTC (permalink / raw) To: Serge Petrenko; +Cc: tml >>> +} >>> + >>> +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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/4] introduce indices over UUID 2020-04-03 23:02 [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Serge Petrenko ` (3 preceding siblings ...) 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID Serge Petrenko @ 2020-04-05 21:21 ` Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko 4 siblings, 1 reply; 17+ messages in thread From: Vladislav Shpilevoy @ 2020-04-05 21:21 UTC (permalink / raw) To: Serge Petrenko; +Cc: tarantool-patches 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Tarantool-patches] [PATCH 0/4] introduce indices over UUID 2020-04-05 21:21 ` [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Vladislav Shpilevoy @ 2020-04-09 23:46 ` Serge Petrenko 0 siblings, 0 replies; 17+ messages in thread From: Serge Petrenko @ 2020-04-09 23:46 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches > 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2020-04-11 13:35 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-04-03 23:02 [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Serge Petrenko 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 1/4] decimal: fix comment typo Serge Petrenko 2020-04-05 21:22 ` Vladislav Shpilevoy 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 2/4] uuid: expose additional from_string constructors Serge Petrenko 2020-04-05 21:22 ` Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko 2020-04-10 16:56 ` Vladislav Shpilevoy 2020-04-11 13:35 ` Serge Petrenko 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 3/4] box: add MsgPack encoding/decoding for UUID Serge Petrenko 2020-04-05 21:26 ` Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko 2020-04-03 23:02 ` [Tarantool-patches] [PATCH 4/4] box: introduce indices by UUID Serge Petrenko 2020-04-05 21:29 ` Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko 2020-04-10 16:56 ` Vladislav Shpilevoy 2020-04-05 21:21 ` [Tarantool-patches] [PATCH 0/4] introduce indices over UUID Vladislav Shpilevoy 2020-04-09 23:46 ` Serge Petrenko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox