List:Commits« Previous MessageNext Message »
From:Jon Olav Hauglid Date:July 21 2010 10:35am
Subject:bzr commit into mysql-trunk-runtime branch (jon.hauglid:3081) Bug#54106
View as plain text  
#At file:///export/home/x/mysql-trunk-runtime-bug54106/ based on revid:jon.hauglid@stripped

 3081 Jon Olav Hauglid	2010-07-21
      Bug #54106 assert in Protocol::end_statement,
                 INSERT IGNORE ... SELECT ... UNION SELECT ...
      
      This assert was triggered by INSERT IGNORE ... SELECT. The assert checks that a
      statement either sends OK or an error to the client. If the bug was triggered
      on release builds, it caused OK to be sent to the client instead of the correct
      error message (in this case ER_FIELD_SPECIFIED_TWICE).
      
      The reason the assert was triggered, was that lex->no_error was set to TRUE
      during JOIN::optimize() because of IGNORE. This causes all errors to be ignored.
      However, not all errors can be ignored. Some, such as ER_FIELD_SPECIFIED_TWICE
      will cause the INSERT to fail no matter what. But since lex->no_error was set,
      the critical errors were ignored, the INSERT failed and neither OK nor the
      error message was sent to the client.
      
      This patch fixes the problem by temporarily turning off lex->no_error in
      places where errors cannot be ignored during processing of INSERT ... SELECT.
      
      Test case added to insert.test.

    modified:
      mysql-test/r/insert.result
      mysql-test/t/insert.test
      sql/sql_insert.cc
      sql/sql_update.cc
=== modified file 'mysql-test/r/insert.result'
--- a/mysql-test/r/insert.result	2010-04-11 06:52:42 +0000
+++ b/mysql-test/r/insert.result	2010-07-21 10:35:04 +0000
@@ -671,3 +671,18 @@ drop table t1;
 #
 # End of 5.4 tests
 #
+#
+# Bug#54106 assert in Protocol::end_statement,
+#           INSERT IGNORE ... SELECT ... UNION SELECT ...
+#
+DROP TABLE IF EXISTS t1;
+CREATE TABLE t1 (a INT);
+INSERT INTO t1 (a, a) VALUES (1, 1);
+ERROR 42000: Column 'a' specified twice
+INSERT IGNORE t1 (a, a) VALUES (1, 1);
+ERROR 42000: Column 'a' specified twice
+INSERT IGNORE t1 (a, a) SELECT 1,1;
+ERROR 42000: Column 'a' specified twice
+INSERT IGNORE t1 (a, a) SELECT 1,1 UNION SELECT 2,2;
+ERROR 42000: Column 'a' specified twice
+DROP TABLE t1;

=== modified file 'mysql-test/t/insert.test'
--- a/mysql-test/t/insert.test	2010-04-11 06:52:42 +0000
+++ b/mysql-test/t/insert.test	2010-07-21 10:35:04 +0000
@@ -525,3 +525,29 @@ drop table t1;
 --echo #
 --echo # End of 5.4 tests
 --echo #
+
+
+--echo #
+--echo # Bug#54106 assert in Protocol::end_statement,
+--echo #           INSERT IGNORE ... SELECT ... UNION SELECT ...
+--echo #
+
+--disable_warnings
+DROP TABLE IF EXISTS t1;
+--enable_warnings
+
+CREATE TABLE t1 (a INT);
+
+--error ER_FIELD_SPECIFIED_TWICE
+INSERT INTO t1 (a, a) VALUES (1, 1);
+# Verify that ER_FIELD_SPECIFIED_TWICE is not ignorable
+--error ER_FIELD_SPECIFIED_TWICE
+INSERT IGNORE t1 (a, a) VALUES (1, 1);
+
+--error ER_FIELD_SPECIFIED_TWICE
+INSERT IGNORE t1 (a, a) SELECT 1,1;
+# Used to cause an assert
+--error ER_FIELD_SPECIFIED_TWICE
+INSERT IGNORE t1 (a, a) SELECT 1,1 UNION SELECT 2,2;
+
+DROP TABLE t1;

