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

[Vladislav Shpilevoy <v.shpilevoy@tarantool.org>]

Thanks for the patch!

The patchset is almost perfect, just a few nits are left.
See 2 of them below.

> 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);
>  }
>  
> +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 = mp_decode_ext(&field_a, &type, &len);
> +	assert(type == MP_UUID && len == UUID_PACKED_LEN);
> +	str_b = mp_decode_ext(&field_b, &type, &len);
> +	assert(type == MP_UUID && len == UUID_PACKED_LEN);

1. I would either do field_a += 2, field_b += 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.

+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().

> +	/*
> +	 * 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 = require('test_run')
> + | ---
> + | ...
> +test_run = env.new()
> + | ---
> + | ...
> +engine = test_run:get_cfg('engine')
> + | ---
> + | ...
> +
> +uuid = require('uuid')
> + | ---
> + | ...
> +ffi = require('ffi')
> + | ---
> + | ...
> +
> +-- check uuid indices

2. Lets mention the ticket here, and

    check -> Check
    indices -> indices.

> +_ = box.schema.space.create('test', {engine=engine})
> + | ---
> + | ...
> +_ = box.space.test:create_index('pk', {parts={1,'uuid'}})
> + | ---
> + | ...