List:Commits« Previous MessageNext Message »
From:Tor Didriksen Date:September 7 2012 2:11pm
Subject:bzr push into mysql-5.6 branch (tor.didriksen:4250 to 4251) Bug#14579877
View as plain text  
 4251 Tor Didriksen	2012-09-07
      Bug#14579877 GOT ERROR 159 WHEN READING TABLE
      
      Rename report_error() to report_handler_error().
      Don't print anything to the error log for HA_ERR_TABLE_DEF_CHANGED
      
      A typical scenario is like this:
      t1> begin; select * from t;
      t2> alter table u add primary key(a);
      t1> select * from u;
      -- gets HA_ERR_TABLE_DEF_CHANGED
      
      This is a temporary error (t1 can just re-start it's transaction)
      which should not fill the error log with ERROR messages.
      
      Add a simple unit test framework for testing the handler interface.
      Test desired behaviour with a unit test, rather than an mtr test.

    added:
      unittest/gunit/handler-t.cc
      unittest/gunit/handler-t.h
    modified:
      sql/item_subselect.cc
      sql/sql_executor.cc
      sql/sql_executor.h
      unittest/gunit/CMakeLists.txt
      unittest/gunit/fake_table.h
      unittest/gunit/test_utils.cc
 4250 Akhil Mohan	2012-09-07 [merge]
      Empty version change upmerge

=== modified file 'sql/item_subselect.cc'
--- a/sql/item_subselect.cc	2012-08-27 11:23:37 +0000
+++ b/sql/item_subselect.cc	2012-09-07 14:07:26 +0000
@@ -2772,13 +2772,13 @@ bool subselect_indexsubquery_engine::sca
   if (table->file->inited &&
       (error= table->file->ha_index_end()))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     DBUG_RETURN(true);
   }
  
   if ((error= table->file->ha_rnd_init(1)))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     DBUG_RETURN(true);
   }
   table->file->extra_opt(HA_EXTRA_CACHE,
@@ -2789,7 +2789,7 @@ bool subselect_indexsubquery_engine::sca
     error=table->file->ha_rnd_next(table->record[0]);
     if (error && error != HA_ERR_END_OF_FILE)
     {
-      error= report_error(table, error);
+      error= report_handler_error(table, error);
       break;
     }
     /* No more rows */
@@ -3026,7 +3026,7 @@ bool subselect_indexsubquery_engine::exe
   if (!table->file->inited &&
       (error= table->file->ha_index_init(tab->ref.key, !unique /* sorted */)))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     DBUG_RETURN(true);
   }
   error= table->file->ha_index_read_map(table->record[0],
@@ -3035,7 +3035,7 @@ bool subselect_indexsubquery_engine::exe
                                         HA_READ_KEY_EXACT);
   if (error &&
       error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-    error= report_error(table, error);
+    error= report_handler_error(table, error);
   else
   {
     for (;;)
@@ -3066,7 +3066,7 @@ bool subselect_indexsubquery_engine::exe
                                               tab->ref.key_length);
         if (error && error != HA_ERR_END_OF_FILE)
         {
-          error= report_error(table, error);
+          error= report_handler_error(table, error);
           break;
         }
       }

=== modified file 'sql/sql_executor.cc'
--- a/sql/sql_executor.cc	2012-08-29 13:40:11 +0000
+++ b/sql/sql_executor.cc	2012-09-07 14:07:26 +0000
@@ -1715,7 +1715,7 @@ evaluate_null_complemented_join_record(J
 
 /** Help function when we get some an error from the table handler. */
 
-int report_error(TABLE *table, int error)
+int report_handler_error(TABLE *table, int error)
 {
   if (error == HA_ERR_END_OF_FILE || error == HA_ERR_KEY_NOT_FOUND)
   {
@@ -1723,11 +1723,14 @@ int report_error(TABLE *table, int error
     return -1;					// key not found; ok
   }
   /*
-    Locking reads can legally return also these errors, do not
-    print them to the .err log
+    Do not spam the error log with these temporary errors:
+       LOCK_DEADLOCK LOCK_WAIT_TIMEOUT TABLE_DEF_CHANGED
+    Also skip printing to error log if the current thread has been killed.
   */
-  if (error != HA_ERR_LOCK_DEADLOCK && error != HA_ERR_LOCK_WAIT_TIMEOUT
-      && !table->in_use->killed)
+  if (error != HA_ERR_LOCK_DEADLOCK &&
+      error != HA_ERR_LOCK_WAIT_TIMEOUT &&
+      error != HA_ERR_TABLE_DEF_CHANGED &&
+      !table->in_use->killed)
     sql_print_error("Got error %d when reading table '%s'",
 		    error, table->s->path.str);
   table->file->print_error(error,MYF(0));
@@ -1743,7 +1746,7 @@ int safe_index_read(JOIN_TAB *tab)
                                             tab->ref.key_buff,
                                             make_prev_keypart_map(tab->ref.key_parts),
                                             HA_READ_KEY_EXACT)))
