Thanks for your fixes.   >Понедельник, 16 августа 2021, 10:53 +03:00 от Serge Petrenko : > >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   I’m not sure that it’s correct. I’ve added a reference to SO ( https://stackoverflow.com/questions/61946190/lua-eq-on-tables-with-different-metatables ) and it seems condition should be following mta and mta.__eq and mta == mtb. Otherwise tables have different __eq metamathods and you should compare their content.   I’m not sure that my proposal is completely correct and suggest to ask Igor about it.   >          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     -- Oleg Babin