Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua

[Oleg Babin <olegrok@tarantool.org>]

Thanks for your review!

On 24/11/2020 18:20, Igor Munkin wrote:
> Oleg,
>
> Thanks for the patch! Considering the version on the remote branch, LGTM
> except a single nit below.
>
> On 18.11.20,olegrok@tarantool.org  wrote:
>> From: Oleg Babin<babinoleg@mail.ru>
>>
>> Since Tarantool has uuid data type sometimes we want to compare
>> uuid vaues as it's possible for primitive types and decimals. This
>> patch exports function for uuid comparison and implements le and
>> lt metamethods for uuid type.
> Minor: Strictly saying, you're right, but AFAICS we usually use the key
> name (i.e. __le and __lt) for metamethods in description.

Fixed. Description is updated, branch is force-pushed. New description:

     Since Tarantool has uuid data type sometimes we want to compare
     uuid values as it's possible for primitive types and decimals. This
     patch exports function for uuid comparison and implements __le and
     __lt metamethods for uuid type.


>> Closes #5511
>>
>> @TarantoolBot document
>> Title: uuid comparison is supported
>>
>> Currently comparison between uuid values is supported.
>> Example:
>> ```lua
>> u1 = uuid.fromstr('aaaaaaaa-aaaa-4000-b000-000000000001')
>> u2 = uuid.fromstr('bbbbbbbb-bbbb-4000-b000-000000000001')
>>
>> u1 > u2  -- false
>> u1 >= u2 -- false
>> u1 <= u2 -- true
>> u1 < u2  -- true
>> ```
>> ---
>> Issue:https://github.com/tarantool/tarantool/issues/5511
>> Branch:https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2
>>
>>   src/exports.h          |  1 +
>>   src/lua/uuid.lua       | 25 +++++++++++++
>>   test/app/uuid.result   | 79 ++++++++++++++++++++++++++++++++++++++++++
>>   test/app/uuid.test.lua | 29 ++++++++++++++++
>>   4 files changed, 134 insertions(+)
>>
> <snipped>
>
>> diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua
>> index 42016601d..08991cfeb 100644
>> --- a/src/lua/uuid.lua
>> +++ b/src/lua/uuid.lua
> <snipped>
>
>> +local uuid_lt = function(lhs, rhs)
>> +    return uuid_cmp(lhs, rhs) < 0
>> +end
>> +local uuid_le = function(lhs, rhs)
>> +    return uuid_cmp(lhs, rhs) <= 0
>> +end
> Side note: I see no reason to define these oneline functions (you can
> simply store an anonymous function right to the metatable), but I see
> them everywhere in Tarantool sources, so feel free to ignore this note.

Let's keep general style of this module and define comparators as local 
functions. I stay them as is.