From 014f5aa177fd9817db5a55f31d8fc4a88f1b68a3 Mon Sep 17 00:00:00 2001
From: Ilya Verbin <iverbin@tarantool.org>
Date: Mon, 11 Jul 2022 15:30:20 +0300
Subject: [PATCH] box: return 1-based fkey field numbers to Lua

In Lua field's numbers are counted from base 1, however currently
space:format() and space.foreign_key return zero-based foreign key
fields, which leads to an error on space:format(space:format()).

Closes #7350

NO_DOC=bugfix
---
 .../gh-7350-1-based-fkey-field-no.md          |  4 ++
 src/box/lua/schema.lua                        | 34 ++++++++++++++-
 src/box/lua/space.cc                          |  3 +-
 .../gh_6436_complex_foreign_key_test.lua      |  4 +-
 .../gh_6436_field_foreign_key_test.lua        |  8 +---
 .../gh_7350_fkey_field_no_mismatch_test.lua   | 42 +++++++++++++++++++
 6 files changed, 84 insertions(+), 11 deletions(-)
 create mode 100644 changelogs/unreleased/gh-7350-1-based-fkey-field-no.md
 create mode 100644 test/engine-luatest/gh_7350_fkey_field_no_mismatch_test.lua

diff --git a/changelogs/unreleased/gh-7350-1-based-fkey-field-no.md b/changelogs/unreleased/gh-7350-1-based-fkey-field-no.md
new file mode 100644
index 0000000000..7ba0495783
--- /dev/null
+++ b/changelogs/unreleased/gh-7350-1-based-fkey-field-no.md
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Fixed incorrect number of a foreign key field returned by
+  `space_object:format()` or `space_object.foreign_key` (gh-7350).
diff --git a/src/box/lua/schema.lua b/src/box/lua/schema.lua
index cb396fb137..97c88cdada 100644
--- a/src/box/lua/schema.lua
+++ b/src/box/lua/schema.lua
@@ -727,6 +727,38 @@ local function normalize_format(space_id, space_name, format)
 end
 box.internal.space.normalize_format = normalize_format -- for space.upgrade
 
+local function denormalize_foreign_key_one(fkey)
+    assert(type(fkey.field) == 'string' or type(fkey.field) == 'number')
+    local result = fkey
+    if type(fkey.field) == 'number' then
+        -- convert to one-based index
+        result.field = result.field + 1
+    end
+    return result
+end
+
+local function denormalize_foreign_key(fkey)
+    local result = setmap{}
+    for k, v in pairs(fkey) do
+        result[k] = denormalize_foreign_key_one(v)
+    end
+    return result
+end
+
+-- Convert zero-based foreign key field numbers to one-based
+local function denormalize_format(format)
+    local result = setmetatable({}, { __serialize = 'seq' })
+    for i, f in ipairs(format) do
+        result[i] = f
+        for k, v in pairs(f) do
+            if k == 'foreign_key' then
+                result[i][k] = denormalize_foreign_key(v)
+            end
+        end
+    end
+    return result
+end
+
 box.schema.space = {}
 box.schema.space.create = function(name, options)
     check_param(name, 'name', 'string')
@@ -819,7 +851,7 @@ function box.schema.space.format(id, format)
     end
 
     if format == nil then
-        return tuple.format
+        return denormalize_format(tuple.format)
     else
         check_param(format, 'format', 'table')
         format = normalize_format(id, tuple.name, format)
diff --git a/src/box/lua/space.cc b/src/box/lua/space.cc
index aaafe492b0..bc37e4af4f 100644
--- a/src/box/lua/space.cc
+++ b/src/box/lua/space.cc
@@ -269,13 +269,14 @@ lbox_push_space_constraint(struct lua_State *L, struct space *space, int i)
 /**
  * Helper function of lbox_push_space_foreign_key.
  * Push a value @a def to the top of lua stack @a L.
+ * ID-defined fields are converted to one-based index.
  */
 static void
 lbox_push_field_id(struct lua_State *L,
 		   struct tuple_constraint_field_id *def)
 {
 	if (def->name_len == 0)
-		lua_pushnumber(L, def->id);
+		lua_pushnumber(L, def->id + 1);
 	else
 		lua_pushstring(L, def->name);
 }
