From d238bd6755b8c64b4db9da827178de7daf87048f Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@alik." <> Date: Tue, 3 Oct 2006 18:42:59 +0400 Subject: [PATCH 01/23] Patch for BUG#15934: im_daemon_life_cycle fails sporadically. The problem was a race condition in a test case. The fix eliminates the race condition by explicit wait on UNIX socket to start accepting connections. The patch affects only test suite (i.e. does not touch server codebase). --- mysql-test/mysql-test-run.pl | 6 +++ mysql-test/r/im_daemon_life_cycle.result | 1 + mysql-test/t/im_daemon_life_cycle.imtest | 6 +++ mysql-test/t/wait_for_socket.sh | 62 ++++++++++++++++++++++++ 4 files changed, 75 insertions(+) create mode 100755 mysql-test/t/wait_for_socket.sh diff --git a/mysql-test/mysql-test-run.pl b/mysql-test/mysql-test-run.pl index 06dd3864212..41d86944ada 100755 --- a/mysql-test/mysql-test-run.pl +++ b/mysql-test/mysql-test-run.pl @@ -1249,6 +1249,9 @@ sub environment_setup () { $ENV{'IM_PATH_PID'}= $instance_manager->{path_pid}; $ENV{'IM_PATH_ANGEL_PID'}= $instance_manager->{path_angel_pid}; $ENV{'IM_PORT'}= $instance_manager->{port}; + $ENV{'IM_PATH_SOCK'}= $instance_manager->{path_sock}; + $ENV{'IM_USERNAME'}= $instance_manager->{admin_login}; + $ENV{'IM_PASSWORD'}= $instance_manager->{admin_password}; $ENV{'IM_MYSQLD1_SOCK'}= $instance_manager->{instances}->[0]->{path_sock}; $ENV{'IM_MYSQLD1_PORT'}= $instance_manager->{instances}->[0]->{port}; @@ -1257,6 +1260,9 @@ sub environment_setup () { $ENV{'IM_MYSQLD2_PORT'}= $instance_manager->{instances}->[1]->{port}; $ENV{'IM_MYSQLD2_PATH_PID'}=$instance_manager->{instances}->[1]->{path_pid}; + $ENV{'EXE_MYSQL'}= $exe_mysql; + + $ENV{MTR_BUILD_THREAD}= 0 unless $ENV{MTR_BUILD_THREAD}; # Set if not set # We are nice and report a bit about our settings diff --git a/mysql-test/r/im_daemon_life_cycle.result b/mysql-test/r/im_daemon_life_cycle.result index 52a69d98fcf..a0849313b19 100644 --- a/mysql-test/r/im_daemon_life_cycle.result +++ b/mysql-test/r/im_daemon_life_cycle.result @@ -8,6 +8,7 @@ mysqld2 offline Killing the process... Sleeping... Success: the process was restarted. +Success: server is ready to accept connection on socket. -------------------------------------------------------------------- -- Test for BUG#12751 diff --git a/mysql-test/t/im_daemon_life_cycle.imtest b/mysql-test/t/im_daemon_life_cycle.imtest index acd1f0d887b..408133ca3ba 100644 --- a/mysql-test/t/im_daemon_life_cycle.imtest +++ b/mysql-test/t/im_daemon_life_cycle.imtest @@ -17,6 +17,12 @@ ########################################################################### +# Wait for IM to start accepting connections. + +--exec $MYSQL_TEST_DIR/t/wait_for_socket.sh $EXE_MYSQL $IM_PATH_SOCK $IM_USERNAME $IM_PASSWORD '' 30 + +########################################################################### + # # BUG#12751: Instance Manager: client hangs # diff --git a/mysql-test/t/wait_for_socket.sh b/mysql-test/t/wait_for_socket.sh new file mode 100755 index 00000000000..3b900fa2208 --- /dev/null +++ b/mysql-test/t/wait_for_socket.sh @@ -0,0 +1,62 @@ +#!/bin/sh + +########################################################################### + +if [ $# -ne 6 ]; then + echo "Usage: wait_for_socket.sh " + exit 0 +fi + +client_exe="$1" +socket_path="$2" +username="$3" +password="$4" +db="$5" +total_timeout="$6" + +########################################################################### + +if [ -z "$client_exe" ]; then + echo "Error: invalid path to client executable ($client_exe)." + exit 0; +fi + +if [ ! -x "$client_exe" ]; then + echo "Error: client by path '$client_exe' is not available." + exit 0; +fi + +if [ -z "$socket_path" ]; then + echo "Error: invalid socket patch." + exit 0 +fi + +########################################################################### + +client_args="--silent --socket=$socket_path " + +[ -n "$username" ] && client_args="$client_args --user=$username " +[ -n "$password" ] && client_args="$client_args --password=$password " +[ -n "$db" ] && client_args="$client_args $db" + +########################################################################### + +cur_attempt=1 + +while true; do + + if ( echo 'quit' | "$client_exe" $client_args >/dev/null 2>&1 ); then + echo "Success: server is ready to accept connection on socket." + exit 0 + fi + + [ $cur_attempt -ge $total_timeout ] && break + + sleep 1 + + cur_attempt=`expr $cur_attempt + 1` + +done + +echo "Error: server does not accept connections after $total_timeout seconds." +exit 0 From 2073e4a1cb22bbb9c0246c9222c818377348a81f Mon Sep 17 00:00:00 2001 From: "petr/cps@mysql.com/owlet.local" <> Date: Thu, 5 Oct 2006 01:24:53 +0400 Subject: [PATCH 02/23] Fix Bug #22472 IM: --socket option should be removed from Windows version the option is useless on windows. It was removed from listing of mysqlmanager --help on Windows --- server-tools/instance-manager/options.cc | 4 ++-- server-tools/instance-manager/options.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/server-tools/instance-manager/options.cc b/server-tools/instance-manager/options.cc index a98c0e291be..c6c709295bc 100644 --- a/server-tools/instance-manager/options.cc +++ b/server-tools/instance-manager/options.cc @@ -45,10 +45,10 @@ const char *default_password_file_name= QUOTE(DEFAULT_PASSWORD_FILE_NAME); const char *default_log_file_name= QUOTE(DEFAULT_LOG_FILE_NAME); const char *Options::config_file= QUOTE(DEFAULT_CONFIG_FILE); const char *Options::angel_pid_file_name= NULL; +const char *Options::socket_file_name= QUOTE(DEFAULT_SOCKET_FILE_NAME); #endif const char *Options::log_file_name= default_log_file_name; const char *Options::pid_file_name= QUOTE(DEFAULT_PID_FILE_NAME); -const char *Options::socket_file_name= QUOTE(DEFAULT_SOCKET_FILE_NAME); const char *Options::password_file_name= default_password_file_name; const char *Options::default_mysqld_path= QUOTE(DEFAULT_MYSQLD_PATH); const char *Options::bind_address= 0; /* No default value */ @@ -106,11 +106,11 @@ static struct my_option my_long_options[] = (gptr *) &Options::angel_pid_file_name, (gptr *) &Options::angel_pid_file_name, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 }, -#endif { "socket", OPT_SOCKET, "Socket file to use for connection.", (gptr *) &Options::socket_file_name, (gptr *) &Options::socket_file_name, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0 }, +#endif { "passwd", 'P', "Prepare entry for passwd file and exit.", 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0 }, diff --git a/server-tools/instance-manager/options.h b/server-tools/instance-manager/options.h index ad3458869b6..f32e106bd7a 100644 --- a/server-tools/instance-manager/options.h +++ b/server-tools/instance-manager/options.h @@ -36,11 +36,11 @@ struct Options static char run_as_service; /* handle_options doesn't support bool */ static const char *user; static const char *angel_pid_file_name; + static const char *socket_file_name; #endif static bool is_forced_default_file; static const char *log_file_name; static const char *pid_file_name; - static const char *socket_file_name; static const char *password_file_name; static const char *default_mysqld_path; /* the option which should be passed to process_default_option_files */ From 96c6f6f5c206283b35d4b4a3cbb21f92295c3790 Mon Sep 17 00:00:00 2001 From: "petr/cps@mysql.com/owlet.local" <> Date: Thu, 5 Oct 2006 22:07:21 +0400 Subject: [PATCH 03/23] Fix Bug #19368 Failure in "flush_instances" causes assert in Thread_registry Stop guardian and all the rest of threads before shutdown in case of an error --- server-tools/instance-manager/instance_map.cc | 4 +++- server-tools/instance-manager/manager.cc | 18 +++++++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/server-tools/instance-manager/instance_map.cc b/server-tools/instance-manager/instance_map.cc index 3b7f58d8a09..f6662847440 100644 --- a/server-tools/instance-manager/instance_map.cc +++ b/server-tools/instance-manager/instance_map.cc @@ -215,7 +215,9 @@ int Instance_map::flush_instances() hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0, get_instance_key, delete_instance, 0); rc= load(); - guardian->init(); // TODO: check error status. + /* don't init guardian if we failed to load instances */ + if (!rc) + guardian->init(); // TODO: check error status. pthread_mutex_unlock(&LOCK_instance_map); guardian->unlock(); return rc; diff --git a/server-tools/instance-manager/manager.cc b/server-tools/instance-manager/manager.cc index 353dfcf64dc..6f28c39da77 100644 --- a/server-tools/instance-manager/manager.cc +++ b/server-tools/instance-manager/manager.cc @@ -104,6 +104,19 @@ int my_sigwait(const sigset_t *set, int *sig) #endif +void stop_all(Guardian_thread *guardian, Thread_registry *registry) +{ + /* + Let guardian thread know that it should break it's processing cycle, + once it wakes up. + */ + guardian->request_shutdown(true); + /* wake guardian */ + pthread_cond_signal(&guardian->COND_guardian); + /* stop all threads */ + registry->deliver_shutdown(); +} + /* manager - entry point to the main instance manager process: start listener thread, write pid file and enter into signal handling. @@ -210,7 +223,8 @@ void manager(const Options &options) log_error("Cannot init instances repository. This might be caused by " "the wrong config file options. For instance, missing mysqld " "binary. Aborting."); - return; + stop_all(&guardian_thread, &thread_registry); + goto err; } /* create the listener */ @@ -227,6 +241,7 @@ void manager(const Options &options) if (rc) { log_error("manager(): set_stacksize_n_create_thread(listener) failed"); + stop_all(&guardian_thread, &thread_registry); goto err; } @@ -245,6 +260,7 @@ void manager(const Options &options) if ((status= my_sigwait(&mask, &signo)) != 0) { log_error("sigwait() failed"); + stop_all(&guardian_thread, &thread_registry); goto err; } From ee0cebf9a7fa364df44aeb0579bdaa80d11f0125 Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Fri, 6 Oct 2006 13:34:07 +0400 Subject: [PATCH 04/23] BUG#21726: Incorrect result with multiple invocations of LAST_INSERT_ID. Note: bug#21726 does not directly apply to 4.1, as it doesn't have stored procedures. However, 4.1 had some bugs that were fixed in 5.0 by the patch for bug#21726, and this patch is a backport of those fixes. Namely, in 4.1 it fixes: - LAST_INSERT_ID(expr) didn't return value of expr (4.1 specific). - LAST_INSERT_ID() could return the value generated by current statement if the call happens after the generation, like in CREATE TABLE t1 (i INT AUTO_INCREMENT PRIMARY KEY, j INT); INSERT INTO t1 VALUES (NULL, 0), (NULL, LAST_INSERT_ID()); - Redundant binary log LAST_INSERT_ID_EVENTs could be generated. --- mysql-test/r/rpl_insert_id.result | 27 +++++++++++++++++ mysql-test/t/rpl_insert_id.test | 32 +++++++++++++++++++++ sql/item_func.cc | 31 ++++++++++++++++++-- sql/item_func.h | 1 + sql/log_event.cc | 1 - sql/set_var.cc | 8 ++++-- sql/sql_class.h | 48 +++++++++++++++++++++---------- sql/sql_insert.cc | 8 ++---- sql/sql_load.cc | 12 +++----- sql/sql_parse.cc | 6 ++++ sql/sql_select.cc | 11 +++++-- sql/sql_update.cc | 4 +-- tests/mysql_client_test.c | 38 ++++++++++++++++++++++++ 13 files changed, 189 insertions(+), 38 deletions(-) diff --git a/mysql-test/r/rpl_insert_id.result b/mysql-test/r/rpl_insert_id.result index fbdc9dc06cf..43aba68d041 100644 --- a/mysql-test/r/rpl_insert_id.result +++ b/mysql-test/r/rpl_insert_id.result @@ -108,6 +108,33 @@ a 1 drop table t1; drop table t2; +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 ( +i INT NOT NULL AUTO_INCREMENT PRIMARY KEY, +j INT DEFAULT 0 +); +INSERT INTO t1 VALUES (NULL, -1); +INSERT INTO t1 VALUES (NULL, LAST_INSERT_ID()), (NULL, LAST_INSERT_ID(5)), +(NULL, @@LAST_INSERT_ID); +INSERT INTO t1 VALUES (NULL, 0), (NULL, LAST_INSERT_ID()); +UPDATE t1 SET j= -1 WHERE i IS NULL; +SELECT * FROM t1; +i j +1 -1 +2 1 +3 5 +4 1 +5 -1 +6 2 +SELECT * FROM t1; +i j +1 -1 +2 1 +3 5 +4 1 +5 -1 +6 2 +DROP TABLE t1; # # End of 4.1 tests # diff --git a/mysql-test/t/rpl_insert_id.test b/mysql-test/t/rpl_insert_id.test index 327094a1394..7fb514fb7af 100644 --- a/mysql-test/t/rpl_insert_id.test +++ b/mysql-test/t/rpl_insert_id.test @@ -108,6 +108,38 @@ drop table t1; drop table t2; sync_slave_with_master; + +# +# BUG#21726: Incorrect result with multiple invocations of +# LAST_INSERT_ID +# +connection master; + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 ( + i INT NOT NULL AUTO_INCREMENT PRIMARY KEY, + j INT DEFAULT 0 +); + +INSERT INTO t1 VALUES (NULL, -1); +INSERT INTO t1 VALUES (NULL, LAST_INSERT_ID()), (NULL, LAST_INSERT_ID(5)), + (NULL, @@LAST_INSERT_ID); +# Test replication of substitution "IS NULL" -> "= LAST_INSERT_ID". +INSERT INTO t1 VALUES (NULL, 0), (NULL, LAST_INSERT_ID()); +UPDATE t1 SET j= -1 WHERE i IS NULL; + +SELECT * FROM t1; + +sync_slave_with_master; +SELECT * FROM t1; + +connection master; +DROP TABLE t1; + + --echo # --echo # End of 4.1 tests --echo # diff --git a/sql/item_func.cc b/sql/item_func.cc index 91ccef6511f..6f8eed42e51 100644 --- a/sql/item_func.cc +++ b/sql/item_func.cc @@ -2230,6 +2230,30 @@ longlong Item_func_release_lock::val_int() } +bool Item_func_last_insert_id::fix_fields(THD *thd, TABLE_LIST *tables, + Item **ref) +{ + DBUG_ASSERT(fixed == 0); + + if (Item_int_func::fix_fields(thd, tables, ref)) + return TRUE; + + if (arg_count == 0) + { + /* + As this statement calls LAST_INSERT_ID(), set + THD::last_insert_id_used. + */ + thd->last_insert_id_used= TRUE; + null_value= FALSE; + } + + thd->lex->uncacheable(UNCACHEABLE_SIDEEFFECT); + + return FALSE; +} + + longlong Item_func_last_insert_id::val_int() { DBUG_ASSERT(fixed == 1); @@ -2239,12 +2263,13 @@ longlong Item_func_last_insert_id::val_int() longlong value=args[0]->val_int(); thd->insert_id(value); null_value=args[0]->null_value; + return value; } - else - thd->lex->uncacheable(UNCACHEABLE_SIDEEFFECT); - return thd->last_insert_id_used ? thd->current_insert_id : thd->insert_id(); + + return thd->current_insert_id; } + /* This function is just used to test speed of different functions */ longlong Item_func_benchmark::val_int() diff --git a/sql/item_func.h b/sql/item_func.h index bc6c955b99f..467b88eda76 100644 --- a/sql/item_func.h +++ b/sql/item_func.h @@ -758,6 +758,7 @@ public: longlong val_int(); const char *func_name() const { return "last_insert_id"; } void fix_length_and_dec() { if (arg_count) max_length= args[0]->max_length; } + bool fix_fields(THD *thd, TABLE_LIST *tables, Item **ref); }; class Item_func_benchmark :public Item_int_func diff --git a/sql/log_event.cc b/sql/log_event.cc index 19c32b2d28e..412ebbce0ac 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -2255,7 +2255,6 @@ int Intvar_log_event::exec_event(struct st_relay_log_info* rli) { switch (type) { case LAST_INSERT_ID_EVENT: - thd->last_insert_id_used = 1; thd->last_insert_id = val; break; case INSERT_ID_EVENT: diff --git a/sql/set_var.cc b/sql/set_var.cc index 4acedc7bcbd..88e120632e2 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -2404,8 +2404,12 @@ bool sys_var_last_insert_id::update(THD *thd, set_var *var) byte *sys_var_last_insert_id::value_ptr(THD *thd, enum_var_type type, LEX_STRING *base) { - thd->sys_var_tmp.long_value= (long) thd->insert_id(); - return (byte*) &thd->last_insert_id; + /* + As this statement reads @@LAST_INSERT_ID, set + THD::last_insert_id_used. + */ + thd->last_insert_id_used= TRUE; + return (byte*) &thd->current_insert_id; } diff --git a/sql/sql_class.h b/sql/sql_class.h index a995a492bc8..5c4c3af7acb 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -835,17 +835,29 @@ public: generated auto_increment value in handler.cc */ ulonglong next_insert_id; + /* - The insert_id used for the last statement or set by SET LAST_INSERT_ID=# - or SELECT LAST_INSERT_ID(#). Used for binary log and returned by - LAST_INSERT_ID() + At the beginning of the statement last_insert_id holds the first + generated value of the previous statement. During statement + execution it is updated to the value just generated, but then + restored to the value that was generated first, so for the next + statement it will again be "the first generated value of the + previous statement". + + It may also be set with "LAST_INSERT_ID(expr)" or + "@@LAST_INSERT_ID= expr", but the effect of such setting will be + seen only in the next statement. */ ulonglong last_insert_id; + /* - Set to the first value that LAST_INSERT_ID() returned for the last - statement. When this is set, last_insert_id_used is set to true. + current_insert_id remembers the first generated value of the + previous statement, and does not change during statement + execution. Its value returned from LAST_INSERT_ID() and + @@LAST_INSERT_ID. */ ulonglong current_insert_id; + ulonglong limit_found_rows; ha_rows cuted_fields, sent_row_count, examined_row_count; @@ -896,7 +908,22 @@ public: bool locked, some_tables_deleted; bool last_cuted_field; bool no_errors, password, is_fatal_error; - bool query_start_used,last_insert_id_used,insert_id_used,rand_used; + bool query_start_used, rand_used; + + /* + last_insert_id_used is set when current statement calls + LAST_INSERT_ID() or reads @@LAST_INSERT_ID, so that binary log + LAST_INSERT_ID_EVENT be generated. + */ + bool last_insert_id_used; + + /* + insert_id_used is set when current statement updates + THD::last_insert_id, so that binary log INSERT_ID_EVENT be + generated. + */ + bool insert_id_used; + /* for IS NULL => = last_insert_id() fix in remove_eq_conds() */ bool substitute_null_with_insert_id; bool time_zone_used; @@ -996,15 +1023,6 @@ public: insert_id_used=1; substitute_null_with_insert_id= TRUE; } - inline ulonglong insert_id(void) - { - if (!last_insert_id_used) - { - last_insert_id_used=1; - current_insert_id=last_insert_id; - } - return last_insert_id; - } inline ulonglong found_rows(void) { return limit_found_rows; diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 283fe571d53..43c7e5d456f 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -374,10 +374,8 @@ int mysql_insert(THD *thd,TABLE_LIST *table_list, if (error) break; /* - If auto_increment values are used, save the first one - for LAST_INSERT_ID() and for the update log. - We can't use insert_id() as we don't want to touch the - last_insert_id_used flag. + If auto_increment values are used, save the first one for + LAST_INSERT_ID() and for the update log. */ if (! id && thd->insert_id_used) { // Get auto increment value @@ -1687,7 +1685,7 @@ bool select_insert::send_data(List &values) { table->next_number_field->reset(); if (! last_insert_id && thd->insert_id_used) - last_insert_id=thd->insert_id(); + last_insert_id= thd->last_insert_id; } } DBUG_RETURN(error); diff --git a/sql/sql_load.cc b/sql/sql_load.cc index 4e6c458cc43..48862506729 100644 --- a/sql/sql_load.cc +++ b/sql/sql_load.cc @@ -466,10 +466,8 @@ read_fixed_length(THD *thd,COPY_INFO &info,TABLE *table,List &fields, if (write_record(table,&info)) DBUG_RETURN(1); /* - If auto_increment values are used, save the first one - for LAST_INSERT_ID() and for the binary/update log. - We can't use insert_id() as we don't want to touch the - last_insert_id_used flag. + If auto_increment values are used, save the first one for + LAST_INSERT_ID() and for the binary/update log. */ if (!id && thd->insert_id_used) id= thd->last_insert_id; @@ -572,10 +570,8 @@ read_sep_field(THD *thd,COPY_INFO &info,TABLE *table, if (write_record(table,&info)) DBUG_RETURN(1); /* - If auto_increment values are used, save the first one - for LAST_INSERT_ID() and for the binary/update log. - We can't use insert_id() as we don't want to touch the - last_insert_id_used flag. + If auto_increment values are used, save the first one for + LAST_INSERT_ID() and for the binary/update log. */ if (!id && thd->insert_id_used) id= thd->last_insert_id; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 98199ed22f1..cb2fa0f7014 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -1977,6 +1977,12 @@ mysql_execute_command(THD *thd) SELECT_LEX_UNIT *unit= &lex->unit; DBUG_ENTER("mysql_execute_command"); + /* + Remember first generated insert id value of the previous + statement. + */ + thd->current_insert_id= thd->last_insert_id; + /* Reset warning count for each query that uses tables A better approach would be to reset this for any commands diff --git a/sql/sql_select.cc b/sql/sql_select.cc index da96c98cd4f..117795059f0 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -4820,7 +4820,7 @@ remove_eq_conds(THD *thd, COND *cond, Item::cond_result *cond_value) Field *field=((Item_field*) args[0])->field; if (field->flags & AUTO_INCREMENT_FLAG && !field->table->maybe_null && (thd->options & OPTION_AUTO_IS_NULL) && - thd->insert_id() && thd->substitute_null_with_insert_id) + thd->current_insert_id && thd->substitute_null_with_insert_id) { #ifdef HAVE_QUERY_CACHE query_cache_abort(&thd->net); @@ -4828,9 +4828,16 @@ remove_eq_conds(THD *thd, COND *cond, Item::cond_result *cond_value) COND *new_cond; if ((new_cond= new Item_func_eq(args[0], new Item_int("last_insert_id()", - thd->insert_id(), + thd->current_insert_id, 21)))) { + /* + Set THD::last_insert_id_used manually, as this statement + uses LAST_INSERT_ID() in a sense, and should issue + LAST_INSERT_ID_EVENT. + */ + thd->last_insert_id_used= TRUE; + cond=new_cond; cond->fix_fields(thd, 0, &cond); } diff --git a/sql/sql_update.cc b/sql/sql_update.cc index af4ba8025f9..51643fc611d 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -408,7 +408,7 @@ int mysql_update(THD *thd, (ulong) thd->cuted_fields); send_ok(thd, (thd->client_capabilities & CLIENT_FOUND_ROWS) ? found : updated, - thd->insert_id_used ? thd->insert_id() : 0L,buff); + thd->insert_id_used ? thd->last_insert_id : 0L,buff); DBUG_PRINT("info",("%d records updated",updated)); } thd->count_cuted_fields= CHECK_FIELD_IGNORE; /* calc cuted fields */ @@ -1318,6 +1318,6 @@ bool multi_update::send_eof() (ulong) thd->cuted_fields); ::send_ok(thd, (thd->client_capabilities & CLIENT_FOUND_ROWS) ? found : updated, - thd->insert_id_used ? thd->insert_id() : 0L,buff); + thd->insert_id_used ? thd->last_insert_id : 0L,buff); return 0; } diff --git a/tests/mysql_client_test.c b/tests/mysql_client_test.c index 9fabde993b8..6ae9dcb9476 100644 --- a/tests/mysql_client_test.c +++ b/tests/mysql_client_test.c @@ -11908,6 +11908,43 @@ static void test_bug20152() } +/* + Bug#21726: Incorrect result with multiple invocations of + LAST_INSERT_ID + + Test that client gets updated value of insert_id on UPDATE that uses + LAST_INSERT_ID(expr). +*/ +static void test_bug21726() +{ + const char *update_query = "UPDATE t1 SET i= LAST_INSERT_ID(i + 1)"; + int rc; + my_ulonglong insert_id; + + DBUG_ENTER("test_bug21726"); + myheader("test_bug21726"); + + rc= mysql_query(mysql, "DROP TABLE IF EXISTS t1"); + myquery(rc); + rc= mysql_query(mysql, "CREATE TABLE t1 (i INT)"); + myquery(rc); + rc= mysql_query(mysql, "INSERT INTO t1 VALUES (1)"); + myquery(rc); + + rc= mysql_query(mysql, update_query); + myquery(rc); + insert_id= mysql_insert_id(mysql); + DIE_UNLESS(insert_id == 2); + + rc= mysql_query(mysql, update_query); + myquery(rc); + insert_id= mysql_insert_id(mysql); + DIE_UNLESS(insert_id == 3); + + DBUG_VOID_RETURN; +} + + /* Read and parse arguments and MySQL options from my.cnf */ @@ -12134,6 +12171,7 @@ static struct my_tests_st my_tests[]= { { "test_bug12925", test_bug12925 }, { "test_bug15613", test_bug15613 }, { "test_bug20152", test_bug20152 }, + { "test_bug21726", test_bug21726 }, { 0, 0 } }; From 6e809b249e8d8cc3c22eca6df5cf24c828368347 Mon Sep 17 00:00:00 2001 From: "malff/marcsql@weblab.(none)" <> Date: Mon, 9 Oct 2006 09:59:02 -0700 Subject: [PATCH 05/23] Bug#21462 (Stored procedures with no arguments require parenthesis) The syntax of the CALL statement, to invoke a stored procedure, has been changed to make the use of parenthesis optional in the argument list. With this change, "CALL p;" is equivalent to "CALL p();". While the SQL spec does not explicitely mandate this syntax, supporting it is needed for practical reasons, for integration with JDBC / ODBC connectors. Also, warnings in the sql/sql_yacc.yy file, which were not reported by Bison 2.1 but are now reported by Bison 2.2, have been fixed. The warning found were: bison -y -p MYSQL -d --debug --verbose sql_yacc.yy sql_yacc.yy:653.9-18: warning: symbol UNLOCK_SYM redeclared sql_yacc.yy:656.9-17: warning: symbol UNTIL_SYM redeclared sql_yacc.yy:658.9-18: warning: symbol UPDATE_SYM redeclared sql_yacc.yy:5169.11-5174.11: warning: unused value: $2 sql_yacc.yy:5208.11-5220.11: warning: unused value: $5 sql_yacc.yy:5221.11-5234.11: warning: unused value: $5 conflicts: 249 shift/reduce "unused value: $2" correspond to the $$=$1 assignment in the 1st {} block in table_ref -> join_table {} {}, which does not procude a result ($$) for the rule but an intermediate $2 value for the action instead. "unused value: $5" are similar, with $$ assignments in {} actions blocks which are not for the final reduce. --- mysql-test/r/sp.result | 27 +++++++++++++++++++++++++++ mysql-test/t/sp.test | 33 +++++++++++++++++++++++++++++++++ sql/sql_yacc.yy | 20 +++++++++++--------- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 31aa96ab05d..6e8a609d669 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -5470,5 +5470,32 @@ CAD CHF DROP FUNCTION bug21493| DROP TABLE t3,t4| +drop procedure if exists proc_21462_a| +drop procedure if exists proc_21462_b| +create procedure proc_21462_a() +begin +select "Called A"; +end| +create procedure proc_21462_b(x int) +begin +select "Called B"; +end| +call proc_21462_a| +Called A +Called A +call proc_21462_a()| +Called A +Called A +call proc_21462_a(1)| +ERROR 42000: Incorrect number of arguments for PROCEDURE test.proc_21462_a; expected 0, got 1 +call proc_21462_b| +ERROR 42000: Incorrect number of arguments for PROCEDURE test.proc_21462_b; expected 1, got 0 +call proc_21462_b()| +ERROR 42000: Incorrect number of arguments for PROCEDURE test.proc_21462_b; expected 1, got 0 +call proc_21462_b(1)| +Called B +Called B +drop procedure proc_21462_a| +drop procedure proc_21462_b| End of 5.0 tests drop table t1,t2; diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index 87d19baf888..ffbbf56d3ac 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -6420,6 +6420,39 @@ SELECT bug21493(Member_ID) FROM t3| DROP FUNCTION bug21493| DROP TABLE t3,t4| +# +# Bug#21462 Stored procedures with no arguments require parenthesis +# + +--disable_warnings +drop procedure if exists proc_21462_a| +drop procedure if exists proc_21462_b| +--enable_warnings + +create procedure proc_21462_a() +begin + select "Called A"; +end| + +create procedure proc_21462_b(x int) +begin + select "Called B"; +end| + +call proc_21462_a| +call proc_21462_a()| +-- error ER_SP_WRONG_NO_OF_ARGS +call proc_21462_a(1)| + +-- error ER_SP_WRONG_NO_OF_ARGS +call proc_21462_b| +-- error ER_SP_WRONG_NO_OF_ARGS +call proc_21462_b()| +call proc_21462_b(1)| + +drop procedure proc_21462_a| +drop procedure proc_21462_b| + --echo End of 5.0 tests diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index cb105d05332..06afbabbba4 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -650,11 +650,8 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize); %token UNIX_TIMESTAMP %token UNKNOWN_SYM %token UNLOCK_SYM -%token UNLOCK_SYM %token UNSIGNED %token UNTIL_SYM -%token UNTIL_SYM -%token UPDATE_SYM %token UPDATE_SYM %token UPGRADE_SYM %token USAGE @@ -1447,15 +1444,20 @@ call: lex->value_list.empty(); sp_add_used_routine(lex, YYTHD, $2, TYPE_ENUM_PROCEDURE); } - '(' sp_cparam_list ')' {} + opt_sp_cparam_list {} ; /* CALL parameters */ -sp_cparam_list: +opt_sp_cparam_list: /* Empty */ - | sp_cparams + | '(' opt_sp_cparams ')' ; +opt_sp_cparams: + /* Empty */ + | sp_cparams + ; + sp_cparams: sp_cparams ',' expr { @@ -5166,7 +5168,7 @@ when_list2: /* Warning - may return NULL in case of incomplete SELECT */ table_ref: table_factor { $$=$1; } - | join_table { $$=$1; } + | join_table { LEX *lex= Lex; if (!($$= lex->current_select->nest_last_join(lex->thd))) @@ -5208,7 +5210,7 @@ join_table: | table_ref normal_join table_ref ON { - YYERROR_UNLESS($1 && ($$=$3)); + YYERROR_UNLESS($1 && $3); /* Change the current name resolution context to a local context. */ if (push_new_name_resolution_context(YYTHD, $1, $3)) YYABORT; @@ -5223,7 +5225,7 @@ join_table: | table_ref STRAIGHT_JOIN table_factor ON { - YYERROR_UNLESS($1 && ($$=$3)); + YYERROR_UNLESS($1 && $3); /* Change the current name resolution context to a local context. */ if (push_new_name_resolution_context(YYTHD, $1, $3)) YYABORT; From 4a28f8f1a02284047b0d3c82b2e24770546799e9 Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Tue, 10 Oct 2006 13:44:04 +0400 Subject: [PATCH 06/23] Bug#19111: TRIGGERs selecting from a VIEW on the firing base table fail. In a trigger or a function used in a statement it is possible to do SELECT from a table being modified by the statement. However, encapsulation of such SELECT into a view and selecting from a view instead of direct SELECT was not possible. This happened because tables used by views (which in their turn were used from functions/triggers) were not excluded from checks in unique_table() routine as it happens for the rest of tables added to the statement table list for prelocking. With this fix we ignore all such tables in unique_table(), thus providing consistency: inside a trigger or a functions SELECT from a view may be used where plain SELECT is allowed. Modification of the same table from function or trigger is still disallowed. Also, this patch doesn't affect the case where SELECT from the table being modified is done outside of function of trigger, such SELECTs are still disallowed (this limitation and visibility problem when function select from a table being modified are subjects of bug 21326). See also bug 22427. --- mysql-test/r/view.result | 17 ++++++++++++++++- mysql-test/t/view.test | 39 ++++++++++++++++++++++++++++++++++++++- sql/sql_base.cc | 11 ++++++++--- 3 files changed, 62 insertions(+), 5 deletions(-) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 72cffb9531c..39db2164f2f 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -2735,4 +2735,19 @@ m e 4 a 1 b DROP VIEW v1; -DROP TABLE IF EXISTS t1,t2; +DROP TABLE t1,t2; +DROP FUNCTION IF EXISTS f1; +DROP VIEW IF EXISTS v1; +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (i INT); +INSERT INTO t1 VALUES (1); +CREATE VIEW v1 AS SELECT MAX(i) FROM t1; +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW +SET NEW.i = (SELECT * FROM v1) + 1; +INSERT INTO t1 VALUES (1); +CREATE FUNCTION f1() RETURNS INT RETURN (SELECT * FROM v1); +UPDATE t1 SET i= f1(); +DROP FUNCTION f1; +DROP VIEW v1; +DROP TABLE t1; +End of 5.0 tests. diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index a1c1e9b2ad1..56c0529a67b 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -2595,4 +2595,41 @@ CREATE TABLE t2 SELECT * FROM v1; SELECT * FROM t2; DROP VIEW v1; -DROP TABLE IF EXISTS t1,t2; +DROP TABLE t1,t2; + + +# +# Bug#19111: TRIGGERs selecting from a VIEW on the firing base table +# fail +# +# Allow to select from a view on a table being modified in a trigger +# and stored function, since plain select is allowed there. +# +--disable_warnings +DROP FUNCTION IF EXISTS f1; +DROP VIEW IF EXISTS v1; +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (i INT); +INSERT INTO t1 VALUES (1); + +CREATE VIEW v1 AS SELECT MAX(i) FROM t1; + +# Plain 'SET NEW.i = (SELECT MAX(i) FROM t1) + 1' works, so select +# from a view should work too. +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW + SET NEW.i = (SELECT * FROM v1) + 1; +INSERT INTO t1 VALUES (1); + +# Plain 'RETURN (SELECT MAX(i) FROM t1)' works in INSERT, so select +# from a view should work too. +CREATE FUNCTION f1() RETURNS INT RETURN (SELECT * FROM v1); +UPDATE t1 SET i= f1(); + +DROP FUNCTION f1; +DROP VIEW v1; +DROP TABLE t1; + + +--echo End of 5.0 tests. diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 5383bb52aaa..858d820c85e 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -805,6 +805,10 @@ TABLE_LIST *find_table_in_list(TABLE_LIST *table, Also SELECT::exclude_from_table_unique_test used to exclude from check tables of main SELECT of multi-delete and multi-update + We also skip tables with TABLE_LIST::prelocking_placeholder set, + because we want to allow SELECTs from them, and their modification + will rise the error anyway. + TODO: when we will have table/view change detection we can do this check only once for PS/SP @@ -851,12 +855,13 @@ TABLE_LIST* unique_table(THD *thd, TABLE_LIST *table, TABLE_LIST *table_list) if (((! (res= find_table_in_global_list(table_list, d_name, t_name))) && (! (res= mysql_lock_have_duplicate(thd, table, table_list)))) || ((!res->table || res->table != table->table) && - res->select_lex && !res->select_lex->exclude_from_table_unique_test)) + res->select_lex && !res->select_lex->exclude_from_table_unique_test && + !res->prelocking_placeholder)) break; /* - If we found entry of this table or or table of SELECT which already + If we found entry of this table or table of SELECT which already processed in derived table or top select of multi-update/multi-delete - (exclude_from_table_unique_test). + (exclude_from_table_unique_test) or prelocking placeholder. */ table_list= res->next_global; DBUG_PRINT("info", From fbf6507cf74688be3b5b687c3f99f3ef88e8dcb9 Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Tue, 10 Oct 2006 17:08:47 +0400 Subject: [PATCH 07/23] BUG#21354: (COUNT(*) = 1) not working in SELECT inside prepared statement. The problem was that during statement re-execution if the result was empty the old result could be returned for group functions. The solution is to implement proper cleanup() method in group functions. --- mysql-test/r/ps.result | 97 ++++++++++++++++++++++++++++++++++ mysql-test/t/func_gconcat.test | 2 +- mysql-test/t/ps.test | 74 +++++++++++++++++++++++++- sql/item_sum.cc | 1 + sql/item_sum.h | 48 ++++++++++++++++- 5 files changed, 219 insertions(+), 3 deletions(-) diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index 01aa4ddf859..82d8ba275b6 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -889,3 +889,100 @@ create temporary table if not exists t1 (a1 int); execute stmt; drop temporary table t1; deallocate prepare stmt; +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (i INT, INDEX(i)); +INSERT INTO t1 VALUES (1); +PREPARE stmt FROM "SELECT (COUNT(i) = 1), COUNT(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +(COUNT(i) = 1) COUNT(i) +0 0 +SET @a = 1; +EXECUTE stmt USING @a; +(COUNT(i) = 1) COUNT(i) +1 1 +SET @a = 0; +EXECUTE stmt USING @a; +(COUNT(i) = 1) COUNT(i) +0 0 +PREPARE stmt FROM "SELECT (AVG(i) = 1), AVG(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +(AVG(i) = 1) AVG(i) +NULL NULL +SET @a = 1; +EXECUTE stmt USING @a; +(AVG(i) = 1) AVG(i) +1 1.0000 +SET @a = 0; +EXECUTE stmt USING @a; +(AVG(i) = 1) AVG(i) +NULL NULL +PREPARE stmt FROM "SELECT (VARIANCE(i) = 1), VARIANCE(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +(VARIANCE(i) = 1) VARIANCE(i) +NULL NULL +SET @a = 1; +EXECUTE stmt USING @a; +(VARIANCE(i) = 1) VARIANCE(i) +0 0.0000 +SET @a = 0; +EXECUTE stmt USING @a; +(VARIANCE(i) = 1) VARIANCE(i) +NULL NULL +PREPARE stmt FROM "SELECT (STDDEV(i) = 1), STDDEV(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +(STDDEV(i) = 1) STDDEV(i) +NULL NULL +SET @a = 1; +EXECUTE stmt USING @a; +(STDDEV(i) = 1) STDDEV(i) +0 0.0000 +SET @a = 0; +EXECUTE stmt USING @a; +(STDDEV(i) = 1) STDDEV(i) +NULL NULL +PREPARE stmt FROM "SELECT (BIT_OR(i) = 1), BIT_OR(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +(BIT_OR(i) = 1) BIT_OR(i) +0 0 +SET @a = 1; +EXECUTE stmt USING @a; +(BIT_OR(i) = 1) BIT_OR(i) +1 1 +SET @a = 0; +EXECUTE stmt USING @a; +(BIT_OR(i) = 1) BIT_OR(i) +0 0 +PREPARE stmt FROM "SELECT (BIT_AND(i) = 1), BIT_AND(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +(BIT_AND(i) = 1) BIT_AND(i) +0 18446744073709551615 +SET @a = 1; +EXECUTE stmt USING @a; +(BIT_AND(i) = 1) BIT_AND(i) +1 1 +SET @a = 0; +EXECUTE stmt USING @a; +(BIT_AND(i) = 1) BIT_AND(i) +0 18446744073709551615 +PREPARE stmt FROM "SELECT (BIT_XOR(i) = 1), BIT_XOR(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +(BIT_XOR(i) = 1) BIT_XOR(i) +0 0 +SET @a = 1; +EXECUTE stmt USING @a; +(BIT_XOR(i) = 1) BIT_XOR(i) +1 1 +SET @a = 0; +EXECUTE stmt USING @a; +(BIT_XOR(i) = 1) BIT_XOR(i) +0 0 +DEALLOCATE PREPARE stmt; +DROP TABLE t1; +End of 4.1 tests. diff --git a/mysql-test/t/func_gconcat.test b/mysql-test/t/func_gconcat.test index 8f50690dd8b..21ec7362877 100644 --- a/mysql-test/t/func_gconcat.test +++ b/mysql-test/t/func_gconcat.test @@ -99,7 +99,7 @@ select ifnull(group_concat(concat(t1.id, ':', t1.name)), 'shortname') as 'withou select distinct ifnull(group_concat(concat(t1.id, ':', t1.name)), 'shortname') as 'with distinct: cutoff at length of shortname' from t1; drop table t1; -# check zero rows +# check zero rows (bug#836) create table t1(id int); create table t2(id int); insert into t1 values(0),(1); diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index 0ca293eb1ba..03fcc241d23 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -951,4 +951,76 @@ execute stmt; drop temporary table t1; deallocate prepare stmt; -# End of 4.1 tests + +# +# BUG#21354: (COUNT(*) = 1) not working in SELECT inside prepared +# statement +# +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (i INT, INDEX(i)); +INSERT INTO t1 VALUES (1); + +PREPARE stmt FROM "SELECT (COUNT(i) = 1), COUNT(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +SET @a = 1; +EXECUTE stmt USING @a; +SET @a = 0; +EXECUTE stmt USING @a; + +PREPARE stmt FROM "SELECT (AVG(i) = 1), AVG(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +SET @a = 1; +EXECUTE stmt USING @a; +SET @a = 0; +EXECUTE stmt USING @a; + +PREPARE stmt FROM "SELECT (VARIANCE(i) = 1), VARIANCE(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +SET @a = 1; +EXECUTE stmt USING @a; +SET @a = 0; +EXECUTE stmt USING @a; + +PREPARE stmt FROM "SELECT (STDDEV(i) = 1), STDDEV(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +SET @a = 1; +EXECUTE stmt USING @a; +SET @a = 0; +EXECUTE stmt USING @a; + +PREPARE stmt FROM "SELECT (BIT_OR(i) = 1), BIT_OR(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +SET @a = 1; +EXECUTE stmt USING @a; +SET @a = 0; +EXECUTE stmt USING @a; + +PREPARE stmt FROM "SELECT (BIT_AND(i) = 1), BIT_AND(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +SET @a = 1; +EXECUTE stmt USING @a; +SET @a = 0; +EXECUTE stmt USING @a; + +PREPARE stmt FROM "SELECT (BIT_XOR(i) = 1), BIT_XOR(i) FROM t1 WHERE i = ?"; +SET @a = 0; +EXECUTE stmt USING @a; +SET @a = 1; +EXECUTE stmt USING @a; +SET @a = 0; +EXECUTE stmt USING @a; + +DEALLOCATE PREPARE stmt; +DROP TABLE t1; + + +--echo End of 4.1 tests. diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 0b9b10d05d4..e480cb031d8 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -312,6 +312,7 @@ longlong Item_sum_count::val_int() void Item_sum_count::cleanup() { DBUG_ENTER("Item_sum_count::cleanup"); + clear(); Item_sum_int::cleanup(); used_table_cache= ~(table_map) 0; DBUG_VOID_RETURN; diff --git a/sql/item_sum.h b/sql/item_sum.h index 0cc2a20faa3..ea0863fc41c 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -58,9 +58,30 @@ public: Item_sum(THD *thd, Item_sum *item); enum Type type() const { return SUM_FUNC_ITEM; } virtual enum Sumfunctype sum_func () const=0; + + /* + This method is similar to add(), but it is called when the current + aggregation group changes. Thus it performs a combination of + clear() and add(). + */ inline bool reset() { clear(); return add(); }; + + /* + Prepare this item for evaluation of an aggregate value. This is + called by reset() when a group changes, or, for correlated + subqueries, between subquery executions. E.g. for COUNT(), this + method should set count= 0; + */ virtual void clear()= 0; + + /* + This method is called for the next row in the same group. Its + purpose is to aggregate the new value to the previous values in + the group (i.e. since clear() was called last time). For example, + for COUNT(), do count++. + */ virtual bool add()=0; + /* Called when new group is started and results are being saved in a temporary table. Similar to reset(), but must also store value in @@ -86,7 +107,17 @@ public: void make_field(Send_field *field); void print(String *str); void fix_num_length_and_dec(); - void no_rows_in_result() { reset(); } + + /* + This function is called by the execution engine to assign 'NO ROWS + FOUND' value to an aggregate item, when the underlying result set + has no rows. Such value, in a general case, may be different from + the default value of the item after 'clear()': e.g. a numeric item + may be initialized to 0 by clear() and to NULL by + no_rows_in_result(). + */ + void no_rows_in_result() { clear(); } + virtual bool setup(THD *thd) {return 0;} virtual void make_unique() {} Item *get_tmp_table_item(THD *thd); @@ -304,6 +335,11 @@ class Item_sum_avg :public Item_sum_num void no_rows_in_result() {} const char *func_name() const { return "avg"; } Item *copy_or_same(THD* thd); + void cleanup() + { + clear(); + Item_sum_num::cleanup(); + } }; class Item_sum_variance; @@ -361,6 +397,11 @@ class Item_sum_variance : public Item_sum_num void no_rows_in_result() {} const char *func_name() const { return "variance"; } Item *copy_or_same(THD* thd); + void cleanup() + { + clear(); + Item_sum_num::cleanup(); + } }; class Item_sum_std; @@ -485,6 +526,11 @@ public: void update_field(); void fix_length_and_dec() { decimals=0; max_length=21; unsigned_flag=1; maybe_null=null_value=0; } + void cleanup() + { + clear(); + Item_sum_int::cleanup(); + } }; From b2c6ff7ab131e5ede05418cedb35ddf7e5333f4d Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Tue, 10 Oct 2006 17:51:12 +0400 Subject: [PATCH 08/23] Fix after manial merge. --- mysql-test/r/view.result | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 92123d16b3f..1b2f20702b2 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -2736,20 +2736,6 @@ m e 1 b DROP VIEW v1; DROP TABLE t1,t2; -DROP FUNCTION IF EXISTS f1; -DROP VIEW IF EXISTS v1; -DROP TABLE IF EXISTS t1; -CREATE TABLE t1 (i INT); -INSERT INTO t1 VALUES (1); -CREATE VIEW v1 AS SELECT MAX(i) FROM t1; -CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW -SET NEW.i = (SELECT * FROM v1) + 1; -INSERT INTO t1 VALUES (1); -CREATE FUNCTION f1() RETURNS INT RETURN (SELECT * FROM v1); -UPDATE t1 SET i= f1(); -DROP FUNCTION f1; -DROP VIEW v1; -DROP TABLE t1; CREATE TABLE t1 (a INT NOT NULL, b INT NULL DEFAULT NULL); CREATE VIEW v1 AS SELECT a, b FROM t1; INSERT INTO v1 (b) VALUES (2); @@ -2970,4 +2956,18 @@ View Create View v1 CREATE ALGORITHM=MERGE DEFINER=`root`@`localhost` SQL SECURITY DEFINER VIEW `v1` AS select `t1`.`pk` AS `pk` from (`t1` join `t2` on(((`t2`.`fk` = `t1`.`pk`) and (`t2`.`ver` = (select max(`t`.`ver`) AS `MAX(t.ver)` from `t2` `t` where (`t`.`org` = `t2`.`org`)))))) DROP VIEW v1; DROP TABLE t1, t2; +DROP FUNCTION IF EXISTS f1; +DROP VIEW IF EXISTS v1; +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (i INT); +INSERT INTO t1 VALUES (1); +CREATE VIEW v1 AS SELECT MAX(i) FROM t1; +CREATE TRIGGER t1_bi BEFORE INSERT ON t1 FOR EACH ROW +SET NEW.i = (SELECT * FROM v1) + 1; +INSERT INTO t1 VALUES (1); +CREATE FUNCTION f1() RETURNS INT RETURN (SELECT * FROM v1); +UPDATE t1 SET i= f1(); +DROP FUNCTION f1; +DROP VIEW v1; +DROP TABLE t1; End of 5.0 tests. From 591c06d4b771124cc2cf453fbf51d5d99a4ad95e Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Thu, 12 Oct 2006 18:02:57 +0400 Subject: [PATCH 09/23] BUG#20953: create proc with a create view that uses local vars/params should fail to create The problem was that this type of errors was checked during view creation, which doesn't happen when CREATE VIEW is a statement of a created stored routine. The solution is to perform the checks at parse time. The idea of the fix is that the parser checks if a construction just parsed is allowed in current circumstances by testing certain flags, and this flags are reset for VIEWs. The side effect of this change is that if the user already have such bogus routines, it will now get a error when trying to do SHOW CREATE PROCEDURE proc; (and some other) and when trying to execute such routine he will get ERROR 1457 (HY000): Failed to load routine test.p5. The table mysql.proc is missing, corrupt, or contains bad data (internal code -6) However there should be very few such users (if any), and they may (and should) drop these bogus routines. --- mysql-test/r/sp-error.result | 24 ++++++ mysql-test/r/view.result | 10 +-- mysql-test/t/sp-error.test | 42 +++++++++++ mysql-test/t/view.test | 20 ++--- sql/sql_lex.cc | 1 - sql/sql_lex.h | 21 +++++- sql/sql_view.cc | 22 +----- sql/sql_yacc.yy | 139 ++++++++++++++++++++++++++--------- 8 files changed, 204 insertions(+), 75 deletions(-) diff --git a/mysql-test/r/sp-error.result b/mysql-test/r/sp-error.result index 924963017eb..a967f4de10c 100644 --- a/mysql-test/r/sp-error.result +++ b/mysql-test/r/sp-error.result @@ -1174,3 +1174,27 @@ drop procedure bug15091; drop function if exists bug16896; create aggregate function bug16896() returns int return 1; ERROR 42000: AGGREGATE is not supported for stored functions +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (i INT); +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO @a; +ERROR HY000: View's SELECT contains a 'INTO' clause +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO DUMPFILE "file"; +ERROR HY000: View's SELECT contains a 'INTO' clause +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO OUTFILE "file"; +ERROR HY000: View's SELECT contains a 'INTO' clause +CREATE PROCEDURE bug20953() +CREATE VIEW v AS SELECT i FROM t1 PROCEDURE ANALYSE(); +ERROR HY000: View's SELECT contains a 'PROCEDURE' clause +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 FROM (SELECT 1) AS d1; +ERROR HY000: View's SELECT contains a subquery in the FROM clause +CREATE PROCEDURE bug20953(i INT) CREATE VIEW v AS SELECT i; +ERROR HY000: View's SELECT contains a variable or parameter +CREATE PROCEDURE bug20953() +BEGIN +DECLARE i INT; +CREATE VIEW v AS SELECT i; +END | +ERROR HY000: View's SELECT contains a variable or parameter +PREPARE stmt FROM "CREATE VIEW v AS SELECT ?"; +ERROR HY000: View's SELECT contains a variable or parameter +DROP TABLE t1; diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 72cffb9531c..99e71dd30bd 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -12,6 +12,9 @@ create table t1 (a int, b int); insert into t1 values (1,2), (1,3), (2,4), (2,5), (3,10); create view v1 (c,d) as select a,b+@@global.max_user_connections from t1; ERROR HY000: View's SELECT contains a variable or parameter +create view v1 (c,d) as select a,b from t1 +where a = @@global.max_user_connections; +ERROR HY000: View's SELECT contains a variable or parameter create view v1 (c) as select b+1 from t1; select c from v1; c @@ -596,11 +599,6 @@ ERROR HY000: View 'test.v1' references invalid table(s) or column(s) or function drop view v1; create view v1 (a,a) as select 'a','a'; ERROR 42S21: Duplicate column name 'a' -drop procedure if exists p1; -create procedure p1 () begin declare v int; create view v1 as select v; end;// -call p1(); -ERROR HY000: View's SELECT contains a variable or parameter -drop procedure p1; create table t1 (col1 int,col2 char(22)); insert into t1 values(5,'Hello, world of views'); create view v1 as select * from t1; @@ -886,6 +884,8 @@ ERROR HY000: View's SELECT contains a 'INTO' clause create table t1 (a int); create view v1 as select a from t1 procedure analyse(); ERROR HY000: View's SELECT contains a 'PROCEDURE' clause +create view v1 as select 1 from (select 1) as d1; +ERROR HY000: View's SELECT contains a subquery in the FROM clause drop table t1; create table t1 (s1 int, primary key (s1)); create view v1 as select * from t1; diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test index a4ab5d98922..2340bfdc899 100644 --- a/mysql-test/t/sp-error.test +++ b/mysql-test/t/sp-error.test @@ -1706,6 +1706,48 @@ drop function if exists bug16896; create aggregate function bug16896() returns int return 1; + +# +# BUG#20953: create proc with a create view that uses local +# vars/params should fail to create +# +# See test case for what syntax is forbidden in a view. +# +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (i INT); + +# We do not have to drop this procedure and view because they won't be +# created. +--error ER_VIEW_SELECT_CLAUSE +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO @a; +--error ER_VIEW_SELECT_CLAUSE +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO DUMPFILE "file"; +--error ER_VIEW_SELECT_CLAUSE +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 INTO OUTFILE "file"; +--error ER_VIEW_SELECT_CLAUSE +CREATE PROCEDURE bug20953() + CREATE VIEW v AS SELECT i FROM t1 PROCEDURE ANALYSE(); +--error ER_VIEW_SELECT_DERIVED +CREATE PROCEDURE bug20953() CREATE VIEW v AS SELECT 1 FROM (SELECT 1) AS d1; +--error ER_VIEW_SELECT_VARIABLE +CREATE PROCEDURE bug20953(i INT) CREATE VIEW v AS SELECT i; +delimiter |; +--error ER_VIEW_SELECT_VARIABLE +CREATE PROCEDURE bug20953() +BEGIN + DECLARE i INT; + CREATE VIEW v AS SELECT i; +END | +delimiter ;| +--error ER_VIEW_SELECT_VARIABLE +PREPARE stmt FROM "CREATE VIEW v AS SELECT ?"; + +DROP TABLE t1; + + # # BUG#NNNN: New bug synopsis # diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index a1c1e9b2ad1..48555960924 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -23,8 +23,11 @@ create table t1 (a int, b int); insert into t1 values (1,2), (1,3), (2,4), (2,5), (3,10); # view with variable --- error 1351 +-- error ER_VIEW_SELECT_VARIABLE create view v1 (c,d) as select a,b+@@global.max_user_connections from t1; +-- error ER_VIEW_SELECT_VARIABLE +create view v1 (c,d) as select a,b from t1 + where a = @@global.max_user_connections; # simple view create view v1 (c) as select b+1 from t1; @@ -486,19 +489,6 @@ drop view v1; -- error 1060 create view v1 (a,a) as select 'a','a'; -# -# SP variables inside view test -# ---disable_warnings -drop procedure if exists p1; ---enable_warnings -delimiter //; -create procedure p1 () begin declare v int; create view v1 as select v; end;// -delimiter ;// --- error 1351 -call p1(); -drop procedure p1; - # # updatablity should be transitive # @@ -820,6 +810,8 @@ create view v1 as select 5 into outfile 'ttt'; create table t1 (a int); -- error 1350 create view v1 as select a from t1 procedure analyse(); +-- error ER_VIEW_SELECT_DERIVED +create view v1 as select 1 from (select 1) as d1; drop table t1; # diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index efbf29cf207..6f3b785d87e 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -150,7 +150,6 @@ void lex_start(THD *thd, uchar *buf,uint length) lex->safe_to_cache_query= 1; lex->time_zone_tables_used= 0; lex->leaf_tables_insert= 0; - lex->variables_used= 0; lex->empty_field_list_on_rset= 0; lex->select_lex.select_number= 1; lex->next_state=MY_LEX_START; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index e5b087fc72a..d2669fb4ec3 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -801,6 +801,25 @@ public: }; +/* + st_parsing_options contains the flags for constructions that are + allowed in the current statement. +*/ + +struct st_parsing_options +{ + bool allows_variable; + bool allows_select_into; + bool allows_select_procedure; + bool allows_derived; + + st_parsing_options() + : allows_variable(TRUE), allows_select_into(TRUE), + allows_select_procedure(TRUE), allows_derived(TRUE) + {} +}; + + /* The state of the lex parsing. This is saved in the THD struct */ typedef struct st_lex : public Query_tables_list @@ -944,7 +963,7 @@ typedef struct st_lex : public Query_tables_list bool stmt_prepare_mode; bool safe_to_cache_query; bool subqueries, ignore; - bool variables_used; + st_parsing_options parsing_options; ALTER_INFO alter_info; /* Prepared statements SQL syntax:*/ LEX_STRING prepared_stmt_name; /* Statement name (in all queries) */ diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 1561ade78af..a53e6bcf9a5 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -186,25 +186,9 @@ bool mysql_create_view(THD *thd, bool res= FALSE; DBUG_ENTER("mysql_create_view"); - if (lex->proc_list.first || - lex->result) - { - my_error(ER_VIEW_SELECT_CLAUSE, MYF(0), (lex->result ? - "INTO" : - "PROCEDURE")); - res= TRUE; - goto err; - } - if (lex->derived_tables || - lex->variables_used || lex->param_list.elements) - { - int err= (lex->derived_tables ? - ER_VIEW_SELECT_DERIVED : - ER_VIEW_SELECT_VARIABLE); - my_message(err, ER(err), MYF(0)); - res= TRUE; - goto err; - } + /* This is ensured in the parser. */ + DBUG_ASSERT(!lex->proc_list.first && !lex->result && + !lex->param_list.elements && !lex->derived_tables); if (mode != VIEW_CREATE_NEW) sp_cache_invalidate(); diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy index deac9cb5c40..523a359e3dd 100644 --- a/sql/sql_yacc.yy +++ b/sql/sql_yacc.yy @@ -721,7 +721,7 @@ bool my_yyoverflow(short **a, YYSTYPE **b, ulong *yystacksize); %type literal text_literal insert_ident order_ident simple_ident select_item2 expr opt_expr opt_else sum_expr in_sum_expr - bool_term bool_factor bool_test bool_pri + variable variable_aux bool_term bool_factor bool_test bool_pri predicate bit_expr bit_term bit_factor value_expr term factor table_wild simple_expr udf_expr expr_or_default set_expr_or_default interval_expr @@ -4281,32 +4281,7 @@ simple_expr: } | literal | param_marker - | '@' ident_or_text SET_VAR expr - { - $$= new Item_func_set_user_var($2,$4); - LEX *lex= Lex; - lex->uncacheable(UNCACHEABLE_RAND); - lex->variables_used= 1; - } - | '@' ident_or_text - { - $$= new Item_func_get_user_var($2); - LEX *lex= Lex; - lex->uncacheable(UNCACHEABLE_RAND); - lex->variables_used= 1; - } - | '@' '@' opt_var_ident_type ident_or_text opt_component - { - - if ($4.str && $5.str && check_reserved_words(&$4)) - { - yyerror(ER(ER_SYNTAX_ERROR)); - YYABORT; - } - if (!($$= get_system_var(YYTHD, $3, $4, $5))) - YYABORT; - Lex->variables_used= 1; - } + | variable | sum_expr | simple_expr OR_OR_SYM simple_expr { $$= new Item_func_concat($1, $3); } @@ -5006,6 +4981,46 @@ sum_expr: $5->empty(); }; +variable: + '@' + { + if (! Lex->parsing_options.allows_variable) + { + my_error(ER_VIEW_SELECT_VARIABLE, MYF(0)); + YYABORT; + } + } + variable_aux + { + $$= $3; + } + ; + +variable_aux: + ident_or_text SET_VAR expr + { + $$= new Item_func_set_user_var($1, $3); + LEX *lex= Lex; + lex->uncacheable(UNCACHEABLE_RAND); + } + | ident_or_text + { + $$= new Item_func_get_user_var($1); + LEX *lex= Lex; + lex->uncacheable(UNCACHEABLE_RAND); + } + | '@' opt_var_ident_type ident_or_text opt_component + { + if ($3.str && $4.str && check_reserved_words(&$3)) + { + yyerror(ER(ER_SYNTAX_ERROR)); + YYABORT; + } + if (!($$= get_system_var(YYTHD, $2, $3, $4))) + YYABORT; + } + ; + opt_distinct: /* empty */ { $$ = 0; } |DISTINCT { $$ = 1; }; @@ -5428,6 +5443,13 @@ select_derived_init: SELECT_SYM { LEX *lex= Lex; + + if (! lex->parsing_options.allows_derived) + { + my_error(ER_VIEW_SELECT_DERIVED, MYF(0)); + YYABORT; + } + SELECT_LEX *sel= lex->current_select; TABLE_LIST *embedding; if (!sel->embedding || sel->end_nested_join(lex->thd)) @@ -5787,6 +5809,13 @@ procedure_clause: | PROCEDURE ident /* Procedure name */ { LEX *lex=Lex; + + if (! lex->parsing_options.allows_select_procedure) + { + my_error(ER_VIEW_SELECT_CLAUSE, MYF(0), "PROCEDURE"); + YYABORT; + } + if (&lex->select_lex != lex->current_select) { my_error(ER_WRONG_USAGE, MYF(0), "PROCEDURE", "subquery"); @@ -5886,28 +5915,40 @@ select_var_ident: ; into: - INTO OUTFILE TEXT_STRING_filesystem + INTO + { + if (! Lex->parsing_options.allows_select_into) + { + my_error(ER_VIEW_SELECT_CLAUSE, MYF(0), "INTO"); + YYABORT; + } + } + into_destination + ; + +into_destination: + OUTFILE TEXT_STRING_filesystem { LEX *lex= Lex; lex->uncacheable(UNCACHEABLE_SIDEEFFECT); - if (!(lex->exchange= new sql_exchange($3.str, 0)) || + if (!(lex->exchange= new sql_exchange($2.str, 0)) || !(lex->result= new select_export(lex->exchange))) YYABORT; } opt_field_term opt_line_term - | INTO DUMPFILE TEXT_STRING_filesystem + | DUMPFILE TEXT_STRING_filesystem { LEX *lex=Lex; if (!lex->describe) { lex->uncacheable(UNCACHEABLE_SIDEEFFECT); - if (!(lex->exchange= new sql_exchange($3.str,1))) + if (!(lex->exchange= new sql_exchange($2.str,1))) YYABORT; if (!(lex->result= new select_dump(lex->exchange))) YYABORT; } } - | INTO select_var_list_init + | select_var_list_init { Lex->uncacheable(UNCACHEABLE_SIDEEFFECT); } @@ -7067,8 +7108,13 @@ param_marker: { THD *thd=YYTHD; LEX *lex= thd->lex; - Item_param *item= new Item_param((uint) (lex->tok_start - - (uchar *) thd->query)); + Item_param *item; + if (! lex->parsing_options.allows_variable) + { + my_error(ER_VIEW_SELECT_VARIABLE, MYF(0)); + YYABORT; + } + item= new Item_param((uint) (lex->tok_start - (uchar *) thd->query)); if (!($$= item) || lex->param_list.push_back(item)) { my_message(ER_OUT_OF_RESOURCES, ER(ER_OUT_OF_RESOURCES), MYF(0)); @@ -7188,6 +7234,12 @@ simple_ident: if (spc && (spv = spc->find_variable(&$1))) { /* We're compiling a stored procedure and found a variable */ + if (! lex->parsing_options.allows_variable) + { + my_error(ER_VIEW_SELECT_VARIABLE, MYF(0)); + YYABORT; + } + Item_splocal *splocal; splocal= new Item_splocal($1, spv->offset, spv->type, lex->tok_start_prev - @@ -7197,7 +7249,6 @@ simple_ident: splocal->m_sp= lex->sphead; #endif $$ = (Item*) splocal; - lex->variables_used= 1; lex->safe_to_cache_query=0; } else @@ -9038,6 +9089,24 @@ view_list: ; view_select: + { + LEX *lex= Lex; + lex->parsing_options.allows_variable= FALSE; + lex->parsing_options.allows_select_into= FALSE; + lex->parsing_options.allows_select_procedure= FALSE; + lex->parsing_options.allows_derived= FALSE; + } + view_select_aux + { + LEX *lex= Lex; + lex->parsing_options.allows_variable= TRUE; + lex->parsing_options.allows_select_into= TRUE; + lex->parsing_options.allows_select_procedure= TRUE; + lex->parsing_options.allows_derived= TRUE; + } + ; + +view_select_aux: SELECT_SYM remember_name select_init2 { THD *thd=YYTHD; From e7c31e81301546b653e05ab63495168e99b0b624 Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Thu, 12 Oct 2006 19:36:43 +0400 Subject: [PATCH 10/23] Fix after manual merge. --- mysql-test/t/sp-error.test | 43 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/mysql-test/t/sp-error.test b/mysql-test/t/sp-error.test index 058c45f7f2f..7dbf64e35c1 100644 --- a/mysql-test/t/sp-error.test +++ b/mysql-test/t/sp-error.test @@ -1745,6 +1745,27 @@ drop function if exists bug16896; create aggregate function bug16896() returns int return 1; +# +# BUG#14702: misleading error message when syntax error in CREATE +# PROCEDURE +# +# Misleading error message was given when IF NOT EXISTS was used in +# CREATE PROCEDURE. +# +--disable_warnings +DROP PROCEDURE IF EXISTS bug14702; +--enable_warnings + +--error ER_PARSE_ERROR +CREATE IF NOT EXISTS PROCEDURE bug14702() +BEGIN +END; + +--error ER_PARSE_ERROR +CREATE PROCEDURE IF NOT EXISTS bug14702() +BEGIN +END; + # # BUG#20953: create proc with a create view that uses local @@ -1787,28 +1808,6 @@ PREPARE stmt FROM "CREATE VIEW v AS SELECT ?"; DROP TABLE t1; -# -# BUG#14702: misleading error message when syntax error in CREATE -# PROCEDURE -# -# Misleading error message was given when IF NOT EXISTS was used in -# CREATE PROCEDURE. -# ---disable_warnings -DROP PROCEDURE IF EXISTS bug14702; ---enable_warnings - ---error ER_PARSE_ERROR -CREATE IF NOT EXISTS PROCEDURE bug14702() -BEGIN -END; - ---error ER_PARSE_ERROR -CREATE PROCEDURE IF NOT EXISTS bug14702() -BEGIN -END; - - # # BUG#NNNN: New bug synopsis # From 9fd2c861038facb19b9dc8df3c0c645915a6f407 Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Wed, 18 Oct 2006 15:20:34 +0400 Subject: [PATCH 11/23] Fix for valgrind warning introduced by the fix for bug#21354: (COUNT(*) = 1) not working in SELECT inside prepared statement. Note: the warning was introduced in 5.0 and 5.1, 4.1 is OK with the original fix. The problem was that in 5.0 and 5.1 clear() for group functions may access hybrid_type member, and this member is initialized in fix_fields(). So we should not call clear() from item cleanup() methods, as cleanup() may be called for unfixed items. --- sql/item_sum.cc | 2 +- sql/item_sum.h | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/sql/item_sum.cc b/sql/item_sum.cc index 513fa46ecb3..397293bc131 100644 --- a/sql/item_sum.cc +++ b/sql/item_sum.cc @@ -1055,7 +1055,7 @@ longlong Item_sum_count::val_int() void Item_sum_count::cleanup() { DBUG_ENTER("Item_sum_count::cleanup"); - clear(); + count= 0; Item_sum_int::cleanup(); used_table_cache= ~(table_map) 0; DBUG_VOID_RETURN; diff --git a/sql/item_sum.h b/sql/item_sum.h index 4a0930184c7..fe7edd76ecf 100644 --- a/sql/item_sum.h +++ b/sql/item_sum.h @@ -643,8 +643,8 @@ public: Field *create_tmp_field(bool group, TABLE *table, uint convert_blob_length); void cleanup() { - clear(); - Item_sum_num::cleanup(); + count= 0; + Item_sum_sum::cleanup(); } }; @@ -727,7 +727,8 @@ public: enum Item_result result_type () const { return REAL_RESULT; } void cleanup() { - clear(); + cur_dec= 0; + count= 0; Item_sum_num::cleanup(); } }; @@ -862,7 +863,7 @@ public: { decimals= 0; max_length=21; unsigned_flag= 1; maybe_null= null_value= 0; } void cleanup() { - clear(); + bits= reset_bits; Item_sum_int::cleanup(); } }; From 00b2fc6affe6ffcef7f50c77b1fb2cbc3116560b Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Thu, 19 Oct 2006 14:43:52 +0400 Subject: [PATCH 12/23] BUG#21856: Prepared Statements: crash if bad create When statement to be prepared contained CREATE PROCEDURE, CREATE FUNCTION or CREATE TRIGGER statements with a syntax error in it, the preparation would fail with syntax error message, but the memory could be corrupted. The problem occurred because we switch memroot when parse stored routine or trigger definitions, and on parse error we restored the original memroot only after performing some memory operations. In more detail: - prepared statement would activate its own memory root to parse the definition of the stored procedure. - SP would reset this memory root with its own memory root to parse SP statements - a syntax error would happen - prepared statement would restore the original memory root - stored procedure would restore what it thinks was the original memory root, but actually was the statement memory root. That led to double free - in destruction of the statement and in a next call to mysql_parse(). The solution is to restore memroot right after the failed parsing. --- mysql-test/r/ps.result | 1 + mysql-test/t/ps.test | 20 ++++++++++++++++++++ sql/sql_parse.cc | 9 +++++++-- sql/sql_prepare.cc | 11 +++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/ps.result b/mysql-test/r/ps.result index 080187cfa7b..c895ef54e55 100644 --- a/mysql-test/r/ps.result +++ b/mysql-test/r/ps.result @@ -1311,4 +1311,5 @@ EXECUTE stmt USING @a; i j i i j DEALLOCATE PREPARE stmt; DROP TABLE IF EXISTS t1, t2, t3; +DROP PROCEDURE IF EXISTS p1; End of 5.0 tests. diff --git a/mysql-test/t/ps.test b/mysql-test/t/ps.test index 5b2e37ecc94..fd74c52f3d5 100644 --- a/mysql-test/t/ps.test +++ b/mysql-test/t/ps.test @@ -1358,4 +1358,24 @@ DEALLOCATE PREPARE stmt; DROP TABLE IF EXISTS t1, t2, t3; +# +# BUG#21856: Prepared Statments: crash if bad create +# +--disable_warnings +DROP PROCEDURE IF EXISTS p1; +--enable_warnings + +let $iterations= 100; +--disable_query_log +--disable_result_log +while ($iterations > 0) +{ + --error ER_PARSE_ERROR + PREPARE stmt FROM "CREATE PROCEDURE p1()"; + dec $iterations; +} +--enable_query_log +--enable_result_log + + --echo End of 5.0 tests. diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 2acbf18f1e6..f75bc5f073c 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -5852,14 +5852,19 @@ void mysql_parse(THD *thd, char *inBuf, uint length) DBUG_ASSERT(thd->net.report_error); DBUG_PRINT("info",("Command aborted. Fatal_error: %d", thd->is_fatal_error)); - query_cache_abort(&thd->net); - lex->unit.cleanup(); + + /* + The first thing we do after parse error is freeing sp_head to + ensure that we have restored original memroot. + */ if (thd->lex->sphead) { /* Clean up after failed stored procedure/function */ delete thd->lex->sphead; thd->lex->sphead= NULL; } + query_cache_abort(&thd->net); + lex->unit.cleanup(); } thd->proc_info="freeing items"; thd->end_statement(); diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 32f0ca6859d..fcbf495463a 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -2773,6 +2773,16 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) error= MYSQLparse((void *)thd) || thd->is_fatal_error || thd->net.report_error || init_param_array(this); + + /* + The first thing we do after parse error is freeing sp_head to + ensure that we have restored original memroot. + */ + if (error && lex->sphead) + { + delete lex->sphead; + lex->sphead= NULL; + } /* While doing context analysis of the query (in check_prepared_statement) we allocate a lot of additional memory: for open tables, JOINs, derived @@ -2798,6 +2808,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) if (error == 0) error= check_prepared_statement(this, name.str != 0); + /* Free sp_head if check_prepared_statement() failed. */ if (error && lex->sphead) { delete lex->sphead; From ea0998cacafce4e5efca6de8ccfd135b10adc5b3 Mon Sep 17 00:00:00 2001 From: "malff/marcsql@weblab.(none)" <> Date: Thu, 19 Oct 2006 11:39:51 -0700 Subject: [PATCH 13/23] Bug#20028 (Function with select return no data) This patch reverts a change introduced by Bug 6951, which incorrectly set thd->abort_on_warning for stored procedures. As per internal discussions about the SQL_MODE=TRADITIONAL, the correct behavior is to *not* abort on warnings even inside an INSERT/UPDATE trigger. Tests for Stored Procedures, Stored Functions, Triggers involving SQL_MODE have been included or revised, to reflect the intended behavior. (reposting approved patch, to work around source control issues, no review needed) --- mysql-test/include/sp-vars.inc | 9 +++ mysql-test/r/sp-vars.result | 23 ++++++ mysql-test/r/sp.result | 129 ++++++++++++++++++++++++++++++++ mysql-test/r/trigger.result | 64 +++++++++++++++- mysql-test/t/sp-vars.test | 10 +++ mysql-test/t/sp.test | 131 +++++++++++++++++++++++++++++++++ mysql-test/t/trigger.test | 63 +++++++++++++++- sql/sp_head.cc | 3 +- 8 files changed, 424 insertions(+), 8 deletions(-) diff --git a/mysql-test/include/sp-vars.inc b/mysql-test/include/sp-vars.inc index 3e02c9d1709..4bac883ee0e 100644 --- a/mysql-test/include/sp-vars.inc +++ b/mysql-test/include/sp-vars.inc @@ -119,4 +119,13 @@ END| --------------------------------------------------------------------------- +CREATE FUNCTION sp_vars_div_zero() RETURNS INTEGER +BEGIN + DECLARE div_zero INTEGER; + SELECT 1/0 INTO div_zero; + RETURN div_zero; +END| + +--------------------------------------------------------------------------- + delimiter ;| diff --git a/mysql-test/r/sp-vars.result b/mysql-test/r/sp-vars.result index 14040f8420e..f362187cd14 100644 --- a/mysql-test/r/sp-vars.result +++ b/mysql-test/r/sp-vars.result @@ -4,6 +4,7 @@ DROP FUNCTION IF EXISTS sp_vars_check_ret1; DROP FUNCTION IF EXISTS sp_vars_check_ret2; DROP FUNCTION IF EXISTS sp_vars_check_ret3; DROP FUNCTION IF EXISTS sp_vars_check_ret4; +DROP FUNCTION IF EXISTS sp_vars_div_zero; SET @@sql_mode = 'ansi'; CREATE PROCEDURE sp_vars_check_dflt() BEGIN @@ -88,6 +89,12 @@ CREATE FUNCTION sp_vars_check_ret4() RETURNS DECIMAL(64, 2) BEGIN RETURN 12 * 10 + 34 + 0.1234; END| +CREATE FUNCTION sp_vars_div_zero() RETURNS INTEGER +BEGIN +DECLARE div_zero INTEGER; +SELECT 1/0 INTO div_zero; +RETURN div_zero; +END| --------------------------------------------------------------- Calling the routines, created in ANSI mode. @@ -172,6 +179,9 @@ sp_vars_check_ret4() 154.12 Warnings: Note 1265 Data truncated for column 'sp_vars_check_ret4()' at row 1 +SELECT sp_vars_div_zero(); +sp_vars_div_zero() +NULL SET @@sql_mode = 'traditional'; --------------------------------------------------------------- @@ -257,12 +267,16 @@ sp_vars_check_ret4() 154.12 Warnings: Note 1265 Data truncated for column 'sp_vars_check_ret4()' at row 1 +SELECT sp_vars_div_zero(); +sp_vars_div_zero() +NULL DROP PROCEDURE sp_vars_check_dflt; DROP PROCEDURE sp_vars_check_assignment; DROP FUNCTION sp_vars_check_ret1; DROP FUNCTION sp_vars_check_ret2; DROP FUNCTION sp_vars_check_ret3; DROP FUNCTION sp_vars_check_ret4; +DROP FUNCTION sp_vars_div_zero; CREATE PROCEDURE sp_vars_check_dflt() BEGIN DECLARE v1 TINYINT DEFAULT 1e200; @@ -346,6 +360,12 @@ CREATE FUNCTION sp_vars_check_ret4() RETURNS DECIMAL(64, 2) BEGIN RETURN 12 * 10 + 34 + 0.1234; END| +CREATE FUNCTION sp_vars_div_zero() RETURNS INTEGER +BEGIN +DECLARE div_zero INTEGER; +SELECT 1/0 INTO div_zero; +RETURN div_zero; +END| --------------------------------------------------------------- Calling the routines, created in TRADITIONAL mode. @@ -366,6 +386,8 @@ sp_vars_check_ret4() 154.12 Warnings: Note 1265 Data truncated for column 'sp_vars_check_ret4()' at row 1 +SELECT sp_vars_div_zero(); +ERROR 22012: Division by 0 SET @@sql_mode = 'ansi'; DROP PROCEDURE sp_vars_check_dflt; DROP PROCEDURE sp_vars_check_assignment; @@ -373,6 +395,7 @@ DROP FUNCTION sp_vars_check_ret1; DROP FUNCTION sp_vars_check_ret2; DROP FUNCTION sp_vars_check_ret3; DROP FUNCTION sp_vars_check_ret4; +DROP FUNCTION sp_vars_div_zero; --------------------------------------------------------------- BIT data type tests diff --git a/mysql-test/r/sp.result b/mysql-test/r/sp.result index 6e8a609d669..1bb4b3a405b 100644 --- a/mysql-test/r/sp.result +++ b/mysql-test/r/sp.result @@ -5470,6 +5470,135 @@ CAD CHF DROP FUNCTION bug21493| DROP TABLE t3,t4| +drop function if exists func_20028_a| +drop function if exists func_20028_b| +drop function if exists func_20028_c| +drop procedure if exists proc_20028_a| +drop procedure if exists proc_20028_b| +drop procedure if exists proc_20028_c| +drop table if exists table_20028| +create table table_20028 (i int)| +SET @save_sql_mode=@@sql_mode| +SET sql_mode=''| +create function func_20028_a() returns integer +begin +declare temp integer; +select i into temp from table_20028 limit 1; +return ifnull(temp, 0); +end| +create function func_20028_b() returns integer +begin +return func_20028_a(); +end| +create function func_20028_c() returns integer +begin +declare div_zero integer; +set SQL_MODE='TRADITIONAL'; +select 1/0 into div_zero; +return div_zero; +end| +create procedure proc_20028_a() +begin +declare temp integer; +select i into temp from table_20028 limit 1; +end| +create procedure proc_20028_b() +begin +call proc_20028_a(); +end| +create procedure proc_20028_c() +begin +declare div_zero integer; +set SQL_MODE='TRADITIONAL'; +select 1/0 into div_zero; +end| +select func_20028_a()| +func_20028_a() +0 +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +select func_20028_b()| +func_20028_b() +0 +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +select func_20028_c()| +ERROR 22012: Division by 0 +call proc_20028_a()| +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +call proc_20028_b()| +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +call proc_20028_c()| +ERROR 22012: Division by 0 +SET sql_mode='TRADITIONAL'| +drop function func_20028_a| +drop function func_20028_b| +drop function func_20028_c| +drop procedure proc_20028_a| +drop procedure proc_20028_b| +drop procedure proc_20028_c| +create function func_20028_a() returns integer +begin +declare temp integer; +select i into temp from table_20028 limit 1; +return ifnull(temp, 0); +end| +create function func_20028_b() returns integer +begin +return func_20028_a(); +end| +create function func_20028_c() returns integer +begin +declare div_zero integer; +set SQL_MODE=''; +select 1/0 into div_zero; +return div_zero; +end| +create procedure proc_20028_a() +begin +declare temp integer; +select i into temp from table_20028 limit 1; +end| +create procedure proc_20028_b() +begin +call proc_20028_a(); +end| +create procedure proc_20028_c() +begin +declare div_zero integer; +set SQL_MODE=''; +select 1/0 into div_zero; +end| +select func_20028_a()| +func_20028_a() +0 +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +select func_20028_b()| +func_20028_b() +0 +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +select func_20028_c()| +func_20028_c() +NULL +call proc_20028_a()| +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +call proc_20028_b()| +Warnings: +Warning 1329 No data - zero rows fetched, selected, or processed +call proc_20028_c()| +SET @@sql_mode=@save_sql_mode| +drop function func_20028_a| +drop function func_20028_b| +drop function func_20028_c| +drop procedure proc_20028_a| +drop procedure proc_20028_b| +drop procedure proc_20028_c| +drop table table_20028| drop procedure if exists proc_21462_a| drop procedure if exists proc_21462_b| create procedure proc_21462_a() diff --git a/mysql-test/r/trigger.result b/mysql-test/r/trigger.result index 0b0bf1086db..5d643057666 100644 --- a/mysql-test/r/trigger.result +++ b/mysql-test/r/trigger.result @@ -1073,10 +1073,11 @@ SELECT @x; NULL SET @x=2; UPDATE t1 SET i1 = @x; -ERROR 22012: Division by 0 +Warnings: +Error 1365 Division by 0 SELECT @x; @x -2 +NULL SET SQL_MODE=''; SET @x=3; INSERT INTO t1 VALUES (@x); @@ -1085,10 +1086,12 @@ SELECT @x; NULL SET @x=4; UPDATE t1 SET i1 = @x; -ERROR 22012: Division by 0 +Warnings: +Error 1365 Division by 0 +Error 1365 Division by 0 SELECT @x; @x -4 +NULL SET @@sql_mode=@save_sql_mode; DROP TRIGGER t1_ai; DROP TRIGGER t1_au; @@ -1174,6 +1177,59 @@ ERROR HY000: String '1234567890abcdefghij1234567890abcdefghij1234567890abcdefghi DROP TABLE t1; DROP TABLE t2; drop table if exists t1; +drop table if exists t2; +drop table if exists t3; +drop table if exists t4; +SET @save_sql_mode=@@sql_mode; +SET sql_mode='TRADITIONAL'| +create table t1 (id int(10) not null primary key, v int(10) )| +create table t2 (id int(10) not null primary key, v int(10) )| +create table t3 (id int(10) not null primary key, v int(10) )| +create table t4 (c int)| +create trigger t4_bi before insert on t4 for each row set @t4_bi_called:=1| +create trigger t4_bu before update on t4 for each row set @t4_bu_called:=1| +insert into t1 values(10, 10)| +set @a:=1/0| +Warnings: +Error 1365 Division by 0 +select 1/0 from t1| +1/0 +NULL +Warnings: +Error 1365 Division by 0 +create trigger t1_bi before insert on t1 for each row set @a:=1/0| +insert into t1 values(20, 20)| +Warnings: +Error 1365 Division by 0 +drop trigger t1_bi| +create trigger t1_bi before insert on t1 for each row +begin +insert into t2 values (new.id, new.v); +update t2 set v=v+1 where id= new.id; +replace t3 values (new.id, 0); +update t2, t3 set t2.v=new.v, t3.v=new.v where t2.id=t3.id; +create temporary table t5 select * from t1; +delete from t5; +insert into t5 select * from t1; +insert into t4 values (0); +set @check= (select count(*) from t5); +update t4 set c= @check; +drop temporary table t5; +set @a:=1/0; +end| +set @check=0, @t4_bi_called=0, @t4_bu_called=0| +insert into t1 values(30, 30)| +Warnings: +Error 1365 Division by 0 +select @check, @t4_bi_called, @t4_bu_called| +@check @t4_bi_called @t4_bu_called +2 1 1 +SET @@sql_mode=@save_sql_mode; +drop table t1; +drop table t2; +drop table t3; +drop table t4; +drop table if exists t1; create table t1 (i int, j int key); insert into t1 values (1,1), (2,2), (3,3); create trigger t1_bu before update on t1 for each row diff --git a/mysql-test/t/sp-vars.test b/mysql-test/t/sp-vars.test index 48dbd4de7aa..7cf92dc5d0d 100644 --- a/mysql-test/t/sp-vars.test +++ b/mysql-test/t/sp-vars.test @@ -15,6 +15,7 @@ DROP FUNCTION IF EXISTS sp_vars_check_ret1; DROP FUNCTION IF EXISTS sp_vars_check_ret2; DROP FUNCTION IF EXISTS sp_vars_check_ret3; DROP FUNCTION IF EXISTS sp_vars_check_ret4; +DROP FUNCTION IF EXISTS sp_vars_div_zero; --enable_warnings @@ -49,6 +50,8 @@ SELECT sp_vars_check_ret3(); SELECT sp_vars_check_ret4(); +SELECT sp_vars_div_zero(); + # Check that changing sql_mode after creating a store procedure does not # matter. @@ -72,6 +75,8 @@ SELECT sp_vars_check_ret3(); SELECT sp_vars_check_ret4(); +SELECT sp_vars_div_zero(); + # Create the procedure in TRADITIONAL mode. Check that error will be thrown on # execution. @@ -81,6 +86,7 @@ DROP FUNCTION sp_vars_check_ret1; DROP FUNCTION sp_vars_check_ret2; DROP FUNCTION sp_vars_check_ret3; DROP FUNCTION sp_vars_check_ret4; +DROP FUNCTION sp_vars_div_zero; --source include/sp-vars.inc @@ -110,6 +116,9 @@ SELECT sp_vars_check_ret3(); SELECT sp_vars_check_ret4(); +--error ER_DIVISION_BY_ZERO +SELECT sp_vars_div_zero(); + SET @@sql_mode = 'ansi'; # @@ -122,6 +131,7 @@ DROP FUNCTION sp_vars_check_ret1; DROP FUNCTION sp_vars_check_ret2; DROP FUNCTION sp_vars_check_ret3; DROP FUNCTION sp_vars_check_ret4; +DROP FUNCTION sp_vars_div_zero; ########################################################################### # diff --git a/mysql-test/t/sp.test b/mysql-test/t/sp.test index ffbbf56d3ac..95444a04ce5 100644 --- a/mysql-test/t/sp.test +++ b/mysql-test/t/sp.test @@ -6420,6 +6420,137 @@ SELECT bug21493(Member_ID) FROM t3| DROP FUNCTION bug21493| DROP TABLE t3,t4| +# +# Bug#20028 Function with select return no data +# + +--disable_warnings +drop function if exists func_20028_a| +drop function if exists func_20028_b| +drop function if exists func_20028_c| +drop procedure if exists proc_20028_a| +drop procedure if exists proc_20028_b| +drop procedure if exists proc_20028_c| +drop table if exists table_20028| +--enable_warnings + +create table table_20028 (i int)| + +SET @save_sql_mode=@@sql_mode| + +SET sql_mode=''| + +create function func_20028_a() returns integer +begin + declare temp integer; + select i into temp from table_20028 limit 1; + return ifnull(temp, 0); +end| + +create function func_20028_b() returns integer +begin + return func_20028_a(); +end| + +create function func_20028_c() returns integer +begin + declare div_zero integer; + set SQL_MODE='TRADITIONAL'; + select 1/0 into div_zero; + return div_zero; +end| + +create procedure proc_20028_a() +begin + declare temp integer; + select i into temp from table_20028 limit 1; +end| + +create procedure proc_20028_b() +begin + call proc_20028_a(); +end| + +create procedure proc_20028_c() +begin + declare div_zero integer; + set SQL_MODE='TRADITIONAL'; + select 1/0 into div_zero; +end| + +select func_20028_a()| +select func_20028_b()| +--error ER_DIVISION_BY_ZERO +select func_20028_c()| +call proc_20028_a()| +call proc_20028_b()| +--error ER_DIVISION_BY_ZERO +call proc_20028_c()| + +SET sql_mode='TRADITIONAL'| + +drop function func_20028_a| +drop function func_20028_b| +drop function func_20028_c| +drop procedure proc_20028_a| +drop procedure proc_20028_b| +drop procedure proc_20028_c| + +create function func_20028_a() returns integer +begin + declare temp integer; + select i into temp from table_20028 limit 1; + return ifnull(temp, 0); +end| + +create function func_20028_b() returns integer +begin + return func_20028_a(); +end| + +create function func_20028_c() returns integer +begin + declare div_zero integer; + set SQL_MODE=''; + select 1/0 into div_zero; + return div_zero; +end| + +create procedure proc_20028_a() +begin + declare temp integer; + select i into temp from table_20028 limit 1; +end| + +create procedure proc_20028_b() +begin + call proc_20028_a(); +end| + +create procedure proc_20028_c() +begin + declare div_zero integer; + set SQL_MODE=''; + select 1/0 into div_zero; +end| + +select func_20028_a()| +select func_20028_b()| +select func_20028_c()| +call proc_20028_a()| +call proc_20028_b()| +call proc_20028_c()| + +SET @@sql_mode=@save_sql_mode| + +drop function func_20028_a| +drop function func_20028_b| +drop function func_20028_c| +drop procedure proc_20028_a| +drop procedure proc_20028_b| +drop procedure proc_20028_c| +drop table table_20028| + # # Bug#21462 Stored procedures with no arguments require parenthesis # diff --git a/mysql-test/t/trigger.test b/mysql-test/t/trigger.test index 6bd812d473e..92320648033 100644 --- a/mysql-test/t/trigger.test +++ b/mysql-test/t/trigger.test @@ -1274,7 +1274,6 @@ INSERT INTO t1 VALUES (@x); SELECT @x; SET @x=2; ---error ER_DIVISION_BY_ZERO UPDATE t1 SET i1 = @x; SELECT @x; @@ -1285,7 +1284,6 @@ INSERT INTO t1 VALUES (@x); SELECT @x; SET @x=4; ---error ER_DIVISION_BY_ZERO UPDATE t1 SET i1 = @x; SELECT @x; @@ -1420,6 +1418,67 @@ CREATE DEFINER=some_user_name@1234567890abcdefghij1234567890abcdefghij1234567890 DROP TABLE t1; DROP TABLE t2; +# +# Bug#20028 Function with select return no data +# + +--disable_warnings +drop table if exists t1; +drop table if exists t2; +drop table if exists t3; +drop table if exists t4; +--enable_warnings + +SET @save_sql_mode=@@sql_mode; + +delimiter |; +SET sql_mode='TRADITIONAL'| +create table t1 (id int(10) not null primary key, v int(10) )| +create table t2 (id int(10) not null primary key, v int(10) )| +create table t3 (id int(10) not null primary key, v int(10) )| +create table t4 (c int)| + +create trigger t4_bi before insert on t4 for each row set @t4_bi_called:=1| +create trigger t4_bu before update on t4 for each row set @t4_bu_called:=1| + +insert into t1 values(10, 10)| +set @a:=1/0| +select 1/0 from t1| + +create trigger t1_bi before insert on t1 for each row set @a:=1/0| + +insert into t1 values(20, 20)| + +drop trigger t1_bi| +create trigger t1_bi before insert on t1 for each row +begin + insert into t2 values (new.id, new.v); + update t2 set v=v+1 where id= new.id; + replace t3 values (new.id, 0); + update t2, t3 set t2.v=new.v, t3.v=new.v where t2.id=t3.id; + create temporary table t5 select * from t1; + delete from t5; + insert into t5 select * from t1; + insert into t4 values (0); + set @check= (select count(*) from t5); + update t4 set c= @check; + drop temporary table t5; + + set @a:=1/0; +end| + +set @check=0, @t4_bi_called=0, @t4_bu_called=0| +insert into t1 values(30, 30)| +select @check, @t4_bi_called, @t4_bu_called| + +delimiter ;| + +SET @@sql_mode=@save_sql_mode; + +drop table t1; +drop table t2; +drop table t3; +drop table t4; # # Bug#20670 "UPDATE using key and invoking trigger that modifies diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 9761de625be..a06bfe28a6f 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -991,8 +991,7 @@ sp_head::execute(THD *thd) save_sql_mode= thd->variables.sql_mode; thd->variables.sql_mode= m_sql_mode; save_abort_on_warning= thd->abort_on_warning; - thd->abort_on_warning= - (m_sql_mode & (MODE_STRICT_TRANS_TABLES | MODE_STRICT_ALL_TABLES)); + thd->abort_on_warning= 0; /* It is also more efficient to save/restore current thd->lex once when From 3fce634fc11344abf4ebac71c921683fcca36a9f Mon Sep 17 00:00:00 2001 From: "dlenev@mockturtle.local" <> Date: Fri, 20 Oct 2006 15:47:52 +0400 Subject: [PATCH 14/23] Fix for bug#15228 "'invalid access to non-static data member' warnings in sql_trigger.cc and sql_view.cc". According to the current version of C++ standard offsetof() macro can't be used for non-POD types. So warnings were emitted when we tried to use this macro for TABLE_LIST and Table_triggers_list classes. Note that despite of these warnings it was probably safe thing to do. This fix tries to circumvent this limitation by implementing custom version of offsetof() macro to be used with these classes. This hack should go away once we will refactor File_parser class. Alternative approaches such as disabling this warning for sql_trigger.cc/sql_view.cc or for the whole server were considered less explicit. Also I was unable to find a way to disable particular warning for particular _part_ of file in GCC. --- sql/parse_file.h | 16 ++++++++++++++++ sql/sql_trigger.cc | 8 ++++---- sql/sql_view.cc | 24 ++++++++++++------------ 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/sql/parse_file.h b/sql/parse_file.h index 33871588e11..5fb65b4c7ec 100644 --- a/sql/parse_file.h +++ b/sql/parse_file.h @@ -107,4 +107,20 @@ public: bool bad_format_errors); }; + +/* + Custom version of standard offsetof() macro which can be used to get + offsets of members in class for non-POD types (according to the current + version of C++ standard offsetof() macro can't be used in such cases and + attempt to do so causes warnings to be emitted, OTOH in many cases it is + still OK to assume that all instances of the class has the same offsets + for the same members). + + This is temporary solution which should be removed once File_parser class + and related routines are refactored. +*/ + +#define my_offsetof(TYPE, MEMBER) \ + ((size_t)((char *)&(((TYPE *)0x10)->MEMBER) - (char*)0x10)) + #endif /* _PARSE_FILE_H_ */ diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index 6bb50d602c3..c6d85934820 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -36,17 +36,17 @@ static File_option triggers_file_parameters[]= { { {(char *) STRING_WITH_LEN("triggers") }, - offsetof(class Table_triggers_list, definitions_list), + my_offsetof(class Table_triggers_list, definitions_list), FILE_OPTIONS_STRLIST }, { {(char *) STRING_WITH_LEN("sql_modes") }, - offsetof(class Table_triggers_list, definition_modes_list), + my_offsetof(class Table_triggers_list, definition_modes_list), FILE_OPTIONS_ULLLIST }, { {(char *) STRING_WITH_LEN("definers") }, - offsetof(class Table_triggers_list, definers_list), + my_offsetof(class Table_triggers_list, definers_list), FILE_OPTIONS_STRLIST }, { { 0, 0 }, 0, FILE_OPTIONS_STRING } @@ -55,7 +55,7 @@ static File_option triggers_file_parameters[]= File_option sql_modes_parameters= { {(char*) STRING_WITH_LEN("sql_modes") }, - offsetof(class Table_triggers_list, definition_modes_list), + my_offsetof(class Table_triggers_list, definition_modes_list), FILE_OPTIONS_ULLLIST }; diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 4e2b48d9faf..12fa8cfc06a 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -582,40 +582,40 @@ static const int num_view_backups= 3; */ static File_option view_parameters[]= {{{(char*) STRING_WITH_LEN("query")}, - offsetof(TABLE_LIST, query), + my_offsetof(TABLE_LIST, query), FILE_OPTIONS_ESTRING}, {{(char*) STRING_WITH_LEN("md5")}, - offsetof(TABLE_LIST, md5), + my_offsetof(TABLE_LIST, md5), FILE_OPTIONS_STRING}, {{(char*) STRING_WITH_LEN("updatable")}, - offsetof(TABLE_LIST, updatable_view), + my_offsetof(TABLE_LIST, updatable_view), FILE_OPTIONS_ULONGLONG}, {{(char*) STRING_WITH_LEN("algorithm")}, - offsetof(TABLE_LIST, algorithm), + my_offsetof(TABLE_LIST, algorithm), FILE_OPTIONS_ULONGLONG}, {{(char*) STRING_WITH_LEN("definer_user")}, - offsetof(TABLE_LIST, definer.user), + my_offsetof(TABLE_LIST, definer.user), FILE_OPTIONS_STRING}, {{(char*) STRING_WITH_LEN("definer_host")}, - offsetof(TABLE_LIST, definer.host), + my_offsetof(TABLE_LIST, definer.host), FILE_OPTIONS_STRING}, {{(char*) STRING_WITH_LEN("suid")}, - offsetof(TABLE_LIST, view_suid), + my_offsetof(TABLE_LIST, view_suid), FILE_OPTIONS_ULONGLONG}, {{(char*) STRING_WITH_LEN("with_check_option")}, - offsetof(TABLE_LIST, with_check), + my_offsetof(TABLE_LIST, with_check), FILE_OPTIONS_ULONGLONG}, {{(char*) STRING_WITH_LEN("revision")}, - offsetof(TABLE_LIST, revision), + my_offsetof(TABLE_LIST, revision), FILE_OPTIONS_REV}, {{(char*) STRING_WITH_LEN("timestamp")}, - offsetof(TABLE_LIST, timestamp), + my_offsetof(TABLE_LIST, timestamp), FILE_OPTIONS_TIMESTAMP}, {{(char*)STRING_WITH_LEN("create-version")}, - offsetof(TABLE_LIST, file_version), + my_offsetof(TABLE_LIST, file_version), FILE_OPTIONS_ULONGLONG}, {{(char*) STRING_WITH_LEN("source")}, - offsetof(TABLE_LIST, source), + my_offsetof(TABLE_LIST, source), FILE_OPTIONS_ESTRING}, {{NullS, 0}, 0, FILE_OPTIONS_STRING} From 643606cac9ee6e7ca3deefaf2a98af85223eff29 Mon Sep 17 00:00:00 2001 From: "anozdrin/alik@alik." <> Date: Fri, 20 Oct 2006 22:26:40 +0400 Subject: [PATCH 15/23] Instance Manager polishing. --- server-tools/instance-manager/guardian.cc | 72 +++++++++-------------- server-tools/instance-manager/guardian.h | 4 +- server-tools/instance-manager/instance.cc | 23 ++++---- server-tools/instance-manager/listener.cc | 4 +- server-tools/instance-manager/manager.cc | 5 +- 5 files changed, 47 insertions(+), 61 deletions(-) diff --git a/server-tools/instance-manager/guardian.cc b/server-tools/instance-manager/guardian.cc index e2142c97f33..3587a599160 100644 --- a/server-tools/instance-manager/guardian.cc +++ b/server-tools/instance-manager/guardian.cc @@ -66,11 +66,11 @@ Guardian_thread::~Guardian_thread() } -void Guardian_thread::request_shutdown(bool stop_instances_arg) +void Guardian_thread::request_shutdown() { pthread_mutex_lock(&LOCK_guardian); /* stop instances or just clean up Guardian repository */ - stop_instances(stop_instances_arg); + stop_instances(); shutdown_requested= TRUE; pthread_mutex_unlock(&LOCK_guardian); } @@ -118,11 +118,11 @@ void Guardian_thread::process_instance(Instance *instance, { /* Pid file not created yet, don't go to STARTED state yet */ } - else + else if (current_node->state != STARTED) { /* clear status fields */ - log_info("guardian: instance %s is running, set state to STARTED", - instance->options.instance_name); + log_info("guardian: instance '%s' is running, set state to STARTED.", + (const char *) instance->options.instance_name); current_node->restart_counter= 0; current_node->crash_moment= 0; current_node->state= STARTED; @@ -132,8 +132,8 @@ void Guardian_thread::process_instance(Instance *instance, { switch (current_node->state) { case NOT_STARTED: - log_info("guardian: starting instance %s", - instance->options.instance_name); + log_info("guardian: starting instance '%s'...", + (const char *) instance->options.instance_name); /* NOTE, set state to STARTING _before_ start() is called */ current_node->state= STARTING; @@ -157,8 +157,8 @@ void Guardian_thread::process_instance(Instance *instance, if (instance->is_crashed()) { instance->start(); - log_info("guardian: starting instance %s", - instance->options.instance_name); + log_info("guardian: starting instance '%s'...", + (const char *) instance->options.instance_name); } } else @@ -175,8 +175,8 @@ void Guardian_thread::process_instance(Instance *instance, instance->start(); current_node->last_checked= current_time; current_node->restart_counter++; - log_info("guardian: restarting instance %s", - instance->options.instance_name); + log_info("guardian: restarting instance '%s'...", + (const char *) instance->options.instance_name); } } else @@ -382,12 +382,11 @@ int Guardian_thread::stop_guard(Instance *instance) SYNOPSYS stop_instances() - stop_instances_arg whether we should stop instances at shutdown DESCRIPTION Loops through the guarded_instances list and prepares them for shutdown. - If stop_instances was requested, we need to issue a stop command and change - the state accordingly. Otherwise we simply delete an entry. + For each instance we issue a stop command and change the state + accordingly. NOTE Guardian object should be locked by the calling function. @@ -397,42 +396,29 @@ int Guardian_thread::stop_guard(Instance *instance) 1 - error occured */ -int Guardian_thread::stop_instances(bool stop_instances_arg) +int Guardian_thread::stop_instances() { LIST *node; node= guarded_instances; while (node != NULL) { - if (!stop_instances_arg) + GUARD_NODE *current_node= (GUARD_NODE *) node->data; + /* + If instance is running or was running (and now probably hanging), + request stop. + */ + if (current_node->instance->is_running() || + (current_node->state == STARTED)) { - /* just forget about an instance */ - guarded_instances= list_delete(guarded_instances, node); - /* - This should still work fine, as we have only removed the - node from the list. The pointer to the next one is still valid - */ - node= node->next; + current_node->state= STOPPING; + current_node->last_checked= time(NULL); } else - { - GUARD_NODE *current_node= (GUARD_NODE *) node->data; - /* - If instance is running or was running (and now probably hanging), - request stop. - */ - if (current_node->instance->is_running() || - (current_node->state == STARTED)) - { - current_node->state= STOPPING; - current_node->last_checked= time(NULL); - } - else - /* otherwise remove it from the list */ - guarded_instances= list_delete(guarded_instances, node); - /* But try to kill it anyway. Just in case */ - current_node->instance->kill_instance(SIGTERM); - node= node->next; - } + /* otherwise remove it from the list */ + guarded_instances= list_delete(guarded_instances, node); + /* But try to kill it anyway. Just in case */ + current_node->instance->kill_instance(SIGTERM); + node= node->next; } return 0; } @@ -440,7 +426,7 @@ int Guardian_thread::stop_instances(bool stop_instances_arg) void Guardian_thread::lock() { - pthread_mutex_lock(&LOCK_guardian); + pthread_mutex_lock(&LOCK_guardian); } diff --git a/server-tools/instance-manager/guardian.h b/server-tools/instance-manager/guardian.h index 16b4c373c91..f1c54262c12 100644 --- a/server-tools/instance-manager/guardian.h +++ b/server-tools/instance-manager/guardian.h @@ -89,7 +89,7 @@ public: /* Initialize or refresh the list of guarded instances */ int init(); /* Request guardian shutdown. Stop instances if needed */ - void request_shutdown(bool stop_instances); + void request_shutdown(); /* Start instance protection */ int guard(Instance *instance, bool nolock= FALSE); /* Stop instance protection */ @@ -104,7 +104,7 @@ public: private: /* Prepares Guardian shutdown. Stops instances is needed */ - int stop_instances(bool stop_instances_arg); + int stop_instances(); /* check instance state and act accordingly */ void process_instance(Instance *instance, GUARD_NODE *current_node, LIST **guarded_instances, LIST *elem); diff --git a/server-tools/instance-manager/instance.cc b/server-tools/instance-manager/instance.cc index b792d132da0..daa8082ef2f 100644 --- a/server-tools/instance-manager/instance.cc +++ b/server-tools/instance-manager/instance.cc @@ -156,8 +156,8 @@ static int start_process(Instance_options *instance_options, /* exec never returns */ exit(1); case -1: - log_info("cannot create a new process to start instance %s", - instance_options->instance_name); + log_info("cannot create a new process to start instance '%s'.", + (const char *) instance_options->instance_name); return 1; } return 0; @@ -252,7 +252,8 @@ static void start_and_monitor_instance(Instance_options *old_instance_options, MAX_INSTANCE_NAME_LEN - 1); instance_name_len= old_instance_options->instance_name_len; - log_info("starting instance %s", instance_name_buff); + log_info("starting instance '%s'...", + (const char *) instance_name_buff); if (start_process(old_instance_options, &process_info)) { @@ -286,9 +287,9 @@ void Instance::remove_pid() int pid; if ((pid= options.get_pid()) != 0) /* check the pidfile */ if (options.unlink_pidfile()) /* remove stalled pidfile */ - log_error("cannot remove pidfile for instance %i, this might be \ + log_error("cannot remove pidfile for instance '%s', this might be \ since IM lacks permmissions or hasn't found the pidifle", - options.instance_name); + (const char *) options.instance_name); } @@ -435,9 +436,9 @@ bool Instance::is_running() We have successfully connected to the server using fake username/password. Write a warning to the logfile. */ - log_info("The Instance Manager was able to log into you server \ - with faked compiled-in password while checking server status. \ - Looks like something is wrong."); + log_info("The Instance Manager was able to log into you server " + "with faked compiled-in password while checking server status. " + "Looks like something is wrong."); pthread_mutex_unlock(&LOCK_instance); return_val= TRUE; /* server is alive */ } @@ -577,10 +578,10 @@ void Instance::kill_instance(int signum) /* Kill suceeded */ if (signum == SIGKILL) /* really killed instance with SIGKILL */ { - log_error("The instance %s is being stopped forcibly. Normally" \ - "it should not happen. Probably the instance has been" \ + log_error("The instance '%s' is being stopped forcibly. Normally" + "it should not happen. Probably the instance has been" "hanging. You should also check your IM setup", - options.instance_name); + (const char *) options.instance_name); /* After sucessful hard kill the pidfile need to be removed */ options.unlink_pidfile(); } diff --git a/server-tools/instance-manager/listener.cc b/server-tools/instance-manager/listener.cc index 58a4093dd05..3f60c2cc74a 100644 --- a/server-tools/instance-manager/listener.cc +++ b/server-tools/instance-manager/listener.cc @@ -280,7 +280,7 @@ int Listener_thread::create_tcp_socket() FD_SET(ip_socket, &read_fds); sockets[num_sockets++]= ip_socket; - log_info("accepting connections on ip socket"); + log_info("accepting connections on ip socket (port: %d)", (int) im_port); return 0; } @@ -334,7 +334,7 @@ create_unix_socket(struct sockaddr_un &unix_socket_address) /* make sure that instances won't be listening our sockets */ set_no_inherit(unix_socket); - log_info("accepting connections on unix socket %s", + log_info("accepting connections on unix socket '%s'", unix_socket_address.sun_path); sockets[num_sockets++]= unix_socket; FD_SET(unix_socket, &read_fds); diff --git a/server-tools/instance-manager/manager.cc b/server-tools/instance-manager/manager.cc index 6f28c39da77..ec22e5c4fbb 100644 --- a/server-tools/instance-manager/manager.cc +++ b/server-tools/instance-manager/manager.cc @@ -110,7 +110,7 @@ void stop_all(Guardian_thread *guardian, Thread_registry *registry) Let guardian thread know that it should break it's processing cycle, once it wakes up. */ - guardian->request_shutdown(true); + guardian->request_shutdown(); /* wake guardian */ pthread_cond_signal(&guardian->COND_guardian); /* stop all threads */ @@ -282,8 +282,7 @@ void manager(const Options &options) { if (!guardian_thread.is_stopped()) { - bool stop_instances= true; - guardian_thread.request_shutdown(stop_instances); + guardian_thread.request_shutdown(); pthread_cond_signal(&guardian_thread.COND_guardian); } else From 81962878d83e159bda413a541291292a1197983d Mon Sep 17 00:00:00 2001 From: "kostja@bodhi.local" <> Date: Mon, 23 Oct 2006 13:55:29 +0400 Subject: [PATCH 16/23] A post-merge fix. --- mysql-test/r/view.result | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index e13ec2165b9..6f53a56a4ce 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -2968,6 +2968,8 @@ INSERT INTO t1 VALUES (1); CREATE FUNCTION f1() RETURNS INT RETURN (SELECT * FROM v1); UPDATE t1 SET i= f1(); DROP FUNCTION f1; +DROP VIEW v1; +DROP TABLE t1; CREATE TABLE t1(id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, val INT UNSIGNED NOT NULL); CREATE VIEW v1 AS SELECT id, val FROM t1 WHERE val >= 1 AND val <= 5 WITH CHECK OPTION; INSERT INTO v1 (val) VALUES (2); From b7b991cec395da1cf3277b00df9328c0b6a1b7fa Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Wed, 25 Oct 2006 19:53:26 +0400 Subject: [PATCH 17/23] BUG#18819: DELETE IGNORE hangs on foreign key parent delete If the error happens during DELETE IGNORE, nothing could be send to the client, thus leaving it frozen expecting the reply. The problem was that if some error occurred, it wouldn't be reported to the client because of IGNORE, but neither success would be reported. MySQL 4.1 would not freeze the client, but will report ERROR 1105 (HY000): Unknown error instead, which is also a bug. The solution is to report success if we are in DELETE IGNORE and some non-fatal error has happened. --- mysql-test/r/innodb_mysql.result | 16 ++++++++++++++++ mysql-test/t/innodb_mysql.test | 29 +++++++++++++++++++++++++++++ sql/sql_delete.cc | 3 ++- 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/innodb_mysql.result b/mysql-test/r/innodb_mysql.result index ee4c114087d..68995565752 100644 --- a/mysql-test/r/innodb_mysql.result +++ b/mysql-test/r/innodb_mysql.result @@ -104,3 +104,19 @@ SELECT `id1` FROM `t1` WHERE `id1` NOT IN (SELECT `id1` FROM `t2` WHERE `id2` = id1 2 DROP TABLE t1, t2; +DROP TABLE IF EXISTS t2, t1; +CREATE TABLE t1 (i INT NOT NULL PRIMARY KEY) ENGINE= InnoDB; +CREATE TABLE t2 ( +i INT NOT NULL, +FOREIGN KEY (i) REFERENCES t1 (i) ON DELETE NO ACTION +) ENGINE= InnoDB; +INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES (1); +DELETE IGNORE FROM t1 WHERE i = 1; +Warnings: +Error 1217 Cannot delete or update a parent row: a foreign key constraint fails +SELECT * FROM t1, t2; +i i +1 1 +DROP TABLE t2, t1; +End of 4.1 tests. diff --git a/mysql-test/t/innodb_mysql.test b/mysql-test/t/innodb_mysql.test index a5fe248604f..9593982826b 100644 --- a/mysql-test/t/innodb_mysql.test +++ b/mysql-test/t/innodb_mysql.test @@ -117,3 +117,32 @@ INSERT INTO `t2`(`id1`,`id2`,`id3`,`id4`) VALUES SELECT `id1` FROM `t1` WHERE `id1` NOT IN (SELECT `id1` FROM `t2` WHERE `id2` = 1 AND `id3` = 2); DROP TABLE t1, t2; + + +# +# BUG#18819: DELETE IGNORE hangs on foreign key parent delete +# +# The bug itself does not relate to InnoDB, but we have to use foreign +# keys to reproduce it. +# +--disable_warnings +DROP TABLE IF EXISTS t2, t1; +--enable_warnings + +CREATE TABLE t1 (i INT NOT NULL PRIMARY KEY) ENGINE= InnoDB; +CREATE TABLE t2 ( + i INT NOT NULL, + FOREIGN KEY (i) REFERENCES t1 (i) ON DELETE NO ACTION +) ENGINE= InnoDB; + +INSERT INTO t1 VALUES (1); +INSERT INTO t2 VALUES (1); + +DELETE IGNORE FROM t1 WHERE i = 1; + +SELECT * FROM t1, t2; + +DROP TABLE t2, t1; + + +--echo End of 4.1 tests. diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index b085d37be78..506eec88500 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -253,7 +253,8 @@ cleanup: mysql_unlock_tables(thd, thd->lock); thd->lock=0; } - if (error >= 0 || thd->net.report_error) + if ((error >= 0 || thd->net.report_error) && + (!thd->lex->ignore || thd->is_fatal_error)) send_error(thd,thd->killed ? ER_SERVER_SHUTDOWN: 0); else { From 2c034449714bbe3166023898dbb8b5dc39e862cc Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Wed, 25 Oct 2006 20:09:31 +0400 Subject: [PATCH 18/23] Fix after manual merge. --- mysql-test/r/innodb_mysql.result | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/mysql-test/r/innodb_mysql.result b/mysql-test/r/innodb_mysql.result index cc0fc4289dc..a96bc4786fa 100644 --- a/mysql-test/r/innodb_mysql.result +++ b/mysql-test/r/innodb_mysql.result @@ -295,7 +295,6 @@ b c d drop table t1,t4; - DROP TABLE IF EXISTS t2, t1; CREATE TABLE t1 (i INT NOT NULL PRIMARY KEY) ENGINE= InnoDB; CREATE TABLE t2 ( @@ -306,7 +305,7 @@ INSERT INTO t1 VALUES (1); INSERT INTO t2 VALUES (1); DELETE IGNORE FROM t1 WHERE i = 1; Warnings: -Error 1217 Cannot delete or update a parent row: a foreign key constraint fails +Error 1451 Cannot delete or update a parent row: a foreign key constraint fails (`test/t2`, CONSTRAINT `t2_ibfk_1` FOREIGN KEY (`i`) REFERENCES `t1` (`i`) ON DELETE NO ACTION) SELECT * FROM t1, t2; i i 1 1 From 0e0f7a0423a88363ea5263ffeadde1bbb7ba2820 Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Fri, 27 Oct 2006 13:32:41 +0400 Subject: [PATCH 19/23] BUG#22584: last_insert_id not updated after inserting a record through a updatable view. When there's a VIEW on a base table that have AUTO_INCREMENT column, and this VIEW doesn't provide an access such column, after INSERT to such VIEW LAST_INSERT_ID() did not return the value just generated. This behaviour is intended and correct, because if the VIEW doesn't list some columns then these columns are effectively hidden from the user, and so any side effects of inserting default values to them. However, there was a bug that such statement inserting into a view would reset LAST_INSERT_ID() instead of leaving it unchanged. This patch restores the original value of LAST_INSERT_ID() instead of resetting it to zero. --- mysql-test/r/view.result | 34 ++++++++++++++++++++++++++++++++++ mysql-test/t/view.test | 38 ++++++++++++++++++++++++++++++++++++++ sql/sql_parse.cc | 19 +++++++++++++++++-- 3 files changed, 89 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 13921dc88d7..198233adf71 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -2970,4 +2970,38 @@ UPDATE t1 SET i= f1(); DROP FUNCTION f1; DROP VIEW v1; DROP TABLE t1; +DROP VIEW IF EXISTS v1, v2; +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (i INT AUTO_INCREMENT PRIMARY KEY, j INT); +CREATE VIEW v1 AS SELECT j FROM t1; +CREATE VIEW v2 AS SELECT * FROM t1; +INSERT INTO t1 (j) VALUES (1); +SELECT LAST_INSERT_ID(); +LAST_INSERT_ID() +1 +INSERT INTO v1 (j) VALUES (2); +# LAST_INSERT_ID() should not change. +SELECT LAST_INSERT_ID(); +LAST_INSERT_ID() +1 +INSERT INTO v2 (j) VALUES (3); +# LAST_INSERT_ID() should be updated. +SELECT LAST_INSERT_ID(); +LAST_INSERT_ID() +3 +INSERT INTO v1 (j) SELECT j FROM t1; +# LAST_INSERT_ID() should not change. +SELECT LAST_INSERT_ID(); +LAST_INSERT_ID() +3 +SELECT * FROM t1; +i j +1 1 +2 2 +3 3 +4 1 +5 2 +6 3 +DROP VIEW v1, v2; +DROP TABLE t1; End of 5.0 tests. diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index d3dcd857e62..401bda27bb4 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -2908,4 +2908,42 @@ DROP VIEW v1; DROP TABLE t1; +# +# BUG#22584: last_insert_id not updated after inserting a record +# through a updatable view +# +# We still do not update LAST_INSERT_ID if AUTO_INCREMENT column is +# not accessible through a view. However, we do not reset the value +# of LAST_INSERT_ID, but keep it unchanged. +# +--disable_warnings +DROP VIEW IF EXISTS v1, v2; +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (i INT AUTO_INCREMENT PRIMARY KEY, j INT); +CREATE VIEW v1 AS SELECT j FROM t1; +CREATE VIEW v2 AS SELECT * FROM t1; + +INSERT INTO t1 (j) VALUES (1); +SELECT LAST_INSERT_ID(); + +INSERT INTO v1 (j) VALUES (2); +--echo # LAST_INSERT_ID() should not change. +SELECT LAST_INSERT_ID(); + +INSERT INTO v2 (j) VALUES (3); +--echo # LAST_INSERT_ID() should be updated. +SELECT LAST_INSERT_ID(); + +INSERT INTO v1 (j) SELECT j FROM t1; +--echo # LAST_INSERT_ID() should not change. +SELECT LAST_INSERT_ID(); + +SELECT * FROM t1; + +DROP VIEW v1, v2; +DROP TABLE t1; + + --echo End of 5.0 tests. diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 7b5387e5e2f..d2cbd5b0008 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3369,8 +3369,16 @@ end_with_restore_list: res= mysql_insert(thd, all_tables, lex->field_list, lex->many_values, lex->update_list, lex->value_list, lex->duplicates, lex->ignore); + + /* + If we have inserted into a VIEW, and the base table has + AUTO_INCREMENT column, but this column is not accessible through + a view, then we should restore LAST_INSERT_ID to the value it + had before the statement. + */ if (first_table->view && !first_table->contain_auto_increment) - thd->last_insert_id= 0; // do not show last insert ID if VIEW have not it + thd->last_insert_id= thd->current_insert_id; + break; } case SQLCOM_REPLACE_SELECT: @@ -3431,8 +3439,15 @@ end_with_restore_list: select_lex->table_list.first= (byte*) first_table; } + /* + If we have inserted into a VIEW, and the base table has + AUTO_INCREMENT column, but this column is not accessible through + a view, then we should restore LAST_INSERT_ID to the value it + had before the statement. + */ if (first_table->view && !first_table->contain_auto_increment) - thd->last_insert_id= 0; // do not show last insert ID if VIEW have not it + thd->last_insert_id= thd->current_insert_id; + break; } case SQLCOM_TRUNCATE: From 8143148bc16be19d8ed3f327d1399886a08c0eb1 Mon Sep 17 00:00:00 2001 From: "kostja@bodhi.local" <> Date: Mon, 30 Oct 2006 11:36:30 +0300 Subject: [PATCH 20/23] Cleanup. --- sql/sql_table.cc | 202 ----------------------------------------------- 1 file changed, 202 deletions(-) diff --git a/sql/sql_table.cc b/sql/sql_table.cc index f6dbba4aa79..73dd2c809a6 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -2934,208 +2934,6 @@ err: } -#ifdef NOT_USED -/* - CREATE INDEX and DROP INDEX are implemented by calling ALTER TABLE with - the proper arguments. This isn't very fast but it should work for most - cases. - One should normally create all indexes with CREATE TABLE or ALTER TABLE. -*/ - -int mysql_create_indexes(THD *thd, TABLE_LIST *table_list, List &keys) -{ - List fields; - List drop; - List alter; - HA_CREATE_INFO create_info; - int rc; - uint idx; - uint db_options; - uint key_count; - TABLE *table; - Field **f_ptr; - KEY *key_info_buffer; - char path[FN_REFLEN+1]; - DBUG_ENTER("mysql_create_index"); - - /* - Try to use online generation of index. - This requires that all indexes can be created online. - Otherwise, the old alter table procedure is executed. - - Open the table to have access to the correct table handler. - */ - if (!(table=open_ltable(thd,table_list,TL_WRITE_ALLOW_READ))) - DBUG_RETURN(-1); - - /* - The add_index method takes an array of KEY structs for the new indexes. - Preparing a new table structure generates this array. - It needs a list with all fields of the table, which does not need to - be correct in every respect. The field names are important. - */ - for (f_ptr= table->field; *f_ptr; f_ptr++) - { - create_field *c_fld= new create_field(*f_ptr, *f_ptr); - c_fld->unireg_check= Field::NONE; /*avoid multiple auto_increments*/ - fields.push_back(c_fld); - } - bzero((char*) &create_info,sizeof(create_info)); - create_info.db_type=DB_TYPE_DEFAULT; - create_info.default_table_charset= thd->variables.collation_database; - db_options= 0; - if (mysql_prepare_table(thd, &create_info, &fields, - &keys, /*tmp_table*/ 0, &db_options, table->file, - &key_info_buffer, key_count, - /*select_field_count*/ 0)) - DBUG_RETURN(-1); - - /* - Check if all keys can be generated with the add_index method. - If anyone cannot, then take the old way. - */ - for (idx=0; idx< key_count; idx++) - { - DBUG_PRINT("info", ("creating index %s", key_info_buffer[idx].name)); - if (!(table->file->index_ddl_flags(key_info_buffer+idx)& - (HA_DDL_ONLINE| HA_DDL_WITH_LOCK))) - break ; - } - if ((idx < key_count)|| !key_count) - { - /* Re-initialize the create_info, which was changed by prepare table. */ - bzero((char*) &create_info,sizeof(create_info)); - create_info.db_type=DB_TYPE_DEFAULT; - create_info.default_table_charset= thd->variables.collation_database; - /* Cleanup the fields list. We do not want to create existing fields. */ - fields.delete_elements(); - if (real_alter_table(thd, table_list->db, table_list->table_name, - &create_info, table_list, table, - fields, keys, drop, alter, 0, (ORDER*)0, - ALTER_ADD_INDEX, DUP_ERROR)) - /* Don't need to free((gptr) key_info_buffer);*/ - DBUG_RETURN(-1); - } - else - { - if (table->file->add_index(table, key_info_buffer, key_count)|| - build_table_path(path, sizeof(path), table_list->db, - (lower_case_table_names == 2) ? - table_list->alias : table_list->table_name, - reg_ext) == 0 || - mysql_create_frm(thd, path, &create_info, - fields, key_count, key_info_buffer, table->file)) - /* don't need to free((gptr) key_info_buffer);*/ - DBUG_RETURN(-1); - } - /* don't need to free((gptr) key_info_buffer);*/ - DBUG_RETURN(0); -} - - -int mysql_drop_indexes(THD *thd, TABLE_LIST *table_list, - List &drop) -{ - List fields; - List keys; - List alter; - HA_CREATE_INFO create_info; - uint idx; - uint db_options; - uint key_count; - uint *key_numbers; - TABLE *table; - Field **f_ptr; - KEY *key_info; - KEY *key_info_buffer; - char path[FN_REFLEN]; - DBUG_ENTER("mysql_drop_index"); - - /* - Try to use online generation of index. - This requires that all indexes can be created online. - Otherwise, the old alter table procedure is executed. - - Open the table to have access to the correct table handler. - */ - if (!(table=open_ltable(thd,table_list,TL_WRITE_ALLOW_READ))) - DBUG_RETURN(-1); - - /* - The drop_index method takes an array of key numbers. - It cannot get more entries than keys in the table. - */ - key_numbers= (uint*) thd->alloc(sizeof(uint*)*table->keys); - key_count= 0; - - /* - Get the number of each key and check if it can be created online. - */ - List_iterator drop_it(drop); - Alter_drop *drop_key; - while ((drop_key= drop_it++)) - { - /* Find the key in the table. */ - key_info=table->key_info; - for (idx=0; idx< table->keys; idx++, key_info++) - { - if (!my_strcasecmp(system_charset_info, key_info->name, drop_key->name)) - break; - } - if (idx>= table->keys) - { - my_error(ER_CANT_DROP_FIELD_OR_KEY, MYF(0), drop_key->name); - /*don't need to free((gptr) key_numbers);*/ - DBUG_RETURN(-1); - } - /* - Check if the key can be generated with the add_index method. - If anyone cannot, then take the old way. - */ - DBUG_PRINT("info", ("dropping index %s", table->key_info[idx].name)); - if (!(table->file->index_ddl_flags(table->key_info+idx)& - (HA_DDL_ONLINE| HA_DDL_WITH_LOCK))) - break ; - key_numbers[key_count++]= idx; - } - - bzero((char*) &create_info,sizeof(create_info)); - create_info.db_type=DB_TYPE_DEFAULT; - create_info.default_table_charset= thd->variables.collation_database; - - if ((drop_key)|| (drop.elements<= 0)) - { - if (real_alter_table(thd, table_list->db, table_list->table_name, - &create_info, table_list, table, - fields, keys, drop, alter, 0, (ORDER*)0, - ALTER_DROP_INDEX, DUP_ERROR)) - /*don't need to free((gptr) key_numbers);*/ - DBUG_RETURN(-1); - } - else - { - db_options= 0; - if (table->file->drop_index(table, key_numbers, key_count)|| - mysql_prepare_table(thd, &create_info, &fields, - &keys, /*tmp_table*/ 0, &db_options, table->file, - &key_info_buffer, key_count, - /*select_field_count*/ 0)|| - build_table_path(path, sizeof(path), table_list->db, - (lower_case_table_names == 2) ? - table_list->alias : table_list->table_name, - reg_ext) == 0 || - mysql_create_frm(thd, path, &create_info, - fields, key_count, key_info_buffer, table->file)) - /*don't need to free((gptr) key_numbers);*/ - DBUG_RETURN(-1); - } - - /*don't need to free((gptr) key_numbers);*/ - DBUG_RETURN(0); -} -#endif /* NOT_USED */ - - /* Alter table */ From dfc4dd423c922585ff36ceb1192ea8ca7e825f54 Mon Sep 17 00:00:00 2001 From: "kostja@bodhi.local" <> Date: Mon, 30 Oct 2006 12:03:42 +0300 Subject: [PATCH 21/23] A cleanup. --- sql/sql_parse.cc | 6 ------ 1 file changed, 6 deletions(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index d2cbd5b0008..2a46656e8d2 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -3045,11 +3045,6 @@ end_with_restore_list: case SQLCOM_ALTER_TABLE: DBUG_ASSERT(first_table == all_tables && first_table != 0); -#if defined(DONT_ALLOW_SHOW_COMMANDS) - my_message(ER_NOT_ALLOWED_COMMAND, ER(ER_NOT_ALLOWED_COMMAND), - MYF(0)); /* purecov: inspected */ - goto error; -#else { ulong priv=0; if (lex->name && (!lex->name[0] || strlen(lex->name) > NAME_LEN)) @@ -3115,7 +3110,6 @@ end_with_restore_list: } break; } -#endif /*DONT_ALLOW_SHOW_COMMANDS*/ case SQLCOM_RENAME_TABLE: { DBUG_ASSERT(first_table == all_tables && first_table != 0); From 1bfe00e0da3496937bc2eaef4532e014a6ad9d30 Mon Sep 17 00:00:00 2001 From: "kroki/tomash@moonlight.intranet" <> Date: Mon, 30 Oct 2006 17:47:02 +0300 Subject: [PATCH 22/23] BUG#21915: Changing limits of table_cache when setting max_connections If the user has specified --max-connections=N or --table-open-cache=M options to the server, a warning could be given that some values were recalculated, and table-open-cache could be assigned greater value. Note that both warning and increase of table-open-cache were totally harmless. This patch fixes recalculation code to ensure that table-open-cache will be never increased automatically and that a warning will be given only if some values had to be decreased due to operating system limits. No test case is provided because we neither can't predict nor control operating system limits for maximal number of open files. --- sql/mysql_priv.h | 2 ++ sql/mysqld.cc | 42 +++++++++++++++++++++++++++++++++--------- 2 files changed, 35 insertions(+), 9 deletions(-) diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 4a5658c5ccf..c2d378c0be9 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -95,6 +95,8 @@ MY_LOCALE *my_locale_by_name(const char *name); #define MAX_ACCEPT_RETRY 10 // Test accept this many times #define MAX_FIELDS_BEFORE_HASH 32 #define USER_VARS_HASH_SIZE 16 +#define TABLE_OPEN_CACHE_MIN 64 +#define TABLE_OPEN_CACHE_DEFAULT 64 #define STACK_MIN_SIZE 8192 // Abort if less stack during eval. #define STACK_BUFF_ALLOC 64 // For stack overrun checks #ifndef MYSQLD_NET_RETRY_COUNT diff --git a/sql/mysqld.cc b/sql/mysqld.cc index bf83772a8d8..f7b1a2687ea 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -2526,19 +2526,43 @@ static int init_common_variables(const char *conf_file_name, int argc, /* connections and databases needs lots of files */ { - uint files, wanted_files; + uint files, wanted_files, max_open_files; - wanted_files= 10+(uint) max(max_connections*5, - max_connections+table_cache_size*2); - set_if_bigger(wanted_files, open_files_limit); - files= my_set_max_open_files(wanted_files); + /* MyISAM requires two file handles per table. */ + wanted_files= 10+max_connections+table_cache_size*2; + /* + We are trying to allocate no less than max_connections*5 file + handles (i.e. we are trying to set the limit so that they will + be available). In addition, we allocate no less than how much + was already allocated. However below we report a warning and + recompute values only if we got less file handles than were + explicitly requested. No warning and re-computation occur if we + can't get max_connections*5 but still got no less than was + requested (value of wanted_files). + */ + max_open_files= max(max(wanted_files, max_connections*5), + open_files_limit); + files= my_set_max_open_files(max_open_files); if (files < wanted_files) { if (!open_files_limit) { - max_connections= (ulong) min((files-10),max_connections); - table_cache_size= (ulong) max((files-10-max_connections)/2,64); + /* + If we have requested too much file handles than we bring + max_connections in supported bounds. + */ + max_connections= (ulong) min(files-10-TABLE_OPEN_CACHE_MIN*2, + max_connections); + /* + Decrease table_cache_size according to max_connections, but + not below TABLE_OPEN_CACHE_MIN. Outer min() ensures that we + never increase table_cache_size automatically (that could + happen if max_connections is decreased above). + */ + table_cache_size= (ulong) min(max((files-10-max_connections)/2, + TABLE_OPEN_CACHE_MIN), + table_cache_size); DBUG_PRINT("warning", ("Changed limits: max_open_files: %u max_connections: %ld table_cache: %ld", files, max_connections, table_cache_size)); @@ -5511,8 +5535,8 @@ The minimum value for this variable is 4096.", 0, 0, 0, 0}, {"table_cache", OPT_TABLE_CACHE, "The number of open tables for all threads.", (gptr*) &table_cache_size, - (gptr*) &table_cache_size, 0, GET_ULONG, REQUIRED_ARG, 64, 1, 512*1024L, - 0, 1, 0}, + (gptr*) &table_cache_size, 0, GET_ULONG, REQUIRED_ARG, + TABLE_OPEN_CACHE_DEFAULT, 1, 512*1024L, 0, 1, 0}, {"thread_cache_size", OPT_THREAD_CACHE_SIZE, "How many threads we should keep in a cache for reuse.", (gptr*) &thread_cache_size, (gptr*) &thread_cache_size, 0, GET_ULONG, From bd1b57f930569cbb976a2765f52fd7ca36f1ba50 Mon Sep 17 00:00:00 2001 From: "dlenev@mockturtle.local" <> Date: Wed, 1 Nov 2006 15:41:48 +0300 Subject: [PATCH 23/23] Small cleanup in code handling stored routines/table prelocking. Use lazy initialization for Query_tables_list::sroutines hash. This step should significantly decrease amount of memory consumed by stored routines as we no longer will allocate chunk of memory required for this HASH for each statement in routine. --- include/hash.h | 2 ++ sql/sp.cc | 4 ++++ sql/sql_lex.cc | 11 ++++++++++- sql/sql_lex.h | 6 +++++- 4 files changed, 21 insertions(+), 2 deletions(-) diff --git a/include/hash.h b/include/hash.h index 8f5ff21ae5e..4d6ee77fa0c 100644 --- a/include/hash.h +++ b/include/hash.h @@ -65,6 +65,8 @@ my_bool hash_check(HASH *hash); /* Only in debug library */ #define hash_clear(H) bzero((char*) (H),sizeof(*(H))) #define hash_inited(H) ((H)->array.buffer != 0) +#define hash_init_opt(A,B,C,D,E,F,G,H) \ + (!hash_inited(A) && _hash_init(A,B,C,D,E,F,G, H CALLER_INFO)) #ifdef __cplusplus } diff --git a/sql/sp.cc b/sql/sp.cc index 49dbed79fe5..5d5c2b148d4 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -1369,6 +1369,10 @@ static bool add_used_routine(LEX *lex, Query_arena *arena, const LEX_STRING *key, TABLE_LIST *belong_to_view) { + hash_init_opt(&lex->sroutines, system_charset_info, + Query_tables_list::START_SROUTINES_HASH_SIZE, + 0, 0, sp_sroutine_key, 0, 0); + if (!hash_search(&lex->sroutines, (byte *)key->str, key->length)) { Sroutine_hash_entry *rn= diff --git a/sql/sql_lex.cc b/sql/sql_lex.cc index 405f576ac04..24b9766a658 100644 --- a/sql/sql_lex.cc +++ b/sql/sql_lex.cc @@ -1634,9 +1634,18 @@ void Query_tables_list::reset_query_tables_list(bool init) query_tables_last= &query_tables; query_tables_own_last= 0; if (init) - hash_init(&sroutines, system_charset_info, 0, 0, 0, sp_sroutine_key, 0, 0); + { + /* + We delay real initialization of hash (and therefore related + memory allocation) until first insertion into this hash. + */ + hash_clear(&sroutines); + } else if (sroutines.records) + { + /* Non-zero sroutines.records means that hash was initialized. */ my_hash_reset(&sroutines); + } sroutines_list.empty(); sroutines_list_own_last= sroutines_list.next; sroutines_list_own_elements= 0; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 378f968118e..bfe7d9518f3 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -742,7 +742,11 @@ public: 0 - indicates that this query does not need prelocking. */ TABLE_LIST **query_tables_own_last; - /* Set of stored routines called by statement. */ + /* + Set of stored routines called by statement. + (Note that we use lazy-initialization for this hash). + */ + enum { START_SROUTINES_HASH_SIZE= 16 }; HASH sroutines; /* List linking elements of 'sroutines' set. Allows you to add new elements