-    return report_error(table, error);
+    return report_handler_error(table, error);
   return 0;
 }
 
@@ -1897,7 +1900,7 @@ join_read_system(JOIN_TAB *tab)
 					   table->s->primary_key)))
     {
       if (error != HA_ERR_END_OF_FILE)
-	return report_error(table, error);
+	return report_handler_error(table, error);
       mark_as_null_row(tab->table);
       empty_record(table);			// Make empty record
       return -1;
@@ -1951,7 +1954,7 @@ join_read_const(JOIN_TAB *tab)
       empty_record(table);
       if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
       {
-        const int ret= report_error(table, error);
+        const int ret= report_handler_error(table, error);
         DBUG_RETURN(ret);
       }
       DBUG_RETURN(-1);
@@ -1998,7 +2001,7 @@ join_read_key(JOIN_TAB *tab)
     DBUG_ASSERT(!tab->sorted);  // Don't expect sort req. for single row.
     if ((error= table->file->ha_index_init(table_ref->key, tab->sorted)))
     {
-      (void) report_error(table, error);
+      (void) report_handler_error(table, error);
       return 1;
     }
   }
@@ -2029,7 +2032,7 @@ join_read_key(JOIN_TAB *tab)
                                           make_prev_keypart_map(table_ref->key_parts),
                                           HA_READ_KEY_EXACT);
     if (error && error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-      return report_error(table, error);
+      return report_handler_error(table, error);
 
     if (! error)
     {
@@ -2109,7 +2112,7 @@ join_read_linked_first(JOIN_TAB *tab)
                                       tab->ref.key_buff,
                                       make_prev_keypart_map(tab->ref.key_parts));
   if (unlikely(error && error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE))
-    DBUG_RETURN(report_error(table, error));
+    DBUG_RETURN(report_handler_error(table, error));
 
   table->null_row=0;
   int rc= table->status ? -1 : 0;
@@ -2126,7 +2129,7 @@ join_read_linked_next(READ_RECORD *info)
   if (error)
   {
     if (unlikely(error != HA_ERR_END_OF_FILE))
-      DBUG_RETURN(report_error(table, error));
+      DBUG_RETURN(report_handler_error(table, error));
     table->status= STATUS_GARBAGE;
     DBUG_RETURN(-1);
   }
@@ -2165,7 +2168,7 @@ join_read_always_key(JOIN_TAB *tab)
   if (!table->file->inited &&
       (error= table->file->ha_index_init(tab->ref.key, tab->sorted)))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     return 1;
   }
 
@@ -2185,7 +2188,7 @@ join_read_always_key(JOIN_TAB *tab)
                                              HA_READ_KEY_EXACT)))
   {
     if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-      return report_error(table, error);
+      return report_handler_error(table, error);
     return -1; /* purecov: inspected */
   }
   return 0;
@@ -2206,7 +2209,7 @@ join_read_last_key(JOIN_TAB *tab)
   if (!table->file->inited &&
       (error= table->file->ha_index_init(tab->ref.key, tab->sorted)))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     return 1;
   }
   if (cp_buffer_from_ref(tab->join->thd, table, &tab->ref))
@@ -2216,7 +2219,7 @@ join_read_last_key(JOIN_TAB *tab)
                                               make_prev_keypart_map(tab->ref.key_parts))))
   {
     if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-      return report_error(table, error);
+      return report_handler_error(table, error);
     return -1; /* purecov: inspected */
   }
   return 0;
