extend prelocking to FK-accessed tables

Backport of f136291

Author: vuvova

mysql-test/suite/innodb/r/foreign-keys.result

@@ -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`
 (`people_id`);
 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;

mysql-test/suite/innodb/t/foreign-keys.test

@@ -1,6 +1,8 @@
 --source include/have_innodb.inc
 --source include/have_debug.inc
 
+--enable_connect_log
+
 --echo #
 --echo # Bug #19471516 SERVER CRASHES WHEN EXECUTING ALTER TABLE
 --echo # ADD FOREIGN KEY
@@ -24,3 +26,49 @@ ALTER TABLE `title` ADD FOREIGN KEY (`title_reporter_fk`) REFERENCES `people`
 (`people_id`);
 
 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;

sql/sp_head.cc

@@ -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,
            key_buff + stab->db_length + 1, stab->table_name_length,
            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);
 
       tab_buff+= ALIGN_SIZE(sizeof(TABLE_LIST));

sql/sql_base.cc

@@ -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
   elements:
@@ -4841,6 +4860,52 @@ handle_table(THD *thd, Query_tables_list *prelocking_ctx,
           add_tables_and_routines_for_triggers(thd, prelocking_ctx, table_list))
         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;

sql/sql_class.cc

@@ -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)
   {
      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;
         break;

sql/table.h

@@ -1713,14 +1713,16 @@ struct TABLE_LIST
                              size_t table_name_length_arg,
                              const char *alias_arg,
                              enum thr_lock_type lock_type_arg,
+                             bool routine,
                              TABLE_LIST *belong_to_view_arg,
                              uint8 trg_event_map_arg,
                              TABLE_LIST ***last_ptr)
   {
     init_one_table(db_name_arg, db_length_arg, table_name_arg,
                    table_name_length_arg, alias_arg, lock_type_arg);
     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;
     trg_event_map= trg_event_map_arg;
 
@@ -2004,7 +2006,7 @@ struct TABLE_LIST
     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.
   */
-  bool          prelocking_placeholder;
+  enum { USER, ROUTINE, FK } prelocking_placeholder;
   /**
      Indicates that if TABLE_LIST object corresponds to the table/view
      which requires special handling.