* [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable @ 2020-11-18 7:56 olegrok 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua olegrok ` (5 more replies) 0 siblings, 6 replies; 21+ messages in thread From: olegrok @ 2020-11-18 7:56 UTC (permalink / raw) To: v.shpilevoy, lvasiliev; +Cc: tarantool-patches From: Oleg Babin <babinoleg@mail.ru> This patchset makes uuid values comparable. The first one allows to compare only uuid values. It's just define lt and le methods for uuid values. The second one allows to compare uuid values with string representations of uuid. Just note that this patch breaks backward compatibility - before all attempts to check equality with non-uuid values returned false. Currently it's not so. It we want do that this patch could be omitted. Issue: https://github.com/tarantool/tarantool/issues/5511 Branch: https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2 Changes with v1: - Use static_alloc when string is converted to uuid to compare - Style fixes - Extend "eq" for comparison with string - Split patch in two parts Oleg Babin (2): uuid: support comparison of uuid values in Lua uuid: support uuid comparison with strings src/exports.h | 1 + src/lua/uuid.lua | 52 ++++++++++++++- test/app/uuid.result | 148 +++++++++++++++++++++++++++++++++++++++++ test/app/uuid.test.lua | 56 ++++++++++++++++ 4 files changed, 255 insertions(+), 2 deletions(-) -- 2.29.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-18 7:56 [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable olegrok @ 2020-11-18 7:56 ` olegrok 2020-11-21 15:17 ` Vladislav Shpilevoy 2020-11-24 15:20 ` Igor Munkin 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings olegrok ` (4 subsequent siblings) 5 siblings, 2 replies; 21+ messages in thread From: olegrok @ 2020-11-18 7:56 UTC (permalink / raw) To: v.shpilevoy, lvasiliev; +Cc: tarantool-patches 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. 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(+) 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..08991cfeb 100644 --- a/src/lua/uuid.lua +++ b/src/lua/uuid.lua @@ -19,6 +19,8 @@ bool tt_uuid_is_equal(const struct tt_uuid *lhs, const struct tt_uuid *rhs); char * tt_uuid_str(const struct tt_uuid *uu); +int +tt_uuid_compare(const struct tt_uuid *a, const struct tt_uuid *b); extern const struct tt_uuid uuid_nil; ]] @@ -118,9 +120,32 @@ local uuid_new_str = function() return uuid_tostring(uuidbuf) end +local check_uuid = function(value, index) + if is_uuid(value) then + return value + end + + local err_fmt = 'incorrect value to compare with uuid as %d argument' + error(err_fmt:format(index), 0) +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 + local uuid_mt = { __tostring = uuid_tostring; __eq = uuid_eq; + __lt = uuid_lt; + __le = uuid_le; __index = { isnil = uuid_isnil; bin = uuid_tobin; -- binary host byteorder diff --git a/test/app/uuid.result b/test/app/uuid.result index 9fe0e7fb4..e06331001 100644 --- a/test/app/uuid.result +++ b/test/app/uuid.result @@ -291,6 +291,85 @@ uuid.is_uuid(require('decimal').new('123')) --- - false ... +-- +-- gh-5511: allow to compare uuid values +-- +u1 = uuid.fromstr('aaaaaaaa-aaaa-4000-b000-000000000001') +--- +... +u2 = uuid.fromstr('bbbbbbbb-bbbb-4000-b000-000000000001') +--- +... +u1 > u1 +--- +- false +... +u1 >= u1 +--- +- true +... +u1 <= u1 +--- +- true +... +u1 < u1 +--- +- false +... +u1 > u2 +--- +- false +... +u1 >= u2 +--- +- false +... +u1 <= u2 +--- +- true +... +u1 < u2 +--- +- true +... +u1 < 1 +--- +- error: incorrect value to compare with uuid as 2 argument +... +u1 <= 1 +--- +- error: incorrect value to compare with uuid as 2 argument +... +u1 < 'abc' +--- +- error: incorrect value to compare with uuid as 2 argument +... +u1 <= 'abc' +--- +- error: incorrect value to compare with uuid as 2 argument +... +1 < u1 +--- +- error: incorrect value to compare with uuid as 1 argument +... +1 <= u1 +--- +- error: incorrect value to compare with uuid as 1 argument +... +'abc' < u1 +--- +- error: incorrect value to compare with uuid as 1 argument +... +'abc' <= u1 +--- +- error: incorrect value to compare with uuid as 1 argument +... +u1 = nil +--- +... +u2 = nil +--- +... uuid = nil --- ... diff --git a/test/app/uuid.test.lua b/test/app/uuid.test.lua index 47a96f3c6..34ab38d35 100644 --- a/test/app/uuid.test.lua +++ b/test/app/uuid.test.lua @@ -108,6 +108,35 @@ uuid.is_uuid(uuid.new():str()) uuid.is_uuid(1) uuid.is_uuid(require('decimal').new('123')) +-- +-- gh-5511: allow to compare uuid values +-- + +u1 = uuid.fromstr('aaaaaaaa-aaaa-4000-b000-000000000001') +u2 = uuid.fromstr('bbbbbbbb-bbbb-4000-b000-000000000001') + +u1 > u1 +u1 >= u1 +u1 <= u1 +u1 < u1 + +u1 > u2 +u1 >= u2 +u1 <= u2 +u1 < u2 + +u1 < 1 +u1 <= 1 +u1 < 'abc' +u1 <= 'abc' +1 < u1 +1 <= u1 +'abc' < u1 +'abc' <= u1 + +u1 = nil +u2 = nil + uuid = nil test_run:cmd("clear filter") -- 2.29.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua olegrok @ 2020-11-21 15:17 ` Vladislav Shpilevoy 2020-11-21 19:07 ` Oleg Babin 2020-11-24 15:20 ` Igor Munkin 1 sibling, 1 reply; 21+ messages in thread From: Vladislav Shpilevoy @ 2020-11-21 15:17 UTC (permalink / raw) To: olegrok, lvasiliev; +Cc: tarantool-patches Hi! Thanks for the patch! See 3 comments below. On 18.11.2020 08:56, 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 1. vaues -> values. > 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(+) > > 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) 2. Please, keep the lines sorted, as it is asked in the first lines of this file. 'r' > 'o', so tt_uuid_compare should be before tt_uuid_create. > 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..08991cfeb 100644 > --- a/src/lua/uuid.lua > +++ b/src/lua/uuid.lua > @@ -118,9 +120,32 @@ local uuid_new_str = function() > return uuid_tostring(uuidbuf) > end > > +local check_uuid = function(value, index) > + if is_uuid(value) then > + return value > + end > + > + local err_fmt = 'incorrect value to compare with uuid as %d argument' > + error(err_fmt:format(index), 0) 3. Why did you use 0? You loose the error location then. I changed it to 4, and got more informative messages, showing the error location: [001] 'abc' <= u1 [001] --- [001] -- error: incorrect value to compare with uuid as 1 argument [001] +- error: '[string "return ''abc'' <= u1 "]:1: incorrect value to compare with uuid [001] + as 1 argument' I also noticed just now, that we already have uuid_eq, which looks different. I think you should unify the comparators, while you are here. Otherwise eq has its own 'check_uuid', and here you add a second one. It is inconsistent, even error messages look different. Also you anyway change it in the next commit, so it should not increase diff much. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-21 15:17 ` Vladislav Shpilevoy @ 2020-11-21 19:07 ` Oleg Babin 2020-11-23 21:58 ` Vladislav Shpilevoy 0 siblings, 1 reply; 21+ messages in thread From: Oleg Babin @ 2020-11-21 19:07 UTC (permalink / raw) To: Vladislav Shpilevoy, lvasiliev; +Cc: tarantool-patches Thanks for your comments. I've force-pushed fixes to my branch (https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2). See my answers below. On 21/11/2020 18:17, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > See 3 comments below. > > On 18.11.2020 08:56,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 > 1. vaues -> values. Thanks! Fixed. Consider a new commit message. I moved docbot request to the next patch. ``` uuid: support comparison of uuid values in Lua 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 ``` >> 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(+) >> >> 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) > 2. Please, keep the lines sorted, as it is asked in the > first lines of this file. 'r' > 'o', so tt_uuid_compare > should be before tt_uuid_create. My bad. Fixed. >> 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..08991cfeb 100644 >> --- a/src/lua/uuid.lua >> +++ b/src/lua/uuid.lua >> @@ -118,9 +120,32 @@ local uuid_new_str = function() >> return uuid_tostring(uuidbuf) >> end >> >> +local check_uuid = function(value, index) >> + if is_uuid(value) then >> + return value >> + end >> + >> + local err_fmt = 'incorrect value to compare with uuid as %d argument' >> + error(err_fmt:format(index), 0) > 3. Why did you use 0? You loose the error location then. > I changed it to 4, and got more informative messages, showing > the error location: > > [001] 'abc' <= u1 > [001] --- > [001] -- error: incorrect value to compare with uuid as 1 argument > [001] +- error: '[string "return ''abc'' <= u1 "]:1: incorrect value to compare with uuid > [001] + as 1 argument' > > I also noticed just now, that we already have uuid_eq, which looks > different. I think you should unify the comparators, while you are > here. Otherwise eq has its own 'check_uuid', and here you add a second > one. It is inconsistent, even error messages look different. Also you > anyway change it in the next commit, so it should not increase diff > much. Agree. Consider new diff diff --git a/src/exports.h b/src/exports.h index ffbb84e3b..2116036fa 100644 --- a/src/exports.h +++ b/src/exports.h @@ -507,8 +507,8 @@ EXPORT(tnt_iconv) EXPORT(tnt_iconv_close) EXPORT(tnt_iconv_open) EXPORT(tt_uuid_bswap) -EXPORT(tt_uuid_create) EXPORT(tt_uuid_compare) +EXPORT(tt_uuid_create) 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 08991cfeb..275f4e5c5 100644 --- a/src/lua/uuid.lua +++ b/src/lua/uuid.lua @@ -93,13 +93,20 @@ local uuid_isnil = function(uu) return builtin.tt_uuid_is_nil(uu) end +local check_uuid = function(value, index) + if is_uuid(value) then + return value + end + + local err_fmt = 'incorrect value to compare with uuid as %d argument' + error(err_fmt:format(index), 4) +end + local uuid_eq = function(lhs, rhs) if not is_uuid(rhs) then return false end - if not is_uuid(lhs) then - return error('Usage: uuid == var') - end + lhs = check_uuid(lhs, 1) return builtin.tt_uuid_is_equal(lhs, rhs) end @@ -120,15 +127,6 @@ local uuid_new_str = function() return uuid_tostring(uuidbuf) end -local check_uuid = function(value, index) - if is_uuid(value) then - return value - end - - local err_fmt = 'incorrect value to compare with uuid as %d argument' - error(err_fmt:format(index), 0) -end - local uuid_cmp = function(lhs, rhs) lhs = check_uuid(lhs, 1) rhs = check_uuid(rhs, 2) diff --git a/test/app/uuid.result b/test/app/uuid.result index e06331001..b70cb0e32 100644 --- a/test/app/uuid.result +++ b/test/app/uuid.result @@ -334,35 +334,41 @@ u1 < u2 ... u1 < 1 --- -- error: incorrect value to compare with uuid as 2 argument +- error: '[string "return u1 < 1 "]:1: incorrect value to compare with uuid as 2 argument' ... u1 <= 1 --- -- error: incorrect value to compare with uuid as 2 argument +- error: '[string "return u1 <= 1 "]:1: incorrect value to compare with uuid as 2 + argument' ... u1 < 'abc' --- -- error: incorrect value to compare with uuid as 2 argument +- error: '[string "return u1 < ''abc'' "]:1: incorrect value to compare with uuid + as 2 argument' ... u1 <= 'abc' --- -- error: incorrect value to compare with uuid as 2 argument +- error: '[string "return u1 <= ''abc'' "]:1: incorrect value to compare with uuid + as 2 argument' ... 1 < u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: '[string "return 1 < u1 "]:1: incorrect value to compare with uuid as 1 argument' ... 1 <= u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: '[string "return 1 <= u1 "]:1: incorrect value to compare with uuid as 1 + argument' ... 'abc' < u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: '[string "return ''abc'' < u1 "]:1: incorrect value to compare with uuid + as 1 argument' ... 'abc' <= u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: '[string "return ''abc'' <= u1 "]:1: incorrect value to compare with uuid + as 1 argument' ... u1 = nil --- ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-21 19:07 ` Oleg Babin @ 2020-11-23 21:58 ` Vladislav Shpilevoy 2020-11-24 5:57 ` Oleg Babin 2020-11-27 15:17 ` Oleg Babin 0 siblings, 2 replies; 21+ messages in thread From: Vladislav Shpilevoy @ 2020-11-23 21:58 UTC (permalink / raw) To: Oleg Babin, lvasiliev; +Cc: tarantool-patches Hi! Thanks for the fixes! > diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua > index 08991cfeb..275f4e5c5 100644 > --- a/src/lua/uuid.lua > +++ b/src/lua/uuid.lua > @@ -93,13 +93,20 @@ local uuid_isnil = function(uu) > return builtin.tt_uuid_is_nil(uu) > end > > +local check_uuid = function(value, index) > + if is_uuid(value) then > + return value > + end > + > + local err_fmt = 'incorrect value to compare with uuid as %d argument' > + error(err_fmt:format(index), 4) > +end > + > local uuid_eq = function(lhs, rhs) > if not is_uuid(rhs) then > return false > end > - if not is_uuid(lhs) then > - return error('Usage: uuid == var') > - end > + lhs = check_uuid(lhs, 1) Check_uuid() here uses incorrect error index. check_uuid() raises error(..., 4). So check_uuid() itself is level 1. uuid_eq() is level 2. And the calling code is level 3. You will raise one level beyond the caller code. For uuid_lt and uuid_le it works, because check_uuid() is level 1, uuid_cmp() is level 2, uuid_lt/le() is level 3, and the calling code is level 4. But I can't give you a test, since I have no idea how to provide lhs with non-uuid type. Igor said we can give metatable of uuid to some non-uuid value, but I failed to assign uuid metatable to anything else. After you fix this (but I don't insist on adding a test, because I don't know how), I suggest you to get a second review from Igor. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-23 21:58 ` Vladislav Shpilevoy @ 2020-11-24 5:57 ` Oleg Babin 2020-11-24 13:06 ` Igor Munkin 2020-11-27 15:17 ` Oleg Babin 1 sibling, 1 reply; 21+ messages in thread From: Oleg Babin @ 2020-11-24 5:57 UTC (permalink / raw) To: Vladislav Shpilevoy, Igor Munkin; +Cc: tarantool-patches Hi! Thanks for comments! On 24/11/2020 00:58, Vladislav Shpilevoy wrote: > Hi! Thanks for the fixes! > >> diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua >> index 08991cfeb..275f4e5c5 100644 >> --- a/src/lua/uuid.lua >> +++ b/src/lua/uuid.lua >> @@ -93,13 +93,20 @@ local uuid_isnil = function(uu) >> return builtin.tt_uuid_is_nil(uu) >> end >> >> +local check_uuid = function(value, index) >> + if is_uuid(value) then >> + return value >> + end >> + >> + local err_fmt = 'incorrect value to compare with uuid as %d argument' >> + error(err_fmt:format(index), 4) >> +end >> + >> local uuid_eq = function(lhs, rhs) >> if not is_uuid(rhs) then >> return false >> end >> - if not is_uuid(lhs) then >> - return error('Usage: uuid == var') >> - end >> + lhs = check_uuid(lhs, 1) > Check_uuid() here uses incorrect error index. check_uuid() > raises error(..., 4). So check_uuid() itself is level 1. > uuid_eq() is level 2. And the calling code is level 3. You > will raise one level beyond the caller code. > > For uuid_lt and uuid_le it works, because check_uuid() is > level 1, uuid_cmp() is level 2, uuid_lt/le() is level 3, and > the calling code is level 4. Good catch! Thanks. Fixed in the following way: diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua index 275f4e5c5..1171dfcc4 100644 --- a/src/lua/uuid.lua +++ b/src/lua/uuid.lua @@ -93,20 +93,20 @@ local uuid_isnil = function(uu) return builtin.tt_uuid_is_nil(uu) end -local check_uuid = function(value, index) +local check_uuid = function(value, index, err_lvl) if is_uuid(value) then return value end local err_fmt = 'incorrect value to compare with uuid as %d argument' - error(err_fmt:format(index), 4) + error(err_fmt:format(index), err_lvl) end local uuid_eq = function(lhs, rhs) if not is_uuid(rhs) then return false end - lhs = check_uuid(lhs, 1) + lhs = check_uuid(lhs, 1, 3) return builtin.tt_uuid_is_equal(lhs, rhs) end @@ -128,8 +128,8 @@ local uuid_new_str = function() end local uuid_cmp = function(lhs, rhs) - lhs = check_uuid(lhs, 1) - rhs = check_uuid(rhs, 2) + lhs = check_uuid(lhs, 1, 4) + rhs = check_uuid(rhs, 2, 4) return builtin.tt_uuid_compare(lhs, rhs) end local uuid_lt = function(lhs, rhs) > But I can't give you a test, since I have no idea how to provide > lhs with non-uuid type. Igor said we can give metatable of uuid to > some non-uuid value, but I failed to assign uuid metatable to > anything else. It's possible but I don't think it's really needed. I put small snippet below: uuid = require('uuid') eq = debug.getmetatable(uuid()).__eq eq('', uuid()) -- error: incorrect value to compare with uuid as 1 argument proxy = newproxy() debug.setmetatable(proxy, {__eq = eq}) proxy == uuid() -- error: '[string "return proxy == uuid()"]:1: incorrect value to compare with uuid as 1 argument' I can add it to our test suite but it looks really strange > After you fix this (but I don't insist on adding a test, because > I don't know how), I suggest you to get a second review from Igor. Ok. I add Igor to CC. Igor, could you provide the second review? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-24 5:57 ` Oleg Babin @ 2020-11-24 13:06 ` Igor Munkin 0 siblings, 0 replies; 21+ messages in thread From: Igor Munkin @ 2020-11-24 13:06 UTC (permalink / raw) To: Oleg Babin; +Cc: tarantool-patches, Vladislav Shpilevoy Oleg, This is not a review, I just want to clarify Vlad's point here. I'll make a review in a separate reply. On 24.11.20, Oleg Babin wrote: > Hi! Thanks for comments! > > On 24/11/2020 00:58, Vladislav Shpilevoy wrote: <snipped> > > > But I can't give you a test, since I have no idea how to provide > > lhs with non-uuid type. Igor said we can give metatable of uuid to > > some non-uuid value, but I failed to assign uuid metatable to > > anything else. > > It's possible but I don't think it's really needed. I put small snippet > below: > > uuid = require('uuid') > eq = debug.getmetatable(uuid()).__eq > eq('', uuid()) -- error: incorrect value to compare with uuid as 1 argument > proxy = newproxy() > debug.setmetatable(proxy, {__eq = eq}) > proxy == uuid() -- error: '[string "return proxy == uuid()"]:1: > incorrect value to compare with uuid as 1 argument' > > I can add it to our test suite but it looks really strange Unfortunately this simply doesn't work the way you want. Consider the following: | $ ./src/tarantool | Tarantool 2.7.0-42-g374917337 | type 'help' for interactive help | tarantool> require('ffi').typeof(box.error.new(box.error.UNKNOWN)) | --- | - ctype<const struct error &> | ... | | tarantool> require('ffi').typeof(require('uuid')()) | --- | - ctype<struct tt_uuid> | ... | | tarantool> debug.getmetatable(box.error.new(box.error.UNKNOWN)) == debug.getmetatable(require('uuid')()) | --- | - true | ... | | tarantool> Oops... It occurs ctype<const struct error &> shares the same metatable as ctype<struct tt_uuid>, doesn't it? Yes, it does. At this place FFI conforms Lua reference manual section[1] describing the metatables: | Tables and full userdata have individual metatables (although multiple | tables and userdata can share their metatables). Values of all other | types share one single metatable per type; that is, there is one single | metatable for all numbers, one for all strings, etc. I guess there is no legal (i.e. using only Lua with no LuaJIT internals) way to check that the function you obtained in the test is <lj_cf_ffi_meta___eq> fast function (although, you can check yourself via luajit-gdb.py), so just trust me. */me making Jedi mind tricks here* Anyway, how does this magic work? We need to distinguish *metatables* and *metatypes* -- the latter one is specific only for GCcdata. The metatype metamethod is chosen underneath <lj_cf_ffi_meta___eq> considering the ctype passed via the one of the arguments. I've already described metatype metamethods resolution in the previous reply. There is uuid in both of the tests above, so we can see only the error related to the miscomparison, but AFAIU Vlad asked about the error raised on check_uuid (with the "Usage: blah-blah" error message). The current implementation provides no way to reproduce it (maybe via other debug.* interfaces, I haven't checked this yet). > <snipped> > [1]: https://www.lua.org/manual/5.1/manual.html#2.8 -- Best regards, IM ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-23 21:58 ` Vladislav Shpilevoy 2020-11-24 5:57 ` Oleg Babin @ 2020-11-27 15:17 ` Oleg Babin 1 sibling, 0 replies; 21+ messages in thread From: Oleg Babin @ 2020-11-27 15:17 UTC (permalink / raw) To: Vladislav Shpilevoy; +Cc: tarantool-patches Hi! On 24/11/2020 00:58, Vladislav Shpilevoy wrote: > After you fix this (but I don't insist on adding a test, because > I don't know how), I suggest you to get a second review from Igor. Vlad, I've got LGTM for the patchset from Igor. Could you please continue your review? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua olegrok 2020-11-21 15:17 ` Vladislav Shpilevoy @ 2020-11-24 15:20 ` Igor Munkin 2020-11-24 19:23 ` Oleg Babin 1 sibling, 1 reply; 21+ messages in thread From: Igor Munkin @ 2020-11-24 15:20 UTC (permalink / raw) To: olegrok; +Cc: tarantool-patches, v.shpilevoy 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 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua 2020-11-24 15:20 ` Igor Munkin @ 2020-11-24 19:23 ` Oleg Babin 0 siblings, 0 replies; 21+ messages in thread From: Oleg Babin @ 2020-11-24 19:23 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches, v.shpilevoy 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. ^ permalink raw reply [flat|nested] 21+ messages in thread
* [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings 2020-11-18 7:56 [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable olegrok 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua olegrok @ 2020-11-18 7:56 ` olegrok 2020-11-21 15:17 ` Vladislav Shpilevoy 2020-11-24 15:20 ` Igor Munkin 2020-11-18 8:02 ` [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable Oleg Babin ` (3 subsequent siblings) 5 siblings, 2 replies; 21+ messages in thread From: olegrok @ 2020-11-18 7:56 UTC (permalink / raw) To: v.shpilevoy, lvasiliev; +Cc: tarantool-patches From: Oleg Babin <babinoleg@mail.ru> Before this patch it was impossible to compare uuid values with string representations of uuid. However we have cases when such comparisons is possible (e.g. "decimal" where we can compare decimal values with strings and numbers). This patch extends uuid comparators (eq, lt, le) and every string argument is tried to be converted to uuid value to compare then. Follow-up #5511 @TarantoolBot document Title: uuid values could be compared with strings Currently it's possible to compare uuid values with its string representations: ```lua u1_str = 'aaaaaaaa-aaaa-4000-b000-000000000001' u1 = uuid.fromstr(u1_str) u2_str = 'bbbbbbbb-bbbb-4000-b000-000000000001' u1 == u1_str -- true u1 == u2_str -- false u1 >= u1_str -- true u1 < u2_str -- true ``` --- Issue: https://github.com/tarantool/tarantool/issues/5511 Branch: https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2 src/lua/uuid.lua | 31 +++++++++++++-- test/app/uuid.result | 85 ++++++++++++++++++++++++++++++++++++++---- test/app/uuid.test.lua | 27 ++++++++++++++ 3 files changed, 131 insertions(+), 12 deletions(-) diff --git a/src/lua/uuid.lua b/src/lua/uuid.lua index 08991cfeb..1d5d835df 100644 --- a/src/lua/uuid.lua +++ b/src/lua/uuid.lua @@ -93,11 +93,33 @@ local uuid_isnil = function(uu) return builtin.tt_uuid_is_nil(uu) end +local fromstr = function(str) + local uu = static_alloc('struct tt_uuid') + local rc = builtin.tt_uuid_from_string(str, uu) + if rc ~= 0 then + return nil + end + return uu +end + +local to_uuid = function(value) + if is_uuid(value) then + return value + end + if type(value) == 'string' then + return fromstr(value) + end + return nil +end + local uuid_eq = function(lhs, rhs) - if not is_uuid(rhs) then + rhs = to_uuid(rhs) + if rhs == nil then return false end - if not is_uuid(lhs) then + + lhs = to_uuid(lhs) + if lhs == nil then return error('Usage: uuid == var') end return builtin.tt_uuid_is_equal(lhs, rhs) @@ -121,11 +143,12 @@ local uuid_new_str = function() end local check_uuid = function(value, index) - if is_uuid(value) then + value = to_uuid(value) + if value ~= nil then return value end - local err_fmt = 'incorrect value to compare with uuid as %d argument' + local err_fmt = 'incorrect value to convert to uuid as %d argument' error(err_fmt:format(index), 0) end diff --git a/test/app/uuid.result b/test/app/uuid.result index e06331001..5b9ffa230 100644 --- a/test/app/uuid.result +++ b/test/app/uuid.result @@ -334,35 +334,35 @@ u1 < u2 ... u1 < 1 --- -- error: incorrect value to compare with uuid as 2 argument +- error: incorrect value to convert to uuid as 2 argument ... u1 <= 1 --- -- error: incorrect value to compare with uuid as 2 argument +- error: incorrect value to convert to uuid as 2 argument ... u1 < 'abc' --- -- error: incorrect value to compare with uuid as 2 argument +- error: incorrect value to convert to uuid as 2 argument ... u1 <= 'abc' --- -- error: incorrect value to compare with uuid as 2 argument +- error: incorrect value to convert to uuid as 2 argument ... 1 < u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: incorrect value to convert to uuid as 1 argument ... 1 <= u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: incorrect value to convert to uuid as 1 argument ... 'abc' < u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: incorrect value to convert to uuid as 1 argument ... 'abc' <= u1 --- -- error: incorrect value to compare with uuid as 1 argument +- error: incorrect value to convert to uuid as 1 argument ... u1 = nil --- @@ -370,6 +370,75 @@ u1 = nil u2 = nil --- ... +-- +-- allow to compare uuid values with strings +-- +u1_str = 'aaaaaaaa-aaaa-4000-b000-000000000001' +--- +... +u1 = uuid.fromstr(u1_str) +--- +... +u2_str = 'bbbbbbbb-bbbb-4000-b000-000000000001' +--- +... +u1 == u1_str +--- +- true +... +u1 == u2_str +--- +- false +... +u1_str == u1 +--- +- true +... +u2_str == u1 +--- +- false +... +u1 > u1_str +--- +- false +... +u1 >= u1_str +--- +- true +... +u1 < u1_str +--- +- false +... +u1 <= u1_str +--- +- true +... +u1 > u2_str +--- +- false +... +u1 >= u2_str +--- +- false +... +u1 < u2_str +--- +- true +... +u1 <= u2_str +--- +- true +... +u1 = nil +--- +... +u1_str = nil +--- +... +u2_str = nil +--- +... uuid = nil --- ... diff --git a/test/app/uuid.test.lua b/test/app/uuid.test.lua index 34ab38d35..867bbd832 100644 --- a/test/app/uuid.test.lua +++ b/test/app/uuid.test.lua @@ -137,6 +137,33 @@ u1 <= 'abc' u1 = nil u2 = nil +-- +-- allow to compare uuid values with strings +-- + +u1_str = 'aaaaaaaa-aaaa-4000-b000-000000000001' +u1 = uuid.fromstr(u1_str) +u2_str = 'bbbbbbbb-bbbb-4000-b000-000000000001' + +u1 == u1_str +u1 == u2_str +u1_str == u1 +u2_str == u1 + +u1 > u1_str +u1 >= u1_str +u1 < u1_str +u1 <= u1_str + +u1 > u2_str +u1 >= u2_str +u1 < u2_str +u1 <= u2_str + +u1 = nil +u1_str = nil +u2_str = nil + uuid = nil test_run:cmd("clear filter") -- 2.29.0 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings olegrok @ 2020-11-21 15:17 ` Vladislav Shpilevoy 2020-11-21 19:07 ` Oleg Babin 2020-11-24 15:20 ` Igor Munkin 1 sibling, 1 reply; 21+ messages in thread From: Vladislav Shpilevoy @ 2020-11-21 15:17 UTC (permalink / raw) To: olegrok, lvasiliev; +Cc: tarantool-patches Thanks for the patch! See 2 comments below. On 18.11.2020 08:56, olegrok@tarantool.org wrote: > From: Oleg Babin <babinoleg@mail.ru> > > Before this patch it was impossible to compare uuid values with > string representations of uuid. However we have cases when such > comparisons is possible (e.g. "decimal" where we can compare > decimal values with strings and numbers). > > This patch extends uuid comparators (eq, lt, le) and every string > argument is tried to be converted to uuid value to compare then. > > Follow-up #5511 > > @TarantoolBot document > Title: uuid values could be compared with strings > > Currently it's possible to compare uuid values with its string > representations: > ```lua > u1_str = 'aaaaaaaa-aaaa-4000-b000-000000000001' > u1 = uuid.fromstr(u1_str) > u2_str = 'bbbbbbbb-bbbb-4000-b000-000000000001' > > u1 == u1_str -- true > u1 == u2_str -- false > > u1 >= u1_str -- true > u1 < u2_str -- true > ``` 1. Better use a single docbot request. It will be simpler for the doc team to handle it, since both the updates are about the same place. But up to you. > --- > Issue: https://github.com/tarantool/tarantool/issues/5511 > Branch: https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2 > > diff --git a/test/app/uuid.result b/test/app/uuid.result > index e06331001..5b9ffa230 100644 > --- a/test/app/uuid.result > +++ b/test/app/uuid.result 2. I don't see a test for == with an incorrect string. Is it there? ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings 2020-11-21 15:17 ` Vladislav Shpilevoy @ 2020-11-21 19:07 ` Oleg Babin 0 siblings, 0 replies; 21+ messages in thread From: Oleg Babin @ 2020-11-21 19:07 UTC (permalink / raw) To: Vladislav Shpilevoy, lvasiliev; +Cc: tarantool-patches Hi! Thanks for your comments. See my answers below. On 21/11/2020 18:17, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 2 comments below. > > On 18.11.2020 08:56,olegrok@tarantool.org wrote: >> From: Oleg Babin<babinoleg@mail.ru> >> >> Before this patch it was impossible to compare uuid values with >> string representations of uuid. However we have cases when such >> comparisons is possible (e.g. "decimal" where we can compare >> decimal values with strings and numbers). >> >> This patch extends uuid comparators (eq, lt, le) and every string >> argument is tried to be converted to uuid value to compare then. >> >> Follow-up #5511 >> >> @TarantoolBot document >> Title: uuid values could be compared with strings >> >> Currently it's possible to compare uuid values with its string >> representations: >> ```lua >> u1_str = 'aaaaaaaa-aaaa-4000-b000-000000000001' >> u1 = uuid.fromstr(u1_str) >> u2_str = 'bbbbbbbb-bbbb-4000-b000-000000000001' >> >> u1 == u1_str -- true >> u1 == u2_str -- false >> >> u1 >= u1_str -- true >> u1 < u2_str -- true >> ``` > 1. Better use a single docbot request. It will be simpler for the doc > team to handle it, since both the updates are about the same place. > But up to you. Agree. I've merged them. ``` Title: uuid comparison rules 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 ``` Also it's possible to compare uuid values with its string representations: ```lua u1_str = 'aaaaaaaa-aaaa-4000-b000-000000000001' u1 = uuid.fromstr(u1_str) u2_str = 'bbbbbbbb-bbbb-4000-b000-000000000001' u1 == u1_str -- true u1 == u2_str -- false u1 >= u1_str -- true u1 < u2_str -- true ``` ``` >> --- >> Issue:https://github.com/tarantool/tarantool/issues/5511 >> Branch:https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2 >> >> diff --git a/test/app/uuid.result b/test/app/uuid.result >> index e06331001..5b9ffa230 100644 >> --- a/test/app/uuid.result >> +++ b/test/app/uuid.result > 2. I don't see a test for == with an incorrect string. Is it > there? Yes, it was added before https://github.com/tarantool/tarantool/blob/e23b14dc23d749d092295a9b84854e1d64b2db27/test/app/uuid.test.lua#L89 ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings olegrok 2020-11-21 15:17 ` Vladislav Shpilevoy @ 2020-11-24 15:20 ` Igor Munkin 2020-11-24 19:23 ` Oleg Babin 1 sibling, 1 reply; 21+ messages in thread From: Igor Munkin @ 2020-11-24 15:20 UTC (permalink / raw) To: olegrok; +Cc: tarantool-patches, v.shpilevoy 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> > > Before this patch it was impossible to compare uuid values with > string representations of uuid. However we have cases when such > comparisons is possible (e.g. "decimal" where we can compare > decimal values with strings and numbers). > > This patch extends uuid comparators (eq, lt, le) and every string Minor: Strictly saying, you're right, but AFAICS we usually use the key name (i.e. __eq, __le and __lt) for metamethods in description. > argument is tried to be converted to uuid value to compare then. > > Follow-up #5511 > > @TarantoolBot document > Title: uuid values could be compared with strings > > Currently it's possible to compare uuid values with its string > representations: > ```lua > u1_str = 'aaaaaaaa-aaaa-4000-b000-000000000001' > u1 = uuid.fromstr(u1_str) > u2_str = 'bbbbbbbb-bbbb-4000-b000-000000000001' > > u1 == u1_str -- true > u1 == u2_str -- false > > u1 >= u1_str -- true > u1 < u2_str -- true > ``` > --- > Issue: https://github.com/tarantool/tarantool/issues/5511 > Branch: https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2 > > src/lua/uuid.lua | 31 +++++++++++++-- > test/app/uuid.result | 85 ++++++++++++++++++++++++++++++++++++++---- > test/app/uuid.test.lua | 27 ++++++++++++++ > 3 files changed, 131 insertions(+), 12 deletions(-) > <snipped> > -- > 2.29.0 > -- Best regards, IM ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings 2020-11-24 15:20 ` Igor Munkin @ 2020-11-24 19:23 ` Oleg Babin 0 siblings, 0 replies; 21+ messages in thread From: Oleg Babin @ 2020-11-24 19:23 UTC (permalink / raw) To: Igor Munkin; +Cc: tarantool-patches, v.shpilevoy Thanks for 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> >> >> Before this patch it was impossible to compare uuid values with >> string representations of uuid. However we have cases when such >> comparisons is possible (e.g. "decimal" where we can compare >> decimal values with strings and numbers). >> >> This patch extends uuid comparators (eq, lt, le) and every string > Minor: Strictly saying, you're right, but AFAICS we usually use the key > name (i.e. __eq, __le and __lt) for metamethods in description. I've updated and force-pushed branch. I cite only changed part: This patch extends uuid comparators (__eq, __lt and __le) and every string argument is tried to be converted to uuid value to compare then. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable 2020-11-18 7:56 [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable olegrok 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua olegrok 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings olegrok @ 2020-11-18 8:02 ` Oleg Babin 2020-11-27 22:39 ` Vladislav Shpilevoy ` (2 subsequent siblings) 5 siblings, 0 replies; 21+ messages in thread From: Oleg Babin @ 2020-11-18 8:02 UTC (permalink / raw) To: v.shpilevoy, lvasiliev; +Cc: tarantool-patches @Changelog: - Allow to compare uuid values (gh-5511) ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable 2020-11-18 7:56 [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable olegrok ` (2 preceding siblings ...) 2020-11-18 8:02 ` [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable Oleg Babin @ 2020-11-27 22:39 ` Vladislav Shpilevoy 2020-11-27 22:40 ` Vladislav Shpilevoy 2020-12-07 23:24 ` Vladislav Shpilevoy 5 siblings, 0 replies; 21+ messages in thread From: Vladislav Shpilevoy @ 2020-11-27 22:39 UTC (permalink / raw) To: olegrok, lvasiliev; +Cc: tarantool-patches Hi! Thanks for the patch! LGTM. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable 2020-11-18 7:56 [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable olegrok ` (3 preceding siblings ...) 2020-11-27 22:39 ` Vladislav Shpilevoy @ 2020-11-27 22:40 ` Vladislav Shpilevoy 2020-12-04 8:13 ` Oleg Babin 2020-12-07 23:24 ` Vladislav Shpilevoy 5 siblings, 1 reply; 21+ messages in thread From: Vladislav Shpilevoy @ 2020-11-27 22:40 UTC (permalink / raw) To: olegrok, lvasiliev, Alexander V. Tikhonov; +Cc: tarantool-patches Sasha (Tikh.), please, take a look at CI and tell if we can push this. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable 2020-11-27 22:40 ` Vladislav Shpilevoy @ 2020-12-04 8:13 ` Oleg Babin 2020-12-07 23:04 ` Alexander V. Tikhonov 0 siblings, 1 reply; 21+ messages in thread From: Oleg Babin @ 2020-12-04 8:13 UTC (permalink / raw) To: Vladislav Shpilevoy, Alexander V. Tikhonov; +Cc: tarantool-patches Sasha, CI [1] on my branch [2] is green. Could you confirm we can push this patch? [1] https://gitlab.com/tarantool/tarantool/-/pipelines/220727298 [2] https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2 On 28/11/2020 01:40, Vladislav Shpilevoy wrote: > Sasha (Tikh.), please, take a look at CI and tell if we can push this. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable 2020-12-04 8:13 ` Oleg Babin @ 2020-12-07 23:04 ` Alexander V. Tikhonov 0 siblings, 0 replies; 21+ messages in thread From: Alexander V. Tikhonov @ 2020-12-07 23:04 UTC (permalink / raw) To: Oleg Babin; +Cc: tarantool-patches Hi Oleg, , thanks for the patch, as I see no new degradation found in gitlab-ci testing commit criteria pipeline, patch LGTM. On Fri, Dec 04, 2020 at 11:13:09AM +0300, Oleg Babin wrote: > Sasha, CI [1] on my branch [2] is green. Could you confirm we can push this > patch? > > > [1] https://gitlab.com/tarantool/tarantool/-/pipelines/220727298 > > [2] https://github.com/tarantool/tarantool/tree/olegrok/5511-uuid-cmp-v2 > > On 28/11/2020 01:40, Vladislav Shpilevoy wrote: > > Sasha (Tikh.), please, take a look at CI and tell if we can push this. ^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable 2020-11-18 7:56 [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable olegrok ` (4 preceding siblings ...) 2020-11-27 22:40 ` Vladislav Shpilevoy @ 2020-12-07 23:24 ` Vladislav Shpilevoy 5 siblings, 0 replies; 21+ messages in thread From: Vladislav Shpilevoy @ 2020-12-07 23:24 UTC (permalink / raw) To: olegrok, lvasiliev; +Cc: tarantool-patches Pushed to master. ^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2020-12-07 23:24 UTC | newest] Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-11-18 7:56 [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable olegrok 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 1/2] uuid: support comparison of uuid values in Lua olegrok 2020-11-21 15:17 ` Vladislav Shpilevoy 2020-11-21 19:07 ` Oleg Babin 2020-11-23 21:58 ` Vladislav Shpilevoy 2020-11-24 5:57 ` Oleg Babin 2020-11-24 13:06 ` Igor Munkin 2020-11-27 15:17 ` Oleg Babin 2020-11-24 15:20 ` Igor Munkin 2020-11-24 19:23 ` Oleg Babin 2020-11-18 7:56 ` [Tarantool-patches] [PATCH v2 2/2] uuid: support uuid comparison with strings olegrok 2020-11-21 15:17 ` Vladislav Shpilevoy 2020-11-21 19:07 ` Oleg Babin 2020-11-24 15:20 ` Igor Munkin 2020-11-24 19:23 ` Oleg Babin 2020-11-18 8:02 ` [Tarantool-patches] [PATCH v2 0/2] Make uuid values comparable Oleg Babin 2020-11-27 22:39 ` Vladislav Shpilevoy 2020-11-27 22:40 ` Vladislav Shpilevoy 2020-12-04 8:13 ` Oleg Babin 2020-12-07 23:04 ` Alexander V. Tikhonov 2020-12-07 23:24 ` Vladislav Shpilevoy
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox