#At file:///data0/martin/bzr/bug45235/5.1bt-gca/ based on revid:davi.arnaut@stripped
2936 Martin Hansson 2009-09-01
Bug#45235: 5.1 does not support 5.0-only syntax triggers in any way
A parse error when opening a trigger file would disallow the user to
drop the trigger, the most common scenario being upgrade from 5.0 to 5.1
where a lot of syntax is deprecated. The problem was that the trigger
name has to be parsed out in order to successfully drop the trigger.
Obviously this cannot be guaranteed if there was a parse error.
Fixed by analyzing LEX structure in the event of parse errors during
trigger load and recovering the trigger name if present by making a local
copy before the LEX structure is cleaned up. The error is then silenced.
We will end up with a partially initialized trigger structure, but it
serves to execute a DROP TRIGGER command.
@ mysql-test/r/trigger-compat.result
Bug#45235: Test result.
@ mysql-test/t/trigger-compat.test
Bug#45235: Test case.
@ sql/sql_parse.cc
Bug#45235: Removed assertion which can no longer be maintained.
@ sql/sql_trigger.cc
Bug#45235: Various code changes.
1) Error handler class that silences parse errors and picks up trigger name if possible.
2) Warnings elimination
3) Comment correction
4, 5) Fix.
6) Indentation correction
7) Split up giant assertion
modified:
mysql-test/r/trigger-compat.result
mysql-test/t/trigger-compat.test
sql/sql_parse.cc
sql/sql_trigger.cc
=== modified file 'mysql-test/r/trigger-compat.result'
--- a/mysql-test/r/trigger-compat.result 2009-02-19 23:24:25 +0000
+++ b/mysql-test/r/trigger-compat.result 2009-09-01 08:56:15 +0000
@@ -43,3 +43,42 @@ DROP TABLE t2;
DROP USER mysqltest_dfn@localhost;
DROP USER mysqltest_inv@localhost;
DROP DATABASE mysqltest_db1;
+USE test;
+#
+# Bug#45235: 5.1 does not support 5.0-only syntax triggers in any way
+#
+CREATE TABLE t1 ( a INT );
+CREATE TABLE t2 ( a INT );
+INSERT INTO t1 VALUES (1), (2), (3);
+INSERT INTO t2 VALUES (1), (2), (3);
+SHOW TRIGGERS;
+Trigger Event Table Statement Timing Created sql_mode Definer character_set_client collation_connection Database Collation
+CREATE TRIGGER tr2 BEFORE INSERT ON t1 FOR EACH ROW INSERT INTO t1 VALUES (1);
+Warnings:
+Warning 1603 Triggers for table `test`.`t1` have no creation context
+# Trigger tr2 should show up.
+SHOW TRIGGERS;
+Trigger Event Table Statement Timing Created sql_mode Definer character_set_client collation_connection Database Collation
+tr2 INSERT t1 INSERT INTO t1 VALUES (1) BEFORE NULL root@localhost latin1 latin1_swedish_ci latin1_swedish_ci
+DROP TRIGGER tr1;
+SHOW TRIGGERS;
+Trigger Event Table Statement Timing Created sql_mode Definer character_set_client collation_connection Database Collation
+tr2 INSERT t1 INSERT INTO t1 VALUES (1) BEFORE NULL root@localhost latin1 latin1_swedish_ci latin1_swedish_ci
+DROP TRIGGER tr2;
+SHOW TRIGGERS;
+Trigger Event Table Statement Timing Created sql_mode Definer character_set_client collation_connection Database Collation
+# Nothing to do in this case, but make sure we don't crash on parse error
+DROP TRIGGER tr0;
+ERROR HY000: Trigger does not exist
+# Make sure there is no trigger file left.
+t1.MYD
+t1.MYI
+t1.frm
+t2.MYD
+t2.MYI
+t2.TRG
+t2.frm
+tr0.TRN
+DROP TABLE t1, t2;
+Warnings:
+Warning 1603 Triggers for table `test`.`t2` have no creation context
=== modified file 'mysql-test/t/trigger-compat.test'
--- a/mysql-test/t/trigger-compat.test 2009-02-19 23:24:25 +0000
+++ b/mysql-test/t/trigger-compat.test 2009-09-01 08:56:15 +0000
@@ -106,4 +106,64 @@ DROP TABLE t2;
DROP USER mysqltest_dfn@localhost;
DROP USER mysqltest_inv@localhost;
DROP DATABASE mysqltest_db1;
+USE test;
+--echo #
+--echo # Bug#45235: 5.1 does not support 5.0-only syntax triggers in any way
+--echo #
+let $MYSQLD_DATADIR=`SELECT @@datadir`;
+
+CREATE TABLE t1 ( a INT );
+CREATE TABLE t2 ( a INT );
+INSERT INTO t1 VALUES (1), (2), (3);
+INSERT INTO t2 VALUES (1), (2), (3);
+
+# We simulate importing a trigger from 5.0 by writing a .TRN and .TRG file as
+# MySQL 5.0 would have done it, with syntax allowed in 5.0.
+#
+# Note that in 5.0 the following lines are missing from t1.TRG:
+#
+# client_cs_names='latin1'
+# connection_cl_names='latin1_swedish_ci'
+# db_cl_names='latin1_swedish_ci'
+--write_file $MYSQLD_DATADIR/test/tr1.TRN
+TYPE=TRIGGERNAME
+trigger_table=t1
+EOF
+
+--write_file $MYSQLD_DATADIR/test/tr0.TRN
+TYPE=TRIGGERNAME
+trigger_table=t2
+EOF
+
+--write_file $MYSQLD_DATADIR/test/t1.TRG
+TYPE=TRIGGERS
+triggers='CREATE DEFINER=`root`@`localhost` TRIGGER tr1 BEFORE INSERT ON t1 FOR EACH ROW DELETE FROM t1 a USING t1 a'
+sql_modes=0
+definers='root@localhost'
+EOF
+
+--write_file $MYSQLD_DATADIR/test/t2.TRG
+TYPE=TRIGGERS
+triggers='Not allowed syntax here, and trigger name cant be extracted either.'
+sql_modes=0
+definers='root@localhost'
+EOF
+
+SHOW TRIGGERS;
+CREATE TRIGGER tr2 BEFORE INSERT ON t1 FOR EACH ROW INSERT INTO t1 VALUES (1);
+--echo # Trigger tr2 should show up.
+SHOW TRIGGERS;
+DROP TRIGGER tr1;
+SHOW TRIGGERS;
+DROP TRIGGER tr2;
+SHOW TRIGGERS;
+
+--echo # Nothing to do in this case, but make sure we don't crash on parse error
+--error ER_TRG_DOES_NOT_EXIST
+DROP TRIGGER tr0;
+
+--echo # Make sure there is no trigger file left.
+--exec ls $MYSQLD_DATADIR/test/
+
+DROP TABLE t1, t2;
=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc 2009-06-05 11:23:58 +0000
+++ b/sql/sql_parse.cc 2009-09-01 08:56:15 +0000
@@ -7800,11 +7800,6 @@ bool parse_sql(THD *thd,
bool mysql_parse_status= MYSQLparse(thd) != 0;
- /* Check that if MYSQLparse() failed, thd->is_error() is set. */
-
- DBUG_ASSERT(!mysql_parse_status ||
- mysql_parse_status && thd->is_error());
-
/* Reset parser state. */
thd->m_parser_state= NULL;
=== modified file 'sql/sql_trigger.cc'
--- a/sql/sql_trigger.cc 2009-05-15 12:57:51 +0000
+++ b/sql/sql_trigger.cc 2009-09-01 08:56:15 +0000
@@ -292,6 +292,37 @@ private:
};
+class Old_syntax_trigger_handler : public Internal_error_handler
+{
+private:
+ bool m_got_trigger_name;
+
+public:
+
+ LEX_STRING trigger_name;
+
+ Old_syntax_trigger_handler() : m_got_trigger_name(FALSE) {}
+
+ bool handle_error(uint sql_errno, const char *message,
+ MYSQL_ERROR::enum_warning_level level, THD *thd)
+ {
+ if (sql_errno == ER_PARSE_ERROR)
+ {
+ if(thd->lex->spname)
+ {
+ m_got_trigger_name= TRUE;
+ trigger_name= thd->lex->spname->m_name;
+ }
+ return TRUE;
+ }
+ return FALSE;
+ }
+
+ bool got_trigger_name() { return m_got_trigger_name; }
+
+};
+
+
/**
Create or drop trigger for table.
@@ -344,7 +375,7 @@ bool mysql_create_or_drop_trigger(THD *t
need second part of condition below, since check_access() function also
checks that db is specified.
*/
- if (!thd->lex->spname->m_db.length || create && !tables->db_length)
+ if (!thd->lex->spname->m_db.length || (create && !tables->db_length))
{
my_error(ER_NO_DB_ERROR, MYF(0));
DBUG_RETURN(TRUE);
@@ -837,7 +868,7 @@ static bool rm_trigger_file(char *path,
@param path char buffer of size FN_REFLEN to be used
for constructing path to .TRN file.
@param db trigger's database name
- @param table_name trigger's name
+ @param trigger_name trigger's name
@retval
False success
@@ -1301,12 +1332,11 @@ bool Table_triggers_list::check_n_load(T
lex_start(thd);
thd->spcont= NULL;
- if (parse_sql(thd, & parser_state, creation_ctx))
- {
- /* Currently sphead is always deleted in case of a parse error */
- DBUG_ASSERT(lex.sphead == 0);
- goto err_with_lex_cleanup;
- }
+ Old_syntax_trigger_handler error_handler;
+ thd->push_internal_handler(&error_handler);
+ bool parse_error= parse_sql(thd, & parser_state, creation_ctx);
+ thd->pop_internal_handler();
+
/*
Not strictly necessary to invoke this method here, since we know
that we've parsed CREATE TRIGGER and not an
@@ -1317,6 +1347,36 @@ bool Table_triggers_list::check_n_load(T
*/
lex.set_trg_event_type_for_tables();
+ if (parse_error)
+ {
+ /* Currently sphead is always deleted in case of a parse error */
+ DBUG_ASSERT(lex.sphead == 0);
+ if (error_handler.got_trigger_name())
+ {
+ if (triggers->names_list.push_back(&error_handler.trigger_name,
+ &table->mem_root))
+ goto err_with_lex_cleanup;
+ else
+ {
+ /*
+ The Table_triggers_list is not constructed as a list of
+ trigger objects as one would expect, but rather of lists of
+ properties of equal length. Thus, even if we don't get the
+ trigger name, we still fill all in all the lists with
+ placeholders as we might otherwise create a skew in the
+ lists. Obviously, this has to be refactored.
+ */
+ LEX_STRING empty= thd->alloc_lex_string(NULL, "", 0, TRUE);
+ if (triggers->names_list.push_back(empty, &table->mem_root) ||
+ triggers->on_table_names_list.push_back(empty,
+ &table->mem_root))
+ goto err_with_lex_cleanup;
+ }
+ }
+ lex_end(&lex);
+ continue;
+ }
+
lex.sphead->set_info(0, 0, &lex.sp_chistics, (ulong) *trg_sql_mode);
int event= lex.trg_chistics.event;
@@ -1357,8 +1417,8 @@ bool Table_triggers_list::check_n_load(T
if (triggers->names_list.push_back(&lex.sphead->m_name,
&table->mem_root))
- goto err_with_lex_cleanup;
-
+ goto err_with_lex_cleanup;
+
if (!(on_table_name= alloc_lex_string(&table->mem_root)))
goto err_with_lex_cleanup;
@@ -1383,8 +1443,8 @@ bool Table_triggers_list::check_n_load(T
char fname[NAME_LEN + 1];
DBUG_ASSERT((!my_strcasecmp(table_alias_charset, lex.query_tables->db, db) ||
(check_n_cut_mysql50_prefix(db, fname, sizeof(fname)) &&
- !my_strcasecmp(table_alias_charset, lex.query_tables->db, fname))) &&
- (!my_strcasecmp(table_alias_charset, lex.query_tables->table_name,
+ !my_strcasecmp(table_alias_charset, lex.query_tables->db, fname))));
+ DBUG_ASSERT((!my_strcasecmp(table_alias_charset, lex.query_tables->table_name,
table_name) ||
(check_n_cut_mysql50_prefix(table_name, fname, sizeof(fname)) &&
!my_strcasecmp(table_alias_charset, lex.query_tables->table_name, fname))));
Attachment: [text/bzr-bundle] bzr/martin.hansson@sun.com-20090901085615-pqw377085krb6la3.bundle