@@ -2243,7 +2246,7 @@ join_read_next_same(READ_RECORD *info)
                                               tab->ref.key_length)))
   {
     if (error != HA_ERR_END_OF_FILE)
-      return report_error(table, error);
+      return report_handler_error(table, error);
     table->status= STATUS_GARBAGE;
     return -1;
   }
@@ -2269,7 +2272,7 @@ join_read_prev_same(READ_RECORD *info)
   DBUG_ASSERT(table->file->pushed_idx_cond == NULL);
 
   if ((error= table->file->ha_index_prev(table->record[0])))
-    return report_error(table, error);
+    return report_handler_error(table, error);
   if (key_cmp_if_same(table, tab->ref.key_buff, tab->ref.key,
                       tab->ref.key_length))
   {
@@ -2353,7 +2356,7 @@ int join_init_read_record(JOIN_TAB *tab)
   if (tab->select && tab->select->quick && (error= tab->select->quick->reset()))
   {
     /* Ensures error status is propageted back to client */
-    report_error(tab->table, error);
+    report_handler_error(tab->table, error);
     return 1;
   }
   if (init_read_record(&tab->read_record, tab->join->thd, tab->table,
@@ -2457,13 +2460,13 @@ join_read_first(JOIN_TAB *tab)
   if (!table->file->inited &&
       (error= table->file->ha_index_init(tab->index, tab->sorted)))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     return 1;
   }
   if ((error= tab->table->file->ha_index_first(tab->table->record[0])))
   {
     if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-      report_error(table, error);
+      report_handler_error(table, error);
     return -1;
   }
   return 0;
@@ -2475,7 +2478,7 @@ join_read_next(READ_RECORD *info)
 {
   int error;
   if ((error= info->table->file->ha_index_next(info->record)))
-    return report_error(info->table, error);
+    return report_handler_error(info->table, error);
   return 0;
 }
 
@@ -2495,11 +2498,11 @@ join_read_last(JOIN_TAB *tab)
   if (!table->file->inited &&
       (error= table->file->ha_index_init(tab->index, tab->sorted)))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     return 1;
   }
   if ((error= tab->table->file->ha_index_last(tab->table->record[0])))
-    return report_error(table, error);
+    return report_handler_error(table, error);
   return 0;
 }
 
@@ -2509,7 +2512,7 @@ join_read_prev(READ_RECORD *info)
 {
   int error;
   if ((error= info->table->file->ha_index_prev(info->record)))
-    return report_error(info->table, error);
+    return report_handler_error(info->table, error);
   return 0;
 }
 
@@ -2523,13 +2526,13 @@ join_ft_read_first(JOIN_TAB *tab)
   if (!table->file->inited &&
       (error= table->file->ha_index_init(tab->ref.key, tab->sorted)))
   {
-    (void) report_error(table, error);
+    (void) report_handler_error(table, error);
     return 1;
   }
   table->file->ft_init();
 
   if ((error= table->file->ft_read(table->record[0])))
-    return report_error(table, error);
+    return report_handler_error(table, error);
   return 0;
 }
 
@@ -2538,7 +2541,7 @@ join_ft_read_next(READ_RECORD *info)
 {
   int error;
   if ((error= info->table->file->ft_read(info->table->record[0])))
-    return report_error(info->table, error);
+    return report_handler_error(info->table, error);
   return 0;
 }
 

