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:
  BUG#14767 test suite.
mysql-test/t/query_cache_notembedded.test:
  BUG#14767 test suite.
sql/sql_insert.cc:
  Query cache invalidation table we was inserting in just after
  unlocking table added to avoid the race condition as we had
  with SP.
sql/sql_parse.cc:
  Query cache invalidation table we was inserting in just after
  unlocking table added to avoid the race condition as we had
  with SP.
sql/sql_yacc.yy:
  We should prohibit concurent read of unserting file in SP
  because it can cause problem with query cache.

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,32 +545,30 @@ 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
 
-    /*
-      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 ((changed= (info.copied || info.deleted || info.updated)))
     {
-      if (mysql_bin_log.is_open())
+      /*
+        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
+      */
+      query_cache_invalidate3(thd, table_list, 1);
+      if (error <= 0 || !transactional_table)
       {
-        if (error <= 0)
-          thd->clear_error();
-	Query_log_event qinfo(thd, thd->query, thd->query_length,
-			      transactional_table, FALSE);
-	if (mysql_bin_log.write(&qinfo) && transactional_table)
-	  error=1;
+        if (mysql_bin_log.is_open())
+        {
+          if (error <= 0)
+            thd->clear_error();
+          Query_log_event qinfo(thd, thd->query, thd->query_length,
+                                transactional_table, FALSE);
+          if (mysql_bin_log.write(&qinfo) && transactional_table)
+            error=1;
+        }
+        if (!transactional_table)
+          thd->options|=OPTION_STATUS_NO_TRANS_UPDATE;
       }
-      if (!transactional_table)
-	thd->options|=OPTION_STATUS_NO_TRANS_UPDATE;
     }
     if (transactional_table)
       error=ha_autocommit_or_rollback(thd,error);
@@ -577,6 +576,16 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list,
     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 @@ mysql_execute_command(THD *thd)
         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; };