From 66cd6d0c8f24825a688f8dd8d091c7a50d72fbb5 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 19 Oct 2007 19:57:08 +0400 Subject: [PATCH 01/31] Patch for BUG#31111: --read-only crashes MySQL (events fail to load). There actually were several problems here: - WRITE-lock is required to load events from the mysql.event table, but in the read-only mode an ordinary user can not acquire it; - Security_context::master_access attribute was not properly initialized in Security_context::init(), which led to differences in behavior with and without debug configure options. - if the server failed to load events from mysql.event, it forgot to close the mysql.event table, that led to the coredump, described in the bug report. The patch is to fix all these problems: - Use the super-user to acquire WRITE-lock on the mysql.even table; - The WRITE-lock is acquired by the event scheduler in two cases: - on initial loading of events from the database; - when an event has been executed, so its attributes should be updated. Other cases when WRITE-lock is needed for the mysql.event table happen under the user account. So, nothing should be changed there for the read-only mode. The user is able to create/update/drop an event only if he is a super-user. - Initialize Security_context::master_access; - Close the mysql.event table in case something went wrong. mysql-test/r/events_bugs.result: Update result file. mysql-test/t/events_bugs.test: A test case for BUG#31111: --read-only crashes MySQL (events fail to load). sql/event_data_objects.cc: When the worker thread is going to drop event after the execution we should do it under the super-user privileges in order to be able to lock the mysql.event table for writing in the read-only mode. This is a system operation, where user SQL can not be executed. So, there is no risk in compromising security by dropping an event under the super-user privileges. sql/event_db_repository.cc: 1. Close tables if something went wrong in simple_open_n_lock_tables(); 2. As soon as the system event scheduler thread is running under the super-user privileges, we should always be able to acquire WRITE-lock on the mysql.event table. However, let's have an assert to check this. sql/event_scheduler.cc: Run the system event scheduler thread under the super-user privileges. In particular, this is needed to be able to lock the mysql.event table for writing when the server is running in the read-only mode. The event scheduler executes only system operations and does not execute user SQL (this is what the worker threads for). So, there is no risk in compromising security by running the event scheduler under the super-user privileges. sql/events.cc: Open the mysql.event table as the super user to be able to acquire WRITE-lock in the read-only mode. sql/sql_class.cc: Initialize Security_context::master_acces. --- mysql-test/r/events_bugs.result | 103 ++++++++++++++- mysql-test/t/events_bugs.test | 224 ++++++++++++++++++++++++++++++-- sql/event_data_objects.cc | 16 ++- sql/event_db_repository.cc | 9 ++ sql/event_scheduler.cc | 7 + sql/events.cc | 16 ++- sql/sql_class.cc | 1 + 7 files changed, 360 insertions(+), 16 deletions(-) diff --git a/mysql-test/r/events_bugs.result b/mysql-test/r/events_bugs.result index 3c9e6384c64..b6b77101874 100644 --- a/mysql-test/r/events_bugs.result +++ b/mysql-test/r/events_bugs.result @@ -610,7 +610,6 @@ id ev_nm ev_cnt 6 ev_sched_1823 6 DROP TABLE event_log; SET GLOBAL event_scheduler = OFF; -DROP DATABASE events_test; SET GLOBAL event_scheduler= ON; CREATE EVENT bug28641 ON SCHEDULE AT '2038.01.18 03:00:00' DO BEGIN @@ -618,3 +617,105 @@ SELECT 1; END;| SET GLOBAL event_scheduler= OFF; DROP EVENT bug28641; + +##################################################################### +# +# BUG#31111: --read-only crashes MySQL (events fail to load). +# +##################################################################### + +DROP USER mysqltest_u1@localhost; +DROP EVENT IF EXISTS e1; +DROP EVENT IF EXISTS e2; + +GRANT EVENT ON *.* TO mysqltest_u1@localhost; + +SET GLOBAL READ_ONLY = 1; + +# +# Connection: u1_con (mysqltest_u1@localhost/events_test). +# + +CREATE EVENT e1 ON SCHEDULE AT '2020-01-01 00:00:00' DO SET @a = 1; +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement + +ALTER EVENT e1 COMMENT 'comment'; +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement + +DROP EVENT e1; +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement + +# +# Connection: root_con (root@localhost/events_test). +# + +CREATE EVENT e1 ON SCHEDULE AT '2020-01-01 00:00:00' DO SET @a = 1; + +ALTER EVENT e1 COMMENT 'comment'; + +DROP EVENT e1; + +SET GLOBAL READ_ONLY = 0; + +# +# Connection: u1_con (mysqltest_u1@localhost/test). +# + +CREATE EVENT e1 ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND DO SET @a = 1; +CREATE EVENT e2 ON SCHEDULE EVERY 1 SECOND DO SET @a = 1; + +SELECT +event_name, +last_executed IS NULL, +definer +FROM INFORMATION_SCHEMA.EVENTS +WHERE event_schema = 'events_test'; +event_name last_executed IS NULL definer +e1 1 mysqltest_u1@localhost +e2 1 mysqltest_u1@localhost + +# +# Connection: root_con (root@localhost/events_test). +# + +SET GLOBAL READ_ONLY = 1; + +SET GLOBAL EVENT_SCHEDULER = ON; + +# Waiting for the event scheduler to execute and drop event e1... + +# Waiting for the event scheduler to execute and update event e2... + +SET GLOBAL EVENT_SCHEDULER = OFF; + +SELECT +event_name, +last_executed IS NULL, +definer +FROM INFORMATION_SCHEMA.EVENTS +WHERE event_schema = 'events_test'; +event_name last_executed IS NULL definer +e2 0 mysqltest_u1@localhost + +DROP EVENT e1; +ERROR HY000: Unknown event 'e1' + +# Cleanup. + +DROP EVENT e2; + +SET GLOBAL READ_ONLY = 0; + +# +# Connection: default +# + +DROP USER mysqltest_u1@localhost; + +##################################################################### +# +# End of BUG#31111. +# +##################################################################### + +DROP DATABASE events_test; diff --git a/mysql-test/t/events_bugs.test b/mysql-test/t/events_bugs.test index 36052fdb9af..ebd86f3a3d2 100644 --- a/mysql-test/t/events_bugs.test +++ b/mysql-test/t/events_bugs.test @@ -712,18 +712,6 @@ DROP TABLE event_log; #DROP DATABASE ev_db_1; SET GLOBAL event_scheduler = OFF; -# -# End of tests -# - -let $wait_condition= - select count(*) = 0 from information_schema.processlist - where db='events_test' and command = 'Connect' and user=current_user(); ---source include/wait_condition.inc - -DROP DATABASE events_test; - - # # Bug#28641 CREATE EVENT with '2038.01.18 03:00:00' let server crash. # @@ -737,3 +725,215 @@ CREATE EVENT bug28641 ON SCHEDULE AT '2038.01.18 03:00:00' DELIMITER ;| SET GLOBAL event_scheduler= OFF; DROP EVENT bug28641; + +########################################################################### + +--echo +--echo ##################################################################### +--echo # +--echo # BUG#31111: --read-only crashes MySQL (events fail to load). +--echo # +--echo ##################################################################### +--echo + +--error 0,ER_CANNOT_USER +DROP USER mysqltest_u1@localhost; + +--disable_warnings +DROP EVENT IF EXISTS e1; +DROP EVENT IF EXISTS e2; +--enable_warnings + +--echo + +# Check that an ordinary user can not create/update/drop events in the +# read-only mode. + +GRANT EVENT ON *.* TO mysqltest_u1@localhost; + +--echo + +SET GLOBAL READ_ONLY = 1; + +--echo + +--echo # +--echo # Connection: u1_con (mysqltest_u1@localhost/events_test). +--echo # + +--connect(u1_con,localhost,mysqltest_u1,,events_test) + +--echo + +--error ER_OPTION_PREVENTS_STATEMENT +CREATE EVENT e1 ON SCHEDULE AT '2020-01-01 00:00:00' DO SET @a = 1; + +--echo + +--error ER_OPTION_PREVENTS_STATEMENT +ALTER EVENT e1 COMMENT 'comment'; + +--echo + +--error ER_OPTION_PREVENTS_STATEMENT +DROP EVENT e1; + +--echo + +# Check that the super user still can create/update/drop events. + +--echo # +--echo # Connection: root_con (root@localhost/events_test). +--echo # + +--connect(root_con,localhost,root,,events_test) + +--echo + +CREATE EVENT e1 ON SCHEDULE AT '2020-01-01 00:00:00' DO SET @a = 1; + +--echo + +ALTER EVENT e1 COMMENT 'comment'; + +--echo + +DROP EVENT e1; + +--echo + +# +# Switch to read-write mode; create test events under the user mysqltest_u1; +# switch back to read-only mode. +# + +SET GLOBAL READ_ONLY = 0; + +--echo + +--echo # +--echo # Connection: u1_con (mysqltest_u1@localhost/test). +--echo # + +--connection u1_con + +--echo + +CREATE EVENT e1 ON SCHEDULE AT CURRENT_TIMESTAMP + INTERVAL 1 SECOND DO SET @a = 1; +CREATE EVENT e2 ON SCHEDULE EVERY 1 SECOND DO SET @a = 1; + +--echo + +SELECT + event_name, + last_executed IS NULL, + definer +FROM INFORMATION_SCHEMA.EVENTS +WHERE event_schema = 'events_test'; + +--echo + +--echo # +--echo # Connection: root_con (root@localhost/events_test). +--echo # + +--connection root_con + +--echo + +SET GLOBAL READ_ONLY = 1; + +# Check that the event scheduler is able to update event. + +--echo + +SET GLOBAL EVENT_SCHEDULER = ON; + +--echo + +--echo # Waiting for the event scheduler to execute and drop event e1... + +let $wait_timeout = 2; +let $wait_condition = + SELECT COUNT(*) = 0 + FROM INFORMATION_SCHEMA.EVENTS + WHERE event_schema = 'events_test' AND event_name = 'e1'; +--source include/wait_condition.inc + +--echo + +--echo # Waiting for the event scheduler to execute and update event e2... + +let $wait_condition = + SELECT last_executed IS NOT NULL + FROM INFORMATION_SCHEMA.EVENTS + WHERE event_schema = 'events_test' AND event_name = 'e2'; +--source include/wait_condition.inc + +--echo + +SET GLOBAL EVENT_SCHEDULER = OFF; + +--echo + +SELECT + event_name, + last_executed IS NULL, + definer +FROM INFORMATION_SCHEMA.EVENTS +WHERE event_schema = 'events_test'; + +--echo + +--error ER_EVENT_DOES_NOT_EXIST +DROP EVENT e1; + +--echo +--echo # Cleanup. +--echo + +DROP EVENT e2; + +--echo + +SET GLOBAL READ_ONLY = 0; + +--echo + +--echo # +--echo # Connection: default +--echo # + +--disconnect u1_con +--disconnect root_con +--connection default + +--echo + +DROP USER mysqltest_u1@localhost; + +--echo +--echo ##################################################################### +--echo # +--echo # End of BUG#31111. +--echo # +--echo ##################################################################### +--echo + + +########################################################################### +# +# End of tests +# +# !!! KEEP this section AT THE END of this file !!! +# +########################################################################### + +let $wait_condition= + select count(*) = 0 from information_schema.processlist + where db='events_test' and command = 'Connect' and user=current_user(); +--source include/wait_condition.inc + +DROP DATABASE events_test; + +# THIS MUST BE THE LAST LINE in this file. diff --git a/sql/event_data_objects.cc b/sql/event_data_objects.cc index 787b04c12c6..adac2b596c1 100644 --- a/sql/event_data_objects.cc +++ b/sql/event_data_objects.cc @@ -2017,6 +2017,7 @@ end_no_lex_start: ret= 1; else { + ulong saved_master_access; /* Peculiar initialization order is a crutch to avoid races in SHOW PROCESSLIST which reads thd->{query/query_length} without a mutex. @@ -2024,8 +2025,19 @@ end_no_lex_start: thd->query_length= 0; thd->query= sp_sql.c_ptr_safe(); thd->query_length= sp_sql.length(); - if (Events::drop_event(thd, dbname, name, FALSE)) - ret= 1; + + /* + NOTE: even if we run in read-only mode, we should be able to lock + the mysql.event table for writing. In order to achieve this, we + should call mysql_lock_tables() under the super-user. + */ + + saved_master_access= thd->security_ctx->master_access; + thd->security_ctx->master_access |= SUPER_ACL; + + ret= Events::drop_event(thd, dbname, name, FALSE); + + thd->security_ctx->master_access= saved_master_access; } } #ifndef NO_EMBEDDED_ACCESS_CHECKS diff --git a/sql/event_db_repository.cc b/sql/event_db_repository.cc index 705bd8b2704..4451e763ff7 100644 --- a/sql/event_db_repository.cc +++ b/sql/event_db_repository.cc @@ -525,6 +525,10 @@ Event_db_repository::fill_schema_events(THD *thd, TABLE_LIST *tables, - whether this open mode would work under LOCK TABLES, or inside a stored function or trigger. + Note that if the table can't be locked successfully this operation will + close it. Therefore it provides guarantee that it either opens and locks + table or fails without leaving any tables open. + @param[in] thd Thread context @param[in] lock_type How to lock the table @param[out] table We will store the open table here @@ -544,7 +548,10 @@ Event_db_repository::open_event_table(THD *thd, enum thr_lock_type lock_type, tables.init_one_table("mysql", "event", lock_type); if (simple_open_n_lock_tables(thd, &tables)) + { + close_thread_tables(thd, FALSE, FALSE); DBUG_RETURN(TRUE); + } *table= tables.table; tables.table->use_all_columns(); @@ -995,6 +1002,8 @@ update_timing_fields_for_event(THD *thd, if (thd->current_stmt_binlog_row_based) thd->clear_current_stmt_binlog_row_based(); + DBUG_ASSERT(thd->security_ctx->master_access & SUPER_ACL); + if (open_event_table(thd, TL_WRITE, &table)) goto end; diff --git a/sql/event_scheduler.cc b/sql/event_scheduler.cc index b03b51f1134..d3a031fd8f8 100644 --- a/sql/event_scheduler.cc +++ b/sql/event_scheduler.cc @@ -399,6 +399,13 @@ Event_scheduler::start() new_thd->system_thread= SYSTEM_THREAD_EVENT_SCHEDULER; new_thd->command= COM_DAEMON; + /* + We should run the event scheduler thread under the super-user privileges. + In particular, this is needed to be able to lock the mysql.event table + for writing when the server is running in the read-only mode. + */ + new_thd->security_ctx->master_access |= SUPER_ACL; + scheduler_param_value= (struct scheduler_param *)my_malloc(sizeof(struct scheduler_param), MYF(0)); scheduler_param_value->thd= new_thd; diff --git a/sql/events.cc b/sql/events.cc index 5246bccc388..1bfbc5d6645 100644 --- a/sql/events.cc +++ b/sql/events.cc @@ -1124,11 +1124,25 @@ Events::load_events_from_db(THD *thd) READ_RECORD read_record_info; bool ret= TRUE; uint count= 0; + ulong saved_master_access; DBUG_ENTER("Events::load_events_from_db"); DBUG_PRINT("enter", ("thd: 0x%lx", (long) thd)); - if (db_repository->open_event_table(thd, TL_WRITE, &table)) + /* + NOTE: even if we run in read-only mode, we should be able to lock the + mysql.event table for writing. In order to achieve this, we should call + mysql_lock_tables() under the super user. + */ + + saved_master_access= thd->security_ctx->master_access; + thd->security_ctx->master_access |= SUPER_ACL; + + ret= db_repository->open_event_table(thd, TL_WRITE, &table); + + thd->security_ctx->master_access= saved_master_access; + + if (ret) { sql_print_error("Event Scheduler: Failed to open table mysql.event"); DBUG_RETURN(TRUE); diff --git a/sql/sql_class.cc b/sql/sql_class.cc index ffbf0649961..d3ebfd94aa7 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -2404,6 +2404,7 @@ void Security_context::init() host= user= priv_user= ip= 0; host_or_ip= "connecting host"; priv_host[0]= '\0'; + master_access= 0; #ifndef NO_EMBEDDED_ACCESS_CHECKS db_access= NO_ACCESS; #endif From 079ae230032ee2c8aadcce9f70c1f3d686a13da8 Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 20 Oct 2007 01:20:38 +0400 Subject: [PATCH 02/31] Rename: query_error -> is_slave_error. Add comments. sql/ha_ndbcluster_binlog.cc: query_error -> slave_error sql/handler.cc: query_error -> slave_error sql/log.cc: query_error -> slave_error sql/log_event.cc: query_error -> slave_error sql/log_event_old.cc: query_error -> slave_error sql/mysqld.cc: query_error -> slave_error sql/protocol.cc: query_error -> slave_error sql/slave.cc: query_error -> slave_error sql/sp_head.cc: query_error -> slave_error sql/sql_class.cc: query_error -> slave_error sql/sql_class.h: Rename: query_error -> is_slave_error, to avoid confusion. Add commenta. sql/sql_connect.cc: Rename: query_error -> is_slave_error, to avoid confusion. Originally it was the same code to handle init-connect and init-slave mysqld options. Then init-connect implementation forked off, but the one who copy-pasted the code didn't change it to not use a replication-specific variable. --- sql/ha_ndbcluster_binlog.cc | 4 +-- sql/handler.cc | 6 ++--- sql/log.cc | 4 +-- sql/log_event.cc | 54 ++++++++++++++++++++----------------- sql/log_event_old.cc | 12 ++++----- sql/mysqld.cc | 2 +- sql/protocol.cc | 4 +-- sql/slave.cc | 6 ++--- sql/sp_head.cc | 6 ++--- sql/sql_class.cc | 2 +- sql/sql_class.h | 11 ++++++-- sql/sql_connect.cc | 2 +- 12 files changed, 63 insertions(+), 50 deletions(-) diff --git a/sql/ha_ndbcluster_binlog.cc b/sql/ha_ndbcluster_binlog.cc index 5d5c8a26447..c22d5ac53f5 100644 --- a/sql/ha_ndbcluster_binlog.cc +++ b/sql/ha_ndbcluster_binlog.cc @@ -259,7 +259,7 @@ static void run_query(THD *thd, char *buf, char *end, DBUG_PRINT("query", ("%s", thd->query)); mysql_parse(thd, thd->query, thd->query_length, &found_semicolon); - if (no_print_error && thd->query_error) + if (no_print_error && thd->is_slave_error) { int i; Thd_ndb *thd_ndb= get_thd_ndb(thd); @@ -271,7 +271,7 @@ static void run_query(THD *thd, char *buf, char *end, sql_print_error("NDB: %s: error %s %d(ndb: %d) %d %d", buf, thd->net.last_error, thd->net.last_errno, thd_ndb->m_error_code, - thd->net.report_error, thd->query_error); + thd->net.report_error, thd->is_slave_error); } thd->options= save_thd_options; diff --git a/sql/handler.cc b/sql/handler.cc index 75c3a64bc27..706da76c000 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1447,7 +1447,7 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, (We should in the future either rewrite handler::print_error() or make a nice method of this. */ - bool query_error= thd->query_error; + bool is_slave_error= thd->is_slave_error; sp_rcontext *spcont= thd->spcont; SELECT_LEX *current_select= thd->lex->current_select; char buff[sizeof(thd->net.last_error)]; @@ -1455,7 +1455,7 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, int last_errno= thd->net.last_errno; strmake(buff, thd->net.last_error, sizeof(buff)-1); - thd->query_error= 0; + thd->is_slave_error= 0; thd->spcont= NULL; thd->lex->current_select= 0; thd->net.last_error[0]= 0; @@ -1475,7 +1475,7 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, strmake(new_error, thd->net.last_error, sizeof(buff)-1); /* restore thd */ - thd->query_error= query_error; + thd->is_slave_error= is_slave_error; thd->spcont= spcont; thd->lex->current_select= current_select; thd->net.last_errno= last_errno; diff --git a/sql/log.cc b/sql/log.cc index e923418b23a..02986bef493 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -2891,8 +2891,8 @@ int MYSQL_BIN_LOG::purge_logs(const char *to_log, *decrease_log_space-= file_size; ha_binlog_index_purge_file(current_thd, log_info.log_file_name); - if (current_thd->query_error) { - DBUG_PRINT("info",("query error: %d", current_thd->query_error)); + if (current_thd->is_slave_error) { + DBUG_PRINT("info",("slave error: %d", current_thd->is_slave_error)); if (my_errno == EMFILE) { DBUG_PRINT("info",("my_errno: %d, set ret = LOG_INFO_EMFILE", my_errno)); ret = LOG_INFO_EMFILE; diff --git a/sql/log_event.cc b/sql/log_event.cc index a435894382b..a6d07e72033 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -148,7 +148,7 @@ static void pretty_print_str(IO_CACHE* cache, char* str, int len) static void clear_all_errors(THD *thd, Relay_log_info *rli) { - thd->query_error = 0; + thd->is_slave_error = 0; thd->clear_error(); rli->clear_error(); } @@ -2106,7 +2106,7 @@ and was aborted. There is a chance that your master is inconsistent at this \ point. If you are sure that your master is ok, run this query manually on the \ slave and then restart the slave with SET GLOBAL SQL_SLAVE_SKIP_COUNTER=1; \ START SLAVE; . Query: '%s'", expected_error, thd->query); - thd->query_error= 1; + thd->is_slave_error= 1; } goto end; } @@ -2138,7 +2138,7 @@ Default database: '%s'. Query: '%s'", actual_error ? thd->net.last_error: "no error", actual_error, print_slave_db_safe(db), query_arg); - thd->query_error= 1; + thd->is_slave_error= 1; } /* If we get the same error code as expected, or they should be ignored. @@ -2153,14 +2153,14 @@ Default database: '%s'. Query: '%s'", /* Other cases: mostly we expected no error and get one. */ - else if (thd->query_error || thd->is_fatal_error) + else if (thd->is_slave_error || thd->is_fatal_error) { rli->report(ERROR_LEVEL, actual_error, "Error '%s' on query. Default database: '%s'. Query: '%s'", (actual_error ? thd->net.last_error : "unexpected success or fatal error"), print_slave_db_safe(thd->db), query_arg); - thd->query_error= 1; + thd->is_slave_error= 1; } /* @@ -2171,7 +2171,7 @@ Default database: '%s'. Query: '%s'", sql_print_error("Slave: did not get the expected number of affected \ rows running query from master - expected %d, got %d (this numbers \ should have matched modulo 4294967296).", 0, ...); - thd->query_error = 1; + thd->is_slave_error = 1; } We may also want an option to tell the slave to ignore "affected" mismatch. This mismatch could be implemented with a new ER_ code, and @@ -2215,7 +2215,7 @@ end: thd->first_successful_insert_id_in_prev_stmt= 0; thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt= 0; free_root(thd->mem_root,MYF(MY_KEEP_PREALLOC)); - return thd->query_error; + return thd->is_slave_error; } int Query_log_event::do_update_pos(Relay_log_info *rli) @@ -3255,7 +3255,7 @@ int Load_log_event::do_apply_event(NET* net, Relay_log_info const *rli, thd->set_db(new_db.str, new_db.length); DBUG_ASSERT(thd->query == 0); thd->query_length= 0; // Should not be needed - thd->query_error= 0; + thd->is_slave_error= 0; clear_all_errors(thd, const_cast(rli)); /* see Query_log_event::do_apply_event() and BUG#13360 */ @@ -3429,7 +3429,7 @@ int Load_log_event::do_apply_event(NET* net, Relay_log_info const *rli, List tmp_list; if (mysql_load(thd, &ex, &tables, field_list, tmp_list, tmp_list, handle_dup, ignore, net != 0)) - thd->query_error= 1; + thd->is_slave_error= 1; if (thd->cuted_fields) { /* log_pos is the position of the LOAD event in the master log */ @@ -3468,9 +3468,9 @@ error: close_thread_tables(thd); DBUG_EXECUTE_IF("LOAD_DATA_INFILE_has_fatal_error", - thd->query_error= 0; thd->is_fatal_error= 1;); + thd->is_slave_error= 0; thd->is_fatal_error= 1;); - if (thd->query_error) + if (thd->is_slave_error) { /* this err/sql_errno code is copy-paste from net_send_error() */ const char *err; @@ -5655,7 +5655,7 @@ Rows_log_event::Rows_log_event(THD *thd_arg, TABLE *tbl_arg, ulong tid, m_width(tbl_arg ? tbl_arg->s->fields : 1), m_rows_buf(0), m_rows_cur(0), m_rows_end(0), m_flags(0) #ifdef HAVE_REPLICATION - ,m_key(NULL), m_curr_row(NULL), m_curr_row_end(NULL) + , m_curr_row(NULL), m_curr_row_end(NULL), m_key(NULL) #endif { /* @@ -5703,7 +5703,7 @@ Rows_log_event::Rows_log_event(const char *buf, uint event_len, #endif m_table_id(0), m_rows_buf(0), m_rows_cur(0), m_rows_end(0) #if !defined(MYSQL_CLIENT) && defined(HAVE_REPLICATION) - ,m_key(NULL), m_curr_row(NULL), m_curr_row_end(NULL) + , m_curr_row(NULL), m_curr_row_end(NULL), m_key(NULL) #endif { DBUG_ENTER("Rows_log_event::Rows_log_event(const char*,...)"); @@ -5951,7 +5951,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) { if (!need_reopen) { - if (thd->query_error || thd->is_fatal_error) + if (thd->is_slave_error || thd->is_fatal_error) { /* Error reporting borrowed from Query_log_event with many excessive @@ -5995,7 +5995,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) uint tables_count= rli->tables_to_lock_count; if ((error= open_tables(thd, &tables, &tables_count, 0))) { - if (thd->query_error || thd->is_fatal_error) + if (thd->is_slave_error || thd->is_fatal_error) { /* Error reporting borrowed from Query_log_event with many excessive @@ -6006,7 +6006,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) "Error '%s' on reopening tables", (actual_error ? thd->net.last_error : "unexpected success or fatal error")); - thd->query_error= 1; + thd->is_slave_error= 1; } const_cast(rli)->clear_tables_to_lock(); DBUG_RETURN(error); @@ -6029,7 +6029,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) { mysql_unlock_tables(thd, thd->lock); thd->lock= 0; - thd->query_error= 1; + thd->is_slave_error= 1; const_cast(rli)->clear_tables_to_lock(); DBUG_RETURN(ERR_BAD_TABLE_DEF); } @@ -6159,7 +6159,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) "Error in %s event: row application failed. %s", get_type_str(), thd->net.last_error ? thd->net.last_error : ""); - thd->query_error= 1; + thd->is_slave_error= 1; break; } @@ -6221,7 +6221,7 @@ int Rows_log_event::do_apply_event(Relay_log_info const *rli) */ thd->reset_current_stmt_binlog_row_based(); const_cast(rli)->cleanup_context(thd, error); - thd->query_error= 1; + thd->is_slave_error= 1; DBUG_RETURN(error); } @@ -6519,9 +6519,15 @@ Table_map_log_event::Table_map_log_event(THD *thd, TABLE *tbl, ulong tid, m_dblen(m_dbnam ? tbl->s->db.length : 0), m_tblnam(tbl->s->table_name.str), m_tbllen(tbl->s->table_name.length), - m_colcnt(tbl->s->fields), m_field_metadata(0), - m_field_metadata_size(0), m_memory(NULL), m_meta_memory(NULL), m_data_size(0), - m_table_id(tid), m_null_bits(0), m_flags(flags) + m_colcnt(tbl->s->fields), + m_memory(NULL), + m_table_id(tid), + m_flags(flags), + m_data_size(0), + m_field_metadata(0), + m_field_metadata_size(0), + m_null_bits(0), + m_meta_memory(NULL) { DBUG_ASSERT(m_table_id != ~0UL); /* @@ -6798,7 +6804,7 @@ int Table_map_log_event::do_apply_event(Relay_log_info const *rli) TABLE_LIST *tmp_table_list= table_list; if ((error= open_tables(thd, &tmp_table_list, &count, 0))) { - if (thd->query_error || thd->is_fatal_error) + if (thd->is_slave_error || thd->is_fatal_error) { /* Error reporting borrowed from Query_log_event with many excessive @@ -6810,7 +6816,7 @@ int Table_map_log_event::do_apply_event(Relay_log_info const *rli) (actual_error ? thd->net.last_error : "unexpected success or fatal error"), table_list->db, table_list->table_name); - thd->query_error= 1; + thd->is_slave_error= 1; } goto err; } diff --git a/sql/log_event_old.cc b/sql/log_event_old.cc index 949179386ea..c6b691ec010 100644 --- a/sql/log_event_old.cc +++ b/sql/log_event_old.cc @@ -68,7 +68,7 @@ Old_rows_log_event::do_apply_event(Rows_log_event *ev, const Relay_log_info *rli { if (!need_reopen) { - if (thd->query_error || thd->is_fatal_error) + if (thd->is_slave_error || thd->is_fatal_error) { /* Error reporting borrowed from Query_log_event with many excessive @@ -112,7 +112,7 @@ Old_rows_log_event::do_apply_event(Rows_log_event *ev, const Relay_log_info *rli uint tables_count= rli->tables_to_lock_count; if ((error= open_tables(thd, &tables, &tables_count, 0))) { - if (thd->query_error || thd->is_fatal_error) + if (thd->is_slave_error || thd->is_fatal_error) { /* Error reporting borrowed from Query_log_event with many excessive @@ -123,7 +123,7 @@ Old_rows_log_event::do_apply_event(Rows_log_event *ev, const Relay_log_info *rli "Error '%s' on reopening tables", (actual_error ? thd->net.last_error : "unexpected success or fatal error")); - thd->query_error= 1; + thd->is_slave_error= 1; } const_cast(rli)->clear_tables_to_lock(); DBUG_RETURN(error); @@ -146,7 +146,7 @@ Old_rows_log_event::do_apply_event(Rows_log_event *ev, const Relay_log_info *rli { mysql_unlock_tables(thd, thd->lock); thd->lock= 0; - thd->query_error= 1; + thd->is_slave_error= 1; const_cast(rli)->clear_tables_to_lock(); DBUG_RETURN(Rows_log_event::ERR_BAD_TABLE_DEF); } @@ -255,7 +255,7 @@ Old_rows_log_event::do_apply_event(Rows_log_event *ev, const Relay_log_info *rli "Error in %s event: row application failed. %s", ev->get_type_str(), thd->net.last_error ? thd->net.last_error : ""); - thd->query_error= 1; + thd->is_slave_error= 1; break; } @@ -300,7 +300,7 @@ Old_rows_log_event::do_apply_event(Rows_log_event *ev, const Relay_log_info *rli */ thd->reset_current_stmt_binlog_row_based(); const_cast(rli)->cleanup_context(thd, error); - thd->query_error= 1; + thd->is_slave_error= 1; DBUG_RETURN(error); } diff --git a/sql/mysqld.cc b/sql/mysqld.cc index a355c560996..cc964e417bf 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -2593,7 +2593,7 @@ int my_message_sql(uint error, const char *str, myf MyFlags) DBUG_RETURN(0); } - thd->query_error= 1; // needed to catch query errors during replication + thd->is_slave_error= 1; // needed to catch query errors during replication if (!thd->no_warnings_for_error) push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, error, str); diff --git a/sql/protocol.cc b/sql/protocol.cc index 9d473912ba3..31d23ec94dd 100644 --- a/sql/protocol.cc +++ b/sql/protocol.cc @@ -84,7 +84,7 @@ void net_send_error(THD *thd, uint sql_errno, const char *err) DBUG_VOID_RETURN; } - thd->query_error= 1; // needed to catch query errors during replication + thd->is_slave_error= 1; // needed to catch query errors during replication if (!err) { if (sql_errno) @@ -162,7 +162,7 @@ net_printf_error(THD *thd, uint errcode, ...) DBUG_VOID_RETURN; } - thd->query_error= 1; // needed to catch query errors during replication + thd->is_slave_error= 1; // needed to catch query errors during replication #ifndef EMBEDDED_LIBRARY query_cache_abort(net); // Safety #endif diff --git a/sql/slave.cc b/sql/slave.cc index fcbd4eb841b..494e13d8c9f 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -980,7 +980,7 @@ static int create_table_from_dump(THD* thd, MYSQL *mysql, const char* db, DBUG_RETURN(1); } thd->query= query; - thd->query_error = 0; + thd->is_slave_error = 0; thd->net.no_send_ok = 1; bzero((char*) &tables,sizeof(tables)); @@ -1009,7 +1009,7 @@ static int create_table_from_dump(THD* thd, MYSQL *mysql, const char* db, thd->db_length= save_db_length; thd->options = save_options; - if (thd->query_error) + if (thd->is_slave_error) goto err; // mysql_parse took care of the error send thd->proc_info = "Opening master dump table"; @@ -2501,7 +2501,7 @@ log '%s' at position %s, relay log '%s' position: %s", RPL_LOG_NAME, if (sys_init_slave.value_length) { execute_init_command(thd, &sys_init_slave, &LOCK_sys_init_slave); - if (thd->query_error) + if (thd->is_slave_error) { sql_print_error("\ Slave SQL thread aborted. Can't execute init_slave query"); diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 093544cdd0a..f7ab9bac3b1 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -1108,7 +1108,7 @@ sp_head::execute(THD *thd) if ((ctx= thd->spcont)) ctx->clear_handler(); - thd->query_error= 0; + thd->is_slave_error= 0; old_arena= thd->stmt_arena; /* @@ -1275,8 +1275,8 @@ sp_head::execute(THD *thd) state= EXECUTED; done: - DBUG_PRINT("info", ("err_status: %d killed: %d query_error: %d report_error: %d", - err_status, thd->killed, thd->query_error, + DBUG_PRINT("info", ("err_status: %d killed: %d is_slave_error: %d report_error: %d", + err_status, thd->killed, thd->is_slave_error, thd->net.report_error)); if (thd->killed) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index ffbf0649961..8c02fdb8289 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -395,7 +395,7 @@ THD::THD() count_cuted_fields= CHECK_FIELD_IGNORE; killed= NOT_KILLED; col_access=0; - query_error= thread_specific_used= FALSE; + is_slave_error= thread_specific_used= FALSE; hash_clear(&handler_tables_hash); tmp_table=0; used_tables=0; diff --git a/sql/sql_class.h b/sql/sql_class.h index 97a63ed9448..5aec68e1c61 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1466,7 +1466,14 @@ public: /* for IS NULL => = last_insert_id() fix in remove_eq_conds() */ bool substitute_null_with_insert_id; bool in_lock_tables; - bool query_error, bootstrap, cleanup_done; + /** + True if a slave error. Causes the slave to stop. Not the same + as the statement execution error (net.report_error), since + a statement may be expected to return an error, e.g. because + it returned an error on master, and this is OK on the slave. + */ + bool is_slave_error; + bool bootstrap, cleanup_done; /** is set if some thread specific value(s) used in a statement. */ bool thread_specific_used; @@ -1695,7 +1702,7 @@ public: net.last_error[0]= 0; net.last_errno= 0; net.report_error= 0; - query_error= 0; + is_slave_error= 0; DBUG_VOID_RETURN; } inline bool vio_ok() const { return net.vio != 0; } diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index 094bef9324e..afd97c27a55 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -1030,7 +1030,7 @@ static void prepare_new_connection_state(THD* thd) if (sys_init_connect.value_length && !(sctx->master_access & SUPER_ACL)) { execute_init_command(thd, &sys_init_connect, &LOCK_sys_init_connect); - if (thd->query_error) + if (thd->net.report_error) { thd->killed= THD::KILL_CONNECTION; sql_print_warning(ER(ER_NEW_ABORTING_CONNECTION), From d1ddc24a1ae8bff060ac721f94c6948010a04642 Mon Sep 17 00:00:00 2001 From: unknown Date: Sat, 20 Oct 2007 21:48:15 +0400 Subject: [PATCH 03/31] Fix for BUG#31148: bool close_thread_table(THD*, TABLE**): Assertion `table->key_read == 0' failed. The problem was that key_read on a table in a sub-select was not properly reset. That happens because the code responsible for that is copy&pasted all around the server. In some place, it was obviously forgotten to be pasted. The fix is to reset key_read properly. mysql-test/r/key.result: Update result file. mysql-test/t/key.test: A test case for BUG#31148: bool close_thread_table(THD*, TABLE**): Assertion `table->key_read == 0' failed. sql/sql_select.cc: Reset key_read before closing index. --- mysql-test/r/key.result | 15 +++++++++++++++ mysql-test/t/key.test | 23 +++++++++++++++++++++++ sql/sql_select.cc | 9 ++++++++- 3 files changed, 46 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/key.result b/mysql-test/r/key.result index a2bed75a709..6c115435fb6 100644 --- a/mysql-test/r/key.result +++ b/mysql-test/r/key.result @@ -530,3 +530,18 @@ ORDER BY c.b, c.d a b c d e f g h i j a b c d 2 2 1 2004-11-30 12:00:00 1 0 0 0 0 0 2 3388000 -553000 NULL DROP TABLE t1, t2; +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (a INT PRIMARY KEY AUTO_INCREMENT); +INSERT INTO t1 VALUES (), (), (); +SELECT 1 AS c1 +FROM t1 +ORDER BY ( +SELECT 1 AS c2 +FROM t1 +GROUP BY GREATEST(LAST_INSERT_ID(), t1.a) ASC +LIMIT 1); +c1 +1 +1 +1 +DROP TABLE t1; diff --git a/mysql-test/t/key.test b/mysql-test/t/key.test index f1eb8e68b49..cd6c480407d 100644 --- a/mysql-test/t/key.test +++ b/mysql-test/t/key.test @@ -501,3 +501,26 @@ ORDER BY c.b, c.d ; DROP TABLE t1, t2; + +# +# Bug #31148: bool close_thread_table(THD*, TABLE**): Assertion +# `table->key_read == 0' failed. +# + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (a INT PRIMARY KEY AUTO_INCREMENT); + +INSERT INTO t1 VALUES (), (), (); + +SELECT 1 AS c1 +FROM t1 +ORDER BY ( + SELECT 1 AS c2 + FROM t1 + GROUP BY GREATEST(LAST_INSERT_ID(), t1.a) ASC + LIMIT 1); + +DROP TABLE t1; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index eb2731205ec..d041b2edcfa 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -6651,7 +6651,14 @@ void JOIN::cleanup(bool full) for (tab= join_tab, end= tab+tables; tab != end; tab++) { if (tab->table) - tab->table->file->ha_index_or_rnd_end(); + { + if (tab->table->key_read) + { + tab->table->key_read= 0; + tab->table->file->extra(HA_EXTRA_NO_KEYREAD); + } + tab->table->file->ha_index_or_rnd_end(); + } } } } From 8c98e5e1808725722ac9b355890b41280310331b Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 22 Oct 2007 23:02:05 +0400 Subject: [PATCH 04/31] WL#4104: Deprecate the Instance Manager. A deprecation warning added. server-tools/instance-manager/mysqlmanager.cc: Add a deprecation warning. --- server-tools/instance-manager/mysqlmanager.cc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server-tools/instance-manager/mysqlmanager.cc b/server-tools/instance-manager/mysqlmanager.cc index 6d6ebbee57d..276d1ca3b49 100644 --- a/server-tools/instance-manager/mysqlmanager.cc +++ b/server-tools/instance-manager/mysqlmanager.cc @@ -79,6 +79,9 @@ int main(int argc, char *argv[]) { int return_value; + puts("\n" + "WARNING: This program is deprecated and will be removed in 6.0.\n"); + /* Initialize. */ MY_INIT(argv[0]); From 22e972ffeb8af1f6bc100eea38fb15483805bde8 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 23 Oct 2007 09:05:39 -0300 Subject: [PATCH 05/31] Bug#31669 Buffer overflow in mysql_change_user() The problem is that when copying the supplied username and database, no bounds checking is performed on the fixed-length buffer. A sufficiently large (> 512) user string can easily cause stack corruption. Since this API can be used from PHP and other programs, this is a serious problem. The solution is to increase the buffer size to the accepted size in similar functions and perform bounds checking when copying the username and database. libmysql/libmysql.c: Increase the buffer size and perform bounds checking when copying the supplied arguments. tests/mysql_client_test.c: Add test case for Bug#31669 --- libmysql/libmysql.c | 7 +-- tests/mysql_client_test.c | 94 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+), 3 deletions(-) diff --git a/libmysql/libmysql.c b/libmysql/libmysql.c index c7bdfc4c42c..5c015bd6b0f 100644 --- a/libmysql/libmysql.c +++ b/libmysql/libmysql.c @@ -706,7 +706,8 @@ int cli_read_change_user_result(MYSQL *mysql, char *buff, const char *passwd) my_bool STDCALL mysql_change_user(MYSQL *mysql, const char *user, const char *passwd, const char *db) { - char buff[512],*end=buff; + char buff[USERNAME_LENGTH+SCRAMBLED_PASSWORD_CHAR_LENGTH+NAME_LEN+2]; + char *end= buff; int rc; DBUG_ENTER("mysql_change_user"); @@ -716,7 +717,7 @@ my_bool STDCALL mysql_change_user(MYSQL *mysql, const char *user, passwd=""; /* Store user into the buffer */ - end=strmov(end,user)+1; + end= strmake(end, user, USERNAME_LENGTH) + 1; /* write scrambled password according to server capabilities */ if (passwd[0]) @@ -736,7 +737,7 @@ my_bool STDCALL mysql_change_user(MYSQL *mysql, const char *user, else *end++= '\0'; /* empty password */ /* Add database if needed */ - end= strmov(end, db ? db : "") + 1; + end= strmake(end, db ? db : "", NAME_LEN) + 1; /* Write authentication package */ simple_command(mysql,COM_CHANGE_USER, buff,(ulong) (end-buff),1); diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 9962a108e45..59f0150aa7a 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -15857,6 +15857,99 @@ static void test_bug29306() DBUG_VOID_RETURN; } + +/** + Bug#31669 Buffer overflow in mysql_change_user() +*/ + +#define LARGE_BUFFER_SIZE 2048 + +static void test_bug31669() +{ + int rc; + static char buff[LARGE_BUFFER_SIZE+1]; +#ifndef EMBEDDED_LIBRARY + static char user[USERNAME_LENGTH+1]; + static char db[NAME_LEN+1]; + static char query[LARGE_BUFFER_SIZE*2]; +#endif + + DBUG_ENTER("test_bug31669"); + myheader("test_bug31669"); + + rc= mysql_change_user(mysql, NULL, NULL, NULL); + DIE_UNLESS(rc); + + rc= mysql_change_user(mysql, "", "", ""); + DIE_UNLESS(rc); + + memset(buff, 'a', sizeof(buff)); + + rc= mysql_change_user(mysql, buff, buff, buff); + DIE_UNLESS(rc); + + rc = mysql_change_user(mysql, opt_user, opt_password, current_db); + DIE_UNLESS(!rc); + +#ifndef EMBEDDED_LIBRARY + memset(db, 'a', sizeof(db)); + db[NAME_LEN]= 0; + strxmov(query, "CREATE DATABASE IF NOT EXISTS ", db, NullS); + rc= mysql_query(mysql, query); + myquery(rc); + + memset(user, 'b', sizeof(user)); + user[USERNAME_LENGTH]= 0; + memset(buff, 'c', sizeof(buff)); + buff[LARGE_BUFFER_SIZE]= 0; + strxmov(query, "GRANT ALL PRIVILEGES ON *.* TO '", user, "'@'%' IDENTIFIED BY " + "'", buff, "' WITH GRANT OPTION", NullS); + rc= mysql_query(mysql, query); + myquery(rc); + + rc= mysql_query(mysql, "FLUSH PRIVILEGES"); + myquery(rc); + + rc= mysql_change_user(mysql, user, buff, db); + DIE_UNLESS(!rc); + + user[USERNAME_LENGTH-1]= 'a'; + rc= mysql_change_user(mysql, user, buff, db); + DIE_UNLESS(rc); + + user[USERNAME_LENGTH-1]= 'b'; + buff[LARGE_BUFFER_SIZE-1]= 'd'; + rc= mysql_change_user(mysql, user, buff, db); + DIE_UNLESS(rc); + + buff[LARGE_BUFFER_SIZE-1]= 'c'; + db[NAME_LEN-1]= 'e'; + rc= mysql_change_user(mysql, user, buff, db); + DIE_UNLESS(rc); + + db[NAME_LEN-1]= 'a'; + rc= mysql_change_user(mysql, user, buff, db); + DIE_UNLESS(!rc); + + rc= mysql_change_user(mysql, user + 1, buff + 1, db + 1); + DIE_UNLESS(rc); + + rc = mysql_change_user(mysql, opt_user, opt_password, current_db); + DIE_UNLESS(!rc); + + strxmov(query, "DROP DATABASE ", db, NullS); + rc= mysql_query(mysql, query); + myquery(rc); + + strxmov(query, "DELETE FROM mysql.user WHERE User='", user, "'", NullS); + rc= mysql_query(mysql, query); + myquery(rc); + DIE_UNLESS(mysql_affected_rows(mysql) == 1); +#endif + + DBUG_VOID_RETURN; +} + /* Read and parse arguments and MySQL options from my.cnf */ @@ -16149,6 +16242,7 @@ static struct my_tests_st my_tests[]= { { "test_bug27592", test_bug27592 }, { "test_bug29948", test_bug29948 }, { "test_bug29306", test_bug29306 }, + { "test_bug31669", test_bug31669 }, { 0, 0 } }; From 6044965c4fe14785eb7294c68067486f7451a85f Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 23 Oct 2007 18:03:51 +0400 Subject: [PATCH 06/31] Patch for BUG#30736: Row Size Too Large Error Creating a Table and Inserting Data. The problem was that under some circumstances Field class was not properly initialized before calling create_length_to_internal_length() function, which led to assert failure. The fix is to do the proper initialization. The user-visible problem was that under some circumstances CREATE TABLE ... SELECT statement crashed the server or led to wrong error message (wrong results). mysql-test/r/select.result: Update result file. mysql-test/t/select.test: Add a test case for BUG#30736: Row Size Too Large Error Creating a Table and Inserting Data. sql/sql_table.cc: Move sql_field->decimals initialization before sql_field->create_length_to_internal_length() call. --- mysql-test/r/select.result | 35 +++++++++++++++++++++++++++ mysql-test/t/select.test | 48 ++++++++++++++++++++++++++++++++++++++ sql/sql_table.cc | 2 +- 3 files changed, 84 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/select.result b/mysql-test/r/select.result index ed120a1bbb8..76022053702 100644 --- a/mysql-test/r/select.result +++ b/mysql-test/r/select.result @@ -4096,4 +4096,39 @@ SELECT `x` FROM v3; x 1 DROP VIEW v1, v2, v3; + +# +# Bug#30736: Row Size Too Large Error Creating a Table and +# Inserting Data. +# +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; + +CREATE TABLE t1( +c1 DECIMAL(10, 2), +c2 FLOAT); + +INSERT INTO t1 VALUES (0, 1), (2, 3), (4, 5); + +CREATE TABLE t2( +c3 DECIMAL(10, 2)) +SELECT +c1 * c2 AS c3 +FROM t1; + +SELECT * FROM t1; +c1 c2 +0.00 1 +2.00 3 +4.00 5 + +SELECT * FROM t2; +c3 +0.00 +6.00 +20.00 + +DROP TABLE t1; +DROP TABLE t2; + End of 5.0 tests diff --git a/mysql-test/t/select.test b/mysql-test/t/select.test index 5c30a17e08e..6deb951c4e8 100644 --- a/mysql-test/t/select.test +++ b/mysql-test/t/select.test @@ -3484,4 +3484,52 @@ DROP VIEW v1, v2, v3; --enable_ps_protocol +########################################################################### + +--echo +--echo # +--echo # Bug#30736: Row Size Too Large Error Creating a Table and +--echo # Inserting Data. +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +DROP TABLE IF EXISTS t2; +--enable_warnings + +--echo + +CREATE TABLE t1( + c1 DECIMAL(10, 2), + c2 FLOAT); + +--echo + +INSERT INTO t1 VALUES (0, 1), (2, 3), (4, 5); + +--echo + +CREATE TABLE t2( + c3 DECIMAL(10, 2)) + SELECT + c1 * c2 AS c3 + FROM t1; + +--echo + +SELECT * FROM t1; + +--echo + +SELECT * FROM t2; + +--echo + +DROP TABLE t1; +DROP TABLE t2; + +--echo + +########################################################################### + --echo End of 5.0 tests diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 6cbe98fe862..b5628ab011b 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -955,8 +955,8 @@ static int mysql_prepare_table(THD *thd, HA_CREATE_INFO *create_info, sql_field->length= dup_field->char_length; sql_field->pack_length= dup_field->pack_length; sql_field->key_length= dup_field->key_length; - sql_field->create_length_to_internal_length(); sql_field->decimals= dup_field->decimals; + sql_field->create_length_to_internal_length(); sql_field->unireg_check= dup_field->unireg_check; /* We're making one field from two, the result field will have From 35ca78a0d449e9c8260a5a9d1d1ade0c6e8fbb80 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 24 Oct 2007 19:01:08 -0600 Subject: [PATCH 07/31] Bug#30854 (Tables name show as binary in slave err msg on vm-win2003-64-b) The root cause of this defect is that a call to my_error() is using a 'LEX_STRING' parameter instead of a 'char*' This patch fixes the failing calls to my_error(), as well as similar calls found during investigation. This is a compiling bug (see the instrumentation in the bug report), no test cases provided. sql/sql_base.cc: Fix broken calls to "..." (va_args) functions. sql/sql_table.cc: Fix broken calls to "..." (va_args) functions. --- sql/sql_base.cc | 2 +- sql/sql_table.cc | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index c4e90165ced..6307564c14f 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -7393,7 +7393,7 @@ open_new_frm(THD *thd, TABLE_SHARE *share, const char *alias, else { /* only VIEWs are supported now */ - my_error(ER_FRM_UNKNOWN_TYPE, MYF(0), share->path, parser->type()->str); + my_error(ER_FRM_UNKNOWN_TYPE, MYF(0), share->path.str, parser->type()->str); goto err; } DBUG_RETURN(0); diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 462b944e635..64d739aae7e 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -5200,7 +5200,8 @@ bool alter_table_manage_keys(TABLE *table, int indexes_were_disabled, if (error == HA_ERR_WRONG_COMMAND) { push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_NOTE, - ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA), table->s->table_name); + ER_ILLEGAL_HA, ER(ER_ILLEGAL_HA), + table->s->table_name.str); error= 0; } else if (error) table->file->print_error(error, MYF(0)); @@ -5392,7 +5393,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, { if (def->change && ! def->field) { - my_error(ER_BAD_FIELD_ERROR, MYF(0), def->change, table->s->table_name); + my_error(ER_BAD_FIELD_ERROR, MYF(0), def->change, table->s->table_name.str); goto err; } /* @@ -5427,7 +5428,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, } if (!find) { - my_error(ER_BAD_FIELD_ERROR, MYF(0), def->after, table->s->table_name); + my_error(ER_BAD_FIELD_ERROR, MYF(0), def->after, table->s->table_name.str); goto err; } find_it.after(def); // Put element after this @@ -5437,7 +5438,7 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, if (alter_info->alter_list.elements) { my_error(ER_BAD_FIELD_ERROR, MYF(0), - alter_info->alter_list.head()->name, table->s->table_name); + alter_info->alter_list.head()->name, table->s->table_name.str); goto err; } if (!new_create_list.elements) From 7c92f118e4fbfa5e96a9261ef18c59e74e650ce7 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 29 Oct 2007 15:42:49 +0300 Subject: [PATCH 08/31] Fix for BUG#27610: ALTER TABLE ROW_FORMAT=... does not rebuild the table. The problem was that ROW_FORMAT clause in ALTER TABLE did not trigger table reconstruction. The fix is to rebuild a table if ROW_FORMAT is specified. mysql-test/include/mix1.inc: Add a test case for BUG#27610: ALTER TABLE ROW_FORMAT=... does not rebuild the table. mysql-test/r/innodb_mysql.result: Update result file. sql/sql_table.cc: Rebuild a table if ROW_FORMAT was specified in ALTER TABLE. --- mysql-test/include/mix1.inc | 49 ++++++++++++++++++++++++++++++++ mysql-test/r/innodb_mysql.result | 34 ++++++++++++++++++++++ sql/sql_table.cc | 1 + 3 files changed, 84 insertions(+) diff --git a/mysql-test/include/mix1.inc b/mysql-test/include/mix1.inc index aee5613ff35..df9d79157af 100644 --- a/mysql-test/include/mix1.inc +++ b/mysql-test/include/mix1.inc @@ -1019,6 +1019,55 @@ SELECT * FROM t1 ORDER BY b DESC, a ASC; DROP TABLE t1; +########################################################################### + +--echo +--echo # +--echo # Bug#27610: ALTER TABLE ROW_FORMAT=... does not rebuild the table. +--echo # + +--echo +--echo # - prepare; +--echo + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +--echo + +CREATE TABLE t1(c INT) + ENGINE = InnoDB + ROW_FORMAT = COMPACT; + +--echo +--echo # - initial check; +--echo + +SELECT table_schema, table_name, row_format +FROM INFORMATION_SCHEMA.TABLES +WHERE table_schema = DATABASE() AND table_name = 't1'; + +--echo +--echo # - change ROW_FORMAT and check; +--echo + +ALTER TABLE t1 ROW_FORMAT = REDUNDANT; + +--echo + +SELECT table_schema, table_name, row_format +FROM INFORMATION_SCHEMA.TABLES +WHERE table_schema = DATABASE() AND table_name = 't1'; + +--echo +--echo # - that's it, cleanup. +--echo + +DROP TABLE t1; + +########################################################################### + --echo End of 5.0 tests # Fix for BUG#19243 "wrong LAST_INSERT_ID() after ON DUPLICATE KEY diff --git a/mysql-test/r/innodb_mysql.result b/mysql-test/r/innodb_mysql.result index 3a6758b38f4..2a0f9a930b8 100644 --- a/mysql-test/r/innodb_mysql.result +++ b/mysql-test/r/innodb_mysql.result @@ -1286,6 +1286,40 @@ a b 2 2 3 2 1 1 +DROP TABLE t1; + +# +# Bug#27610: ALTER TABLE ROW_FORMAT=... does not rebuild the table. +# + +# - prepare; + +DROP TABLE IF EXISTS t1; + +CREATE TABLE t1(c INT) +ENGINE = InnoDB +ROW_FORMAT = COMPACT; + +# - initial check; + +SELECT table_schema, table_name, row_format +FROM INFORMATION_SCHEMA.TABLES +WHERE table_schema = DATABASE() AND table_name = 't1'; +table_schema table_name row_format +test t1 Compact + +# - change ROW_FORMAT and check; + +ALTER TABLE t1 ROW_FORMAT = REDUNDANT; + +SELECT table_schema, table_name, row_format +FROM INFORMATION_SCHEMA.TABLES +WHERE table_schema = DATABASE() AND table_name = 't1'; +table_schema table_name row_format +test t1 Redundant + +# - that's it, cleanup. + DROP TABLE t1; End of 5.0 tests CREATE TABLE `t2` ( diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 64d739aae7e..2bd89d4a0ae 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4985,6 +4985,7 @@ compare_tables(TABLE *table, create_info->used_fields & HA_CREATE_USED_ENGINE || create_info->used_fields & HA_CREATE_USED_CHARSET || create_info->used_fields & HA_CREATE_USED_DEFAULT_CHARSET || + create_info->used_fields & HA_CREATE_USED_ROW_FORMAT || (alter_info->flags & (ALTER_RECREATE | ALTER_FOREIGN_KEY)) || order_num || !table->s->mysql_version || From 5f4783cf4ba129388cd175020db80311759ccefe Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 30 Oct 2007 15:25:39 +0300 Subject: [PATCH 09/31] Make sure rpl.rpl_innodb_mixed_dml passes even if rpl_mixed.dat is read-only. This is important for a development environment where not all source files are checked out. --- mysql-test/suite/rpl/include/rpl_mixed_dml.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql-test/suite/rpl/include/rpl_mixed_dml.inc b/mysql-test/suite/rpl/include/rpl_mixed_dml.inc index 96dfdbed541..a3ff022c43c 100644 --- a/mysql-test/suite/rpl/include/rpl_mixed_dml.inc +++ b/mysql-test/suite/rpl/include/rpl_mixed_dml.inc @@ -53,7 +53,7 @@ DELETE FROM t2 WHERE a = 2; --echo ******************** LOAD DATA INFILE ******************** --exec cp ./suite/rpl/data/rpl_mixed.dat $MYSQLTEST_VARDIR/tmp/ LOAD DATA INFILE '../tmp/rpl_mixed.dat' INTO TABLE t1 FIELDS TERMINATED BY '|' ; ---exec rm $MYSQLTEST_VARDIR/tmp/rpl_mixed.dat +--remove_file $MYSQLTEST_VARDIR/tmp/rpl_mixed.dat SELECT * FROM t1; --source suite/rpl/include/rpl_mixed_check_select.inc --source suite/rpl/include/rpl_mixed_clear_tables.inc From d4d4ba9a9f49a6abc9cd00924e1e33bfd2278b87 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 30 Oct 2007 15:39:50 +0300 Subject: [PATCH 10/31] Fix failing init_connect.test (5.1-runtime). sql/sql_connect.cc: Fix failing init_connect.test (5.1-runtime). Add comments for the unjustified use of thd->is_slave_error. --- sql/sql_connect.cc | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index afd97c27a55..c4367b2a8fb 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -1030,7 +1030,17 @@ static void prepare_new_connection_state(THD* thd) if (sys_init_connect.value_length && !(sctx->master_access & SUPER_ACL)) { execute_init_command(thd, &sys_init_connect, &LOCK_sys_init_connect); - if (thd->net.report_error) + /* + execute_init_command calls net_send_error. + If there was an error during execution of the init statements, + the error at this moment is present in thd->net.last_error and also + thd->is_slave_error and thd->net.report_error are set. + net_send_error sends the contents of thd->net.last_error and + clears thd->net.report_error. It doesn't, however, clean + thd->is_slave_error or thd->net.last_error. Here we make use of this + fact. + */ + if (thd->is_slave_error) { thd->killed= THD::KILL_CONNECTION; sql_print_warning(ER(ER_NEW_ABORTING_CONNECTION), From 2437001234eba37ad821be11052e57f04293d5bd Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 30 Oct 2007 20:08:16 +0300 Subject: [PATCH 11/31] Use an inline getter method (thd->is_error()) to query if there is an error in THD. In future the error may be stored elsewhere (not in net.report_error) and it's important to start using an opaque getter to simplify merges. sql/filesort.cc: net.report_error -> is_error() sql/ha_ndbcluster_binlog.cc: net.report_error -> is_error() sql/item_func.cc: net.report_error -> is_error() sql/item_subselect.cc: net.report_error -> is_error() sql/set_var.cc: net.report_error -> is_error() sql/sp.cc: net.report_error -> is_error() sql/sp_head.cc: net.report_error -> is_error() sql/sql_base.cc: net.report_error -> is_error() sql/sql_class.cc: net.report_error -> is_error() sql/sql_class.h: net.report_error -> is_error() sql/sql_connect.cc: net.report_error -> is_error() sql/sql_delete.cc: net.report_error -> is_error() sql/sql_insert.cc: net.report_error -> is_error() sql/sql_parse.cc: net.report_error -> is_error() sql/sql_prepare.cc: net.report_error -> is_error() sql/sql_select.cc: net.report_error -> is_error() sql/sql_union.cc: net.report_error -> is_error() sql/sql_update.cc: net.report_error -> is_error() sql/sql_view.cc: net.report_error -> is_error() sql/sql_yacc.yy: net.report_error -> is_error() --- sql/filesort.cc | 4 ++-- sql/ha_ndbcluster_binlog.cc | 2 +- sql/item_func.cc | 4 ++-- sql/item_subselect.cc | 2 +- sql/set_var.cc | 2 +- sql/sp.cc | 2 +- sql/sp_head.cc | 6 +++--- sql/sql_base.cc | 10 +++++----- sql/sql_class.cc | 4 ++-- sql/sql_class.h | 16 +++++++++++++++- sql/sql_connect.cc | 6 +++--- sql/sql_delete.cc | 8 ++++---- sql/sql_insert.cc | 12 ++++++------ sql/sql_parse.cc | 22 +++++++++++----------- sql/sql_prepare.cc | 2 +- sql/sql_select.cc | 30 +++++++++++++++--------------- sql/sql_union.cc | 2 +- sql/sql_update.cc | 6 +++--- sql/sql_view.cc | 6 +++--- sql/sql_yacc.yy | 4 ++-- 20 files changed, 82 insertions(+), 68 deletions(-) diff --git a/sql/filesort.cc b/sql/filesort.cc index c074f90e780..2e6f0ecaf05 100644 --- a/sql/filesort.cc +++ b/sql/filesort.cc @@ -555,7 +555,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, else file->unlock_row(); /* It does not make sense to read more keys in case of a fatal error */ - if (thd->net.report_error) + if (thd->is_error()) break; } if (quick_select) @@ -573,7 +573,7 @@ static ha_rows find_all_keys(SORTPARAM *param, SQL_SELECT *select, file->ha_rnd_end(); } - if (thd->net.report_error) + if (thd->is_error()) DBUG_RETURN(HA_POS_ERROR); /* Signal we should use orignal column read and write maps */ diff --git a/sql/ha_ndbcluster_binlog.cc b/sql/ha_ndbcluster_binlog.cc index c22d5ac53f5..fc35a7a930e 100644 --- a/sql/ha_ndbcluster_binlog.cc +++ b/sql/ha_ndbcluster_binlog.cc @@ -271,7 +271,7 @@ static void run_query(THD *thd, char *buf, char *end, sql_print_error("NDB: %s: error %s %d(ndb: %d) %d %d", buf, thd->net.last_error, thd->net.last_errno, thd_ndb->m_error_code, - thd->net.report_error, thd->is_slave_error); + (int) thd->is_error(), thd->is_slave_error); } thd->options= save_thd_options; diff --git a/sql/item_func.cc b/sql/item_func.cc index 64574a9ad74..2d4d61de61b 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -187,7 +187,7 @@ Item_func::fix_fields(THD *thd, Item **ref) } } fix_length_and_dec(); - if (thd->net.report_error) // An error inside fix_length_and_dec occured + if (thd->is_error()) // An error inside fix_length_and_dec occured return TRUE; fixed= 1; return FALSE; @@ -4073,7 +4073,7 @@ my_decimal *user_var_entry::val_decimal(my_bool *null_value, my_decimal *val) NOTES For now it always return OK. All problem with value evaluating - will be caught by thd->net.report_error check in sql_set_variables(). + will be caught by thd->is_error() check in sql_set_variables(). RETURN FALSE OK. diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 843c6ced263..8eb7421f854 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -248,7 +248,7 @@ bool Item_subselect::exec() { int res; - if (thd->net.report_error) + if (thd->is_error()) /* Do not execute subselect in case of a fatal error */ return 1; diff --git a/sql/set_var.cc b/sql/set_var.cc index 138c71fa559..11874b1020c 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -3051,7 +3051,7 @@ int sql_set_variables(THD *thd, List *var_list) if ((error= var->check(thd))) goto err; } - if (!(error= test(thd->net.report_error))) + if (!(error= test(thd->is_error()))) { it.rewind(); while ((var= it++)) diff --git a/sql/sp.cc b/sql/sp.cc index 1b5d8ca87b8..b4f0ec7729b 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -1866,7 +1866,7 @@ sp_cache_routines_and_add_tables_aux(THD *thd, LEX *lex, an error with it's return value without calling my_error(), we set the generic "mysql.proc table corrupt" error here. */ - if (!thd->net.report_error) + if (! thd->is_error()) { /* SP allows full NAME_LEN chars thus he have to allocate enough diff --git a/sql/sp_head.cc b/sql/sp_head.cc index f7ab9bac3b1..6e8749aa745 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -370,7 +370,7 @@ sp_eval_expr(THD *thd, Field *result_field, Item **expr_item_ptr) thd->abort_on_warning= save_abort_on_warning; thd->transaction.stmt.modified_non_trans_table= save_stmt_modified_non_trans_table; - if (thd->net.report_error) + if (thd->is_error()) { /* Return error status if something went wrong. */ err_status= TRUE; @@ -1277,7 +1277,7 @@ sp_head::execute(THD *thd) done: DBUG_PRINT("info", ("err_status: %d killed: %d is_slave_error: %d report_error: %d", err_status, thd->killed, thd->is_slave_error, - thd->net.report_error)); + thd->is_error())); if (thd->killed) err_status= TRUE; @@ -2673,7 +2673,7 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, cleanup_items() is called in sp_head::execute() */ - DBUG_RETURN(res || thd->net.report_error); + DBUG_RETURN(res || thd->is_error()); } diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 10e6716cadf..223485f1e42 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -6260,7 +6260,7 @@ bool setup_fields(THD *thd, Item **ref_pointer_array, thd->lex->allow_sum_func= save_allow_sum_func; thd->mark_used_columns= save_mark_used_columns; DBUG_PRINT("info", ("thd->mark_used_columns: %d", thd->mark_used_columns)); - DBUG_RETURN(test(thd->net.report_error)); + DBUG_RETURN(test(thd->is_error())); } @@ -6804,7 +6804,7 @@ int setup_conds(THD *thd, TABLE_LIST *tables, TABLE_LIST *leaves, select_lex->conds_processed_with_permanent_arena= 1; } thd->lex->current_select->is_item_list_lookup= save_is_item_list_lookup; - DBUG_RETURN(test(thd->net.report_error)); + DBUG_RETURN(test(thd->is_error())); err_no_arena: select_lex->is_item_list_lookup= save_is_item_list_lookup; @@ -6886,7 +6886,7 @@ fill_record(THD * thd, List &fields, List &values, goto err; } } - DBUG_RETURN(thd->net.report_error); + DBUG_RETURN(thd->is_error()); err: if (table) table->auto_increment_field_not_null= FALSE; @@ -6971,7 +6971,7 @@ fill_record(THD *thd, Field **ptr, List &values, bool ignore_errors) table= (*ptr)->table; table->auto_increment_field_not_null= FALSE; } - while ((field = *ptr++) && !thd->net.report_error) + while ((field = *ptr++) && ! thd->is_error()) { value=v++; table= field->table; @@ -6980,7 +6980,7 @@ fill_record(THD *thd, Field **ptr, List &values, bool ignore_errors) if (value->save_in_field(field, 0) < 0) goto err; } - DBUG_RETURN(thd->net.report_error); + DBUG_RETURN(thd->is_error()); err: if (table) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index d77e531ec13..707c8e83007 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1368,7 +1368,7 @@ bool select_send::send_data(List &items) thd->sent_row_count++; if (!thd->vio_ok()) DBUG_RETURN(0); - if (!thd->net.report_error) + if (! thd->is_error()) DBUG_RETURN(protocol->write()); protocol->remove_last_row(); DBUG_RETURN(1); @@ -1389,7 +1389,7 @@ bool select_send::send_eof() mysql_unlock_tables(thd, thd->lock); thd->lock=0; } - if (!thd->net.report_error) + if (! thd->is_error()) { ::send_eof(thd); status= 0; diff --git a/sql/sql_class.h b/sql/sql_class.h index a7f196cd825..a3e71e183f3 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1468,7 +1468,7 @@ public: bool in_lock_tables; /** True if a slave error. Causes the slave to stop. Not the same - as the statement execution error (net.report_error), since + as the statement execution error (is_error()), since a statement may be expected to return an error, e.g. because it returned an error on master, and this is OK on the slave. */ @@ -1716,6 +1716,20 @@ public: net.report_error= 1; DBUG_PRINT("error",("Fatal error set")); } + /** + TRUE if there is an error in the error stack. + + Please use this method instead of direct access to + net.report_error. + + If TRUE, the current (sub)-statement should be aborted. + The main difference between this member and is_fatal_error + is that a fatal error can not be handled by a stored + procedure continue handler, whereas a normal error can. + + To raise this flag, use my_error(). + */ + inline bool is_error() const { return net.report_error; } inline CHARSET_INFO *charset() { return variables.character_set_client; } void update_charset(); diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index afd97c27a55..006cca4ee90 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -975,12 +975,12 @@ void end_connection(THD *thd) decrease_user_connections(thd->user_connect); if (thd->killed || - net->error && net->vio != 0 && net->report_error) + net->error && net->vio != 0 && thd->is_error()) { statistic_increment(aborted_threads,&LOCK_status); } - if (net->error && net->vio != 0 && net->report_error) + if (net->error && net->vio != 0 && thd->is_error()) { if (!thd->killed && thd->variables.log_warnings > 1) { @@ -1030,7 +1030,7 @@ static void prepare_new_connection_state(THD* thd) if (sys_init_connect.value_length && !(sctx->master_access & SUPER_ACL)) { execute_init_command(thd, &sys_init_connect, &LOCK_sys_init_connect); - if (thd->net.report_error) + if (thd->is_error()) { thd->killed= THD::KILL_CONNECTION; sql_print_warning(ER(ER_NEW_ABORTING_CONNECTION), diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index 4c57fad8d87..219dc90429b 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -252,10 +252,10 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, table->mark_columns_needed_for_delete(); while (!(error=info.read_record(&info)) && !thd->killed && - !thd->net.report_error) + ! thd->is_error()) { - // thd->net.report_error is tested to disallow delete row on error - if (!(select && select->skip_record())&& !thd->net.report_error ) + // thd->is_error() is tested to disallow delete row on error + if (!(select && select->skip_record())&& ! thd->is_error() ) { if (table->triggers && @@ -389,7 +389,7 @@ cleanup: send_ok(thd, (ha_rows) thd->row_count_func); DBUG_PRINT("info",("%ld records deleted",(long) deleted)); } - DBUG_RETURN(error >= 0 || thd->net.report_error); + DBUG_RETURN(error >= 0 || thd->is_error()); } diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index e868afede51..077141c4d7a 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -738,7 +738,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, table->triggers, TRG_EVENT_INSERT)) { - if (values_list.elements != 1 && !thd->net.report_error) + if (values_list.elements != 1 && ! thd->is_error()) { info.records++; continue; @@ -769,7 +769,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, table->triggers, TRG_EVENT_INSERT)) { - if (values_list.elements != 1 && ! thd->net.report_error) + if (values_list.elements != 1 && ! thd->is_error()) { info.records++; continue; @@ -1909,7 +1909,7 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) thd->proc_info="got old table"; if (di->thd.killed) { - if (di->thd.net.report_error) + if (di->thd.is_error()) { /* Copy the error message. Note that we don't treat fatal @@ -1940,7 +1940,7 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) pthread_mutex_unlock(&di->mutex); if (table_list->table) { - DBUG_ASSERT(thd->net.report_error == 0); + DBUG_ASSERT(! thd->is_error()); thd->di= di; } /* Unlock the delayed insert object after its last access. */ @@ -1949,7 +1949,7 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) end_create: pthread_mutex_unlock(&LOCK_delayed_create); - DBUG_RETURN(thd->net.report_error); + DBUG_RETURN(thd->is_error()); } @@ -3015,7 +3015,7 @@ bool select_insert::send_data(List &values) thd->count_cuted_fields= CHECK_FIELD_WARN; // Calculate cuted fields store_values(values); thd->count_cuted_fields= CHECK_FIELD_IGNORE; - if (thd->net.report_error) + if (thd->is_error()) DBUG_RETURN(1); if (table_list) // Not CREATE ... SELECT { diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index e6d953bcbe1..ac042960388 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -436,7 +436,7 @@ pthread_handler_t handle_bootstrap(void *arg) if (thd->is_fatal_error) break; - if (thd->net.report_error) + if (thd->is_error()) { /* The query failed, send error to log and abort bootstrap */ net_send_error(thd); @@ -990,7 +990,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd, mysql_parse(thd, thd->query, thd->query_length, & found_semicolon); - while (!thd->killed && found_semicolon && !thd->net.report_error) + while (!thd->killed && found_semicolon && ! thd->is_error()) { char *next_packet= (char*) found_semicolon; net->no_send_error= 0; @@ -1355,9 +1355,9 @@ bool dispatch_command(enum enum_server_command command, THD *thd, thd->transaction.xid_state.xid.null(); /* report error issued during command execution */ - if (thd->killed_errno() && !thd->net.report_error) + if (thd->killed_errno() && ! thd->is_error()) thd->send_kill_message(); - if (thd->net.report_error) + if (thd->is_error()) net_send_error(thd); log_slow_statement(thd); @@ -3930,7 +3930,7 @@ create_sp_error: thd->row_count_func)); else { - DBUG_ASSERT(thd->net.report_error == 1 || thd->killed); + DBUG_ASSERT(thd->is_error() || thd->killed); goto error; // Substatement should already have sent error } } @@ -4523,7 +4523,7 @@ finish: */ start_waiting_global_read_lock(thd); } - DBUG_RETURN(res || thd->net.report_error); + DBUG_RETURN(res || thd->is_error()); } @@ -5467,7 +5467,7 @@ void mysql_parse(THD *thd, const char *inBuf, uint length, else #endif { - if (! thd->net.report_error) + if (! thd->is_error()) { /* Binlog logs a string starting from thd->query and having length @@ -5491,7 +5491,7 @@ void mysql_parse(THD *thd, const char *inBuf, uint length, } else { - DBUG_ASSERT(thd->net.report_error); + DBUG_ASSERT(thd->is_error()); DBUG_PRINT("info",("Command aborted. Fatal_error: %d", thd->is_fatal_error)); @@ -6311,7 +6311,7 @@ void add_join_natural(TABLE_LIST *a, TABLE_LIST *b, List *using_fields, RETURN 0 ok - !=0 error. thd->killed or thd->net.report_error is set + !=0 error. thd->killed or thd->is_error() is set */ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables, @@ -7274,10 +7274,10 @@ bool parse_sql(THD *thd, bool mysql_parse_status= MYSQLparse(thd) != 0; - /* Check that if MYSQLparse() failed, thd->net.report_error is set. */ + /* Check that if MYSQLparse() failed, thd->is_error() is set. */ DBUG_ASSERT(!mysql_parse_status || - mysql_parse_status && thd->net.report_error); + mysql_parse_status && thd->is_error()); /* Reset Lex_input_stream. */ diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index a6cdcf14881..b1b1502f015 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2864,7 +2864,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) lex_start(thd); error= parse_sql(thd, &lip, NULL) || - thd->net.report_error || + thd->is_error() || init_param_array(this); lex->set_trg_event_type_for_tables(); diff --git a/sql/sql_select.cc b/sql/sql_select.cc index fd0ce630a01..ef1151d82f3 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -263,8 +263,8 @@ bool handle_select(THD *thd, LEX *lex, select_result *result, result, unit, select_lex); } DBUG_PRINT("info",("res: %d report_error: %d", res, - thd->net.report_error)); - res|= thd->net.report_error; + thd->is_error())); + res|= thd->is_error(); if (unlikely(res)) result->abort(); @@ -491,7 +491,7 @@ JOIN::prepare(Item ***rref_pointer_array, (having->fix_fields(thd, &having) || having->check_cols(1))); select_lex->having_fix_field= 0; - if (having_fix_rc || thd->net.report_error) + if (having_fix_rc || thd->is_error()) DBUG_RETURN(-1); /* purecov: inspected */ thd->lex->allow_sum_func= save_allow_sum_func; } @@ -817,7 +817,7 @@ JOIN::optimize() } conds= optimize_cond(this, conds, join_list, &cond_value); - if (thd->net.report_error) + if (thd->is_error()) { error= 1; DBUG_PRINT("error",("Error from optimize_cond")); @@ -826,7 +826,7 @@ JOIN::optimize() { having= optimize_cond(this, having, join_list, &having_value); - if (thd->net.report_error) + if (thd->is_error()) { error= 1; DBUG_PRINT("error",("Error from optimize_cond")); @@ -1031,7 +1031,7 @@ JOIN::optimize() { ORDER *org_order= order; order=remove_const(this, order,conds,1, &simple_order); - if (thd->net.report_error) + if (thd->is_error()) { error= 1; DBUG_PRINT("error",("Error from remove_const")); @@ -1162,7 +1162,7 @@ JOIN::optimize() group_list= remove_const(this, (old_group_list= group_list), conds, rollup.state == ROLLUP::STATE_NONE, &simple_group); - if (thd->net.report_error) + if (thd->is_error()) { error= 1; DBUG_PRINT("error",("Error from remove_const")); @@ -1185,7 +1185,7 @@ JOIN::optimize() { group_list= procedure->group= remove_const(this, procedure->group, conds, 1, &simple_group); - if (thd->net.report_error) + if (thd->is_error()) { error= 1; DBUG_PRINT("error",("Error from remove_const")); @@ -2098,10 +2098,10 @@ JOIN::exec() } } } - /* XXX: When can we have here thd->net.report_error not zero? */ - if (thd->net.report_error) + /* XXX: When can we have here thd->is_error() not zero? */ + if (thd->is_error()) { - error= thd->net.report_error; + error= thd->is_error(); DBUG_VOID_RETURN; } curr_join->having= curr_join->tmp_having; @@ -2307,7 +2307,7 @@ mysql_select(THD *thd, Item ***rref_pointer_array, join->having_history= (join->having?join->having:join->tmp_having); } - if (thd->net.report_error) + if (thd->is_error()) goto err; join->exec(); @@ -2333,7 +2333,7 @@ err: { thd->proc_info="end"; err|= select_lex->cleanup(); - DBUG_RETURN(err || thd->net.report_error); + DBUG_RETURN(err || thd->is_error()); } DBUG_RETURN(join->error); } @@ -10717,7 +10717,7 @@ do_select(JOIN *join,List *fields,TABLE *table,Procedure *procedure) DBUG_PRINT("error",("Error: do_select() failed")); } #endif - DBUG_RETURN(join->thd->net.report_error ? -1 : rc); + DBUG_RETURN(join->thd->is_error() ? -1 : rc); } @@ -16050,7 +16050,7 @@ bool mysql_explain_union(THD *thd, SELECT_LEX_UNIT *unit, select_result *result) first->options | thd->options | SELECT_DESCRIBE, result, unit, first); } - DBUG_RETURN(res || thd->net.report_error); + DBUG_RETURN(res || thd->is_error()); } diff --git a/sql/sql_union.cc b/sql/sql_union.cc index 5da4b97cc5d..a48cff82715 100644 --- a/sql/sql_union.cc +++ b/sql/sql_union.cc @@ -58,7 +58,7 @@ bool select_union::send_data(List &values) return 0; } fill_record(thd, table->field, values, 1); - if (thd->net.report_error) + if (thd->is_error()) return 1; if ((error= table->file->ha_write_row(table->record[0]))) diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 4071bb86c90..036d7e06d0e 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -844,7 +844,7 @@ int mysql_update(THD *thd, } thd->count_cuted_fields= CHECK_FIELD_IGNORE; /* calc cuted fields */ thd->abort_on_warning= 0; - DBUG_RETURN((error >= 0 || thd->net.report_error) ? 1 : 0); + DBUG_RETURN((error >= 0 || thd->is_error()) ? 1 : 0); err: delete select; @@ -1193,8 +1193,8 @@ bool mysql_multi_update(THD *thd, OPTION_SETUP_TABLES_DONE, result, unit, select_lex); DBUG_PRINT("info",("res: %d report_error: %d", res, - thd->net.report_error)); - res|= thd->net.report_error; + (int) thd->is_error())); + res|= thd->is_error(); if (unlikely(res)) { /* If we had a another error reported earlier then this will be ignored */ diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 6e27af63e8a..f7223cafb5e 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -607,7 +607,7 @@ err: thd->proc_info= "end"; lex->link_first_table_back(view, link_to_local); unit->cleanup(); - DBUG_RETURN(res || thd->net.report_error); + DBUG_RETURN(res || thd->is_error()); } @@ -823,7 +823,7 @@ loop_out: view_parameters + revision_number_position, 1, &file_parser_dummy_hook)) { - error= thd->net.report_error? -1 : 0; + error= thd->is_error() ? -1 : 0; goto err; } } @@ -886,7 +886,7 @@ loop_out: if (sql_create_definition_file(&dir, &file, view_file_type, (uchar*)view, view_parameters, num_view_backups)) { - error= thd->net.report_error? -1 : 1; + error= thd->is_error() ? -1 : 1; goto err; } DBUG_RETURN(0); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index 109e8f5434f..b893014aacb 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -9804,7 +9804,7 @@ NUM_literal: | DECIMAL_NUM { $$= new Item_decimal($1.str, $1.length, YYTHD->charset()); - if (YYTHD->net.report_error) + if (YYTHD->is_error()) { MYSQL_YYABORT; } @@ -9812,7 +9812,7 @@ NUM_literal: | FLOAT_NUM { $$ = new Item_float($1.str, $1.length); - if (YYTHD->net.report_error) + if (YYTHD->is_error()) { MYSQL_YYABORT; } From 8365a74e47afd6a9598f21b75f3360448b69fcf6 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 30 Oct 2007 22:35:14 +0300 Subject: [PATCH 12/31] In ha_delete_table, use a standard mechanism to intercept the error message and convert it to a warning instead of direct manipulation with the thread error stack. Fix a bug in handler::print_erorr when a garbled message was printed for HA_ERR_NO_SUCH_TABLE. This is a pre-requisite patch for the fix for Bug#12713 Error in a stored function called from a SELECT doesn't cause ROLLBACK of statem sql/handler.cc: Use a standard mechanism to intercept the error message, instead of direct manipulation with thread error stack. Fix a bug when for HA_ERR_NO_SUCH_TABLE handler::print_error() would print a garbled message. sql/log.cc: Extend internal error handler interface to carry the message text. sql/mysqld.cc: Extend internal error handler interface to carry the message text. sql/sql_base.cc: Extend internal error handler interface to carry the message text. sql/sql_class.cc: Extend internal error handler interface to carry the message text. sql/sql_class.h: Extend internal error handler interface to carry the message text. sql/sql_error.cc: Extend internal error handler interface to carry the message text. --- sql/handler.cc | 72 +++++++++++++++++++++++++++++++----------------- sql/log.cc | 3 +- sql/mysqld.cc | 2 +- sql/sql_base.cc | 3 +- sql/sql_class.cc | 4 +-- sql/sql_class.h | 3 +- sql/sql_error.cc | 2 +- 7 files changed, 56 insertions(+), 33 deletions(-) diff --git a/sql/handler.cc b/sql/handler.cc index 3ba52ca1c71..22374d57e99 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1414,6 +1414,36 @@ static const char *check_lowercase_names(handler *file, const char *path, } +/** + An interceptor to hijack the text of the error message without + setting an error in the thread. We need the text to present it + in the form of a warning to the user. +*/ + +struct Ha_delete_table_error_handler: public Internal_error_handler +{ +public: + virtual bool handle_error(uint sql_errno, + const char *message, + MYSQL_ERROR::enum_warning_level level, + THD *thd); + char buff[MYSQL_ERRMSG_SIZE]; +}; + + +bool +Ha_delete_table_error_handler:: +handle_error(uint sql_errno, + const char *message, + MYSQL_ERROR::enum_warning_level level, + THD *thd) +{ + /* Grab the error message */ + strmake(buff, message, sizeof(buff)-1); + return TRUE; +} + + /** @brief This should return ENOENT if the file doesn't exists. The .frm file will be deleted only if we return 0 or ENOENT @@ -1442,23 +1472,11 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, { /* Because file->print_error() use my_error() to generate the error message - we must store the error state in thd, reset it and restore it to - be able to get hold of the error message. - (We should in the future either rewrite handler::print_error() or make - a nice method of this. + we use an internal error handler to intercept it and store the text + in a temporary buffer. Later the message will be presented to user + as a warning. */ - bool is_slave_error= thd->is_slave_error; - sp_rcontext *spcont= thd->spcont; - SELECT_LEX *current_select= thd->lex->current_select; - char buff[sizeof(thd->net.last_error)]; - char new_error[sizeof(thd->net.last_error)]; - int last_errno= thd->net.last_errno; - - strmake(buff, thd->net.last_error, sizeof(buff)-1); - thd->is_slave_error= 0; - thd->spcont= NULL; - thd->lex->current_select= 0; - thd->net.last_error[0]= 0; + Ha_delete_table_error_handler ha_delete_table_error_handler; /* Fill up strucutures that print_error may need */ dummy_share.path.str= (char*) path; @@ -1471,16 +1489,18 @@ int ha_delete_table(THD *thd, handlerton *table_type, const char *path, file->table_share= &dummy_share; file->table= &dummy_table; - file->print_error(error, 0); - strmake(new_error, thd->net.last_error, sizeof(buff)-1); - /* restore thd */ - thd->is_slave_error= is_slave_error; - thd->spcont= spcont; - thd->lex->current_select= current_select; - thd->net.last_errno= last_errno; - strmake(thd->net.last_error, buff, sizeof(buff)-1); - push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, error, new_error); + thd->push_internal_handler(&ha_delete_table_error_handler); + file->print_error(error, 0); + + thd->pop_internal_handler(); + + /* + XXX: should we convert *all* errors to warnings here? + What if the error is fatal? + */ + push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, error, + ha_delete_table_error_handler.buff); } delete file; DBUG_RETURN(error); @@ -2203,7 +2223,7 @@ void handler::print_error(int error, myf errflag) case HA_ERR_NO_SUCH_TABLE: my_error(ER_NO_SUCH_TABLE, MYF(0), table_share->db.str, table_share->table_name.str); - break; + DBUG_VOID_RETURN; case HA_ERR_RBR_LOGGING_FAILED: textno= ER_BINLOG_ROW_LOGGING_FAILED; break; diff --git a/sql/log.cc b/sql/log.cc index 02986bef493..688ed03d5d1 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -72,13 +72,14 @@ public: virtual ~Silence_log_table_errors() {} - virtual bool handle_error(uint sql_errno, + virtual bool handle_error(uint sql_errno, const char *message, MYSQL_ERROR::enum_warning_level level, THD *thd); }; bool Silence_log_table_errors::handle_error(uint /* sql_errno */, + const char * /* message */, MYSQL_ERROR::enum_warning_level /* level */, THD * /* thd */) { diff --git a/sql/mysqld.cc b/sql/mysqld.cc index cc964e417bf..c76bd8bd712 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -2583,7 +2583,7 @@ int my_message_sql(uint error, const char *str, myf MyFlags) TODO: There are two exceptions mechanism (THD and sp_rcontext), this could be improved by having a common stack of handlers. */ - if (thd->handle_error(error, + if (thd->handle_error(error, str, MYSQL_ERROR::WARN_LEVEL_ERROR)) DBUG_RETURN(0); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 223485f1e42..2584390d756 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -44,7 +44,7 @@ public: virtual ~Prelock_error_handler() {} - virtual bool handle_error(uint sql_errno, + virtual bool handle_error(uint sql_errno, const char *message, MYSQL_ERROR::enum_warning_level level, THD *thd); @@ -58,6 +58,7 @@ private: bool Prelock_error_handler::handle_error(uint sql_errno, + const char * /* message */, MYSQL_ERROR::enum_warning_level /* level */, THD * /* thd */) { diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 707c8e83007..b5a29783044 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -498,12 +498,12 @@ void THD::push_internal_handler(Internal_error_handler *handler) } -bool THD::handle_error(uint sql_errno, +bool THD::handle_error(uint sql_errno, const char *message, MYSQL_ERROR::enum_warning_level level) { if (m_internal_handler) { - return m_internal_handler->handle_error(sql_errno, level, this); + return m_internal_handler->handle_error(sql_errno, message, level, this); } return FALSE; // 'FALSE', as per coding style diff --git a/sql/sql_class.h b/sql/sql_class.h index a3e71e183f3..db847c814a0 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -969,6 +969,7 @@ public: @return true if the error is handled */ virtual bool handle_error(uint sql_errno, + const char *message, MYSQL_ERROR::enum_warning_level level, THD *thd) = 0; }; @@ -1923,7 +1924,7 @@ public: @param level the error level @return true if the error is handled */ - virtual bool handle_error(uint sql_errno, + virtual bool handle_error(uint sql_errno, const char *message, MYSQL_ERROR::enum_warning_level level); /** diff --git a/sql/sql_error.cc b/sql/sql_error.cc index 8bdb2e59ed5..89cff73d153 100644 --- a/sql/sql_error.cc +++ b/sql/sql_error.cc @@ -137,7 +137,7 @@ MYSQL_ERROR *push_warning(THD *thd, MYSQL_ERROR::enum_warning_level level, level= MYSQL_ERROR::WARN_LEVEL_ERROR; } - if (thd->handle_error(code, level)) + if (thd->handle_error(code, msg, level)) DBUG_RETURN(NULL); if (thd->spcont && From bf18f6d4b863b1c45467ea55b0662f4155ccc8fb Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 30 Oct 2007 20:51:04 -0200 Subject: [PATCH 13/31] Bug#30904 SET PASSWORD statement is non-transactional The SET PASSWORD statement is non-transactional (no explicit transaction boundaries) in nature and hence is forbidden inside stored functions and triggers, but it weren't being effectively forbidden. The implemented fix is to issue a implicit commit with every SET PASSWORD statement, effectively prohibiting these statements in stored functions and triggers. mysql-test/r/sp-error.result: Add test case result for Bug#30904 mysql-test/t/sp-error.test: Add test case for Bug#30904 sql/sql_lex.h: Add variable to set that a statement with SET PASSWORD causes a implicit commit. sql/sql_parse.cc: End active transaction in SET PASSWORD. sql/sql_yacc.yy: Set the correct flag on SET PASSWORD if inside a SP, thus effectively prohibiting SET PASSWORD statements in stored functions and triggers. --- mysql-test/r/sp-error.result | 10 ++++++++++ mysql-test/t/sp-error.test | 19 +++++++++++++++++++ sql/sql_lex.h | 2 +- sql/sql_parse.cc | 4 ++++ sql/sql_yacc.yy | 8 ++++++++ 5 files changed, 42 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result index 1b14d75cd9c..300fa42f3ad 100644 --- a/mysql-test/r/sp-error.result +++ b/mysql-test/r/sp-error.result @@ -1523,3 +1523,13 @@ ERROR 42000: You have an error in your SQL syntax; check the manual that corresp SELECT ..inexistent(); ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '.inexistent()' at line 1 USE test; +create function f1() returns int +begin +set @test = 1, password = password('foo'); +return 1; +end| +ERROR HY000: Not allowed to set autocommit from a stored function or trigger +create trigger t1 +before insert on t2 for each row set password = password('foo'); +delimiter ;| +ERROR HY000: Not allowed to set autocommit from a stored function or trigger diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test index a956a246770..9f20d02480c 100644 --- a/mysql-test/t/sp-error.test +++ b/mysql-test/t/sp-error.test @@ -2222,6 +2222,25 @@ SELECT .inexistent(); SELECT ..inexistent(); USE test; +# +# Bug#30904 SET PASSWORD statement is non-transactional +# + +delimiter |; + +--error ER_SP_CANT_SET_AUTOCOMMIT +create function f1() returns int +begin + set @test = 1, password = password('foo'); + return 1; +end| + +--error ER_SP_CANT_SET_AUTOCOMMIT +create trigger t1 + before insert on t2 for each row set password = password('foo'); + +delimiter ;| + # # BUG#NNNN: New bug synopsis # diff --git a/sql/sql_lex.h b/sql/sql_lex.h index a26c4fd1ca0..507d64daf89 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -1616,7 +1616,7 @@ typedef struct st_lex : public Query_tables_list uint8 create_view_algorithm; uint8 create_view_check; bool drop_if_exists, drop_temporary, local_file, one_shot_set; - + bool autocommit; bool verbose, no_write_to_binlog; bool tx_chain, tx_release; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index ac042960388..85457fea41b 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3053,6 +3053,10 @@ end_with_restore_list: case SQLCOM_SET_OPTION: { List *lex_var_list= &lex->var_list; + + if (lex->autocommit && end_active_trans(thd)) + goto error; + if ((check_table_access(thd, SELECT_ACL, all_tables, 0) || open_and_lock_tables(thd, all_tables))) goto error; diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index b893014aacb..69cd7060778 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -10516,6 +10516,7 @@ set: lex->option_type=OPT_SESSION; lex->var_list.empty(); lex->one_shot_set= 0; + lex->autocommit= 0; } option_value_list {} @@ -10558,6 +10559,7 @@ option_type_value: lex->option_type=OPT_SESSION; lex->var_list.empty(); lex->one_shot_set= 0; + lex->autocommit= 0; lex->sphead->m_tmp_query= lip->get_tok_start(); } } @@ -10799,10 +10801,16 @@ option_value: user->host=null_lex_str; user->user.str=thd->security_ctx->priv_user; thd->lex->var_list.push_back(new set_var_password(user, $3)); + thd->lex->autocommit= TRUE; + if (lex->sphead) + lex->sphead->m_flags|= sp_head::HAS_SET_AUTOCOMMIT_STMT; } | PASSWORD FOR_SYM user equal text_or_password { Lex->var_list.push_back(new set_var_password($3,$5)); + Lex->autocommit= TRUE; + if (Lex->sphead) + Lex->sphead->m_flags|= sp_head::HAS_SET_AUTOCOMMIT_STMT; } ; From 77c8c76f8d53228521179d1ac07dea61f599dc36 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 31 Oct 2007 09:53:58 +0300 Subject: [PATCH 14/31] Fix result files after patch for BUG#31035. mysql-test/r/group_min_max.result: Update result file. mysql-test/r/index_merge_myisam.result: Update result file. --- mysql-test/r/group_min_max.result | 4 ++-- mysql-test/r/index_merge_myisam.result | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/group_min_max.result b/mysql-test/r/group_min_max.result index 02b1459afd0..f62fd662a5d 100644 --- a/mysql-test/r/group_min_max.result +++ b/mysql-test/r/group_min_max.result @@ -2251,7 +2251,7 @@ EXPLAIN SELECT 1 FROM t1 AS t1_outer WHERE (SELECT max(b) FROM t1 GROUP BY a HAVING a < 2) > 12; id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY NULL NULL NULL NULL NULL NULL NULL Impossible WHERE -2 SUBQUERY t1 range NULL a 5 NULL 8 Using index for group-by +2 SUBQUERY t1 range NULL a 5 NULL 8 EXPLAIN SELECT 1 FROM t1 AS t1_outer WHERE a IN (SELECT max(b) FROM t1 GROUP BY a HAVING a < 2); id select_type table type possible_keys key key_len ref rows Extra @@ -2268,7 +2268,7 @@ AND t1_outer1.b = t1_outer2.b; id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY t1_outer1 ref a a 5 const 1 Using where; Using index 1 PRIMARY t1_outer2 index NULL a 10 NULL 15 Using where; Using index; Using join buffer -2 SUBQUERY t1 range NULL a 5 NULL 8 Using index for group-by +2 SUBQUERY t1 range NULL a 5 NULL 8 EXPLAIN SELECT (SELECT (SELECT max(b) FROM t1 GROUP BY a HAVING a < 2) x FROM t1 AS t1_outer) x2 FROM t1 AS t1_outer2; id select_type table type possible_keys key key_len ref rows Extra diff --git a/mysql-test/r/index_merge_myisam.result b/mysql-test/r/index_merge_myisam.result index 9d7d06f7f1b..ebeba53fdfa 100644 --- a/mysql-test/r/index_merge_myisam.result +++ b/mysql-test/r/index_merge_myisam.result @@ -286,7 +286,7 @@ NULL UNION RESULT ALL NULL NULL NULL NULL NULL explain select * from (select * from t1 where key1 = 3 or key2 =3) as Z where key8 >5; id select_type table type possible_keys key key_len ref rows Extra 1 PRIMARY system NULL NULL NULL NULL 1 -2 DERIVED t1 index_merge i1,i2 i1,i2 4,4 NULL 2 Using union(i1,i2); Using where; Using index +2 DERIVED t1 index_merge i1,i2 i1,i2 4,4 NULL 2 Using union(i1,i2); Using where create table t3 like t0; insert into t3 select * from t0; alter table t3 add key9 int not null, add index i9(key9); From 47d17e5d7f241153c3a3f6b3da7f8ba96f5f3046 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 31 Oct 2007 12:04:19 +0300 Subject: [PATCH 15/31] Fix for a BUG#31649: events.test fails: NULL "state" field of SHOW PROCESSLIST. The problem was a race condition: if the Event Scheduler was not quick enough, the following scenario happens: - The Event Scheduler picks up the created event; - The event is executed; - event_scheduler_thread->proc_info is set to NULL; - The client issues SELECT FROM I_S. The fix is to wait for the Event Scheduler to reach 'Waiting for next activation' state. mysql-test/t/events.test: Wait for the Event Scheduler to start waiting for the activation. --- mysql-test/t/events.test | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mysql-test/t/events.test b/mysql-test/t/events.test index d1ca5f1b609..a4c7eaebc30 100644 --- a/mysql-test/t/events.test +++ b/mysql-test/t/events.test @@ -454,7 +454,8 @@ create event закачка on schedule every 10 hour do select get_lock("test_l --echo "Should have only 2 processes: the scheduler and the locked event" let $wait_condition= select count(*) = 2 from information_schema.processlist where ( (state like 'User lock%' AND info like 'select get_lock%') - OR (command='Daemon' AND user='event_scheduler')); + OR (command='Daemon' AND user='event_scheduler' AND + state = 'Waiting for next activation')); --source include/wait_condition.inc select /*2*/ user, host, db, command, state, info From 8d69dd398d53b6dd990c3879649e467ec8b5f894 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 31 Oct 2007 12:25:18 +0100 Subject: [PATCH 16/31] Bug#31347 Increase in memory usage after many DROP USER statements Dropping users causes huge increase in memory usage because field values were allocated on the server memory root for temporary usage but never deallocated. This patch changes the target memory root to be that of the thread handler instead since this root is cleared between each statement. sql/sql_acl.cc: Changed memory root from server life time memory to thread life time memory. --- sql/sql_acl.cc | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 6bc6cce5e72..d03aacd7f07 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -311,7 +311,7 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables) continue; } - const char *password= get_field(&mem, table->field[2]); + const char *password= get_field(thd->mem_root, table->field[2]); uint password_len= password ? strlen(password) : 0; set_user_salt(&user, password, password_len); if (user.salt_len == 0 && password_len != 0) @@ -364,7 +364,7 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables) /* Starting from 4.0.2 we have more fields */ if (table->s->fields >= 31) { - char *ssl_type=get_field(&mem, table->field[next_field++]); + char *ssl_type=get_field(thd->mem_root, table->field[next_field++]); if (!ssl_type) user.ssl_type=SSL_TYPE_NONE; else if (!strcmp(ssl_type, "ANY")) @@ -378,11 +378,11 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables) user.x509_issuer= get_field(&mem, table->field[next_field++]); user.x509_subject= get_field(&mem, table->field[next_field++]); - char *ptr = get_field(&mem, table->field[next_field++]); + char *ptr = get_field(thd->mem_root, table->field[next_field++]); user.user_resource.questions=ptr ? atoi(ptr) : 0; - ptr = get_field(&mem, table->field[next_field++]); + ptr = get_field(thd->mem_root, table->field[next_field++]); user.user_resource.updates=ptr ? atoi(ptr) : 0; - ptr = get_field(&mem, table->field[next_field++]); + ptr = get_field(thd->mem_root, table->field[next_field++]); user.user_resource.conn_per_hour= ptr ? atoi(ptr) : 0; if (user.user_resource.questions || user.user_resource.updates || user.user_resource.conn_per_hour) @@ -391,7 +391,7 @@ static my_bool acl_load(THD *thd, TABLE_LIST *tables) if (table->s->fields >= 36) { /* Starting from 5.0.3 we have max_user_connections field */ - ptr= get_field(&mem, table->field[next_field++]); + ptr= get_field(thd->mem_root, table->field[next_field++]); user.user_resource.user_conn= ptr ? atoi(ptr) : 0; } else @@ -4865,6 +4865,7 @@ static int handle_grant_table(TABLE_LIST *tables, uint table_no, bool drop, byte user_key[MAX_KEY_LENGTH]; uint key_prefix_length; DBUG_ENTER("handle_grant_table"); + THD *thd= current_thd; if (! table_no) // mysql.user table { @@ -4932,17 +4933,18 @@ static int handle_grant_table(TABLE_LIST *tables, uint table_no, bool drop, DBUG_PRINT("info",("scan error: %d", error)); continue; } - if (! (host= get_field(&mem, host_field))) + if (! (host= get_field(thd->mem_root, host_field))) host= ""; - if (! (user= get_field(&mem, user_field))) + if (! (user= get_field(thd->mem_root, user_field))) user= ""; #ifdef EXTRA_DEBUG DBUG_PRINT("loop",("scan fields: '%s'@'%s' '%s' '%s' '%s'", user, host, - get_field(&mem, table->field[1]) /*db*/, - get_field(&mem, table->field[3]) /*table*/, - get_field(&mem, table->field[4]) /*column*/)); + get_field(thd->mem_root, table->field[1]) /*db*/, + get_field(thd->mem_root, table->field[3]) /*table*/, + get_field(thd->mem_root, + table->field[4]) /*column*/)); #endif if (strcmp(user_str, user) || my_strcasecmp(system_charset_info, host_str, host)) From 70987ab97c76c1268b79486758c44ccd6a6ce254 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 31 Oct 2007 17:16:53 +0300 Subject: [PATCH 17/31] Cleanup: use helper functions to set an error in MYSQL or MYSQL_STMT. No functionality added or changed. This is a pre-requisite for the fix for Bug#12713 Error in a stored function called from a SELECT doesn't cause ROLLBACK of statem Address post-review comments. include/sql_common.h: Declare auxiliary functions to manipulate mysql.net.last_er* and mysql_stmt.last_er* libmysql/libmysql.c: Use helper functions to set an error in MYSQL or MYSQL_STMT libmysqld/lib_sql.cc: Use helper functions to set an error in MYSQL or MYSQL_STMT sql-common/client.c: Use helper functions to set an error in MYSQL or MYSQL_STMT --- include/sql_common.h | 6 +- libmysql/libmysql.c | 152 +++++++++++++++++++------------------------ libmysqld/lib_sql.cc | 22 +++---- sql-common/client.c | 148 +++++++++++++++++++---------------------- 4 files changed, 144 insertions(+), 184 deletions(-) diff --git a/include/sql_common.h b/include/sql_common.h index 80504140fae..56e7305130f 100644 --- a/include/sql_common.h +++ b/include/sql_common.h @@ -36,8 +36,10 @@ cli_advanced_command(MYSQL *mysql, enum enum_server_command command, const unsigned char *arg, ulong arg_length, my_bool skip_check, MYSQL_STMT *stmt); unsigned long cli_safe_read(MYSQL *mysql); -void set_stmt_errmsg(MYSQL_STMT * stmt, const char *err, int errcode, - const char *sqlstate); +void net_clear_error(NET *net); +void set_stmt_errmsg(MYSQL_STMT *stmt, NET *net); +void set_stmt_error(MYSQL_STMT *stmt, int errcode, const char *sqlstate, + const char *err); void set_mysql_error(MYSQL *mysql, int errcode, const char *sqlstate); #ifdef __cplusplus } diff --git a/libmysql/libmysql.c b/libmysql/libmysql.c index e2e42fe4a2d..76928ec798c 100644 --- a/libmysql/libmysql.c +++ b/libmysql/libmysql.c @@ -686,9 +686,7 @@ int cli_read_change_user_result(MYSQL *mysql, char *buff, const char *passwd) if (my_net_write(net, (uchar*) buff, SCRAMBLE_LENGTH_323 + 1) || net_flush(net)) { - net->last_errno= CR_SERVER_LOST; - strmov(net->sqlstate, unknown_sqlstate); - strmov(net->last_error,ER(net->last_errno)); + set_mysql_error(mysql, CR_SERVER_LOST, unknown_sqlstate); return 1; } /* Read what server thinks about out new auth message report */ @@ -860,8 +858,7 @@ my_bool handle_local_infile(MYSQL *mysql, const char *net_filename) /* copy filename into local memory and allocate read buffer */ if (!(buf=my_malloc(packet_length, MYF(0)))) { - strmov(net->sqlstate, unknown_sqlstate); - strmov(net->last_error, ER(net->last_errno=CR_OUT_OF_MEMORY)); + set_mysql_error(mysql, CR_OUT_OF_MEMORY, unknown_sqlstate); DBUG_RETURN(1); } @@ -887,9 +884,7 @@ my_bool handle_local_infile(MYSQL *mysql, const char *net_filename) { DBUG_PRINT("error", ("Lost connection to MySQL server during LOAD DATA of local file")); - strmov(net->sqlstate, unknown_sqlstate); - net->last_errno=CR_SERVER_LOST; - strmov(net->last_error,ER(net->last_errno)); + set_mysql_error(mysql, CR_SERVER_LOST, unknown_sqlstate); goto err; } } @@ -897,9 +892,7 @@ my_bool handle_local_infile(MYSQL *mysql, const char *net_filename) /* Send empty packet to mark end of file */ if (my_net_write(net, (const uchar*) "", 0) || net_flush(net)) { - strmov(net->sqlstate, unknown_sqlstate); - net->last_errno=CR_SERVER_LOST; - sprintf(net->last_error,ER(net->last_errno),errno); + set_mysql_error(mysql, CR_SERVER_LOST, unknown_sqlstate); goto err; } @@ -1400,9 +1393,7 @@ const char *cli_read_statistics(MYSQL *mysql) mysql->net.read_pos[mysql->packet_length]=0; /* End of stat string */ if (!mysql->net.read_pos[0]) { - strmov(mysql->net.sqlstate, unknown_sqlstate); - mysql->net.last_errno=CR_WRONG_HOST_INFO; - strmov(mysql->net.last_error, ER(mysql->net.last_errno)); + set_mysql_error(mysql, CR_WRONG_HOST_INFO, unknown_sqlstate); return mysql->net.last_error; } return (char*) mysql->net.read_pos; @@ -1848,24 +1839,17 @@ static my_bool my_realloc_str(NET *net, ulong length) if (buf_length + length > net->max_packet) { res= net_realloc(net, buf_length + length); + if (res) + { + strmov(net->sqlstate, unknown_sqlstate); + strmov(net->last_error, ER(net->last_errno)); + } net->write_pos= net->buff+ buf_length; } DBUG_RETURN(res); } -/* Clear possible error statee of struct NET */ - -static void net_clear_error(NET *net) -{ - if (net->last_errno) - { - net->last_errno= 0; - net->last_error[0]= '\0'; - strmov(net->sqlstate, not_error_sqlstate); - } -} - static void stmt_clear_error(MYSQL_STMT *stmt) { if (stmt->last_errno) @@ -1876,18 +1860,21 @@ static void stmt_clear_error(MYSQL_STMT *stmt) } } -/* +/** Set statement error code, sqlstate, and error message from given errcode and sqlstate. */ -static void set_stmt_error(MYSQL_STMT * stmt, int errcode, - const char *sqlstate) +void set_stmt_error(MYSQL_STMT * stmt, int errcode, + const char *sqlstate, const char *err) { DBUG_ENTER("set_stmt_error"); DBUG_PRINT("enter", ("error: %d '%s'", errcode, ER(errcode))); DBUG_ASSERT(stmt != 0); + if (err == 0) + err= ER(errcode); + stmt->last_errno= errcode; strmov(stmt->last_error, ER(errcode)); strmov(stmt->sqlstate, sqlstate); @@ -1896,21 +1883,24 @@ static void set_stmt_error(MYSQL_STMT * stmt, int errcode, } -/* - Set statement error code, sqlstate, and error message. +/** + Set statement error code, sqlstate, and error message from NET. + + @param stmt a statement handle. Copy the error here. + @param net mysql->net. Source of the error. */ -void set_stmt_errmsg(MYSQL_STMT * stmt, const char *err, int errcode, - const char *sqlstate) +void set_stmt_errmsg(MYSQL_STMT *stmt, NET *net) { DBUG_ENTER("set_stmt_errmsg"); - DBUG_PRINT("enter", ("error: %d/%s '%s'", errcode, sqlstate, err)); + DBUG_PRINT("enter", ("error: %d/%s '%s'", net->last_errno, net->sqlstate, + net->last_error)); DBUG_ASSERT(stmt != 0); - stmt->last_errno= errcode; - if (err && err[0]) - strmov(stmt->last_error, err); - strmov(stmt->sqlstate, sqlstate); + stmt->last_errno= net->last_errno; + if (net->last_error && net->last_error[0]) + strmov(stmt->last_error, net->last_error); + strmov(stmt->sqlstate, net->sqlstate); DBUG_VOID_RETURN; } @@ -2085,7 +2075,7 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, const char *query, ulong length) if (!mysql) { /* mysql can be reset in mysql_close called from mysql_reconnect */ - set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate); + set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -2123,23 +2113,20 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, const char *query, ulong length) stmt->state= MYSQL_STMT_INIT_DONE; if (stmt_command(mysql, COM_STMT_CLOSE, buff, 4, stmt)) { - set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, - mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); DBUG_RETURN(1); } } if (stmt_command(mysql, COM_STMT_PREPARE, (const uchar*) query, length, stmt)) { - set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, - mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); DBUG_RETURN(1); } if ((*mysql->methods->read_prepare_result)(mysql, stmt)) { - set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, - mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); DBUG_RETURN(1); } @@ -2154,7 +2141,7 @@ mysql_stmt_prepare(MYSQL_STMT *stmt, const char *query, ulong length) (stmt->param_count + stmt->field_count)))) { - set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate); + set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate, NULL); DBUG_RETURN(1); } stmt->bind= stmt->params + stmt->param_count; @@ -2284,7 +2271,7 @@ mysql_stmt_result_metadata(MYSQL_STMT *stmt) if (!(result=(MYSQL_RES*) my_malloc(sizeof(*result), MYF(MY_WME | MY_ZEROFILL)))) { - set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate); + set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate, NULL); DBUG_RETURN(0); } @@ -2517,7 +2504,7 @@ static my_bool store_param(MYSQL_STMT *stmt, MYSQL_BIND *param) */ if ((my_realloc_str(net, *param->length))) { - set_stmt_error(stmt, net->last_errno, unknown_sqlstate); + set_stmt_errmsg(stmt, net); DBUG_RETURN(1); } (*param->store_param_func)(net, param); @@ -2554,7 +2541,7 @@ static my_bool execute(MYSQL_STMT *stmt, char *packet, ulong length) stmt->insert_id= mysql->insert_id; if (res) { - set_stmt_errmsg(stmt, net->last_error, net->last_errno, net->sqlstate); + set_stmt_errmsg(stmt, net); DBUG_RETURN(1); } DBUG_RETURN(0); @@ -2577,13 +2564,13 @@ int cli_stmt_execute(MYSQL_STMT *stmt) if (!stmt->bind_param_done) { - set_stmt_error(stmt, CR_PARAMS_NOT_BOUND, unknown_sqlstate); + set_stmt_error(stmt, CR_PARAMS_NOT_BOUND, unknown_sqlstate, NULL); DBUG_RETURN(1); } if (mysql->status != MYSQL_STATUS_READY || mysql->server_status & SERVER_MORE_RESULTS_EXISTS) { - set_stmt_error(stmt, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate); + set_stmt_error(stmt, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -2592,7 +2579,7 @@ int cli_stmt_execute(MYSQL_STMT *stmt) null_count= (stmt->param_count+7) /8; if (my_realloc_str(net, null_count + 1)) { - set_stmt_error(stmt, net->last_errno, unknown_sqlstate); + set_stmt_errmsg(stmt, net); DBUG_RETURN(1); } bzero((char*) net->write_pos, null_count); @@ -2605,7 +2592,7 @@ int cli_stmt_execute(MYSQL_STMT *stmt) { if (my_realloc_str(net, 2 * stmt->param_count)) { - set_stmt_error(stmt, net->last_errno, unknown_sqlstate); + set_stmt_errmsg(stmt, net); DBUG_RETURN(1); } /* @@ -2628,7 +2615,7 @@ int cli_stmt_execute(MYSQL_STMT *stmt) /* TODO: Look into avoding the following memdup */ if (!(param_data= my_memdup(net->buff, length, MYF(0)))) { - set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate); + set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate, NULL); DBUG_RETURN(1); } result= execute(stmt, param_data, length); @@ -2692,20 +2679,19 @@ static int stmt_read_row_unbuffered(MYSQL_STMT *stmt, unsigned char **row) */ if (!mysql) { - set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate); + set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate, NULL); return 1; } if (mysql->status != MYSQL_STATUS_GET_RESULT) { set_stmt_error(stmt, stmt->unbuffered_fetch_cancelled ? CR_FETCH_CANCELED : CR_COMMANDS_OUT_OF_SYNC, - unknown_sqlstate); + unknown_sqlstate, NULL); goto error; } if ((*mysql->methods->unbuffered_fetch)(mysql, (char**) row)) { - set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, - mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); /* If there was an error, there are no more pending rows: reset statement status to not hang up in following @@ -2766,7 +2752,7 @@ stmt_read_row_from_cursor(MYSQL_STMT *stmt, unsigned char **row) buff, sizeof(buff), (uchar*) 0, 0, 1, NULL)) { - set_stmt_errmsg(stmt, net->last_error, net->last_errno, net->sqlstate); + set_stmt_errmsg(stmt, net); return 1; } if ((*mysql->methods->read_rows_from_cursor)(stmt)) @@ -2797,7 +2783,7 @@ static int stmt_read_row_no_result_set(MYSQL_STMT *stmt __attribute__((unused)), unsigned char **row __attribute__((unused))) { - set_stmt_error(stmt, CR_NO_RESULT_SET, unknown_sqlstate); + set_stmt_error(stmt, CR_NO_RESULT_SET, unknown_sqlstate, NULL); return 1; } @@ -2847,7 +2833,7 @@ my_bool STDCALL mysql_stmt_attr_set(MYSQL_STMT *stmt, } return FALSE; err_not_implemented: - set_stmt_error(stmt, CR_NOT_IMPLEMENTED, unknown_sqlstate); + set_stmt_error(stmt, CR_NOT_IMPLEMENTED, unknown_sqlstate, NULL); return TRUE; } @@ -3232,7 +3218,7 @@ my_bool STDCALL mysql_stmt_bind_param(MYSQL_STMT *stmt, MYSQL_BIND *my_bind) { if ((int) stmt->state < (int) MYSQL_STMT_PREPARE_DONE) { - set_stmt_error(stmt, CR_NO_PREPARE_STMT, unknown_sqlstate); + set_stmt_error(stmt, CR_NO_PREPARE_STMT, unknown_sqlstate, NULL); DBUG_RETURN(1); } DBUG_RETURN(0); @@ -3397,7 +3383,7 @@ mysql_stmt_send_long_data(MYSQL_STMT *stmt, uint param_number, */ if (param_number >= stmt->param_count) { - set_stmt_error(stmt, CR_INVALID_PARAMETER_NO, unknown_sqlstate); + set_stmt_error(stmt, CR_INVALID_PARAMETER_NO, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -3433,8 +3419,7 @@ mysql_stmt_send_long_data(MYSQL_STMT *stmt, uint param_number, buff, sizeof(buff), (uchar*) data, length, 1, NULL)) { - set_stmt_errmsg(stmt, mysql->net.last_error, - mysql->net.last_errno, mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); DBUG_RETURN(1); } } @@ -3903,7 +3888,8 @@ static void fetch_float_with_conversion(MYSQL_BIND *param, MYSQL_FIELD *field, if (field->flags & ZEROFILL_FLAG && length < field->length && field->length < MAX_DOUBLE_STRING_REP_LENGTH - 1) { - bmove_upp((char*) buff + field->length, buff + length, length); + bmove_upp((uchar*) buff + field->length, (uchar*) buff + length, + length); bfill((char*) buff, field->length - length, '0'); length= field->length; } @@ -4502,7 +4488,7 @@ my_bool STDCALL mysql_stmt_bind_result(MYSQL_STMT *stmt, MYSQL_BIND *my_bind) { int errorcode= (int) stmt->state < (int) MYSQL_STMT_PREPARE_DONE ? CR_NO_PREPARE_STMT : CR_NO_STMT_METADATA; - set_stmt_error(stmt, errorcode, unknown_sqlstate); + set_stmt_error(stmt, errorcode, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -4682,12 +4668,12 @@ int STDCALL mysql_stmt_fetch_column(MYSQL_STMT *stmt, MYSQL_BIND *my_bind, if ((int) stmt->state < (int) MYSQL_STMT_FETCH_DONE) { - set_stmt_error(stmt, CR_NO_DATA, unknown_sqlstate); + set_stmt_error(stmt, CR_NO_DATA, unknown_sqlstate, NULL); return 1; } if (column >= stmt->field_count) { - set_stmt_error(stmt, CR_INVALID_PARAMETER_NO, unknown_sqlstate); + set_stmt_error(stmt, CR_INVALID_PARAMETER_NO, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -4733,7 +4719,7 @@ int cli_read_binary_rows(MYSQL_STMT *stmt) if (!mysql) { - set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate); + set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -4748,7 +4734,7 @@ int cli_read_binary_rows(MYSQL_STMT *stmt) if (!(cur= (MYSQL_ROWS*) alloc_root(&result->alloc, sizeof(MYSQL_ROWS) + pkt_len - 1))) { - set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate); + set_stmt_error(stmt, CR_OUT_OF_MEMORY, unknown_sqlstate, NULL); goto err; } cur->data= (MYSQL_ROW) (cur+1); @@ -4769,7 +4755,7 @@ int cli_read_binary_rows(MYSQL_STMT *stmt) DBUG_RETURN(0); } } - set_stmt_errmsg(stmt, net->last_error, net->last_errno, net->sqlstate); + set_stmt_errmsg(stmt, net); err: DBUG_RETURN(1); @@ -4836,7 +4822,7 @@ int STDCALL mysql_stmt_store_result(MYSQL_STMT *stmt) if ((int) stmt->state < (int) MYSQL_STMT_EXECUTE_DONE) { - set_stmt_error(stmt, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate); + set_stmt_error(stmt, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -4856,13 +4842,13 @@ int STDCALL mysql_stmt_store_result(MYSQL_STMT *stmt) if (cli_advanced_command(mysql, COM_STMT_FETCH, buff, sizeof(buff), (uchar*) 0, 0, 1, NULL)) { - set_stmt_errmsg(stmt, net->last_error, net->last_errno, net->sqlstate); + set_stmt_errmsg(stmt, net); DBUG_RETURN(1); } } else if (mysql->status != MYSQL_STATUS_GET_RESULT) { - set_stmt_error(stmt, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate); + set_stmt_error(stmt, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate, NULL); DBUG_RETURN(1); } @@ -5043,8 +5029,7 @@ static my_bool reset_stmt_handle(MYSQL_STMT *stmt, uint flags) if ((*mysql->methods->advanced_command)(mysql, COM_STMT_RESET, buff, sizeof(buff), 0, 0, 0, NULL)) { - set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, - mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); stmt->state= MYSQL_STMT_INIT_DONE; return 1; } @@ -5117,8 +5102,7 @@ my_bool STDCALL mysql_stmt_close(MYSQL_STMT *stmt) int4store(buff, stmt->stmt_id); if ((rc= stmt_command(mysql, COM_STMT_CLOSE, buff, 4, stmt))) { - set_stmt_errmsg(stmt, mysql->net.last_error, mysql->net.last_errno, - mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); } } } @@ -5139,7 +5123,7 @@ my_bool STDCALL mysql_stmt_reset(MYSQL_STMT *stmt) if (!stmt->mysql) { /* mysql can be reset in mysql_close called from mysql_reconnect */ - set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate); + set_stmt_error(stmt, CR_SERVER_LOST, unknown_sqlstate, NULL); DBUG_RETURN(1); } /* Reset the client and server sides of the prepared statement */ @@ -5243,15 +5227,11 @@ int STDCALL mysql_next_result(MYSQL *mysql) if (mysql->status != MYSQL_STATUS_READY) { - strmov(mysql->net.sqlstate, unknown_sqlstate); - strmov(mysql->net.last_error, - ER(mysql->net.last_errno=CR_COMMANDS_OUT_OF_SYNC)); + set_mysql_error(mysql, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate); DBUG_RETURN(1); } - mysql->net.last_error[0]= 0; - mysql->net.last_errno= 0; - strmov(mysql->net.sqlstate, not_error_sqlstate); + net_clear_error(&mysql->net); mysql->affected_rows= ~(my_ulonglong) 0; if (mysql->last_used_con->server_status & SERVER_MORE_RESULTS_EXISTS) diff --git a/libmysqld/lib_sql.cc b/libmysqld/lib_sql.cc index 4e525f8447f..13847c324e1 100644 --- a/libmysqld/lib_sql.cc +++ b/libmysqld/lib_sql.cc @@ -81,8 +81,7 @@ emb_advanced_command(MYSQL *mysql, enum enum_server_command command, /* Check that we are calling the client functions in right order */ if (mysql->status != MYSQL_STATUS_READY) { - strmov(net->last_error, - ER(net->last_errno=CR_COMMANDS_OUT_OF_SYNC)); + set_mysql_error(mysql, CR_COMMANDS_OUT_OF_SYNC, unknown_sqlstate); return 1; } @@ -90,7 +89,7 @@ emb_advanced_command(MYSQL *mysql, enum enum_server_command command, thd->clear_error(); mysql->affected_rows= ~(my_ulonglong) 0; mysql->field_count= 0; - net->last_errno= 0; + net_clear_error(net); thd->current_stmt= stmt; thd->store_globals(); // Fix if more than one connect @@ -245,8 +244,7 @@ static my_bool emb_read_query_result(MYSQL *mysql) mysql->fields= res->embedded_info->fields_list; mysql->affected_rows= res->embedded_info->affected_rows; mysql->insert_id= res->embedded_info->insert_id; - mysql->net.last_errno= 0; - mysql->net.last_error[0]= 0; + net_clear_error(&mysql->net); mysql->info= 0; if (res->embedded_info->info[0]) @@ -288,7 +286,7 @@ static int emb_stmt_execute(MYSQL_STMT *stmt) if (res) { NET *net= &stmt->mysql->net; - set_stmt_errmsg(stmt, net->last_error, net->last_errno, net->sqlstate); + set_stmt_errmsg(stmt, net); DBUG_RETURN(1); } DBUG_RETURN(0); @@ -299,14 +297,12 @@ int emb_read_binary_rows(MYSQL_STMT *stmt) MYSQL_DATA *data; if (!(data= emb_read_rows(stmt->mysql, 0, 0))) { - set_stmt_errmsg(stmt, stmt->mysql->net.last_error, - stmt->mysql->net.last_errno, stmt->mysql->net.sqlstate); + set_stmt_errmsg(stmt, &stmt->mysql->net); return 1; } stmt->result= *data; my_free((char *) data, MYF(0)); - set_stmt_errmsg(stmt, stmt->mysql->net.last_error, - stmt->mysql->net.last_errno, stmt->mysql->net.sqlstate); + set_stmt_errmsg(stmt, &stmt->mysql->net); return 0; } @@ -320,16 +316,14 @@ int emb_read_rows_from_cursor(MYSQL_STMT *stmt) if (res->embedded_info->last_errno) { embedded_get_error(mysql, res); - set_stmt_errmsg(stmt, mysql->net.last_error, - mysql->net.last_errno, mysql->net.sqlstate); + set_stmt_errmsg(stmt, &mysql->net); return 1; } thd->cur_data= res; mysql->warning_count= res->embedded_info->warning_count; mysql->server_status= res->embedded_info->server_status; - mysql->net.last_errno= 0; - mysql->net.last_error[0]= 0; + net_clear_error(&mysql->net); return emb_read_binary_rows(stmt); } diff --git a/sql-common/client.c b/sql-common/client.c index baf154dcc43..a6e02376a40 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -312,42 +312,34 @@ HANDLE create_named_pipe(NET *net, uint connect_timeout, char **arg_host, break; if (GetLastError() != ERROR_PIPE_BUSY) { - net->last_errno=CR_NAMEDPIPEOPEN_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno), host, unix_socket, - (ulong) GetLastError()); + set_mysql_extended_error(mysql, CR_NAMEDPIPEOPEN_ERROR, + unknown_sqlstate, ER(CR_NAMEDPIPEOPEN_ERROR), + host, unix_socket, (ulong) GetLastError()); return INVALID_HANDLE_VALUE; } /* wait for for an other instance */ if (! WaitNamedPipe(pipe_name, connect_timeout*1000) ) { - net->last_errno=CR_NAMEDPIPEWAIT_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno), host, unix_socket, - (ulong) GetLastError()); + set_mysql_extended_error(mysql, CR_NAMEDPIPEWAIT_ERROR, unknown_sqlstate, + ER(CR_NAMEDPIPEWAIT_ERROR), + host, unix_socket, (ulong) GetLastError()); return INVALID_HANDLE_VALUE; } } if (hPipe == INVALID_HANDLE_VALUE) { - net->last_errno=CR_NAMEDPIPEOPEN_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno), host, unix_socket, - (ulong) GetLastError()); + set_mysql_extended_error(mysql, CR_NAMEDPIPEOPEN_ERROR, unknown_sqlstate, + ER(CR_NAMEDPIPEOPEN_ERROR), host, unix_socket, + (ulong) GetLastError()); return INVALID_HANDLE_VALUE; } dwMode = PIPE_READMODE_BYTE | PIPE_WAIT; if ( !SetNamedPipeHandleState(hPipe, &dwMode, NULL, NULL) ) { CloseHandle( hPipe ); - net->last_errno=CR_NAMEDPIPESETSTATE_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno),host, unix_socket, - (ulong) GetLastError()); + set_mysql_extended_error(mysql, CR_NAMEDPIPESETSTATE_ERROR, + unknown_sqlstate, ER(CR_NAMEDPIPESETSTATE_ERROR), + host, unix_socket, (ulong) GetLastError()); return INVALID_HANDLE_VALUE; } *arg_host=host ; *arg_unix_socket=unix_socket; /* connect arg */ @@ -566,14 +558,12 @@ err: CloseHandle(handle_connect_file_map); if (error_allow) { - net->last_errno=error_allow; - strmov(net->sqlstate, unknown_sqlstate); if (error_allow == CR_SHARED_MEMORY_EVENT_ERROR) - my_snprintf(net->last_error,sizeof(net->last_error)-1, - ER(net->last_errno),suffix_pos,error_code); + set_mysql_extended_error(mysql, error_allow, unknown_sqlstate, + ER(error_allow), suffix_pos, error_code); else - my_snprintf(net->last_error,sizeof(net->last_error)-1, - ER(net->last_errno),error_code); + set_mysql_extended_error(mysql, error_allow, unknown_sqlstate, + ER(error_allow), error_code); return(INVALID_HANDLE_VALUE); } return(handle_map); @@ -683,10 +673,8 @@ cli_advanced_command(MYSQL *mysql, enum enum_server_command command, DBUG_RETURN(1); } - net->last_error[0]=0; - net->last_errno= 0; - strmov(net->sqlstate, not_error_sqlstate); - mysql->net.report_error=0; + net_clear_error(net); + net->report_error=0; mysql->info=0; mysql->affected_rows= ~(my_ulonglong) 0; /* @@ -703,8 +691,7 @@ cli_advanced_command(MYSQL *mysql, enum enum_server_command command, socket_errno)); if (net->last_errno == ER_NET_PACKET_TOO_LARGE) { - net->last_errno=CR_NET_PACKET_TOO_LARGE; - strmov(net->last_error,ER(net->last_errno)); + set_mysql_error(mysql, CR_NET_PACKET_TOO_LARGE, unknown_sqlstate); goto end; } end_server(mysql); @@ -713,8 +700,7 @@ cli_advanced_command(MYSQL *mysql, enum enum_server_command command, if (net_write_command(net,(uchar) command, header, header_length, arg, arg_length)) { - net->last_errno=CR_SERVER_GONE_ERROR; - strmov(net->last_error,ER(net->last_errno)); + set_mysql_error(mysql, CR_SERVER_GONE_ERROR, unknown_sqlstate); goto end; } } @@ -760,6 +746,19 @@ void set_mysql_error(MYSQL *mysql, int errcode, const char *sqlstate) DBUG_VOID_RETURN; } +/** + Clear possible error state of struct NET + + @param net clear the state of the argument +*/ + +void net_clear_error(NET *net) +{ + net->last_errno= 0; + net->last_error[0]= '\0'; + strmov(net->sqlstate, not_error_sqlstate); +} + static void set_mysql_extended_error(MYSQL *mysql, int errcode, const char *sqlstate, @@ -846,9 +845,8 @@ static int check_license(MYSQL *mysql) { if (net->last_errno == ER_UNKNOWN_SYSTEM_VARIABLE) { - net->last_errno= CR_WRONG_LICENSE; - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno), required_license); + set_mysql_extended_error(mysql, CR_WRONG_LICENSE, unknown_sqlstate, + ER(CR_WRONG_LICENSE), required_license); } return 1; } @@ -864,9 +862,8 @@ static int check_license(MYSQL *mysql) (!row || !row[0] || strncmp(row[0], required_license, sizeof(required_license)))) { - net->last_errno= CR_WRONG_LICENSE; - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno), required_license); + set_mysql_extended_error(mysql, CR_WRONG_LICENSE, unknown_sqlstate, + ER(CR_WRONG_LICENSE), required_license); } mysql_free_result(res); return net->last_errno; @@ -1761,24 +1758,22 @@ int mysql_init_character_set(MYSQL *mysql) } charsets_dir= save; } - + if (!mysql->charset) { - net->last_errno=CR_CANT_READ_CHARSET; - strmov(net->sqlstate, unknown_sqlstate); if (mysql->options.charset_dir) - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno), - mysql->options.charset_name, - mysql->options.charset_dir); + set_mysql_extended_error(mysql, CR_CANT_READ_CHARSET, unknown_sqlstate, + ER(CR_CANT_READ_CHARSET), + mysql->options.charset_name, + mysql->options.charset_dir); else { char cs_dir_name[FN_REFLEN]; get_charsets_dir(cs_dir_name); - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(net->last_errno), - mysql->options.charset_name, - cs_dir_name); + set_mysql_extended_error(mysql, CR_CANT_READ_CHARSET, unknown_sqlstate, + ER(CR_CANT_READ_CHARSET), + mysql->options.charset_name, + cs_dir_name); } return 1; } @@ -1910,10 +1905,10 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, DBUG_PRINT("info",("Using UNIX sock '%s'",unix_socket)); if ((sock = socket(AF_UNIX,SOCK_STREAM,0)) == SOCKET_ERROR) { - net->last_errno=CR_SOCKET_CREATE_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error,sizeof(net->last_error)-1, - ER(net->last_errno),socket_errno); + set_mysql_extended_error(mysql, CR_SOCKET_CREATE_ERROR, + unknown_sqlstate, + ER(CR_SOCKET_CREATE_ERROR), + socket_errno); goto error; } net->vio= vio_new(sock, VIO_TYPE_SOCKET, @@ -1926,10 +1921,10 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, { DBUG_PRINT("error",("Got error %d on connect to local server", socket_errno)); - net->last_errno=CR_CONNECTION_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error,sizeof(net->last_error)-1, - ER(net->last_errno),unix_socket,socket_errno); + set_mysql_extended_error(mysql, CR_CONNECTION_ERROR, + unknown_sqlstate, + ER(CR_CONNECTION_ERROR), + unix_socket, socket_errno); goto error; } mysql->options.protocol=MYSQL_PROTOCOL_SOCKET; @@ -1986,10 +1981,8 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, #endif if (sock == SOCKET_ERROR) { - net->last_errno=CR_IPSOCK_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error,sizeof(net->last_error)-1, - ER(net->last_errno),socket_errno); + set_mysql_extended_error(mysql, CR_IPSOCK_ERROR, unknown_sqlstate, + ER(CR_IPSOCK_ERROR), socket_errno); goto error; } net->vio= vio_new(sock, VIO_TYPE_TCPIP, VIO_BUFFERED_READ); @@ -2014,10 +2007,8 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, if (!hp) { my_gethostbyname_r_free(); - net->last_errno=CR_UNKNOWN_HOST; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(CR_UNKNOWN_HOST), host, tmp_errno); + set_mysql_extended_error(mysql, CR_UNKNOWN_HOST, unknown_sqlstate, + ER(CR_UNKNOWN_HOST), host, tmp_errno); goto error; } memcpy(&sock_addr.sin_addr, hp->h_addr, @@ -2030,10 +2021,8 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, { DBUG_PRINT("error",("Got error %d on connect to '%s'",socket_errno, host)); - net->last_errno= CR_CONN_HOST_ERROR; - strmov(net->sqlstate, unknown_sqlstate); - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(CR_CONN_HOST_ERROR), host, socket_errno); + set_mysql_extended_error(mysql, CR_CONN_HOST_ERROR, unknown_sqlstate, + ER(CR_CONN_HOST_ERROR), host, socket_errno); goto error; } } @@ -2097,11 +2086,9 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, PROTOCOL_VERSION, mysql->protocol_version)); if (mysql->protocol_version != PROTOCOL_VERSION) { - strmov(net->sqlstate, unknown_sqlstate); - net->last_errno= CR_VERSION_ERROR; - my_snprintf(net->last_error, sizeof(net->last_error)-1, - ER(CR_VERSION_ERROR), mysql->protocol_version, - PROTOCOL_VERSION); + set_mysql_extended_error(mysql, CR_VERSION_ERROR, unknown_sqlstate, + ER(CR_VERSION_ERROR), mysql->protocol_version, + PROTOCOL_VERSION); goto error; } end=strend((char*) net->read_pos+1); @@ -2625,7 +2612,7 @@ void mysql_detach_stmt_list(LIST **stmt_list __attribute__((unused)), for (; element; element= element->next) { MYSQL_STMT *stmt= (MYSQL_STMT *) element->data; - set_stmt_errmsg(stmt, buff, CR_STMT_CLOSED, unknown_sqlstate); + set_stmt_error(stmt, CR_STMT_CLOSED, unknown_sqlstate, buff); stmt->mysql= 0; /* No need to call list_delete for statement here */ } @@ -3142,11 +3129,8 @@ int STDCALL mysql_set_character_set(MYSQL *mysql, const char *cs_name) { char cs_dir_name[FN_REFLEN]; get_charsets_dir(cs_dir_name); - mysql->net.last_errno= CR_CANT_READ_CHARSET; - strmov(mysql->net.sqlstate, unknown_sqlstate); - my_snprintf(mysql->net.last_error, sizeof(mysql->net.last_error) - 1, - ER(mysql->net.last_errno), cs_name, cs_dir_name); - + set_mysql_extended_error(mysql, CR_CANT_READ_CHARSET, unknown_sqlstate, + ER(CR_CANT_READ_CHARSET), cs_name, cs_dir_name); } charsets_dir= save_csdir; return mysql->net.last_errno; From 9ad4366b7b4b6812b653be65576d9b561ad3a41c Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 31 Oct 2007 18:33:13 +0300 Subject: [PATCH 18/31] Cleanup: rename select_send::status to select_send::is_result_set_started. Add select_send::cleanup. Fix a compilation warning. Issues spotted while working on the fix for Bug#12713. sql-common/client.c: Fix a warning. sql/sql_class.cc: Give a variable a more specific name. Rewrite an incorrect comment. Add a cleanup for select_send. The only case now this cleanup can be necessary is when we have a prepared statement inside a stored procedure, and a continue handler. At first execution, the statement is killed after having executed select_send::send_fields. At the second execution it is killed after having executed select_send::send_fields. sql/sql_class.h: Rename a member. Add comments. --- sql-common/client.c | 1 - sql/sql_class.cc | 30 ++++++++++++++++++++++-------- sql/sql_class.h | 10 ++++++++-- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/sql-common/client.c b/sql-common/client.c index a6e02376a40..e92a9bd0cdc 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -1714,7 +1714,6 @@ static MYSQL_METHODS client_methods= C_MODE_START int mysql_init_character_set(MYSQL *mysql) { - NET *net= &mysql->net; const char *default_collation_name; /* Set character set */ diff --git a/sql/sql_class.cc b/sql/sql_class.cc index b5a29783044..a904023cbff 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -1305,23 +1305,26 @@ bool select_send::send_fields(List &list, uint flags) { bool res; if (!(res= thd->protocol->send_fields(&list, flags))) - status= 1; + is_result_set_started= 1; return res; } void select_send::abort() { DBUG_ENTER("select_send::abort"); - if (status && thd->spcont && + if (is_result_set_started && thd->spcont && thd->spcont->find_handler(thd, thd->net.last_errno, MYSQL_ERROR::WARN_LEVEL_ERROR)) { /* - Executing stored procedure without a handler. - Here we should actually send an error to the client, - but as an error will break a multiple result set, the only thing we - can do for now is to nicely end the current data set and remembering - the error so that the calling routine will abort + We're executing a stored procedure, have an open result + set, an SQL exception conditiona and a handler for it. + In this situation we must abort the current statement, + silence the error and start executing the continue/exit + handler. + Before aborting the statement, let's end the open result set, as + otherwise the client will hang due to the violation of the + client/server protocol. */ thd->net.report_error= 0; send_eof(); @@ -1331,6 +1334,17 @@ void select_send::abort() } +/** + Cleanup an instance of this class for re-use + at next execution of a prepared statement/ + stored procedure statement. +*/ + +void select_send::cleanup() +{ + is_result_set_started= FALSE; +} + /* Send data to client. Returns 0 if ok */ bool select_send::send_data(List &items) @@ -1392,7 +1406,7 @@ bool select_send::send_eof() if (! thd->is_error()) { ::send_eof(thd); - status= 0; + is_result_set_started= 0; return 0; } else diff --git a/sql/sql_class.h b/sql/sql_class.h index db847c814a0..632c440266f 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2051,14 +2051,20 @@ public: class select_send :public select_result { - int status; + /** + True if we have sent result set metadata to the client. + In this case the client always expects us to end the result + set with an eof or error packet + */ + bool is_result_set_started; public: - select_send() :status(0) {} + select_send() :is_result_set_started(FALSE) {} bool send_fields(List &list, uint flags); bool send_data(List &items); bool send_eof(); virtual bool check_simple_select() const { return FALSE; } void abort(); + virtual void cleanup(); }; From 10ed5ae79420ec7081c0a3db725690b482e7fcc3 Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 31 Oct 2007 13:54:53 -0200 Subject: [PATCH 19/31] Post merge fix for bug 31669. tests/mysql_client_test.c: Macro name changed in 5.1 --- tests/mysql_client_test.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 808d6fe30de..0cb9ff49128 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -17112,8 +17112,8 @@ static void test_bug31669() int rc; static char buff[LARGE_BUFFER_SIZE+1]; #ifndef EMBEDDED_LIBRARY - static char user[USERNAME_LENGTH+1]; - static char db[NAME_LEN+1]; + static char user[USERNAME_CHAR_LENGTH+1]; + static char db[NAME_CHAR_LEN+1]; static char query[LARGE_BUFFER_SIZE*2]; #endif @@ -17136,13 +17136,13 @@ static void test_bug31669() #ifndef EMBEDDED_LIBRARY memset(db, 'a', sizeof(db)); - db[NAME_LEN]= 0; + db[NAME_CHAR_LEN]= 0; strxmov(query, "CREATE DATABASE IF NOT EXISTS ", db, NullS); rc= mysql_query(mysql, query); myquery(rc); memset(user, 'b', sizeof(user)); - user[USERNAME_LENGTH]= 0; + user[USERNAME_CHAR_LENGTH]= 0; memset(buff, 'c', sizeof(buff)); buff[LARGE_BUFFER_SIZE]= 0; strxmov(query, "GRANT ALL PRIVILEGES ON *.* TO '", user, "'@'%' IDENTIFIED BY " @@ -17156,21 +17156,21 @@ static void test_bug31669() rc= mysql_change_user(mysql, user, buff, db); DIE_UNLESS(!rc); - user[USERNAME_LENGTH-1]= 'a'; + user[USERNAME_CHAR_LENGTH-1]= 'a'; rc= mysql_change_user(mysql, user, buff, db); DIE_UNLESS(rc); - user[USERNAME_LENGTH-1]= 'b'; + user[USERNAME_CHAR_LENGTH-1]= 'b'; buff[LARGE_BUFFER_SIZE-1]= 'd'; rc= mysql_change_user(mysql, user, buff, db); DIE_UNLESS(rc); buff[LARGE_BUFFER_SIZE-1]= 'c'; - db[NAME_LEN-1]= 'e'; + db[NAME_CHAR_LEN-1]= 'e'; rc= mysql_change_user(mysql, user, buff, db); DIE_UNLESS(rc); - db[NAME_LEN-1]= 'a'; + db[NAME_CHAR_LEN-1]= 'a'; rc= mysql_change_user(mysql, user, buff, db); DIE_UNLESS(!rc); From d757ae80d66a7c7fba582b5979e9aee08712aa1a Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 31 Oct 2007 23:48:41 +0300 Subject: [PATCH 20/31] Try to fix a Windows compilation warning. sql-common/client.c: Move a block of auxiliary functions to the beginning of the file, to avoid compilation warnings on Windows. Add comments, albeit trivial, while we're at it. --- sql-common/client.c | 125 +++++++++++++++++++++++++------------------- 1 file changed, 70 insertions(+), 55 deletions(-) diff --git a/sql-common/client.c b/sql-common/client.c index e92a9bd0cdc..5ece9c39d6c 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -272,6 +272,76 @@ static int wait_for_data(my_socket fd, uint timeout) } #endif /* defined(__WIN__) || defined(__NETWARE__) */ +/** + Set the internal error message to mysql handler + + @param mysql connection handle (client side) + @param errcode CR_ error code, passed to ER macro to get + error text + @parma sqlstate SQL standard sqlstate +*/ + +void set_mysql_error(MYSQL *mysql, int errcode, const char *sqlstate) +{ + NET *net; + DBUG_ENTER("set_mysql_error"); + DBUG_PRINT("enter", ("error :%d '%s'", errcode, ER(errcode))); + DBUG_ASSERT(mysql != 0); + + net= &mysql->net; + net->last_errno= errcode; + strmov(net->last_error, ER(errcode)); + strmov(net->sqlstate, sqlstate); + + DBUG_VOID_RETURN; +} + +/** + Clear possible error state of struct NET + + @param net clear the state of the argument +*/ + +void net_clear_error(NET *net) +{ + net->last_errno= 0; + net->last_error[0]= '\0'; + strmov(net->sqlstate, not_error_sqlstate); +} + +/** + Set an error message on the client. + + @param mysql connection handle + @param errcode CR_* errcode, for client errors + @param sqlstate SQL standard sql state, unknown_sqlstate for the + majority of client errors. + @param format error message template, in sprintf format + @param ... variable number of arguments +*/ + +static void set_mysql_extended_error(MYSQL *mysql, int errcode, + const char *sqlstate, + const char *format, ...) +{ + NET *net; + va_list args; + DBUG_ENTER("set_mysql_extended_error"); + DBUG_PRINT("enter", ("error :%d '%s'", errcode, format)); + DBUG_ASSERT(mysql != 0); + + net= &mysql->net; + net->last_errno= errcode; + va_start(args, format); + my_vsnprintf(net->last_error, sizeof(net->last_error)-1, + format, args); + va_end(args); + strmov(net->sqlstate, sqlstate); + + DBUG_VOID_RETURN; +} + + /* Create a named pipe connection @@ -727,61 +797,6 @@ void free_old_query(MYSQL *mysql) DBUG_VOID_RETURN; } -/* - Set the internal error message to mysql handler -*/ - -void set_mysql_error(MYSQL *mysql, int errcode, const char *sqlstate) -{ - NET *net; - DBUG_ENTER("set_mysql_error"); - DBUG_PRINT("enter", ("error :%d '%s'", errcode, ER(errcode))); - DBUG_ASSERT(mysql != 0); - - net= &mysql->net; - net->last_errno= errcode; - strmov(net->last_error, ER(errcode)); - strmov(net->sqlstate, sqlstate); - - DBUG_VOID_RETURN; -} - -/** - Clear possible error state of struct NET - - @param net clear the state of the argument -*/ - -void net_clear_error(NET *net) -{ - net->last_errno= 0; - net->last_error[0]= '\0'; - strmov(net->sqlstate, not_error_sqlstate); -} - - -static void set_mysql_extended_error(MYSQL *mysql, int errcode, - const char *sqlstate, - const char *format, ...) -{ - NET *net; - va_list args; - DBUG_ENTER("set_mysql_extended_error"); - DBUG_PRINT("enter", ("error :%d '%s'", errcode, format)); - DBUG_ASSERT(mysql != 0); - - net= &mysql->net; - net->last_errno= errcode; - va_start(args, format); - my_vsnprintf(net->last_error, sizeof(net->last_error)-1, - format, args); - va_end(args); - strmov(net->sqlstate, sqlstate); - - DBUG_VOID_RETURN; -} - - /* Flush result set sent from server */ From bfae4730530ed6ae42739d560d655eb570641625 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 00:10:58 +0300 Subject: [PATCH 21/31] Remove net_printf_error(). Do not talk to network directly in check_user()/check_connection()/check_for_max_user_connections(). This is a pre-requisite patch for the fix for Bug#12713 "Error in a stored function called from a SELECT doesn't cause ROLLBACK of statem" Implement review comments. sql/mysql_priv.h: check_for_max_user_connections() is used in one place only, make it static. sql/mysqld.cc: Remove net_printf_error(): a consolidation of error reporting facilities is necessary to simplify maintenance of the query cache, the client-server protocol, stored procedure continue handlers. Rewrite the only place where its use is somewhat justified (my_error() can not be used since we need to report an error for the thread that does not exist) with my_snprintf()/net_send_error(). sql/protocol.cc: Remove net_printf_error(). sql/protocol.h: Remove net_printf_error(). sql/sql_connect.cc: Remove net_printf_error(). In check_connection()/check_user()/ check_for_max_user_connections() do not write directly to the network, but use the standard my_error() mechanism to record an error in THD. It will be sent to the client by the caller. This was the last place in the server that would attempt to send an error directly, mainly left untouched by 5.0 refactoring because it is executed only during thread startup. sql/sql_parse.cc: In the old code, when res was greater than 0, it contained an exact error code, e.g. ER_OUT_OF_RESOURCES or NO SUCH DATABASE, or ER_HANDSHAKE_ERROR. I don't know the reason why this error code was ignored, and instead a generic ER_UNKNOWN_COM_ERROR was pushed into the error stack, but knowing the relaxed attitude towards preserving the error codes in the old code, I'm inclinded to think that it was a bug. After this patch, the most specific error message is already pushed, so calling my_message() again is useless. If res is < 0, the error used to be already sent. This is not done by the new code, but will be done later, in the end of dispatch_command(). When this is done, clear_error() will be called for us - it is in the first lines of do_command. To sum up, this change is to remove COM_CHANGE_USER specific error handling in favor of the standard one employed for all other COM_* commands. --- sql/mysql_priv.h | 1 - sql/mysqld.cc | 6 +- sql/protocol.cc | 120 +---------------------------- sql/protocol.h | 1 - sql/sql_connect.cc | 183 +++++++++++++++++++++++++-------------------- sql/sql_parse.cc | 5 -- 6 files changed, 108 insertions(+), 208 deletions(-) diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 3b88fe0fca8..3484b8096e3 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -920,7 +920,6 @@ bool init_new_connection_handler_thread(); void reset_mqh(LEX_USER *lu, bool get_them); bool check_mqh(THD *thd, uint check_command); void time_out_user_resource_limits(THD *thd, USER_CONN *uc); -int check_for_max_user_connections(THD *thd, USER_CONN *uc); void decrease_user_connections(USER_CONN *uc); void thd_init_client_charset(THD *thd, uint cs_number); bool setup_connection_thread_globals(THD *thd); diff --git a/sql/mysqld.cc b/sql/mysqld.cc index c76bd8bd712..4cf6e05751f 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4302,6 +4302,7 @@ void create_thread_to_handle_connection(THD *thd) } else { + char error_message_buff[MYSQL_ERRMSG_SIZE]; /* Create new thread to handle connection */ int error; thread_created++; @@ -4320,7 +4321,10 @@ void create_thread_to_handle_connection(THD *thd) thd->killed= THD::KILL_CONNECTION; // Safety (void) pthread_mutex_unlock(&LOCK_thread_count); statistic_increment(aborted_connects,&LOCK_status); - net_printf_error(thd, ER_CANT_CREATE_THREAD, error); + /* Can't use my_error() since store_globals has not been called. */ + my_snprintf(error_message_buff, sizeof(error_message_buff), + ER(ER_CANT_CREATE_THREAD), error); + net_send_error(thd, ER_CANT_CREATE_THREAD, error_message_buff); (void) pthread_mutex_lock(&LOCK_thread_count); close_connection(thd,0,0); delete thd; diff --git a/sql/protocol.cc b/sql/protocol.cc index 51d408de9de..bf8faec006a 100644 --- a/sql/protocol.cc +++ b/sql/protocol.cc @@ -58,7 +58,7 @@ bool Protocol_binary::net_store_data(const uchar *from, size_t length) Design note: - net_printf_error and net_send_error are low-level functions + net_send_error is a low-level functions that shall be used only when a new connection is being established or at server startup. For SIGNAL/RESIGNAL and GET DIAGNOSTICS functionality it's @@ -120,124 +120,6 @@ void net_send_error(THD *thd, uint sql_errno, const char *err) DBUG_VOID_RETURN; } -/* - Write error package and flush to client - It's a little too low level, but I don't want to use another buffer for - this - - Design note: - - net_printf_error and net_send_error are low-level functions - that shall be used only when a new connection is being - established or at server startup. - For SIGNAL/RESIGNAL and GET DIAGNOSTICS functionality it's - critical that every error that can be intercepted is issued in one - place only, my_message_sql. -*/ - -void -net_printf_error(THD *thd, uint errcode, ...) -{ - va_list args; - uint length,offset; - const char *format; -#ifndef EMBEDDED_LIBRARY - const char *text_pos; - int head_length= NET_HEADER_SIZE; -#else - char text_pos[1024]; -#endif - NET *net= &thd->net; - - DBUG_ENTER("net_printf_error"); - DBUG_PRINT("enter",("message: %u",errcode)); - - DBUG_ASSERT(!thd->spcont); - - if (net && net->no_send_error) - { - thd->clear_error(); - thd->is_fatal_error= 0; // Error message is given - DBUG_PRINT("info", ("sending error messages prohibited")); - DBUG_VOID_RETURN; - } - - thd->is_slave_error= 1; // needed to catch query errors during replication -#ifndef EMBEDDED_LIBRARY - query_cache_abort(net); // Safety -#endif - va_start(args,errcode); - /* - The following is needed to make net_printf_error() work with 0 argument - for errorcode and use the argument after that as the format string. This - is useful for rare errors that are not worth the hassle to put in - errmsg.sys, but at the same time, the message is not fixed text - */ - if (errcode) - format= ER(errcode); - else - { - format=va_arg(args,char*); - errcode= ER_UNKNOWN_ERROR; - } - offset= (net->return_errno ? - ((thd->client_capabilities & CLIENT_PROTOCOL_41) ? - 2+SQLSTATE_LENGTH+1 : 2) : 0); -#ifndef EMBEDDED_LIBRARY - text_pos=(char*) net->buff + head_length + offset + 1; - length= (uint) ((char*)net->buff_end - text_pos); -#else - length=sizeof(text_pos)-1; -#endif - length=my_vsnprintf(my_const_cast(char*) (text_pos), - min(length, sizeof(net->last_error)), - format,args); - va_end(args); - - /* Replication slave relies on net->last_* to see if there was error */ - net->last_errno= errcode; - strmake(net->last_error, text_pos, sizeof(net->last_error)-1); - -#ifndef EMBEDDED_LIBRARY - if (net->vio == 0) - { - if (thd->bootstrap) - { - /* - In bootstrap it's ok to print on stderr - This may also happen when we get an error from a slave thread - */ - fprintf(stderr,"ERROR: %d %s\n",errcode,text_pos); - thd->fatal_error(); - } - DBUG_VOID_RETURN; - } - - int3store(net->buff,length+1+offset); - net->buff[3]= (net->compress) ? 0 : (uchar) (net->pkt_nr++); - net->buff[head_length]=(uchar) 255; // Error package - if (offset) - { - uchar *pos= net->buff+head_length+1; - int2store(pos, errcode); - if (thd->client_capabilities & CLIENT_PROTOCOL_41) - { - pos[2]= '#'; /* To make the protocol backward compatible */ - memcpy(pos+3, mysql_errno_to_sqlstate(errcode), SQLSTATE_LENGTH); - } - } - VOID(net_real_write(net, net->buff, length+head_length+1+offset)); -#else - net->last_errno= errcode; - strmake(net->last_error, text_pos, length); - strmake(net->sqlstate, mysql_errno_to_sqlstate(errcode), SQLSTATE_LENGTH); -#endif - if (thd->killed != THD::KILL_CONNECTION) - push_warning(thd, MYSQL_ERROR::WARN_LEVEL_ERROR, errcode, - text_pos ? text_pos : ER(errcode)); - thd->is_fatal_error=0; // Error message is given - DBUG_VOID_RETURN; -} /* Return ok to the client. diff --git a/sql/protocol.h b/sql/protocol.h index 46a2b6d36b6..53584326f03 100644 --- a/sql/protocol.h +++ b/sql/protocol.h @@ -172,7 +172,6 @@ public: }; void send_warning(THD *thd, uint sql_errno, const char *err=0); -void net_printf_error(THD *thd, uint sql_errno, ...); void net_send_error(THD *thd, uint sql_errno=0, const char *err=0); void send_ok(THD *thd, ha_rows affected_rows=0L, ulonglong id=0L, const char *info=0); diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index 8a817e5993a..9b5f1a9b0e5 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -87,7 +87,7 @@ static int get_or_create_user_conn(THD *thd, const char *user, my_malloc(sizeof(struct user_conn) + temp_len+1, MYF(MY_WME))))) { - net_send_error(thd, 0, NullS); // Out of memory + /* MY_WME ensures an error is set in THD. */ return_val= 1; goto end; } @@ -100,8 +100,8 @@ static int get_or_create_user_conn(THD *thd, const char *user, uc->reset_utime= thd->thr_create_utime; if (my_hash_insert(&hash_user_connections, (uchar*) uc)) { + /* The only possible error is out of memory, MY_WME sets an error. */ my_free((char*) uc,0); - net_send_error(thd, 0, NullS); // Out of memory return_val= 1; goto end; } @@ -132,6 +132,7 @@ end: 1 error */ +static int check_for_max_user_connections(THD *thd, USER_CONN *uc) { int error=0; @@ -141,7 +142,7 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc) if (max_user_connections && !uc->user_resources.user_conn && max_user_connections < (uint) uc->connections) { - net_printf_error(thd, ER_TOO_MANY_USER_CONNECTIONS, uc->user); + my_error(ER_TOO_MANY_USER_CONNECTIONS, MYF(0), uc->user); error=1; goto end; } @@ -149,24 +150,24 @@ int check_for_max_user_connections(THD *thd, USER_CONN *uc) if (uc->user_resources.user_conn && uc->user_resources.user_conn < uc->connections) { - net_printf_error(thd, ER_USER_LIMIT_REACHED, uc->user, - "max_user_connections", - (long) uc->user_resources.user_conn); + my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, + "max_user_connections", + (long) uc->user_resources.user_conn); error= 1; goto end; } if (uc->user_resources.conn_per_hour && uc->user_resources.conn_per_hour <= uc->conn_per_hour) { - net_printf_error(thd, ER_USER_LIMIT_REACHED, uc->user, - "max_connections_per_hour", - (long) uc->user_resources.conn_per_hour); + my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, + "max_connections_per_hour", + (long) uc->user_resources.conn_per_hour); error=1; goto end; } uc->conn_per_hour++; - end: +end: if (error) uc->connections--; // no need for decrease_user_connections() here (void) pthread_mutex_unlock(&LOCK_user_conn); @@ -258,8 +259,8 @@ bool check_mqh(THD *thd, uint check_command) if (uc->user_resources.questions && uc->questions++ >= uc->user_resources.questions) { - net_printf_error(thd, ER_USER_LIMIT_REACHED, uc->user, "max_questions", - (long) uc->user_resources.questions); + my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, "max_questions", + (long) uc->user_resources.questions); error=1; goto end; } @@ -270,8 +271,8 @@ bool check_mqh(THD *thd, uint check_command) (sql_command_flags[check_command] & CF_CHANGES_DATA) && uc->updates++ >= uc->user_resources.updates) { - net_printf_error(thd, ER_USER_LIMIT_REACHED, uc->user, "max_updates", - (long) uc->user_resources.updates); + my_error(ER_USER_LIMIT_REACHED, MYF(0), uc->user, "max_updates", + (long) uc->user_resources.updates); error=1; goto end; } @@ -284,33 +285,33 @@ end: #endif /* NO_EMBEDDED_ACCESS_CHECKS */ -/* - Check if user exist and password supplied is correct. +/** + Check if user exist and password supplied is correct. - SYNOPSIS - check_user() - thd thread handle, thd->security_ctx->{host,user,ip} are used - command originator of the check: now check_user is called - during connect and change user procedures; used for - logging. - passwd scrambled password received from client - passwd_len length of scrambled password - db database name to connect to, may be NULL - check_count dont know exactly + @param thd thread handle, thd->security_ctx->{host,user,ip} are used + @param command originator of the check: now check_user is called + during connect and change user procedures; used for + logging. + @param passwd scrambled password received from client + @param passwd_len length of scrambled password + @param db database name to connect to, may be NULL + @param check_count TRUE if establishing a new connection. In this case + check that we have not exceeded the global + max_connections limist - Note, that host, user and passwd may point to communication buffer. - Current implementation does not depend on that, but future changes - should be done with this in mind; 'thd' is INOUT, all other params - are 'IN'. + @note Host, user and passwd may point to communication buffer. + Current implementation does not depend on that, but future changes + should be done with this in mind; 'thd' is INOUT, all other params + are 'IN'. - RETURN VALUE - 0 OK; thd->security_ctx->user/master_access/priv_user/db_access and - thd->db are updated; OK is sent to client; - -1 access denied or handshake error; error is sent to client; - >0 error, not sent to client + @retval 0 OK; thd->security_ctx->user/master_access/priv_user/db_access and + thd->db are updated; OK is sent to the client. + @retval 1 error, e.g. access denied or handshake error, not sent to + the client. A message is pushed into the error stack. */ -int check_user(THD *thd, enum enum_server_command command, +int +check_user(THD *thd, enum enum_server_command command, const char *passwd, uint passwd_len, const char *db, bool check_count) { @@ -328,11 +329,7 @@ int check_user(THD *thd, enum enum_server_command command, */ thd->reset_db(NULL, 0); if (mysql_change_db(thd, &db_str, FALSE)) - { - /* Send the error to the client */ - net_send_error(thd); - DBUG_RETURN(-1); - } + DBUG_RETURN(1); } send_ok(thd); DBUG_RETURN(0); @@ -349,14 +346,17 @@ int check_user(THD *thd, enum enum_server_command command, */ if (opt_secure_auth_local && passwd_len == SCRAMBLE_LENGTH_323) { - net_printf_error(thd, ER_NOT_SUPPORTED_AUTH_MODE); + my_error(ER_NOT_SUPPORTED_AUTH_MODE, MYF(0)); general_log_print(thd, COM_CONNECT, ER(ER_NOT_SUPPORTED_AUTH_MODE)); - DBUG_RETURN(-1); + DBUG_RETURN(1); } if (passwd_len != 0 && passwd_len != SCRAMBLE_LENGTH && passwd_len != SCRAMBLE_LENGTH_323) - DBUG_RETURN(ER_HANDSHAKE_ERROR); + { + my_error(ER_HANDSHAKE_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + DBUG_RETURN(1); + } /* Clear thd->db as it points to something, that will be freed when @@ -380,20 +380,21 @@ int check_user(THD *thd, enum enum_server_command command, NET *net= &thd->net; if (opt_secure_auth_local) { - net_printf_error(thd, ER_SERVER_IS_IN_SECURE_AUTH_MODE, - thd->main_security_ctx.user, - thd->main_security_ctx.host_or_ip); + my_error(ER_SERVER_IS_IN_SECURE_AUTH_MODE, MYF(0), + thd->main_security_ctx.user, + thd->main_security_ctx.host_or_ip); general_log_print(thd, COM_CONNECT, ER(ER_SERVER_IS_IN_SECURE_AUTH_MODE), thd->main_security_ctx.user, thd->main_security_ctx.host_or_ip); - DBUG_RETURN(-1); + DBUG_RETURN(1); } /* We have to read very specific packet size */ if (send_old_password_request(thd) || my_net_read(net) != SCRAMBLE_LENGTH_323 + 1) { inc_host_errors(&thd->remote.sin_addr); - DBUG_RETURN(ER_HANDSHAKE_ERROR); + my_error(ER_HANDSHAKE_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + DBUG_RETURN(1); } /* Final attempt to check the user based on reply */ /* So as passwd is short, errcode is always >= 0 */ @@ -427,8 +428,8 @@ int check_user(THD *thd, enum enum_server_command command, VOID(pthread_mutex_unlock(&LOCK_thread_count)); if (!count_ok) { // too many connections - net_send_error(thd, ER_CON_COUNT_ERROR); - DBUG_RETURN(-1); + my_error(ER_CON_COUNT_ERROR, MYF(0)); + DBUG_RETURN(1); } } @@ -462,24 +463,29 @@ int check_user(THD *thd, enum enum_server_command command, (opt_old_style_user_limits ? thd->main_security_ctx.host_or_ip : thd->main_security_ctx.priv_host), &ur)) - DBUG_RETURN(-1); + { + /* The error is set by get_or_create_user_conn(). */ + DBUG_RETURN(1); + } if (thd->user_connect && (thd->user_connect->user_resources.conn_per_hour || thd->user_connect->user_resources.user_conn || max_user_connections) && check_for_max_user_connections(thd, thd->user_connect)) - DBUG_RETURN(-1); + { + /* The error is set in check_for_max_user_connections(). */ + DBUG_RETURN(1); + } /* Change database if necessary */ if (db && db[0]) { if (mysql_change_db(thd, &db_str, FALSE)) { - /* Send error to the client */ - net_send_error(thd); + /* mysql_change_db() has pushed the error message. */ if (thd->user_connect) decrease_user_connections(thd->user_connect); - DBUG_RETURN(-1); + DBUG_RETURN(1); } } send_ok(thd); @@ -490,19 +496,19 @@ int check_user(THD *thd, enum enum_server_command command, } else if (res == 2) // client gave short hash, server has long hash { - net_printf_error(thd, ER_NOT_SUPPORTED_AUTH_MODE); + my_error(ER_NOT_SUPPORTED_AUTH_MODE, MYF(0)); general_log_print(thd, COM_CONNECT, ER(ER_NOT_SUPPORTED_AUTH_MODE)); - DBUG_RETURN(-1); + DBUG_RETURN(1); } - net_printf_error(thd, ER_ACCESS_DENIED_ERROR, - thd->main_security_ctx.user, - thd->main_security_ctx.host_or_ip, - passwd_len ? ER(ER_YES) : ER(ER_NO)); + my_error(ER_ACCESS_DENIED_ERROR, MYF(0), + thd->main_security_ctx.user, + thd->main_security_ctx.host_or_ip, + passwd_len ? ER(ER_YES) : ER(ER_NO)); general_log_print(thd, COM_CONNECT, ER(ER_ACCESS_DENIED_ERROR), thd->main_security_ctx.user, thd->main_security_ctx.host_or_ip, passwd_len ? ER(ER_YES) : ER(ER_NO)); - DBUG_RETURN(-1); + DBUG_RETURN(1); #endif /* NO_EMBEDDED_ACCESS_CHECKS */ } @@ -666,9 +672,12 @@ static int check_connection(THD *thd) char ip[30]; if (vio_peer_addr(net->vio, ip, &thd->peer_port)) - return (ER_BAD_HOST_ERROR); - if (!(thd->main_security_ctx.ip= my_strdup(ip,MYF(0)))) - return (ER_OUT_OF_RESOURCES); + { + my_error(ER_BAD_HOST_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + return 1; + } + if (!(thd->main_security_ctx.ip= my_strdup(ip,MYF(MY_WME)))) + return 1; /* The error is set by my_strdup(). */ thd->main_security_ctx.host_or_ip= thd->main_security_ctx.ip; vio_in_addr(net->vio,&thd->remote.sin_addr); if (!(specialflag & SPECIAL_NO_RESOLVE)) @@ -685,7 +694,10 @@ static int check_connection(THD *thd) thd->main_security_ctx.host_or_ip= thd->main_security_ctx.host; } if (connect_errors > max_connect_errors) - return(ER_HOST_IS_BLOCKED); + { + my_error(ER_HOST_IS_BLOCKED, MYF(0), thd->main_security_ctx.host_or_ip); + return 1; + } } DBUG_PRINT("info",("Host: %s ip: %s", (thd->main_security_ctx.host ? @@ -693,7 +705,11 @@ static int check_connection(THD *thd) (thd->main_security_ctx.ip ? thd->main_security_ctx.ip : "unknown ip"))); if (acl_check_host(thd->main_security_ctx.host, thd->main_security_ctx.ip)) - return(ER_HOST_NOT_PRIVILEGED); + { + my_error(ER_HOST_NOT_PRIVILEGED, MYF(0), + thd->main_security_ctx.host_or_ip); + return 1; + } } else /* Hostname given means that the connection was on a socket */ { @@ -753,7 +769,9 @@ static int check_connection(THD *thd) pkt_len < MIN_HANDSHAKE_SIZE) { inc_host_errors(&thd->remote.sin_addr); - return(ER_HANDSHAKE_ERROR); + my_error(ER_HANDSHAKE_ERROR, MYF(0), + thd->main_security_ctx.host_or_ip); + return 1; } } #ifdef _CUSTOMCONFIG_ @@ -762,7 +780,7 @@ static int check_connection(THD *thd) if (connect_errors) reset_host_errors(&thd->remote.sin_addr); if (thd->packet.alloc(thd->variables.net_buffer_length)) - return(ER_OUT_OF_RESOURCES); + return 1; /* The error is set by alloc(). */ thd->client_capabilities=uint2korr(net->read_pos); if (thd->client_capabilities & CLIENT_PROTOCOL_41) @@ -790,14 +808,16 @@ static int check_connection(THD *thd) if (!ssl_acceptor_fd) { inc_host_errors(&thd->remote.sin_addr); - return(ER_HANDSHAKE_ERROR); + my_error(ER_HANDSHAKE_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + return 1; } DBUG_PRINT("info", ("IO layer change in progress...")); if (sslaccept(ssl_acceptor_fd, net->vio, net->read_timeout)) { DBUG_PRINT("error", ("Failed to accept new SSL connection")); inc_host_errors(&thd->remote.sin_addr); - return(ER_HANDSHAKE_ERROR); + my_error(ER_HANDSHAKE_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + return 1; } DBUG_PRINT("info", ("Reading user information over SSL layer")); if ((pkt_len= my_net_read(net)) == packet_error || @@ -806,7 +826,8 @@ static int check_connection(THD *thd) DBUG_PRINT("error", ("Failed to read user information (pkt_len= %lu)", pkt_len)); inc_host_errors(&thd->remote.sin_addr); - return(ER_HANDSHAKE_ERROR); + my_error(ER_HANDSHAKE_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + return 1; } } #endif /* HAVE_OPENSSL */ @@ -814,7 +835,8 @@ static int check_connection(THD *thd) if (end >= (char*) net->read_pos+ pkt_len +2) { inc_host_errors(&thd->remote.sin_addr); - return(ER_HANDSHAKE_ERROR); + my_error(ER_HANDSHAKE_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + return 1; } if (thd->client_capabilities & CLIENT_INTERACTIVE) @@ -851,7 +873,8 @@ static int check_connection(THD *thd) if (passwd + passwd_len + db_len > (char *)net->read_pos + pkt_len) { inc_host_errors(&thd->remote.sin_addr); - return ER_HANDSHAKE_ERROR; + my_error(ER_HANDSHAKE_ERROR, MYF(0), thd->main_security_ctx.host_or_ip); + return 1; } /* Since 4.1 all database names are stored in utf8 */ @@ -879,8 +902,8 @@ static int check_connection(THD *thd) if (thd->main_security_ctx.user) x_free(thd->main_security_ctx.user); - if (!(thd->main_security_ctx.user= my_strdup(user, MYF(0)))) - return (ER_OUT_OF_RESOURCES); + if (!(thd->main_security_ctx.user= my_strdup(user, MYF(MY_WME)))) + return 1; /* The error is set by my_strdup(). */ return check_user(thd, COM_CONNECT, passwd, passwd_len, db, TRUE); } @@ -929,7 +952,6 @@ bool setup_connection_thread_globals(THD *thd) bool login_connection(THD *thd) { - int error; NET *net= &thd->net; Security_context *sctx= thd->security_ctx; DBUG_ENTER("login_connection"); @@ -942,10 +964,9 @@ bool login_connection(THD *thd) my_net_set_read_timeout(net, connect_timeout); my_net_set_write_timeout(net, connect_timeout); - if ((error=check_connection(thd))) + if (check_connection(thd)) { // Wrong permissions - if (error > 0) - net_printf_error(thd, error, sctx->host_or_ip); + net_send_error(thd); #ifdef __NT__ if (vio_type(net->vio) == VIO_TYPE_NAMEDPIPE) my_sleep(1000); /* must wait after eof() */ diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index ac042960388..803d29d83d6 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -915,11 +915,6 @@ bool dispatch_command(enum enum_server_command command, THD *thd, if (res) { - /* authentication failure, we shall restore old user */ - if (res > 0) - my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0)); - else - thd->clear_error(); // Error already sent to client x_free(thd->security_ctx->user); *thd->security_ctx= save_security_ctx; thd->user_connect= save_user_connect; From 189ec0e0af2ab7d50340f66285ffecc4f06586c6 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 00:31:57 +0300 Subject: [PATCH 22/31] A fix for Bug#32007 select udf_function() doesn't return an error if error during udf initialization. The bug is spotted while working on Bug 12713. If a user-defined function was used in a SELECT statement, and an error would occur during UDF initialization, this error would not terminate execution of the SELECT, but rather would be converted to a warning. The fix is to use a stack buffer to store the message from udf_init instead of private my_error() buffer. mysql-test/r/udf.result: Update the result to reflect the fix for Bug#32007 select udf_function() doesn't return an error if error during udf initialization mysql-test/t/udf.test: Update the test to reflect the fix for Bug #32007 select udf_function() doesn't return an error if error during udf initialization sql/item_func.cc: A fix for Bug#32007. net.last_error buffer was used to store the temporary message from udf_init. Then, when my_error() was called, net.last_error was not empty so my_error() would conclude that there is already an error in the error stack, and not "overwrite" it. However, thd->net.report_error was not set, so the the SELECT was not aborted. The fix is to use a stack buffer instead of thd->net.last_error to store the message from udf_init. The message will end up in thd->net.last_error anyway after a call to my_error. --- mysql-test/r/udf.result | 12 ++++++------ mysql-test/t/udf.test | 12 ++++++------ sql/item_func.cc | 5 +++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/mysql-test/r/udf.result b/mysql-test/r/udf.result index da27a71c1a1..cb5afcf5f17 100644 --- a/mysql-test/r/udf.result +++ b/mysql-test/r/udf.result @@ -11,7 +11,7 @@ RETURNS STRING SONAME "UDF_EXAMPLE_LIB"; CREATE AGGREGATE FUNCTION avgcost RETURNS REAL SONAME "UDF_EXAMPLE_LIB"; select myfunc_double(); -ERROR HY000: myfunc_double must have at least one argument +ERROR HY000: Can't initialize function 'myfunc_double'; myfunc_double must have at least one argument select myfunc_double(1); myfunc_double(1) 49.00 @@ -24,26 +24,26 @@ select myfunc_int(); myfunc_int() 0 select lookup(); -ERROR HY000: Wrong arguments to lookup; Use the source +ERROR HY000: Can't initialize function 'lookup'; Wrong arguments to lookup; Use the source select lookup("127.0.0.1"); lookup("127.0.0.1") 127.0.0.1 select lookup(127,0,0,1); -ERROR HY000: Wrong arguments to lookup; Use the source +ERROR HY000: Can't initialize function 'lookup'; Wrong arguments to lookup; Use the source select lookup("localhost"); lookup("localhost") 127.0.0.1 select reverse_lookup(); -ERROR HY000: Wrong number of arguments to reverse_lookup; Use the source +ERROR HY000: Can't initialize function 'reverse_lookup'; Wrong number of arguments to reverse_lookup; Use the source select reverse_lookup("127.0.0.1"); select reverse_lookup(127,0,0,1); select reverse_lookup("localhost"); reverse_lookup("localhost") NULL select avgcost(); -ERROR HY000: wrong number of arguments: AVGCOST() requires two arguments +ERROR HY000: Can't initialize function 'avgcost'; wrong number of arguments: AVGCOST() requires two arguments select avgcost(100,23.76); -ERROR HY000: wrong argument type: AVGCOST() requires an INT and a REAL +ERROR HY000: Can't initialize function 'avgcost'; wrong argument type: AVGCOST() requires an INT and a REAL create table t1(sum int, price float(24)); insert into t1 values(100, 50.00), (100, 100.00); select avgcost(sum, price) from t1; diff --git a/mysql-test/t/udf.test b/mysql-test/t/udf.test index 663dc08d72e..32cfca57546 100644 --- a/mysql-test/t/udf.test +++ b/mysql-test/t/udf.test @@ -35,20 +35,20 @@ eval CREATE FUNCTION reverse_lookup eval CREATE AGGREGATE FUNCTION avgcost RETURNS REAL SONAME "$UDF_EXAMPLE_LIB"; ---error 0 +--error ER_CANT_INITIALIZE_UDF select myfunc_double(); select myfunc_double(1); select myfunc_double(78654); --error 1305 select myfunc_nonexist(); select myfunc_int(); ---error 0 +--error ER_CANT_INITIALIZE_UDF select lookup(); select lookup("127.0.0.1"); ---error 0 +--error ER_CANT_INITIALIZE_UDF select lookup(127,0,0,1); select lookup("localhost"); ---error 0 +--error ER_CANT_INITIALIZE_UDF select reverse_lookup(); # These two functions should return "localhost", but it's @@ -59,9 +59,9 @@ select reverse_lookup(127,0,0,1); --enable_result_log select reverse_lookup("localhost"); ---error 0 +--error ER_CANT_INITIALIZE_UDF select avgcost(); ---error 0 +--error ER_CANT_INITIALIZE_UDF select avgcost(100,23.76); create table t1(sum int, price float(24)); insert into t1 values(100, 50.00), (100, 100.00); diff --git a/sql/item_func.cc b/sql/item_func.cc index 2d4d61de61b..ec0ecc89394 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -2897,6 +2897,7 @@ udf_handler::fix_fields(THD *thd, Item_result_field *func, if (u_d->func_init) { + char init_msg_buff[MYSQL_ERRMSG_SIZE]; char *to=num_buffer; for (uint i=0; i < arg_count; i++) { @@ -2949,10 +2950,10 @@ udf_handler::fix_fields(THD *thd, Item_result_field *func, } thd->net.last_error[0]=0; Udf_func_init init= u_d->func_init; - if ((error=(uchar) init(&initid, &f_args, thd->net.last_error))) + if ((error=(uchar) init(&initid, &f_args, init_msg_buff))) { my_error(ER_CANT_INITIALIZE_UDF, MYF(0), - u_d->name.str, thd->net.last_error); + u_d->name.str, init_msg_buff); free_udf(u_d); DBUG_RETURN(TRUE); } From 16bf418de58aaa892c787ece33c8c2ec49a8cb8c Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 00:55:25 +0300 Subject: [PATCH 23/31] Another attempt to fix the Windows compilation failre. --- sql-common/client.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sql-common/client.c b/sql-common/client.c index 5ece9c39d6c..0ca7ef16c0d 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -349,7 +349,7 @@ static void set_mysql_extended_error(MYSQL *mysql, int errcode, #ifdef __WIN__ -HANDLE create_named_pipe(NET *net, uint connect_timeout, char **arg_host, +HANDLE create_named_pipe(MYSQL *mysql, uint connect_timeout, char **arg_host, char **arg_unix_socket) { HANDLE hPipe=INVALID_HANDLE_VALUE; @@ -1950,8 +1950,8 @@ CLI_MYSQL_REAL_CONNECT(MYSQL *mysql,const char *host, const char *user, (! have_tcpip && (unix_socket || !host && is_NT())))) { sock=0; - if ((hPipe=create_named_pipe(net, mysql->options.connect_timeout, - (char**) &host, (char**) &unix_socket)) == + if ((hPipe= create_named_pipe(mysql, mysql->options.connect_timeout, + (char**) &host, (char**) &unix_socket)) == INVALID_HANDLE_VALUE) { DBUG_PRINT("error", From fbf75e0dcfe5a3438c1851b84a787d7cb385cd2b Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 17:00:24 +0300 Subject: [PATCH 24/31] Fix a compilation warning. sql/sql_connect.cc: Fix a compilation warning (unused variable). --- sql/sql_connect.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index 9b5f1a9b0e5..90df233896a 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -953,7 +953,6 @@ bool setup_connection_thread_globals(THD *thd) bool login_connection(THD *thd) { NET *net= &thd->net; - Security_context *sctx= thd->security_ctx; DBUG_ENTER("login_connection"); DBUG_PRINT("info", ("login_connection called by thread %lu", thd->thread_id)); From ecef837931ae10467bb112eb5364051e419521dc Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 17:08:02 +0300 Subject: [PATCH 25/31] Use thd->is_error() instead of direct access to thd->net.report_error in evaluate_join_record(). A minor cleanup required for the fix for Bug#12713. sql/sql_select.cc: Use thd->is_error() instead of direct access to thd->net.report_error in evaluate_join_record() --- sql/sql_select.cc | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sql/sql_select.cc b/sql/sql_select.cc index ef1151d82f3..338b5f0cc3f 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -122,7 +122,7 @@ static int do_select(JOIN *join,List *fields,TABLE *tmp_table, static enum_nested_loop_state evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, - int error, my_bool *report_error); + int error); static enum_nested_loop_state evaluate_null_complemented_join_record(JOIN *join, JOIN_TAB *join_tab); static enum_nested_loop_state @@ -10871,7 +10871,6 @@ sub_select(JOIN *join,JOIN_TAB *join_tab,bool end_of_records) int error; enum_nested_loop_state rc; - my_bool *report_error= &(join->thd->net.report_error); READ_RECORD *info= &join_tab->read_record; if (join->resume_nested_loop) @@ -10903,13 +10902,13 @@ sub_select(JOIN *join,JOIN_TAB *join_tab,bool end_of_records) join->thd->row_count= 0; error= (*join_tab->read_first_record)(join_tab); - rc= evaluate_join_record(join, join_tab, error, report_error); + rc= evaluate_join_record(join, join_tab, error); } while (rc == NESTED_LOOP_OK) { error= info->read_record(info); - rc= evaluate_join_record(join, join_tab, error, report_error); + rc= evaluate_join_record(join, join_tab, error); } if (rc == NESTED_LOOP_NO_MORE_ROWS && @@ -10933,13 +10932,13 @@ sub_select(JOIN *join,JOIN_TAB *join_tab,bool end_of_records) static enum_nested_loop_state evaluate_join_record(JOIN *join, JOIN_TAB *join_tab, - int error, my_bool *report_error) + int error) { bool not_used_in_distinct=join_tab->not_used_in_distinct; ha_rows found_records=join->found_records; COND *select_cond= join_tab->select_cond; - if (error > 0 || (*report_error)) // Fatal error + if (error > 0 || (join->thd->is_error())) // Fatal error return NESTED_LOOP_ERROR; if (error < 0) return NESTED_LOOP_NO_MORE_ROWS; From 3536c5d5f5a404e8872df11caf8f9b3a8b4cc45f Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 18:06:46 +0300 Subject: [PATCH 26/31] Use Internal_error_handler mechanism to silence ER_TOO_MANY_FIELDS error in mysql_create_frm instead of direct access to my_error() members. This is a pre-requisite for the patch for Bug#12713. sql/unireg.cc: Use Internal_error_handler mechanism to silence ER_TOO_MANY_FIELDS error in mysql_create_frm instead of direct access to my_error() members. --- sql/unireg.cc | 48 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 6 deletions(-) diff --git a/sql/unireg.cc b/sql/unireg.cc index f9e8e54439a..f2238d69973 100644 --- a/sql/unireg.cc +++ b/sql/unireg.cc @@ -47,6 +47,35 @@ static bool make_empty_rec(THD *thd, int file, enum legacy_db_type table_type, uint reclength, ulong data_offset, handler *handler); +/** + An interceptor to hijack ER_TOO_MANY_FIELDS error from + pack_screens and retry again without UNIREG screens. + + XXX: what is a UNIREG screen? +*/ + +struct Pack_header_error_handler: public Internal_error_handler +{ + virtual bool handle_error(uint sql_errno, + const char *message, + MYSQL_ERROR::enum_warning_level level, + THD *thd); + bool is_handled; + Pack_header_error_handler() :is_handled(FALSE) {} +}; + + +bool +Pack_header_error_handler:: +handle_error(uint sql_errno, + const char * /* message */, + MYSQL_ERROR::enum_warning_level /* level */, + THD * /* thd */) +{ + is_handled= (sql_errno == ER_TOO_MANY_FIELDS); + return is_handled; +} + /* Create a frm (table definition) file @@ -86,6 +115,8 @@ bool mysql_create_frm(THD *thd, const char *file_name, #ifdef WITH_PARTITION_STORAGE_ENGINE partition_info *part_info= thd->work_part_info; #endif + Pack_header_error_handler pack_header_error_handler; + int error; DBUG_ENTER("mysql_create_frm"); DBUG_ASSERT(*fn_rext((char*)file_name)); // Check .frm extension @@ -99,17 +130,22 @@ bool mysql_create_frm(THD *thd, const char *file_name, create_info->null_bits++; data_offset= (create_info->null_bits + 7) / 8; - if (pack_header(forminfo, ha_legacy_type(create_info->db_type), - create_fields,info_length, - screens, create_info->table_options, - data_offset, db_file)) + thd->push_internal_handler(&pack_header_error_handler); + + error= pack_header(forminfo, ha_legacy_type(create_info->db_type), + create_fields,info_length, + screens, create_info->table_options, + data_offset, db_file); + + thd->pop_internal_handler(); + + if (error) { my_free(screen_buff, MYF(0)); - if (thd->net.last_errno != ER_TOO_MANY_FIELDS) + if (! pack_header_error_handler.is_handled) DBUG_RETURN(1); // Try again without UNIREG screens (to get more columns) - thd->net.last_error[0]=0; if (!(screen_buff=pack_screens(create_fields,&info_length,&screens,1))) DBUG_RETURN(1); if (pack_header(forminfo, ha_legacy_type(create_info->db_type), From 6dd04c22a695d5548045d9eda451af2cc016f7fb Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 18:33:51 +0300 Subject: [PATCH 27/31] Cleanup execute_ddl_log_recovery() to not generate an error if there is nothing to recover. Discovered while working on Bug#12713 sql/sql_table.cc: Use MYF(0) in my_open() in read_ddl_log_header() called from execute_ddl_log_recovery() called during the server start up to not generate an error if no ddl log exists. This is not an erroneous situation, in fact it's the case in any server statrtup. The error was lost anyway, since it was pushed into the stack of the artificial thd. --- sql/sql_table.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 6edb8494b03..328b5e71a44 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -434,7 +434,7 @@ static uint read_ddl_log_header() create_ddl_log_file_name(file_name); if ((global_ddl_log.file_id= my_open(file_name, - O_RDWR | O_BINARY, MYF(MY_WME))) >= 0) + O_RDWR | O_BINARY, MYF(0))) >= 0) { if (read_ddl_log_file_entry(0UL)) { From 8f0df2efe85ae6d241ed045a4100d207ddb4ed1c Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 17:29:20 -0200 Subject: [PATCH 28/31] Bug#31850 Test crashes in "embedded" server The mysql_change_user command fails to properly update the database pointer when no database is selected, leading to "use after free" errors. The same happens on the user privilege pointer in the thread security context. The solution is to properly reset and update the database name. Also update the user_priv pointer so that it doesn't point to freed memory. sql/sql_connect.cc: After a successful call to check_user() without specifying a new database name, the previous database thd->db) is freed but the pointer is not updated to NULL. sql/sql_parse.cc: Update the security_ctx->priv_user pointer as it is a alias for the user security_ctx->user pointer. Also remove unneeded cast, the x_free macro casts the argument. --- sql/sql_connect.cc | 23 +++++++++-------------- sql/sql_parse.cc | 5 +++-- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/sql/sql_connect.cc b/sql/sql_connect.cc index 8a817e5993a..adf0b9e01ed 100644 --- a/sql/sql_connect.cc +++ b/sql/sql_connect.cc @@ -316,17 +316,20 @@ int check_user(THD *thd, enum enum_server_command command, { DBUG_ENTER("check_user"); LEX_STRING db_str= { (char *) db, db ? strlen(db) : 0 }; - + + /* + Clear thd->db as it points to something, that will be freed when + connection is closed. We don't want to accidentally free a wrong + pointer if connect failed. Also in case of 'CHANGE USER' failure, + current database will be switched to 'no database selected'. + */ + thd->reset_db(NULL, 0); + #ifdef NO_EMBEDDED_ACCESS_CHECKS thd->main_security_ctx.master_access= GLOBAL_ACLS; // Full rights /* Change database if necessary */ if (db && db[0]) { - /* - thd->db is saved in caller and needs to be freed by caller if this - function returns 0 - */ - thd->reset_db(NULL, 0); if (mysql_change_db(thd, &db_str, FALSE)) { /* Send the error to the client */ @@ -358,14 +361,6 @@ int check_user(THD *thd, enum enum_server_command command, passwd_len != SCRAMBLE_LENGTH_323) DBUG_RETURN(ER_HANDSHAKE_ERROR); - /* - Clear thd->db as it points to something, that will be freed when - connection is closed. We don't want to accidentally free a wrong pointer - if connect failed. Also in case of 'CHANGE USER' failure, current - database will be switched to 'no database selected'. - */ - thd->reset_db(NULL, 0); - USER_RESOURCES ur; int res= acl_getroot(thd, &ur, passwd, passwd_len); #ifndef EMBEDDED_LIBRARY diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 85457fea41b..b2f2e74999b 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -911,6 +911,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd, /* Clear variables that are allocated */ thd->user_connect= 0; + thd->security_ctx->priv_user= thd->security_ctx->user; res= check_user(thd, COM_CHANGE_USER, passwd, passwd_len, db, FALSE); if (res) @@ -933,8 +934,8 @@ bool dispatch_command(enum enum_server_command command, THD *thd, if (save_user_connect) decrease_user_connections(save_user_connect); #endif /* NO_EMBEDDED_ACCESS_CHECKS */ - x_free((uchar*) save_db); - x_free((uchar*) save_security_ctx.user); + x_free(save_db); + x_free(save_security_ctx.user); if (cs_number) { From 611dbd0bb3e05d047ed0f42bd5211f17b8330bd2 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 1 Nov 2007 18:52:56 -0200 Subject: [PATCH 29/31] Bug#30882 Dropping a temporary table inside a stored function may cause a server crash If a stored function that contains a drop temporary table statement is invoked by a create temporary table of the same name may cause a server crash. The problem is that when dropping a table no check is done to ensure that table is not being used by some outer query (or outer statement), potentially leaving the outer query with a reference to a stale (freed) table. The solution is when dropping a temporary table, always check if the table is being used by some outer statement as a temporary table can be dropped inside stored procedures. The check is performed by looking at the TABLE::query_id value for temporary tables. To simplify this check and to solve a bug related to handling of temporary tables in prelocked mode, this patch changes the way in which this member is used to track the fact that table is used/unused. Now we ensure that TABLE::query_id is zero for unused temporary tables (which means that all temporary tables which were used by a statement should be marked as free for reuse after it's execution has been completed). mysql-test/include/handler.inc: Add test case for side effect of Bug#30882 mysql-test/r/handler_innodb.result: Add test case result for side effect of Bug#30882 mysql-test/r/handler_myisam.result: Add test case result for side effect of Bug#30882 mysql-test/r/sp-error.result: Add test case result for Bug#30882 mysql-test/t/sp-error.test: Add test case for Bug#30882 sql/event_db_repository.cc: Update close_thread_tables call, no more default values. sql/mysql_priv.h: Remove implicit default parameters values of the close_thread_tables function as no callers are using it. sql/slave.cc: Update close_thread_tables call, no more default values sql/sp_head.cc: Update close_thread_tables call, no more default values sql/sql_base.cc: Changed the approach to distinguishing currently unused temporary tables. Now we ensure that such tables always have TABLE::query_id set to 0 and use this fact to perform checks during opening and dropping of temporary tables. This means that we have to call close_thread_tables() even for statements which use only temporary tables. To make this call cheaper, we re-factored close_thread_tables() to not take LOCK_open unless there are open base tables. sql/sql_handler.cc: Properly close temporary tables associated with a handler. sql/sql_insert.cc: close_temporary_table is now merged into drop_temporary_table. sql/sql_parse.cc: Now the condition doesn't cover all cases because close_thread_tables() must be called even for statements that use only temporary tables. sql/sql_table.cc: Use drop_temporary_table which perform checks to verify if the table is not being used. Error path problem is due to a handler tables issue and is going to be addressed in bug 31397. sql/table.h: Rename previously unused clear_query_id and document the usage of query_id and open_by_handler. --- mysql-test/include/handler.inc | 32 +++++++ mysql-test/r/handler_innodb.result | 62 +++++++++++++ mysql-test/r/handler_myisam.result | 62 +++++++++++++ mysql-test/r/sp-error.result | 52 ++++++++++- mysql-test/t/sp-error.test | 74 ++++++++++++++- sql/event_db_repository.cc | 2 +- sql/mysql_priv.h | 4 +- sql/slave.cc | 2 +- sql/sp_head.cc | 2 +- sql/sql_base.cc | 141 ++++++++++++++++++----------- sql/sql_handler.cc | 14 +++ sql/sql_insert.cc | 2 +- sql/sql_parse.cc | 15 ++- sql/sql_table.cc | 28 ++++-- sql/table.h | 22 ++++- 15 files changed, 431 insertions(+), 83 deletions(-) diff --git a/mysql-test/include/handler.inc b/mysql-test/include/handler.inc index 71647112126..79e21382d4c 100644 --- a/mysql-test/include/handler.inc +++ b/mysql-test/include/handler.inc @@ -566,3 +566,35 @@ reap; connection default; drop table t2; disconnect flush; + +# +# Bug#30882 Dropping a temporary table inside a stored function may cause a server crash +# +# Test HANDLER statements in conjunction with temporary tables. While the temporary table +# is open by a HANDLER, no other statement can access it. +# + +--disable_warnings +drop table if exists t1; +--enable_warnings +create temporary table t1 (a int, b char(1), key a(a), key b(a,b)); +insert into t1 values (0,"a"),(1,"b"),(2,"c"),(3,"d"),(4,"e"), + (5,"f"),(6,"g"),(7,"h"),(8,"i"),(9,"j"); +select a,b from t1; +handler t1 open as a1; +handler a1 read a first; +handler a1 read a next; +handler a1 read a next; +--error ER_CANT_REOPEN_TABLE +select a,b from t1; +handler a1 read a prev; +handler a1 read a prev; +handler a1 read a=(6) where b="g"; +handler a1 close; +select a,b from t1; +handler t1 open as a2; +handler a2 read a first; +handler a2 read a last; +handler a2 read a prev; +handler a2 close; +drop table t1; diff --git a/mysql-test/r/handler_innodb.result b/mysql-test/r/handler_innodb.result index e9e5c7dbdd5..d9a1a0aa12b 100644 --- a/mysql-test/r/handler_innodb.result +++ b/mysql-test/r/handler_innodb.result @@ -575,3 +575,65 @@ ERROR 42S02: Table 'test.t1' doesn't exist handler t1 close; handler t2 close; drop table t2; +drop table if exists t1; +create temporary table t1 (a int, b char(1), key a(a), key b(a,b)); +insert into t1 values (0,"a"),(1,"b"),(2,"c"),(3,"d"),(4,"e"), +(5,"f"),(6,"g"),(7,"h"),(8,"i"),(9,"j"); +select a,b from t1; +a b +0 a +1 b +2 c +3 d +4 e +5 f +6 g +7 h +8 i +9 j +handler t1 open as a1; +handler a1 read a first; +a b +0 a +handler a1 read a next; +a b +1 b +handler a1 read a next; +a b +2 c +select a,b from t1; +ERROR HY000: Can't reopen table: 'a1' +handler a1 read a prev; +a b +1 b +handler a1 read a prev; +a b +0 a +handler a1 read a=(6) where b="g"; +a b +6 g +handler a1 close; +select a,b from t1; +a b +0 a +1 b +2 c +3 d +4 e +5 f +6 g +7 h +8 i +9 j +handler t1 open as a2; +handler a2 read a first; +a b +0 a +handler a2 read a last; +a b +9 j +handler a2 read a prev; +a b +8 i +handler a2 close; +drop table t1; diff --git a/mysql-test/r/handler_myisam.result b/mysql-test/r/handler_myisam.result index 715e5ab03d6..b42fdc864fe 100644 --- a/mysql-test/r/handler_myisam.result +++ b/mysql-test/r/handler_myisam.result @@ -575,3 +575,65 @@ ERROR 42S02: Table 'test.t1' doesn't exist handler t1 close; handler t2 close; drop table t2; +drop table if exists t1; +create temporary table t1 (a int, b char(1), key a(a), key b(a,b)); +insert into t1 values (0,"a"),(1,"b"),(2,"c"),(3,"d"),(4,"e"), +(5,"f"),(6,"g"),(7,"h"),(8,"i"),(9,"j"); +select a,b from t1; +a b +0 a +1 b +2 c +3 d +4 e +5 f +6 g +7 h +8 i +9 j +handler t1 open as a1; +handler a1 read a first; +a b +0 a +handler a1 read a next; +a b +1 b +handler a1 read a next; +a b +2 c +select a,b from t1; +ERROR HY000: Can't reopen table: 'a1' +handler a1 read a prev; +a b +1 b +handler a1 read a prev; +a b +0 a +handler a1 read a=(6) where b="g"; +a b +6 g +handler a1 close; +select a,b from t1; +a b +0 a +1 b +2 c +3 d +4 e +5 f +6 g +7 h +8 i +9 j +handler t1 open as a2; +handler a2 read a first; +a b +0 a +handler a2 read a last; +a b +9 j +handler a2 read a prev; +a b +8 i +handler a2 close; +drop table t1; diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result index 300fa42f3ad..b81f8ea64c9 100644 --- a/mysql-test/r/sp-error.result +++ b/mysql-test/r/sp-error.result @@ -1428,7 +1428,6 @@ create function bug20701() returns varchar(25) binary return "test"; ERROR 42000: This version of MySQL doesn't yet support 'return value collation' create function bug20701() returns varchar(25) return "test"; drop function bug20701; -End of 5.1 tests create procedure proc_26503_error_1() begin retry: @@ -1530,6 +1529,53 @@ return 1; end| ERROR HY000: Not allowed to set autocommit from a stored function or trigger create trigger t1 -before insert on t2 for each row set password = password('foo'); -delimiter ;| +before insert on t2 for each row set password = password('foo');| ERROR HY000: Not allowed to set autocommit from a stored function or trigger +drop function if exists f1; +drop function if exists f2; +drop table if exists t1, t2; +create function f1() returns int +begin +drop temporary table t1; +return 1; +end| +create temporary table t1 as select f1(); +ERROR HY000: Can't reopen table: 't1' +create function f2() returns int +begin +create temporary table t2 as select f1(); +return 1; +end| +create temporary table t1 as select f2(); +ERROR HY000: Can't reopen table: 't1' +drop function f1; +drop function f2; +create function f1() returns int +begin +drop temporary table t2,t1; +return 1; +end| +create function f2() returns int +begin +create temporary table t2 as select f1(); +return 1; +end| +create temporary table t1 as select f2(); +ERROR HY000: Can't reopen table: 't2' +drop function f1; +drop function f2; +create temporary table t2(a int); +select * from t2; +a +create function f2() returns int +begin +drop temporary table t2; +return 1; +end| +select f2(); +f2() +1 +drop function f2; +drop table t2; +ERROR 42S02: Unknown table 't2' +End of 5.1 tests diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test index 9f20d02480c..606c2a673bc 100644 --- a/mysql-test/t/sp-error.test +++ b/mysql-test/t/sp-error.test @@ -2078,10 +2078,6 @@ create function bug20701() returns varchar(25) binary return "test"; create function bug20701() returns varchar(25) return "test"; drop function bug20701; - ---echo End of 5.1 tests - - # # Bug#26503 (Illegal SQL exception handler code causes the server to crash) # @@ -2237,10 +2233,78 @@ end| --error ER_SP_CANT_SET_AUTOCOMMIT create trigger t1 - before insert on t2 for each row set password = password('foo'); + before insert on t2 for each row set password = password('foo');| delimiter ;| +# +# Bug#30882 Dropping a temporary table inside a stored function may cause a server crash +# + +--disable_warnings +drop function if exists f1; +drop function if exists f2; +drop table if exists t1, t2; +--enable_warnings + +delimiter |; +create function f1() returns int +begin + drop temporary table t1; + return 1; +end| +delimiter ;| +--error ER_CANT_REOPEN_TABLE +create temporary table t1 as select f1(); + +delimiter |; +create function f2() returns int +begin + create temporary table t2 as select f1(); + return 1; +end| +delimiter ;| +--error ER_CANT_REOPEN_TABLE +create temporary table t1 as select f2(); + +drop function f1; +drop function f2; + +delimiter |; +create function f1() returns int +begin + drop temporary table t2,t1; + return 1; +end| +create function f2() returns int +begin + create temporary table t2 as select f1(); + return 1; +end| +delimiter ;| +--error ER_CANT_REOPEN_TABLE +create temporary table t1 as select f2(); + +drop function f1; +drop function f2; + +create temporary table t2(a int); +select * from t2; +delimiter |; +create function f2() returns int +begin + drop temporary table t2; + return 1; +end| +delimiter ;| +select f2(); + +drop function f2; +--error ER_BAD_TABLE_ERROR +drop table t2; + +--echo End of 5.1 tests + # # BUG#NNNN: New bug synopsis # diff --git a/sql/event_db_repository.cc b/sql/event_db_repository.cc index 4451e763ff7..9a33b33d8c9 100644 --- a/sql/event_db_repository.cc +++ b/sql/event_db_repository.cc @@ -549,7 +549,7 @@ Event_db_repository::open_event_table(THD *thd, enum thr_lock_type lock_type, if (simple_open_n_lock_tables(thd, &tables)) { - close_thread_tables(thd, FALSE, FALSE); + close_thread_tables(thd); DBUG_RETURN(TRUE); } diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 3484b8096e3..12838317646 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -679,7 +679,7 @@ extern my_decimal decimal_zero; void free_items(Item *item); void cleanup_items(Item *item); class THD; -void close_thread_tables(THD *thd, bool locked=0, bool skip_derived=0); +void close_thread_tables(THD *thd); bool check_one_table_access(THD *thd, ulong privilege, TABLE_LIST *tables); bool check_single_table_access(THD *thd, ulong privilege, TABLE_LIST *tables, bool no_errors); @@ -1419,7 +1419,7 @@ TABLE_LIST *unique_table(THD *thd, TABLE_LIST *table, TABLE_LIST *table_list, bool check_alias); TABLE *find_temporary_table(THD *thd, const char *db, const char *table_name); TABLE *find_temporary_table(THD *thd, TABLE_LIST *table_list); -bool close_temporary_table(THD *thd, TABLE_LIST *table_list); +int drop_temporary_table(THD *thd, TABLE_LIST *table_list); void close_temporary_table(THD *thd, TABLE *table, bool free_share, bool delete_table); void close_temporary(TABLE *table, bool free_share, bool delete_table); diff --git a/sql/slave.cc b/sql/slave.cc index 494e13d8c9f..7abcf50fa75 100644 --- a/sql/slave.cc +++ b/sql/slave.cc @@ -2353,7 +2353,7 @@ err: change_rpl_status(RPL_ACTIVE_SLAVE,RPL_IDLE_SLAVE); DBUG_ASSERT(thd->net.buff != 0); net_end(&thd->net); // destructor will not free it, because net.vio is 0 - close_thread_tables(thd, 0); + close_thread_tables(thd); pthread_mutex_lock(&LOCK_thread_count); THD_CHECK_SENTRY(thd); delete thd; diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 6e8749aa745..c0ea73a6c00 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -1872,7 +1872,7 @@ sp_head::execute_procedure(THD *thd, List *args) we'll leave it here. */ if (!thd->in_sub_stmt) - close_thread_tables(thd, 0, 0); + close_thread_tables(thd); DBUG_PRINT("info",(" %.*s: eval args done", (int) m_name.length, m_name.str)); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 2584390d756..ddc5f88f577 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -1055,6 +1055,29 @@ bool close_cached_connection_tables(THD *thd, bool if_wait_for_refresh, } +/** + Mark all temporary tables which were used by the current statement or + substatement as free for reuse, but only if the query_id can be cleared. + + @param thd thread context + + @remark For temp tables associated with a open SQL HANDLER the query_id + is not reset until the HANDLER is closed. +*/ + +static void mark_temp_tables_as_free_for_reuse(THD *thd) +{ + for (TABLE *table= thd->temporary_tables ; table ; table= table->next) + { + if ((table->query_id == thd->query_id) && ! table->open_by_handler) + { + table->query_id= 0; + table->file->ha_reset(); + } + } +} + + /* Mark all tables in the list which were used by current substatement as free for reuse. @@ -1091,6 +1114,42 @@ static void mark_used_tables_as_free_for_reuse(THD *thd, TABLE *table) } +/** + Auxiliary function to close all tables in the open_tables list. + + @param thd Thread context. + + @remark It should not ordinarily be called directly. +*/ + +static void close_open_tables(THD *thd) +{ + bool found_old_table= 0; + + safe_mutex_assert_not_owner(&LOCK_open); + + VOID(pthread_mutex_lock(&LOCK_open)); + + DBUG_PRINT("info", ("thd->open_tables: 0x%lx", (long) thd->open_tables)); + + while (thd->open_tables) + found_old_table|= close_thread_table(thd, &thd->open_tables); + thd->some_tables_deleted= 0; + + /* Free tables to hold down open files */ + while (open_cache.records > table_cache_size && unused_tables) + VOID(hash_delete(&open_cache,(uchar*) unused_tables)); /* purecov: tested */ + check_unused(); + if (found_old_table) + { + /* Tell threads waiting for refresh that something has happened */ + broadcast_refresh(); + } + + VOID(pthread_mutex_unlock(&LOCK_open)); +} + + /* Close all tables used by the current substatement, or all tables used by this thread if we are on the upper level. @@ -1098,26 +1157,19 @@ static void mark_used_tables_as_free_for_reuse(THD *thd, TABLE *table) SYNOPSIS close_thread_tables() thd Thread handler - lock_in_use Set to 1 (0 = default) if caller has a lock on - LOCK_open - skip_derived Set to 1 (0 = default) if we should not free derived - tables. - stopper When closing tables from thd->open_tables(->next)*, - don't close/remove tables starting from stopper. IMPLEMENTATION Unlocks tables and frees derived tables. Put all normal tables used by thread in free list. - When in prelocked mode it will only close/mark as free for reuse - tables opened by this substatement, it will also check if we are - closing tables after execution of complete query (i.e. we are on - upper level) and will leave prelocked mode if needed. + It will only close/mark as free for reuse tables opened by this + substatement, it will also check if we are closing tables after + execution of complete query (i.e. we are on upper level) and will + leave prelocked mode if needed. */ -void close_thread_tables(THD *thd, bool lock_in_use, bool skip_derived) +void close_thread_tables(THD *thd) { - bool found_old_table; prelocked_mode_type prelocked_mode= thd->prelocked_mode; DBUG_ENTER("close_thread_tables"); @@ -1132,7 +1184,7 @@ void close_thread_tables(THD *thd, bool lock_in_use, bool skip_derived) derived tables with (sub-)statement instead of thread and destroy them at the end of its execution. */ - if (thd->derived_tables && !skip_derived) + if (thd->derived_tables) { TABLE *table, *next; /* @@ -1147,13 +1199,10 @@ void close_thread_tables(THD *thd, bool lock_in_use, bool skip_derived) thd->derived_tables= 0; } - if (prelocked_mode) - { - /* - Mark all temporary tables used by this substatement as free for reuse. - */ - mark_used_tables_as_free_for_reuse(thd, thd->temporary_tables); - } + /* + Mark all temporary tables used by this statement as free for reuse. + */ + mark_temp_tables_as_free_for_reuse(thd); if (thd->locked_tables || prelocked_mode) { @@ -1217,28 +1266,8 @@ void close_thread_tables(THD *thd, bool lock_in_use, bool skip_derived) if (!thd->active_transaction()) thd->transaction.xid_state.xid.null(); - if (!lock_in_use) - VOID(pthread_mutex_lock(&LOCK_open)); - - DBUG_PRINT("info", ("thd->open_tables: 0x%lx", (long) thd->open_tables)); - - found_old_table= 0; - while (thd->open_tables) - found_old_table|= close_thread_table(thd, &thd->open_tables); - thd->some_tables_deleted=0; - - /* Free tables to hold down open files */ - while (open_cache.records > table_cache_size && unused_tables) - VOID(hash_delete(&open_cache,(uchar*) unused_tables)); /* purecov: tested */ - check_unused(); - if (found_old_table) - { - /* Tell threads waiting for refresh that something has happened */ - broadcast_refresh(); - } - if (!lock_in_use) - VOID(pthread_mutex_unlock(&LOCK_open)); - /* VOID(pthread_sigmask(SIG_SETMASK,&thd->signals,NULL)); */ + if (thd->open_tables) + close_open_tables(thd); if (prelocked_mode == PRELOCKED) { @@ -1675,6 +1704,7 @@ TABLE *find_temporary_table(THD *thd, TABLE_LIST *table_list) Try to locate the table in the list of thd->temporary_tables. If the table is found: + - if the table is being used by some outer statement, fail. - if the table is in thd->locked_tables, unlock it and remove it from the list of locked tables. Currently only transactional temporary tables are present in the locked_tables list. @@ -1689,24 +1719,34 @@ TABLE *find_temporary_table(THD *thd, TABLE_LIST *table_list) thd->temporary_tables list, it's impossible to tell here whether we're dealing with an internal or a user temporary table. - @retval TRUE the table was not found in the list of temporary tables - of this thread - @retval FALSE the table was found and dropped successfully. + @retval 0 the table was found and dropped successfully. + @retval 1 the table was not found in the list of temporary tables + of this thread + @retval -1 the table is in use by a outer query */ -bool close_temporary_table(THD *thd, TABLE_LIST *table_list) +int drop_temporary_table(THD *thd, TABLE_LIST *table_list) { TABLE *table; + DBUG_ENTER("drop_temporary_table"); if (!(table= find_temporary_table(thd, table_list))) - return 1; + DBUG_RETURN(1); + + /* Table might be in use by some outer statement. */ + if (table->query_id && table->query_id != thd->query_id) + { + my_error(ER_CANT_REOPEN_TABLE, MYF(0), table->alias); + DBUG_RETURN(-1); + } + /* If LOCK TABLES list is not empty and contains this table, unlock the table and remove the table from this list. */ mysql_lock_remove(thd, thd->locked_tables, table, FALSE); close_temporary_table(thd, table, 1, 1); - return 0; + DBUG_RETURN(0); } /* @@ -2285,8 +2325,7 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, is always represented by only one TABLE object in THD, and it can not be cloned. Emit an error for an unsupported behaviour. */ - if (table->query_id == thd->query_id || - thd->prelocked_mode && table->query_id) + if (table->query_id) { DBUG_PRINT("error", ("query_id: %lu server_id: %u pseudo_thread_id: %lu", @@ -2296,7 +2335,6 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, DBUG_RETURN(0); } table->query_id= thd->query_id; - table->clear_query_id= 1; thd->thread_specific_used= TRUE; DBUG_PRINT("info",("Using temporary table")); goto reset; @@ -4306,7 +4344,6 @@ void close_tables_for_reopen(THD *thd, TABLE_LIST **tables) sp_remove_not_own_routines(thd->lex); for (TABLE_LIST *tmp= *tables; tmp; tmp= tmp->next_global) tmp->table= 0; - mark_used_tables_as_free_for_reuse(thd, thd->temporary_tables); close_thread_tables(thd); } diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index ed7e30c1fef..19a99f9d12b 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -151,6 +151,14 @@ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables) } VOID(pthread_mutex_unlock(&LOCK_open)); } + else if (tables->table) + { + /* Must be a temporary table */ + TABLE *table= tables->table; + table->file->ha_index_or_rnd_end(); + table->query_id= thd->query_id; + table->open_by_handler= 0; + } } /* @@ -282,6 +290,12 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) goto err; } + /* + If it's a temp table, don't reset table->query_id as the table is + being used by this handler. Otherwise, no meaning at all. + */ + tables->table->open_by_handler= 1; + if (! reopen) send_ok(thd); DBUG_PRINT("exit",("OK")); diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 077141c4d7a..d3010e4309b 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -3399,7 +3399,7 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info, it preparable for open. But let us do close_temporary_table() here just in case. */ - close_temporary_table(thd, create_table); + drop_temporary_table(thd, create_table); } } } diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index ea6a25d9866..7d04f564cf3 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -992,9 +992,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd, /* Multiple queries exits, execute them individually */ - if (thd->lock || thd->open_tables || thd->derived_tables || - thd->prelocked_mode) - close_thread_tables(thd); + close_thread_tables(thd); ulong length= (ulong)(packet_end - next_packet); log_slow_statement(thd); @@ -1331,12 +1329,11 @@ bool dispatch_command(enum enum_server_command command, THD *thd, my_message(ER_UNKNOWN_COM_ERROR, ER(ER_UNKNOWN_COM_ERROR), MYF(0)); break; } - if (thd->lock || thd->open_tables || thd->derived_tables || - thd->prelocked_mode) - { - thd->proc_info="closing tables"; - close_thread_tables(thd); /* Free tables */ - } + + thd->proc_info= "closing tables"; + /* Free tables */ + close_thread_tables(thd); + /* assume handlers auto-commit (if some doesn't - transaction handling in MySQL should be redesigned to support it; it's a big change, diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 6edb8494b03..e10e89be478 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -1503,7 +1503,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, char path[FN_REFLEN], *alias; uint path_length; String wrong_tables; - int error; + int error= 0; int non_temp_tables_count= 0; bool some_tables_deleted=0, tmp_table_deleted=0, foreign_key_error=0; String built_query; @@ -1563,10 +1563,27 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, enum legacy_db_type frm_db_type; mysql_ha_flush(thd, table, MYSQL_HA_CLOSE_FINAL, 1); - if (!close_temporary_table(thd, table)) - { - tmp_table_deleted=1; - continue; // removed temporary table + + error= drop_temporary_table(thd, table); + + switch (error) { + case 0: + // removed temporary table + tmp_table_deleted= 1; + continue; + case -1: + // table already in use + /* + XXX: This branch should never be taken outside of SF, trigger or + prelocked mode. + + DBUG_ASSERT(thd->in_sub_stmt); + */ + error= 1; + goto err_with_placeholders; + default: + // temporary table not found + error= 0; } /* @@ -1593,7 +1610,6 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, built_query.append("`,"); } - error=0; table_type= table->db_type; if (!drop_temporary) { diff --git a/sql/table.h b/sql/table.h index 6554b6ed578..2bbd71b70c6 100644 --- a/sql/table.h +++ b/sql/table.h @@ -499,6 +499,24 @@ struct st_table { my_bitmap_map *bitmap_init_value; MY_BITMAP def_read_set, def_write_set, tmp_set; /* containers */ MY_BITMAP *read_set, *write_set; /* Active column sets */ + /* + The ID of the query that opened and is using this table. Has different + meanings depending on the table type. + + Temporary tables: + + table->query_id is set to thd->query_id for the duration of a statement + and is reset to 0 once it is closed by the same statement. A non-zero + table->query_id means that a statement is using the table even if it's + not the current statement (table is in use by some outer statement). + + Non-temporary tables: + + Under pre-locked or LOCK TABLES mode: query_id is set to thd->query_id + for the duration of a statement and is reset to 0 once it is closed by + the same statement. A non-zero query_id is used to control which tables + in the list of pre-opened and locked tables are actually being used. + */ query_id_t query_id; /* @@ -593,8 +611,8 @@ struct st_table { my_bool locked_by_name; my_bool fulltext_searched; my_bool no_cache; - /* To signal that we should reset query_id for tables and cols */ - my_bool clear_query_id; + /* To signal that the table is associated with a HANDLER statement */ + my_bool open_by_handler; /* To indicate that a non-null value of the auto_increment field was provided by the user or retrieved from the current record. From 0c1bdcbd2443a0e6514652aac5ba41c72882ccd0 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 2 Nov 2007 02:36:12 +0300 Subject: [PATCH 30/31] A fix for Bug#32030 "DELETE does not return an error and deletes rows if error evaluating WHERE" DELETE with a subquery in WHERE clause would sometimes ignore subquery evaluation error and proceed with deletion. The fix is to check for an error after evaluation of the WHERE clause in DELETE. Addressed review comments. mysql-test/r/group_min_max.result: Update the test results to reflect the fix for Bug#32030. mysql-test/r/ps.result: Update test results (Bug#32030) mysql-test/t/group_min_max.test: Update the test case to reflect the fix for Bug#32030 mysql-test/t/ps.test: Add a test case for Bug#32030 sql/sql_delete.cc: Check for an error before calling send_ok(). Two different places are covered because the subquery code has slightly different execution paths depending on ps-protocol/old-protocol --- mysql-test/r/group_min_max.result | 3 +-- mysql-test/r/ps.result | 20 +++++++++++++++++++ mysql-test/t/group_min_max.test | 1 + mysql-test/t/ps.test | 33 +++++++++++++++++++++++++++++++ sql/sql_delete.cc | 11 +++++++++-- 5 files changed, 64 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/group_min_max.result b/mysql-test/r/group_min_max.result index f62fd662a5d..270f248ea9c 100644 --- a/mysql-test/r/group_min_max.result +++ b/mysql-test/r/group_min_max.result @@ -2299,8 +2299,7 @@ Handler_read_next 0 FLUSH STATUS; DELETE FROM t3 WHERE (SELECT (SELECT MAX(b) FROM t1 GROUP BY a HAVING a < 2) x FROM t1) > 10000; -Warnings: -Error 1242 Subquery returns more than 1 row +ERROR 21000: Subquery returns more than 1 row SHOW STATUS LIKE 'handler_read__e%'; Variable_name Value Handler_read_key 8 diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index edac68a88d6..db9f0613d7f 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -2680,4 +2680,24 @@ t1 CREATE TABLE `t1` ( KEY `c` (`c`(10)) ) ENGINE=MyISAM DEFAULT CHARSET=latin1 drop table t1; +drop table if exists t1, t2; +Warnings: +Note 1051 Unknown table 't1' +Note 1051 Unknown table 't2' +create table t1 (a int, b int); +create table t2 like t1; +insert into t1 (a, b) values (1,1), (1,2), (1,3), (1,4), (1,5), +(2,2), (2,3), (2,1), (3,1), (4,1), (4,2), (4,3), (4,4), (4,5), (4,6); +insert into t2 select a, max(b) from t1 group by a; +prepare stmt from "delete from t2 where (select (select max(b) from t1 group +by a having a < 2) x from t1) > 10000"; +delete from t2 where (select (select max(b) from t1 group +by a having a < 2) x from t1) > 10000; +ERROR 21000: Subquery returns more than 1 row +execute stmt; +ERROR 21000: Subquery returns more than 1 row +execute stmt; +ERROR 21000: Subquery returns more than 1 row +deallocate prepare stmt; +drop table t1, t2; End of 5.1 tests. diff --git a/mysql-test/t/group_min_max.test b/mysql-test/t/group_min_max.test index cf25b4c61be..9d1e065797d 100644 --- a/mysql-test/t/group_min_max.test +++ b/mysql-test/t/group_min_max.test @@ -890,6 +890,7 @@ FLUSH STATUS; DELETE FROM t3 WHERE (SELECT MAX(b) FROM t1 GROUP BY a HAVING a < 2) > 10000; SHOW STATUS LIKE 'handler_read__e%'; FLUSH STATUS; +--error ER_SUBQUERY_NO_1_ROW DELETE FROM t3 WHERE (SELECT (SELECT MAX(b) FROM t1 GROUP BY a HAVING a < 2) x FROM t1) > 10000; SHOW STATUS LIKE 'handler_read__e%'; diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index dea86bdd2fa..c528fddaf93 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -2778,4 +2778,37 @@ execute stmt; show create table t1; drop table t1; +# +# Bug #32030 DELETE does not return an error and deletes rows if error +# evaluating WHERE +# +# Test that there is an error for prepared delete just like for the normal +# one. +# + +drop table if exists t1, t2; +create table t1 (a int, b int); +create table t2 like t1; + +insert into t1 (a, b) values (1,1), (1,2), (1,3), (1,4), (1,5), + (2,2), (2,3), (2,1), (3,1), (4,1), (4,2), (4,3), (4,4), (4,5), (4,6); + +insert into t2 select a, max(b) from t1 group by a; + +prepare stmt from "delete from t2 where (select (select max(b) from t1 group +by a having a < 2) x from t1) > 10000"; + +--error ER_SUBQUERY_NO_1_ROW +delete from t2 where (select (select max(b) from t1 group +by a having a < 2) x from t1) > 10000; +--error ER_SUBQUERY_NO_1_ROW +execute stmt; +--error ER_SUBQUERY_NO_1_ROW +execute stmt; + +deallocate prepare stmt; +drop table t1, t2; + + + --echo End of 5.1 tests. diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index 219dc90429b..f183cb3142f 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -35,6 +35,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, READ_RECORD info; bool using_limit=limit != HA_POS_ERROR; bool transactional_table, safe_update, const_cond; + bool const_cond_result; ha_rows deleted= 0; uint usable_index= MAX_KEY; SELECT_LEX *select_lex= &thd->lex->select_lex; @@ -86,6 +87,12 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, select_lex->no_error= thd->lex->ignore; + const_cond_result= const_cond && (!conds || conds->val_int()); + if (thd->is_error()) + { + /* Error evaluating val_int(). */ + DBUG_RETURN(TRUE); + } /* Test if the user wants to delete all rows and deletion doesn't have any side-effects (because of triggers), so we can use optimized @@ -105,7 +112,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, - We should not be binlogging this statement row-based, and - there should be no delete triggers associated with the table. */ - if (!using_limit && const_cond && (!conds || conds->val_int()) && + if (!using_limit && const_cond_result && !(specialflag & (SPECIAL_NO_NEW_FUNC | SPECIAL_SAFE_MODE)) && (thd->lex->sql_command == SQLCOM_TRUNCATE || (!thd->current_stmt_binlog_row_based && @@ -300,7 +307,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *table_list, COND *conds, else table->file->unlock_row(); // Row failed selection, release lock on it } - if (thd->killed && !error) + if (thd->killed || thd->is_error()) error= 1; // Aborted if (will_batch && (loc_error= table->file->end_bulk_delete())) { From 6a79a2196fc40367191883a3dbe0d350bd7dfc7f Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 2 Nov 2007 14:47:18 +0300 Subject: [PATCH 31/31] Cleanup the test case for Bug#32030 "DELETE does not return an error and deletes rows if error evaluating WHERE" mysql-test/r/ps.result: Disable warnings. mysql-test/t/ps.test: Disable warnings. --- mysql-test/r/ps.result | 3 --- mysql-test/t/ps.test | 3 ++- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index db9f0613d7f..9d0906ad5e7 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -2681,9 +2681,6 @@ t1 CREATE TABLE `t1` ( ) ENGINE=MyISAM DEFAULT CHARSET=latin1 drop table t1; drop table if exists t1, t2; -Warnings: -Note 1051 Unknown table 't1' -Note 1051 Unknown table 't2' create table t1 (a int, b int); create table t2 like t1; insert into t1 (a, b) values (1,1), (1,2), (1,3), (1,4), (1,5), diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index c528fddaf93..fd7caeb195e 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -2785,8 +2785,9 @@ drop table t1; # Test that there is an error for prepared delete just like for the normal # one. # - +--disable_warnings drop table if exists t1, t2; +--enable_warnings create table t1 (a int, b int); create table t2 like t1;