=== modified file 'sql/sql_executor.h'
--- a/sql/sql_executor.h	2012-08-13 20:13:27 +0000
+++ b/sql/sql_executor.h	2012-09-07 14:07:26 +0000
@@ -250,7 +250,10 @@ evaluate_join_record(JOIN *join, JOIN_TA
 void copy_fields(TMP_TABLE_PARAM *param);
 bool copy_funcs(Item **func_ptr, const THD *thd);
 bool cp_buffer_from_ref(THD *thd, TABLE *table, TABLE_REF *ref);
-int report_error(TABLE *table, int error);
+
+/** Help function when we get some an error from the table handler. */
+int report_handler_error(TABLE *table, int error);
+
 int safe_index_read(JOIN_TAB *tab);
 SORT_FIELD * make_unireg_sortorder(ORDER *order, uint *length,
                                   SORT_FIELD *sortorder);

=== modified file 'unittest/gunit/CMakeLists.txt'
--- a/unittest/gunit/CMakeLists.txt	2012-09-05 08:29:51 +0000
+++ b/unittest/gunit/CMakeLists.txt	2012-09-07 14:07:26 +0000
@@ -244,11 +244,11 @@ SET(TESTS
   dynarray
   filesort_buffer
   filesort_compare
-  my_fileutils
   mdl
   mdl_mytap
   my_bitmap
   my_error
+  my_fileutils
   my_regex
   sql_list
   sql_plist
@@ -264,6 +264,7 @@ SET(SERVER_TESTS
   delayable_insert_operation
   field
   get_diagnostics
+  handler
   item
   item_func_now_local
   join_tab_sort

=== modified file 'unittest/gunit/fake_table.h'
--- a/unittest/gunit/fake_table.h	2012-05-07 08:29:18 +0000
+++ b/unittest/gunit/fake_table.h	2012-09-07 14:07:26 +0000
@@ -26,12 +26,16 @@ class Fake_TABLE_SHARE : public TABLE_SH
 public:
   Fake_TABLE_SHARE(uint number_of_columns)
   {
+    static const char *fakepath= "fakepath";
     fields= number_of_columns;
     // fix if you plan to test with >32 columns.
     column_bitmap_size= sizeof(int);
     tmp_table= NO_TMP_TABLE;
     db_low_byte_first= true;
+    path.str= const_cast<char*>(fakepath);
+    path.length= strlen(path.str);
   }
+  ~Fake_TABLE_SHARE() {}
 };
 
 /*
@@ -48,6 +52,7 @@ class Fake_TABLE: public TABLE
   void inititalize()
   {
     s= &table_share;
+    file= NULL;
     in_use= current_thd;
     null_row= '\0';
     write_set= &write_set_struct;
@@ -90,6 +95,9 @@ public:
       which cannot have virtual member functions.
     */ 
   }
+
+  void set_handler(handler *h) { file= h; }
+  TABLE_SHARE *get_share() { return &table_share; }
 };
 
 #endif // FAKE_TABLE_H

=== added file 'unittest/gunit/handler-t.cc'
--- a/unittest/gunit/handler-t.cc	1970-01-01 00:00:00 +0000
+++ b/unittest/gunit/handler-t.cc	2012-09-07 14:07:26 +0000
@@ -0,0 +1,64 @@
+/* Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+
+   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., 51 Franklin St, Fifth Floor, Boston, MA  02111-1307  USA */
+
+#include "handler-t.h"
+#include "test_utils.h"
+#include "fake_table.h"
+#include "mock_field_datetime.h"
+
+#include "sql_executor.h"
+
+namespace {
+
+using my_testing::Server_initializer;
+using my_testing::Mock_error_handler;
+
+class HandlerTest : public ::testing::Test
+{
+protected:
+  virtual void SetUp() { initializer.SetUp(); }
+  virtual void TearDown() { initializer.TearDown(); }
+
+  THD *thd() { return initializer.thd(); }
+
+  Server_initializer initializer;
+};
+
+
+/**
+  Some handler error returns are passed on to report_handler_error()
+  which will:
+    - ignore errors like END_OF_FILE
+    - print most errors to the error log
+    - pass the error code back to handler::print_error()
+ */
+TEST_F(HandlerTest, ReportErrorHandler)
+{
+  Mock_field_datetime field_datetime;
+  Fake_TABLE table(&field_datetime);
+  Mock_HANDLER mock_handler(NULL, table.get_share());
+  table.set_handler(&mock_handler);
+
+  // This error should be ignored.
+  EXPECT_EQ(-1, report_handler_error(&table, HA_ERR_END_OF_FILE));
+  EXPECT_EQ(0, mock_handler.print_error_called());
+
+  // This one should not be printed to stderr, but passed on to the handler.
+  mock_handler.expect_error(HA_ERR_TABLE_DEF_CHANGED);
+  EXPECT_EQ(1, report_handler_error(&table, HA_ERR_TABLE_DEF_CHANGED));
+  EXPECT_EQ(1, mock_handler.print_error_called());
+}
+
+}

