Skip to content

Commit

Permalink
box: loose truncate check in case of foreign key
Browse files Browse the repository at this point in the history
In tarantool#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 tarantool#4348. To
make tests pass I had to perform drop in box.atomic manually.

Closes tarantool#8946

NO_DOC=bugfix

(cherry picked from commit 983a7ec)
  • Loading branch information
alyapunov committed Aug 7, 2023
1 parent 9f47089 commit 9e099bf
Show file tree
Hide file tree
Showing 8 changed files with 188 additions and 11 deletions.
Original file line number Diff line number Diff line change
@@ -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).
45 changes: 41 additions & 4 deletions src/box/alter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,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>
Expand Down Expand Up @@ -2048,6 +2049,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.
Expand Down Expand Up @@ -2537,11 +2574,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;
}

Expand Down Expand Up @@ -2810,7 +2846,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);
Expand Down
2 changes: 1 addition & 1 deletion src/box/space_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,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
Expand Down
6 changes: 3 additions & 3 deletions test/engine-luatest/gh_6436_complex_foreign_key_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down Expand Up @@ -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'}})
Expand Down Expand Up @@ -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'}})
Expand Down
6 changes: 3 additions & 3 deletions test/engine-luatest/gh_6436_field_foreign_key_test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
131 changes: 131 additions & 0 deletions test/engine-luatest/gh_8946_foreign_key_disables_truncate_test.lua
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions test/sql/delete.result
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,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
Expand Down
1 change: 1 addition & 0 deletions test/sql/delete.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,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.
Expand Down

0 comments on commit 9e099bf

Please sign in to comment.