We should prohobit concurent read of inserting file in SP

because it can couse problem with Query cache (BUG#14767)

mysql-test/r/query_cache_notembedded.result

@@ -314,4 +314,34 @@ drop procedure f2;
 drop procedure f3;
 drop procedure f4;
 drop table t1;
+reset query cache;
+drop function if exists f1;
+create table t1 (id int);
+create function f1 ()
+returns int
+begin
+declare i_var int;
+set i_var = sleep(3);
+insert into t1 values(3);
+set i_var = sleep(3);
+return 0;
+end;|
+ select f1();
+select sleep(4);
+sleep(4)
+0
+select * from t1;
+id
+3
+f1()
+0
+select * from t1;
+id
+3
+reset query cache;
+select * from t1;
+id
+3
+drop table t1;
+drop function f1;
 set GLOBAL query_cache_size=0;

mysql-test/t/query_cache_notembedded.test

@@ -180,5 +180,45 @@ drop procedure f3;
 drop procedure f4;
 drop table t1;
 
+#
+# bug#14767: INSERT in SF + concurrent SELECT with query cache
+#
+reset query cache;
+--disable_warnings
+drop function if exists f1;
+--enable_warnings
+create table t1 (id int);
+delimiter |;
+create function f1 ()
+  returns int
+begin
+  declare i_var int;
+  set i_var = sleep(3);
+  insert into t1 values(3);
+  set i_var = sleep(3);
+  return 0;
+end;|
+delimiter ;|
+
+connect (con1,localhost,root,,);
+connect (con2,localhost,root,,);
+
+connection con1;
+send select f1();
+connection con2;
+select sleep(4);
+select * from t1;
+connection con1;
+reap;
+connection con2;
+# This gives wrong result i.e. 't' table seems to be empty
+select * from t1;
+reset query cache;
+select * from t1;
+drop table t1;
+drop function f1;
+disconnect con1;
+disconnect con2;
+connection default;
 
 set GLOBAL query_cache_size=0;

sql/sql_insert.cc

@@ -258,6 +258,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
   bool log_on= (thd->options & OPTION_BIN_LOG) ||
     (!(thd->security_ctx->master_access & SUPER_ACL));
   bool transactional_table, joins_freed= FALSE;
+  bool changed;
   uint value_count;
   ulong counter = 1;
   ulonglong id;
@@ -544,20 +545,17 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
     else if (table->next_number_field && info.copied)
       id=table->next_number_field->val_int();	// Return auto_increment value
 
+    transactional_table= table->file->has_transactions();
+
+    if ((changed= (info.copied || info.deleted || info.updated)))
+    {
       /*
         Invalidate the table in the query cache if something changed.
         For the transactional algorithm to work the invalidation must be
         before binlog writing and ha_autocommit_or_rollback
       */
-    if (info.copied || info.deleted || info.updated)
-    {
       query_cache_invalidate3(thd, table_list, 1);
-    }
-
-    transactional_table= table->file->has_transactions();
-
-    if ((info.copied || info.deleted || info.updated) &&
-	(error <= 0 || !transactional_table))
+      if (error <= 0 || !transactional_table)
       {
         if (mysql_bin_log.is_open())
         {
@@ -571,12 +569,23 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
         if (!transactional_table)
           thd->options|=OPTION_STATUS_NO_TRANS_UPDATE;
       }
+    }
     if (transactional_table)
       error=ha_autocommit_or_rollback(thd,error);
 
     if (thd->lock)
     {
       mysql_unlock_tables(thd, thd->lock);
+      /*
+        Invalidate the table in the query cache if something changed
+        after unlocking when changes become fisible.
+        TODO: this is workaround. right way will be move invalidating in
+        the unlock procedure.
+      */
+      if (lock_type ==  TL_WRITE_CONCURRENT_INSERT && changed)
+      {
+        query_cache_invalidate3(thd, table_list, 1);
+      }
       thd->lock=0;
     }
   }

sql/sql_parse.cc

@@ -3314,6 +3314,19 @@ end_with_restore_list:
         select_lex->context.table_list= 
           select_lex->context.first_name_resolution_table= second_table;
 	res= handle_select(thd, lex, result, OPTION_SETUP_TABLES_DONE);
+        /*
+          Invalidate the table in the query cache if something changed
+          after unlocking when changes become visible.
+          TODO: this is workaround. right way will be move invalidating in
+          the unlock procedure.
+        */
+        if (first_table->lock_type ==  TL_WRITE_CONCURRENT_INSERT &&
+            thd->lock)
+        {
+          mysql_unlock_tables(thd, thd->lock);
+          query_cache_invalidate3(thd, first_table, 1);
+          thd->lock=0;
+        }
         delete result;
       }
       /* revert changes for SP */

sql/sql_yacc.yy

@@ -6068,7 +6068,19 @@ replace:
 	;
 
 insert_lock_option:
-	/* empty */	{ $$= TL_WRITE_CONCURRENT_INSERT; }
+	/* empty */
+          {
+#ifdef HAVE_QUERY_CACHE
+            /*
+              If it is SP we do not allow insert optimisation whan result of
+              insert visible only after the table unlocking but everyone can
+              read table.
+            */
+            $$= (Lex->sphead ? TL_WRITE :TL_WRITE_CONCURRENT_INSERT);
+#else
+            $$= TL_WRITE_CONCURRENT_INSERT;
+#endif
+          }
 	| LOW_PRIORITY	{ $$= TL_WRITE_LOW_PRIORITY; }
 	| DELAYED_SYM	{ $$= TL_WRITE_DELAYED; }
 	| HIGH_PRIORITY { $$= TL_WRITE; }
@@ -6925,7 +6937,16 @@ opt_local:
 
 load_data_lock:
 	/* empty */	{ $$= YYTHD->update_lock_default; }
-	| CONCURRENT	{ $$= TL_WRITE_CONCURRENT_INSERT ; }
+	| CONCURRENT
+          {
+#ifdef HAVE_QUERY_CACHE
+            /*
+              Ignore this option in SP to avoid problem with query cache
+            */
+            if (Lex->sphead != 0)
+#endif
+              $$= TL_WRITE_CONCURRENT_INSERT;
+          }
 	| LOW_PRIORITY	{ $$= TL_WRITE_LOW_PRIORITY; };