From: Tor Didriksen Date: September 7 2012 2:11pm Subject: bzr push into mysql-trunk branch (tor.didriksen:4443 to 4444) List-Archive: http://lists.mysql.com/commits/144714 Message-Id: <20120907141128.16209.48735.4444@atum07.no.oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 4444 Tor Didriksen 2012-09-07 [merge] merge 5.6 => trunk 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 4443 Akhil Mohan 2012-09-07 [merge] Empty version change upmerge === modified file 'sql/item_subselect.cc' --- a/sql/item_subselect.cc 2012-08-30 09:16:46 +0000 +++ b/sql/item_subselect.cc 2012-09-07 14:09:27 +0000 @@ -2768,13 +2768,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, @@ -2785,7 +2785,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 */ @@ -3022,7 +3022,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], @@ -3031,7 +3031,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 (;;) @@ -3062,7 +3062,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(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 + +#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).