#At file:///export/home/z/mysql-6.0-codebase-bugfixing-bug30977/ based on revid:guilhem@strippedzs7mouolfiy49fz
3655 Jon Olav Hauglid 2009-10-19
Bug #30977 Concurrent statement using stored function and DROP FUNCTION
breaks SBR
The problem was that concurrent execution of DML statements that use stored
functions and DDL statements that drop/modify the same function might result
in incorrect binary log in statement (and mixed) mode and therefore break
replication.
This patch fixes the problem by introducing metadata locking for stored
procedures and functions. This is similar to what is done in Bug#989 for
tables and views. Procedures and functions now are locked using metadata
locks until the transaction is either committed or rolled back. This
prevents other statements from modifying the procedure/function while it
is being executed. This provides commit ordering - guaranteeing
serializability across multiple transactions and thus fixes the reported
binlog problem.
Note that we do not take locks for top-level CALLs. This means that
procedures called directly are not protected from changes by simultaneous
DDL operations so the are executed at the state they had at the time of
the CALL. By not taking locks for top-level CALLs, we still allow
transactions to be started inside procedures.
This patches also changes the logic of prepared statement validation.
A stored procedure used by a prepared statement is now validated
against the stored procedure cache once a metadata lock has been
acquired. A version mismatch causes flushing of the sp cache and
statement reprepare.
Incompatible changes:
1) ER_LOCK_DEADLOCK is reported for a transaction trying to access a
procedure/function that is locked by a DDL operation in another
connection.
2) Procedure/function DDL operations are now prohibited in LOCK TABLES mode
as exclusive locks must be taken all at once and LOCK TABLES provides no
way to specifiy procedures/functions to be locked.
Test cases have been added to sp_lock.test.
Work on this bug has very much been a team effort and this patch includes and
is based on contributions from Davi Arnout, Dmitry Lenev, Magne Mæhre
and Konstantin Osipov.
@ mysql-test/t/sp-error.test
Procedure DDL operations are now disallowed in LOCK TABLES mode.
added:
mysql-test/r/sp_lock.result
mysql-test/t/sp_lock.test
modified:
mysql-test/r/ps_ddl.result
mysql-test/r/sp-error.result
mysql-test/suite/backup/r/backup_bml_not_falcon.result
mysql-test/t/ps_ddl.test
mysql-test/t/sp-error.test
sql/lock.cc
sql/mysql_priv.h
sql/sp.cc
sql/sp.h
sql/sp_cache.cc
sql/sp_cache.h
sql/sql_base.cc
sql/sql_class.h
sql/sql_parse.cc
sql/sql_prepare.cc
sql/sql_show.cc
sql/sql_table.cc
=== modified file 'mysql-test/r/ps_ddl.result'
--- a/mysql-test/r/ps_ddl.result 2009-08-24 09:56:29 +0000
+++ b/mysql-test/r/ps_ddl.result 2009-10-19 14:04:56 +0000
@@ -269,8 +269,6 @@ Part 7: TABLE -> TABLE (TRIGGER dependen
=====================================================================
# Test 7-a: dependent PROCEDURE has changed
#
-# Note, this scenario is not supported, subject of Bug#12093
-#
create table t1 (a int);
create trigger t1_ai after insert on t1 for each row
call p1(new.a);
@@ -282,10 +280,9 @@ drop procedure p1;
create procedure p1 (a int) begin end;
set @var= 2;
execute stmt using @var;
-ERROR 42000: PROCEDURE test.p1 does not exist
# Cleanup
drop procedure p1;
-call p_verify_reprepare_count(0);
+call p_verify_reprepare_count(1);
SUCCESS
# Test 7-b: dependent FUNCTION has changed
@@ -361,11 +358,13 @@ set @var=8;
# XXX: bug, the SQL statement in the trigger is still
# pointing at table 't3', since the view was expanded
# at first statement execution.
+# Since the view definition is inlined in the statement
+# at prepare, changing the view definition does not cause
+# repreparation.
# Repreparation of the main statement doesn't cause repreparation
# of trigger statements.
execute stmt using @var;
-ERROR 42S02: Table 'test.t3' doesn't exist
-call p_verify_reprepare_count(1);
+call p_verify_reprepare_count(0);
SUCCESS
#
@@ -382,6 +381,7 @@ select * from t3;
a
6
7
+8
flush table t1;
set @var=9;
execute stmt using @var;
@@ -396,6 +396,7 @@ select * from t3;
a
6
7
+8
drop view v1;
drop table t1,t2,t3;
# Test 7-d: dependent TABLE has changed
=== modified file 'mysql-test/r/sp-error.result'
--- a/mysql-test/r/sp-error.result 2009-09-20 14:42:51 +0000
+++ b/mysql-test/r/sp-error.result 2009-10-19 14:04:56 +0000
@@ -512,7 +512,7 @@ select * from t1;
end|
lock table t1 read|
alter procedure bug9566 comment 'Some comment'|
-ERROR HY000: Table 'proc' was not locked with LOCK TABLES
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
unlock tables|
drop procedure bug9566|
drop procedure if exists bug7299|
=== added file 'mysql-test/r/sp_lock.result'
--- a/mysql-test/r/sp_lock.result 1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/sp_lock.result 2009-10-19 14:04:56 +0000
@@ -0,0 +1,236 @@
+#
+# Test 1: Verify that the preceding transaction is
+# (implicitly) committed before CREATE/ALTER/DROP
+# PROCEDURE
+#
+CREATE TABLE t1 (id int primary key, val varchar(10) NOT NULL)
+ENGINE= InnoDB;
+BEGIN;
+INSERT INTO t1 VALUES(1, 'xxx');
+CREATE PROCEDURE p1() SELECT SLEEP(5);
+ROLLBACK;
+SELECT COUNT(*) FROM t1;
+COUNT(*)
+1
+BEGIN;
+INSERT INTO t1 VALUES (2, 'xxx');
+ALTER PROCEDURE p1 COMMENT 'Changed comment';
+ROLLBACK;
+SELECT COUNT(*) FROM t1;
+COUNT(*)
+2
+BEGIN;
+INSERT INTO t1 VALUES (3, 'xxx');
+DROP PROCEDURE p1;
+ROLLBACK;
+SELECT COUNT(*) FROM t1;
+COUNT(*)
+3
+DROP TABLE t1;
+#
+# Test 2: Verify that procedure DDL operations fail
+# under LOCK TABLES
+#
+CREATE TABLE t1 (id int primary key, val varchar(10) NOT NULL);
+CREATE PROCEDURE p1() SELECT SLEEP(1);
+LOCK TABLE t1 READ;
+CREATE PROCEDURE p2() SELECT SLEEP(1);
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+ALTER PROCEDURE p1 COMMENT 'Changed comment';
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+DROP PROCEDURE p1;
+ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
+UNLOCK TABLES;
+DROP TABLE t1;
+DROP PROCEDURE p1;
+#
+# Test 3: Verify that ALTER/DROP PROCEDURE grabs an
+# excl.lock
+#
+# con1
+CREATE PROCEDURE p1() SELECT 'Executing p1';
+SET DEBUG_SYNC= 'before_wait_locked_pname SIGNAL sp_exlock WAIT_FOR sp_unlock';
+DROP PROCEDURE p1;
+# con2
+SET DEBUG_SYNC= 'now WAIT_FOR sp_exlock';
+ALTER PROCEDURE p1 COMMENT 'new comment';
+SET DEBUG_SYNC= 'now SIGNAL sp_unlock';
+# con1
+SET DEBUG_SYNC= 'RESET';
+# con1
+CREATE PROCEDURE p1() SELECT 'Executing p1';
+SET DEBUG_SYNC='after_wait_locked_pname SIGNAL sp_exlock WAIT_FOR sp_unlock';
+DROP PROCEDURE p1;
+# con2
+SET DEBUG_SYNC='now WAIT_FOR sp_exlock';
+SET DEBUG_SYNC='mdl_acquire_exclusive_locks_wait SIGNAL sp_unlock';
+ALTER PROCEDURE p1 COMMENT 'new comment';;
+# con1
+# con2
+ERROR 42000: PROCEDURE test.p1 does not exist
+SET DEBUG_SYNC= 'RESET';
+#
+# Test 4: MDL lock should not be taken for
+# toplevel CALL statement
+#
+# con1
+CREATE PROCEDURE p4()
+BEGIN
+SET DEBUG_SYNC= 'now SIGNAL sp_shlock WAIT_FOR sp_unlock';
+SELECT 'Procedure executed';
+END|
+CALL p4();
+# con2
+SET DEBUG_SYNC= 'before_wait_locked_pname WAIT_FOR sp_shlock';
+Got signal from CALL statement
+ALTER PROCEDURE p4 COMMENT 'Changed comment';
+SET DEBUG_SYNC= 'now SIGNAL sp_unlock';
+# con1
+Procedure executed
+Procedure executed
+DROP PROCEDURE p4;
+SET DEBUG_SYNC= 'RESET';
+#
+# Test 5: Locks should be taken on routines
+# used indirectly by views or triggers
+#
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+CREATE TABLE t1(id INT PRIMARY KEY);
+CREATE TABLE t2(id INT PRIMARY KEY, val INT);
+CREATE TRIGGER tr1 AFTER INSERT ON t1
+FOR EACH ROW BEGIN
+INSERT INTO t2 VALUES (NEW.id, f1());
+END;
+|
+# con1
+SET DEBUG_SYNC= 'after_shared_lock_pname SIGNAL sp_shlock WAIT_FOR sp_unlock';
+INSERT INTO t1(id) VALUES (1), (2);
+# con2
+SET DEBUG_SYNC= 'now WAIT_FOR sp_shlock';
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL sp_unlock';
+DROP FUNCTION f1;
+SELECT * FROM t2;
+id val
+1 1
+2 1
+DROP TABLE t2;
+DROP TABLE t1;
+SET DEBUG_SYNC= 'RESET';
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+CREATE VIEW v1 AS SELECT f1();
+# con1
+SET DEBUG_SYNC= 'after_shared_lock_pname SIGNAL sp_shlock WAIT_FOR sp_unlock';
+SELECT * FROM v1;;
+# con2
+SET DEBUG_SYNC= 'now WAIT_FOR sp_shlock';
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL sp_unlock';
+DROP FUNCTION f1;
+f1()
+1
+DROP VIEW v1;
+SET DEBUG_SYNC= 'RESET';
+#
+# Test 6: Check that ER_LOCK_DEADLOCK is reported if
+# acquiring a shared lock fails during an transaction.
+#
+CREATE TABLE t1(id INT PRIMARY KEY);
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+CREATE VIEW v1 AS SELECT f1();
+# con1
+SET DEBUG_SYNC='after_wait_locked_pname SIGNAL sp_exlock WAIT_FOR sp_unlock';
+DROP FUNCTION f1;
+# con2
+SET DEBUG_SYNC='now WAIT_FOR sp_exlock';
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+SELECT * FROM v1;
+ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
+ROLLBACK;
+SET DEBUG_SYNC='now SIGNAL sp_unlock';
+# con1
+DROP TABLE t1;
+DROP VIEW v1;
+SET DEBUG_SYNC= 'RESET';
+#
+# Test 7: A stored routine could change after dispatch_command()
+# but before a MDL lock is taken. This must be noticed and the
+# sp cache flushed so the correct version can be loaded.
+#
+# con1
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+# Get f1 cached
+SELECT f1();
+f1()
+1
+# Then start executing it again...
+SET DEBUG_SYNC= 'before_execute_sql_command SIGNAL before WAIT_FOR changed';
+SELECT f1();
+# con2
+SET DEBUG_SYNC= 'now WAIT_FOR before';
+# ... but before f1 is locked, change it.
+DROP FUNCTION f1;
+CREATE FUNCTION f1() RETURNS INT RETURN 2;
+SET DEBUG_SYNC= 'now SIGNAL changed';
+# con1
+# We should now get '2' and not '1'.
+f1()
+2
+DROP FUNCTION f1;
+SET DEBUG_SYNC= 'RESET';
+#
+# Test 8: Binary log with concurrent DDL and DML.
+# This is based on the test case supplied in the bug report
+# for Bug#30977
+#
+# con1
+CREATE TABLE t1 (i int);
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+RESET MASTER;
+SET DEBUG_SYNC= 'after_shared_lock_pname SIGNAL s_locked WAIT_FOR x_wait';
+INSERT INTO t1 VALUES (f1());
+# con2
+SET DEBUG_SYNC= 'now WAIT_FOR s_locked';
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL x_wait';
+DROP FUNCTION f1;
+# con1
+# con2
+SELECT * FROM t1;
+i
+1
+show binlog events from <binlog_start>;
+Log_name Pos Event_type Server_id End_log_pos Info
+master-bin.000001 # Query # # use `test`; INSERT INTO t1 VALUES (f1())
+master-bin.000001 # Query # # use `test`; DROP FUNCTION f1
+DROP TABLE t1;
+SET DEBUG_SYNC= 'RESET';
+#
+# Test 9: Binary log with prepared statement.
+# Alternative of test 8, this time with the DDL in a
+# prepared statement.
+#
+# con1
+CREATE TABLE t1 (i INT);
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+PREPARE stmt1 FROM 'INSERT INTO t1 VALUES (? + f1())';
+SET @a = 1;
+EXECUTE stmt1 USING @a;
+RESET MASTER;
+SET @a = 2;
+SET DEBUG_SYNC='after_shared_lock_pname SIGNAL s_locked WAIT_FOR x_wait';
+EXECUTE stmt1 USING @a;
+# con2
+SET DEBUG_SYNC='now WAIT_FOR s_locked';
+SET DEBUG_SYNC='mdl_acquire_exclusive_locks_wait SIGNAL x_wait';
+DROP FUNCTION f1;
+# con1
+# con2
+SELECT * FROM t1;
+i
+2
+3
+show binlog events from <binlog_start>;
+Log_name Pos Event_type Server_id End_log_pos Info
+master-bin.000001 # Query # # use `test`; INSERT INTO t1 VALUES (2 + f1())
+master-bin.000001 # Query # # use `test`; DROP FUNCTION f1
+DROP TABLE t1;
+SET DEBUG_SYNC= 'RESET';
=== modified file 'mysql-test/suite/backup/r/backup_bml_not_falcon.result'
--- a/mysql-test/suite/backup/r/backup_bml_not_falcon.result 2009-06-30 07:51:04 +0000
+++ b/mysql-test/suite/backup/r/backup_bml_not_falcon.result 2009-10-19 14:04:56 +0000
@@ -1897,6 +1897,7 @@ f1 CREATE DEFINER=`root`@`localhost` FU
RETURN 1 latin1 latin1_swedish_ci latin1_swedish_ci
Procedure sql_mode Create Procedure character_set_client collation_connection Database Collation
p1 CREATE DEFINER=`root`@`localhost` PROCEDURE `p1`()
+ COMMENT 'testing alter'
SET @foo=1 latin1 latin1_swedish_ci latin1_swedish_ci
event_name
e2
=== modified file 'mysql-test/t/ps_ddl.test'
--- a/mysql-test/t/ps_ddl.test 2009-08-24 09:56:29 +0000
+++ b/mysql-test/t/ps_ddl.test 2009-10-19 14:04:56 +0000
@@ -278,8 +278,6 @@ deallocate prepare stmt;
--echo # Test 7-a: dependent PROCEDURE has changed
--echo #
---echo # Note, this scenario is not supported, subject of Bug#12093
---echo #
create table t1 (a int);
create trigger t1_ai after insert on t1 for each row
@@ -291,11 +289,10 @@ execute stmt using @var;
drop procedure p1;
create procedure p1 (a int) begin end;
set @var= 2;
---error ER_SP_DOES_NOT_EXIST
execute stmt using @var;
--echo # Cleanup
drop procedure p1;
-call p_verify_reprepare_count(0);
+call p_verify_reprepare_count(1);
--echo # Test 7-b: dependent FUNCTION has changed
--echo #
@@ -355,11 +352,13 @@ set @var=8;
--echo # XXX: bug, the SQL statement in the trigger is still
--echo # pointing at table 't3', since the view was expanded
--echo # at first statement execution.
+--echo # Since the view definition is inlined in the statement
+--echo # at prepare, changing the view definition does not cause
+--echo # repreparation.
--echo # Repreparation of the main statement doesn't cause repreparation
--echo # of trigger statements.
---error ER_NO_SUCH_TABLE
execute stmt using @var;
-call p_verify_reprepare_count(1);
+call p_verify_reprepare_count(0);
--echo #
--echo # Sic: the insert went into t3, even though the view now
--echo # points at t2. This is because neither the merged view
=== modified file 'mysql-test/t/sp-error.test'
--- a/mysql-test/t/sp-error.test 2009-09-20 14:42:51 +0000
+++ b/mysql-test/t/sp-error.test 2009-10-19 14:04:56 +0000
@@ -723,7 +723,7 @@ lock table t1 read|
# This should fail since we forgot to lock mysql.proc for writing
# explicitly, and we can't open mysql.proc for _writing_ if there
# are locked tables.
---error 1100
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
alter procedure bug9566 comment 'Some comment'|
unlock tables|
# This should succeed
=== added file 'mysql-test/t/sp_lock.test'
--- a/mysql-test/t/sp_lock.test 1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/sp_lock.test 2009-10-19 14:04:56 +0000
@@ -0,0 +1,424 @@
+# Copyright (C) 2009 Sun Microsystems, Inc
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; version 2 of the License.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+
+#
+# Metadata lock handling for stored procedures and
+# functions
+#
+# See: Bug #30977
+#
+
+--source include/have_binlog_format_mixed.inc
+--source include/have_debug_sync.inc
+--source include/have_innodb.inc
+# Save the initial number of concurrent sessions.
+--source include/count_sessions.inc
+
+connect(con1, localhost, root,,);
+connect(con2, localhost, root,,);
+connect(con3, localhost, root,,);
+
+
+--echo #
+--echo # Test 1: Verify that the preceding transaction is
+--echo # (implicitly) committed before CREATE/ALTER/DROP
+--echo # PROCEDURE
+--echo #
+
+# Start a transaction, insert a row into a table,
+# then call a DDL operation on a procedure, and then rollback.
+# The inserts should have survived, since the DDL operation
+# should have caused an implicit commit.
+
+connection con1;
+CREATE TABLE t1 (id int primary key, val varchar(10) NOT NULL)
+ ENGINE= InnoDB;
+
+# Test CREATE
+BEGIN;
+INSERT INTO t1 VALUES(1, 'xxx');
+CREATE PROCEDURE p1() SELECT SLEEP(5);
+ROLLBACK;
+
+# COUNT(*) should return 1, since the inserts were implicitly committed
+SELECT COUNT(*) FROM t1;
+
+# Test ALTER
+BEGIN;
+INSERT INTO t1 VALUES (2, 'xxx');
+ALTER PROCEDURE p1 COMMENT 'Changed comment';
+ROLLBACK;
+
+# COUNT should now be 2
+SELECT COUNT(*) FROM t1;
+
+# Test DROP
+BEGIN;
+INSERT INTO t1 VALUES (3, 'xxx');
+DROP PROCEDURE p1;
+ROLLBACK;
+
+# now 3
+SELECT COUNT(*) FROM t1;
+
+# cleanup
+DROP TABLE t1;
+
+
+--echo #
+--echo # Test 2: Verify that procedure DDL operations fail
+--echo # under LOCK TABLES
+--echo #
+
+# Due to potential deadlock issues, procedure DDL operations
+# should fail under LOCK TABLES
+
+connection con1;
+CREATE TABLE t1 (id int primary key, val varchar(10) NOT NULL);
+CREATE PROCEDURE p1() SELECT SLEEP(1);
+
+LOCK TABLE t1 READ;
+# CREATE PROCEDURE should now fail..
+# catch error here
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+CREATE PROCEDURE p2() SELECT SLEEP(1);
+# same with ALTER and DROP
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+ALTER PROCEDURE p1 COMMENT 'Changed comment';
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+DROP PROCEDURE p1;
+UNLOCK TABLES;
+# cleanup
+DROP TABLE t1;
+DROP PROCEDURE p1;
+
+
+--echo #
+--echo # Test 3: Verify that ALTER/DROP PROCEDURE grabs an
+--echo # excl.lock
+--echo #
+
+# Create a function f1 and a view v1 that uses f1.
+# Add a sync point before ex.lock is grabbed, and
+# send a DROP on f1. Then access v1 from con2
+# It should succeed.
+
+--echo # con1
+connection con1;
+CREATE PROCEDURE p1() SELECT 'Executing p1';
+
+SET DEBUG_SYNC= 'before_wait_locked_pname SIGNAL sp_exlock WAIT_FOR sp_unlock';
+--send DROP PROCEDURE p1
+
+--echo # con2
+connection con2;
+SET DEBUG_SYNC= 'now WAIT_FOR sp_exlock';
+ALTER PROCEDURE p1 COMMENT 'new comment';
+SET DEBUG_SYNC= 'now SIGNAL sp_unlock';
+
+--echo # con1
+connection con1;
+--reap
+
+SET DEBUG_SYNC= 'RESET';
+
+# Now set a new sync point in con1 _after_ the ex.lock is grabbed.
+# Let con2 try to access v1 again. This should
+# cause backoff-retry when trying to lock f1. Signal con1 so
+# it will resume. The CALL should then fail since f1
+# has now been dropped.
+
+--echo # con1
+connection con1;
+CREATE PROCEDURE p1() SELECT 'Executing p1';
+
+SET DEBUG_SYNC='after_wait_locked_pname SIGNAL sp_exlock WAIT_FOR sp_unlock';
+--send DROP PROCEDURE p1
+
+--echo # con2
+connection con2;
+SET DEBUG_SYNC='now WAIT_FOR sp_exlock';
+SET DEBUG_SYNC='mdl_acquire_exclusive_locks_wait SIGNAL sp_unlock';
+--send ALTER PROCEDURE p1 COMMENT 'new comment';
+
+--echo # con1
+--connection con1
+--reap
+
+--echo # con2
+--connection con2
+--error ER_SP_DOES_NOT_EXIST
+--reap
+
+SET DEBUG_SYNC= 'RESET';
+
+
+--echo #
+--echo # Test 4: MDL lock should not be taken for
+--echo # toplevel CALL statement
+--echo #
+
+# Note: This is a requirement due to a potential
+# deadlock issue. In theory, top-level CALL
+# statements should take a shared MDL lock
+
+# Set a sync point after shared locks on procedures
+# have been taken, and wait. Then try to grab an
+# exclusive lock. If the ex-lock is granted, the
+# shared lock was never taken
+
+--connection con1
+--echo # con1
+delimiter |;
+CREATE PROCEDURE p4()
+BEGIN
+ SET DEBUG_SYNC= 'now SIGNAL sp_shlock WAIT_FOR sp_unlock';
+ SELECT 'Procedure executed';
+END|
+delimiter ;|
+# will sleep
+--send CALL p4()
+
+--connection con2
+--echo # con2
+SET DEBUG_SYNC= 'before_wait_locked_pname WAIT_FOR sp_shlock';
+--echo Got signal from CALL statement
+ALTER PROCEDURE p4 COMMENT 'Changed comment';
+SET DEBUG_SYNC= 'now SIGNAL sp_unlock';
+--connection con1
+--echo # con1
+--reap
+
+DROP PROCEDURE p4;
+SET DEBUG_SYNC= 'RESET';
+
+
+--echo #
+--echo # Test 5: Locks should be taken on routines
+--echo # used indirectly by views or triggers
+--echo #
+
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+
+CREATE TABLE t1(id INT PRIMARY KEY);
+CREATE TABLE t2(id INT PRIMARY KEY, val INT);
+
+delimiter |;
+CREATE TRIGGER tr1 AFTER INSERT ON t1
+ FOR EACH ROW BEGIN
+ INSERT INTO t2 VALUES (NEW.id, f1());
+ END;
+|
+delimiter ;|
+
+--connection con1
+--echo # con1
+SET DEBUG_SYNC= 'after_shared_lock_pname SIGNAL sp_shlock WAIT_FOR sp_unlock';
+--send INSERT INTO t1(id) VALUES (1), (2)
+
+--connection con2
+--echo # con2
+SET DEBUG_SYNC= 'now WAIT_FOR sp_shlock';
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL sp_unlock';
+--send DROP FUNCTION f1
+
+--connection con1
+--reap
+--connection con2
+--reap
+--connection con1
+
+SELECT * FROM t2;
+
+DROP TABLE t2;
+DROP TABLE t1;
+SET DEBUG_SYNC= 'RESET';
+
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+CREATE VIEW v1 AS SELECT f1();
+
+--connection con1
+--echo # con1
+SET DEBUG_SYNC= 'after_shared_lock_pname SIGNAL sp_shlock WAIT_FOR sp_unlock';
+--send SELECT * FROM v1;
+
+--connection con2
+--echo # con2
+SET DEBUG_SYNC= 'now WAIT_FOR sp_shlock';
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL sp_unlock';
+--send DROP FUNCTION f1
+
+--connection con1
+--reap
+--connection con2
+--reap
+--connection con1
+
+DROP VIEW v1;
+SET DEBUG_SYNC= 'RESET';
+
+
+--echo #
+--echo # Test 6: Check that ER_LOCK_DEADLOCK is reported if
+--echo # acquiring a shared lock fails during an transaction.
+--echo #
+
+CREATE TABLE t1(id INT PRIMARY KEY);
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+CREATE VIEW v1 AS SELECT f1();
+
+--connection con1
+--echo # con1
+SET DEBUG_SYNC='after_wait_locked_pname SIGNAL sp_exlock WAIT_FOR sp_unlock';
+--send DROP FUNCTION f1
+
+--connection con2
+--echo # con2
+SET DEBUG_SYNC='now WAIT_FOR sp_exlock';
+START TRANSACTION;
+INSERT INTO t1 VALUES (1);
+--error ER_LOCK_DEADLOCK
+SELECT * FROM v1;
+--disable_warnings
+ROLLBACK;
+--enable_warnings
+SET DEBUG_SYNC='now SIGNAL sp_unlock';
+
+--connection con1
+--echo # con1
+--reap
+
+DROP TABLE t1;
+DROP VIEW v1;
+SET DEBUG_SYNC= 'RESET';
+
+
+--echo #
+--echo # Test 7: A stored routine could change after dispatch_command()
+--echo # but before a MDL lock is taken. This must be noticed and the
+--echo # sp cache flushed so the correct version can be loaded.
+--echo #
+
+--echo # con1
+connection con1;
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+--echo # Get f1 cached
+SELECT f1();
+--echo # Then start executing it again...
+SET DEBUG_SYNC= 'before_execute_sql_command SIGNAL before WAIT_FOR changed';
+--send SELECT f1()
+
+--echo # con2
+connection con2;
+SET DEBUG_SYNC= 'now WAIT_FOR before';
+--echo # ... but before f1 is locked, change it.
+DROP FUNCTION f1;
+CREATE FUNCTION f1() RETURNS INT RETURN 2;
+SET DEBUG_SYNC= 'now SIGNAL changed';
+
+--echo # con1
+--echo # We should now get '2' and not '1'.
+connection con1;
+--reap
+
+connection default;
+DROP FUNCTION f1;
+SET DEBUG_SYNC= 'RESET';
+
+
+--echo #
+--echo # Test 8: Binary log with concurrent DDL and DML.
+--echo # This is based on the test case supplied in the bug report
+--echo # for Bug#30977
+--echo #
+
+--echo # con1
+connection con1;
+CREATE TABLE t1 (i int);
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+RESET MASTER;
+
+SET DEBUG_SYNC= 'after_shared_lock_pname SIGNAL s_locked WAIT_FOR x_wait';
+--send INSERT INTO t1 VALUES (f1())
+
+--echo # con2
+connection con2;
+SET DEBUG_SYNC= 'now WAIT_FOR s_locked';
+SET DEBUG_SYNC= 'mdl_acquire_exclusive_locks_wait SIGNAL x_wait';
+--send DROP FUNCTION f1
+
+--echo # con1
+connection con1;
+--reap
+
+--echo # con2
+connection con2;
+--reap
+SELECT * FROM t1;
+let $VERSION=`select version()`;
+source include/show_binlog_events.inc;
+
+connection default;
+DROP TABLE t1;
+SET DEBUG_SYNC= 'RESET';
+
+--echo #
+--echo # Test 9: Binary log with prepared statement.
+--echo # Alternative of test 8, this time with the DDL in a
+--echo # prepared statement.
+--echo #
+
+--echo # con1
+connection con1;
+CREATE TABLE t1 (i INT);
+CREATE FUNCTION f1() RETURNS INT RETURN 1;
+PREPARE stmt1 FROM 'INSERT INTO t1 VALUES (? + f1())';
+SET @a = 1;
+EXECUTE stmt1 USING @a;
+RESET MASTER;
+
+SET @a = 2;
+SET DEBUG_SYNC='after_shared_lock_pname SIGNAL s_locked WAIT_FOR x_wait';
+--send EXECUTE stmt1 USING @a
+
+--echo # con2
+connection con2;
+SET DEBUG_SYNC='now WAIT_FOR s_locked';
+SET DEBUG_SYNC='mdl_acquire_exclusive_locks_wait SIGNAL x_wait';
+--send DROP FUNCTION f1
+
+--echo # con1
+connection con1;
+--reap
+
+--echo # con2
+connection con2;
+--reap
+SELECT * FROM t1;
+let $VERSION=`select version()`;
+source include/show_binlog_events.inc;
+
+connection default;
+DROP TABLE t1;
+SET DEBUG_SYNC= 'RESET';
+
+connection default;
+disconnect con3;
+disconnect con2;
+disconnect con1;
+
+# Check that all connections opened by test cases in this file are really
+# gone so execution of other tests won't be affected by their presence.
+--source include/wait_until_count_sessions.inc
=== modified file 'sql/lock.cc'
--- a/sql/lock.cc 2009-09-25 10:48:44 +0000
+++ b/sql/lock.cc 2009-10-19 14:04:56 +0000
@@ -994,6 +994,54 @@ void unlock_table_names(THD *thd)
DBUG_VOID_RETURN;
}
+/**
+ Obtain an exclusive metadata lock on the stored routine name.
+
+ @param thd Thread handle
+ @param type Stored routine type
+ (TYPE_ENUM_PROCEDURE or TYPE_ENUM_FUNCTION).
+ @param sp Stored routine object to lock the name of.
+
+ This function assumes that no metadata locks were acquired
+ before calling it. Also it cannot be called while holding
+ LOCK_open mutex. Both these invariants are enforced by asserts
+ in MDL_context::acquire_exclusive_locks().
+ To avoid deadlocks, we do not try to obtain exclusive metadata
+ locks in LOCK TABLES mode, since in this mode there may be
+ other metadata locks already taken by the current connection,
+ and we must not wait for MDL locks while holding locks.
+
+ @retval FALSE Success.
+ @retval TRUE Failure: we're in LOCK TABLES mode, or out of memory,
+ or this connection was killed.
+*/
+
+bool lock_routine_name(THD *thd, bool is_function,
+ const char *db, const char *name)
+{
+ MDL_key::enum_mdl_namespace mdl_type= (is_function ?
+ MDL_key::FUNCTION :
+ MDL_key::PROCEDURE);
+ MDL_request mdl_request;
+
+ if (thd->locked_tables_mode)
+ {
+ my_message(ER_LOCK_OR_ACTIVE_TRANSACTION,
+ ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
+ return TRUE;
+ }
+
+ DBUG_ASSERT(name);
+ DEBUG_SYNC(thd, "before_wait_locked_pname");
+
+ mdl_request.init(mdl_type, db, name, MDL_EXCLUSIVE);
+
+ if (thd->mdl_context.acquire_exclusive_lock(&mdl_request))
+ return TRUE;
+
+ DEBUG_SYNC(thd, "after_wait_locked_pname");
+ return FALSE;
+}
static void print_lock_error(int error, const char *table)
{
=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h 2009-09-25 14:15:30 +0000
+++ b/sql/mysql_priv.h 2009-10-19 14:04:56 +0000
@@ -2154,6 +2154,9 @@ int set_handler_table_locks(THD *thd, TA
bool lock_table_names(THD *thd, TABLE_LIST *table_list);
void unlock_table_names(THD *thd);
+/* Lock based on stored routine name */
+bool lock_routine_name(THD *thd, bool is_function, const char *db,
+ const char *name);
/* old unireg functions */
=== modified file 'sql/sp.cc'
--- a/sql/sp.cc 2009-09-25 14:15:30 +0000
+++ b/sql/sp.cc 2009-10-19 14:04:56 +0000
@@ -752,6 +752,18 @@ sp_create_routine(THD *thd, int type, sp
*/
thd->clear_current_stmt_binlog_row_based();
+ /* Grab an exclusive MDL lock. */
+ if (lock_routine_name(thd, type == TYPE_ENUM_FUNCTION,
+ sp->m_db.str, sp->m_name.str))
+ DBUG_RETURN(SP_OPEN_TABLE_FAILED);
+
+ /*
+ Creating a stored routine x can make it necessary
+ to reparse another routine y if y uses x.
+ */
+ sp_cache_flush_obsolete(&thd->sp_func_cache);
+ sp_cache_flush_obsolete(&thd->sp_proc_cache);
+
saved_count_cuted_fields= thd->count_cuted_fields;
thd->count_cuted_fields= CHECK_FIELD_WARN;
@@ -994,6 +1006,11 @@ sp_drop_routine(THD *thd, int type, sp_n
*/
thd->clear_current_stmt_binlog_row_based();
+ /* Grab an exclusive MDL lock. */
+ if (lock_routine_name(thd, type == TYPE_ENUM_FUNCTION,
+ name->m_db.str, name->m_name.str))
+ DBUG_RETURN(SP_DELETE_ROW_FAILED);
+
if (!(table= open_proc_table_for_update(thd)))
DBUG_RETURN(SP_OPEN_TABLE_FAILED);
if ((ret= db_find_routine_aux(thd, type, name, table)) == SP_OK)
@@ -1048,6 +1065,11 @@ sp_update_routine(THD *thd, int type, sp
*/
thd->clear_current_stmt_binlog_row_based();
+ /* Grab an exclusive MDL lock. */
+ if (lock_routine_name(thd, type == TYPE_ENUM_FUNCTION,
+ name->m_db.str, name->m_name.str))
+ DBUG_RETURN(SP_OPEN_TABLE_FAILED);
+
if (!(table= open_proc_table_for_update(thd)))
DBUG_RETURN(SP_OPEN_TABLE_FAILED);
if ((ret= db_find_routine_aux(thd, type, name, table)) == SP_OK)
@@ -1451,6 +1473,7 @@ bool sp_add_used_routine(Query_tables_li
my_hash_insert(&prelocking_ctx->sroutines, (uchar *)rn);
prelocking_ctx->sroutines_list.link_in_list((uchar *)rn, (uchar **)&rn->next);
rn->belong_to_view= belong_to_view;
+ rn->m_sp_cache_version= 0;
return TRUE;
}
return FALSE;
@@ -1602,8 +1625,7 @@ void sp_update_stmt_used_routines(THD *t
loading.
@param[in] thd Thread context.
- @param[in] type Type of object (TYPE_ENUM_FUNCTION or TYPE_ENUM_PROCEDURE).
- @param[in] name Name of routine.
+ @param[in] rt Pointer to the routine
@param[out] sp Pointer to sp_head object for routine, NULL if routine was
not found,
@@ -1612,19 +1634,33 @@ void sp_update_stmt_used_routines(THD *t
@retval non-0 Error while loading routine from mysql,proc table.
*/
-int sp_cache_routine(THD *thd, int type, sp_name *name, sp_head **sp)
+int sp_cache_routine(THD *thd, Sroutine_hash_entry *rt, sp_head **sp)
{
int ret= 0;
+ char qname_buff[NAME_LEN*2+1+1];
+ sp_name name(&rt->mdl_request.key, qname_buff);
+ MDL_key::enum_mdl_namespace mdl_type= rt->mdl_request.key.mdl_namespace();
+ int type= ((mdl_type == MDL_key::FUNCTION) ?
+ TYPE_ENUM_FUNCTION : TYPE_ENUM_PROCEDURE);
DBUG_ENTER("sp_cache_routine");
- DBUG_ASSERT(type == TYPE_ENUM_FUNCTION || type == TYPE_ENUM_PROCEDURE);
+ DBUG_ASSERT(mdl_type == MDL_key::FUNCTION || mdl_type == MDL_key::PROCEDURE);
+
+ /*
+ Check that we have an MDL lock on this routine, unless it's a top-level
+ CALL. The assert below should be unambiguous: the first element
+ in sroutines_list has an MDL lock unless it's a top-level call, or a
+ trigger, but triggers can't occur here (see the preceding assert).
+ */
+ DBUG_ASSERT(rt->mdl_request.ticket ||
+ rt == (Sroutine_hash_entry*) thd->lex->sroutines_list.first);
if (!(*sp= sp_cache_lookup((type == TYPE_ENUM_FUNCTION ?
&thd->sp_func_cache : &thd->sp_proc_cache),
- name)))
+ &name)))
{
- switch ((ret= db_find_routine(thd, type, name, sp)))
+ switch ((ret= db_find_routine(thd, type, &name, sp)))
{
case SP_OK:
if (type == TYPE_ENUM_FUNCTION)
@@ -1664,8 +1700,8 @@ int sp_cache_routine(THD *thd, int type,
char n[NAME_LEN*2+2];
/* m_qname.str is not always \0 terminated */
- memcpy(n, name->m_qname.str, name->m_qname.length);
- n[name->m_qname.length]= '\0';
+ memcpy(n, name.m_qname.str, name.m_qname.length);
+ n[name.m_qname.length]= '\0';
my_error(ER_SP_PROC_TABLE_CORRUPT, MYF(0), n, ret);
}
break;
=== modified file 'sql/sp.h'
--- a/sql/sp.h 2009-09-16 13:25:29 +0000
+++ b/sql/sp.h 2009-10-19 14:04:56 +0000
@@ -71,7 +71,7 @@ sp_find_routine(THD *thd, int type, sp_n
sp_cache **cp, bool cache_only);
int
-sp_cache_routine(THD *thd, int type, sp_name *name, sp_head **sp);
+sp_cache_routine(THD *thd, Sroutine_hash_entry *rt, sp_head **sp);
bool
sp_exist_routines(THD *thd, TABLE_LIST *procs, bool any);
@@ -116,6 +116,20 @@ public:
statement uses routine both via view and directly.
*/
TABLE_LIST *belong_to_view;
+ /**
+ This is for prepared statement validation purposes.
+ A statement looks up and pre-loads all its stored functions
+ at prepare. Later on, if a function is gone from the cache,
+ execute may fail.
+ Remember the cache version to be able to invalidate the prepared
+ statement at execute if it changes.
+ We only need to care about version of the stored functions cache:
+ if a prepared statement uses a stored procedure, it's indirect,
+ via a stored function. The only exception is SQLCOM_CALL,
+ but the latter one looks up the stored procedure each time
+ it's invoked, rather than once at prepare.
+ */
+ ulong m_sp_cache_version;
};
=== modified file 'sql/sp_cache.cc'
--- a/sql/sp_cache.cc 2009-01-27 02:08:48 +0000
+++ b/sql/sp_cache.cc 2009-10-19 14:04:56 +0000
@@ -230,6 +230,19 @@ ulong sp_cache_version(sp_cache **cp)
}
+/**
+ Check if the cache needs to be invalidated
+*/
+
+bool sp_cache_is_invalid(sp_cache **cp)
+{
+ sp_cache *c= *cp;
+ if (c)
+ return c->version < Cversion;
+ return FALSE;
+}
+
+
/*************************************************************************
Internal functions
*************************************************************************/
=== modified file 'sql/sp_cache.h'
--- a/sql/sp_cache.h 2008-12-02 22:02:52 +0000
+++ b/sql/sp_cache.h 2009-10-19 14:04:56 +0000
@@ -60,5 +60,6 @@ sp_head *sp_cache_lookup(sp_cache **cp,
void sp_cache_invalidate();
void sp_cache_flush_obsolete(sp_cache **cp);
ulong sp_cache_version(sp_cache **cp);
+bool sp_cache_is_invalid(sp_cache **cp);
#endif /* _SP_CACHE_H_ */
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2009-09-25 14:15:30 +0000
+++ b/sql/sql_base.cc 2009-10-19 14:04:56 +0000
@@ -21,6 +21,7 @@
#include "sql_select.h"
#include "sp_head.h"
#include "sp.h"
+#include "sp_cache.h"
#include "sql_trigger.h"
#include "transaction.h"
#include "sql_prepare.h"
@@ -3456,6 +3457,56 @@ check_and_update_table_version(THD *thd,
/**
+ Compares versions of an stored routine obtained from the sp cache
+ and the version used at prepare.
+
+ @details If the new and the old values mismatch, invoke
+ Metadata_version_observer.
+ At prepared statement prepare, all Sroutine_hash_entry version values
+ are NULL and we always have a mismatch. But there is no observer set
+ in THD, and therefore no error is reported. Instead, we update
+ the value in Sroutine_hash_entry, effectively recording the original
+ version.
+ At prepared statement execute, an observer may be installed. If
+ there is a version mismatch, we push an error and return TRUE.
+
+ For conventional execution (no prepared statements), the
+ observer is never installed.
+
+ @param[in] thd used to report errors
+ @param[in] rt pointer to stored routine entry
+
+ @retval TRUE an error, which has been reported
+ @retval FALSE success, version in Sroutine_hash_entry has been updated
+*/
+
+static bool
+check_and_update_routine_version(THD *thd, Sroutine_hash_entry *rt)
+{
+ sp_cache **cp = rt->mdl_request.key.mdl_namespace() == MDL_key::FUNCTION ?
+ &thd->sp_func_cache : & thd->sp_proc_cache;
+
+ if (rt->m_sp_cache_version != sp_cache_version(cp))
+ {
+ if (thd->m_reprepare_observer &&
+ thd->m_reprepare_observer->report_error(thd))
+ {
+ /*
+ Version of the sp cache is different from the
+ previous execution of the prepared statement, and it is
+ unacceptable for this SQLCOM. Error has been reported.
+ */
+ DBUG_ASSERT(thd->is_error());
+ return TRUE;
+ }
+ /* Always maintain the latest cache version. */
+ rt->m_sp_cache_version= sp_cache_version(cp);
+ }
+ return FALSE;
+}
+
+
+/**
Open view by getting its definition from disk (and table cache in future).
@param thd Thread handle
@@ -3671,13 +3722,16 @@ request_backoff_action(enum_open_table_a
/**
- Recover from failed attempt ot open table by performing requested action.
+ Recover from failed attempt of open table by performing requested action.
@param thd Thread context
- @param table Table list element for table that caused problem
- @param action Type of action requested by failed open_table() call
+ @param mdl_request MDL_request of the object that caused the problem.
+ @param table Optional (can be NULL). Used only if action is OT_REPAIR.
+ In that case a TABLE_LIST for the table to be repaired.
+ @todo: It's unnecessary and should be removed.
- @pre This function should be called only with "action" != OT_NO_ACTION.
+ @pre This function should be called only with "action" != OT_NO_ACTION
+ and after having called @sa close_tables_for_reopen().
@retval FALSE - Success. One should try to open tables once again.
@retval TRUE - Error
@@ -3685,7 +3739,8 @@ request_backoff_action(enum_open_table_a
bool
Open_table_context::
-recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *table)
+recover_from_failed_open(THD *thd, MDL_request *mdl_request,
+ TABLE_LIST *table)
{
bool result= FALSE;
/* Execute the action. */
@@ -3697,14 +3752,20 @@ recover_from_failed_open_table_attempt(T
break;
case OT_DISCOVER:
{
- MDL_request mdl_xlock_request(&table->mdl_request);
+ MDL_request mdl_xlock_request(mdl_request);
mdl_xlock_request.set_type(MDL_EXCLUSIVE);
if ((result=
thd->mdl_context.acquire_exclusive_lock(&mdl_xlock_request)))
break;
+
+ DBUG_ASSERT(mdl_request->key.mdl_namespace() == MDL_key::TABLE);
pthread_mutex_lock(&LOCK_open);
- tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name);
- ha_create_table_from_engine(thd, table->db, table->table_name);
+ tdc_remove_table(thd, TDC_RT_REMOVE_ALL,
+ mdl_request->key.db_name(),
+ mdl_request->key.name());
+ ha_create_table_from_engine(thd,
+ mdl_request->key.db_name(),
+ mdl_request->key.name());
pthread_mutex_unlock(&LOCK_open);
thd->warning_info->clear_warning_info(thd->query_id);
@@ -3714,20 +3775,30 @@ recover_from_failed_open_table_attempt(T
}
case OT_REPAIR:
{
- MDL_request mdl_xlock_request(&table->mdl_request);
+ MDL_request mdl_xlock_request(mdl_request);
mdl_xlock_request.set_type(MDL_EXCLUSIVE);
if ((result=
thd->mdl_context.acquire_exclusive_lock(&mdl_xlock_request)))
break;
+ DBUG_ASSERT(mdl_request->key.mdl_namespace() == MDL_key::TABLE);
pthread_mutex_lock(&LOCK_open);
- tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name);
+ tdc_remove_table(thd, TDC_RT_REMOVE_ALL,
+ mdl_request->key.db_name(),
+ mdl_request->key.name());
pthread_mutex_unlock(&LOCK_open);
result= auto_repair_table(thd, table);
thd->mdl_context.release_lock(mdl_xlock_request.ticket);
break;
}
+ case OT_FLUSH_SP_CACHE:
+ {
+ /* Back off because a routine has changed, so flush the sp cache. */
+ sp_cache_flush_obsolete(&thd->sp_func_cache);
+ sp_cache_flush_obsolete(&thd->sp_proc_cache);
+ break;
+ }
default:
DBUG_ASSERT(0);
}
@@ -3782,7 +3853,11 @@ thr_lock_type read_lock_type_for_table(T
@param[in] prelocking_strategy Strategy which specifies how the
prelocking set should be extended when
one of its elements is processed.
- @param[out] need_prelocking Set to TRUE if it was detected that this
+ @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.
@@ -3794,25 +3869,58 @@ static bool
open_and_process_routine(THD *thd, Query_tables_list *prelocking_ctx,
Sroutine_hash_entry *rt,
Prelocking_strategy *prelocking_strategy,
+ bool has_prelocking_list,
+ Open_table_context *ot_ctx,
bool *need_prelocking)
{
+ MDL_key::enum_mdl_namespace mdl_type= rt->mdl_request.key.mdl_namespace();
DBUG_ENTER("open_and_process_routine");
- switch (rt->mdl_request.key.mdl_namespace())
+ switch (mdl_type)
{
case MDL_key::FUNCTION:
case MDL_key::PROCEDURE:
{
- char qname_buff[NAME_LEN*2+1+1];
- sp_name name(&rt->mdl_request.key, qname_buff);
sp_head *sp;
- int type= (rt->mdl_request.key.mdl_namespace() == MDL_key::FUNCTION) ?
- TYPE_ENUM_FUNCTION : TYPE_ENUM_PROCEDURE;
- if (sp_cache_routine(thd, type, &name, &sp))
+ /*
+ Try to get MDL lock on the routine.
+ Note that we do not take locks on top-level CALLs as this can
+ lead to a deadlock. Not locking top-level CALLs does not break
+ the binlog as only the statements in the called procedure show
+ up there, not the CALL itself.
+ */
+ if (rt != (Sroutine_hash_entry*)prelocking_ctx->sroutines_list.first ||
+ mdl_type != MDL_key::PROCEDURE)
+ {
+ ot_ctx->add_request(&rt->mdl_request);
+ if (thd->mdl_context.try_acquire_shared_lock(&rt->mdl_request))
+ DBUG_RETURN(TRUE);
+
+ if (rt->mdl_request.ticket == NULL)
+ {
+ ot_ctx->request_backoff_action(Open_table_context::OT_WAIT);
+ DBUG_RETURN(TRUE);
+ }
+
+ if (sp_cache_is_invalid(mdl_type == MDL_key::FUNCTION ?
+ &thd->sp_func_cache :
+ &thd->sp_proc_cache))
+ {
+ ot_ctx->request_backoff_action(Open_table_context::OT_FLUSH_SP_CACHE);
+ DBUG_RETURN(TRUE);
+ }
+ DEBUG_SYNC(thd, "after_shared_lock_pname");
+ }
+
+ if (sp_cache_routine(thd, rt, &sp))
+ DBUG_RETURN(TRUE);
+
+ /* Check and update the metadata version of the routine. */
+ if (check_and_update_routine_version(thd, rt))
DBUG_RETURN(TRUE);
- if (sp)
+ if (sp && !has_prelocking_list)
{
prelocking_strategy->handle_routine(thd, prelocking_ctx, rt, sp,
need_prelocking);
@@ -3820,6 +3928,35 @@ open_and_process_routine(THD *thd, Query
}
break;
case MDL_key::TRIGGER:
+ /**
+ We add trigger entries to lex->sroutines_list, but we don't
+ load them here. The trigger entry is only used when building
+ a transitive closure of objects used in a statement, to avoid
+ adding to this closure object that are used in the trigger more
+ than once.
+ E.g. if a trigger trg refers to table t2, and the trigger table t1
+ is used multiple times in the statement (say, because it's used in
+ function f1() twice), we will only add t2 once to the list of
+ tables to prelock.
+
+ We don't take metadata locks on triggers either: they are protected
+ by a respective lock on the table, on which the trigger is defined.
+
+ The only case when this leads to "trouble" is the SHOW CREATE TRIGGER
+ statement. Statement syntax doesn't specify the table on which this
+ trigger is defined, so we have to make a "dirty" read in the data
+ dictionary to find out the table name. Once we discover the table name,
+ we take a metadata lock on it, and this protects all trigger operations.
+ Of course the table, in theory, may disappear between the dirty read
+ and metadata lock acquisition, but in that case we just return a run-time
+ error.
+
+ Grammar of trigger DDL statements (CREATE, DROP) requires
+ the table to be specified explicitly, so we use the table metadata
+ lock to protect trigger metadata in these statements. Similarly, in
+ DML we always use triggers together with their tables, and thus don't
+ need to take separate metadata locks on them.
+ */
break;
default:
/* Impossible type value. */
@@ -3939,7 +4076,7 @@ open_and_process_table(THD *thd, LEX *le
if (error)
{
- if (! ot_ctx->can_recover_from_failed_open_table() && safe_to_ignore_table)
+ if (! ot_ctx->can_recover_from_failed_open() && safe_to_ignore_table)
{
DBUG_PRINT("info", ("open_table: ignoring table '%s'.'%s'",
tables->db, tables->alias));
@@ -4158,7 +4295,6 @@ bool open_tables(THD *thd, TABLE_LIST **
*/
while (*table_to_open ||
(thd->locked_tables_mode <= LTM_LOCK_TABLES &&
- ! has_prelocking_list &&
*sroutine_to_open))
{
/*
@@ -4175,7 +4311,7 @@ bool open_tables(THD *thd, TABLE_LIST **
if (error)
{
- if (ot_ctx.can_recover_from_failed_open_table())
+ if (ot_ctx.can_recover_from_failed_open())
{
/*
We have met exclusive metadata lock or old version of table.
@@ -4199,7 +4335,8 @@ bool open_tables(THD *thd, TABLE_LIST **
TABLE_LIST element. Altough currently this assumption is valid
it may change in future.
*/
- if (ot_ctx.recover_from_failed_open_table_attempt(thd, failed_table))
+ if (ot_ctx.recover_from_failed_open(thd, &failed_table->mdl_request,
+ failed_table))
goto err;
error= FALSE;
@@ -4213,8 +4350,11 @@ bool open_tables(THD *thd, TABLE_LIST **
If we are not already in prelocked mode and extended table list is
not yet built for our statement we need to cache routines it uses
and build the prelocking list for it.
+ If we are not in prelocked mode but have built the extended table
+ list, we still need to call open_and_process_routine() to take
+ MDL locks on the routines.
*/
- if (thd->locked_tables_mode <= LTM_LOCK_TABLES && ! has_prelocking_list)
+ if (thd->locked_tables_mode <= LTM_LOCK_TABLES)
{
bool need_prelocking= FALSE;
TABLE_LIST **save_query_tables_last= thd->lex->query_tables_last;
@@ -4230,12 +4370,21 @@ bool open_tables(THD *thd, TABLE_LIST **
for (Sroutine_hash_entry *rt= *sroutine_to_open; rt;
sroutine_to_open= &rt->next, rt= rt->next)
{
- error= open_and_process_routine(thd, thd->lex, rt,
- prelocking_strategy,
+ error= open_and_process_routine(thd, thd->lex, rt, prelocking_strategy,
+ has_prelocking_list, &ot_ctx,
&need_prelocking);
if (error)
{
+ if (ot_ctx.can_recover_from_failed_open())
+ {
+ close_tables_for_reopen(thd, start);
+ if (ot_ctx.recover_from_failed_open(thd, &rt->mdl_request, NULL))
+ goto err;
+
+ error= FALSE;
+ goto restart;
+ }
/*
Serious error during reading stored routines from mysql.proc table.
Something is wrong with the table or its contents, and an error has
@@ -4640,7 +4789,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST
retry:
while ((error= open_table(thd, table_list, thd->mem_root, &ot_ctx, 0)) &&
- ot_ctx.can_recover_from_failed_open_table())
+ ot_ctx.can_recover_from_failed_open())
{
/*
Even though we have failed to open table we still need to
@@ -4650,7 +4799,8 @@ retry:
if (! thd->locked_tables_mode)
thd->mdl_context.release_all_locks();
table_list->mdl_request.ticket= 0;
- if (ot_ctx.recover_from_failed_open_table_attempt(thd, table_list))
+ if (ot_ctx.recover_from_failed_open(thd, &table_list->mdl_request,
+ table_list))
break;
}
@@ -5203,6 +5353,11 @@ void close_tables_for_reopen(THD *thd, T
if (first_not_own_table == *tables)
*tables= 0;
thd->lex->chop_off_not_own_tables();
+ /* Reset MDL tickets for procedures/functions */
+ for (Sroutine_hash_entry *rt=
+ (Sroutine_hash_entry*)thd->lex->sroutines_list.first;
+ rt; rt= rt->next)
+ rt->mdl_request.ticket= NULL;
sp_remove_not_own_routines(thd->lex);
for (tmp= *tables; tmp; tmp= tmp->next_global)
{
@@ -8335,6 +8490,10 @@ tdc_wait_for_old_versions(THD *thd, MDL_
MDL_request_list::Iterator it(*mdl_requests);
while ((mdl_request= it++))
{
+ /* Skip requests on non-TDC objects. */
+ if (mdl_request->key.mdl_namespace() != MDL_key::TABLE)
+ continue;
+
if ((share= get_cached_table_share(mdl_request->key.db_name(),
mdl_request->key.name())) &&
share->version != refresh_version)
=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h 2009-09-30 15:24:05 +0000
+++ b/sql/sql_class.h 2009-10-19 14:04:56 +0000
@@ -1287,7 +1287,7 @@ private:
/**
A context of open_tables() function, used to recover
- from a failed open_table() attempt.
+ from a failed open_table() or open_routine() attempt.
Implemented in sql_base.cc.
*/
@@ -1300,17 +1300,19 @@ public:
OT_NO_ACTION= 0,
OT_WAIT,
OT_DISCOVER,
- OT_REPAIR
+ OT_REPAIR,
+ OT_FLUSH_SP_CACHE
};
Open_table_context(THD *thd);
- bool recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *tables);
+ bool recover_from_failed_open(THD *thd, MDL_request *mdl_request,
+ TABLE_LIST *table);
bool request_backoff_action(enum_open_table_action action_arg);
void add_request(MDL_request *request)
{ m_mdl_requests.push_front(request); }
- bool can_recover_from_failed_open_table() const
+ bool can_recover_from_failed_open() const
{ return m_action != OT_NO_ACTION; }
bool can_deadlock() const { return m_can_deadlock; }
private:
=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc 2009-09-25 14:15:30 +0000
+++ b/sql/sql_parse.cc 2009-10-19 14:04:56 +0000
@@ -290,14 +290,20 @@ void init_update_queries(void)
sql_command_flags[SQLCOM_GRANT]= CF_CHANGES_DATA;
sql_command_flags[SQLCOM_REVOKE]= CF_CHANGES_DATA;
sql_command_flags[SQLCOM_ALTER_DB]= CF_CHANGES_DATA;
- sql_command_flags[SQLCOM_CREATE_FUNCTION]= CF_CHANGES_DATA;
- sql_command_flags[SQLCOM_DROP_FUNCTION]= CF_CHANGES_DATA;
sql_command_flags[SQLCOM_OPTIMIZE]= CF_CHANGES_DATA;
- sql_command_flags[SQLCOM_CREATE_PROCEDURE]= CF_CHANGES_DATA;
- sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA;
- sql_command_flags[SQLCOM_DROP_PROCEDURE]= CF_CHANGES_DATA;
- sql_command_flags[SQLCOM_ALTER_PROCEDURE]= CF_CHANGES_DATA;
- sql_command_flags[SQLCOM_ALTER_FUNCTION]= CF_CHANGES_DATA;
+ sql_command_flags[SQLCOM_CREATE_FUNCTION]= CF_CHANGES_DATA;
+ sql_command_flags[SQLCOM_CREATE_PROCEDURE]= CF_CHANGES_DATA |
+ CF_AUTO_COMMIT_TRANS;
+ sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA |
+ CF_AUTO_COMMIT_TRANS;
+ sql_command_flags[SQLCOM_DROP_PROCEDURE]= CF_CHANGES_DATA |
+ CF_AUTO_COMMIT_TRANS;
+ sql_command_flags[SQLCOM_DROP_FUNCTION]= CF_CHANGES_DATA |
+ CF_AUTO_COMMIT_TRANS;
+ sql_command_flags[SQLCOM_ALTER_PROCEDURE]= CF_CHANGES_DATA |
+ CF_AUTO_COMMIT_TRANS;
+ sql_command_flags[SQLCOM_ALTER_FUNCTION]= CF_CHANGES_DATA |
+ CF_AUTO_COMMIT_TRANS;
sql_command_flags[SQLCOM_INSTALL_PLUGIN]= CF_CHANGES_DATA;
sql_command_flags[SQLCOM_UNINSTALL_PLUGIN]= CF_CHANGES_DATA;
@@ -325,10 +331,6 @@ void init_update_queries(void)
sql_command_flags[SQLCOM_REVOKE]|= CF_AUTO_COMMIT_TRANS;
sql_command_flags[SQLCOM_GRANT]|= CF_AUTO_COMMIT_TRANS;
- sql_command_flags[SQLCOM_CREATE_PROCEDURE]|= CF_AUTO_COMMIT_TRANS;
- sql_command_flags[SQLCOM_CREATE_SPFUNCTION]|= CF_AUTO_COMMIT_TRANS;
- sql_command_flags[SQLCOM_ALTER_PROCEDURE]|= CF_AUTO_COMMIT_TRANS;
- sql_command_flags[SQLCOM_ALTER_FUNCTION]|= CF_AUTO_COMMIT_TRANS;
sql_command_flags[SQLCOM_ASSIGN_TO_KEYCACHE]= CF_AUTO_COMMIT_TRANS;
sql_command_flags[SQLCOM_PRELOAD_KEYS]= CF_AUTO_COMMIT_TRANS;
@@ -2054,6 +2056,12 @@ mysql_execute_command(THD *thd)
#endif
case SQLCOM_SHOW_STATUS_PROC:
case SQLCOM_SHOW_STATUS_FUNC:
+ /* Until we use MDL for these commands, we have to flush manually. */
+ if (lex->sql_command == SQLCOM_SHOW_STATUS_PROC)
+ sp_cache_flush_obsolete(&thd->sp_proc_cache);
+ else
+ sp_cache_flush_obsolete(&thd->sp_func_cache);
+
if (!(res= check_table_access(thd, SELECT_ACL, all_tables, FALSE, FALSE,
UINT_MAX)))
res= execute_sqlcom_select(thd, all_tables);
@@ -4355,6 +4363,15 @@ create_sp_error:
sp_head *sp;
/*
+ sp_cache_flush_obsolete() is usually called after acquiring
+ a MDL lock and discovering that the version has changed.
+ But since MDL locks are not taken for top-level CALLs, flush
+ must be called here for those cases.
+ */
+ if (!lex->sp_lex_in_use)
+ sp_cache_flush_obsolete(&thd->sp_proc_cache);
+
+ /*
This will cache all SP and SF and open and lock all tables
required for execution.
*/
@@ -4596,6 +4613,14 @@ create_sp_error:
if (!thd->locked_tables_mode)
thd->mdl_context.release_all_locks();
+ /* Conditionally writes to binlog */
+ sp_result= sp_drop_routine(thd, type, lex->spname);
+
+ /* Commit or rollback the statement transaction. */
+ thd->is_error() ? trans_rollback_stmt(thd) : trans_commit_stmt(thd);
+ /* Commit the normal transaction if one is active. */
+ trans_commit_implicit(thd);
+
#ifndef NO_EMBEDDED_ACCESS_CHECKS
if (sp_automatic_privileges && !opt_noacl &&
sp_revoke_privileges(thd, db, name,
@@ -4606,8 +4631,6 @@ create_sp_error:
ER(ER_PROC_AUTO_REVOKE_FAIL));
}
#endif
- /* Conditionally writes to binlog */
- sp_result= sp_drop_routine(thd, type, lex->spname);
}
res= sp_result;
switch (sp_result) {
@@ -4637,6 +4660,16 @@ create_sp_error:
}
case SQLCOM_SHOW_CREATE_PROC:
{
+ /* Until we use MDL for SHOW CREATE, we have to flush manually. */
+ sp_cache_flush_obsolete(&thd->sp_proc_cache);
+
+ /**
+ @todo: Consider using prelocking for this code as well. Currently
+ SHOW CREATE PROCEDURE/FUNCTION is a dirty read of the data
+ dictionary, i.e. takes no metadata locks.
+ It is "safe" to do as long as it doesn't affect the results
+ of the binary log or the query cache, which currently it does not.
+ */
if (sp_show_create_routine(thd, TYPE_ENUM_PROCEDURE, lex->spname))
{
my_error(ER_SP_DOES_NOT_EXIST, MYF(0),
@@ -4647,6 +4680,16 @@ create_sp_error:
}
case SQLCOM_SHOW_CREATE_FUNC:
{
+ /* Until we use MDL for SHOW CREATE, we have to flush manually. */
+ sp_cache_flush_obsolete(&thd->sp_func_cache);
+
+ /**
+ @todo: Consider using prelocking for this code as well. Currently
+ SHOW CREATE PROCEDURE/FUNCTION is a dirty read of the data
+ dictionary, i.e. takes no metadata locks.
+ It is "safe" to do as long as it doesn't affect the results
+ of the binary log or the query cache, which currently it does not.
+ */
if (sp_show_create_routine(thd, TYPE_ENUM_FUNCTION, lex->spname))
{
my_error(ER_SP_DOES_NOT_EXIST, MYF(0),
@@ -5940,9 +5983,6 @@ void mysql_parse(THD *thd, const char *i
{
LEX *lex= thd->lex;
- sp_cache_flush_obsolete(&thd->sp_proc_cache);
- sp_cache_flush_obsolete(&thd->sp_func_cache);
-
Parser_state parser_state(thd, inBuf, length);
bool err= parse_sql(thd, & parser_state, NULL);
=== modified file 'sql/sql_prepare.cc'
--- a/sql/sql_prepare.cc 2009-09-25 14:15:30 +0000
+++ b/sql/sql_prepare.cc 2009-10-19 14:04:56 +0000
@@ -173,8 +173,6 @@ private:
SELECT_LEX and other classes).
*/
MEM_ROOT main_mem_root;
- /* Version of the stored functions cache at the time of prepare. */
- ulong m_sp_cache_version;
private:
bool set_db(const char *db, uint db_length);
bool set_parameters(String *expanded_query,
@@ -2139,9 +2137,6 @@ void mysqld_stmt_prepare(THD *thd, const
DBUG_VOID_RETURN;
}
- sp_cache_flush_obsolete(&thd->sp_proc_cache);
- sp_cache_flush_obsolete(&thd->sp_func_cache);
-
thd->protocol= &thd->protocol_binary;
if (stmt->prepare(packet, packet_length))
@@ -2417,6 +2412,13 @@ void reinit_stmt_before_use(THD *thd, LE
{
tables->reinit_before_use(thd);
}
+
+ /* Reset MDL tickets for procedures/functions */
+ for (Sroutine_hash_entry *rt=
+ (Sroutine_hash_entry*)thd->lex->sroutines_list.first;
+ rt; rt= rt->next)
+ rt->mdl_request.ticket= NULL;
+
/*
Cleanup of the special case of DELETE t1, t2 FROM t1, t2, t3 ...
(multi-delete). We do a full clean up, although at the moment all we
@@ -2510,9 +2512,6 @@ void mysqld_stmt_execute(THD *thd, char
DBUG_PRINT("exec_query", ("%s", stmt->query));
DBUG_PRINT("info",("stmt: %p", stmt));
- sp_cache_flush_obsolete(&thd->sp_proc_cache);
- sp_cache_flush_obsolete(&thd->sp_func_cache);
-
open_cursor= test(flags & (ulong) CURSOR_TYPE_READ_ONLY);
thd->protocol= &thd->protocol_binary;
@@ -2962,8 +2961,7 @@ Prepared_statement::Prepared_statement(T
param_array(0),
param_count(0),
last_errno(0),
- flags((uint) IS_IN_USE),
- m_sp_cache_version(0)
+ flags((uint) IS_IN_USE)
{
init_sql_alloc(&main_mem_root, thd_arg->variables.query_alloc_block_size,
thd_arg->variables.query_prealloc_size);
@@ -3235,20 +3233,6 @@ bool Prepared_statement::prepare(const c
init_stmt_after_parse(lex);
state= Query_arena::PREPARED;
flags&= ~ (uint) IS_IN_USE;
- /*
- This is for prepared statement validation purposes.
- A statement looks up and pre-loads all its stored functions
- at prepare. Later on, if a function is gone from the cache,
- execute may fail.
- Remember the cache version to be able to invalidate the prepared
- statement at execute if it changes.
- We only need to care about version of the stored functions cache:
- if a prepared statement uses a stored procedure, it's indirect,
- via a stored function. The only exception is SQLCOM_CALL,
- but the latter one looks up the stored procedure each time
- it's invoked, rather than once at prepare.
- */
- m_sp_cache_version= sp_cache_version(&thd->sp_func_cache);
/*
Log COM_EXECUTE to the general log. Note, that in case of SQL
@@ -3595,7 +3579,6 @@ Prepared_statement::swap_prepared_statem
swap_variables(LEX_STRING, name, copy->name);
/* Ditto */
swap_variables(char *, db, copy->db);
- swap_variables(ulong, m_sp_cache_version, copy->m_sp_cache_version);
DBUG_ASSERT(db_length == copy->db_length);
DBUG_ASSERT(param_count == copy->param_count);
@@ -3655,19 +3638,6 @@ bool Prepared_statement::execute(String
}
/*
- Reprepare the statement if we're using stored functions
- and the version of the stored routines cache has changed.
- */
- if (lex->uses_stored_routines() &&
- m_sp_cache_version != sp_cache_version(&thd->sp_func_cache) &&
- thd->m_reprepare_observer &&
- thd->m_reprepare_observer->report_error(thd))
- {
- return TRUE;
- }
-
-
- /*
For SHOW VARIABLES lex->result is NULL, as it's a non-SELECT
command. For such queries we don't return an error and don't
open a cursor -- the client library will recognize this case and
=== modified file 'sql/sql_show.cc'
--- a/sql/sql_show.cc 2009-09-25 14:15:30 +0000
+++ b/sql/sql_show.cc 2009-10-19 14:04:56 +0000
@@ -22,6 +22,7 @@
#include "repl_failsafe.h"
#include "sp.h"
#include "sp_head.h"
+#include "sp_cache.h"
#include "sp_pcontext.h"
#include "sql_trigger.h"
#include "authors.h"
@@ -4661,6 +4662,13 @@ int fill_schema_proc(THD *thd, TABLE_LIS
get_schema_table_idx(tables->schema_table);
DBUG_ENTER("fill_schema_proc");
+ /*
+ Until we use MDL for I_S queries regarding stored routines,
+ we have to flush manually.
+ */
+ sp_cache_flush_obsolete(&thd->sp_proc_cache);
+ sp_cache_flush_obsolete(&thd->sp_func_cache);
+
strxmov(definer, thd->security_ctx->priv_user, "@",
thd->security_ctx->priv_host, NullS);
/* We use this TABLE_LIST instance only for checking of privileges. */
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2009-09-29 09:36:22 +0000
+++ b/sql/sql_table.cc 2009-10-19 14:04:56 +0000
@@ -21,6 +21,7 @@
#include <myisam.h>
#include <my_dir.h>
#include "sp_head.h"
+#include "sp.h"
#include "sql_trigger.h"
#include "sql_show.h"
#include "transaction.h"
@@ -4972,6 +4973,23 @@ send_result_message:
trans_commit_implicit(thd);
close_thread_tables(thd);
table->table=0; // For query cache
+
+ /*
+ If it is CHECK TABLE v1, v2, v3, and v1, v2, v3 are views, we will run
+ separate open_tables() for each CHECK TABLE argument.
+ Right now we do not have a separate method to reset the prelocking
+ state in the lex to the state after parsing, so each open will pollute
+ this state: add elements to lex->srotuines_list, TABLE_LISTs to
+ lex->query_tables. Below is a lame attempt to recover from this
+ pollution.
+ @todo: have a method to reset a prelocking context, or use separate
+ contexts for each open.
+ */
+ for (Sroutine_hash_entry *rt=
+ (Sroutine_hash_entry*)thd->lex->sroutines_list.first;
+ rt; rt= rt->next)
+ rt->mdl_request.ticket= NULL;
+
if (protocol->write())
goto err;
}
Attachment: [text/bzr-bundle] bzr/jon.hauglid@sun.com-20091019140456-9a9nsw319vel6zd9.bundle
| Thread |
|---|
| • bzr commit into mysql-6.0-codebase-bugfixing branch (jon.hauglid:3655)Bug#30977 | Jon Olav Hauglid | 19 Oct |
