From 983a7ec215d46b6d02935d1baa8bbe07fc371795 Mon Sep 17 00:00:00 2001
From: Aleksandr Lyapunov <alyapunov@tarantool.org>
Date: Thu, 3 Aug 2023 19:06:37 +0300
Subject: [PATCH] box: loose truncate check in case of foreign key
In #7309 a truncation of a space that was referenced by foreign
key from some other space was prohibited.
It appeared that this solution is too bothering since a user can't
truncate a space even if he truncated referring space before that.
Fix it by allowing space truncate if referring spaces are empty.
Also allow drop of the primary index in the same case with the
same reason: logically the index along with all space data is
not needed for consistency if there's no referring data.
Note that by design space truncate is implemented quite similar
to space drop. Both delete all indexes, from secondary to primary.
Since this patch allows deletion of the primary index (which is
the action that actually deletes all data from the space), this
patch changes the result of space drop too: the space remains
alive with no indexes, while before this patch it remained alive
with no secondary indexes but with present primary. In both cases
the behaviour is quite strange and must be fixed in #4348. To
make tests pass I had to perform drop in box.atomic manually.
Closes #8946
NO_DOC=bugfix
---
.../gh-8946-foreign-key-disables-truncate.md | 4 +
src/box/alter.cc | 45 +++++-
src/box/space_cache.h | 2 +-
.../gh_6436_complex_foreign_key_test.lua | 6 +-
.../gh_6436_field_foreign_key_test.lua | 6 +-
...946_foreign_key_disables_truncate_test.lua | 131 ++++++++++++++++++
test/sql/delete.result | 4 +
test/sql/delete.test.lua | 1 +
8 files changed, 188 insertions(+), 11 deletions(-)
create mode 100644 changelogs/unreleased/gh-8946-foreign-key-disables-truncate.md
create mode 100644 test/engine-luatest/gh_8946_foreign_key_disables_truncate_test.lua
diff --git a/changelogs/unreleased/gh-8946-foreign-key-disables-truncate.md b/changelogs/unreleased/gh-8946-foreign-key-disables-truncate.md
new file mode 100644
index 0000000000..92dbfc5f4a
--- /dev/null
+++ b/changelogs/unreleased/gh-8946-foreign-key-disables-truncate.md
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Fixed a bug when a space that is referenced by a foreign key could not
+ be truncated even if the referring space was empty (gh-8946).
diff --git a/src/box/alter.cc b/src/box/alter.cc
index 9aa186fc43..f8784da6ed 100644
--- a/src/box/alter.cc
+++ b/src/box/alter.cc
@@ -40,6 +40,7 @@
#include "coll_id_def.h"
#include "txn.h"
#include "tuple.h"
+#include "tuple_constraint.h"
#include "fiber.h" /* for gc_pool */
#include "scoped_guard.h"
#include <base64.h>
@@ -1951,6 +1952,42 @@ space_check_pinned(struct space *space)
return 0;
}
+/**
+ * Check whether @a space holders prohibit truncate of the space.
+ * For example truncation in not allowed if another non-empty space refers
+ * to this space via foreign key link.
+ * Return 0 if allowed, or -1 if not allowed (diag is set).
+ */
+static int
+space_check_truncate(struct space *space)
+{
+ /* Check for foreign keys that refers to this space. */
+ struct space_cache_holder *h;
+ rlist_foreach_entry(h, &space->space_cache_pin_list, link) {
+ if (h->selfpin)
+ continue;
+ if (h->type != SPACE_HOLDER_FOREIGN_KEY)
+ continue;
+ struct tuple_constraint *constr =
+ container_of(h, struct tuple_constraint,
+ space_cache_holder);
+ struct space *other_space = constr->space;
+ /*
+ * If the referring space is empty then the truncate can't
+ * break foreign key consistency.
+ */
+ if (space_bsize(other_space) == 0)
+ continue;
+ const char *type_str =
+ space_cache_holder_type_strs[h->type];
+ diag_set(ClientError, ER_ALTER_SPACE,
+ space_name(space),
+ tt_sprintf("space is referenced by %s", type_str));
+ return -1;
+ }
+ return 0;
+}
+
/**
* A trigger which is invoked on replace in a data dictionary
* space _space.
@@ -2404,11 +2441,10 @@ on_replace_dd_index(struct trigger * /* trigger */, void *event)
"space sequence exists");
return -1;
}
-
/*
- * Must not truncate pinned space.
+ * Check space's holders.
*/
- if (space_check_pinned(old_space) != 0)
+ if (space_check_truncate(old_space) != 0)
return -1;
}
@@ -2667,7 +2703,8 @@ on_replace_dd_truncate(struct trigger * /* trigger */, void *event)
return -1;
}
- if (space_check_pinned(old_space) != 0)
+ /* Check space's holders. */
+ if (space_check_truncate(old_space) != 0)
return -1;
struct alter_space *alter = alter_space_new(old_space);
diff --git a/src/box/space_cache.h b/src/box/space_cache.h
index 344f8359ec..97d64b52b1 100644
--- a/src/box/space_cache.h
+++ b/src/box/space_cache.h
@@ -176,7 +176,7 @@ space_cache_unpin(struct space_cache_holder *holder);
/**
* Check whether the @a space has holders or not.
* If it has, @a type argument is set to the first holder's type.
- * The function must be in cache (asserted).
+ * The space must be in cache (asserted).
* If a space has holders, it must not be deleted (asserted).
*/
bool
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 e024a0882a..9a26e28516 100644
--- a/test/engine-luatest/gh_6436_complex_foreign_key_test.lua
+++ b/test/engine-luatest/gh_6436_complex_foreign_key_test.lua
@@ -109,7 +109,7 @@ g.test_complex_foreign_key_primary = function(cg)
t.assert_equals(country:select{}, {{1, 11, 'Russia'}, {1, 12, 'France'}})
t.assert_error_msg_content_equals(
"Can't modify space 'country': space is referenced by foreign key",
- function() country:drop() end
+ box.atomic, country.drop, country
)
t.assert_error_msg_content_equals(
"Foreign key 'country' integrity check failed: wrong foreign field name",
@@ -198,7 +198,7 @@ g.test_complex_foreign_key_secondary = function(cg)
{101, 1, 'earth', 'rf', 'France'}})
t.assert_error_msg_content_equals(
"Can't modify space 'country': space is referenced by foreign key",
- function() country:drop() end
+ box.atomic, country.drop, country
)
t.assert_equals(country:select{}, {{100, 1, 'earth', 'ru', 'Russia'},
{101, 1, 'earth', 'rf', 'France'}})
@@ -293,7 +293,7 @@ g.test_complex_foreign_key_numeric = function(cg)
{101, 1, 'earth', 'rf', 'France'}})
t.assert_error_msg_content_equals(
"Can't modify space 'country': space is referenced by foreign key",
- function() country:drop() end
+ box.atomic, country.drop, country
)
t.assert_equals(country:select{}, {{100, 1, 'earth', 'ru', 'Russia'},
{101, 1, 'earth', 'rf', 'France'}})
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 133d2ea0c3..bdb04acde4 100644
--- a/test/engine-luatest/gh_6436_field_foreign_key_test.lua
+++ b/test/engine-luatest/gh_6436_field_foreign_key_test.lua
@@ -137,7 +137,7 @@ g.test_foreign_key_primary = function(cg)
t.assert_equals(country:select{}, {{1, 'ru', 'Russia'}, {2, 'fr', 'France'}})
t.assert_error_msg_content_equals(
"Can't modify space 'country': space is referenced by foreign key",
- function() country:drop() end
+ box.atomic, country.drop, country
)
t.assert_equals(country:select{}, {{1, 'ru', 'Russia'}, {2, 'fr', 'France'}})
t.assert_error_msg_content_equals(
@@ -215,7 +215,7 @@ g.test_foreign_key_secondary = function(cg)
t.assert_equals(country:select{}, {{1, 'ru', 'Russia'}, {2, 'fr', 'France'}})
t.assert_error_msg_content_equals(
"Can't modify space 'country': space is referenced by foreign key",
- function() country:drop() end
+ box.atomic, country.drop, country
)
t.assert_equals(country:select{}, {{1, 'ru', 'Russia'}, {2, 'fr', 'France'}})
t.assert_error_msg_content_equals(
@@ -297,7 +297,7 @@ g.test_foreign_key_numeric = function(cg)
t.assert_equals(country:select{}, {{1, 'ru', 'Russia'}, {2, 'fr', 'France'}})
t.assert_error_msg_content_equals(
"Can't modify space 'country': space is referenced by foreign key",
- function() country:drop() end
+ box.atomic, country.drop, country
)
t.assert_equals(country:select{}, {{1, 'ru', 'Russia'}, {2, 'fr', 'France'}})
t.assert_error_msg_content_equals(
diff --git a/test/engine-luatest/gh_8946_foreign_key_disables_truncate_test.lua b/test/engine-luatest/gh_8946_foreign_key_disables_truncate_test.lua
new file mode 100644
index 0000000000..17c6d3f2c0
--- /dev/null
+++ b/test/engine-luatest/gh_8946_foreign_key_disables_truncate_test.lua
@@ -0,0 +1,131 @@
+-- https://github.com/tarantool/tarantool/issues/8946
+-- Test that a space that another empty space refers to can be truncated.
+local server = require('luatest.server')
+local t = require('luatest')
+
+local engines = {{engine = 'memtx'}, {engine = 'vinyl'}}
+local g = t.group('gh-8946-foreign-key-truncate-test', engines)
+
+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.client_phones then
+ box.space.client_phones:drop()
+ end
+ if box.space.client then
+ box.space.client:drop()
+ end
+ end)
+end)
+
+-- Field foreign key must not disable truncate if referring space is empty.
+g.test_field_foreign_key_truncate = function(cg)
+ local engine = cg.params.engine
+
+ cg.server:exec(function(engine)
+ box.schema.space.create('client', {engine = engine})
+ box.space.client:format({
+ {name = 'customer_id', type = 'string', is_nullable = false},
+ {name = 'esia_id', type = 'string', is_nullable = false},
+ })
+
+ box.space.client:create_index('pk_client_customer_id', {
+ parts = {{field = 'customer_id', collation = 'unicode'}},
+ type = 'tree', unique = true
+ })
+ box.space.client:create_index('idx_client_esia_id', {
+ parts = {{field = 'esia_id', collation = 'unicode'}},
+ type = 'tree', unique = false
+ })
+
+ box.schema.space.create('client_phones', {engine = engine})
+ box.space.client_phones:format({
+ {name = 'phone', type = 'string', is_nullable = false},
+ {name = 'customer_id',
+ foreign_key = {space = 'client', field = 'customer_id'}},
+ })
+
+ box.space.client_phones:create_index('idx_client_phones_phone', {
+ parts = {{field = 'phone', collation = 'unicode'}},
+ type = 'tree', unique = true
+ })
+
+ box.space.client:insert{'01','esia-01'}
+ box.space.client:insert{'02','esia-02'}
+
+ box.space.client_phones:insert{'9121234','01'}
+ box.space.client_phones:insert{'3222222','02'}
+
+ -- Now truncate is prohibited.
+ t.assert_error_msg_content_equals(
+ "Can't modify space 'client': space is referenced by foreign key",
+ box.space.client.truncate, box.space.client)
+
+ box.space.client_phones:truncate()
+
+ -- Now truncate is allowed.
+ box.space.client:truncate()
+ end, {engine})
+end
+
+-- Tuple foreign key must not disable truncate if referring space is empty.
+g.test_tuple_foreign_key_truncate = function(cg)
+ local engine = cg.params.engine
+
+ cg.server:exec(function(engine)
+ box.schema.space.create('client', {engine = engine})
+ box.space.client:format({
+ {name = 'customer_id', type = 'string', is_nullable = false},
+ {name = 'esia_id', type = 'string', is_nullable = false},
+ })
+
+ box.space.client:create_index('pk_client_customer_id', {
+ parts = {{field = 'customer_id', collation = 'unicode'}},
+ type = 'tree', unique = true
+ })
+ box.space.client:create_index('idx_client_esia_id', {
+ parts = {{field = 'esia_id', collation = 'unicode'}},
+ type = 'tree', unique = false
+ })
+
+ box.schema.space.create('client_phones', {
+ engine = engine,
+ foreign_key = {space = 'client',
+ field = {customer_id = 'customer_id'}}
+ })
+ box.space.client_phones:format({
+ {name = 'phone', type = 'string', is_nullable = false},
+ {name = 'customer_id'},
+ })
+
+ box.space.client_phones:create_index('idx_client_phones_phone', {
+ parts = {{field = 'phone', collation = 'unicode'}},
+ type = 'tree', unique = true
+ })
+
+ box.space.client:insert{'01','esia-01'}
+ box.space.client:insert{'02','esia-02'}
+
+ box.space.client_phones:insert{'9121234','01'}
+ box.space.client_phones:insert{'3222222','02'}
+
+ -- Now truncate is prohibited.
+ t.assert_error_msg_content_equals(
+ "Can't modify space 'client': space is referenced by foreign key",
+ box.space.client.truncate, box.space.client)
+
+ box.space.client_phones:truncate()
+
+ -- Now truncate is allowed.
+ box.space.client:truncate()
+ end, {engine})
+end
diff --git a/test/sql/delete.result b/test/sql/delete.result
index 3c2ed1e2e9..3f3b8e5ef0 100644
--- a/test/sql/delete.result
+++ b/test/sql/delete.result
@@ -141,6 +141,10 @@ box.execute("CREATE TABLE t2(x INT PRIMARY KEY REFERENCES t1(id));")
---
- row_count: 1
...
+box.execute("INSERT INTO t2 VALUES(1);")
+---
+- row_count: 1
+...
box.execute("TRUNCATE TABLE t1;")
---
- null
diff --git a/test/sql/delete.test.lua b/test/sql/delete.test.lua
index 82cf332232..8eab0b0aea 100644
--- a/test/sql/delete.test.lua
+++ b/test/sql/delete.test.lua
@@ -60,6 +60,7 @@ box.execute("TRUNCATE TABLE v1;")
-- Can't truncate table with FK.
box.execute("CREATE TABLE t2(x INT PRIMARY KEY REFERENCES t1(id));")
+box.execute("INSERT INTO t2 VALUES(1);")
box.execute("TRUNCATE TABLE t1;")
-- Table triggers should be ignored.
--
GitLab