From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 654434696C3 for ; Sat, 11 Apr 2020 17:14:59 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.4 \(3608.80.23.2.2\)) From: Serge Petrenko In-Reply-To: <2df5574a-10b4-c086-ce76-6d8b5df3aef4@tarantool.org> Date: Sat, 11 Apr 2020 17:14:58 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <2df5574a-10b4-c086-ce76-6d8b5df3aef4@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 4/4] box: introduce indices by UUID List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: tarantool-patches@dev.tarantool.org > 10 =D0=B0=D0=BF=D1=80. 2020 =D0=B3., =D0=B2 19:56, Vladislav Shpilevoy = =D0=BD=D0=B0=D0=BF=D0=B8=D1=81=D0=B0=D0=BB(=D0= =B0): >=20 > Thanks for the patch! >=20 Hi! Thanks for the review! > The patchset is almost perfect, just a few nits are left. > See 2 of them below. >=20 >> diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc >> index 3f8a0ce24..a47f7ac6d 100644 >> --- a/src/box/tuple_compare.cc >> +++ b/src/box/tuple_compare.cc >> @@ -378,6 +382,25 @@ mp_compare_bin(const char *field_a, const char = *field_b) >> return COMPARE_RESULT(size_a, size_b); >> } >>=20 >> +static inline int >> +mp_compare_uuid(const char *field_a, const char *field_b) >> +{ >> + const char *str_a, *str_b; >> + int8_t type; >> + uint32_t len; >> + str_a =3D mp_decode_ext(&field_a, &type, &len); >> + assert(type =3D=3D MP_UUID && len =3D=3D UUID_PACKED_LEN); >> + str_b =3D mp_decode_ext(&field_b, &type, &len); >> + assert(type =3D=3D MP_UUID && len =3D=3D UUID_PACKED_LEN); >=20 > 1. I would either do field_a +=3D 2, field_b +=3D 2; or just > memcmp(field_a, field_b), because the same prefix won't > affect the result. Up to you. +2 is the fastest solution I > think. >=20 > +2 version could be even moved to mp_uuid.h as something like > mp_strip_uuid_header() or mp_decode_ext_uuidl() (similar to > mp_decode_strl()). Point is we eliminate lots of instructions > and switch-case mp_decode_extl(). Lets just do field_a + 2, field_b + 2. >=20 >> + /* >> + * Packed uuid fields are in the right order for >> + * comparison and are big-endian, so memcmp is >> + * the same as tt_uuid_compare() and lets us >> + * spare 2 mp_uuid_unpack() calls. >> + */ >> + return memcmp(str_a, str_b, UUID_PACKED_LEN); >> +}> diff --git a/test/engine/uuid.result b/test/engine/uuid.result >> new file mode 100644 >> index 000000000..c4c186e92 >> --- /dev/null >> +++ b/test/engine/uuid.result >> @@ -0,0 +1,55 @@ >> +-- test-run result file version 2 >> +env =3D require('test_run') >> + | --- >> + | ... >> +test_run =3D env.new() >> + | --- >> + | ... >> +engine =3D test_run:get_cfg('engine') >> + | --- >> + | ... >> + >> +uuid =3D require('uuid') >> + | --- >> + | ... >> +ffi =3D require('ffi') >> + | --- >> + | ... >> + >> +-- check uuid indices >=20 > 2. Lets mention the ticket here, and >=20 > check -> Check > indices -> indices. Ok. >=20 >> +_ =3D box.schema.space.create('test', {engine=3Dengine}) >> + | --- >> + | ... >> +_ =3D box.space.test:create_index('pk', {parts=3D{1,'uuid'}}) >> + | --- >> + | ... diff --git a/src/box/field_def.c b/src/box/field_def.c index 82a2493fa..213e91699 100644 --- a/src/box/field_def.c +++ b/src/box/field_def.c @@ -33,7 +33,7 @@ #include "trivia/util.h" #include "key_def.h" #include "mp_extension_types.h" -#include "lib/uuid/mp_uuid.h" +#include "uuid/mp_uuid.h" #include "uuid/tt_uuid.h" =20 const char *mp_type_strs[] =3D { diff --git a/src/box/tuple_compare.cc b/src/box/tuple_compare.cc index a47f7ac6d..bc01fe068 100644 --- a/src/box/tuple_compare.cc +++ b/src/box/tuple_compare.cc @@ -35,7 +35,7 @@ #include #include "lib/core/decimal.h" #include "lib/core/mp_decimal.h" -#include "lib/uuid/mp_uuid.h" +#include "uuid/mp_uuid.h" #include "lib/core/mp_extension_types.h" =20 /* {{{ tuple_compare */ @@ -385,20 +385,14 @@ mp_compare_bin(const char *field_a, const char = *field_b) static inline int mp_compare_uuid(const char *field_a, const char *field_b) { - const char *str_a, *str_b; - int8_t type; - uint32_t len; - str_a =3D mp_decode_ext(&field_a, &type, &len); - assert(type =3D=3D MP_UUID && len =3D=3D UUID_PACKED_LEN); - str_b =3D mp_decode_ext(&field_b, &type, &len); - assert(type =3D=3D MP_UUID && len =3D=3D UUID_PACKED_LEN); /* * Packed uuid fields are in the right order for * comparison and are big-endian, so memcmp is * the same as tt_uuid_compare() and lets us * spare 2 mp_uuid_unpack() calls. + * "field_a + 2" to skip the uuid header. */ - return memcmp(str_a, str_b, UUID_PACKED_LEN); + return memcmp(field_a + 2, field_b + 2, UUID_PACKED_LEN); } =20 typedef int (*mp_compare_f)(const char *, const char *); diff --git a/test/engine/uuid.test.lua b/test/engine/uuid.test.lua index f34795ce4..34e74e68b 100644 --- a/test/engine/uuid.test.lua +++ b/test/engine/uuid.test.lua @@ -5,7 +5,7 @@ engine =3D test_run:get_cfg('engine') uuid =3D require('uuid') ffi =3D require('ffi') =20 --- check uuid indices +-- Check uuid indices (gh-4268). _ =3D box.schema.space.create('test', {engine=3Dengine}) _ =3D box.space.test:create_index('pk', {parts=3D{1,'uuid'}}) =20 -- Serge Petrenko sergepetrenko@tarantool.org