diff --git a/changelogs/unreleased/gh-7652-complex-fkey-new-field-name.md b/changelogs/unreleased/gh-7652-complex-fkey-new-field-name.md
new file mode 100644
index 0000000000000000000000000000000000000000..93a751eb31b1e30bde1d2cb71984f357190dca89
--- /dev/null
+++ b/changelogs/unreleased/gh-7652-complex-fkey-new-field-name.md
@@ -0,0 +1,4 @@
+## bugfix/box
+
+* Fixed a bug in foreign key creation together with fields that participate
+ in that foreign key (gh-7652).
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 05af6521b2fea1cf3a616a8d48181f3008fff830..febedb9c3e078e652e16bdb50bd95adf8bb666f7 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -1058,44 +1058,43 @@ CheckSpaceFormat::prepare(struct alter_space *alter)
/** Change non-essential properties of a space. */
class ModifySpace: public AlterSpaceOp
{
+ void space_swap_dictionaries(struct space_def *old_def,
+ struct space_def *new_def,
+ struct tuple_format *new_format);
public:
ModifySpace(struct alter_space *alter, struct space_def *def)
- :AlterSpaceOp(alter), new_def(def), new_dict(NULL) {}
+ : AlterSpaceOp(alter), new_def(def) {}
/* New space definition. */
struct space_def *new_def;
- /**
- * Newely created field dictionary. When new space_def is
- * created, it allocates new dictionary. Alter moves new
- * names into an old dictionary and deletes new one.
- */
- struct tuple_dictionary *new_dict;
virtual void alter_def(struct alter_space *alter);
virtual void alter(struct alter_space *alter);
virtual void rollback(struct alter_space *alter);
virtual ~ModifySpace();
};
-/** Amend the definition of the new space. */
void
-ModifySpace::alter_def(struct alter_space *alter)
+ModifySpace::space_swap_dictionaries(struct space_def *old_def,
+ struct space_def *new_def,
+ struct tuple_format *new_format)
{
/*
- * Unless it's online space upgrade, Use the old dictionary for the new
- * space, because it's already referenced by existing tuple formats.
- * We will update it in place in ModifySpace::alter.
- *
- * For online space upgrade, all tuples fetched from the space will be
- * upgraded to the new format before returning to the user so it isn't
- * necessary. Moreover, the old tuples may be incompatible with the new
- * format so using the new dictionary for them would be wrong and could
- * result in error accessing fields by name from the space upgrade
- * function.
+ * When new space_def is created, it allocates new dictionary.
+ * Move new names into an old dictionary, which already is referenced
+ * by existing tuple formats. New dictionary object is deleted later,
+ * in alter_space_delete.
*/
- if (new_def->opts.upgrade_def == NULL) {
- new_dict = new_def->dict;
- new_def->dict = alter->old_space->def->dict;
- tuple_dictionary_ref(new_def->dict);
- }
+ tuple_dictionary_unref(new_format->dict);
+ new_format->dict = old_def->dict;
+ tuple_dictionary_ref(new_format->dict);
+
+ SWAP(old_def->dict, new_def->dict);
+ tuple_dictionary_swap(old_def->dict, new_def->dict);
+}
+
+/** Amend the definition of the new space. */
+void
+ModifySpace::alter_def(struct alter_space *alter)
+{
new_def->view_ref_count = alter->old_space->def->view_ref_count;
space_def_delete(alter->space_def);
alter->space_def = new_def;
@@ -1107,25 +1106,35 @@ void
ModifySpace::alter(struct alter_space *alter)
{
/*
- * Move new names into an old dictionary, which already is
- * referenced by existing tuple formats. New dictionary
- * object is deleted later, in destructor.
+ * Unless it's online space upgrade, use the old dictionary for the new
+ * space, because it's already referenced by existing tuple formats.
+ *
+ * For online space upgrade, all tuples fetched from the space will be
+ * upgraded to the new format before returning to the user so it isn't
+ * necessary. Moreover, the old tuples may be incompatible with the new
+ * format so using the new dictionary for them would be wrong and could
+ * result in error accessing fields by name from the space upgrade
+ * function.
*/
- if (new_dict != NULL)
- tuple_dictionary_swap(alter->new_space->def->dict, new_dict);
+ if (alter->space_def->opts.upgrade_def == NULL) {
+ space_swap_dictionaries(alter->old_space->def,
+ alter->new_space->def,
+ alter->new_space->format);
+ }
}
void
ModifySpace::rollback(struct alter_space *alter)
{
- if (new_dict != NULL)
- tuple_dictionary_swap(alter->new_space->def->dict, new_dict);
+ if (alter->space_def->opts.upgrade_def == NULL) {
+ space_swap_dictionaries(alter->new_space->def,
+ alter->old_space->def,
+ alter->new_space->format);
+ }
}
ModifySpace::~ModifySpace()
{
- if (new_dict != NULL)
- tuple_dictionary_unref(new_dict);
if (new_def != NULL)
space_def_delete(new_def);
}
diff --git a/test/engine-luatest/gh_6436_field_constraint_test.lua b/test/engine-luatest/gh_6436_field_constraint_test.lua
index f1eac82ee15ccbfefe383203dfb7fe85863d162a..54251e2a6de8b86d2cfb1f06bb31c7a55c4993a2 100644
--- a/test/engine-luatest/gh_6436_field_constraint_test.lua
+++ b/test/engine-luatest/gh_6436_field_constraint_test.lua
@@ -536,7 +536,7 @@ g.test_field_constraint_integrity = function(cg)
t.assert_equals(s:replace{1, 2, 300}, {1, 2, 300})
t.assert_error_msg_content_equals(
- "Check constraint 'field_constr3' failed for field '3'",
+ "Check constraint 'field_constr3' failed for field '3 (id3)'",
function() s:format{{"id1"},
{"id2", constraint='field_constr2'},
{"id3", constraint='field_constr3'}} end
diff --git a/test/engine-luatest/gh_7652_complex_fkey_new_field_name_test.lua b/test/engine-luatest/gh_7652_complex_fkey_new_field_name_test.lua
new file mode 100644
index 0000000000000000000000000000000000000000..d100dd15bf18420af42df50a99ec7589a8a93849
--- /dev/null
+++ b/test/engine-luatest/gh_7652_complex_fkey_new_field_name_test.lua
@@ -0,0 +1,92 @@
+local t = require('luatest')
+local g = t.group('gh-7652', {{engine = 'memtx'}, {engine = 'vinyl'}})
+
+g.before_all(function(cg)
+ local server = require('test.luatest_helpers.server')
+ 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.space2 then box.space.space2:drop() end
+ if box.space.space1 then box.space.space1:drop() end
+ end)
+end)
+
+-- Check that s2:insert{} doesn't fail with:
+-- "Foreign key constraint 'one' failed: wrong local field name"
+g.test_space_replace = function(cg)
+ local engine = cg.params.engine
+
+ cg.server:exec(function(engine)
+ local fmt = {{'id', 'integer'}}
+ local opts = {engine = engine, format = fmt}
+ local s1 = box.schema.space.create('space1', opts)
+ s1:create_index('i1')
+ s1:insert{1}
+
+ opts = {engine = engine}
+ local s2 = box.schema.space.create('space2', opts)
+ s2:create_index('i2')
+ opts = {foreign_key = {one = {space = s1.id,
+ field = {ext_id = 'id'}}}}
+ fmt = {{name = 'id', type = 'integer'},
+ {name = 'ext_id', type = 'integer'}}
+ box.space._space:replace({s2.id, 1, 'space2', engine, 0, opts, fmt})
+ s2:insert{11, 1}
+ end, {engine})
+end
+
+-- Check that s2:insert{} doesn't fail with:
+-- "Foreign key constraint 'one' failed: wrong local field name"
+g.test_space_update = function(cg)
+ local engine = cg.params.engine
+
+ cg.server:exec(function(engine)
+ local fmt = {{'id1', 'integer'}, {'id2', 'integer'}}
+ local opts = {engine = engine, format = fmt}
+ local s1 = box.schema.space.create('space1', opts)
+ s1:create_index('i1', {parts={{1}, {2}}})
+ s1:insert{1, 1}
+
+ fmt = {{name = 'id', type = 'integer'},
+ {name = 'ext_id1', type = 'integer'}}
+ opts = {engine = engine, format = fmt}
+ local s2 = box.schema.space.create('space2', opts)
+ s2:create_index('i2')
+ opts = {foreign_key = {one = {space = s1.id,
+ field = {ext_id2 = 'id2',
+ ext_id1 = 'id1'}}}}
+ fmt = {{name = 'id', type = 'integer'},
+ {name = 'ext_id1', type = 'integer'},
+ {name = 'ext_id2', type = 'integer'}}
+ box.space._space:update(s2.id, {{'=', 7, fmt}, {'=', 6, opts}})
+ s2:insert{11, 1, 1}
+ end, {engine})
+end
+
+-- Check that new_name doesn't leak through box.rollback()
+g.test_dict_rollback = function(cg)
+ local engine = cg.params.engine
+
+ cg.server:exec(function(engine)
+ local fmt = {{'id'}, {'old_name'}}
+ local opts = {engine = engine, format = fmt}
+ local s1 = box.schema.space.create('space1', opts)
+ s1:create_index('pk')
+
+ box.begin()
+ s1:format({{'id'}, {'new_name'}})
+ box.rollback()
+ local t = require('luatest')
+ t.assert_error_msg_content_equals(
+ "Tuple field 2 (old_name) required by space format is missing",
+ function() s1:insert{1} end)
+ end, {engine})
+end
diff --git a/test/engine/ddl.result b/test/engine/ddl.result
index b1dc950bae8b51e324e8bd2c5a399fda4fc5bb7b..b857a63b9b87ac32447011977cde18ba1d5da4da 100644
--- a/test/engine/ddl.result
+++ b/test/engine/ddl.result
@@ -1567,7 +1567,7 @@ format[7] = {name = 'field7', type = 'unsigned'}
-- Fail, the tuple {1, ... 1} is invalid for a new format.
s:format(format)
---
-- error: Tuple field 7 required by space format is missing
+- error: Tuple field 7 (field7) required by space format is missing
...
s:drop()
---
diff --git a/test/engine/json.result b/test/engine/json.result
index fac53d3736ef633a085080d52586522f72aa8d32..6f3034ea548e9179a37981b630c68cd86113e47a 100644
--- a/test/engine/json.result
+++ b/test/engine/json.result
@@ -59,7 +59,7 @@ format = {{'id', 'unsigned'}, {'meta', 'unsigned'}, {'data', 'array'}, {'age', '
...
s:format(format)
---
-- error: Field 3 has type 'array' in one index, but type 'map' in another
+- error: Field 3 (data) has type 'array' in one index, but type 'map' in another
...
format = {{'id', 'unsigned'}, {'meta', 'unsigned'}, {'data', 'map'}, {'age', 'unsigned'}, {'level', 'unsigned'}}
---
diff --git a/test/vinyl/errinj_ddl.result b/test/vinyl/errinj_ddl.result
index c803ba7e5fb3c54af03772c3e80ccdb57854a25d..1dff175b2ff0475272e3a23cd026a1699a810cdd 100644
--- a/test/vinyl/errinj_ddl.result
+++ b/test/vinyl/errinj_ddl.result
@@ -380,7 +380,7 @@ fiber.sleep(0)
...
s:format{{'key', 'unsigned'}, {'value', 'unsigned'}} -- must fail
---
-- error: Tuple field 2 required by space format is missing
+- error: Tuple field 2 (value) required by space format is missing
...
s:select()
---