<HTML><BODY><div>Thanks for your fixes.<br><br> <blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;">Понедельник, 16 августа 2021, 10:53 +03:00 от Serge Petrenko <sergepetrenko@tarantool.org>:<div id=""><div class="js-helper js-readmsg-msg"><div><div id="style_16291004090512195968_BODY"><br>Thanks for a thorough review!<br><br>I've force pushed the following fixes:<br><br>=============================================<br><br>diff --git a/src/lua/table.lua b/src/lua/table.lua<br>index 3d5e69e97..edd60d1be 100644<br>--- a/src/lua/table.lua<br>+++ b/src/lua/table.lua<br>@@ -65,8 +65,11 @@ local function table_equals(a, b)<br> if type(a) ~= 'table' or type(b) ~= 'table' then<br> return type(a) == type(b) and a == b<br> end<br>- local mt = getmetatable(a)<br>- if mt and mt.__eq then<br>+ local mta = getmetatable(a)<br>+ local mtb = getmetatable(b)<br>+ -- Let Lua decide what should happen when at least one of the<br>tables has a<br>+ -- metatable.<br>+ if mta and mta.__eq or mtb and mtb.__eq then</div></div></div></div></blockquote></div><div> </div><div>I’m not sure that it’s correct. I’ve added a reference to SO (<a href="https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables">https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables</a>) and it seems condition should be following</div><div>mta and mta.__eq and mta == mtb. Otherwise tables have different __eq metamathods and you should compare their content.</div><div> </div><div>I’m not sure that my proposal is completely correct and suggest to ask Igor about it.</div><div> </div><div><blockquote style="border-left:1px solid #0857A6; margin:10px; padding:0 0 0 10px;"><div><div class="js-helper js-readmsg-msg"><div><div> return a == b<br> end<br> for k, v in pairs(a) do<br>diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua<br>index ec81593f3..029b923fb 100755<br>--- a/test/app-tap/table.test.lua<br>+++ b/test/app-tap/table.test.lua<br>@@ -8,7 +8,7 @@ yaml.cfg{<br> encode_invalid_as_nil = true,<br> }<br> local test = require('tap').test('table')<br>-test:plan(40)<br>+test:plan(43)<br><br> do -- check basic table.copy (deepcopy)<br> local example_table = {<br>@@ -254,6 +254,14 @@ do -- check table.equals<br> test:ok(table.equals(tbl_a, tbl_c),<br> "table.equals for shallow copied tables after modification")<br> test:is(table.equals(tbl_a, tbl_b), false, "table.equals does a<br>deep check")<br>+ local mt = {__eq = function(a, b) return true end}<br>+ local tbl_d = setmetatable({a=15}, mt)<br>+ local tbl_e = setmetatable({b=2, c=3}, mt)<br>+ test:ok(table.equals(tbl_d, tbl_e), "table.equals respects __eq")<br>+ test:is(table.equals(tbl_d, {a=15}), false,<br>+ "table.equals when metatables don't match")<br>+ test:is(table.equals({a=15}, tbl_d), false,<br>+ "table.equals when metatables don't match")<br> end<br><br> os.exit(test:check() == true and 0 or 1)<br><br>=============================================<br><br>><br>> On 13.08.2021 13:22, Serge Petrenko wrote:<br>>><br>>><br>>> 13.08.2021 08:30, Oleg Babin пишет:<br>>>> Thanks for your patch.<br>>>><br>>>><br>>>> It seems it works in quite strange way:<br>>>><br>>>> ```<br>>>><br>>>> tarantool> table.equals({a = box.NULL}, {})<br>>>> ---<br>>>> - true<br>>>> ...<br>>>><br>>>> tarantool> table.equals({}, {a = box.NULL})<br>>>> ---<br>>>> - false<br>>>> ...<br>>>><br>>>> ```<br>>>><br>>>><br>>>> I can change arguments order to get different result. Expected false<br>>>> in both cases.<br>>>><br>>>> For tap it was considered as buggy behaviour<br>>>> <a href="https://github.com/tarantool/tarantool/issues/4125" target="_blank">https://github.com/tarantool/tarantool/issues/4125</a><br>>>><br>>>><br>>><br>>> ====================================<br>>><br>>> Good catch! Thanks!<br>>><br>>> Check out the diff:<br>>><br>>> diff --git a/src/lua/table.lua b/src/lua/table.lua<br>>> index 5f35a30f6..3d5e69e97 100644<br>>> --- a/src/lua/table.lua<br>>> +++ b/src/lua/table.lua<br>>> @@ -63,7 +63,7 @@ end<br>>> -- @return true when the two tables are equal (false otherwise).<br>>> local function table_equals(a, b)<br>>> if type(a) ~= 'table' or type(b) ~= 'table' then<br>>> - return a == b<br>>> + return type(a) == type(b) and a == b<br>>> end<br>>> local mt = getmetatable(a)<br>>> if mt and mt.__eq then<br>>> diff --git a/test/app-tap/table.test.lua b/test/app-tap/table.test.lua<br>>> index a3c9aa123..ec81593f3 100755<br>>> --- a/test/app-tap/table.test.lua<br>>> +++ b/test/app-tap/table.test.lua<br>>> @@ -8,7 +8,7 @@ yaml.cfg{<br>>> encode_invalid_as_nil = true,<br>>> }<br>>> local test = require('tap').test('table')<br>>> -test:plan(38)<br>>> +test:plan(40)<br>>><br>>> do -- check basic table.copy (deepcopy)<br>>> local example_table = {<br>>> @@ -227,6 +227,10 @@ do -- check table.equals<br>>> test:ok(table.equals({}, {}), "table.equals for empty tables")<br>>> test:is(table.equals({}, {1}), false, "table.equals with one<br>>> empty table")<br>>> test:is(table.equals({1}, {}), false, "table.equals with one<br>>> empty table")<br>>> + test:is(table.equals({key = box.NULL}, {key = nil}), false,<br>>> + "table.equals for box.NULL and nil")<br>>> + test:is(table.equals({key = nil}, {key = box.NULL}), false,<br>>> + "table.equals for box.NULL and nil")<br>>> local tbl_a = {<br>>> first = {<br>>> 1,<br>>><br>>> ====================================<br>>>> On 13.08.2021 02:30, Serge Petrenko via Tarantool-patches wrote:<br>>>>> Introduce table.equals for comparing tables.<br>>>>> The method respects __eq metamethod, if provided.<br>>>>><br>>>>> Needed-for #5894<br>>>>> ---<br>>>>><br>>><br><br>--<br>Serge Petrenko</div></div></div></div></blockquote> <div> </div><div data-signature-widget="container"><div data-signature-widget="content"><div>--<br>Oleg Babin</div></div></div><div> </div></div></BODY></HTML>