=== added file 'unittest/gunit/handler-t.h'
--- a/unittest/gunit/handler-t.h	1970-01-01 00:00:00 +0000
+++ b/unittest/gunit/handler-t.h	2012-09-07 14:07:26 +0000
@@ -0,0 +1,86 @@
+/* Copyright (c) 2012, Oracle and/or its affiliates. All rights reserved.
+
+   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., 51 Franklin St, Fifth Floor, Boston, MA  02111-1307  USA */
+
+// First include (the generated) my_config.h, to get correct platform defines.
+#include "my_config.h"
+#include <gtest/gtest.h>
+
+#include "handler.h"
+
+/**
+  A fake handler which implements all the pure virtuals.
+  This can be used as a base class for mock handlers.
+ */
+class Fake_HANDLER : public handler
+{
+public:
+  virtual int rnd_next(uchar*)
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual int rnd_pos(uchar*, uchar*)
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual void position(const uchar*)
+  { ADD_FAILURE() << "Unexpected call."; return; }
+  virtual int info(uint)
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual const char* table_type() const
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual const char** bas_ext() const
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual ulong index_flags(uint, uint, bool) const
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual THR_LOCK_DATA** store_lock(THD*, THR_LOCK_DATA**, thr_lock_type)
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual int open(const char*, int, uint)
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual int close()
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual int rnd_init(bool)
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual Table_flags table_flags() const
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+  virtual int create(const char*, TABLE*, HA_CREATE_INFO*)
+  { ADD_FAILURE() << "Unexpected call."; return 0; }
+
+  Fake_HANDLER(handlerton *ht_arg, TABLE_SHARE *share_arg)
+    : handler(ht_arg, share_arg)
+  {}
+};
+
+/**
+  A mock handler for testing print_error().
+  TODO: Use gmock instead of this class.
+ */
+class Mock_HANDLER : public Fake_HANDLER
+{
+public:
+  Mock_HANDLER(handlerton *ht_arg, TABLE_SHARE *share_arg)
+    : Fake_HANDLER(ht_arg, share_arg),
+      m_print_error_called(0),
+      m_expected_error(0)
+  {}
+  virtual void print_error(int error, myf errflag)
+  {
+    EXPECT_EQ(m_expected_error, error);
+    ++m_print_error_called;
+  }
+  void expect_error(int val)
+  {
+    m_expected_error= val;
+  }
+  int print_error_called() const { return m_print_error_called; }
+private:
+  int m_print_error_called;
+  int m_expected_error;
+};

=== modified file 'unittest/gunit/test_utils.cc'
--- a/unittest/gunit/test_utils.cc	2012-06-28 15:57:05 +0000
+++ b/unittest/gunit/test_utils.cc	2012-09-07 14:07:26 +0000
@@ -36,6 +36,7 @@ void setup_server_for_unit_tests()
   static char *my_name= strdup(my_progname);
   char *argv[] = { my_name, 0 };
   set_remaining_args(1, argv);
+  mysql_mutex_init(key_LOCK_error_log, &LOCK_error_log, MY_MUTEX_INIT_FAST);
   init_common_variables();
   my_init_signals();
   randominit(&sql_rand, 0, 0);
@@ -43,6 +44,8 @@ void setup_server_for_unit_tests()
   delegates_init();
   gtid_server_init();
   error_handler_hook= test_error_handler_hook;
+  // Initialize logger last, to avoid spurious warnings to stderr.
+  logger.init_base();
 }
 
 void teardown_server_for_unit_tests()
@@ -50,6 +53,9 @@ void teardown_server_for_unit_tests()
   delegates_destroy();
   xid_cache_free();
   gtid_server_cleanup();
+  mysql_mutex_destroy(&LOCK_error_log);
+  logger.cleanup_base();
+  logger.cleanup_end();
 }
 
 void Server_initializer::set_expected_error(uint val)
@@ -87,7 +93,14 @@ Mock_error_handler::~Mock_error_handler(
   // Strange Visual Studio bug: have to store 'this' in local variable.
   Internal_error_handler *me= this;
   EXPECT_EQ(me, m_thd->pop_internal_handler());
-  EXPECT_GE(m_handle_called, 0);
+  if (m_expected_error == 0)
+  {
+    EXPECT_EQ(0, m_handle_called);
+  }
+  else
+  {
+    EXPECT_GT(m_handle_called, 0);
+  }
 }
 
 bool Mock_error_handler::handle_condition(THD *thd,

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.6 branch (tor.didriksen:4250 to 4251) Bug#14579877Tor Didriksen10 Sep