diff --git a/test/engine-luatest/gh_6436_complex_foreign_key_test.lua b/test/engine-luatest/gh_6436_complex_foreign_key_test.lua
index 78a1904ef1..150710844b 100644
--- a/test/engine-luatest/gh_6436_complex_foreign_key_test.lua
+++ b/test/engine-luatest/gh_6436_complex_foreign_key_test.lua
@@ -283,9 +283,7 @@ g.test_complex_foreign_key_numeric = function(cg)
         local fkey = {cntr = {space=country.id,
                               field={[4]=4, [3]=2, [2]=3}}}
         local city = box.schema.create_space('city', city_space_opts(fkey))
-        t.assert_equals(city.foreign_key,
-                        {cntr = {field = {[1] = 2, [2] = 1, [3] = 3},
-                                 space = country.id}});
+        t.assert_equals(city.foreign_key, fkey)
         city:create_index('pk')
 
         t.assert_equals(country:select{}, {{100, 1, 'earth', 'ru', 'Russia'},
diff --git a/test/engine-luatest/gh_6436_field_foreign_key_test.lua b/test/engine-luatest/gh_6436_field_foreign_key_test.lua
index 50d6ab525b..c3df9a9982 100644
--- a/test/engine-luatest/gh_6436_field_foreign_key_test.lua
+++ b/test/engine-luatest/gh_6436_field_foreign_key_test.lua
@@ -287,12 +287,8 @@ g.test_foreign_key_numeric = function(cg)
         local city = box.schema.create_space('city', {engine=engine, format=fmt})
         -- Check that fmt is not modified by create_space()
         t.assert_equals(fmt_copy, fmt)
-        -- Note that the format was normalized and field converted to zero-based.
-        t.assert_equals(city:format(),
-                        { { name = "id", type = "unsigned"},
-                          { foreign_key = {country = {field = 1, space = country.id}},
-                            name = "country_code", type = "string"},
-                          { name = "name", type = "string"} });
+        -- Check that format() returns one-based field number
+        t.assert_equals(city:format(), fmt)
         city:create_index('pk')
 
         t.assert_equals(country:select{}, {{1, 'ru', 'Russia'}, {2, 'fr', 'France'}})
diff --git a/test/engine-luatest/gh_7350_fkey_field_no_mismatch_test.lua b/test/engine-luatest/gh_7350_fkey_field_no_mismatch_test.lua
new file mode 100644
index 0000000000..f431fe6a00
--- /dev/null
+++ b/test/engine-luatest/gh_7350_fkey_field_no_mismatch_test.lua
@@ -0,0 +1,42 @@
+local server = require('test.luatest_helpers.server')
+local t = require('luatest')
+local g = t.group('gh-7350-fkey-field-no-mismatch',
+                  {{engine = 'memtx'}, {engine = 'vinyl'}})
+
+g.before_all(function(cg)
+    cg.server = server:new({alias = 'master'})
+    cg.server:start()
+end)
+
+g.after_all(function(cg)
+    cg.server:stop()
+    cg.server = nil
+end)
+
+g.after_each(function(cg)
+    cg.server:exec(function()
+        if box.space.s2 then box.space.s2:drop() end
+        if box.space.s1 then box.space.s1:drop() end
+    end)
+end)
+
+g.test_fkey_field_no = function(cg)
+    local engine = cg.params.engine
+
+    cg.server:exec(function(engine)
+        local t = require('luatest')
+        local s1 = box.schema.create_space('s1', {engine=engine})
+
+        local fmt = {{name='f1', type='any', foreign_key={k1={space=s1.id, field=2}}},
+                     {name='f2', type='any', foreign_key={k2={space=s1.id, field='f3'}}},
+                     {name='f3', type='any', foreign_key={k3={space=s1.id, field=4}}}}
+        local fkey = {k4={space=s1.id, field={[5]=7, ['f6']='f5', [4]=6}}}
+
+        local opts = {engine=engine, format=fmt, foreign_key=fkey}
+        local s2 = box.schema.create_space('s2', opts)
+
+        t.assert_equals(s2:format(), fmt)
+        t.assert_equals(s2.foreign_key, fkey)
+        t.assert_equals(s2:format(s2:format()), nil)
+    end, {engine})
+end
-- 
GitLab