List:Commits« Previous MessageNext Message »
From:Martin Hansson Date:September 1 2009 8:56am
Subject:bzr commit into mysql-5.1-bugteam branch (martin.hansson:2936) Bug#45235
View as plain text  
#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
Thread
bzr commit into mysql-5.1-bugteam branch (martin.hansson:2936) Bug#45235Martin Hansson1 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (martin.hansson:2936)Bug#45235Dmitry Lenev11 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (martin.hansson:2936)Bug#45235Martin Hansson21 Sep