From f44d73a58d0d8232603d98ac14c08cc142538564 Mon Sep 17 00:00:00 2001 From: "msvensson@neptunus.(none)" <> Date: Tue, 16 Jan 2007 09:35:14 +0100 Subject: [PATCH 1/7] Always use two masters for ndb tests --- mysql-test/lib/mtr_cases.pl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/mysql-test/lib/mtr_cases.pl b/mysql-test/lib/mtr_cases.pl index a00d06d2e60..8c2dbdec804 100644 --- a/mysql-test/lib/mtr_cases.pl +++ b/mysql-test/lib/mtr_cases.pl @@ -537,6 +537,8 @@ sub collect_one_test_case($$$$$$$) { $tinfo->{'comment'}= "No ndbcluster tests(--skip-ndbcluster)"; return; } + # Ndb tests run with two mysqld masters + $tinfo->{'master_num'}= 2; } else { @@ -552,7 +554,7 @@ sub collect_one_test_case($$$$$$$) { if ( $tinfo->{'innodb_test'} ) { - # This is a test that need inndob + # This is a test that need innodb if ( $::mysqld_variables{'innodb'} eq "FALSE" ) { # innodb is not supported, skip it @@ -578,7 +580,6 @@ our @tags= ["include/have_debug.inc", "need_debug", 1], ["include/have_ndb.inc", "ndb_test", 1], ["include/have_ndb_extra.inc", "ndb_extra", 1], - ["include/have_multi_ndb.inc", "master_num", 2], ["require_manager", "require_manager", 1], ); From ba6e6d07543b458239bb06b0be81e1d62cb0ef68 Mon Sep 17 00:00:00 2001 From: "knielsen@ymer.(none)" <> Date: Fri, 19 Jan 2007 09:41:08 +0100 Subject: [PATCH 2/7] Implement mysql-test-run.pl option to limit the number of saved core files. This helps stability of multiple parallel automated test runs, avoiding the situation where one bad build fills up disk with 1000s of core files, causing failures in other test runs. --- mysql-test/mysql-test-run.pl | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index b3a7427c359..4e122b43143 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -300,6 +300,8 @@ our %mysqld_variables; my $source_dist= 0; +our $opt_max_save_core= 5; +my $num_saved_cores= 0; # Number of core files saved in vardir/log/ so far. ###################################################################### # @@ -588,6 +590,7 @@ sub command_line_setup () { 'strace-client' => \$opt_strace_client, 'master-binary=s' => \$exe_master_mysqld, 'slave-binary=s' => \$exe_slave_mysqld, + 'max-save-core=i' => \$opt_max_save_core, # Coverage, profiling etc 'gcov' => \$opt_gcov, @@ -3295,10 +3298,12 @@ sub save_files_before_restore($$) { # Look for core files foreach my $core_file ( glob("$data_dir/core*") ) { + last if $opt_max_save_core > 0 && $num_saved_cores >= $opt_max_save_core; my $core_name= basename($core_file); mtr_report("Saving $core_name"); mkdir($save_name) if ! -d $save_name; rename("$core_file", "$save_name/$core_name"); + ++$num_saved_cores; } } @@ -4891,6 +4896,9 @@ Options for debugging the product master-binary=PATH Specify the master "mysqld" to use slave-binary=PATH Specify the slave "mysqld" to use strace-client Create strace output for mysqltest client + max-save-core Limit the number of core files saved (to avoid filling + up disks for heavily crashing server). Defaults to + $opt_max_save_core, set to 0 for no limit. Options for coverage, profiling etc From 72a6096eb432ddfdef7cdf56588eaf787ae8945c Mon Sep 17 00:00:00 2001 From: "mtaylor@qualinost.(none)" <> Date: Fri, 19 Jan 2007 17:34:47 -0800 Subject: [PATCH 3/7] Moving version-script to acinclude.m4 --- acinclude.m4 | 8 ++++++++ configure.in | 7 ------- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/acinclude.m4 b/acinclude.m4 index 811e9c0b183..eaf6d628e4e 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -1817,6 +1817,14 @@ AC_DEFUN([MYSQL_CHECK_NDBCLUSTER], [ ndbcluster_system_libs="" ndb_mgmclient_libs="\$(top_builddir)/ndb/src/mgmclient/libndbmgmclient.la" MYSQL_CHECK_NDB_OPTIONS + + # libndbclient versioning when linked with GNU ld. + if $LD --version 2>/dev/null|grep -q GNU; then + NDB_LD_VERSION_SCRIPT="-Wl,--version-script=\$(top_builddir)/ndb/src/libndb.ver" + AC_CONFIG_FILES(ndb/src/libndb.ver) + fi + AC_SUBST(NDB_LD_VERSION_SCRIPT) + ;; * ) AC_MSG_RESULT([Not using NDB Cluster]) diff --git a/configure.in b/configure.in index 47965876627..6b8a78615c3 100644 --- a/configure.in +++ b/configure.in @@ -449,13 +449,6 @@ if $LD --version 2>/dev/null|grep -q GNU; then fi AC_SUBST(LD_VERSION_SCRIPT) -# libndbclient versioning when linked with GNU ld. -if $LD --version 2>/dev/null|grep -q GNU; then - NDB_LD_VERSION_SCRIPT="-Wl,--version-script=\$(top_builddir)/ndb/src/libndb.ver" - AC_CONFIG_FILES(ndb/src/libndb.ver) -fi -AC_SUBST(NDB_LD_VERSION_SCRIPT) - # Avoid bug in fcntl on some versions of linux From b448b6227829ca70a6afdd0e92062a00df0c97cf Mon Sep 17 00:00:00 2001 From: "tsmith@siva.hindu.god" <> Date: Sun, 21 Jan 2007 19:55:17 -0700 Subject: [PATCH 4/7] Applied innodb-4.1-ss36 and innodb-4.1-ss38 snapshots Fixes: - Bug #24299: - Bug #25596: --- innobase/dict/dict0dict.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/innobase/dict/dict0dict.c b/innobase/dict/dict0dict.c index 4b23ce047b2..1e2511f88fa 100644 --- a/innobase/dict/dict0dict.c +++ b/innobase/dict/dict0dict.c @@ -27,6 +27,9 @@ Created 1/8/1996 Heikki Tuuri #include "que0que.h" #include "rem0cmp.h" +/* Implement isspace() in a locale-independent way. (Bug #24299) */ +#define ib_isspace(c) ((char) (c) && strchr(" \v\f\t\r\n", c)) + dict_sys_t* dict_sys = NULL; /* the dictionary system */ rw_lock_t dict_operation_lock; /* table create, drop, etc. reserve @@ -2374,7 +2377,7 @@ dict_accept( *success = FALSE; - while (isspace(*ptr)) { + while (ib_isspace(*ptr)) { ptr++; } @@ -2419,7 +2422,7 @@ dict_scan_id( *id = NULL; - while (isspace(*ptr)) { + while (ib_isspace(*ptr)) { ptr++; } @@ -2450,7 +2453,7 @@ dict_scan_id( len++; } } else { - while (!isspace(*ptr) && *ptr != '(' && *ptr != ')' + while (!ib_isspace(*ptr) && *ptr != '(' && *ptr != ')' && (accept_also_dot || *ptr != '.') && *ptr != ',' && *ptr != '\0') { @@ -2480,12 +2483,12 @@ dict_scan_id( if (heap && !quote) { /* EMS MySQL Manager sometimes adds characters 0xA0 (in latin1, a 'non-breakable space') to the end of a table name. - But isspace(0xA0) is not true, which confuses our foreign key - parser. After the UTF-8 conversion in ha_innodb.cc, bytes 0xC2 - and 0xA0 are at the end of the string. + After the UTF-8 conversion in ha_innodb.cc, bytes 0xC2 + and 0xA0 are at the end of the string, and ib_isspace() + does not work for multi-byte UTF-8 characters. - TODO: we should lex the string using thd->charset_info, and - my_isspace(). Only after that, convert id names to UTF-8. */ + In MySQL 5.1 we lex the string using thd->charset_info, and + my_isspace(). This workaround is not needed there. */ b = (byte*)(*id); id_len = strlen(b); @@ -2956,11 +2959,11 @@ loop: ut_a(success); - if (!isspace(*ptr) && *ptr != '"' && *ptr != '`') { + if (!ib_isspace(*ptr) && *ptr != '"' && *ptr != '`') { goto loop; } - while (isspace(*ptr)) { + while (ib_isspace(*ptr)) { ptr++; } @@ -2990,7 +2993,7 @@ loop: goto loop; } - if (!isspace(*ptr)) { + if (!ib_isspace(*ptr)) { goto loop; } @@ -3078,7 +3081,7 @@ col_loop1: } ptr = dict_accept(ptr, "REFERENCES", &success); - if (!success || !isspace(*ptr)) { + if (!success || !ib_isspace(*ptr)) { dict_foreign_report_syntax_err(name, start_of_latest_foreign, ptr); return(DB_CANNOT_ADD_CONSTRAINT); @@ -3461,7 +3464,7 @@ loop: ptr = dict_accept(ptr, "DROP", &success); - if (!isspace(*ptr)) { + if (!ib_isspace(*ptr)) { goto loop; } From 58ae89cb766520e8c048bca3710a2398b7fcbd4e Mon Sep 17 00:00:00 2001 From: "mtaylor@qualinost.(none)" <> Date: Fri, 26 Jan 2007 12:00:38 -0800 Subject: [PATCH 5/7] User visible change - breaks some environments, per Paul DuBois. Reverting in 4.1 and 5.0. --- mysys/default.c | 1 - scripts/mysqld_multi.sh | 8 -------- 2 files changed, 9 deletions(-) diff --git a/mysys/default.c b/mysys/default.c index 1d71399ef71..6e40c48d82a 100644 --- a/mysys/default.c +++ b/mysys/default.c @@ -49,7 +49,6 @@ const char *default_directories[]= { "sys:/etc/", #else "/etc/", -"/etc/mysql/", #endif #ifdef DATADIR DATADIR, diff --git a/scripts/mysqld_multi.sh b/scripts/mysqld_multi.sh index 48e64a6b541..54662d2c017 100644 --- a/scripts/mysqld_multi.sh +++ b/scripts/mysqld_multi.sh @@ -437,14 +437,6 @@ sub find_groups { $data[$i] = $line; } - if (-f "/etc/mysql/my.cnf" && -r "/etc/mysql/my.cnf") - { - open(MY_CNF, ") && close(MY_CNF); - } - for (; ($line = shift @tmp); $i++) - { - $data[$i] = $line; - } if (-f "$homedir/.my.cnf" && -r "$homedir/.my.cnf") { open(MY_CNF, "<$homedir/.my.cnf") && (@tmp=) && close(MY_CNF); From 7eaa82ea38e1a9f0598ecfc88439583b51567e64 Mon Sep 17 00:00:00 2001 From: "msvensson@pilot.mysql.com" <> Date: Mon, 29 Jan 2007 14:31:48 +0100 Subject: [PATCH 6/7] Bug#22943 syscall pruning in libmysql - Set the timeout values only where needed --- sql-common/client.c | 8 +++++++- sql/mysql_priv.h | 2 ++ sql/mysqld.cc | 10 ++-------- sql/net_serv.cc | 27 +++++++++++++++++++++++++-- sql/repl_failsafe.cc | 9 ++++++--- sql/set_var.cc | 4 ++-- sql/slave.cc | 1 - sql/sql_parse.cc | 37 +++++++++++++++++++++++++------------ sql/sql_repl.cc | 6 +++--- vio/vio.c | 2 +- vio/viossl.c | 11 ----------- 11 files changed, 73 insertions(+), 44 deletions(-) diff --git a/sql-common/client.c b/sql-common/client.c index 87e22624dd9..431c1bdf418 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -1881,11 +1881,17 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, goto error; } vio_keepalive(net->vio,TRUE); - /* Override local client variables */ + + /* If user set read_timeout, let it override the default */ if (mysql->options.read_timeout) net->read_timeout= mysql->options.read_timeout; + vio_timeout(net->vio, 0, net->read_timeout); + + /* If user set write_timeout, let it override the default */ if (mysql->options.write_timeout) net->write_timeout= mysql->options.write_timeout; + vio_timeout(net->vio, 1, net->write_timeout); + if (mysql->options.max_allowed_packet) net->max_packet_size= mysql->options.max_allowed_packet; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index a12944437d7..253eadd266e 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -59,6 +59,8 @@ void kill_one_thread(THD *thd, ulong id); bool net_request_file(NET* net, const char* fname); char* query_table_status(THD *thd,const char *db,const char *table_name); +void net_set_write_timeout(NET *net, uint timeout); +void net_set_read_timeout(NET *net, uint timeout); #define x_free(A) { my_free((gptr) (A),MYF(MY_WME | MY_FAE | MY_ALLOW_ZERO_PTR)); } #define safeFree(x) { if(x) { my_free((gptr) x,MYF(0)); x = NULL; } } diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 024e4682a22..59ed60d4c85 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -3601,10 +3601,9 @@ static bool read_init_file(char *file_name) #ifndef EMBEDDED_LIBRARY static void create_new_thread(THD *thd) { + NET *net=&thd->net; DBUG_ENTER("create_new_thread"); - NET *net=&thd->net; // For easy ref - net->read_timeout = (uint) connect_timeout; if (protocol_version > 9) net->return_errno=1; @@ -3899,12 +3898,7 @@ extern "C" pthread_handler_decl(handle_connections_sockets, } if (sock == unix_sock) thd->host=(char*) my_localhost; -#ifdef __WIN__ - /* Set default wait_timeout */ - ulong wait_timeout= global_system_variables.net_wait_timeout * 1000; - (void) setsockopt(new_sock, SOL_SOCKET, SO_RCVTIMEO, (char*)&wait_timeout, - sizeof(wait_timeout)); -#endif + create_new_thread(thd); } diff --git a/sql/net_serv.cc b/sql/net_serv.cc index 08184537896..a5a05d381cd 100644 --- a/sql/net_serv.cc +++ b/sql/net_serv.cc @@ -491,7 +491,7 @@ net_real_write(NET *net,const char *packet,ulong len) thr_alarm(&alarmed,(uint) net->write_timeout,&alarm_buff); #else alarmed=0; - vio_timeout(net->vio, 1, net->write_timeout); + /* Write timeout is set in net_set_write_timeout */ #endif /* NO_ALARM */ pos=(char*) packet; end=pos+len; @@ -684,7 +684,7 @@ my_real_read(NET *net, ulong *complen) if (net_blocking) thr_alarm(&alarmed,net->read_timeout,&alarm_buff); #else - vio_timeout(net->vio, 0, net->read_timeout); + /* Read timeout is set in net_set_read_timeout */ #endif /* NO_ALARM */ pos = net->buff + net->where_b; /* net->packet -4 */ @@ -995,3 +995,26 @@ my_net_read(NET *net) return len; } + +void net_set_read_timeout(NET *net, uint timeout) +{ + DBUG_ENTER("net_set_read_timeout"); + DBUG_PRINT("enter", ("timeout: %d", timeout)); + net->read_timeout= timeout; +#ifdef NO_ALARM + vio_timeout(net->vio, 0, timeout); +#endif + DBUG_VOID_RETURN; +} + + +void net_set_write_timeout(NET *net, uint timeout) +{ + DBUG_ENTER("net_set_write_timeout"); + DBUG_PRINT("enter", ("timeout: %d", timeout)); + net->write_timeout= timeout; +#ifdef NO_ALARM + vio_timeout(net->vio, 1, timeout); +#endif + DBUG_VOID_RETURN; +} diff --git a/sql/repl_failsafe.cc b/sql/repl_failsafe.cc index 61fd5d9bce4..4c8703226a6 100644 --- a/sql/repl_failsafe.cc +++ b/sql/repl_failsafe.cc @@ -57,6 +57,7 @@ static Slave_log_event* find_slave_event(IO_CACHE* log, functions like register_slave()) are working. */ +#if NOT_USED static int init_failsafe_rpl_thread(THD* thd) { DBUG_ENTER("init_failsafe_rpl_thread"); @@ -99,7 +100,7 @@ static int init_failsafe_rpl_thread(THD* thd) thd->set_time(); DBUG_RETURN(0); } - +#endif void change_rpl_status(RPL_STATUS from_status, RPL_STATUS to_status) { @@ -573,12 +574,14 @@ err: } +#if NOT_USED int find_recovery_captain(THD* thd, MYSQL* mysql) { return 0; } +#endif - +#if NOT_USED pthread_handler_decl(handle_failsafe_rpl,arg) { DBUG_ENTER("handle_failsafe_rpl"); @@ -626,7 +629,7 @@ err: pthread_exit(0); DBUG_RETURN(0); } - +#endif int show_slave_hosts(THD* thd) { diff --git a/sql/set_var.cc b/sql/set_var.cc index 71ca382f9d9..bf76d96c0ed 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -1128,14 +1128,14 @@ static void fix_tx_isolation(THD *thd, enum_var_type type) static void fix_net_read_timeout(THD *thd, enum_var_type type) { if (type != OPT_GLOBAL) - thd->net.read_timeout=thd->variables.net_read_timeout; + net_set_read_timeout(&thd->net, thd->variables.net_read_timeout); } static void fix_net_write_timeout(THD *thd, enum_var_type type) { if (type != OPT_GLOBAL) - thd->net.write_timeout=thd->variables.net_write_timeout; + net_set_write_timeout(&thd->net, thd->variables.net_write_timeout); } static void fix_net_retry_count(THD *thd, enum_var_type type) diff --git a/sql/slave.cc b/sql/slave.cc index 6785e92b9f9..9e1ec86dd1d 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -2625,7 +2625,6 @@ static int init_slave_thread(THD* thd, SLAVE_THD_TYPE thd_type) */ thd->variables.max_allowed_packet= global_system_variables.max_allowed_packet + MAX_LOG_EVENT_HEADER; /* note, incr over the global not session var */ - thd->net.read_timeout = slave_net_timeout; thd->master_access= ~(ulong)0; thd->priv_user = 0; thd->slave_thread = 1; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 07064113209..808e08e4b21 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -867,7 +867,7 @@ static int check_connection(THD *thd) return(ER_HANDSHAKE_ERROR); } DBUG_PRINT("info", ("IO layer change in progress...")); - if (sslaccept(ssl_acceptor_fd, net->vio, thd->variables.net_wait_timeout)) + if (sslaccept(ssl_acceptor_fd, net->vio, net->read_timeout)) { DBUG_PRINT("error", ("Failed to read user information (pkt_len= %lu)", pkt_len)); @@ -897,7 +897,6 @@ static int check_connection(THD *thd) if ((thd->client_capabilities & CLIENT_TRANSACTIONS) && opt_using_transactions) net->return_status= &thd->server_status; - net->read_timeout=(uint) thd->variables.net_read_timeout; char *user= end; char *passwd= strend(user)+1; @@ -1029,6 +1028,10 @@ pthread_handler_decl(handle_one_connection,arg) NET *net= &thd->net; thd->thread_stack= (char*) &thd; + /* Use "connect_timeout" value during connection phase */ + net_set_read_timeout(net, connect_timeout); + net_set_write_timeout(net, connect_timeout); + if ((error=check_connection(thd))) { // Wrong permissions if (error > 0) @@ -1058,6 +1061,11 @@ pthread_handler_decl(handle_one_connection,arg) if (thd->query_error) thd->killed= 1; } + + /* Connect completed, set read/write timeouts back to tdefault */ + net_set_read_timeout(net, thd->variables.net_read_timeout); + net_set_write_timeout(net, thd->variables.net_write_timeout); + while (!net->error && net->vio != 0 && !thd->killed) { if (do_command(thd)) @@ -1261,7 +1269,7 @@ err: #ifndef EMBEDDED_LIBRARY /* - Read one command from socket and execute it (query or simple command). + Read one command from connection and execute it (query or simple command). This function is called in loop from thread function. SYNOPSIS do_command() @@ -1272,24 +1280,26 @@ err: bool do_command(THD *thd) { - char *packet; - uint old_timeout; + char *packet= 0; ulong packet_length; - NET *net; + NET *net= &thd->net; enum enum_server_command command; DBUG_ENTER("do_command"); - net= &thd->net; /* indicator of uninitialized lex => normal flow of errors handling (see my_message_sql) */ thd->lex->current_select= 0; - packet=0; - old_timeout=net->read_timeout; - // Wait max for 8 hours - net->read_timeout=(uint) thd->variables.net_wait_timeout; + /* + This thread will do a blocking read from the client which + will be interrupted when the next command is received from + the client, the connection is closed or "net_wait_timeout" + number of seconds has passed + */ + net_set_read_timeout(net, thd->variables.net_wait_timeout); + thd->clear_error(); // Clear error message net_new_transaction(net); @@ -1318,7 +1328,10 @@ bool do_command(THD *thd) vio_description(net->vio), command, command_name[command])); } - net->read_timeout=old_timeout; // restore it + + /* Restore read timeout value */ + net_set_read_timeout(net, thd->variables.net_read_timeout); + /* packet_length contains length of data, as it was stored in packet header. In case of malformed header, packet_length can be zero. diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc index a20f2a6506c..f83313a8fd8 100644 --- a/sql/sql_repl.cc +++ b/sql/sql_repl.cc @@ -89,8 +89,8 @@ static int send_file(THD *thd) The client might be slow loading the data, give him wait_timeout to do the job */ - old_timeout = thd->net.read_timeout; - thd->net.read_timeout = thd->variables.net_wait_timeout; + old_timeout= net->read_timeout; + net_set_read_timeout(net, thd->variables.net_wait_timeout); /* We need net_flush here because the client will not know it needs to send @@ -134,7 +134,7 @@ static int send_file(THD *thd) error = 0; err: - thd->net.read_timeout = old_timeout; + net_set_read_timeout(net, old_timeout); if (fd >= 0) (void) my_close(fd, MYF(0)); if (errmsg) diff --git a/vio/vio.c b/vio/vio.c index 6174acd7024..1eeafe483dc 100644 --- a/vio/vio.c +++ b/vio/vio.c @@ -96,7 +96,7 @@ void vio_reset(Vio* vio, enum enum_vio_type type, vio->in_addr =vio_ssl_in_addr; vio->vioblocking =vio_ssl_blocking; vio->is_blocking =vio_is_blocking; - vio->timeout =vio_ssl_timeout; + vio->timeout =vio_timeout; } else /* default is VIO_TYPE_TCPIP */ #endif /* HAVE_OPENSSL */ diff --git a/vio/viossl.c b/vio/viossl.c index 62145fe5006..8da9723d7d8 100644 --- a/vio/viossl.c +++ b/vio/viossl.c @@ -416,15 +416,4 @@ int vio_ssl_blocking(Vio * vio __attribute__((unused)), } -void vio_ssl_timeout(Vio *vio __attribute__((unused)), - uint which __attribute__((unused)), - uint timeout __attribute__((unused))) -{ -#ifdef __WIN__ - ulong wait_timeout= (ulong) timeout * 1000; - (void) setsockopt(vio->sd, SOL_SOCKET, - which ? SO_SNDTIMEO : SO_RCVTIMEO, (char*) &wait_timeout, - sizeof(wait_timeout)); -#endif /* __WIN__ */ -} #endif /* HAVE_OPENSSL */ From eb415e4920339b834277c507d746eaf0a3296ac7 Mon Sep 17 00:00:00 2001 From: "ramil/ram@mysql.com/ramil.myoffice.izhnet.ru" <> Date: Wed, 31 Jan 2007 09:51:05 +0400 Subject: [PATCH 7/7] fix for bug #19690: ORDER BY eliminates rows from the result Depending on the queries we use different data processing methods and can lose some data in case of double (and decimal in 4.1) fields. The fix consists of two parts: 1. double comparison changed, now double a is equal to double b if (a-b) is less than 5*0.1^(1 + max(a->decimals, b->decimals)). For example, if a->decimals==1, b->decimals==2, a==b if (a-b)<0.005 2. if we use a temporary table, store double values there as is to avoid any data conversion (rounding). --- mysql-test/r/type_float.result | 71 ++++++++++++++++++++++++++++++++++ mysql-test/t/type_float.test | 25 ++++++++++++ sql/field.cc | 2 +- sql/field.h | 15 +++++-- sql/init.cc | 6 +++ sql/item_cmpfunc.cc | 49 +++++++++++++++++++++++ sql/item_cmpfunc.h | 3 ++ sql/mysql_priv.h | 1 + sql/mysqld.cc | 1 + sql/sql_select.cc | 14 ++++--- 10 files changed, 177 insertions(+), 10 deletions(-) diff --git a/mysql-test/r/type_float.result b/mysql-test/r/type_float.result index f157bbc602d..3e66fa70dc1 100644 --- a/mysql-test/r/type_float.result +++ b/mysql-test/r/type_float.result @@ -278,4 +278,75 @@ select 1e-308, 1.00000001e-300, 100000000e-300; select 10e307; 10e307 1e+308 +create table t1(a int, b double(8, 2)); +insert into t1 values +(1, 28.50), (1, 121.85), (1, 157.23), (1, 1351.00), (1, -1965.35), (1, 81.75), +(1, 217.08), (1, 7.94), (4, 96.07), (4, 6404.65), (4, -6500.72), (2, 100.00), +(5, 5.00), (5, -2104.80), (5, 2033.80), (5, 0.07), (5, 65.93), +(3, -4986.24), (3, 5.00), (3, 4857.34), (3, 123.74), (3, 0.16), +(6, -1695.31), (6, 1003.77), (6, 499.72), (6, 191.82); +explain select sum(b) s from t1 group by a; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ALL NULL NULL NULL NULL 26 Using temporary; Using filesort +select sum(b) s from t1 group by a; +s +0.00 +100.00 +0.00 +-0.00 +-0.00 +0.00 +select sum(b) s from t1 group by a having s <> 0; +s +100.00 +select sum(b) s from t1 group by a having s <> 0 order by s; +s +100.00 +select sum(b) s from t1 group by a having s <=> 0; +s +0.00 +0.00 +-0.00 +-0.00 +0.00 +select sum(b) s from t1 group by a having s <=> 0 order by s; +s +-0.00 +-0.00 +0.00 +0.00 +0.00 +alter table t1 add key (a, b); +explain select sum(b) s from t1 group by a; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 index NULL a 14 NULL 26 Using index +select sum(b) s from t1 group by a; +s +0.00 +100.00 +0.00 +-0.00 +0.00 +0.00 +select sum(b) s from t1 group by a having s <> 0; +s +100.00 +select sum(b) s from t1 group by a having s <> 0 order by s; +s +100.00 +select sum(b) s from t1 group by a having s <=> 0; +s +0.00 +0.00 +-0.00 +0.00 +0.00 +select sum(b) s from t1 group by a having s <=> 0 order by s; +s +-0.00 +0.00 +0.00 +0.00 +0.00 +drop table t1; End of 4.1 tests diff --git a/mysql-test/t/type_float.test b/mysql-test/t/type_float.test index 8a484f7bcd0..8c22d22ca66 100644 --- a/mysql-test/t/type_float.test +++ b/mysql-test/t/type_float.test @@ -188,4 +188,29 @@ select 1e-308, 1.00000001e-300, 100000000e-300; # check if overflows are detected correctly select 10e307; +# +# Bug #19690: ORDER BY eliminates rows from the result +# +create table t1(a int, b double(8, 2)); +insert into t1 values +(1, 28.50), (1, 121.85), (1, 157.23), (1, 1351.00), (1, -1965.35), (1, 81.75), +(1, 217.08), (1, 7.94), (4, 96.07), (4, 6404.65), (4, -6500.72), (2, 100.00), +(5, 5.00), (5, -2104.80), (5, 2033.80), (5, 0.07), (5, 65.93), +(3, -4986.24), (3, 5.00), (3, 4857.34), (3, 123.74), (3, 0.16), +(6, -1695.31), (6, 1003.77), (6, 499.72), (6, 191.82); +explain select sum(b) s from t1 group by a; +select sum(b) s from t1 group by a; +select sum(b) s from t1 group by a having s <> 0; +select sum(b) s from t1 group by a having s <> 0 order by s; +select sum(b) s from t1 group by a having s <=> 0; +select sum(b) s from t1 group by a having s <=> 0 order by s; +alter table t1 add key (a, b); +explain select sum(b) s from t1 group by a; +select sum(b) s from t1 group by a; +select sum(b) s from t1 group by a having s <> 0; +select sum(b) s from t1 group by a having s <> 0 order by s; +select sum(b) s from t1 group by a having s <=> 0; +select sum(b) s from t1 group by a having s <=> 0 order by s; +drop table t1; + --echo End of 4.1 tests diff --git a/sql/field.cc b/sql/field.cc index e88b8b313e2..acc837c1d37 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -3318,7 +3318,7 @@ int Field_double::store(double nr) else { double max_value; - if (dec >= NOT_FIXED_DEC) + if (not_fixed) { max_value= DBL_MAX; } diff --git a/sql/field.h b/sql/field.h index e4991ba1961..b10d950163c 100644 --- a/sql/field.h +++ b/sql/field.h @@ -616,6 +616,7 @@ public: class Field_double :public Field_num { public: + my_bool not_fixed; Field_double(char *ptr_arg, uint32 len_arg, uchar *null_ptr_arg, uchar null_bit_arg, enum utype unireg_check_arg, const char *field_name_arg, @@ -623,12 +624,20 @@ public: uint8 dec_arg,bool zero_arg,bool unsigned_arg) :Field_num(ptr_arg, len_arg, null_ptr_arg, null_bit_arg, unireg_check_arg, field_name_arg, table_arg, - dec_arg, zero_arg,unsigned_arg) + dec_arg, zero_arg, unsigned_arg), + not_fixed(dec_arg >= NOT_FIXED_DEC) {} Field_double(uint32 len_arg, bool maybe_null_arg, const char *field_name_arg, struct st_table *table_arg, uint8 dec_arg) - :Field_num((char*) 0, len_arg, maybe_null_arg ? (uchar*) "": 0, (uint) 0, - NONE, field_name_arg, table_arg,dec_arg,0,0) + :Field_num((char *) 0, len_arg, maybe_null_arg ? (uchar *) "" : 0, (uint) 0, + NONE, field_name_arg, table_arg,dec_arg, 0, 0), + not_fixed(dec_arg >= NOT_FIXED_DEC) + {} + Field_double(uint32 len_arg, bool maybe_null_arg, const char *field_name_arg, + struct st_table *table_arg, uint8 dec_arg, my_bool not_fixed_srg) + :Field_num((char *) 0, len_arg, maybe_null_arg ? (uchar *) "" : 0, (uint) 0, + NONE, field_name_arg, table_arg, dec_arg, 0, 0), + not_fixed(not_fixed_srg) {} enum_field_types type() const { return FIELD_TYPE_DOUBLE;} enum ha_base_keytype key_type() const { return HA_KEYTYPE_DOUBLE; } diff --git a/sql/init.cc b/sql/init.cc index 4beb8db0c6f..5e1b6532c75 100644 --- a/sql/init.cc +++ b/sql/init.cc @@ -45,6 +45,12 @@ void unireg_init(ulong options) { /* It's used by filesort... */ log_10[i]= nr ; nr*= 10.0; } + /* Make a tab of powers of 0.1 */ + for (i= 0, nr= 0.1; i < array_elements(log_01); i++) + { + log_01[i]= nr; + nr*= 0.1; + } specialflag|=options; /* Set options from argv */ DBUG_VOID_RETURN; } diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc index 859b4e0ecc1..65f9b279a18 100644 --- a/sql/item_cmpfunc.cc +++ b/sql/item_cmpfunc.cc @@ -352,6 +352,17 @@ int Arg_comparator::set_compare_func(Item_bool_func2 *item, Item_result type) func= &Arg_comparator::compare_e_int_diff_signedness; } } + else if (type == REAL_RESULT) + { + if ((*a)->decimals < NOT_FIXED_DEC && (*b)->decimals < NOT_FIXED_DEC) + { + precision= 5 * log_01[max((*a)->decimals, (*b)->decimals)]; + if (func == &Arg_comparator::compare_real) + func= &Arg_comparator::compare_real_fixed; + else if (func == &Arg_comparator::compare_e_real) + func= &Arg_comparator::compare_e_real_fixed; + } + } return 0; } @@ -459,6 +470,44 @@ int Arg_comparator::compare_e_real() return test(val1 == val2); } + +int Arg_comparator::compare_real_fixed() +{ + /* + Fix yet another manifestation of Bug#2338. 'Volatile' will instruct + gcc to flush double values out of 80-bit Intel FPU registers before + performing the comparison. + */ + volatile double val1, val2; + val1= (*a)->val(); + if (!(*a)->null_value) + { + val2= (*b)->val(); + if (!(*b)->null_value) + { + owner->null_value= 0; + if (val1 == val2 || fabs(val1 - val2) < precision) + return 0; + if (val1 < val2) + return -1; + return 1; + } + } + owner->null_value= 1; + return -1; +} + + +int Arg_comparator::compare_e_real_fixed() +{ + double val1= (*a)->val(); + double val2= (*b)->val(); + if ((*a)->null_value || (*b)->null_value) + return test((*a)->null_value && (*b)->null_value); + return test(val1 == val2 || fabs(val1 - val2) < precision); +} + + int Arg_comparator::compare_int_signed() { longlong val1= (*a)->val_int(); diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h index 0e157fd412c..3dc09f0789a 100644 --- a/sql/item_cmpfunc.h +++ b/sql/item_cmpfunc.h @@ -33,6 +33,7 @@ class Arg_comparator: public Sql_alloc arg_cmp_func func; Item_bool_func2 *owner; Arg_comparator *comparators; // used only for compare_row() + double precision; public: DTCollation cmp_collation; @@ -77,6 +78,8 @@ public: int compare_e_int(); // compare args[0] & args[1] int compare_e_int_diff_signedness(); int compare_e_row(); // compare args[0] & args[1] + int compare_real_fixed(); + int compare_e_real_fixed(); static arg_cmp_func comparator_matrix [4][2]; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 3a240612cfa..40b7b9bcb1a 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -907,6 +907,7 @@ extern char glob_hostname[FN_REFLEN], mysql_home[FN_REFLEN]; extern char pidfile_name[FN_REFLEN], system_time_zone[30], *opt_init_file; extern char log_error_file[FN_REFLEN]; extern double log_10[32]; +extern double log_01[32]; extern ulonglong log_10_int[20]; extern ulonglong keybuff_size; extern ulong refresh_version,flush_version, thread_id,query_id,opened_tables; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 024e4682a22..d31cafd9a33 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -361,6 +361,7 @@ ulong my_bind_addr; /* the address we bind to */ volatile ulong cached_thread_count= 0; double log_10[32]; /* 10 potences */ +double log_01[32]; time_t start_time; char mysql_home[FN_REFLEN], pidfile_name[FN_REFLEN], system_time_zone[30]; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 0254e8f56dc..a6881337d83 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -5006,6 +5006,8 @@ static Field* create_tmp_field_from_field(THD *thd, Field* org_field, new_field->flags&= ~NOT_NULL_FLAG; // Because of outer join if (org_field->type() == FIELD_TYPE_VAR_STRING) table->db_create_options|= HA_OPTION_PACK_RECORD; + else if (org_field->type() == FIELD_TYPE_DOUBLE) + ((Field_double *) new_field)->not_fixed= TRUE; } return new_field; } @@ -5045,7 +5047,7 @@ static Field* create_tmp_field_from_item(THD *thd, Item *item, TABLE *table, switch (item->result_type()) { case REAL_RESULT: new_field=new Field_double(item->max_length, maybe_null, - item->name, table, item->decimals); + item->name, table, item->decimals, TRUE); break; case INT_RESULT: new_field=new Field_longlong(item->max_length, maybe_null, @@ -5136,8 +5138,8 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type, return new Field_string(sizeof(double)+sizeof(longlong), 0, item->name,table,&my_charset_bin); else - return new Field_double(item_sum->max_length,maybe_null, - item->name, table, item_sum->decimals); + return new Field_double(item_sum->max_length, maybe_null, + item->name, table, item_sum->decimals, TRUE); case Item_sum::VARIANCE_FUNC: /* Place for sum & count */ case Item_sum::STD_FUNC: if (group) @@ -5145,7 +5147,7 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type, 0, item->name,table,&my_charset_bin); else return new Field_double(item_sum->max_length, maybe_null, - item->name,table,item_sum->decimals); + item->name, table, item_sum->decimals, TRUE); case Item_sum::UNIQUE_USERS_FUNC: return new Field_long(9,maybe_null,item->name,table,1); case Item_sum::MIN_FUNC: @@ -5160,8 +5162,8 @@ Field *create_tmp_field(THD *thd, TABLE *table,Item *item, Item::Type type, default: switch (item_sum->result_type()) { case REAL_RESULT: - return new Field_double(item_sum->max_length,maybe_null, - item->name,table,item_sum->decimals); + return new Field_double(item_sum->max_length, maybe_null, + item->name, table, item_sum->decimals, TRUE); case INT_RESULT: return new Field_longlong(item_sum->max_length,maybe_null, item->name,table,item->unsigned_flag);