From 1626b42ca34d09844dd628f390d9ef8fb925ec13 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 18 Mar 2008 20:25:34 +0400 Subject: [PATCH 1/5] BUG#34768 - nondeterministic INSERT using LIMIT logged in stmt mode if binlog_format=mixed Statement-based replication of DELETE ... LIMIT, UPDATE ... LIMIT, INSERT ... SELECT ... LIMIT is not safe as order of rows is not defined. With this fix, we issue a warning that this statement is not safe to replicate in statement mode, or go to row-based mode in mixed mode. Note that we may consider a statement as safe if ORDER BY primary_key is present. However it may confuse users to see very similiar statements replicated differently. Note 2: regular UPDATE statement (w/o LIMIT) is unsafe as well, but this patch doesn't address this issue. See comment from Kristian posted 18 Mar 10:55. mysql-test/suite/binlog/r/binlog_stm_ps.result: Updated a test case according to fix for BUG#34768: INSERT ... SELECT ... LIMIT is now replicated in row mode. mysql-test/suite/binlog/r/binlog_unsafe.result: A test case for BUG#34768. mysql-test/suite/binlog/t/binlog_unsafe.test: A test case for BUG#34768. sql/sql_delete.cc: Statement-based replication of DELETE ... LIMIT is not safe as order of rows is not defined, so in mixed mode we go to row-based. sql/sql_insert.cc: Statement-based replication of INSERT ... SELECT ... LIMIT is not safe as order of rows is not defined, so in mixed mode we go to row-based. sql/sql_update.cc: Statement-based replication of UPDATE ... LIMIT is not safe as order of rows is not defined, so in mixed mode we go to row-based. --- .../suite/binlog/r/binlog_stm_ps.result | 3 +- .../suite/binlog/r/binlog_unsafe.result | 28 +++++++++++++++++++ mysql-test/suite/binlog/t/binlog_unsafe.test | 22 ++++++++++++++- sql/sql_delete.cc | 13 +++++++++ sql/sql_insert.cc | 13 +++++++++ sql/sql_update.cc | 13 +++++++++ 6 files changed, 90 insertions(+), 2 deletions(-) diff --git a/mysql-test/suite/binlog/r/binlog_stm_ps.result b/mysql-test/suite/binlog/r/binlog_stm_ps.result index 47934665116..2128367deb5 100644 --- a/mysql-test/suite/binlog/r/binlog_stm_ps.result +++ b/mysql-test/suite/binlog/r/binlog_stm_ps.result @@ -16,5 +16,6 @@ master-bin.000001 # Query # # use `test`; create table t1 (a int) master-bin.000001 # User var # # @`a`=98 master-bin.000001 # Query # # use `test`; insert into t1 values (@a),(98) master-bin.000001 # Query # # use `test`; insert into t1 values (99) -master-bin.000001 # Query # # use `test`; insert into t1 select 100 limit 100 +master-bin.000001 # Table_map # # table_id: # (test.t1) +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F drop table t1; diff --git a/mysql-test/suite/binlog/r/binlog_unsafe.result b/mysql-test/suite/binlog/r/binlog_unsafe.result index 47284ed8bc3..9d7f359ef15 100644 --- a/mysql-test/suite/binlog/r/binlog_unsafe.result +++ b/mysql-test/suite/binlog/r/binlog_unsafe.result @@ -11,3 +11,31 @@ Level Warning Code 1592 Message Statement is not safe to log in statement format. DROP TABLE t1,t2,t3; +CREATE TABLE t1(a INT, b INT, KEY(a), PRIMARY KEY(b)); +INSERT INTO t1 SELECT * FROM t1 LIMIT 1; +Warnings: +Warning 1592 Statement is not safe to log in statement format. +REPLACE INTO t1 SELECT * FROM t1 LIMIT 1; +Warnings: +Warning 1592 Statement is not safe to log in statement format. +UPDATE t1 SET a=1 LIMIT 1; +Warnings: +Warning 1592 Statement is not safe to log in statement format. +DELETE FROM t1 LIMIT 1; +Warnings: +Warning 1592 Statement is not safe to log in statement format. +CREATE PROCEDURE p1() +BEGIN +INSERT INTO t1 SELECT * FROM t1 LIMIT 1; +REPLACE INTO t1 SELECT * FROM t1 LIMIT 1; +UPDATE t1 SET a=1 LIMIT 1; +DELETE FROM t1 LIMIT 1; +END| +CALL p1(); +Warnings: +Warning 1592 Statement is not safe to log in statement format. +Warning 1592 Statement is not safe to log in statement format. +Warning 1592 Statement is not safe to log in statement format. +Warning 1592 Statement is not safe to log in statement format. +DROP PROCEDURE p1; +DROP TABLE t1; diff --git a/mysql-test/suite/binlog/t/binlog_unsafe.test b/mysql-test/suite/binlog/t/binlog_unsafe.test index f34c22dc5f7..de15f5f6f14 100644 --- a/mysql-test/suite/binlog/t/binlog_unsafe.test +++ b/mysql-test/suite/binlog/t/binlog_unsafe.test @@ -15,4 +15,24 @@ query_vertical SHOW WARNINGS; DROP TABLE t1,t2,t3; - +# +# BUG#34768 - nondeterministic INSERT using LIMIT logged in stmt mode if +# binlog_format=mixed +# +CREATE TABLE t1(a INT, b INT, KEY(a), PRIMARY KEY(b)); +INSERT INTO t1 SELECT * FROM t1 LIMIT 1; +REPLACE INTO t1 SELECT * FROM t1 LIMIT 1; +UPDATE t1 SET a=1 LIMIT 1; +DELETE FROM t1 LIMIT 1; +delimiter |; +CREATE PROCEDURE p1() +BEGIN + INSERT INTO t1 SELECT * FROM t1 LIMIT 1; + REPLACE INTO t1 SELECT * FROM t1 LIMIT 1; + UPDATE t1 SET a=1 LIMIT 1; + DELETE FROM t1 LIMIT 1; +END| +delimiter ;| +CALL p1(); +DROP PROCEDURE p1; +DROP TABLE t1; diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index 990f7713561..abf25e96be0 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -418,6 +418,19 @@ bool mysql_prepare_delete(THD *thd, TABLE_LIST *table_list, Item **conds) DBUG_ENTER("mysql_prepare_delete"); List all_fields; + /* + Statement-based replication of DELETE ... LIMIT is not safe as order of + rows is not defined, so in mixed mode we go to row-based. + + Note that we may consider a statement as safe if ORDER BY primary_key + is present. However it may confuse users to see very similiar statements + replicated differently. + */ + if (thd->lex->current_select->select_limit) + { + thd->lex->set_stmt_unsafe(); + thd->set_current_stmt_binlog_row_based_if_mixed(); + } thd->lex->allow_sum_func= 0; if (setup_tables_and_check_access(thd, &thd->lex->select_lex.context, &thd->lex->select_lex.top_join_list, diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 2be932a6040..58acf40964b 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2763,6 +2763,19 @@ bool mysql_insert_select_prepare(THD *thd) TABLE_LIST *first_select_leaf_table; DBUG_ENTER("mysql_insert_select_prepare"); + /* + Statement-based replication of INSERT ... SELECT ... LIMIT is not safe + as order of rows is not defined, so in mixed mode we go to row-based. + + Note that we may consider a statement as safe if ORDER BY primary_key + is present or we SELECT a constant. However it may confuse users to + see very similiar statements replicated differently. + */ + if (lex->current_select->select_limit) + { + lex->set_stmt_unsafe(); + thd->set_current_stmt_binlog_row_based_if_mixed(); + } /* SELECT_LEX do not belong to INSERT statement, so we can't add WHERE clause if table is VIEW diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 7c9ead7591c..77d5fd7421c 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -859,6 +859,19 @@ bool mysql_prepare_update(THD *thd, TABLE_LIST *table_list, SELECT_LEX *select_lex= &thd->lex->select_lex; DBUG_ENTER("mysql_prepare_update"); + /* + Statement-based replication of UPDATE ... LIMIT is not safe as order of + rows is not defined, so in mixed mode we go to row-based. + + Note that we may consider a statement as safe if ORDER BY primary_key + is present. However it may confuse users to see very similiar statements + replicated differently. + */ + if (thd->lex->current_select->select_limit) + { + thd->lex->set_stmt_unsafe(); + thd->set_current_stmt_binlog_row_based_if_mixed(); + } #ifndef NO_EMBEDDED_ACCESS_CHECKS table_list->grant.want_privilege= table->grant.want_privilege= (SELECT_ACL & ~table->grant.privilege); From 95023bb9b0702cc9e080d67ab79cf56d2c12fd97 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 20 Mar 2008 11:40:26 +0400 Subject: [PATCH 2/5] BUG#34790 - 'create server' doesn't handle out of memory scenario well enough CREATE SERVER may cause server crash if there is not enough memory to execute this operation. Fixed that create_server() and prepare_server_struct_for_insert() didn't check return value of functions that allocate memory. As this is out of memory issue fix, not test case available. sql/sql_servers.cc: Fixed that create_server() and prepare_server_struct_for_insert() didn't check return value of functions that allocate memory. --- sql/sql_servers.cc | 68 ++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 26 deletions(-) diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index 8203ca92eed..6255901d782 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -55,8 +55,8 @@ static bool get_server_from_table_to_cache(TABLE *table); static int insert_server(THD *thd, FOREIGN_SERVER *server_options); static int insert_server_record(TABLE *table, FOREIGN_SERVER *server); static int insert_server_record_into_cache(FOREIGN_SERVER *server); -static void prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options, - FOREIGN_SERVER *server); +static FOREIGN_SERVER * +prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options); /* drop functions */ static int delete_server_record(TABLE *table, char *server_name, @@ -966,10 +966,14 @@ int create_server(THD *thd, LEX_SERVER_OPTIONS *server_options) server_options->server_name_length)) goto end; - server= (FOREIGN_SERVER *)alloc_root(&mem, - sizeof(FOREIGN_SERVER)); - prepare_server_struct_for_insert(server_options, server); + if (!(server= prepare_server_struct_for_insert(server_options))) + { + /* purecov: begin inspected */ + error= ER_OUT_OF_RESOURCES; + goto end; + /* purecov: end */ + } error= insert_server(thd, server); @@ -1040,52 +1044,64 @@ end: SYNOPSIS prepare_server_struct_for_insert() LEX_SERVER_OPTIONS *server_options - FOREIGN_SERVER *server NOTES + As FOREIGN_SERVER members are allocated on mem_root, we do not need to + free them in case of error. RETURN VALUE - none + On success filled FOREIGN_SERVER, or NULL in case out of memory. */ -static void -prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options, - FOREIGN_SERVER *server) +static FOREIGN_SERVER * +prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options) { char *unset_ptr= (char*)""; + FOREIGN_SERVER *server; DBUG_ENTER("prepare_server_struct"); + if (!(server= (FOREIGN_SERVER *)alloc_root(&mem, sizeof(FOREIGN_SERVER)))) + DBUG_RETURN(NULL); /* purecov: inspected */ + /* these two MUST be set */ - server->server_name= strdup_root(&mem, server_options->server_name); + if (!(server->server_name= strdup_root(&mem, server_options->server_name))) + DBUG_RETURN(NULL); /* purecov: inspected */ server->server_name_length= server_options->server_name_length; - server->host= server_options->host ? - strdup_root(&mem, server_options->host) : unset_ptr; + if (!(server->host= server_options->host ? + strdup_root(&mem, server_options->host) : unset_ptr)) + DBUG_RETURN(NULL); /* purecov: inspected */ - server->db= server_options->db ? - strdup_root(&mem, server_options->db) : unset_ptr; + if (!(server->db= server_options->db ? + strdup_root(&mem, server_options->db) : unset_ptr)) + DBUG_RETURN(NULL); /* purecov: inspected */ - server->username= server_options->username ? - strdup_root(&mem, server_options->username) : unset_ptr; + if (!(server->username= server_options->username ? + strdup_root(&mem, server_options->username) : unset_ptr)) + DBUG_RETURN(NULL); /* purecov: inspected */ - server->password= server_options->password ? - strdup_root(&mem, server_options->password) : unset_ptr; + if (!(server->password= server_options->password ? + strdup_root(&mem, server_options->password) : unset_ptr)) + DBUG_RETURN(NULL); /* purecov: inspected */ /* set to 0 if not specified */ server->port= server_options->port > -1 ? server_options->port : 0; - server->socket= server_options->socket ? - strdup_root(&mem, server_options->socket) : unset_ptr; + if (!(server->socket= server_options->socket ? + strdup_root(&mem, server_options->socket) : unset_ptr)) + DBUG_RETURN(NULL); /* purecov: inspected */ - server->scheme= server_options->scheme ? - strdup_root(&mem, server_options->scheme) : unset_ptr; + if (!(server->scheme= server_options->scheme ? + strdup_root(&mem, server_options->scheme) : unset_ptr)) + DBUG_RETURN(NULL); /* purecov: inspected */ - server->owner= server_options->owner ? - strdup_root(&mem, server_options->owner) : unset_ptr; + if (!(server->owner= server_options->owner ? + strdup_root(&mem, server_options->owner) : unset_ptr)) + DBUG_RETURN(NULL); /* purecov: inspected */ - DBUG_VOID_RETURN; + DBUG_RETURN(server); } /* From b31a1622e33b92d926e7e4346a027694156353c0 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 20 Mar 2008 11:57:30 +0400 Subject: [PATCH 3/5] BUG#34789 - drop server/create server leaks memory ! When CREATE SERVER is issued, it allocates memory on memory root to store cached server structure. When DROP SERVER is issued, it doesn't release this memory, as it is impossible with the memory root. We use the same allocation strategy for plugins and acl. The problem here that there was no way (except for the server restart) to force 'servers' code to release this memory. With this fix it is possible to release unused server cache memory by FLUSH PRIVILEGES. No test case for this fix. sql/sql_parse.cc: Reload servers table on FLUSH PRIVILEGES. sql/sql_servers.cc: Instead of just marking memory blocks as unused, release memory used by servers cache and initialize new memory root. This is needed for FLUSH PRIVILEGES to release unused memory blocks. --- sql/sql_parse.cc | 2 ++ sql/sql_servers.cc | 8 +++++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index f2e0065f860..046eae4e654 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -6486,6 +6486,8 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables, result= 1; if (grant_reload(thd)) result= 1; + if (servers_reload(thd)) + result= 1; } if (tmp_thd) { diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index 8203ca92eed..42abd9b4eb5 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -166,6 +166,9 @@ end: RETURN VALUES FALSE Success TRUE Error + + TODO + Revert back to old list if we failed to load new one. */ static bool servers_load(THD *thd, TABLE_LIST *tables) @@ -175,10 +178,9 @@ static bool servers_load(THD *thd, TABLE_LIST *tables) bool return_val= TRUE; DBUG_ENTER("servers_load"); - /* first, send all cached rows to sleep with the fishes, oblivion! - I expect this crappy comment replaced */ - free_root(&mem, MYF(MY_MARK_BLOCKS_FREE)); my_hash_reset(&servers_cache); + free_root(&mem, MYF(0)); + init_alloc_root(&mem, ACL_ALLOC_BLOCK_SIZE, 0); init_read_record(&read_record_info,thd,table=tables[0].table,NULL,1,0); while (!(read_record_info.read_record(&read_record_info))) From 2b2ae926f439980f6de50bbae25b37b8b141992f Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 25 Mar 2008 17:28:12 +0400 Subject: [PATCH 4/5] BUG#34768 - nondeterministic INSERT using LIMIT logged in stmt mode if binlog_format=mixed Addition to fix for BUG#34768: fixed test case failures discovered by pushbuild. mysql-test/suite/binlog/r/binlog_stm_ps.result: Fixed binlog_stm_ps failure when binlog format is statement: - this test case makes sence only if binlog format is statement, thus execute it only in this mode; - added a warning that insert ... select ... limit is not safe to execute in statement mode. mysql-test/suite/binlog/t/binlog_stm_ps.test: Fixed binlog_stm_ps failure when binlog format is statement: - this test case makes sence only if binlog format is statement, thus execute it only in this mode; - added a warning that insert ... select ... limit is not safe to execute in statement mode. mysql-test/suite/rpl/t/rpl_optimize.test: rpl_optimize test may be executed in various binlog format modes. In statement mode delete ... limit issues a warning, in mixed and row modes it does not. Fixed a test case so it is still possible to execute it in all binlog format modes by ignoring delete ... limit warnings. mysql-test/suite/rpl/t/rpl_user_variables.test: rpl_user_variables test may be executed in various binlog format modes. In statement mode insert ... select ... limit issues a warning, in mixed and row modes it does not. Fixed a test case so it is still possible to execute it in all binlog format modes by ignoring insert ... select ... limit warnings. --- mysql-test/suite/binlog/r/binlog_stm_ps.result | 5 +++-- mysql-test/suite/binlog/t/binlog_stm_ps.test | 2 +- mysql-test/suite/rpl/t/rpl_optimize.test | 2 ++ mysql-test/suite/rpl/t/rpl_user_variables.test | 2 ++ 4 files changed, 8 insertions(+), 3 deletions(-) diff --git a/mysql-test/suite/binlog/r/binlog_stm_ps.result b/mysql-test/suite/binlog/r/binlog_stm_ps.result index 2128367deb5..1cf7429987e 100644 --- a/mysql-test/suite/binlog/r/binlog_stm_ps.result +++ b/mysql-test/suite/binlog/r/binlog_stm_ps.result @@ -10,12 +10,13 @@ execute s using @a; prepare s from "insert into t1 select 100 limit ?"; set @a=100; execute s using @a; +Warnings: +Warning 1592 Statement is not safe to log in statement format. show binlog events from ; Log_name Pos Event_type Server_id End_log_pos Info master-bin.000001 # Query # # use `test`; create table t1 (a int) master-bin.000001 # User var # # @`a`=98 master-bin.000001 # Query # # use `test`; insert into t1 values (@a),(98) master-bin.000001 # Query # # use `test`; insert into t1 values (99) -master-bin.000001 # Table_map # # table_id: # (test.t1) -master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F +master-bin.000001 # Query # # use `test`; insert into t1 select 100 limit 100 drop table t1; diff --git a/mysql-test/suite/binlog/t/binlog_stm_ps.test b/mysql-test/suite/binlog/t/binlog_stm_ps.test index 83add5af3d7..55e3d30ad23 100644 --- a/mysql-test/suite/binlog/t/binlog_stm_ps.test +++ b/mysql-test/suite/binlog/t/binlog_stm_ps.test @@ -1,7 +1,7 @@ # This test is to verify replication with PS -- source include/not_embedded.inc --- source include/have_binlog_format_mixed_or_statement.inc +-- source include/have_binlog_format_statement.inc -- disable_query_log reset master; # get rid of previous tests binlog diff --git a/mysql-test/suite/rpl/t/rpl_optimize.test b/mysql-test/suite/rpl/t/rpl_optimize.test index 80f0c052fc8..f4582ba1167 100644 --- a/mysql-test/suite/rpl/t/rpl_optimize.test +++ b/mysql-test/suite/rpl/t/rpl_optimize.test @@ -31,7 +31,9 @@ INSERT INTO t1 (a) SELECT null FROM t1; save_master_pos; # a few updates to force OPTIMIZE to do something update t1 set b=(a/2*rand()); +--disable_warnings delete from t1 order by b limit 10000; +--enable_warnings connection slave; sync_with_master; diff --git a/mysql-test/suite/rpl/t/rpl_user_variables.test b/mysql-test/suite/rpl/t/rpl_user_variables.test index 21b2063c9f2..8d570f28f64 100644 --- a/mysql-test/suite/rpl/t/rpl_user_variables.test +++ b/mysql-test/suite/rpl/t/rpl_user_variables.test @@ -315,7 +315,9 @@ create table t1(a int); insert into t1 values (1),(2); prepare s1 from 'insert into t1 select a from t1 limit ?'; set @x='1.1'; +--disable_warnings execute s1 using @x; +--enable_warnings select * from t1; sync_slave_with_master; connection slave; From 811fb145eaf694485395934c81c7576a000e2cc7 Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 25 Mar 2008 17:37:53 +0400 Subject: [PATCH 5/5] BUG#34789 - drop server/create server leaks memory ! BUG#34790 - 'create server' doesn't handle out of memory scenario well enough This is an addition to fixes for these bugs, which makes gcov happy. mysql-test/r/federated.result: CREATE SERVER is only tested by federated_server.test, which requires big-test option. Added dummy test case to make gcov happy. mysql-test/t/federated.test: CREATE SERVER is only tested by federated_server.test, which requires big-test option. Added dummy test case to make gcov happy. sql/sql_parse.cc: Make gcov happy. --- mysql-test/r/federated.result | 2 ++ mysql-test/t/federated.test | 6 ++++++ sql/sql_parse.cc | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/federated.result b/mysql-test/r/federated.result index 65fafcb2166..21743c3a8a6 100644 --- a/mysql-test/r/federated.result +++ b/mysql-test/r/federated.result @@ -2092,6 +2092,8 @@ DROP TABLE t1; DROP TABLE t1; CREATE TABLE t1 (a INT) ENGINE=federated CONNECTION='mysql://@:://'; DROP TABLE t1; +create server 's1' foreign data wrapper 'mysql' options (port 3306); +drop server 's1'; End of 5.1 tests DROP TABLE IF EXISTS federated.t1; DROP DATABASE IF EXISTS federated; diff --git a/mysql-test/t/federated.test b/mysql-test/t/federated.test index 0fd2290d619..6540cf8648e 100644 --- a/mysql-test/t/federated.test +++ b/mysql-test/t/federated.test @@ -1823,5 +1823,11 @@ DROP TABLE t1; CREATE TABLE t1 (a INT) ENGINE=federated CONNECTION='mysql://@:://'; DROP TABLE t1; +# +# Coverage testing of CREATE SERVER. +# +create server 's1' foreign data wrapper 'mysql' options (port 3306); +drop server 's1'; + --echo End of 5.1 tests source include/federated_cleanup.inc; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 046eae4e654..a2708632a56 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -6487,7 +6487,7 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables, if (grant_reload(thd)) result= 1; if (servers_reload(thd)) - result= 1; + result= 1; /* purecov: inspected */ } if (tmp_thd) {