From db1fee989d58252ddb75cda924e7c37a28020e7c Mon Sep 17 00:00:00 2001 From: He Zhenxing Date: Fri, 4 Dec 2009 09:43:39 +0800 Subject: [PATCH 01/16] Bug#49170 Inconsistent placement of semisync plugin prevents it from getting tested Add $basedir/lib/plugin to the search paths for semisync plugins. --- mysql-test/mysql-test-run.pl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index ddee67c124e..e88548595ef 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -1834,11 +1834,13 @@ sub environment_setup { my $lib_semisync_master_plugin= mtr_file_exists(vs_config_dirs('plugin/semisync',$semisync_master_filename), "$basedir/plugin/semisync/.libs/" . $semisync_master_filename, - "$basedir/lib/mysql/plugin/" . $semisync_master_filename); + "$basedir/lib/mysql/plugin/" . $semisync_master_filename, + "$basedir/lib/plugin/" . $semisync_master_filename); my $lib_semisync_slave_plugin= mtr_file_exists(vs_config_dirs('plugin/semisync',$semisync_slave_filename), "$basedir/plugin/semisync/.libs/" . $semisync_slave_filename, - "$basedir/lib/mysql/plugin/" . $semisync_slave_filename); + "$basedir/lib/mysql/plugin/" . $semisync_slave_filename, + "$basedir/lib/plugin/" . $semisync_slave_filename); if ($lib_semisync_master_plugin && $lib_semisync_slave_plugin) { $ENV{'SEMISYNC_MASTER_PLUGIN'}= basename($lib_semisync_master_plugin); From 04c39b1939abc7f80289b0feffe715e29a07f5ad Mon Sep 17 00:00:00 2001 From: Bjorn Munch Date: Tue, 5 Jan 2010 13:05:00 +0100 Subject: [PATCH 02/16] Bug #49166 mtr --combination is broken after restrictions of combination names Combinations beginning with -- not allowed Allow them... --- mysql-test/mysql-test-run.pl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index 7d2426459d0..27c2ace56b2 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -3251,9 +3251,11 @@ sub run_testcase ($) { mtr_verbose("Running test:", $tinfo->{name}); - # Allow only alpanumerics pluss _ - + . in combination names + # Allow only alpanumerics pluss _ - + . in combination names, + # or anything beginning with -- (the latter comes from --combination) my $combination= $tinfo->{combination}; - if ($combination && $combination !~ /^\w[-\w\.\+]+$/) + if ($combination && $combination !~ /^\w[-\w\.\+]+$/ + && $combination !~ /^--/) { mtr_error("Combination '$combination' contains illegal characters"); } From 480054c64d39c840756cb7dbece5e951d7d1006c Mon Sep 17 00:00:00 2001 From: Bjorn Munch Date: Thu, 28 Jan 2010 13:01:01 +0100 Subject: [PATCH 03/16] upmerge 49210 --- mysql-test/lib/v1/mysql-test-run.pl | 6 ++++-- mysql-test/mysql-test-run.pl | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/mysql-test/lib/v1/mysql-test-run.pl b/mysql-test/lib/v1/mysql-test-run.pl index 9630c65ade4..5d06d9c4dd8 100755 --- a/mysql-test/lib/v1/mysql-test-run.pl +++ b/mysql-test/lib/v1/mysql-test-run.pl @@ -1107,14 +1107,16 @@ sub command_line_setup () { if ( ! $opt_testcase_timeout ) { - $opt_testcase_timeout= $default_testcase_timeout; + $opt_testcase_timeout= + $ENV{MTR_TESTCASE_TIMEOUT} || $default_testcase_timeout; $opt_testcase_timeout*= 10 if $opt_valgrind; $opt_testcase_timeout*= 10 if ($opt_debug and $glob_win32); } if ( ! $opt_suite_timeout ) { - $opt_suite_timeout= $default_suite_timeout; + $opt_suite_timeout= + $ENV{MTR_SUITE_TIMEOUT} || $default_suite_timeout; $opt_suite_timeout*= 6 if $opt_valgrind; $opt_suite_timeout*= 6 if ($opt_debug and $glob_win32); } diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index e42ec86a7f6..b006145a677 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -206,10 +206,10 @@ my $opt_mark_progress; my $opt_sleep; -my $opt_testcase_timeout= 15; # minutes -my $opt_suite_timeout = 300; # minutes -my $opt_shutdown_timeout= 10; # seconds -my $opt_start_timeout = 180; # seconds +my $opt_testcase_timeout= $ENV{MTR_TESTCASE_TIMEOUT} || 15; # minutes +my $opt_suite_timeout = $ENV{MTR_SUITE_TIMEOUT} || 300; # minutes +my $opt_shutdown_timeout= $ENV{MTR_SHUTDOWN_TIMEOUT} || 10; # seconds +my $opt_start_timeout = $ENV{MTR_START_TIMEOUT} || 180; # seconds sub testcase_timeout { return $opt_testcase_timeout * 60; }; sub suite_timeout { return $opt_suite_timeout * 60; }; From a1bfae20cb3e9cc8919bf4902865314d1bf9d6f7 Mon Sep 17 00:00:00 2001 From: unknown Date: Sun, 31 Jan 2010 02:26:51 +0800 Subject: [PATCH 04/16] BUG#50157 Assertion !active_tranxs_->is_tranx_end_pos(..) in ReplSemiSyncMaster::commitTrx The root cause of the crash is that a TranxNode is freed before it is used. A TranxNode is allocated and inserted into the active list each time a log event is written and flushed into the binlog file. The memory for TranxNode is allocated with thd_alloc and will be freed at the end of the statement. The after_commit/after_rollback callback was supposed to be called before the end of each statement and remove the node from the active list. However this assumption is not correct in all cases(e.g. call 'CREATE TEMPORARY TABLE myisam_t SELECT * FROM innodb_t' in a transaction and delete all temporary tables automatically when a session closed), and can cause the memory allocated for TranxNode be freed before it was removed from the active list. So The TranxNode pointer in the active list would become a wild pointer and cause the crash. After this patch, We have a class called a TranxNodeAllocate which manages the memory for allocating and freeing TranxNode. It uses my_malloc to allocate memory. sql/rpl_handler.cc: params are not initialized. --- mysql-test/suite/rpl/r/rpl_semi_sync.result | 31 ++- mysql-test/suite/rpl/t/rpl_semi_sync.test | 33 ++- plugin/semisync/semisync_master.cc | 25 +- plugin/semisync/semisync_master.h | 284 ++++++++++++++++++-- sql/rpl_handler.cc | 10 +- 5 files changed, 335 insertions(+), 48 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_semi_sync.result b/mysql-test/suite/rpl/r/rpl_semi_sync.result index 1e220b28d78..74eb14d33e0 100644 --- a/mysql-test/suite/rpl/r/rpl_semi_sync.result +++ b/mysql-test/suite/rpl/r/rpl_semi_sync.result @@ -120,8 +120,27 @@ min(a) select max(a) from t1; max(a) 300 + +# BUG#50157 +# semi-sync replication crashes when replicating a transaction which +# include 'CREATE TEMPORARY TABLE `MyISAM_t` SELECT * FROM `Innodb_t` ; +[ on master ] +SET SESSION AUTOCOMMIT= 0; +CREATE TABLE t2(c1 INT) ENGINE=innodb; +BEGIN; + +# Even though it is in a transaction, this statement is binlogged into binlog +# file immediately. +CREATE TEMPORARY TABLE t3 SELECT c1 FROM t2 where 1=1; + +# These statements will not be binlogged until the transaction is committed +INSERT INTO t2 VALUES(11); +INSERT INTO t2 VALUES(22); +COMMIT; +DROP TABLE t2, t3; +SET SESSION AUTOCOMMIT= 1; # -# Test semi-sync master will switch OFF after one transacton +# Test semi-sync master will switch OFF after one transaction # timeout waiting for slave reply. # include/stop_slave.inc @@ -135,7 +154,7 @@ Variable_name Value Rpl_semi_sync_master_no_tx 0 show status like 'Rpl_semi_sync_master_yes_tx'; Variable_name Value -Rpl_semi_sync_master_yes_tx 301 +Rpl_semi_sync_master_yes_tx 304 show status like 'Rpl_semi_sync_master_clients'; Variable_name Value Rpl_semi_sync_master_clients 1 @@ -150,7 +169,7 @@ Variable_name Value Rpl_semi_sync_master_no_tx 1 show status like 'Rpl_semi_sync_master_yes_tx'; Variable_name Value -Rpl_semi_sync_master_yes_tx 301 +Rpl_semi_sync_master_yes_tx 304 insert into t1 values (100); [ master status should be OFF ] show status like 'Rpl_semi_sync_master_status'; @@ -161,7 +180,7 @@ Variable_name Value Rpl_semi_sync_master_no_tx 302 show status like 'Rpl_semi_sync_master_yes_tx'; Variable_name Value -Rpl_semi_sync_master_yes_tx 301 +Rpl_semi_sync_master_yes_tx 304 # # Test semi-sync status on master will be ON again when slave catches up # @@ -194,7 +213,7 @@ Variable_name Value Rpl_semi_sync_master_no_tx 302 show status like 'Rpl_semi_sync_master_yes_tx'; Variable_name Value -Rpl_semi_sync_master_yes_tx 301 +Rpl_semi_sync_master_yes_tx 304 show status like 'Rpl_semi_sync_master_clients'; Variable_name Value Rpl_semi_sync_master_clients 1 @@ -213,7 +232,7 @@ Variable_name Value Rpl_semi_sync_master_no_tx 302 SHOW STATUS LIKE 'Rpl_semi_sync_master_yes_tx'; Variable_name Value -Rpl_semi_sync_master_yes_tx 302 +Rpl_semi_sync_master_yes_tx 305 FLUSH NO_WRITE_TO_BINLOG STATUS; [ Semi-sync master status variables after FLUSH STATUS ] SHOW STATUS LIKE 'Rpl_semi_sync_master_no_tx'; diff --git a/mysql-test/suite/rpl/t/rpl_semi_sync.test b/mysql-test/suite/rpl/t/rpl_semi_sync.test index 4900acc1e91..b04541aba21 100644 --- a/mysql-test/suite/rpl/t/rpl_semi_sync.test +++ b/mysql-test/suite/rpl/t/rpl_semi_sync.test @@ -11,6 +11,7 @@ disable_query_log; connection master; call mtr.add_suppression("Timeout waiting for reply of binlog"); call mtr.add_suppression("Read semi-sync reply"); +call mtr.add_suppression("Unsafe statement binlogged in statement format since BINLOG_FORMAT = STATEMENT."); connection slave; call mtr.add_suppression("Master server does not support semi-sync"); call mtr.add_suppression("Semi-sync slave .* reply"); @@ -193,8 +194,38 @@ select count(distinct a) from t1; select min(a) from t1; select max(a) from t1; +--echo +--echo # BUG#50157 +--echo # semi-sync replication crashes when replicating a transaction which +--echo # include 'CREATE TEMPORARY TABLE `MyISAM_t` SELECT * FROM `Innodb_t` ; + +connection master; +echo [ on master ]; +SET SESSION AUTOCOMMIT= 0; +CREATE TABLE t2(c1 INT) ENGINE=innodb; +sync_slave_with_master; + +connection master; +BEGIN; +--echo +--echo # Even though it is in a transaction, this statement is binlogged into binlog +--echo # file immediately. +--disable_warnings +CREATE TEMPORARY TABLE t3 SELECT c1 FROM t2 where 1=1; +--enable_warnings +--echo +--echo # These statements will not be binlogged until the transaction is committed +INSERT INTO t2 VALUES(11); +INSERT INTO t2 VALUES(22); +COMMIT; + +DROP TABLE t2, t3; +SET SESSION AUTOCOMMIT= 1; +sync_slave_with_master; + + --echo # ---echo # Test semi-sync master will switch OFF after one transacton +--echo # Test semi-sync master will switch OFF after one transaction --echo # timeout waiting for slave reply. --echo # connection slave; diff --git a/plugin/semisync/semisync_master.cc b/plugin/semisync/semisync_master.cc index c2e329e1fe4..bbbc2a669c4 100644 --- a/plugin/semisync/semisync_master.cc +++ b/plugin/semisync/semisync_master.cc @@ -65,7 +65,7 @@ static int gettimeofday(struct timeval *tv, void *tz) ActiveTranx::ActiveTranx(pthread_mutex_t *lock, unsigned long trace_level) - : Trace(trace_level), + : Trace(trace_level), allocator_(max_connections), num_entries_(max_connections << 1), /* Transaction hash table size * is set to double the size * of max_connections */ @@ -115,25 +115,6 @@ unsigned int ActiveTranx::get_hash_value(const char *log_file_name, return (hash1 + hash2) % num_entries_; } -ActiveTranx::TranxNode* ActiveTranx::alloc_tranx_node() -{ - MYSQL_THD thd= (MYSQL_THD)current_thd; - /* The memory allocated for TranxNode will be automatically freed at - the end of the command of current THD. And because - ha_autocommit_or_rollback() will always be called before that, so - we are sure that the node will be removed from the active list - before it get freed. */ - TranxNode *trx_node = (TranxNode *)thd_alloc(thd, sizeof(TranxNode)); - if (trx_node) - { - trx_node->log_name_[0] = '\0'; - trx_node->log_pos_= 0; - trx_node->next_= 0; - trx_node->hash_next_= 0; - } - return trx_node; -} - int ActiveTranx::compare(const char *log_file_name1, my_off_t log_file_pos1, const char *log_file_name2, my_off_t log_file_pos2) { @@ -159,7 +140,7 @@ int ActiveTranx::insert_tranx_node(const char *log_file_name, function_enter(kWho); - ins_node = alloc_tranx_node(); + ins_node = allocator_.allocate_node(); if (!ins_node) { sql_print_error("%s: transaction node allocation failed for: (%s, %lu)", @@ -271,6 +252,7 @@ int ActiveTranx::clear_active_tranx_nodes(const char *log_file_name, /* Clear the hash table. */ memset(trx_htb_, 0, num_entries_ * sizeof(TranxNode *)); + allocator_.free_all_nodes(); /* Clear the active transaction list. */ if (trx_front_ != NULL) @@ -311,6 +293,7 @@ int ActiveTranx::clear_active_tranx_nodes(const char *log_file_name, } trx_front_ = new_front; + allocator_.free_nodes_before(trx_front_); if (trace_level_ & kTraceDetail) sql_print_information("%s: cleared %d nodes back until pos (%s, %lu)", diff --git a/plugin/semisync/semisync_master.h b/plugin/semisync/semisync_master.h index bfb1cb74cd0..982a7f77a59 100644 --- a/plugin/semisync/semisync_master.h +++ b/plugin/semisync/semisync_master.h @@ -20,6 +20,267 @@ #include "semisync.h" +struct TranxNode { + char log_name_[FN_REFLEN]; + my_off_t log_pos_; + struct TranxNode *next_; /* the next node in the sorted list */ + struct TranxNode *hash_next_; /* the next node during hash collision */ +}; + +/** + @class TranxNodeAllocator + + This class provides memory allocating and freeing methods for + TranxNode. The main target is performance. + + @section ALLOCATE How to allocate a node + The pointer of the first node after 'last_node' in current_block is + returned. current_block will move to the next free Block when all nodes of + it are in use. A new Block is allocated and is put into the rear of the + Block link table if no Block is free. + + The list starts up empty (ie, there is no allocated Block). + + After some nodes are freed, there probably are some free nodes before + the sequence of the allocated nodes, but we do not reuse it. It is better + to keep the allocated nodes are in the sequence, for it is more efficient + for allocating and freeing TranxNode. + + @section FREENODE How to free nodes + There are two methods for freeing nodes. They are free_all_nodes and + free_nodes_before. + + 'A Block is free' means all of its nodes are free. + @subsection free_nodes_before + As all allocated nodes are in the sequence, 'Before one node' means all + nodes before given node in the same Block and all Blocks before the Block + which containing the given node. As such, all Blocks before the given one + ('node') are free Block and moved into the rear of the Block link table. + The Block containing the given 'node', however, is not. For at least the + given 'node' is still in use. This will waste at most one Block, but it is + more efficient. + */ +#define BLOCK_TRANX_NODES 16 +class TranxNodeAllocator +{ +public: + /** + @param reserved_nodes + The number of reserved TranxNodes. It is used to set 'reserved_blocks' + which can contain at least 'reserved_nodes' number of TranxNodes. When + freeing memory, we will reserve at least reserved_blocks of Blocks not + freed. + */ + TranxNodeAllocator(uint reserved_nodes) : + reserved_blocks(reserved_nodes/BLOCK_TRANX_NODES + + (reserved_nodes%BLOCK_TRANX_NODES > 1 ? 2 : 1)), + first_block(NULL), last_block(NULL), + current_block(NULL), last_node(-1), block_num(0) {} + + ~TranxNodeAllocator() + { + Block *block= first_block; + while (block != NULL) + { + Block *next= block->next; + free_block(block); + block= next; + } + } + + /** + The pointer of the first node after 'last_node' in current_block is + returned. current_block will move to the next free Block when all nodes of + it are in use. A new Block is allocated and is put into the rear of the + Block link table if no Block is free. + + @return Return a TranxNode *, or NULL if an error occured. + */ + TranxNode *allocate_node() + { + TranxNode *trx_node; + Block *block= current_block; + + if (last_node == BLOCK_TRANX_NODES-1) + { + current_block= current_block->next; + last_node= -1; + } + + if (current_block == NULL && allocate_block()) + { + current_block= block; + if (current_block) + last_node= BLOCK_TRANX_NODES-1; + return NULL; + } + + trx_node= &(current_block->nodes[++last_node]); + trx_node->log_name_[0] = '\0'; + trx_node->log_pos_= 0; + trx_node->next_= 0; + trx_node->hash_next_= 0; + return trx_node; + } + + /** + All nodes are freed. + + @return Return 0, or 1 if an error occured. + */ + int free_all_nodes() + { + current_block= first_block; + last_node= -1; + free_blocks(); + return 0; + } + + /** + All Blocks before the given 'node' are free Block and moved into the rear + of the Block link table. + + @param node All nodes before 'node' will be freed + + @return Return 0, or 1 if an error occured. + */ + int free_nodes_before(TranxNode* node) + { + Block *block; + Block *prev_block; + + block= first_block; + while (block != current_block->next) + { + /* Find the Block containing the given node */ + if (&(block->nodes[0]) <= node && &(block->nodes[BLOCK_TRANX_NODES]) >= node) + { + /* All Blocks before the given node are put into the rear */ + if (first_block != block) + { + last_block->next= first_block; + first_block= block; + last_block= prev_block; + last_block->next= NULL; + free_blocks(); + } + return 0; + } + prev_block= block; + block= block->next; + } + + /* Node does not find should never happen */ + DBUG_ASSERT(0); + return 1; + } + +private: + uint reserved_blocks; + + /** + A sequence memory which contains BLOCK_TRANX_NODES TranxNodes. + + BLOCK_TRANX_NODES The number of TranxNodes which are in a Block. + + next Every Block has a 'next' pointer which points to the next Block. + These linking Blocks constitute a Block link table. + */ + struct Block { + Block *next; + TranxNode nodes[BLOCK_TRANX_NODES]; + }; + + /** + The 'first_block' is the head of the Block link table; + */ + Block *first_block; + /** + The 'last_block' is the rear of the Block link table; + */ + Block *last_block; + + /** + current_block always points the Block in the Block link table in + which the last allocated node is. The Blocks before it are all in use + and the Blocks after it are all free. + */ + Block *current_block; + + /** + It always points to the last node which has been allocated in the + current_block. + */ + int last_node; + + /** + How many Blocks are in the Block link table. + */ + uint block_num; + + /** + Allocate a block and then assign it to current_block. + */ + int allocate_block() + { + Block *block= (Block *)my_malloc(sizeof(Block), MYF(0)); + if (block) + { + block->next= NULL; + + if (first_block == NULL) + first_block= block; + else + last_block->next= block; + + /* New Block is always put into the rear */ + last_block= block; + /* New Block is always the current_block */ + current_block= block; + ++block_num; + return 0; + } + return 1; + } + + /** + Free a given Block. + @param block The Block will be freed. + */ + void free_block(Block *block) + { + my_free(block, MYF(0)); + --block_num; + } + + + /** + If there are some free Blocks and the total number of the Blocks in the + Block link table is larger than the 'reserved_blocks', Some free Blocks + will be freed until the total number of the Blocks is equal to the + 'reserved_blocks' or there is only one free Block behind the + 'current_block'. + */ + void free_blocks() + { + if (current_block == NULL || current_block->next == NULL) + return; + + /* One free Block is always kept behind the current block */ + Block *block= current_block->next->next; + while (block_num > reserved_blocks && block != NULL) + { + Block *next= block->next; + free_block(block); + block= next; + } + current_block->next->next= block; + if (block == NULL) + last_block= current_block->next; + } +}; + + /** This class manages memory for active transaction list. @@ -31,13 +292,8 @@ class ActiveTranx :public Trace { private: - struct TranxNode { - char log_name_[FN_REFLEN]; - my_off_t log_pos_; - struct TranxNode *next_; /* the next node in the sorted list */ - struct TranxNode *hash_next_; /* the next node during hash collision */ - }; + TranxNodeAllocator allocator_; /* These two record the active transaction list in sort order. */ TranxNode *trx_front_, *trx_rear_; @@ -48,24 +304,22 @@ private: inline void assert_lock_owner(); - inline TranxNode* alloc_tranx_node(); - inline unsigned int calc_hash(const unsigned char *key,unsigned int length); unsigned int get_hash_value(const char *log_file_name, my_off_t log_file_pos); int compare(const char *log_file_name1, my_off_t log_file_pos1, - const TranxNode *node2) { + const TranxNode *node2) { return compare(log_file_name1, log_file_pos1, - node2->log_name_, node2->log_pos_); + node2->log_name_, node2->log_pos_); } int compare(const TranxNode *node1, - const char *log_file_name2, my_off_t log_file_pos2) { + const char *log_file_name2, my_off_t log_file_pos2) { return compare(node1->log_name_, node1->log_pos_, - log_file_name2, log_file_pos2); + log_file_name2, log_file_pos2); } int compare(const TranxNode *node1, const TranxNode *node2) { return compare(node1->log_name_, node1->log_pos_, - node2->log_name_, node2->log_pos_); + node2->log_name_, node2->log_pos_); } public: @@ -88,7 +342,7 @@ public: * 0: success; non-zero: error */ int clear_active_tranx_nodes(const char *log_file_name, - my_off_t log_file_pos); + my_off_t log_file_pos); /* Given a position, check to see whether the position is an active * transaction's ending position by probing the hash table. @@ -99,7 +353,7 @@ public: * (file_name, file_position). */ static int compare(const char *log_file_name1, my_off_t log_file_pos1, - const char *log_file_name2, my_off_t log_file_pos2); + const char *log_file_name2, my_off_t log_file_pos2); }; diff --git a/sql/rpl_handler.cc b/sql/rpl_handler.cc index c4b55e3d068..b347b7c751d 100644 --- a/sql/rpl_handler.cc +++ b/sql/rpl_handler.cc @@ -190,8 +190,8 @@ int Trans_delegate::after_commit(THD *thd, bool all) { Trans_param param; bool is_real_trans= (all || thd->transaction.all.ha_list == 0); - if (is_real_trans) - param.flags |= TRANS_IS_REAL_TRANS; + + param.flags = is_real_trans ? TRANS_IS_REAL_TRANS : 0; Trans_binlog_info *log_info= my_pthread_getspecific_ptr(Trans_binlog_info*, RPL_TRANS_BINLOG_INFO); @@ -218,8 +218,8 @@ int Trans_delegate::after_rollback(THD *thd, bool all) { Trans_param param; bool is_real_trans= (all || thd->transaction.all.ha_list == 0); - if (is_real_trans) - param.flags |= TRANS_IS_REAL_TRANS; + + param.flags = is_real_trans ? TRANS_IS_REAL_TRANS : 0; Trans_binlog_info *log_info= my_pthread_getspecific_ptr(Trans_binlog_info*, RPL_TRANS_BINLOG_INFO); @@ -228,7 +228,7 @@ int Trans_delegate::after_rollback(THD *thd, bool all) param.log_pos= log_info ? log_info->log_pos : 0; int ret= 0; - FOREACH_OBSERVER(ret, after_commit, thd, (¶m)); + FOREACH_OBSERVER(ret, after_rollback, thd, (¶m)); /* This is the end of a real transaction or autocommit statement, we From 55aab082a957e3f20e94f791dbe44750b4b465c8 Mon Sep 17 00:00:00 2001 From: Luis Soares Date: Wed, 3 Feb 2010 16:56:17 +0000 Subject: [PATCH 05/16] BUG#50364: FLUSH LOGS crashes the server (rpl.rpl_heartbeat_basic fails in PB sporadically) The IO thread can concurrently access the relay log IO_CACHE while another thread is performing an FLUSH LOGS procedure. FLUSH LOGS closes and reopens the relay log and while doing so it (re)initializes its IO_CACHE. During this procedure the IO_CACHE mutex is also reinitialized, which can cause problems if some other thread (namely the IO THREAD) is concurrently accessing it at the time . This patch fixes the problem by extending the interface of the flush_master_info function to also include a second paramater, "need_relay_log_lock", stating whether the thread should grab the relay log lock or not before actually flushing the relay log. Also, IO thread now calls flush_master_info with this flag set when it flushes master info with in the event read_event loop. Finally, we also increase loop time in rpl_heartbeat_basic test case, so that the number of calls to flush logs doubles, stressing this part of the code a little more. mysql-test/suite/rpl/t/rpl_heartbeat_basic.test: Doubled the number of iterations on the FLUSH LOGS loop by doubling the time available to perform all iterations. sql/repl_failsafe.cc: Updating flush_master_info call so that it uses two parameters instead of one. sql/rpl_mi.cc: Updating flush_master_info call so that it uses two parameters instead of one. sql/rpl_mi.h: Changed flush_master_info interface. Now takes a second parameter instead of just one. The second parameter is: need_lock_relay_log. sql/rpl_rli.cc: Small fix in comment. sql/slave.cc: Updating flush_master_info call so that it uses two parameters instead of one. sql/sql_repl.cc: Updating flush_master_info call so that it uses two parameters instead of one. --- .../suite/rpl/t/rpl_heartbeat_basic.test | 2 +- sql/repl_failsafe.cc | 2 +- sql/rpl_mi.cc | 19 ++++++++++++++++--- sql/rpl_mi.h | 4 +++- sql/rpl_rli.cc | 2 +- sql/slave.cc | 4 ++-- sql/sql_repl.cc | 2 +- 7 files changed, 25 insertions(+), 10 deletions(-) diff --git a/mysql-test/suite/rpl/t/rpl_heartbeat_basic.test b/mysql-test/suite/rpl/t/rpl_heartbeat_basic.test index 10d327eece0..d371d5916c2 100644 --- a/mysql-test/suite/rpl/t/rpl_heartbeat_basic.test +++ b/mysql-test/suite/rpl/t/rpl_heartbeat_basic.test @@ -382,7 +382,7 @@ let $slave_param_comparison= =; let $rcvd_heartbeats_before= query_get_value(SHOW STATUS LIKE 'slave_received_heartbeats', Value, 1); # Flush logs every 0.1 second during 5 sec --disable_query_log -let $i=50; +let $i=100; while ($i) { FLUSH LOGS; dec $i; diff --git a/sql/repl_failsafe.cc b/sql/repl_failsafe.cc index 7a941b1d99b..275571c2158 100644 --- a/sql/repl_failsafe.cc +++ b/sql/repl_failsafe.cc @@ -977,7 +977,7 @@ bool load_master_data(THD* thd) host was specified; there could have been a problem when replication started, which led to relay log's IO_CACHE to not be inited. */ - if (flush_master_info(active_mi, 0)) + if (flush_master_info(active_mi, FALSE, FALSE)) sql_print_error("Failed to flush master info file"); } mysql_free_result(master_status_res); diff --git a/sql/rpl_mi.cc b/sql/rpl_mi.cc index e83e0ad0ba9..28b0af441f8 100644 --- a/sql/rpl_mi.cc +++ b/sql/rpl_mi.cc @@ -387,7 +387,7 @@ file '%s')", fname); mi->rli.is_relay_log_recovery= FALSE; // now change cache READ -> WRITE - must do this before flush_master_info reinit_io_cache(&mi->file, WRITE_CACHE, 0L, 0, 1); - if ((error=test(flush_master_info(mi, 1)))) + if ((error=test(flush_master_info(mi, TRUE, TRUE)))) sql_print_error("Failed to flush master info file"); pthread_mutex_unlock(&mi->data_lock); DBUG_RETURN(error); @@ -413,7 +413,9 @@ err: 1 - flush master info failed 0 - all ok */ -int flush_master_info(Master_info* mi, bool flush_relay_log_cache) +int flush_master_info(Master_info* mi, + bool flush_relay_log_cache, + bool need_lock_relay_log) { IO_CACHE* file = &mi->file; char lbuf[22]; @@ -436,8 +438,19 @@ int flush_master_info(Master_info* mi, bool flush_relay_log_cache) */ if (flush_relay_log_cache) { + pthread_mutex_t *log_lock= mi->rli.relay_log.get_log_lock(); IO_CACHE *log_file= mi->rli.relay_log.get_log_file(); - if (flush_io_cache(log_file)) + + if (need_lock_relay_log) + pthread_mutex_lock(log_lock); + + safe_mutex_assert_owner(log_lock); + err= flush_io_cache(log_file); + + if (need_lock_relay_log) + pthread_mutex_unlock(log_lock); + + if (err) DBUG_RETURN(2); } diff --git a/sql/rpl_mi.h b/sql/rpl_mi.h index f822a6bc1b1..363c8d4e0b3 100644 --- a/sql/rpl_mi.h +++ b/sql/rpl_mi.h @@ -120,7 +120,9 @@ int init_master_info(Master_info* mi, const char* master_info_fname, bool abort_if_no_master_info_file, int thread_mask); void end_master_info(Master_info* mi); -int flush_master_info(Master_info* mi, bool flush_relay_log_cache); +int flush_master_info(Master_info* mi, + bool flush_relay_log_cache, + bool need_lock_relay_log); int change_master_server_id_cmp(ulong *id1, ulong *id2); #endif /* HAVE_REPLICATION */ diff --git a/sql/rpl_rli.cc b/sql/rpl_rli.cc index 4bbafa0253a..fa979fe9a21 100644 --- a/sql/rpl_rli.cc +++ b/sql/rpl_rli.cc @@ -121,7 +121,7 @@ int init_relay_log_info(Relay_log_info* rli, /* The relay log will now be opened, as a SEQ_READ_APPEND IO_CACHE. Note that the I/O thread flushes it to disk after writing every - event, in flush_master_info(mi, 1). + event, in flush_master_info(mi, 1, ?). */ /* diff --git a/sql/slave.cc b/sql/slave.cc index 27f87d18800..3678c2497de 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -1726,7 +1726,7 @@ static void write_ignored_events_info_to_relay_log(THD *thd, Master_info *mi) " to the relay log, SHOW SLAVE STATUS may be" " inaccurate"); rli->relay_log.harvest_bytes_written(&rli->log_space_total); - if (flush_master_info(mi, 1)) + if (flush_master_info(mi, TRUE, FALSE)) sql_print_error("Failed to flush master info file"); delete ev; } @@ -3047,7 +3047,7 @@ Stopping slave I/O thread due to out-of-memory error from master"); goto err; } - if (flush_master_info(mi, 1)) + if (flush_master_info(mi, TRUE, TRUE)) { sql_print_error("Failed to flush master info file"); goto err; diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index 4e5ce08ab5d..d28a336e5d2 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -1532,7 +1532,7 @@ bool change_master(THD* thd, Master_info* mi) Relay log's IO_CACHE may not be inited, if rli->inited==0 (server was never a slave before). */ - if (flush_master_info(mi, 0)) + if (flush_master_info(mi, FALSE, FALSE)) { my_error(ER_RELAY_LOG_INIT, MYF(0), "Failed to flush master info file"); ret= TRUE; From 6ad93ebbb675f9f15505ae89db454792b6846d22 Mon Sep 17 00:00:00 2001 From: He Zhenxing Date: Thu, 4 Feb 2010 12:14:32 +0800 Subject: [PATCH 06/16] Bug#49894 shifted MYSQL_REPLICATION_PLUGIN number The number for MYSQL_REPLICATION_PLUGIN was shifted when backporting because MYSQL_AUDIT_PLUGIN was not backported. This problem is fixed by backporting only the number of audit plugin and print an error when trying to load audit plugins. Note that replication plugins compiled against old MYSQL_REPLICATION_PLUGIN number will also be recognized as audit plugin and be rejected. include/mysql/plugin.h: backporting the number of audit plugin (MYSQL_AUDIT_PLUGIN) sql/sql_plugin.cc: backporting the number of audit plugin (MYSQL_AUDIT_PLUGIN) print an error when trying to load audit plugins --- include/mysql/plugin.h | 5 +++-- sql/sql_plugin.cc | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/include/mysql/plugin.h b/include/mysql/plugin.h index 45d0234cb67..98b1cec0641 100644 --- a/include/mysql/plugin.h +++ b/include/mysql/plugin.h @@ -80,8 +80,9 @@ typedef struct st_mysql_xid MYSQL_XID; #define MYSQL_FTPARSER_PLUGIN 2 /* Full-text parser plugin */ #define MYSQL_DAEMON_PLUGIN 3 /* The daemon/raw plugin type */ #define MYSQL_INFORMATION_SCHEMA_PLUGIN 4 /* The I_S plugin type */ -#define MYSQL_REPLICATION_PLUGIN 5 /* The replication plugin type */ -#define MYSQL_MAX_PLUGIN_TYPE_NUM 6 /* The number of plugin types */ +#define MYSQL_AUDIT_PLUGIN 5 /* The Audit plugin type */ +#define MYSQL_REPLICATION_PLUGIN 6 /* The replication plugin type */ +#define MYSQL_MAX_PLUGIN_TYPE_NUM 7 /* The number of plugin types */ /* We use the following strings to define licenses for plugins */ #define PLUGIN_LICENSE_PROPRIETARY 0 diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index 8cf8c4cb81f..c977a30b37b 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -47,6 +47,7 @@ const LEX_STRING plugin_type_names[MYSQL_MAX_PLUGIN_TYPE_NUM]= { C_STRING_WITH_LEN("FTPARSER") }, { C_STRING_WITH_LEN("DAEMON") }, { C_STRING_WITH_LEN("INFORMATION SCHEMA") }, + { C_STRING_WITH_LEN("AUDIT") }, { C_STRING_WITH_LEN("REPLICATION") }, }; @@ -87,6 +88,7 @@ static int min_plugin_info_interface_version[MYSQL_MAX_PLUGIN_TYPE_NUM]= MYSQL_FTPARSER_INTERFACE_VERSION, MYSQL_DAEMON_INTERFACE_VERSION, MYSQL_INFORMATION_SCHEMA_INTERFACE_VERSION, + 0x0000, /* place holder for audit plugin */ MYSQL_REPLICATION_INTERFACE_VERSION, }; static int cur_plugin_info_interface_version[MYSQL_MAX_PLUGIN_TYPE_NUM]= @@ -96,6 +98,7 @@ static int cur_plugin_info_interface_version[MYSQL_MAX_PLUGIN_TYPE_NUM]= MYSQL_FTPARSER_INTERFACE_VERSION, MYSQL_DAEMON_INTERFACE_VERSION, MYSQL_INFORMATION_SCHEMA_INTERFACE_VERSION, + 0x0000, /* place holder for audit plugin */ MYSQL_REPLICATION_INTERFACE_VERSION, }; @@ -738,6 +741,14 @@ static bool plugin_add(MEM_ROOT *tmp_root, name_len)) { struct st_plugin_int *tmp_plugin_ptr; + + if (plugin->type == MYSQL_AUDIT_PLUGIN) + { + /* Bug#49894 */ + sql_print_error("Plugin type 'AUDIT' not supported by this server."); + goto err; + } + if (*(int*)plugin->info < min_plugin_info_interface_version[plugin->type] || ((*(int*)plugin->info) >> 8) > From 25a4b33340fbdf5e4f2431649e411e741c690543 Mon Sep 17 00:00:00 2001 From: Bjorn Munch Date: Thu, 4 Feb 2010 10:55:17 +0100 Subject: [PATCH 07/16] Bug #39774 mysql-test-run's remove_file can't use wildcards, this should be documented Added remove_files_wildcard that allows to remove multiple files at once. This is a port of original patch to Windows. --- client/mysqltest.cc | 79 ++++++++++++++++++++++++++++++++++- mysql-test/r/mysqltest.result | 3 ++ mysql-test/t/mysqltest.test | 24 +++++++++-- 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index 12aff05eff5..d2ee5855d26 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -287,7 +287,7 @@ enum enum_commands { Q_SEND_QUIT, Q_CHANGE_USER, Q_MKDIR, Q_RMDIR, Q_LIST_FILES, Q_LIST_FILES_WRITE_FILE, Q_LIST_FILES_APPEND_FILE, Q_SEND_SHUTDOWN, Q_SHUTDOWN_SERVER, - Q_MOVE_FILE, Q_SEND_EVAL, + Q_MOVE_FILE, Q_REMOVE_FILES_WILDCARD, Q_SEND_EVAL, Q_UNKNOWN, /* Unknown command. */ Q_COMMENT, /* Comments, ignored. */ @@ -386,6 +386,7 @@ const char *command_names[]= "send_shutdown", "shutdown_server", "move_file", + "remove_files_wildcard", "send_eval", 0 @@ -2889,6 +2890,81 @@ void do_remove_file(struct st_command *command) } +/* + SYNOPSIS + do_remove_files_wildcard + command called command + + DESCRIPTION + remove_files_wildcard [] + Remove the files in optionally matching +*/ + +void do_remove_files_wildcard(struct st_command *command) +{ + int error= 0; + uint i; + MY_DIR *dir_info; + FILEINFO *file; + char dir_separator[2]; + static DYNAMIC_STRING ds_directory; + static DYNAMIC_STRING ds_wild; + static DYNAMIC_STRING ds_file_to_remove; + char dirname[FN_REFLEN]; + + const struct command_arg rm_args[] = { + { "directory", ARG_STRING, TRUE, &ds_directory, + "Directory containing files to delete" }, + { "filename", ARG_STRING, FALSE, &ds_wild, "File pattern to delete" } + }; + DBUG_ENTER("do_remove_files_wildcard"); + + check_command_args(command, command->first_argument, + rm_args, sizeof(rm_args)/sizeof(struct command_arg), + ' '); + fn_format(dirname, ds_directory.str, "", "", MY_UNPACK_FILENAME); + + DBUG_PRINT("info", ("listing directory: %s", dirname)); + /* Note that my_dir sorts the list if not given any flags */ + if (!(dir_info= my_dir(dirname, MYF(MY_DONT_SORT | MY_WANT_STAT)))) + { + error= 1; + goto end; + } + init_dynamic_string(&ds_file_to_remove, dirname, 1024, 1024); + dir_separator[0]= FN_LIBCHAR; + dir_separator[1]= 0; + dynstr_append(&ds_file_to_remove, dir_separator); + for (i= 0; i < (uint) dir_info->number_off_files; i++) + { + file= dir_info->dir_entry + i; + /* Remove only regular files, i.e. no directories etc. */ + /* if (!MY_S_ISREG(file->mystat->st_mode)) */ + /* MY_S_ISREG does not work here on Windows, just skip directories */ + if (MY_S_ISDIR(file->mystat->st_mode)) + continue; + if (ds_wild.length && + wild_compare(file->name, ds_wild.str, 0)) + continue; + ds_file_to_remove.length= ds_directory.length + 1; + ds_file_to_remove.str[ds_directory.length + 1]= 0; + dynstr_append(&ds_file_to_remove, file->name); + DBUG_PRINT("info", ("removing file: %s", ds_file_to_remove.str)); + error= my_delete(ds_file_to_remove.str, MYF(0)) != 0; + if (error) + break; + } + my_dirend(dir_info); + +end: + handle_command_error(command, error); + dynstr_free(&ds_directory); + dynstr_free(&ds_wild); + dynstr_free(&ds_file_to_remove); + DBUG_VOID_RETURN; +} + + /* SYNOPSIS do_copy_file @@ -7881,6 +7957,7 @@ int main(int argc, char **argv) case Q_ECHO: do_echo(command); command_executed++; break; case Q_SYSTEM: do_system(command); break; case Q_REMOVE_FILE: do_remove_file(command); break; + case Q_REMOVE_FILES_WILDCARD: do_remove_files_wildcard(command); break; case Q_MKDIR: do_mkdir(command); break; case Q_RMDIR: do_rmdir(command); break; case Q_LIST_FILES: do_list_files(command); break; diff --git a/mysql-test/r/mysqltest.result b/mysql-test/r/mysqltest.result index 7d2100c627a..79341b30f7a 100644 --- a/mysql-test/r/mysqltest.result +++ b/mysql-test/r/mysqltest.result @@ -557,6 +557,7 @@ mysqltest: At line 2: Cannot run query on connection between send and reap select * from t1;; drop table t1; mysqltest: At line 1: Missing required argument 'filename' to command 'remove_file' +mysqltest: At line 1: Missing required argument 'directory' to command 'remove_files_wildcard' mysqltest: At line 1: Missing required argument 'filename' to command 'write_file' mysqltest: At line 1: End of file encountered before 'EOF' delimiter was found Content for test_file1 @@ -784,6 +785,8 @@ mysqltest: At line 1: change user failed: Access denied for user 'root'@'localho file1.txt file1.txt file2.txt +file11.txt +dir-list.txt SELECT 'c:\\a.txt' AS col; col z diff --git a/mysql-test/t/mysqltest.test b/mysql-test/t/mysqltest.test index 748f31bb5a3..20b35d41515 100644 --- a/mysql-test/t/mysqltest.test +++ b/mysql-test/t/mysqltest.test @@ -1717,6 +1717,19 @@ drop table t1; --error 1 remove_file non_existing_file; +# ---------------------------------------------------------------------------- +# test for remove_files_wildcard +# ---------------------------------------------------------------------------- + +--error 1 +--exec echo "remove_files_wildcard ;" | $MYSQL_TEST 2>&1 + +--error 1 +remove_files_wildcard non_existing_dir; + +--error 1 +remove_files_wildcard non_existing_dir non_existing_file; + # ---------------------------------------------------------------------------- # test for write_file # ---------------------------------------------------------------------------- @@ -2384,9 +2397,14 @@ rmdir $MYSQLTEST_VARDIR/tmp/testdir; cat_file $MYSQLTEST_VARDIR/tmp/testdir/file3.txt; -remove_file $MYSQLTEST_VARDIR/tmp/testdir/file1.txt; -remove_file $MYSQLTEST_VARDIR/tmp/testdir/file2.txt; -remove_file $MYSQLTEST_VARDIR/tmp/testdir/file3.txt; +list_files_write_file $MYSQLTEST_VARDIR/tmp/testdir/file11.txt $MYSQLTEST_VARDIR/tmp/testdir file?.txt; +remove_files_wildcard $MYSQLTEST_VARDIR/tmp/testdir file?.txt; +list_files_write_file $MYSQLTEST_VARDIR/tmp/testdir/dir-list.txt $MYSQLTEST_VARDIR/tmp/testdir file*.txt; +cat_file $MYSQLTEST_VARDIR/tmp/testdir/dir-list.txt; +remove_files_wildcard $MYSQLTEST_VARDIR/tmp/testdir file*.txt; +list_files $MYSQLTEST_VARDIR/tmp/testdir; +remove_files_wildcard $MYSQLTEST_VARDIR/tmp/testdir; +list_files $MYSQLTEST_VARDIR/tmp/testdir; rmdir $MYSQLTEST_VARDIR/tmp/testdir; # From dca6700620d5f4b325c3aadbbf5ab30c5b888a8c Mon Sep 17 00:00:00 2001 From: Mattias Jonsson Date: Wed, 10 Feb 2010 10:47:14 +0100 Subject: [PATCH 08/16] Bug#50201: Server crashes in explain_filename on an InnoDB partitioned table Problem was that in mysql-trunk the ER() macro is now dependent on current_thd and the innodb monitor thread has no binding to that thd object. This cause the crash because of bad derefencing. Solution was to add a new macro which take the thd as an argument (which the innodb thread uses for the call). (Updated according to reviewers comments, i.e. added ER_THD_OR_DEFAULT and moved test to suite parts.) mysql-test/suite/parts/r/partition_innodb_status_file.result: Bug#50201: Server crashes in explain_filename on an InnoDB partitioned table New test result file mysql-test/suite/parts/t/partition_innodb_status_file-master.opt: Bug#50201: Server crashes in explain_filename on an InnoDB partitioned table New test opt file mysql-test/suite/parts/t/partition_innodb_status_file.test: Bug#50201: Server crashes in explain_filename on an InnoDB partitioned table New test. Note that the innodb monitor thread only runs every 15 seconds, so this test will take at least 15 seconds, so I have moved it to the parts suite. sql/sql_table.cc: Bug#50201: Server crashes in explain_filename on an InnoDB partitioned table Using thd safe ER macro. sql/unireg.h: Bug#50201: Server crashes in explain_filename on an InnoDB partitioned table Added ER macros for use with specified thd pointer. --- .../r/partition_innodb_status_file.result | 14 +++++++++++++ .../t/partition_innodb_status_file-master.opt | 1 + .../parts/t/partition_innodb_status_file.test | 20 +++++++++++++++++++ sql/sql_table.cc | 17 ++++++++++------ sql/unireg.h | 3 +++ 5 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 mysql-test/suite/parts/r/partition_innodb_status_file.result create mode 100644 mysql-test/suite/parts/t/partition_innodb_status_file-master.opt create mode 100644 mysql-test/suite/parts/t/partition_innodb_status_file.test diff --git a/mysql-test/suite/parts/r/partition_innodb_status_file.result b/mysql-test/suite/parts/r/partition_innodb_status_file.result new file mode 100644 index 00000000000..29b5a3b3766 --- /dev/null +++ b/mysql-test/suite/parts/r/partition_innodb_status_file.result @@ -0,0 +1,14 @@ +CREATE TABLE t1 (a INT) ENGINE = InnoDB PARTITION BY HASH(a); +INSERT INTO t1 VALUES (0), (1), (2); +START TRANSACTION; +UPDATE t1 SET a = 5 WHERE a = 1; +# Connection con1 +# InnoDB lock timeout and monitor thread runs every 15 seconds +SET innodb_lock_wait_timeout = 20; +START TRANSACTION; +UPDATE t1 SET a = 3 WHERE a = 1; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +COMMIT; +# Connection default +COMMIT; +DROP TABLE t1; diff --git a/mysql-test/suite/parts/t/partition_innodb_status_file-master.opt b/mysql-test/suite/parts/t/partition_innodb_status_file-master.opt new file mode 100644 index 00000000000..779962e8fca --- /dev/null +++ b/mysql-test/suite/parts/t/partition_innodb_status_file-master.opt @@ -0,0 +1 @@ +--innodb-status-file=1 diff --git a/mysql-test/suite/parts/t/partition_innodb_status_file.test b/mysql-test/suite/parts/t/partition_innodb_status_file.test new file mode 100644 index 00000000000..f066ce5d485 --- /dev/null +++ b/mysql-test/suite/parts/t/partition_innodb_status_file.test @@ -0,0 +1,20 @@ +--source include/have_innodb.inc +--source include/have_partition.inc + +CREATE TABLE t1 (a INT) ENGINE = InnoDB PARTITION BY HASH(a); +INSERT INTO t1 VALUES (0), (1), (2); +START TRANSACTION; +UPDATE t1 SET a = 5 WHERE a = 1; +connect (con1, localhost, root,,); +--echo # Connection con1 +--echo # InnoDB lock timeout and monitor thread runs every 15 seconds +SET innodb_lock_wait_timeout = 20; +START TRANSACTION; +--error ER_LOCK_WAIT_TIMEOUT +UPDATE t1 SET a = 3 WHERE a = 1; +COMMIT; +disconnect con1; +connection default; +--echo # Connection default +COMMIT; +DROP TABLE t1; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index b17edbdd234..6006c818725 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -291,7 +291,8 @@ uint explain_filename(THD* thd, { if (explain_mode == EXPLAIN_ALL_VERBOSE) { - to_p= strnmov(to_p, ER(ER_DATABASE_NAME), end_p - to_p); + to_p= strnmov(to_p, ER_THD_OR_DEFAULT(thd, ER_DATABASE_NAME), + end_p - to_p); *(to_p++)= ' '; to_p= add_identifier(thd, to_p, end_p, db_name, db_name_len); to_p= strnmov(to_p, ", ", end_p - to_p); @@ -304,7 +305,7 @@ uint explain_filename(THD* thd, } if (explain_mode == EXPLAIN_ALL_VERBOSE) { - to_p= strnmov(to_p, ER(ER_TABLE_NAME), end_p - to_p); + to_p= strnmov(to_p, ER_THD_OR_DEFAULT(thd, ER_TABLE_NAME), end_p - to_p); *(to_p++)= ' '; to_p= add_identifier(thd, to_p, end_p, table_name, table_name_len); } @@ -321,18 +322,22 @@ uint explain_filename(THD* thd, if (name_type != NORMAL) { if (name_type == TEMP) - to_p= strnmov(to_p, ER(ER_TEMPORARY_NAME), end_p - to_p); + to_p= strnmov(to_p, ER_THD_OR_DEFAULT(thd, ER_TEMPORARY_NAME), + end_p - to_p); else - to_p= strnmov(to_p, ER(ER_RENAMED_NAME), end_p - to_p); + to_p= strnmov(to_p, ER_THD_OR_DEFAULT(thd, ER_RENAMED_NAME), + end_p - to_p); to_p= strnmov(to_p, " ", end_p - to_p); } - to_p= strnmov(to_p, ER(ER_PARTITION_NAME), end_p - to_p); + to_p= strnmov(to_p, ER_THD_OR_DEFAULT(thd, ER_PARTITION_NAME), + end_p - to_p); *(to_p++)= ' '; to_p= add_identifier(thd, to_p, end_p, part_name, part_name_len); if (subpart_name) { to_p= strnmov(to_p, ", ", end_p - to_p); - to_p= strnmov(to_p, ER(ER_SUBPARTITION_NAME), end_p - to_p); + to_p= strnmov(to_p, ER_THD_OR_DEFAULT(thd, ER_SUBPARTITION_NAME), + end_p - to_p); *(to_p++)= ' '; to_p= add_identifier(thd, to_p, end_p, subpart_name, subpart_name_len); } diff --git a/sql/unireg.h b/sql/unireg.h index a390b755772..e915b234a6b 100644 --- a/sql/unireg.h +++ b/sql/unireg.h @@ -46,6 +46,9 @@ #define ER(X) CURRENT_THD_ERRMSGS[(X) - ER_ERROR_FIRST] #define ER_DEFAULT(X) DEFAULT_ERRMSGS[(X) - ER_ERROR_FIRST] #define ER_SAFE(X) (((X) >= ER_ERROR_FIRST && (X) <= ER_ERROR_LAST) ? ER(X) : "Invalid error code") +#define ER_THD(thd,X) ((thd)->variables.lc_messages->errmsgs->errmsgs[(X) - \ + ER_ERROR_FIRST]) +#define ER_THD_OR_DEFAULT(thd,X) ((thd) ? ER_THD(thd, X) : ER_DEFAULT(X)) #define ERRMAPP 1 /* Errormap f|r my_error */ From 5df69267c25d093fb1a87b3e9952812a66f0f687 Mon Sep 17 00:00:00 2001 From: Luis Soares Date: Wed, 10 Feb 2010 12:12:55 +0000 Subject: [PATCH 09/16] BUG#50984: check_testcase fails for rpl_tmp_table_and_DDL We found that there are some tests that are not cleaning up properly: 1. rpl_tmp_table_and_DDL 2. rpl_do_grant 3. rpl_sync For #1 and #2 we found that the slave would not, for some cases, replicate all the instructions the master processed in the cleanup section. We fix these by deploying some synchronization commands in the test cases so that slave processes all clean up instructions. As for #3, this is tracked as part of another bug (BUG@50442). --- mysql-test/suite/rpl/r/rpl_do_grant.result | 10 +++++++--- mysql-test/suite/rpl/t/rpl_do_grant.test | 18 ++++++++++++++---- .../suite/rpl/t/rpl_tmp_table_and_DDL.test | 1 + 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_do_grant.result b/mysql-test/suite/rpl/r/rpl_do_grant.result index 65c60acc651..9eecc1bab3f 100644 --- a/mysql-test/suite/rpl/r/rpl_do_grant.result +++ b/mysql-test/suite/rpl/r/rpl_do_grant.result @@ -89,6 +89,7 @@ show grants for rpl_do_grant2@localhost; ERROR 42000: There is no such grant defined for user 'rpl_do_grant2' on host 'localhost' show grants for rpl_do_grant2@localhost; ERROR 42000: There is no such grant defined for user 'rpl_do_grant2' on host 'localhost' +call mtr.add_suppression("Slave: Operation DROP USER failed for 'create_rout_db'@'localhost' Error_code: 1396"); DROP DATABASE IF EXISTS bug42217_db; CREATE DATABASE bug42217_db; GRANT CREATE ROUTINE ON bug42217_db.* TO 'create_rout_db'@'localhost' @@ -166,9 +167,12 @@ DROP FUNCTION upgrade_del_func; DROP FUNCTION upgrade_alter_func; DROP DATABASE bug42217_db; DROP USER 'create_rout_db'@'localhost'; -call mtr.add_suppression("Slave: Operation DROP USER failed for 'create_rout_db'@'localhost' Error_code: 1396"); -USE mtr; -call mtr.add_suppression("Slave: Operation DROP USER failed for 'create_rout_db'@'localhost' Error_code: 1396"); +stop slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +reset master; +reset slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +start slave; ######## BUG#49119 ####### ### i) test case from the 'how to repeat section' stop slave; diff --git a/mysql-test/suite/rpl/t/rpl_do_grant.test b/mysql-test/suite/rpl/t/rpl_do_grant.test index d6a06f43d18..df61b6847c9 100644 --- a/mysql-test/suite/rpl/t/rpl_do_grant.test +++ b/mysql-test/suite/rpl/t/rpl_do_grant.test @@ -120,6 +120,9 @@ show grants for rpl_do_grant2@localhost; # BUG42217 mysql.procs_priv does not get replicated ##################################################### connection master; +call mtr.add_suppression("Slave: Operation DROP USER failed for 'create_rout_db'@'localhost' Error_code: 1396"); +sync_slave_with_master; +connection master; --disable_warnings DROP DATABASE IF EXISTS bug42217_db; @@ -209,12 +212,19 @@ USE bug42217_db; DROP FUNCTION upgrade_del_func; DROP FUNCTION upgrade_alter_func; DROP DATABASE bug42217_db; +-- sync_slave_with_master +-- connection master + +# user was already dropped in the slave before +# so no need to wait for the slave to replicate +# this statement (if it did and we later synced +# the slave it would end up in an error anyway) DROP USER 'create_rout_db'@'localhost'; -call mtr.add_suppression("Slave: Operation DROP USER failed for 'create_rout_db'@'localhost' Error_code: 1396"); -connection slave; -USE mtr; -call mtr.add_suppression("Slave: Operation DROP USER failed for 'create_rout_db'@'localhost' Error_code: 1396"); +# finish entire clean up (remove binlogs) +# so that we leave a pristine environment for the +# following tests +-- source include/master-slave-reset.inc # BUG#49119: Master crashes when executing 'REVOKE ... ON # {PROCEDURE|FUNCTION} FROM ...' diff --git a/mysql-test/suite/rpl/t/rpl_tmp_table_and_DDL.test b/mysql-test/suite/rpl/t/rpl_tmp_table_and_DDL.test index 56924a2efe9..b3efb578b68 100644 --- a/mysql-test/suite/rpl/t/rpl_tmp_table_and_DDL.test +++ b/mysql-test/suite/rpl/t/rpl_tmp_table_and_DDL.test @@ -10,4 +10,5 @@ source include/have_binlog_format_row.inc; LET $ENGINE_TYPE= MyISAM; source extra/rpl_tests/rpl_tmp_table_and_DDL.test; +sync_slave_with_master; From 93cd02bc82bfe48bad5c3d4daf3331207f3445a3 Mon Sep 17 00:00:00 2001 From: Magne Mahre Date: Thu, 11 Feb 2010 18:02:41 +0100 Subject: [PATCH 10/16] Bug#50542 5.5.x doesn't check length of key prefixes: corruption and crash results An index creation statement where the index key is larger/wider than the column it references should throw an error. A statement like: CREATE TABLE t1 (a CHAR(1), PRIMARY KEY (A(255))) did not error, but a segmentation fault followed when an insertion was attempted on the table The partial key validiation clause has been restructured to (hopefully) better document which uses of partial keys are valid. --- mysql-test/r/alter_table.result | 12 ++++++++++++ mysql-test/t/alter_table.test | 28 ++++++++++++++++++++++++++++ sql/sql_table.cc | 29 ++++++++++++++--------------- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/mysql-test/r/alter_table.result b/mysql-test/r/alter_table.result index db80e9d2eea..7e14c3a9b23 100644 --- a/mysql-test/r/alter_table.result +++ b/mysql-test/r/alter_table.result @@ -1354,3 +1354,15 @@ DROP i, ADD i INT UNSIGNED NOT NULL AUTO_INCREMENT, AUTO_INCREMENT = 1; DROP TABLE t1; +CREATE TABLE t1 (a CHAR(1), PRIMARY KEY (a(255))); +ERROR HY000: Incorrect prefix key; the used key part isn't a string, the used length is longer than the key part, or the storage engine doesn't support unique prefix keys +CREATE TABLE t1 (a CHAR(1)); +ALTER TABLE t1 ADD PRIMARY KEY (a(20)); +ERROR HY000: Incorrect prefix key; the used key part isn't a string, the used length is longer than the key part, or the storage engine doesn't support unique prefix keys +ALTER TABLE t1 ADD KEY (a(20)); +ERROR HY000: Incorrect prefix key; the used key part isn't a string, the used length is longer than the key part, or the storage engine doesn't support unique prefix keys +CREATE UNIQUE INDEX i1 ON t1 (a(20)); +ERROR HY000: Incorrect prefix key; the used key part isn't a string, the used length is longer than the key part, or the storage engine doesn't support unique prefix keys +CREATE INDEX i2 ON t1 (a(20)); +ERROR HY000: Incorrect prefix key; the used key part isn't a string, the used length is longer than the key part, or the storage engine doesn't support unique prefix keys +DROP TABLE t1; diff --git a/mysql-test/t/alter_table.test b/mysql-test/t/alter_table.test index 2bfe6dbaa62..54c662bccf2 100644 --- a/mysql-test/t/alter_table.test +++ b/mysql-test/t/alter_table.test @@ -1089,3 +1089,31 @@ ALTER TABLE t1 ADD i INT UNSIGNED NOT NULL AUTO_INCREMENT, AUTO_INCREMENT = 1; DROP TABLE t1; + + +# +# Bug#50542 5.5.x doesn't check length of key prefixes: +# corruption and crash results +# +# This case is related to Bug#31031 (above) +# A statement where the index key is larger/wider than +# the column type, should cause an error +# +--error ER_WRONG_SUB_KEY +CREATE TABLE t1 (a CHAR(1), PRIMARY KEY (a(255))); + +# Test other variants of creating indices +CREATE TABLE t1 (a CHAR(1)); +# ALTER TABLE +--error ER_WRONG_SUB_KEY +ALTER TABLE t1 ADD PRIMARY KEY (a(20)); +--error ER_WRONG_SUB_KEY +ALTER TABLE t1 ADD KEY (a(20)); +# CREATE INDEX +--error ER_WRONG_SUB_KEY +CREATE UNIQUE INDEX i1 ON t1 (a(20)); +--error ER_WRONG_SUB_KEY +CREATE INDEX i2 ON t1 (a(20)); +# cleanup +DROP TABLE t1; + diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 6006c818725..04bd2d4c976 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3292,22 +3292,21 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, } } } + // Catch invalid use of partial keys else if (!f_is_geom(sql_field->pack_flag) && - ((column->length > length && - !Field::type_can_have_key_part (sql_field->sql_type)) || - ((f_is_packed(sql_field->pack_flag) || - ((file->ha_table_flags() & HA_NO_PREFIX_CHAR_KEYS) && - (key_info->flags & HA_NOSAME))) && - column->length != length))) - { - /* Catch invalid uses of partial keys. - A key is identified as 'partial' if column->length != length. - A partial key is invalid if they data type does - not allow it, or the field is packed (as in MyISAM), - or the storage engine doesn't allow prefixed search and - the key is primary key. - */ - + // is the key partial? + column->length != length && + // is prefix length bigger than field length? + (column->length > length || + // can the field have a partial key? + !Field::type_can_have_key_part (sql_field->sql_type) || + // a packed field can't be used in a partial key + f_is_packed(sql_field->pack_flag) || + // does the storage engine allow prefixed search? + ((file->ha_table_flags() & HA_NO_PREFIX_CHAR_KEYS) && + // and is this a 'unique' key? + (key_info->flags & HA_NOSAME)))) + { my_message(ER_WRONG_SUB_KEY, ER(ER_WRONG_SUB_KEY), MYF(0)); DBUG_RETURN(TRUE); } From 203793514d4e5035691bb750256532b8ecf0b6da Mon Sep 17 00:00:00 2001 From: Magne Mahre Date: Thu, 11 Feb 2010 18:25:34 +0100 Subject: [PATCH 11/16] Bug#50574 5.5.x allows spatial indexes on non-spatial columns, causing crashes! Adding a SPATIAL INDEX on a non-geometrical column caused a segmentation fault when the table was subsequently inserted into. A test was added in mysql_prepare_create_table to explicitly check whether non-geometrical columns are used in a spatial index, and throw an error if so. mysql-test/t/gis.test: Added test cases to verify that only geometrical columns can get a spatial index. In addition, verify that only a single geom. column can participate in a spatial index. --- mysql-test/r/gis.result | 30 ++++++++++++++++++++++++++ mysql-test/t/gis.test | 45 +++++++++++++++++++++++++++++++++++++++ sql/share/errmsg-utf8.txt | 2 ++ sql/sql_table.cc | 16 ++++++++++---- 4 files changed, 89 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/gis.result b/mysql-test/r/gis.result index 3e28227d542..ac808daae92 100644 --- a/mysql-test/r/gis.result +++ b/mysql-test/r/gis.result @@ -1058,3 +1058,33 @@ SELECT Polygon(12345123,''); Polygon(12345123,'') NULL End of 5.1 tests +CREATE TABLE t1( +col0 BINARY NOT NULL, +col2 TIMESTAMP, +SPATIAL INDEX i1 (col0) +) ENGINE=MyISAM; +ERROR 42000: A SPATIAL index may only contain a geometrical type column +CREATE TABLE t1 ( +col0 BINARY NOT NULL, +col2 TIMESTAMP +) ENGINE=MyISAM; +CREATE SPATIAL INDEX idx0 ON t1(col0); +ERROR 42000: A SPATIAL index may only contain a geometrical type column +ALTER TABLE t1 ADD SPATIAL INDEX i1 (col0); +ERROR 42000: A SPATIAL index may only contain a geometrical type column +CREATE TABLE t2 ( +col0 INTEGER NOT NULL, +col1 POINT, +col2 POINT +); +CREATE SPATIAL INDEX idx0 ON t2 (col1, col2); +ERROR HY000: Incorrect arguments to SPATIAL INDEX +CREATE TABLE t3 ( +col0 INTEGER NOT NULL, +col1 POINT, +col2 LINESTRING, +SPATIAL INDEX i1 (col1, col2) +); +ERROR HY000: Incorrect arguments to SPATIAL INDEX +DROP TABLE t1; +DROP TABLE t2; diff --git a/mysql-test/t/gis.test b/mysql-test/t/gis.test index bc0695aaa93..c5c6a94057a 100644 --- a/mysql-test/t/gis.test +++ b/mysql-test/t/gis.test @@ -723,3 +723,48 @@ SELECT Polygon(1234512,''); SELECT Polygon(12345123,''); --echo End of 5.1 tests + +# +# Bug #50574 5.5.x allows spatial indexes on non-spatial +# columns, causing crashes! +# +--error ER_SPATIAL_MUST_HAVE_GEOM_COL +CREATE TABLE t1( + col0 BINARY NOT NULL, + col2 TIMESTAMP, + SPATIAL INDEX i1 (col0) +) ENGINE=MyISAM; + +# Test other ways to add indices +CREATE TABLE t1 ( + col0 BINARY NOT NULL, + col2 TIMESTAMP +) ENGINE=MyISAM; + +--error ER_SPATIAL_MUST_HAVE_GEOM_COL +CREATE SPATIAL INDEX idx0 ON t1(col0); + +--error ER_SPATIAL_MUST_HAVE_GEOM_COL +ALTER TABLE t1 ADD SPATIAL INDEX i1 (col0); + +CREATE TABLE t2 ( + col0 INTEGER NOT NULL, + col1 POINT, + col2 POINT +); + +--error ER_WRONG_ARGUMENTS +CREATE SPATIAL INDEX idx0 ON t2 (col1, col2); + +--error ER_WRONG_ARGUMENTS +CREATE TABLE t3 ( + col0 INTEGER NOT NULL, + col1 POINT, + col2 LINESTRING, + SPATIAL INDEX i1 (col1, col2) +); + +# cleanup +DROP TABLE t1; +DROP TABLE t2; + diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index db99890235a..d4f3cce805f 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -6260,3 +6260,5 @@ ER_FIELD_TYPE_NOT_ALLOWED_AS_PARTITION_FIELD eng "Field '%-.192s' is of a not allowed type for this type of partitioning" ER_PARTITION_FIELDS_TOO_LONG eng "The total length of the partitioning fields is too large" +ER_SPATIAL_MUST_HAVE_GEOM_COL 42000 + eng "A SPATIAL index may only contain a geometrical type column" diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 04bd2d4c976..e89edb4fcfe 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3193,11 +3193,19 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, { column->length*= sql_field->charset->mbmaxlen; - if (key->type == Key::SPATIAL && column->length) + if (key->type == Key::SPATIAL) { - my_error(ER_WRONG_SUB_KEY, MYF(0)); - DBUG_RETURN(TRUE); - } + if (column->length) + { + my_error(ER_WRONG_SUB_KEY, MYF(0)); + DBUG_RETURN(TRUE); + } + if (!f_is_geom(sql_field->pack_flag)) + { + my_error(ER_SPATIAL_MUST_HAVE_GEOM_COL, MYF(0)); + DBUG_RETURN(TRUE); + } + } if (f_is_blob(sql_field->pack_flag) || (f_is_geom(sql_field->pack_flag) && key->type != Key::SPATIAL)) From 97afccae53a5fda90311f57d328924d736279414 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 12 Feb 2010 12:04:57 +0800 Subject: [PATCH 12/16] Bug #43913 rpl_cross_version can't pass on conflicts complainig clash with --slave-load-tm The MDL_SHARED lock was introduced for an object in 5.4, but the 'TABLE_LIST' object was not initialized with the MDL_SHARED lock when applying event with LOAD DATA INFILE into table. So the failure is caused when checking the MDL_SHARED lock for the object. To fix the problem, the 'TABLE_LIST' object was initialized with the MDL_SHARED lock when applying event with LOAD DATA INFILE into table. mysql-test/suite/rpl/t/disabled.def: Got rid of the line for enabling 'rpl_cross_version' test. --- mysql-test/suite/rpl/t/disabled.def | 1 - mysql-test/suite/rpl/t/rpl_cross_version-master.opt | 2 +- sql/log_event.cc | 5 +---- 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/mysql-test/suite/rpl/t/disabled.def b/mysql-test/suite/rpl/t/disabled.def index 446c233c8a9..a0c57b205ec 100644 --- a/mysql-test/suite/rpl/t/disabled.def +++ b/mysql-test/suite/rpl/t/disabled.def @@ -12,5 +12,4 @@ rpl_get_master_version_and_clock: # Bug#46931 2009-10-17 joro rpl.rpl_get_master_version_and_clock fails rpl_row_create_table : Bug#45576 2009-12-01 joro rpl_row_create_table fails on PB2 -rpl_cross_version : BUG#43913 2009-10-22 luis rpl_cross_version fails with symptom in described in bug report rpl_spec_variables : BUG#47661 2009-10-27 jasonh rpl_spec_variables fails on PB2 hpux diff --git a/mysql-test/suite/rpl/t/rpl_cross_version-master.opt b/mysql-test/suite/rpl/t/rpl_cross_version-master.opt index 0ea05290c11..815a8f81d32 100644 --- a/mysql-test/suite/rpl/t/rpl_cross_version-master.opt +++ b/mysql-test/suite/rpl/t/rpl_cross_version-master.opt @@ -1 +1 @@ ---replicate-same-server-id --relay-log=slave-relay-bin --secure-file-priv=$MYSQL_TMP_DIR +--replicate-same-server-id --relay-log=slave-relay-bin diff --git a/sql/log_event.cc b/sql/log_event.cc index b6c84dd95f4..fbfffb592bf 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -4634,10 +4634,7 @@ int Load_log_event::do_apply_event(NET* net, Relay_log_info const *rli, thd->warning_info->opt_clear_warning_info(thd->query_id); TABLE_LIST tables; - bzero((char*) &tables,sizeof(tables)); - tables.db= thd->strmake(thd->db, thd->db_length); - tables.alias = tables.table_name = (char*) table_name; - tables.lock_type = TL_WRITE; + tables.init_one_table(thd->db, table_name, TL_WRITE); tables.updating= 1; // the table will be opened in mysql_load From 3e0f70d2cc0d233f0be615532ed3a5254930ad17 Mon Sep 17 00:00:00 2001 From: Evgeny Potemkin Date: Fri, 12 Feb 2010 11:51:52 +0300 Subject: [PATCH 13/16] Bug#50539: Wrong result when loose index scan is used for an aggregate function with distinct. Loose index scan is used to find MIN/MAX values using appropriate index and thus allow to avoid grouping. For each found row it updates non-aggregated fields with values from row with found MIN/MAX value. Without loose index scan non-aggregated fields are copied by end_send_group function. With loose index scan there is no need in end_send_group and end_send is used instead. Non-aggregated fields still need to be copied and this was wrongly implemented in QUICK_GROUP_MIN_MAX_SELECT::get_next. WL#3220 added a case when loose index scan can be used with end_send_group to optimize calculation of aggregate functions with distinct. In this case the row found by QUICK_GROUP_MIN_MAX_SELECT::get_next might belong to a next group and copying it will produce wrong result. Update of non-aggregated fields is moved to the end_send function from QUICK_GROUP_MIN_MAX_SELECT::get_next. mysql-test/r/group_min_max.result: Added a test case for the bug#50539. mysql-test/t/group_min_max.test: Added a test case for the bug#50539. sql/opt_range.cc: Bug#50539: Wrong result when loose index scan is used for an aggregate function with distinct. Update of non-aggregated fields is moved to the end_send function from QUICK_GROUP_MIN_MAX_SELECT::get_next. sql/sql_select.cc: Bug#50539: Wrong result when loose index scan is used for an aggregate function with distinct. Update of non-aggregated fields is moved to the end_send function from QUICK_GROUP_MIN_MAX_SELECT::get_next. --- mysql-test/r/group_min_max.result | 27 ++++++++++++++++++++++++--- mysql-test/t/group_min_max.test | 19 +++++++++++++++++++ sql/opt_range.cc | 12 +----------- sql/sql_select.cc | 6 ++++++ 4 files changed, 50 insertions(+), 14 deletions(-) diff --git a/mysql-test/r/group_min_max.result b/mysql-test/r/group_min_max.result index ba1c2a79ad9..94069d0559c 100644 --- a/mysql-test/r/group_min_max.result +++ b/mysql-test/r/group_min_max.result @@ -2686,7 +2686,7 @@ a c COUNT(DISTINCT c, a, b) 1 1 1 1 1 1 1 1 1 -2 1 1 +1 1 1 2 1 1 2 1 1 2 1 1 @@ -2714,7 +2714,7 @@ id select_type table type possible_keys key key_len ref rows Extra 1 SIMPLE t2 range NULL a 10 NULL 9 Using index for group-by SELECT a, COUNT(DISTINCT b), SUM(DISTINCT b) FROM t2 GROUP BY a; a COUNT(DISTINCT b) SUM(DISTINCT b) -2 8 36 +1 8 36 2 8 36 EXPLAIN SELECT COUNT(DISTINCT b), SUM(DISTINCT b) FROM t2 GROUP BY a; id select_type table type possible_keys key key_len ref rows Extra @@ -2761,7 +2761,7 @@ SELECT 42 * (a + c + COUNT(DISTINCT c, a, b)) FROM t2 GROUP BY a, b, c; 126 126 126 -168 +126 168 168 168 @@ -2779,3 +2779,24 @@ SELECT (SUM(DISTINCT a) + MAX(b)) FROM t2 GROUP BY a; 10 DROP TABLE t1,t2; # end of WL#3220 tests +# +# Bug#50539: Wrong result when loose index scan is used for an aggregate +# function with distinct +# +CREATE TABLE t1 ( +f1 int(11) NOT NULL DEFAULT '0', +f2 char(1) NOT NULL DEFAULT '', +PRIMARY KEY (f1,f2) +) ; +insert into t1 values(1,'A'),(1 , 'B'), (1, 'C'), (2, 'A'), +(3, 'A'), (3, 'B'), (3, 'C'), (3, 'D'); +SELECT f1, COUNT(DISTINCT f2) FROM t1 GROUP BY f1; +f1 COUNT(DISTINCT f2) +1 3 +2 1 +3 4 +explain SELECT f1, COUNT(DISTINCT f2) FROM t1 GROUP BY f1; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 range NULL PRIMARY 5 NULL 9 Using index for group-by (scanning) +drop table t1; +# End of test#50539. diff --git a/mysql-test/t/group_min_max.test b/mysql-test/t/group_min_max.test index 0e6fef9b855..1e7f28d5916 100644 --- a/mysql-test/t/group_min_max.test +++ b/mysql-test/t/group_min_max.test @@ -1166,3 +1166,22 @@ SELECT (SUM(DISTINCT a) + MAX(b)) FROM t2 GROUP BY a; DROP TABLE t1,t2; --echo # end of WL#3220 tests + +--echo # +--echo # Bug#50539: Wrong result when loose index scan is used for an aggregate +--echo # function with distinct +--echo # +CREATE TABLE t1 ( + f1 int(11) NOT NULL DEFAULT '0', + f2 char(1) NOT NULL DEFAULT '', + PRIMARY KEY (f1,f2) +) ; +insert into t1 values(1,'A'),(1 , 'B'), (1, 'C'), (2, 'A'), +(3, 'A'), (3, 'B'), (3, 'C'), (3, 'D'); + +SELECT f1, COUNT(DISTINCT f2) FROM t1 GROUP BY f1; +explain SELECT f1, COUNT(DISTINCT f2) FROM t1 GROUP BY f1; + +drop table t1; +--echo # End of test#50539. + diff --git a/sql/opt_range.cc b/sql/opt_range.cc index ac5b1f575de..5c64a6a64ee 100644 --- a/sql/opt_range.cc +++ b/sql/opt_range.cc @@ -10959,17 +10959,7 @@ int QUICK_GROUP_MIN_MAX_SELECT::get_next() } while ((result == HA_ERR_KEY_NOT_FOUND || result == HA_ERR_END_OF_FILE) && is_last_prefix != 0); - if (result == 0) - { - /* - Partially mimic the behavior of end_select_send. Copy the - field data from Item_field::field into Item_field::result_field - of each non-aggregated field (the group fields, and optionally - other fields in non-ANSI SQL mode). - */ - copy_fields(&join->tmp_table_param); - } - else if (result == HA_ERR_KEY_NOT_FOUND) + if (result == HA_ERR_KEY_NOT_FOUND) result= HA_ERR_END_OF_FILE; DBUG_RETURN(result); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 7119650a7a6..e09bd6cab5d 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -12255,6 +12255,12 @@ end_send(JOIN *join, JOIN_TAB *join_tab __attribute__((unused)), if (!end_of_records) { int error; + if (join->tables && + join->join_tab->is_using_loose_index_scan()) + { + /* Copy non-aggregated fields when loose index scan is used. */ + copy_fields(&join->tmp_table_param); + } if (join->having && join->having->val_int() == 0) DBUG_RETURN(NESTED_LOOP_OK); // Didn't match having error=0; From c142ef4679b4a8f73e41fee644a982869a07464c Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 12 Feb 2010 20:17:53 +0100 Subject: [PATCH 14/16] Raise version number after cloning 5.5.2-m2 --- configure.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.in b/configure.in index e50f2897781..4f745886438 100644 --- a/configure.in +++ b/configure.in @@ -7,7 +7,7 @@ AC_PREREQ(2.59) # Remember to also update version.c in ndb. # When changing major version number please also check switch statement # in client/mysqlbinlog.cc:check_master_version(). -AC_INIT([MySQL Server], [5.5.2-m2], [], [mysql]) +AC_INIT([MySQL Server], [5.5.3-m2], [], [mysql]) AC_CONFIG_SRCDIR([sql/mysqld.cc]) AC_CANONICAL_SYSTEM # USTAR format gives us the possibility to store longer path names in From 1b9f84bf01ad421df06e0d229c0b2de4ea8983cd Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Sun, 14 Feb 2010 13:22:03 +0300 Subject: [PATCH 15/16] Fix tree name. --- .bzr-mysql/default.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bzr-mysql/default.conf b/.bzr-mysql/default.conf index 08ca040642e..f33ccdf6753 100644 --- a/.bzr-mysql/default.conf +++ b/.bzr-mysql/default.conf @@ -1,4 +1,4 @@ [MYSQL] post_commit_to = "commits@lists.mysql.com" post_push_to = "commits@lists.mysql.com" -tree_name = "mysql-5.5-trunk-bugfixing" +tree_name = "mysql-5.5-trunk" From 9f6662a0b444d2d2e918d9c09d8a27b97a560bbd Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Sat, 20 Feb 2010 12:26:22 +0300 Subject: [PATCH 16/16] Fix default.conf. --- .bzr-mysql/default.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.bzr-mysql/default.conf b/.bzr-mysql/default.conf index a81954c9bcd..fcb3cab2de6 100644 --- a/.bzr-mysql/default.conf +++ b/.bzr-mysql/default.conf @@ -1,4 +1,4 @@ [MYSQL] post_commit_to = "commits@lists.mysql.com" post_push_to = "commits@lists.mysql.com" -tree_name = "mysql-trunk-mtr" +tree_name = "mysql-trunk"