Below is the list of changes that have just been committed into a local
5.0 repository of thek. When thek does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
ChangeSet@stripped, 2007-07-27 10:00:34+02:00, thek@adventure.(none) +5 -0
Bug #29929 LOCK TABLES does not pre-lock tables used in triggers of the locked tables
When a table was explicitly locked with LOCK TABLES no associated
tables from any related trigger on the subject table were locked.
As a result of this the user could experience unexpected locking
behavior and statement failures similar to "failed: 1100: Table'xx'
was not locked with LOCK TABLES".
This patch fixes this problem by making sure triggers are
pre-loaded on any statement if the subject table was explicitly
locked with LOCK TABLES.
mysql-test/r/trigger.result@stripped, 2007-07-27 10:00:31+02:00, thek@adventure.(none) +39 -0
Added test
mysql-test/t/trigger.test@stripped, 2007-07-27 10:00:31+02:00, thek@adventure.(none) +40 -0
Added test
sql/sql_lex.cc@stripped, 2007-07-27 10:00:31+02:00, thek@adventure.(none) +114 -1
- Moved some conditional logic out of the table iteration.
- Added event map values for LOCK TABLE command.
sql/table.cc@stripped, 2007-07-27 10:00:31+02:00, thek@adventure.(none) +7 -98
- Refactored set_trg_event_tpye into the two simpler functions set_trg_event_map
and set_trg_event_map as methods for manipulating the table event map.
The original function was only called from st_lex::set_trg_event_type_for_tables
so it was possible to move the event map creation logic to this function as
a loop optimization.
sql/table.h@stripped, 2007-07-27 10:00:31+02:00, thek@adventure.(none) +3 -1
- Refactored set_trg_event_tpye into the two simpler functions set_trg_event_map
and set_trg_event_map as methods for manipulating the table event map.
The original function was only called from st_lex::set_trg_event_type_for_tables
so it was possible to move the event map creation logic to this function as
a loop optimization.
diff -Nrup a/mysql-test/r/trigger.result b/mysql-test/r/trigger.result
--- a/mysql-test/r/trigger.result 2007-07-18 12:42:03 +02:00
+++ b/mysql-test/r/trigger.result 2007-07-27 10:00:31 +02:00
@@ -1961,4 +1961,43 @@ select * from t2;
s1
drop table t1;
drop temporary table t2;
+#
+# Bug #29929 LOCK TABLES does not pre-lock tables used in triggers of the locked tables
+#
+CREATE TABLE t1 (c1 INT);
+CREATE TABLE t2 (c2 INT);
+CREATE TABLE t3 (c3 INT);
+INSERT INTO t1 VALUES (1001);
+INSERT INTO t2 VALUES (2001);
+CREATE TRIGGER t1_ai AFTER INSERT ON t1 FOR EACH ROW
+BEGIN
+UPDATE t2 SET c2= c2 + 110;
+END//
+# Take a table lock on t1.
+# This should pre-lock t2 through the trigger.
+LOCK TABLE t1 WRITE;
+# connection conn1.
+# Try to use t2, which is pre-locked through the trigger on t1.
+# The statement should block until UNLOCK TABLES.
+# Contents of t3 shows when INSERT SELECT ran.
+INSERT INTO t3 SELECT c2 as c3 FROM t2;
+# connection default.
+# Wait for the helper thread to sit on its lock.
+INSERT INTO t1 VALUES (1002);
+UNLOCK TABLES;
+# connection conn1.
+UNLOCK TABLES;
+# connection default.
+SELECT * FROM t1;
+c1
+1001
+1002
+SELECT * FROM t2;
+c2
+2111
+SELECT * FROM t3;
+c3
+2111
+DROP TRIGGER t1_ai;
+DROP TABLE t1, t2, t3;
End of 5.0 tests
diff -Nrup a/mysql-test/t/trigger.test b/mysql-test/t/trigger.test
--- a/mysql-test/t/trigger.test 2007-07-18 12:42:03 +02:00
+++ b/mysql-test/t/trigger.test 2007-07-27 10:00:31 +02:00
@@ -2217,4 +2217,44 @@ select * from t1;
select * from t2;
drop table t1;
drop temporary table t2;
+--echo #
+--echo # Bug #29929 LOCK TABLES does not pre-lock tables used in triggers of the locked tables
+--echo #
+CREATE TABLE t1 (c1 INT);
+CREATE TABLE t2 (c2 INT);
+CREATE TABLE t3 (c3 INT);
+INSERT INTO t1 VALUES (1001);
+INSERT INTO t2 VALUES (2001);
+DELIMITER //;
+CREATE TRIGGER t1_ai AFTER INSERT ON t1 FOR EACH ROW
+BEGIN
+ UPDATE t2 SET c2= c2 + 110;
+END//
+DELIMITER ;//
+--echo # Take a table lock on t1.
+--echo # This should pre-lock t2 through the trigger.
+LOCK TABLE t1 WRITE;
+ --echo # connection conn1.
+ connect (conn1,localhost,root,,);
+ --echo # Try to use t2, which is pre-locked through the trigger on t1.
+ --echo # The statement should block until UNLOCK TABLES.
+ --echo # Contents of t3 shows when INSERT SELECT ran.
+ send INSERT INTO t3 SELECT c2 as c3 FROM t2;
+--echo # connection default.
+connection default;
+--echo # Wait for the helper thread to sit on its lock.
+ --sleep 2
+INSERT INTO t1 VALUES (1002);
+UNLOCK TABLES;
+ --echo # connection conn1.
+ connection conn1;
+ reap;
+ UNLOCK TABLES;
+--echo # connection default.
+connection default;
+SELECT * FROM t1;
+SELECT * FROM t2;
+SELECT * FROM t3;
+DROP TRIGGER t1_ai;
+DROP TABLE t1, t2, t3;
--echo End of 5.0 tests
diff -Nrup a/sql/sql_lex.cc b/sql/sql_lex.cc
--- a/sql/sql_lex.cc 2007-07-12 20:26:38 +02:00
+++ b/sql/sql_lex.cc 2007-07-27 10:00:31 +02:00
@@ -2041,6 +2041,106 @@ void st_select_lex_unit::set_limit(SELEC
void st_lex::set_trg_event_type_for_tables()
{
+ enum trg_event_type trg_event;
+
+ uint8 new_trg_event_map= 0;
+
+ /*
+ Some auxiliary operations
+ (e.g. GRANT processing) create TABLE_LIST instances outside
+ the parser. Additionally, some commands (e.g. OPTIMIZE) change
+ the lock type for a table only after parsing is done. Luckily,
+ these do not fire triggers and do not need to pre-load them.
+ For these TABLE_LISTs set_trg_event_type is never called, and
+ trg_event_map is always empty. That means that the pre-locking
+ algorithm will ignore triggers defined on these tables, if
+ any, and the execution will either fail with an assert in
+ sql_trigger.cc or with an error that a used table was not
+ pre-locked, in case of a production build.
+
+ TODO: this usage pattern creates unnecessary module dependencies
+ and should be rewritten to go through the parser.
+ Table list instances created outside the parser in most cases
+ refer to mysql.* system tables. It is not allowed to have
+ a trigger on a system table, but keeping track of
+ initialization provides extra safety in case this limitation
+ is circumvented.
+ */
+
+ /*
+ On a LOCK TABLE, all triggers must be pre-loaded for this TABLE_LIST
+ when opening an associated TABLE.
+ */
+ if (sql_command == SQLCOM_LOCK_TABLES)
+ new_trg_event_map= 0xff;
+ else
+ {
+ bool update_event_map= TRUE;
+ switch (sql_command) {
+ /*
+ Basic INSERT. If there is an additional ON DUPLIATE KEY UPDATE
+ clause, it will be handled later in this method.
+ */
+ case SQLCOM_INSERT: /* fall through */
+ case SQLCOM_INSERT_SELECT:
+ /*
+ LOAD DATA ... INFILE is expected to fire BEFORE/AFTER INSERT
+ triggers.
+ If the statement also has REPLACE clause, it will be
+ handled later in this method.
+ */
+ case SQLCOM_LOAD: /* fall through */
+ /*
+ REPLACE is semantically equivalent to INSERT. In case
+ of a primary or unique key conflict, it deletes the old
+ record and inserts a new one. So we also may need to
+ fire ON DELETE triggers. This functionality is handled
+ later in this method.
+ */
+ case SQLCOM_REPLACE: /* fall through */
+ case SQLCOM_REPLACE_SELECT:
+ /*
+ CREATE TABLE ... SELECT defaults to INSERT if the table or
+ view already exists. REPLACE option of CREATE TABLE ...
+ REPLACE SELECT is handled later in this method.
+ */
+ case SQLCOM_CREATE_TABLE:
+ trg_event= TRG_EVENT_INSERT;
+ break;
+ /* Basic update and multi-update */
+ case SQLCOM_UPDATE: /* fall through */
+ case SQLCOM_UPDATE_MULTI:
+ trg_event= TRG_EVENT_UPDATE;
+ break;
+ /* Basic delete and multi-delete */
+ case SQLCOM_DELETE: /* fall through */
+ case SQLCOM_DELETE_MULTI:
+ trg_event= TRG_EVENT_DELETE;
+ break;
+ default:
+ update_event_map= FALSE;
+ }
+
+ if (update_event_map)
+ new_trg_event_map|= static_cast<uint8>(1 << static_cast<int>(trg_event));
+
+ update_event_map= TRUE;
+ switch (duplicates) {
+ case DUP_UPDATE:
+ trg_event= TRG_EVENT_UPDATE;
+ break;
+ case DUP_REPLACE:
+ trg_event= TRG_EVENT_DELETE;
+ break;
+ case DUP_ERROR:
+ default:
+ update_event_map= FALSE;
+ }
+
+ if (update_event_map)
+ new_trg_event_map|= static_cast<uint8>(1 << static_cast<int>(trg_event));
+ }
+
/*
Do not iterate over sub-selects, only the tables in the outermost
SELECT_LEX can be modified, if any.
@@ -2049,7 +2149,20 @@ void st_lex::set_trg_event_type_for_tabl
while (tables)
{
- tables->set_trg_event_type(this);
+ /*
+ This is a fast check to filter out statements that do
+ not change data, or tables on the right side, in case of
+ INSERT .. SELECT, CREATE TABLE .. SELECT and so on.
+ Here we also filter out OPTIMIZE statement and non-updateable
+ views, for which lock_type is TL_UNLOCK or TL_READ after
+ parsing.
+ */
+ if (static_cast<int>(tables->lock_type) >=
+ static_cast<int>(TL_WRITE_ALLOW_WRITE))
+ {
+ new_trg_event_map |= tables->get_trg_event_map();
+ tables->set_trg_event_map(new_trg_event_map);
+ }
tables= tables->next_local;
}
}
diff -Nrup a/sql/table.cc b/sql/table.cc
--- a/sql/table.cc 2007-07-19 14:13:01 +02:00
+++ b/sql/table.cc 2007-07-27 10:00:31 +02:00
@@ -1803,107 +1803,16 @@ void st_table::reset_item_list(List<Item
*/
void
-TABLE_LIST::set_trg_event_type(const st_lex *lex)
+TABLE_LIST::set_trg_event_map(uint8 new_event_map)
{
- enum trg_event_type trg_event;
-
- /*
- Some auxiliary operations
- (e.g. GRANT processing) create TABLE_LIST instances outside
- the parser. Additionally, some commands (e.g. OPTIMIZE) change
- the lock type for a table only after parsing is done. Luckily,
- these do not fire triggers and do not need to pre-load them.
- For these TABLE_LISTs set_trg_event_type is never called, and
- trg_event_map is always empty. That means that the pre-locking
- algorithm will ignore triggers defined on these tables, if
- any, and the execution will either fail with an assert in
- sql_trigger.cc or with an error that a used table was not
- pre-locked, in case of a production build.
-
- TODO: this usage pattern creates unnecessary module dependencies
- and should be rewritten to go through the parser.
- Table list instances created outside the parser in most cases
- refer to mysql.* system tables. It is not allowed to have
- a trigger on a system table, but keeping track of
- initialization provides extra safety in case this limitation
- is circumvented.
- */
-
- /*
- This is a fast check to filter out statements that do
- not change data, or tables on the right side, in case of
- INSERT .. SELECT, CREATE TABLE .. SELECT and so on.
- Here we also filter out OPTIMIZE statement and non-updateable
- views, for which lock_type is TL_UNLOCK or TL_READ after
- parsing.
- */
- if (static_cast<int>(lock_type) < static_cast<int>(TL_WRITE_ALLOW_WRITE))
- return;
-
- switch (lex->sql_command) {
- /*
- Basic INSERT. If there is an additional ON DUPLIATE KEY UPDATE
- clause, it will be handled later in this method.
- */
- case SQLCOM_INSERT: /* fall through */
- case SQLCOM_INSERT_SELECT:
- /*
- LOAD DATA ... INFILE is expected to fire BEFORE/AFTER INSERT
- triggers.
- If the statement also has REPLACE clause, it will be
- handled later in this method.
- */
- case SQLCOM_LOAD: /* fall through */
- /*
- REPLACE is semantically equivalent to INSERT. In case
- of a primary or unique key conflict, it deletes the old
- record and inserts a new one. So we also may need to
- fire ON DELETE triggers. This functionality is handled
- later in this method.
- */
- case SQLCOM_REPLACE: /* fall through */
- case SQLCOM_REPLACE_SELECT:
- /*
- CREATE TABLE ... SELECT defaults to INSERT if the table or
- view already exists. REPLACE option of CREATE TABLE ...
- REPLACE SELECT is handled later in this method.
- */
- case SQLCOM_CREATE_TABLE:
- trg_event= TRG_EVENT_INSERT;
- break;
- /* Basic update and multi-update */
- case SQLCOM_UPDATE: /* fall through */
- case SQLCOM_UPDATE_MULTI:
- trg_event= TRG_EVENT_UPDATE;
- break;
- /* Basic delete and multi-delete */
- case SQLCOM_DELETE: /* fall through */
- case SQLCOM_DELETE_MULTI:
- trg_event= TRG_EVENT_DELETE;
- break;
- default:
- /*
- OK to return, since value of 'duplicates' is irrelevant
- for non-updating commands.
- */
- return;
- }
- trg_event_map|= static_cast<uint8>(1 << static_cast<int>(trg_event));
-
- switch (lex->duplicates) {
- case DUP_UPDATE:
- trg_event= TRG_EVENT_UPDATE;
- break;
- case DUP_REPLACE:
- trg_event= TRG_EVENT_DELETE;
- break;
- case DUP_ERROR:
- default:
- return;
- }
- trg_event_map|= static_cast<uint8>(1 << static_cast<int>(trg_event));
+ trg_event_map= new_event_map;
}
+uint8
+TABLE_LIST::get_trg_event_map(void)
+{
+ return trg_event_map;
+}
/*
calculate md5 of query
diff -Nrup a/sql/table.h b/sql/table.h
--- a/sql/table.h 2007-07-12 20:26:39 +02:00
+++ b/sql/table.h 2007-07-27 10:00:31 +02:00
@@ -770,7 +770,9 @@ struct TABLE_LIST
void reinit_before_use(THD *thd);
Item_subselect *containing_subselect();
- void set_trg_event_type(const st_lex *lex);
+ void set_trg_event_map(uint8 new_trg_event_map);
+ uint8 get_trg_event_map(void);
+
private:
bool prep_check_option(THD *thd, uint8 check_opt_type);
bool prep_where(THD *thd, Item **conds, bool no_where_clause);