From: Serge Petrenko via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Oleg Babin <olegrok@tarantool.org>, vdavydov@tarantool.org, sergos@tarantool.org Cc: tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method Date: Mon, 16 Aug 2021 10:53:27 +0300 [thread overview] Message-ID: <bec3b2c4-d2e4-69cc-f02c-bf4732d3fa65@tarantool.org> (raw) In-Reply-To: <ee49016b-143a-3639-8ac4-f176e44ce901@tarantool.org> 13.08.2021 23:13, Oleg Babin пишет: > Thanks for your fixes. Seems I found one more issue. > > ``` > > mt = {__eq = function(a, b) return a.a == b.a end} > > t1 = setmetatable({a = 3}, mt) > > t2 = setmetatable({a = 3}, mt) > > tarantool> t1 == t2 -- metamethod is triggered > --- > - true > ... > > tarantool> t1 == {a = 3} -- tables have different metatables > --- > - false > ... > > tarantool> {a = 3} == t1 > --- > - false > ... > > tarantool> table.equals(t1, t2) -- OK > --- > - true > ... > > tarantool> table.equals(t1, {a = 3}) -- Seems also ok > --- > - false > ... > > tarantool> table.equals({a = 3}, t1) -- It's strange > --- > - true > ... > > ``` > > > Seems you can use two tables if they have the same metatable. > > Probably here it's described better > https://stackoverflow.com/questions/32637684/equality-operator-on-mixed-types-in-lua > > Thanks for a thorough review! I've force pushed the following fixes: ============================================= diff --git a/src/lua/table.lua b/src/lua/table.lua index 3d5e69e97..edd60d1be 100644 --- a/src/lua/table.lua +++ b/src/lua/table.lua @@ -65,8 +65,11 @@ local function table_equals(a, b) if type(a) ~= 'table' or type(b) ~= 'table' then return type(a) == type(b) and a == b end - local mt = getmetatable(a) - if mt and mt.__eq then + local mta = getmetatable(a) + local mtb = getmetatable(b) + -- Let Lua decide what should happen when at least one of the tables has a + -- metatable. + if mta and mta.__eq or mtb and mtb.__eq then return a == b end for k, v in pairs(a) do diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua index ec81593f3..029b923fb 100755 --- a/test/app-tap/table.test.lua +++ b/test/app-tap/table.test.lua @@ -8,7 +8,7 @@ yaml.cfg{ encode_invalid_as_nil = true, } local test = require('tap').test('table') -test:plan(40) +test:plan(43) do -- check basic table.copy (deepcopy) local example_table = { @@ -254,6 +254,14 @@ do -- check table.equals test:ok(table.equals(tbl_a, tbl_c), "table.equals for shallow copied tables after modification") test:is(table.equals(tbl_a, tbl_b), false, "table.equals does a deep check") + local mt = {__eq = function(a, b) return true end} + local tbl_d = setmetatable({a=15}, mt) + local tbl_e = setmetatable({b=2, c=3}, mt) + test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq") + test:is(table.equals(tbl_d, {a=15}), false, + "table.equals when metatables don't match") + test:is(table.equals({a=15}, tbl_d), false, + "table.equals when metatables don't match") end os.exit(test:check() == true and 0 or 1) ============================================= > > On 13.08.2021 13:22, Serge Petrenko wrote: >> >> >> 13.08.2021 08:30, Oleg Babin пишет: >>> Thanks for your patch. >>> >>> >>> It seems it works in quite strange way: >>> >>> ``` >>> >>> tarantool> table.equals({a = box.NULL}, {}) >>> --- >>> - true >>> ... >>> >>> tarantool> table.equals({}, {a = box.NULL}) >>> --- >>> - false >>> ... >>> >>> ``` >>> >>> >>> I can change arguments order to get different result. Expected false >>> in both cases. >>> >>> For tap it was considered as buggy behaviour >>> https://github.com/tarantool/tarantool/issues/4125 >>> >>> >> >> ==================================== >> >> Good catch! Thanks! >> >> Check out the diff: >> >> diff --git a/src/lua/table.lua b/src/lua/table.lua >> index 5f35a30f6..3d5e69e97 100644 >> --- a/src/lua/table.lua >> +++ b/src/lua/table.lua >> @@ -63,7 +63,7 @@ end >> -- @return true when the two tables are equal (false otherwise). >> local function table_equals(a, b) >> if type(a) ~= 'table' or type(b) ~= 'table' then >> - return a == b >> + return type(a) == type(b) and a == b >> end >> local mt = getmetatable(a) >> if mt and mt.__eq then >> diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua >> index a3c9aa123..ec81593f3 100755 >> --- a/test/app-tap/table.test.lua >> +++ b/test/app-tap/table.test.lua >> @@ -8,7 +8,7 @@ yaml.cfg{ >> encode_invalid_as_nil = true, >> } >> local test = require('tap').test('table') >> -test:plan(38) >> +test:plan(40) >> >> do -- check basic table.copy (deepcopy) >> local example_table = { >> @@ -227,6 +227,10 @@ do -- check table.equals >> test:ok(table.equals({}, {}), "table.equals for empty tables") >> test:is(table.equals({}, {1}), false, "table.equals with one >> empty table") >> test:is(table.equals({1}, {}), false, "table.equals with one >> empty table") >> + test:is(table.equals({key = box.NULL}, {key = nil}), false, >> + "table.equals for box.NULL and nil") >> + test:is(table.equals({key = nil}, {key = box.NULL}), false, >> + "table.equals for box.NULL and nil") >> local tbl_a = { >> first = { >> 1, >> >> ==================================== >>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote: >>>> Introduce table.equals for comparing tables. >>>> The method respects __eq metamethod, if provided. >>>> >>>> Needed-for #5894 >>>> --- >>>> >> -- Serge Petrenko
next prev parent reply other threads:[~2021-08-16 7:53 UTC|newest] Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-08-12 23:30 [Tarantool-patches] [PATCH v2 0/2] allow upgrading from version 1.6 Serge Petrenko via Tarantool-patches 2021-08-12 23:30 ` [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method Serge Petrenko via Tarantool-patches 2021-08-13 5:30 ` Oleg Babin via Tarantool-patches 2021-08-13 10:22 ` Serge Petrenko via Tarantool-patches 2021-08-13 20:13 ` Oleg Babin via Tarantool-patches 2021-08-16 7:53 ` Serge Petrenko via Tarantool-patches [this message] 2021-08-16 13:03 ` Бабин Олег via Tarantool-patches 2021-08-16 15:36 ` Serge Petrenko via Tarantool-patches 2021-08-16 15:41 ` Бабин Олег via Tarantool-patches 2021-08-16 16:47 ` Serge Petrenko via Tarantool-patches 2021-08-13 11:41 ` Vladimir Davydov via Tarantool-patches 2021-08-12 23:30 ` [Tarantool-patches] [PATCH v2 2/2] box: allow upgrading from version 1.6 Serge Petrenko via Tarantool-patches 2021-08-13 11:40 ` Vladimir Davydov via Tarantool-patches 2021-08-16 13:18 ` Бабин Олег via Tarantool-patches 2021-08-16 16:32 ` Serge Petrenko via Tarantool-patches 2021-08-16 18:22 ` Бабин Олег via Tarantool-patches 2021-08-14 8:12 ` [Tarantool-patches] [PATCH v2 0/2] " Vitaliia Ioffe via Tarantool-patches 2021-08-17 7:28 ` Kirill Yukhin via Tarantool-patches 2021-08-17 8:05 ` Kirill Yukhin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=bec3b2c4-d2e4-69cc-f02c-bf4732d3fa65@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=olegrok@tarantool.org \ --cc=sergepetrenko@tarantool.org \ --cc=sergos@tarantool.org \ --cc=vdavydov@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v2 1/2] lua: introduce table.equals method' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox