From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 9319B469719 for ; Tue, 17 Nov 2020 00:36:45 +0300 (MSK) References: <20201111200408.49148-1-olegrok@tarantool.org> From: Vladislav Shpilevoy Message-ID: <1b1c65f6-2ac5-0112-a5b9-ffa8f94b1b71@tarantool.org> Date: Mon, 16 Nov 2020 22:36:43 +0100 MIME-Version: 1.0 In-Reply-To: <20201111200408.49148-1-olegrok@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] lua: introduce function for uuid comparison List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: olegrok@tarantool.org, lvasiliev@tarantool.org Cc: tarantool-patches@dev.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 > > 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.