=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc	2010-07-08 21:20:08 +0000
+++ b/sql/sql_insert.cc	2010-07-21 10:35:04 +0000
@@ -3057,9 +3057,14 @@ select_insert::prepare(List<Item> &value
     we are fixing fields from insert list.
   */
   lex->current_select= &lex->select_lex;
+
+  /* Errors during check_insert_fields() should not be ignored. */
+  bool no_error_save= lex->current_select->no_error;
+  lex->current_select->no_error= FALSE;
   res= (setup_fields(thd, 0, values, MARK_COLUMNS_READ, 0, 0) ||
         check_insert_fields(thd, table_list, *fields, values,
                             !insert_into_view, 1, &map));
+  lex->current_select->no_error= no_error_save;
 
   if (!res && fields->elements)
   {

=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc	2010-06-22 20:32:29 +0000
+++ b/sql/sql_update.cc	2010-07-21 10:35:04 +0000
@@ -1137,57 +1137,6 @@ int mysql_multi_update_prepare(THD *thd)
 }
 
 
-/**
-   Implementation of the safe update options during UPDATE IGNORE. This syntax
-   causes an UPDATE statement to ignore all errors. In safe update mode,
-   however, we must never ignore the ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE. There
-   is a special hook in my_message_sql that will otherwise delete all errors
-   when the IGNORE option is specified. 
-
-   In the future, all IGNORE handling should be used with this class and all
-   traces of the hack outlined below should be removed.
-
-   - The parser detects IGNORE option and sets thd->lex->ignore= 1
-   
-   - In JOIN::optimize, if this is set, then 
-     thd->lex->current_select->no_error gets set.
-
-   - In my_message_sql(), if the flag above is set then any error is
-     unconditionally converted to a warning.
-
-   We are moving in the direction of using Internal_error_handler subclasses
-   to do all such error tweaking, please continue this effort if new bugs
-   appear.
- */
-class Safe_dml_handler : public Internal_error_handler {
-
-private:
-  bool m_handled_error;
-
-public:
-  explicit Safe_dml_handler() : m_handled_error(FALSE) {}
-
-  bool handle_condition(THD *thd,
-                        uint sql_errno,
-                        const char* sqlstate,
-                        MYSQL_ERROR::enum_warning_level level,
-                        const char* msg,
-                        MYSQL_ERROR ** cond_hdl)
-  {
-    if (level == MYSQL_ERROR::WARN_LEVEL_ERROR &&
-        sql_errno == ER_UPDATE_WITHOUT_KEY_IN_SAFE_MODE)
-    {
-      thd->stmt_da->set_error_status(thd, sql_errno, msg, sqlstate);
-      m_handled_error= TRUE;
-      return TRUE;
-    }
-    return FALSE;
-  }
-
-  bool handled_error() { return m_handled_error; }
-
-};
-
 /*
   Setup multi-update handling and call SELECT to do the join
 */
@@ -1221,11 +1170,6 @@ bool mysql_multi_update(THD *thd,
 
   List<Item> total_list;
 
-  Safe_dml_handler handler;
-  bool using_handler= thd->variables.option_bits & OPTION_SAFE_UPDATES;
-  if (using_handler)
-    thd->push_internal_handler(&handler);
-
   res= mysql_select(thd, &select_lex->ref_pointer_array,
                     table_list, select_lex->with_wild,
                     total_list,
@@ -1235,21 +1179,9 @@ bool mysql_multi_update(THD *thd,
                     OPTION_SETUP_TABLES_DONE,
                     *result, unit, select_lex);
 
-  if (using_handler)
-  {
-    Internal_error_handler *top_handler;
-    top_handler= thd->pop_internal_handler();
-    DBUG_ASSERT(&handler == top_handler);
-  }
-
   DBUG_PRINT("info",("res: %d  report_error: %d", res, (int) thd->is_error()));
   res|= thd->is_error();
-  /*
-    Todo: remove below code and make Safe_dml_handler do error processing
-    instead. That way we can return the actual error instead of
-    ER_UNKNOWN_ERROR.
-  */
-  if (unlikely(res) && (!using_handler || !handler.handled_error()))
+  if (unlikely(res))
   {
     /* If we had a another error reported earlier then this will be ignored */
     (*result)->send_error(ER_UNKNOWN_ERROR, ER(ER_UNKNOWN_ERROR));
@@ -1513,8 +1445,16 @@ multi_update::initialize_tables(JOIN *jo
   TABLE_LIST *table_ref;
   DBUG_ENTER("initialize_tables");
 
-  if ((thd->variables.option_bits & OPTION_SAFE_UPDATES) && error_if_full_join(join))
-    DBUG_RETURN(1);
+  if (thd->variables.option_bits & OPTION_SAFE_UPDATES)
+  {
+    /* This error should not be ignored. */
+    bool no_error_save= thd->lex->current_select->no_error;
+    thd->lex->current_select->no_error= FALSE;
+    bool error= error_if_full_join(join);
+    thd->lex->current_select->no_error= no_error_save;
+    if (error)
+      DBUG_RETURN(TRUE);
+  }
   main_table=join->join_tab->table;
   table_to_update= 0;
 


Attachment: [text/bzr-bundle] bzr/jon.hauglid@oracle.com-20100721103504-6bqoqzxenx9lp2q1.bundle
Thread
bzr commit into mysql-trunk-runtime branch (jon.hauglid:3081) Bug#54106Jon Olav Hauglid21 Jul
  • Re: bzr commit into mysql-trunk-runtime branch (jon.hauglid:3081)Bug#54106Konstantin Osipov21 Jul