Commit 64a23c1c authored by Sergei Golubchik's avatar Sergei Golubchik

extend prelocking to FK-accessed tables

Backport of f1362910
parent 3b365fa8
...@@ -14,3 +14,40 @@ ALTER TABLE `title` ADD FOREIGN KEY (`title_manager_fk`) REFERENCES `people` ...@@ -14,3 +14,40 @@ ALTER TABLE `title` ADD FOREIGN KEY (`title_manager_fk`) REFERENCES `people`
ALTER TABLE `title` ADD FOREIGN KEY (`title_reporter_fk`) REFERENCES `people` ALTER TABLE `title` ADD FOREIGN KEY (`title_reporter_fk`) REFERENCES `people`
(`people_id`); (`people_id`);
drop table title, department, people; drop table title, department, people;
create table t1 (a int primary key, b int) engine=innodb;
create table t2 (c int primary key, d int,
foreign key (d) references t1 (a) on update cascade) engine=innodb;
insert t1 values (1,1),(2,2),(3,3);
insert t2 values (4,1),(5,2),(6,3);
flush table t2 with read lock;
connect con1,localhost,root;
delete from t1 where a=2;
ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`d`) REFERENCES `t1` (`a`) ON UPDATE CASCADE)
update t1 set a=10 where a=1;
connection default;
unlock tables;
connection con1;
connection default;
lock table t2 write;
connection con1;
delete from t1 where a=2;
connection default;
unlock tables;
connection con1;
ERROR 23000: Cannot delete or update a parent row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`d`) REFERENCES `t1` (`a`) ON UPDATE CASCADE)
connection default;
unlock tables;
disconnect con1;
create user foo;
grant select,update on test.t1 to foo;
connect foo,localhost,foo;
update t1 set a=30 where a=3;
disconnect foo;
connection default;
select * from t2;
c d
5 2
4 10
6 30
drop table t2, t1;
drop user foo;
--source include/have_innodb.inc --source include/have_innodb.inc
--source include/have_debug.inc --source include/have_debug.inc
--enable_connect_log
--echo # --echo #
--echo # Bug #19471516 SERVER CRASHES WHEN EXECUTING ALTER TABLE --echo # Bug #19471516 SERVER CRASHES WHEN EXECUTING ALTER TABLE
--echo # ADD FOREIGN KEY --echo # ADD FOREIGN KEY
...@@ -24,3 +26,49 @@ ALTER TABLE `title` ADD FOREIGN KEY (`title_reporter_fk`) REFERENCES `people` ...@@ -24,3 +26,49 @@ ALTER TABLE `title` ADD FOREIGN KEY (`title_reporter_fk`) REFERENCES `people`
(`people_id`); (`people_id`);
drop table title, department, people; drop table title, department, people;
#
# FK and prelocking:
# child table accesses (reads and writes) wait for locks.
#
create table t1 (a int primary key, b int) engine=innodb;
create table t2 (c int primary key, d int,
foreign key (d) references t1 (a) on update cascade) engine=innodb;
insert t1 values (1,1),(2,2),(3,3);
insert t2 values (4,1),(5,2),(6,3);
flush table t2 with read lock; # this takes MDL_SHARED_NO_WRITE
connect (con1,localhost,root);
--error ER_ROW_IS_REFERENCED_2
delete from t1 where a=2;
send update t1 set a=10 where a=1;
connection default;
let $wait_condition= select 1 from information_schema.processlist where state='Waiting for table metadata lock';
source include/wait_condition.inc;
unlock tables;
connection con1;
reap;
connection default;
lock table t2 write; # this takes MDL_SHARED_NO_READ_WRITE
connection con1;
send delete from t1 where a=2;
connection default;
let $wait_condition= select 1 from information_schema.processlist where state='Waiting for table metadata lock';
source include/wait_condition.inc;
unlock tables;
connection con1;
--error ER_ROW_IS_REFERENCED_2
reap;
connection default;
unlock tables;
disconnect con1;
# but privileges should not be checked
create user foo;
grant select,update on test.t1 to foo;
connect(foo,localhost,foo);
update t1 set a=30 where a=3;
disconnect foo;
connection default;
select * from t2;
drop table t2, t1;
drop user foo;
...@@ -4231,7 +4231,7 @@ sp_head::add_used_tables_to_table_list(THD *thd, ...@@ -4231,7 +4231,7 @@ sp_head::add_used_tables_to_table_list(THD *thd,
table->init_one_table_for_prelocking(key_buff, stab->db_length, table->init_one_table_for_prelocking(key_buff, stab->db_length,
key_buff + stab->db_length + 1, stab->table_name_length, key_buff + stab->db_length + 1, stab->table_name_length,
key_buff + stab->db_length + stab->table_name_length + 2, key_buff + stab->db_length + stab->table_name_length + 2,
stab->lock_type, belong_to_view, stab->trg_event_map, stab->lock_type, true, belong_to_view, stab->trg_event_map,
query_tables_last_ptr); query_tables_last_ptr);
tab_buff+= ALIGN_SIZE(sizeof(TABLE_LIST)); tab_buff+= ALIGN_SIZE(sizeof(TABLE_LIST));
......
...@@ -4802,6 +4802,25 @@ handle_routine(THD *thd, Query_tables_list *prelocking_ctx, ...@@ -4802,6 +4802,25 @@ handle_routine(THD *thd, Query_tables_list *prelocking_ctx,
} }
/*
@note this can be changed to use a hash, instead of scanning the linked
list, if the performance of this function will ever become an issue
*/
static bool table_already_fk_prelocked(TABLE_LIST *tl, LEX_STRING *db,
LEX_STRING *table, thr_lock_type lock_type)
{
for (; tl; tl= tl->next_global )
{
if (tl->lock_type >= lock_type &&
tl->prelocking_placeholder == TABLE_LIST::FK &&
strcmp(tl->db, db->str) == 0 &&
strcmp(tl->table_name, table->str) == 0)
return true;
}
return false;
}
/** /**
Defines how prelocking algorithm for DML statements should handle table list Defines how prelocking algorithm for DML statements should handle table list
elements: elements:
...@@ -4841,6 +4860,52 @@ handle_table(THD *thd, Query_tables_list *prelocking_ctx, ...@@ -4841,6 +4860,52 @@ handle_table(THD *thd, Query_tables_list *prelocking_ctx,
add_tables_and_routines_for_triggers(thd, prelocking_ctx, table_list)) add_tables_and_routines_for_triggers(thd, prelocking_ctx, table_list))
return TRUE; return TRUE;
} }
if (table_list->table->file->referenced_by_foreign_key())
{
List <FOREIGN_KEY_INFO> fk_list;
List_iterator<FOREIGN_KEY_INFO> fk_list_it(fk_list);
FOREIGN_KEY_INFO *fk;
Query_arena *arena, backup;
arena= thd->activate_stmt_arena_if_needed(&backup);
table_list->table->file->get_parent_foreign_key_list(thd, &fk_list);
if (thd->is_error())
{
if (arena)
thd->restore_active_arena(arena, &backup);
return TRUE;
}
*need_prelocking= TRUE;
while ((fk= fk_list_it++))
{
// FK_OPTION_RESTRICT and FK_OPTION_NO_ACTION only need read access
static bool can_write[]= { true, false, true, true, false, true };
uint8 op= table_list->trg_event_map;
thr_lock_type lock_type;
if ((op & (1 << TRG_EVENT_DELETE) && can_write[fk->delete_method])
|| (op & (1 << TRG_EVENT_UPDATE) && can_write[fk->update_method]))
lock_type= TL_WRITE_ALLOW_WRITE;
else
lock_type= TL_READ;
if (table_already_fk_prelocked(table_list, fk->foreign_db,
fk->foreign_table, lock_type))
continue;
TABLE_LIST *tl= (TABLE_LIST *) thd->alloc(sizeof(TABLE_LIST));
tl->init_one_table_for_prelocking(fk->foreign_db->str, fk->foreign_db->length,
fk->foreign_table->str, fk->foreign_table->length,
NULL, lock_type, false, table_list->belong_to_view,
op, &prelocking_ctx->query_tables_last);
}
if (arena)
thd->restore_active_arena(arena, &backup);
}
} }
return FALSE; return FALSE;
......
...@@ -5082,7 +5082,8 @@ has_write_table_with_auto_increment_and_select(TABLE_LIST *tables) ...@@ -5082,7 +5082,8 @@ has_write_table_with_auto_increment_and_select(TABLE_LIST *tables)
for(TABLE_LIST *table= tables; table; table= table->next_global) for(TABLE_LIST *table= tables; table; table= table->next_global)
{ {
if (!table->placeholder() && if (!table->placeholder() &&
(table->lock_type <= TL_READ_NO_INSERT)) table->lock_type <= TL_READ_NO_INSERT &&
table->prelocking_placeholder != TABLE_LIST::FK)
{ {
has_select= true; has_select= true;
break; break;
......
...@@ -1713,6 +1713,7 @@ struct TABLE_LIST ...@@ -1713,6 +1713,7 @@ struct TABLE_LIST
size_t table_name_length_arg, size_t table_name_length_arg,
const char *alias_arg, const char *alias_arg,
enum thr_lock_type lock_type_arg, enum thr_lock_type lock_type_arg,
bool routine,
TABLE_LIST *belong_to_view_arg, TABLE_LIST *belong_to_view_arg,
uint8 trg_event_map_arg, uint8 trg_event_map_arg,
TABLE_LIST ***last_ptr) TABLE_LIST ***last_ptr)
...@@ -1720,7 +1721,8 @@ struct TABLE_LIST ...@@ -1720,7 +1721,8 @@ struct TABLE_LIST
init_one_table(db_name_arg, db_length_arg, table_name_arg, init_one_table(db_name_arg, db_length_arg, table_name_arg,
table_name_length_arg, alias_arg, lock_type_arg); table_name_length_arg, alias_arg, lock_type_arg);
cacheable_table= 1; cacheable_table= 1;
prelocking_placeholder= 1; prelocking_placeholder= routine ? ROUTINE : FK;
open_type= routine ? OT_TEMPORARY_OR_BASE : OT_BASE_ONLY;
belong_to_view= belong_to_view_arg; belong_to_view= belong_to_view_arg;
trg_event_map= trg_event_map_arg; trg_event_map= trg_event_map_arg;
...@@ -2004,7 +2006,7 @@ struct TABLE_LIST ...@@ -2004,7 +2006,7 @@ struct TABLE_LIST
This TABLE_LIST object is just placeholder for prelocking, it will be This TABLE_LIST object is just placeholder for prelocking, it will be
used for implicit LOCK TABLES only and won't be used in real statement. used for implicit LOCK TABLES only and won't be used in real statement.
*/ */
bool prelocking_placeholder; enum { USER, ROUTINE, FK } prelocking_placeholder;
/** /**
Indicates that if TABLE_LIST object corresponds to the table/view Indicates that if TABLE_LIST object corresponds to the table/view
which requires special handling. which requires special handling.
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment