List:Commits« Previous MessageNext Message »
From:Gopal Shankar Date:July 20 2012 7:03am
Subject:bzr push into mysql-5.6 branch (gopal.shankar:4052 to 4053) Bug#13036505
View as plain text  
 4053 Gopal Shankar	2012-07-20
      Bug#13036505 62540: TABLE LOCKS WITHIN STORED FUNCTIONS ARE BACK IN
                          5.5 WITH MIXED AND ROW BI.
      
      Problem:-
      -------
      SELECT/SET/DO statements that used stored functions but didn't change
      any data acquired too strong locks on tables used (i.e. read) in these
      functions if binary logging was on and used statement or mixed mode.
      For MyISAM tables this resulted in that concurrent insert to such
      tables were blocked while such a statement was running. For InnoDB
      tables such statements were using locking reads (and as result blocked
      any concurrent changes and SELECT ... FOR UPDATE statements) instead
      of using snapshot isolation.
      
      Analysis:
      --------
      Due to a statement-based replication limitation, statements such as
      INSERT INTO .. SELECT FROM .. and CREATE TABLE .. SELECT FROM need
      to grab a TL_READ_NO_INSERT lock on the source table in order to
      prevent the replication of a concurrent statement that modifies the
      source table. If such a statement gets applied on the slave before
      the INSERT .. SELECT statement finishes, data on the master could
      differ from data on the slave and end-up with a discrepancy between
      the binary log and table state.
      
      This also applies to SELECT/SET/DO statements which use stored
      functions. Calls to such functions are going to be logged as a
      whole and thus should be serialized against concurrent changes
      to tables used by those functions. The current implementation
      does not check if functions only read data and won't be written
      into binary log as result. Currently we use TL_READ_NO_INSERT lock
      for all tables used by stored functions called from SELECTs if
      binary logging is on and uses statement or mixed mode.
      
      Note that even though InnoDB engine does its own locking it still
      relies on thr_lock.c locks set by SQL-layer to infer type of
      row-locks to acquire when reading data. Since TL_READ_NO_INSERT
      is translated to locking reads in InnoDB the above means that
      SELECT/SET/DO that uses stored functions will do locking reads
      on tables used by routines.
      
      We can use weaker type of lock TL_READ, which will allow concurrent
      inserts, if a statement only reads data (since such a statement
      won't get into binary log anyway). For InnoDB this lock will be
      translated to non-locking, snapshot reads (unless in serializable
      mode).
      
      Fix:-
      ---
      If we know that all stored routines which are going to be called by
      statement do not change tables we can choose weaker TL_READ lock for
      tables used by it.
      
      If some substatement which is added to sp_head modifies data, then
      we set this flag. This flag is used in read_lock_type_for_table()
      to choose weaker TL_READ lock for tables.
      
      Currently open_tables() processes a loop, where it initially
      processes known tables and routines, and further in each iteration
      it processes new tables and routines eventually found. Currently
      the lock upgrade happens within the loop where complete
      table/routine list for statement is not known, within which
      we cannot decide if lock upgrade is needed or to be ignored.
      This fix handles lock upgrade upon loop completion. When the
      complete list of tables get prepared, we check if there were
      any SF which write data, if not, we ignore upgrading lock.
      
      Test case:-
      --------
      main.lock_sync and main.innodb_mysql_lock2 test most of above
      mentioned scenario's. These tests have been updated according
      to new behavior. A new test case has been added to test
      SF which modifies temporary table.

    modified:
      mysql-test/r/innodb_mysql_lock2.result
      mysql-test/r/lock_sync.result
      mysql-test/t/innodb_mysql_lock2.test
      mysql-test/t/lock_sync.test
      sql/sp_head.cc
      sql/sp_head.h
      sql/sql_base.cc
      sql/sql_base.h
      sql/sql_update.cc
 4052 Marc Alff	2012-07-19
      Post merge fix

    modified:
      storage/perfschema/pfs_instr.cc
=== modified file 'mysql-test/r/innodb_mysql_lock2.result'
--- a/mysql-test/r/innodb_mysql_lock2.result	2011-05-20 11:50:50 +0000
+++ b/mysql-test/r/innodb_mysql_lock2.result	2012-07-20 06:55:34 +0000
@@ -331,13 +331,14 @@ Success: 'update v2 set j= j-10 where j
 # 4.1 SELECT/SET with a stored function which does not 
 #     modify data and uses SELECT in its turn.
 #
-# In theory there is no need to take row locks on the table
+# There is no need to take row locks on the table
 # being selected from in SF as the call to such function
-# won't get into the binary log. In practice, however, we
-# discover that fact too late in the process to be able to
-# affect the decision what locks should be taken.
-# Hence, strong locks are taken in this case.
-Success: 'select f1()' takes shared row locks on 't1'.
+# won't get into the binary log.
+#
+# However in practice innodb takes strong lock on tables
+# being selected from within SF, when SF is called from
+# non SELECT statements like 'set' statement below.
+Success: 'select f1()' doesn't take row locks on 't1'.
 Success: 'set @a:= f1()' takes shared row locks on 't1'.
 #
 # 4.2 INSERT (or other statement which modifies data) with
@@ -364,13 +365,15 @@ Success: 'set @a:= f2()' takes shared ro
 #      modify data and reads a table through subselect
 #      in a control construct.
 #
-# Again, in theory a call to this function won't get to the
-# binary log and thus no locking is needed. But in practice
-# we don't detect this fact early enough (get_lock_type_for_table())
-# to avoid taking row locks.
-Success: 'select f3()' takes shared row locks on 't1'.
+# Call to this function won't get to the
+# binary log and thus no locking is needed.
+#
+# However in practice innodb takes strong lock on tables
+# being selected from within SF, when SF is called from
+# non SELECT statements like 'set' statement below.
+Success: 'select f3()' doesn't take row locks on 't1'.
 Success: 'set @a:= f3()' takes shared row locks on 't1'.
-Success: 'select f4()' takes shared row locks on 't1'.
+Success: 'select f4()' doesn't take row locks on 't1'.
 Success: 'set @a:= f4()' takes shared row locks on 't1'.
 #
 # 4.5. INSERT (or other statement which modifies data) with
@@ -398,13 +401,15 @@ Success: 'set @a:= f5()' takes shared ro
 #     doesn't modify data and reads tables through
 #     a view.
 #
-# Once again, in theory, calls to such functions won't
-# get into the binary log and thus don't need row
-# locks. But in practice this fact is discovered
-# too late to have any effect.
-Success: 'select f6()' takes shared row locks on 't1'.
+# Calls to such functions won't get into
+# the binary log and thus don't need row locks.
+#
+# However in practice innodb takes strong lock on tables
+# being selected from within SF, when SF is called from
+# non SELECT statements like 'set' statement below.
+Success: 'select f6()' doesn't take row locks on 't1'.
 Success: 'set @a:= f6()' takes shared row locks on 't1'.
-Success: 'select f7()' takes shared row locks on 't1'.
+Success: 'select f7()' doesn't take row locks on 't1'.
 Success: 'set @a:= f7()' takes shared row locks on 't1'.
 #
 # 4.8 INSERT which uses stored function which
@@ -431,10 +436,9 @@ Success: 'select f9()' takes shared row
 #      data and reads a table indirectly, by calling another
 #      function.
 #
-# In theory, calls to such functions won't get into the binary
-# log and thus don't need to acquire row locks. But in practice
-# this fact is discovered too late to have any effect.
-Success: 'select f10()' takes shared row locks on 't1'.
+# Calls to such functions won't get into the binary
+# log and thus don't need to acquire row locks.
+Success: 'select f10()' doesn't take row locks on 't1'.
 #
 # 4.11 INSERT which uses a stored function which doesn't modify
 #      data and reads a table indirectly, by calling another
@@ -494,10 +498,9 @@ Success: 'select f14()' takes shared row
 # 5.3 SELECT that calls a function that doesn't modify data and
 #     uses a CALL statement that reads a table via SELECT.
 #
-# In theory, calls to such functions won't get into the binary
-# log and thus don't need to acquire row locks. But in practice
-# this fact is discovered too late to have any effect.
-Success: 'select f15()' takes shared row locks on 't1'.
+# Calls to such functions won't get into the binary
+# log and thus don't need to acquire row locks.
+Success: 'select f15()' doesn't take row locks on 't1'.
 #
 # 5.4 INSERT which calls function which doesn't modify data and
 #     uses CALL statement which reads table through SELECT.

=== modified file 'mysql-test/r/lock_sync.result'
--- a/mysql-test/r/lock_sync.result	2011-03-29 08:42:29 +0000
+++ b/mysql-test/r/lock_sync.result	2012-07-20 06:55:34 +0000
@@ -27,6 +27,7 @@ drop table if exists t0, t1, t2, t3, t4,
 drop view if exists v1, v2;
 drop procedure if exists p1;
 drop procedure if exists p2;
+drop procedure if exists p3;
 drop function if exists f1;
 drop function if exists f2;
 drop function if exists f3;
@@ -42,6 +43,8 @@ drop function if exists f12;
 drop function if exists f13;
 drop function if exists f14;
 drop function if exists f15;
+drop function if exists f16;
+drop function if exists f17;
 create table t1 (i int primary key);
 insert into t1 values (1), (2), (3), (4), (5);
 create table t2 (j int primary key);
@@ -146,6 +149,26 @@ declare k int;
 call p2(k);
 return k;
 end|
+create function f16() returns int
+begin
+create temporary table if not exists temp1 (a int);
+insert into temp1 select * from t1;
+drop temporary table temp1;
+return 1;
+end|
+create function f17() returns int
+begin
+declare j int;
+select i from t1 where i = 1 into j;
+call p3;
+return 1;
+end|
+create procedure p3()
+begin
+create temporary table if not exists temp1 (a int);
+insert into temp1 select * from t1;
+drop temporary table temp1;
+end|
 create trigger t4_bi before insert on t4 for each row
 begin
 declare k int;
@@ -185,6 +208,7 @@ end|
 # once during its execution.
 show create procedure p1;
 show create procedure p2;
+show create procedure p3;
 show create function f1;
 show create function f2;
 show create function f3;
@@ -200,6 +224,8 @@ show create function f12;
 show create function f13;
 show create function f14;
 show create function f15;
+show create function f16;
+show create function f17;
 # Switch back to connection 'default'.
 #
 # 1. Statements that read tables and do not use subqueries.
@@ -359,14 +385,11 @@ Success: 'update v2 set j= j-10 where j
 # 4.1 SELECT/SET with a stored function which does not 
 #     modify data and uses SELECT in its turn.
 #
-# In theory there is no need to take strong locks on the table
+# There is no need to take strong locks on the table
 # being selected from in SF as the call to such function
-# won't get into the binary log. In practice, however, we
-# discover that fact too late in the process to be able to
-# affect the decision what locks should be taken.
-# Hence, strong locks are taken in this case.
-Success: 'select f1()' doesn't allow concurrent inserts into 't1'.
-Success: 'set @a:= f1()' doesn't allow concurrent inserts into 't1'.
+# won't get into the binary log.
+Success: 'select f1()' allows concurrent inserts into 't1'.
+Success: 'set @a:= f1()' allows concurrent inserts into 't1'.
 #
 # 4.2 INSERT (or other statement which modifies data) with
 #     a stored function which does not modify data and uses
@@ -392,14 +415,12 @@ Success: 'set @a:= f2()' doesn't allow c
 #      modify data and reads a table through subselect
 #      in a control construct.
 #
-# Again, in theory a call to this function won't get to the
-# binary log and thus no strong lock is needed. But in practice
-# we don't detect this fact early enough (get_lock_type_for_table())
-# to avoid taking a strong lock.
-Success: 'select f3()' doesn't allow concurrent inserts into 't1'.
-Success: 'set @a:= f3()' doesn't allow concurrent inserts into 't1'.
-Success: 'select f4()' doesn't allow concurrent inserts into 't1'.
-Success: 'set @a:= f4()' doesn't allow concurrent inserts into 't1'.
+# Call to this function won't get to the
+# binary log and thus no strong lock is needed.
+Success: 'select f3()' allows concurrent inserts into 't1'.
+Success: 'set @a:= f3()' allows concurrent inserts into 't1'.
+Success: 'select f4()' allows concurrent inserts into 't1'.
+Success: 'set @a:= f4()' allows concurrent inserts into 't1'.
 #
 # 4.5. INSERT (or other statement which modifies data) with
 #      a stored function which does not modify data and reads
@@ -426,14 +447,13 @@ Success: 'set @a:= f5()' doesn't allow c
 #     doesn't modify data and reads tables through
 #     a view.
 #
-# Once again, in theory, calls to such functions won't
-# get into the binary log and thus don't need strong
-# locks. But in practice this fact is discovered
-# too late to have any effect.
-Success: 'select f6()' doesn't allow concurrent inserts into 't1'.
-Success: 'set @a:= f6()' doesn't allow concurrent inserts into 't1'.
-Success: 'select f7()' doesn't allow concurrent inserts into 't1'.
-Success: 'set @a:= f7()' doesn't allow concurrent inserts into 't1'.
+# Calls to such functions won't get into
+# the binary log and thus don't need strong
+# locks.
+Success: 'select f6()' allows concurrent inserts into 't1'.
+Success: 'set @a:= f6()' allows concurrent inserts into 't1'.
+Success: 'select f7()' allows concurrent inserts into 't1'.
+Success: 'set @a:= f7()' allows concurrent inserts into 't1'.
 #
 # 4.8 INSERT which uses stored function which
 #     doesn't modify data and reads a table
@@ -459,10 +479,9 @@ Success: 'select f9()' doesn't allow con
 #      data and reads a table indirectly, by calling another
 #      function.
 #
-# In theory, calls to such functions won't get into the binary
-# log and thus don't need to acquire strong locks. But in practice
-# this fact is discovered too late to have any effect.
-Success: 'select f10()' doesn't allow concurrent inserts into 't1'.
+# Calls to such functions won't get into the binary
+# log and thus don't need to acquire strong locks.
+Success: 'select f10()' allows concurrent inserts into 't1'.
 #
 # 4.11 INSERT which uses a stored function which doesn't modify
 #      data and reads a table indirectly, by calling another
@@ -501,6 +520,26 @@ Success: 'select f12((select i+10 from t
 # uses. Therefore it should take strong locks on the data it reads.
 Success: 'insert into t2 values (f13((select i+10 from t1 where i=1)))' doesn't allow concurrent inserts into 't1'.
 #
+# 4.15 SELECT/SET with a stored function which 
+#      inserts data into a temporary table using
+#      SELECT on t1.
+#
+# Since this statement is written to the binary log it should
+# be serialized with concurrent statements affecting the data it
+# uses. Therefore it should take strong locks on the data it reads.
+Success: 'select f16()' doesn't allow concurrent inserts into 't1'.
+Success: 'set @a:= f16()' doesn't allow concurrent inserts into 't1'.
+#
+# 4.16 SELECT/SET with a stored function which call procedure
+#      which inserts data into a temporary table using
+#      SELECT on t1.
+#
+# Since this statement is written to the binary log it should
+# be serialized with concurrent statements affecting the data it
+# uses. Therefore it should take strong locks on the data it reads.
+Success: 'select f17()' doesn't allow concurrent inserts into 't1'.
+Success: 'set @a:= f17()' doesn't allow concurrent inserts into 't1'.
+#
 # 5. Statements that read tables through stored procedures.
 #
 #
@@ -522,10 +561,9 @@ Success: 'select f14()' doesn't allow co
 # 5.3 SELECT that calls a function that doesn't modify data and
 #     uses a CALL statement that reads a table via SELECT.
 #
-# In theory, calls to such functions won't get into the binary
-# log and thus don't need to acquire strong locks. But in practice
-# this fact is discovered too late to have any effect.
-Success: 'select f15()' doesn't allow concurrent inserts into 't1'.
+# Calls to such functions won't get into the binary
+# log and thus don't need to acquire strong locks.
+Success: 'select f15()' allows concurrent inserts into 't1'.
 #
 # 5.4 INSERT which calls function which doesn't modify data and
 #     uses CALL statement which reads table through SELECT.
@@ -585,9 +623,12 @@ drop function f12;
 drop function f13;
 drop function f14;
 drop function f15;
+drop function f16;
+drop function f17;
 drop view v1, v2;
 drop procedure p1;
 drop procedure p2;
+drop procedure p3;
 drop table t1, t2, t3, t4, t5;
 set @@global.concurrent_insert= @old_concurrent_insert;
 #

=== modified file 'mysql-test/t/innodb_mysql_lock2.test'
--- a/mysql-test/t/innodb_mysql_lock2.test	2012-02-06 19:24:47 +0000
+++ b/mysql-test/t/innodb_mysql_lock2.test	2012-07-20 06:55:34 +0000
@@ -440,15 +440,16 @@ let $wait_statement= $statement;
 --echo # 4.1 SELECT/SET with a stored function which does not 
 --echo #     modify data and uses SELECT in its turn.
 --echo #
---echo # In theory there is no need to take row locks on the table
+--echo # There is no need to take row locks on the table
 --echo # being selected from in SF as the call to such function
---echo # won't get into the binary log. In practice, however, we
---echo # discover that fact too late in the process to be able to
---echo # affect the decision what locks should be taken.
---echo # Hence, strong locks are taken in this case.
+--echo # won't get into the binary log.
+--echo #
+--echo # However in practice innodb takes strong lock on tables
+--echo # being selected from within SF, when SF is called from
+--echo # non SELECT statements like 'set' statement below.
 let $statement= select f1();
 let $wait_statement= select i from t1 where i = 1 into j;
---source include/check_shared_row_lock.inc
+--source include/check_no_row_lock.inc
 let $statement= set @a:= f1();
 let $wait_statement= select i from t1 where i = 1 into j;
 --source include/check_shared_row_lock.inc
@@ -486,19 +487,21 @@ let $wait_statement= select i from t1 wh
 --echo #      modify data and reads a table through subselect
 --echo #      in a control construct.
 --echo #
---echo # Again, in theory a call to this function won't get to the
---echo # binary log and thus no locking is needed. But in practice
---echo # we don't detect this fact early enough (get_lock_type_for_table())
---echo # to avoid taking row locks.
+--echo # Call to this function won't get to the
+--echo # binary log and thus no locking is needed.
+--echo #
+--echo # However in practice innodb takes strong lock on tables
+--echo # being selected from within SF, when SF is called from
+--echo # non SELECT statements like 'set' statement below.
 let $statement= select f3();
 let $wait_statement= $statement;
---source include/check_shared_row_lock.inc
+--source include/check_no_row_lock.inc
 let $statement= set @a:= f3();
 let $wait_statement= $statement;
 --source include/check_shared_row_lock.inc
 let $statement= select f4();
 let $wait_statement= $statement;
---source include/check_shared_row_lock.inc
+--source include/check_no_row_lock.inc
 let $statement= set @a:= f4();
 let $wait_statement= $statement;
 --source include/check_shared_row_lock.inc
@@ -539,19 +542,21 @@ let $wait_statement= insert into t2 valu
 --echo #     doesn't modify data and reads tables through
 --echo #     a view.
 --echo #
---echo # Once again, in theory, calls to such functions won't
---echo # get into the binary log and thus don't need row
---echo # locks. But in practice this fact is discovered
---echo # too late to have any effect.
+--echo # Calls to such functions won't get into
+--echo # the binary log and thus don't need row locks.
+--echo #
+--echo # However in practice innodb takes strong lock on tables
+--echo # being selected from within SF, when SF is called from
+--echo # non SELECT statements like 'set' statement below.
 let $statement= select f6();
 let $wait_statement= select i from v1 where i = 1 into k;
---source include/check_shared_row_lock.inc
+--source include/check_no_row_lock.inc
 let $statement= set @a:= f6();
 let $wait_statement= select i from v1 where i = 1 into k;
 --source include/check_shared_row_lock.inc
 let $statement= select f7();
 let $wait_statement= select j from v2 where j = 1 into k;
---source include/check_shared_row_lock.inc
+--source include/check_no_row_lock.inc
 let $statement= set @a:= f7();
 let $wait_statement= select j from v2 where j = 1 into k;
 --source include/check_shared_row_lock.inc
@@ -592,12 +597,11 @@ let $wait_statement= update v2 set j=j+1
 --echo #      data and reads a table indirectly, by calling another
 --echo #      function.
 --echo #
---echo # In theory, calls to such functions won't get into the binary
---echo # log and thus don't need to acquire row locks. But in practice
---echo # this fact is discovered too late to have any effect.
+--echo # Calls to such functions won't get into the binary
+--echo # log and thus don't need to acquire row locks.
 let $statement= select f10();
 let $wait_statement= select i from t1 where i = 1 into j;
---source include/check_shared_row_lock.inc
+--source include/check_no_row_lock.inc
 
 --echo #
 --echo # 4.11 INSERT which uses a stored function which doesn't modify
@@ -676,12 +680,11 @@ let $wait_statement= select i from t1 wh
 --echo # 5.3 SELECT that calls a function that doesn't modify data and
 --echo #     uses a CALL statement that reads a table via SELECT.
 --echo #
---echo # In theory, calls to such functions won't get into the binary
---echo # log and thus don't need to acquire row locks. But in practice
---echo # this fact is discovered too late to have any effect.
+--echo # Calls to such functions won't get into the binary
+--echo # log and thus don't need to acquire row locks.
 let $statement= select f15();
 let $wait_statement= select i from t1 where i = 1 into p;
---source include/check_shared_row_lock.inc
+--source include/check_no_row_lock.inc
 
 --echo #
 --echo # 5.4 INSERT which calls function which doesn't modify data and

=== modified file 'mysql-test/t/lock_sync.test'
--- a/mysql-test/t/lock_sync.test	2012-02-06 19:24:47 +0000
+++ b/mysql-test/t/lock_sync.test	2012-07-20 06:55:34 +0000
@@ -49,6 +49,7 @@ drop table if exists t0, t1, t2, t3, t4,
 drop view if exists v1, v2;
 drop procedure if exists p1;
 drop procedure if exists p2;
+drop procedure if exists p3;
 drop function if exists f1;
 drop function if exists f2;
 drop function if exists f3;
@@ -64,6 +65,8 @@ drop function if exists f12;
 drop function if exists f13;
 drop function if exists f14;
 drop function if exists f15;
+drop function if exists f16;
+drop function if exists f17;
 --enable_warnings
 create table t1 (i int primary key);
 insert into t1 values (1), (2), (3), (4), (5);
@@ -170,6 +173,26 @@ begin
   call p2(k);
   return k;
 end|
+create function f16() returns int
+begin
+  create temporary table if not exists temp1 (a int);
+  insert into temp1 select * from t1;
+  drop temporary table temp1;
+  return 1;
+end|
+create function f17() returns int
+begin
+  declare j int;
+  select i from t1 where i = 1 into j;
+  call p3;
+  return 1;
+end|
+create procedure p3()
+begin
+  create temporary table if not exists temp1 (a int);
+  insert into temp1 select * from t1;
+  drop temporary table temp1;
+end|
 create trigger t4_bi before insert on t4 for each row
 begin
   declare k int;
@@ -217,6 +240,7 @@ connection con1;
 --disable_result_log
 show create procedure p1;
 show create procedure p2;
+show create procedure p3;
 show create function f1;
 show create function f2;
 show create function f3;
@@ -232,6 +256,8 @@ show create function f12;
 show create function f13;
 show create function f14;
 show create function f15;
+show create function f16;
+show create function f17;
 --enable_result_log
 --echo # Switch back to connection 'default'.
 connection default;
@@ -492,18 +518,15 @@ let $restore_table= t2;
 --echo # 4.1 SELECT/SET with a stored function which does not 
 --echo #     modify data and uses SELECT in its turn.
 --echo #
---echo # In theory there is no need to take strong locks on the table
+--echo # There is no need to take strong locks on the table
 --echo # being selected from in SF as the call to such function
---echo # won't get into the binary log. In practice, however, we
---echo # discover that fact too late in the process to be able to
---echo # affect the decision what locks should be taken.
---echo # Hence, strong locks are taken in this case.
+--echo # won't get into the binary log.
 let $statement= select f1();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 let $statement= set @a:= f1();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 
 --echo #
 --echo # 4.2 INSERT (or other statement which modifies data) with
@@ -538,22 +561,20 @@ let $restore_table= t2;
 --echo #      modify data and reads a table through subselect
 --echo #      in a control construct.
 --echo #
---echo # Again, in theory a call to this function won't get to the
---echo # binary log and thus no strong lock is needed. But in practice
---echo # we don't detect this fact early enough (get_lock_type_for_table())
---echo # to avoid taking a strong lock.
+--echo # Call to this function won't get to the
+--echo # binary log and thus no strong lock is needed.
 let $statement= select f3();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 let $statement= set @a:= f3();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 let $statement= select f4();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 let $statement= set @a:= f4();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 
 --echo #
 --echo # 4.5. INSERT (or other statement which modifies data) with
@@ -591,22 +612,21 @@ let $restore_table= t2;
 --echo #     doesn't modify data and reads tables through
 --echo #     a view.
 --echo #
---echo # Once again, in theory, calls to such functions won't
---echo # get into the binary log and thus don't need strong
---echo # locks. But in practice this fact is discovered
---echo # too late to have any effect.
+--echo # Calls to such functions won't get into
+--echo # the binary log and thus don't need strong
+--echo # locks.
 let $statement= select f6();
 let $restore_table= t2;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 let $statement= set @a:= f6();
 let $restore_table= t2;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 let $statement= select f7();
 let $restore_table= t2;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 let $statement= set @a:= f7();
 let $restore_table= t2;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 
 --echo #
 --echo # 4.8 INSERT which uses stored function which
@@ -644,12 +664,11 @@ let $restore_table= t2;
 --echo #      data and reads a table indirectly, by calling another
 --echo #      function.
 --echo #
---echo # In theory, calls to such functions won't get into the binary
---echo # log and thus don't need to acquire strong locks. But in practice
---echo # this fact is discovered too late to have any effect.
+--echo # Calls to such functions won't get into the binary
+--echo # log and thus don't need to acquire strong locks.
 let $statement= select f10();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 
 --echo #
 --echo # 4.11 INSERT which uses a stored function which doesn't modify
@@ -700,6 +719,36 @@ let $statement= insert into t2 values (f
 let $restore_table= t2;
 --source include/check_no_concurrent_insert.inc
 
+--echo #
+--echo # 4.15 SELECT/SET with a stored function which 
+--echo #      inserts data into a temporary table using
+--echo #      SELECT on t1.
+--echo #
+--echo # Since this statement is written to the binary log it should
+--echo # be serialized with concurrent statements affecting the data it
+--echo # uses. Therefore it should take strong locks on the data it reads.
+let $statement= select f16();
+let $restore_table= ;
+--source include/check_no_concurrent_insert.inc
+let $statement= set @a:= f16();
+let $restore_table= ;
+--source include/check_no_concurrent_insert.inc
+
+--echo #
+--echo # 4.16 SELECT/SET with a stored function which call procedure
+--echo #      which inserts data into a temporary table using
+--echo #      SELECT on t1.
+--echo #
+--echo # Since this statement is written to the binary log it should
+--echo # be serialized with concurrent statements affecting the data it
+--echo # uses. Therefore it should take strong locks on the data it reads.
+let $statement= select f17();
+let $restore_table= ;
+--source include/check_no_concurrent_insert.inc
+let $statement= set @a:= f17();
+let $restore_table= ;
+--source include/check_no_concurrent_insert.inc
+
 
 --echo #
 --echo # 5. Statements that read tables through stored procedures.
@@ -730,12 +779,11 @@ let $restore_table= t2;
 --echo # 5.3 SELECT that calls a function that doesn't modify data and
 --echo #     uses a CALL statement that reads a table via SELECT.
 --echo #
---echo # In theory, calls to such functions won't get into the binary
---echo # log and thus don't need to acquire strong locks. But in practice
---echo # this fact is discovered too late to have any effect.
+--echo # Calls to such functions won't get into the binary
+--echo # log and thus don't need to acquire strong locks.
 let $statement= select f15();
 let $restore_table= ;
---source include/check_no_concurrent_insert.inc
+--source include/check_concurrent_insert.inc
 
 --echo #
 --echo # 5.4 INSERT which calls function which doesn't modify data and
@@ -800,7 +848,6 @@ let $statement= update t5 set l= 2 where
 let $restore_table= t5;
 --source include/check_no_concurrent_insert.inc
 
-
 --echo # Clean-up.
 drop function f1;
 drop function f2;
@@ -817,9 +864,12 @@ drop function f12;
 drop function f13;
 drop function f14;
 drop function f15;
+drop function f16;
+drop function f17;
 drop view v1, v2;
 drop procedure p1;
 drop procedure p2;
+drop procedure p3;
 drop table t1, t2, t3, t4, t5;
 
 disconnect con1;

=== modified file 'sql/sp_head.cc'
--- a/sql/sp_head.cc	2012-05-22 13:35:37 +0000
+++ b/sql/sp_head.cc	2012-07-20 06:55:34 +0000
@@ -1446,6 +1446,10 @@ bool sp_head::restore_lex(THD *thd)
   if (sp_update_sp_used_routines(&m_sroutines, &sublex->sroutines))
     return true;
 
+  /* If this substatement is a update query, then mark MODIFIES_DATA */
+  if (is_update_query(sublex->sql_command))
+    m_flags|= MODIFIES_DATA;
+
   /*
     Merge tables used by this statement (but not by its functions or
     procedures) to multiset of tables used by this routine.

=== modified file 'sql/sp_head.h'
--- a/sql/sp_head.h	2012-05-16 13:58:54 +0000
+++ b/sql/sp_head.h	2012-07-20 06:55:34 +0000
@@ -453,7 +453,21 @@ public:
     LOG_SLOW_STATEMENTS= 256,   // Used by events
     LOG_GENERAL_LOG= 512,        // Used by events
     HAS_SQLCOM_RESET= 1024,
-    HAS_SQLCOM_FLUSH= 2048
+    HAS_SQLCOM_FLUSH= 2048,
+
+    /**
+      Marks routines that directly (i.e. not by calling other routines)
+      change tables. Note that this flag is set automatically based on
+      type of statements used in the stored routine and is different
+      from routine characteristic provided by user in a form of CONTAINS
+      SQL, READS SQL DATA, MODIFIES SQL DATA clauses. The latter are
+      accepted by parser but pretty much ignored after that.
+      We don't rely on them:
+      a) for compatibility reasons.
+      b) because in CONTAINS SQL case they don't provide enough
+      information anyway.
+     */
+    MODIFIES_DATA= 4096
   };
 
 public:
@@ -698,6 +712,15 @@ public:
   */
   bool add_instr(THD *thd, sp_instr *instr);
 
+  /**
+    Returns true if any substatement in the routine directly
+    (not through another routine) modifies data/changes table.
+
+    @sa Comment for MODIFIES_DATA flag.
+  */
+  bool modifies_data() const
+  { return m_flags & MODIFIES_DATA; }
+
   uint instructions()
   { return m_instructions.elements(); }
 

=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc	2012-06-01 00:04:28 +0000
+++ b/sql/sql_base.cc	2012-07-20 06:55:34 +0000
@@ -4112,9 +4112,12 @@ recover_from_failed_open(THD *thd)
 /*
   Return a appropriate read lock type given a table object.
 
-  @param thd Thread context
-  @param prelocking_ctx Prelocking context.
-  @param table_list     Table list element for table to be locked.
+  @param thd              Thread context
+  @param prelocking_ctx   Prelocking context.
+  @param table_list       Table list element for table to be locked.
+  @param routine_modifies_data 
+                          Some routine that is invoked by statement 
+                          modifies data.
 
   @remark Due to a statement-based replication limitation, statements such as
           INSERT INTO .. SELECT FROM .. and CREATE TABLE .. SELECT FROM need
@@ -4127,9 +4130,13 @@ recover_from_failed_open(THD *thd)
           This also applies to SELECT/SET/DO statements which use stored
           functions. Calls to such functions are going to be logged as a
           whole and thus should be serialized against concurrent changes
-          to tables used by those functions. This can be avoided if functions
-          only read data but doing so requires more complex analysis than it
-          is done now.
+          to tables used by those functions. This is avoided when functions
+          do not modify data but only read it, since in this case nothing is
+          written to the binary log. Argument routine_modifies_data
+          denotes the same. So effectively, if the statement is not a
+          update query and routine_modifies_data is false, then
+          prelocking_placeholder does not take importance.
+
           Furthermore, this does not apply to I_S and log tables as it's
           always unsafe to replicate such tables under statement-based
           replication as the table on the slave might contain other data
@@ -4144,7 +4151,8 @@ recover_from_failed_open(THD *thd)
 
 thr_lock_type read_lock_type_for_table(THD *thd,
                                        Query_tables_list *prelocking_ctx,
-                                       TABLE_LIST *table_list)
+                                       TABLE_LIST *table_list,
+                                       bool routine_modifies_data)
 {
   /*
     In cases when this function is called for a sub-statement executed in
@@ -4160,7 +4168,7 @@ thr_lock_type read_lock_type_for_table(T
       (table_list->table->s->table_category == TABLE_CATEGORY_RPL_INFO) ||
       (table_list->table->s->table_category == TABLE_CATEGORY_PERFORMANCE) ||
       !(is_update_query(prelocking_ctx->sql_command) ||
-        table_list->prelocking_placeholder ||
+        (routine_modifies_data && table_list->prelocking_placeholder) ||
         (thd->locked_tables_mode > LTM_LOCK_TABLES)))
     return TL_READ;
   else
@@ -4173,19 +4181,21 @@ thr_lock_type read_lock_type_for_table(T
   and, if prelocking strategy prescribes so, extend the prelocking set
   with tables and routines used by it.
 
-  @param[in]  thd                  Thread context.
-  @param[in]  prelocking_ctx       Prelocking context.
-  @param[in]  rt                   Element of prelocking set to be processed.
-  @param[in]  prelocking_strategy  Strategy which specifies how the
-                                   prelocking set should be extended when
-                                   one of its elements is processed.
-  @param[in]  has_prelocking_list  Indicates that prelocking set/list for
-                                   this statement has already been built.
-  @param[in]  ot_ctx               Context of open_table used to recover from
-                                   locking failures.
-  @param[out] need_prelocking      Set to TRUE if it was detected that this
-                                   statement will require prelocked mode for
-                                   its execution, not touched otherwise.
+  @param[in]  thd                   Thread context.
+  @param[in]  prelocking_ctx        Prelocking context.
+  @param[in]  rt                    Element of prelocking set to be processed.
+  @param[in]  prelocking_strategy   Strategy which specifies how the
+                                    prelocking set should be extended when
+                                    one of its elements is processed.
+  @param[in]  has_prelocking_list   Indicates that prelocking set/list for
+                                    this statement has already been built.
+  @param[in]  ot_ctx                Context of open_table used to recover from
+                                    locking failures.
+  @param[out] need_prelocking       Set to TRUE if it was detected that this
+                                    statement will require prelocked mode for
+                                    its execution, not touched otherwise.
+  @param[out] routine_modifies_data Set to TRUE if it was detected that this
+                                    routine does modify table data.
 
   @retval FALSE  Success.
   @retval TRUE   Failure (Conflicting metadata lock, OOM, other errors).
@@ -4197,9 +4207,10 @@ open_and_process_routine(THD *thd, Query
                          Prelocking_strategy *prelocking_strategy,
                          bool has_prelocking_list,
                          Open_table_context *ot_ctx,
-                         bool *need_prelocking)
+                         bool *need_prelocking, bool *routine_modifies_data)
 {
   MDL_key::enum_mdl_namespace mdl_type= rt->mdl_request.key.mdl_namespace();
+  *routine_modifies_data= false;
   DBUG_ENTER("open_and_process_routine");
 
   switch (mdl_type)
@@ -4254,10 +4265,13 @@ open_and_process_routine(THD *thd, Query
           DBUG_RETURN(TRUE);
 
         /* 'sp' is NULL when there is no such routine. */
-        if (sp && !has_prelocking_list)
+        if (sp)
         {
-          prelocking_strategy->handle_routine(thd, prelocking_ctx, rt, sp,
-                                              need_prelocking);
+          *routine_modifies_data= sp->modifies_data();
+
+          if (!has_prelocking_list)
+            prelocking_strategy->handle_routine(thd, prelocking_ctx, rt, sp,
+                                                need_prelocking);
         }
       }
       else
@@ -4598,18 +4612,6 @@ open_and_process_table(THD *thd, LEX *le
       goto end;
   }
 
-  /* Set appropriate TABLE::lock_type. */
-  if (tables->lock_type != TL_UNLOCK && ! thd->locked_tables_mode)
-  {
-    if (tables->lock_type == TL_WRITE_DEFAULT)
-      tables->table->reginfo.lock_type= thd->update_lock_default;
-    else if (tables->lock_type == TL_READ_DEFAULT)
-      tables->table->reginfo.lock_type=
-        read_lock_type_for_table(thd, lex, tables);
-    else
-      tables->table->reginfo.lock_type= tables->lock_type;
-  }
-
   /* Copy grant information from TABLE_LIST instance to TABLE one. */
   tables->table->grant= tables->grant;
 
@@ -4861,6 +4863,7 @@ bool open_tables(THD *thd, TABLE_LIST **
   Open_table_context ot_ctx(thd, flags);
   bool error= FALSE;
   MEM_ROOT new_frm_mem;
+  bool some_routine_modifies_data= FALSE;
   bool has_prelocking_list;
   DBUG_ENTER("open_tables");
 
@@ -5021,6 +5024,7 @@ restart:
     if (thd->locked_tables_mode <= LTM_LOCK_TABLES)
     {
       bool need_prelocking= FALSE;
+      bool routine_modifies_data;
       TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
       /*
         Process elements of the prelocking set which are present there
@@ -5036,7 +5040,8 @@ restart:
       {
         error= open_and_process_routine(thd, thd->lex, rt, prelocking_strategy,
                                         has_prelocking_list, &ot_ctx,
-                                        &need_prelocking);
+                                        &need_prelocking,
+                                        &routine_modifies_data);
 
         if (error)
         {
@@ -5061,6 +5066,9 @@ restart:
           */
           goto err;
         }
+
+        // Remember if any of SF modifies data.
+        some_routine_modifies_data|= routine_modifies_data;
       }
 
       if (need_prelocking && ! thd->lex->requires_prelocking())
@@ -5076,6 +5084,10 @@ restart:
     children, attach the children to their parents. At end of statement,
     the children are detached. Attaching and detaching are always done,
     even under LOCK TABLES.
+
+    We also convert all TL_WRITE_DEFAULT and TL_READ_DEFAULT locks to
+    appropriate "real" lock types to be used for locking and to be passed
+    to storage engine.
   */
   for (tables= *start; tables; tables= tables->next_global)
   {
@@ -5097,6 +5109,21 @@ restart:
         goto err;
       }
     }
+
+    /* Set appropriate TABLE::lock_type. */
+    if (tbl && tables->lock_type != TL_UNLOCK && 
+        !thd->locked_tables_mode)
+    {
+      if (tables->lock_type == TL_WRITE_DEFAULT)
+        tbl->reginfo.lock_type= thd->update_lock_default;
+      else if (tables->lock_type == TL_READ_DEFAULT)
+          tbl->reginfo.lock_type=
+            read_lock_type_for_table(thd, thd->lex, tables,
+                                     some_routine_modifies_data);
+      else
+        tbl->reginfo.lock_type= tables->lock_type;
+    }
+
   }
 
 err:
@@ -5351,11 +5378,15 @@ static bool check_lock_and_start_stmt(TH
     engine is important as, for example, InnoDB uses it to determine
     what kind of row locks should be acquired when executing statement
     in prelocked mode or under LOCK TABLES with @@innodb_table_locks = 0.
+
+    Last argument routine_modifies_data for read_lock_type_for_table()
+    is ignored, as prelocking placeholder will never be set here.
   */
+  DBUG_ASSERT(table_list->prelocking_placeholder == false);
   if (table_list->lock_type == TL_WRITE_DEFAULT)
     lock_type= thd->update_lock_default;
   else if (table_list->lock_type == TL_READ_DEFAULT)
-    lock_type= read_lock_type_for_table(thd, prelocking_ctx, table_list);
+    lock_type= read_lock_type_for_table(thd, prelocking_ctx, table_list, true);
   else
     lock_type= table_list->lock_type;
 

=== modified file 'sql/sql_base.h'
--- a/sql/sql_base.h	2012-05-29 19:18:45 +0000
+++ b/sql/sql_base.h	2012-07-20 06:55:34 +0000
@@ -154,7 +154,8 @@ TABLE *open_table_uncached(THD *thd, con
 TABLE *find_locked_table(TABLE *list, const char *db, const char *table_name);
 thr_lock_type read_lock_type_for_table(THD *thd,
                                        Query_tables_list *prelocking_ctx,
-                                       TABLE_LIST *table_list);
+                                       TABLE_LIST *table_list,
+                                       bool routine_modifies_data);
 
 my_bool mysql_rm_tmp_tables(void);
 bool rm_temporary_table(handlerton *base, const char *path);

=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc	2012-07-10 10:58:12 +0000
+++ b/sql/sql_update.cc	2012-07-20 06:55:34 +0000
@@ -1464,8 +1464,11 @@ int mysql_multi_update_prepare(THD *thd)
         another table instance used by this statement which is going to
         be write-locked (for example, trigger to be invoked might try
         to update this table).
+        Last argument routine_modifies_data for read_lock_type_for_table()
+        is ignored, as prelocking placeholder will never be set here.
       */
-      tl->lock_type= read_lock_type_for_table(thd, lex, tl);
+      DBUG_ASSERT(tl->prelocking_placeholder == false);
+      tl->lock_type= read_lock_type_for_table(thd, lex, tl, true);
       tl->updating= 0;
       /* Update TABLE::lock_type accordingly. */
       if (!tl->placeholder() && !using_lock_tables)

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.6 branch (gopal.shankar:4052 to 4053) Bug#13036505Gopal Shankar20 Jul