From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <imun@tarantool.org>
Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149])
 (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 0F6A8469710
 for <tarantool-patches@dev.tarantool.org>;
 Tue, 24 Nov 2020 18:20:46 +0300 (MSK)
Date: Tue, 24 Nov 2020 18:20:43 +0300
From: Igor Munkin <imun@tarantool.org>
Message-ID: <20201124152043.GE14086@tarantool.org>
References: <cover.1605685304.git.babinoleg@mail.ru>
 <f2b6f003939881319d086af9738190b869693eb9.1605685304.git.babinoleg@mail.ru>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <f2b6f003939881319d086af9738190b869693eb9.1605685304.git.babinoleg@mail.ru>
Subject: Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of
 uuid values in Lua
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: olegrok@tarantool.org
Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org

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.

> 
> 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.

> +

<snipped>

> -- 
> 2.29.0
> 

-- 
Best regards,
IM