Re: [Tarantool-patches] [PATCH] lua: introduce function for uuid comparison

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

Hi! Thanks for the patch!

Lets change subsystem from 'lua:' to 'uuid:'. The former is too
general.

See 4 comments below.

On 11.11.2020 21:04, 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.
> To be consistent with decimal type this patch allows to compare
> uuid with uuid string representation.
> 
> @TarantoolBot document Title: uuid compare

1. I am afraid it won't work this way. You need to have 'Title' on
separate line. You can see if your comment works here:
https://tarantool-docbot.herokuapp.com, right after you pushed the
commit.

> diff --git a/src/exports.h b/src/exports.h
> index 867a027dc..ffbb84e3b 100644
> --- a/src/exports.h
> +++ b/src/exports.h
> @@ -508,6 +508,7 @@ EXPORT(tnt_iconv_close)
>  EXPORT(tnt_iconv_open)
>  EXPORT(tt_uuid_bswap)
>  EXPORT(tt_uuid_create)
> +EXPORT(tt_uuid_compare)
>  EXPORT(tt_uuid_from_string)
>  EXPORT(tt_uuid_is_equal)
>  EXPORT(tt_uuid_is_nil)
> diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua
> index 42016601d..0ab937877 100644
> --- a/src/lua/uuid.lua
> +++ b/src/lua/uuid.lua
> @@ -118,9 +120,38 @@ local uuid_new_str = function()
>      return uuid_tostring(uuidbuf)
>  end
>  
> +local check_uuid = function(value, index)
> +    if is_uuid(value) then
> +        return value
> +    end
> +
> +    if type(value) == 'string' then
> +        value = uuid_fromstr(value)

2. This may be expensive to do for each comparison with a string.
If you care, try to use static_alloc so as not to allocate the
temporary tt_uuid on the heap. It is already used in this file,
you can check out some usage examples.

> +        if value ~= nil then
> +            return value
> +        end
> +    end
> +
> +    error(('incorrect value to convert to uuid as %d argument'):format(index), 0)

3. Out of 80 symbols.

> +end
> +
> +local uuid_cmp = function(lhs, rhs)
> +    lhs = check_uuid(lhs, 1)
> +    rhs = check_uuid(rhs, 2)
> +    return builtin.tt_uuid_compare(lhs, rhs)
> +end
> +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

4. Hmmmmm

tarantool> u1 = 'b0b681e9-dd43-4341-98b4-a973b23f1421'
---
...

tarantool> u2 = uuid()
---
...

tarantool> type(u2)
---
- cdata
...

tarantool> type(u1)
---
- string
...

tarantool> u2 < u1
---
- true
...

tarantool> u1 > u2
---
- true
...

Doesn't this look wrong?

From what I see here: http://lua-users.org/wiki/MetamethodsTutorial
they say, that for '<' Lua uses metatable of the left object. It means,
that if string is on the left, it will use string metamethods. Although
I may be wrong, and you need to check how it actually works. If it is
true, I am afraid we can't support string vs cdata-uuid comparison.
Only cdata-uuid vs cdata-uuid. For string case you may need to add a new
function. Probably in your own application.