From ef885b8e7e0c04f3d7ede7f72833c80abdf5d5fe Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Wed, 7 Mar 2007 16:30:13 +0100 Subject: [PATCH 01/13] Bug#26782 - Patch: myisamchk -rq creates .MYI.MYI file on packed MyISAM tables When fixing the indexes with "myisamchk -rq" after compressing the table with "myisampack", an optionally supplied extension ".MYI" of the index file was not detected. The extension was appended unconditionally. The result was ".MYI.MYI". Now an extension is no longer appended if present already. Thanks to David Shrewsbury for providing this patch. Another problem was a misplaced parenthesis. We did never unpack the file name ("~/..") and always returned a real path. No test case. This is manually tested with the utilities "myisampack" and "myisamchk". --- storage/myisam/mi_create.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/storage/myisam/mi_create.c b/storage/myisam/mi_create.c index 2b8cbcc7da5..e2245aff065 100644 --- a/storage/myisam/mi_create.c +++ b/storage/myisam/mi_create.c @@ -599,10 +599,12 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, } else { + char *iext= strrchr(name, '.'); + int have_iext= iext && !strcmp(iext, MI_NAME_IEXT); fn_format(filename, name, "", MI_NAME_IEXT, - (MY_UNPACK_FILENAME | - (flags & HA_DONT_TOUCH_DATA) ? MY_RETURN_REAL_PATH : 0) | - MY_APPEND_EXT); + MY_UNPACK_FILENAME | + ((flags & HA_DONT_TOUCH_DATA) ? MY_RETURN_REAL_PATH : 0) | + (have_iext ? MY_REPLACE_EXT : MY_APPEND_EXT)); linkname_ptr=0; /* Replace the current file */ create_flag=MY_DELETE_OLD; From e4d93c6bcde8280e38f8d848392a6d2e5391da63 Mon Sep 17 00:00:00 2001 From: "acurtis/antony@xiphis.org/ltamd64.xiphis.org" <> Date: Tue, 13 Mar 2007 11:58:24 -0700 Subject: [PATCH 02/13] Bug#25671 "CREATE/DROP/ALTER SERVER should require privileges" Add check for SUPER privilege when executing CREATE/DROP/ALTER SERVER. Previously, any user even with only USAGE priv can execute those commands. --- mysql-test/r/federated_server.result | 85 ++++++++++++++++++ mysql-test/t/federated_server.test | 127 +++++++++++++++++++++++++++ sql/sql_parse.cc | 12 +++ 3 files changed, 224 insertions(+) diff --git a/mysql-test/r/federated_server.result b/mysql-test/r/federated_server.result index 7a1a6e0970d..3682c0c793f 100644 --- a/mysql-test/r/federated_server.result +++ b/mysql-test/r/federated_server.result @@ -104,6 +104,91 @@ drop table first_db.t1; drop table second_db.t1; drop database first_db; drop database second_db; +create database db_legitimate; +create database db_bogus; +use db_legitimate; +CREATE TABLE db_legitimate.t1 ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ); +INSERT INTO db_legitimate.t1 VALUES ('1','this is legitimate'); +use db_bogus; +CREATE TABLE db_bogus.t1 ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ) +; +INSERT INTO db_bogus.t1 VALUES ('2','this is bogus'); +create server 's1' foreign data wrapper 'mysql' options +(HOST '127.0.0.1', +DATABASE 'db_legitimate', +USER 'root', +PASSWORD '', +PORT SLAVE_PORT, +SOCKET '', +OWNER 'root'); +create user guest_select@localhost; +grant select on federated.* to guest_select@localhost; +create user guest_super@localhost; +grant select,SUPER,RELOAD on *.* to guest_super@localhost; +create user guest_usage@localhost; +grant usage on *.* to guest_usage@localhost; +CREATE TABLE federated.t1 ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ) ENGINE = FEDERATED CONNECTION = 's1'; +select * from federated.t1; +id name +1 this is legitimate +alter server s1 options (database 'db_bogus'); +ERROR 42000: Access denied; you need the SUPER privilege for this operation +flush tables; +select * from federated.t1; +id name +1 this is legitimate +alter server s1 options (database 'db_bogus'); +ERROR 42000: Access denied; you need the SUPER privilege for this operation +flush tables; +select * from federated.t1; +id name +1 this is legitimate +alter server s1 options (database 'db_bogus'); +flush tables; +select * from federated.t1; +id name +2 this is bogus +drop server if exists 's1'; +ERROR 42000: Access denied; you need the SUPER privilege for this operation +create server 's1' foreign data wrapper 'mysql' options +(HOST '127.0.0.1', +DATABASE 'db_legitimate', +USER 'root', +PASSWORD '', +PORT SLAVE_PORT, +SOCKET '', +OWNER 'root'); +ERROR 42000: Access denied; you need the SUPER privilege for this operation +drop server 's1'; +create server 's1' foreign data wrapper 'mysql' options +(HOST '127.0.0.1', +DATABASE 'db_legitimate', +USER 'root', +PASSWORD '', +PORT SLAVE_PORT, +SOCKET '', +OWNER 'root'); +flush tables; +select * from federated.t1; +id name +1 this is legitimate +drop database db_legitimate; +drop database db_bogus; +drop user guest_super@localhost; +drop user guest_usage@localhost; +drop user guest_select@localhost; +drop table federated.t1; +drop server 's1'; +# End of 5.1 tests DROP TABLE IF EXISTS federated.t1; DROP DATABASE IF EXISTS federated; DROP TABLE IF EXISTS federated.t1; diff --git a/mysql-test/t/federated_server.test b/mysql-test/t/federated_server.test index 3e47d9bc95d..e65f319f98d 100644 --- a/mysql-test/t/federated_server.test +++ b/mysql-test/t/federated_server.test @@ -107,4 +107,131 @@ drop table second_db.t1; drop database first_db; drop database second_db; +# +# Bug#25671 - CREATE/DROP/ALTER SERVER should require privileges +# +# Changes to SERVER declarations should require SUPER privilege. +# Based upon test case by Giuseppe Maxia + +create database db_legitimate; +create database db_bogus; + +use db_legitimate; +CREATE TABLE db_legitimate.t1 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ); +INSERT INTO db_legitimate.t1 VALUES ('1','this is legitimate'); + +use db_bogus; +CREATE TABLE db_bogus.t1 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) + ; +INSERT INTO db_bogus.t1 VALUES ('2','this is bogus'); + +connection master; +--replace_result $SLAVE_MYPORT SLAVE_PORT +eval create server 's1' foreign data wrapper 'mysql' options + (HOST '127.0.0.1', + DATABASE 'db_legitimate', + USER 'root', + PASSWORD '', + PORT $SLAVE_MYPORT, + SOCKET '', + OWNER 'root'); + +create user guest_select@localhost; +grant select on federated.* to guest_select@localhost; + +create user guest_super@localhost; +grant select,SUPER,RELOAD on *.* to guest_super@localhost; + +create user guest_usage@localhost; +grant usage on *.* to guest_usage@localhost; + +CREATE TABLE federated.t1 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) ENGINE = FEDERATED CONNECTION = 's1'; + +select * from federated.t1; + +connect (conn_select,127.0.0.1,guest_select,,federated,$MASTER_MYPORT); +connect (conn_usage,127.0.0.1,guest_usage,,,$MASTER_MYPORT); +connect (conn_super,127.0.0.1,guest_super,,,$MASTER_MYPORT); + +connection conn_select; +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +alter server s1 options (database 'db_bogus'); + +connection master; +flush tables; +select * from federated.t1; + +connection conn_usage; +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +alter server s1 options (database 'db_bogus'); + +connection master; +flush tables; +select * from federated.t1; + +connection conn_super; +alter server s1 options (database 'db_bogus'); + +connection master; +flush tables; +select * from federated.t1; + +connection conn_select; +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +drop server if exists 's1'; +--replace_result $SLAVE_MYPORT SLAVE_PORT +--error ER_SPECIFIC_ACCESS_DENIED_ERROR +eval create server 's1' foreign data wrapper 'mysql' options + (HOST '127.0.0.1', + DATABASE 'db_legitimate', + USER 'root', + PASSWORD '', + PORT $SLAVE_MYPORT, + SOCKET '', + OWNER 'root'); + +connection conn_super; +drop server 's1'; +--replace_result $SLAVE_MYPORT SLAVE_PORT +eval create server 's1' foreign data wrapper 'mysql' options + (HOST '127.0.0.1', + DATABASE 'db_legitimate', + USER 'root', + PASSWORD '', + PORT $SLAVE_MYPORT, + SOCKET '', + OWNER 'root'); + +connection master; +flush tables; +select * from federated.t1; + +# clean up test +connection slave; +drop database db_legitimate; +drop database db_bogus; + +disconnect conn_select; +disconnect conn_usage; +disconnect conn_super; + +connection master; +drop user guest_super@localhost; +drop user guest_usage@localhost; +drop user guest_select@localhost; +drop table federated.t1; +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 bfcbd4663b4..6684b741464 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4273,6 +4273,10 @@ create_sp_error: int error; LEX *lex= thd->lex; DBUG_PRINT("info", ("case SQLCOM_CREATE_SERVER")); + + if (check_global_access(thd, SUPER_ACL)) + break; + if ((error= create_server(thd, &lex->server_options))) { DBUG_PRINT("info", ("problem creating server <%s>", @@ -4288,6 +4292,10 @@ create_sp_error: int error; LEX *lex= thd->lex; DBUG_PRINT("info", ("case SQLCOM_ALTER_SERVER")); + + if (check_global_access(thd, SUPER_ACL)) + break; + if ((error= alter_server(thd, &lex->server_options))) { DBUG_PRINT("info", ("problem altering server <%s>", @@ -4303,6 +4311,10 @@ create_sp_error: int err_code; LEX *lex= thd->lex; DBUG_PRINT("info", ("case SQLCOM_DROP_SERVER")); + + if (check_global_access(thd, SUPER_ACL)) + break; + if ((err_code= drop_server(thd, &lex->server_options))) { if (! lex->drop_if_exists && err_code == ER_FOREIGN_SERVER_EXISTS) From 6b7fea5f2b19ad04322026ffef74342d18412167 Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Wed, 14 Mar 2007 16:27:37 +0100 Subject: [PATCH 03/13] Bug#25289 - repair table causes "my_seek.c:56: my_seek: Assertion `fd != -1' failed" In difficult optimize/repair situations the server could crash. Under some circumstances the server retries an optimize/repair with more elaborate options. But it did not check if the first attempt failed so badly that a second one must not be tried. This could happen when a new data file has been created but it was not possible to open it. In this case the repair leaves behind a table with closed data file. This must not be used for another repair attempt. We do now detect the closed data file and do not try another repair attempt in this situation. No test case. The required table corruption can not be repeated easily. There is a test program attached to bug 25433. --- sql/ha_myisam.cc | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/sql/ha_myisam.cc b/sql/ha_myisam.cc index 2cec03a51db..42be008de9e 100644 --- a/sql/ha_myisam.cc +++ b/sql/ha_myisam.cc @@ -867,6 +867,22 @@ int ha_myisam::repair(THD *thd, MI_CHECK ¶m, bool optimize) ha_rows rows= file->state->records; DBUG_ENTER("ha_myisam::repair"); + /* + Normally this method is entered with a properly opened table. If the + repair fails, it can be repeated with more elaborate options. Under + special circumstances it can happen that a repair fails so that it + closed the data file and cannot re-open it. In this case file->dfile + is set to -1. We must not try another repair without an open data + file. (Bug #25289) + */ + if (file->dfile == -1) + { + sql_print_information("Retrying repair of: '%s' failed. " + "Please try REPAIR EXTENDED or myisamchk", + table->path); + DBUG_RETURN(HA_ADMIN_FAILED); + } + param.db_name = table->table_cache_key; param.table_name = table->table_name; param.tmpfile_createflag = O_RDWR | O_TRUNC; From 344f33bb89d60f83e7ec7fdc2275f88dc9468523 Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Mon, 19 Mar 2007 15:56:53 +0100 Subject: [PATCH 04/13] Bug#26996 - Update of a Field in a Memory Table ends with wrong result Using a MEMORY table BTREE index for scanning for updatable rows could lead to an infinite loop. Everytime a key was inserted into a btree index, the position in the index scan was cleared. The search started from the beginning and found the same key again. Now we do not clear the position on key insert an more. --- heap/hp_write.c | 1 - mysql-test/r/heap_btree.result | 15 +++++++++++++++ mysql-test/t/heap_btree.test | 14 ++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/heap/hp_write.c b/heap/hp_write.c index 841dda6264e..59dfca31fd9 100644 --- a/heap/hp_write.c +++ b/heap/hp_write.c @@ -106,7 +106,6 @@ int hp_rb_write_key(HP_INFO *info, HP_KEYDEF *keyinfo, const byte *record, heap_rb_param custom_arg; uint old_allocated; - info->last_pos= NULL; /* For heap_rnext/heap_rprev */ custom_arg.keyseg= keyinfo->seg; custom_arg.key_length= hp_rb_make_key(keyinfo, info->recbuf, record, recpos); if (keyinfo->flag & HA_NOSAME) diff --git a/mysql-test/r/heap_btree.result b/mysql-test/r/heap_btree.result index e6492e90b80..1a0666514be 100644 --- a/mysql-test/r/heap_btree.result +++ b/mysql-test/r/heap_btree.result @@ -280,4 +280,19 @@ a 1 1 drop table t1; +CREATE TABLE t1 ( +c1 CHAR(3), +c2 INTEGER, +KEY USING BTREE(c1), +KEY USING BTREE(c2) +) ENGINE= MEMORY; +INSERT INTO t1 VALUES ('ABC',0), ('A',0), ('B',0), ('C',0); +UPDATE t1 SET c2= c2 + 1 WHERE c1 = 'A'; +SELECT * FROM t1; +c1 c2 +ABC 0 +A 1 +B 0 +C 0 +DROP TABLE t1; End of 4.1 tests diff --git a/mysql-test/t/heap_btree.test b/mysql-test/t/heap_btree.test index 9aa820becd9..63baa968981 100644 --- a/mysql-test/t/heap_btree.test +++ b/mysql-test/t/heap_btree.test @@ -182,4 +182,18 @@ delete from t1 where a >= 2; select a from t1 order by a; drop table t1; +# +# Bug#26996 - Update of a Field in a Memory Table ends with wrong result +# +CREATE TABLE t1 ( + c1 CHAR(3), + c2 INTEGER, + KEY USING BTREE(c1), + KEY USING BTREE(c2) +) ENGINE= MEMORY; +INSERT INTO t1 VALUES ('ABC',0), ('A',0), ('B',0), ('C',0); +UPDATE t1 SET c2= c2 + 1 WHERE c1 = 'A'; +SELECT * FROM t1; +DROP TABLE t1; + --echo End of 4.1 tests From b88e7e0f975e07303d1d246eb9641e49db9b69af Mon Sep 17 00:00:00 2001 From: "svoj@mysql.com/june.mysql.com" <> Date: Wed, 21 Mar 2007 17:12:30 +0400 Subject: [PATCH 05/13] BUG#25908 - corrupted myisam table crashes server even after repair Opening certain tables that have different definitions in .MYI and .frm may result in a server crash. Compare .MYI and .frm definition when myisam table is opened. In case definitions are diffirent refuse to open such table. No test case, since it requires broken table. --- storage/myisam/ha_myisam.cc | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index a67d62f7447..587e5220e1c 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -632,6 +632,9 @@ bool ha_myisam::check_if_locking_is_allowed(uint sql_command, int ha_myisam::open(const char *name, int mode, uint test_if_locked) { + MI_KEYDEF *keyinfo; + MI_COLUMNDEF *recinfo= 0; + uint recs; uint i; /* @@ -654,6 +657,26 @@ int ha_myisam::open(const char *name, int mode, uint test_if_locked) if (!(file=mi_open(name, mode, test_if_locked | HA_OPEN_FROM_SQL_LAYER))) return (my_errno ? my_errno : -1); + if (!table->s->tmp_table) /* No need to perform a check for tmp table */ + { + if ((my_errno= table2myisam(table, &keyinfo, &recinfo, &recs))) + { + /* purecov: begin inspected */ + DBUG_PRINT("error", ("Failed to convert TABLE object to MyISAM " + "key and column definition")); + goto err; + /* purecov: end */ + } + if (check_definition(keyinfo, recinfo, table->s->keys, recs, + file->s->keyinfo, file->s->rec, + file->s->base.keys, file->s->base.fields, true)) + { + /* purecov: begin inspected */ + my_errno= HA_ERR_CRASHED; + goto err; + /* purecov: end */ + } + } if (test_if_locked & (HA_OPEN_IGNORE_IF_LOCKED | HA_OPEN_TMP_TABLE)) VOID(mi_extra(file, HA_EXTRA_NO_WAIT_LOCK, 0)); @@ -675,6 +698,15 @@ int ha_myisam::open(const char *name, int mode, uint test_if_locked) table->key_info[i].block_size= file->s->keyinfo[i].block_length; } return (0); +err: + this->close(); + /* + Both recinfo and keydef are allocated by my_multi_malloc(), thus only + recinfo must be freed. + */ + if (recinfo) + my_free((gptr) recinfo, MYF(0)); + return my_errno; } int ha_myisam::close(void) From 52debcb82097b49d2316303c8196ed9459009411 Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Thu, 22 Mar 2007 21:28:28 +0100 Subject: [PATCH 06/13] After merge fix --- storage/myisam/ha_myisam.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index 6f4d12ec5f7..505febf8828 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -697,9 +697,11 @@ int ha_myisam::open(const char *name, int mode, uint test_if_locked) (struct st_mysql_ftparser *)parser->plugin->info; table->key_info[i].block_size= file->s->keyinfo[i].block_length; } - return (0); -err: + my_errno= 0; + goto end; + err: this->close(); + end: /* Both recinfo and keydef are allocated by my_multi_malloc(), thus only recinfo must be freed. From 3a726ab6e2eb2b1b2962d1f611a9d495a96db4e4 Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Fri, 23 Mar 2007 10:26:14 +0100 Subject: [PATCH 07/13] Bug#26782 - Patch: myisamchk -rq creates .MYI.MYI file on packed MyISAM tables After merge fix In conjunction with the newest 5.1 we always need to create a real path name to be able to detect already open tables. --- storage/myisam/mi_create.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/storage/myisam/mi_create.c b/storage/myisam/mi_create.c index 5ce07c5ac4e..71d377c8b6b 100644 --- a/storage/myisam/mi_create.c +++ b/storage/myisam/mi_create.c @@ -573,6 +573,10 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, pthread_mutex_lock(&THR_LOCK_myisam); + /* + NOTE: For test_if_reopen() we need a real path name. Hence we need + MY_RETURN_REAL_PATH for every fn_format(filename, ...). + */ if (ci->index_file_name) { char *iext= strrchr(ci->index_file_name, '.'); @@ -584,13 +588,14 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, if ((path= strrchr(ci->index_file_name, FN_LIBCHAR))) *path= '\0'; fn_format(filename, name, ci->index_file_name, MI_NAME_IEXT, - MY_REPLACE_DIR | MY_UNPACK_FILENAME | MY_APPEND_EXT); + MY_REPLACE_DIR | MY_UNPACK_FILENAME | + MY_RETURN_REAL_PATH | MY_APPEND_EXT); } else { fn_format(filename, ci->index_file_name, "", MI_NAME_IEXT, - MY_UNPACK_FILENAME | (have_iext ? MY_REPLACE_EXT : - MY_APPEND_EXT)); + MY_UNPACK_FILENAME | MY_RETURN_REAL_PATH | + (have_iext ? MY_REPLACE_EXT : MY_APPEND_EXT)); } fn_format(linkname, name, "", MI_NAME_IEXT, MY_UNPACK_FILENAME|MY_APPEND_EXT); @@ -606,8 +611,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, char *iext= strrchr(name, '.'); int have_iext= iext && !strcmp(iext, MI_NAME_IEXT); fn_format(filename, name, "", MI_NAME_IEXT, - MY_UNPACK_FILENAME | - ((flags & HA_DONT_TOUCH_DATA) ? MY_RETURN_REAL_PATH : 0) | + MY_UNPACK_FILENAME | MY_RETURN_REAL_PATH | (have_iext ? MY_REPLACE_EXT : MY_APPEND_EXT)); linkname_ptr=0; /* Replace the current file */ @@ -620,6 +624,9 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, A TRUNCATE command checks for the table in the cache only and could be fooled to believe, the table is not open. Pull the emergency brake in this situation. (Bug #8306) + + NOTE: The filename is compared against unique_file_name of every + open table. Hence we need a real path here. */ if (test_if_reopen(filename)) { From 14ccc65994e5fead595aecb767851aed93348c8c Mon Sep 17 00:00:00 2001 From: "acurtis/antony@xiphis.org/ltamd64.xiphis.org" <> Date: Fri, 23 Mar 2007 17:31:27 -0700 Subject: [PATCH 08/13] Bug#25721 "Concurrent ALTER/CREATE SERVER can lead to deadlock" Deadlock caused by inconsistant use of mutexes in sql_server.cc One mutex has been removed to resolve deadlock. Many functions were made private which should not be exported. Unused variables and function removed. --- mysql-test/r/federated_server.result | 25 ++ mysql-test/t/federated_server.test | 35 +++ sql/sql_parse.cc | 2 +- sql/sql_servers.cc | 361 ++++++++++++--------------- sql/sql_servers.h | 30 +-- 5 files changed, 226 insertions(+), 227 deletions(-) diff --git a/mysql-test/r/federated_server.result b/mysql-test/r/federated_server.result index 3682c0c793f..8faf33581d1 100644 --- a/mysql-test/r/federated_server.result +++ b/mysql-test/r/federated_server.result @@ -189,6 +189,31 @@ drop user guest_select@localhost; drop table federated.t1; drop server 's1'; # End of 5.1 tests +use test; +create procedure p1 () +begin +DECLARE v INT DEFAULT 0; +DECLARE e INT DEFAULT 0; +DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET e = e + 1; +WHILE v < 10000 do +CREATE SERVER s +FOREIGN DATA WRAPPER mysql +OPTIONS (USER 'Remote', HOST '192.168.1.106', DATABASE 'test'); +ALTER SERVER s OPTIONS (USER 'Remote'); +DROP SERVER s; +SET v = v + 1; +END WHILE; +SELECT e > 0; +END// +use test; +call p1(); +call p1(); +e > 0 +1 +e > 0 +1 +drop procedure p1; +drop server if exists s; DROP TABLE IF EXISTS federated.t1; DROP DATABASE IF EXISTS federated; DROP TABLE IF EXISTS federated.t1; diff --git a/mysql-test/t/federated_server.test b/mysql-test/t/federated_server.test index e65f319f98d..27dd494e5c5 100644 --- a/mysql-test/t/federated_server.test +++ b/mysql-test/t/federated_server.test @@ -234,4 +234,39 @@ drop server 's1'; --echo # End of 5.1 tests + +# +# Bug#25721 - deadlock with ALTER/CREATE SERVER +# +connect (other,localhost,root,,); +connection master; +use test; +delimiter //; +create procedure p1 () +begin + DECLARE v INT DEFAULT 0; + DECLARE e INT DEFAULT 0; + DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET e = e + 1; + WHILE v < 10000 do + CREATE SERVER s + FOREIGN DATA WRAPPER mysql + OPTIONS (USER 'Remote', HOST '192.168.1.106', DATABASE 'test'); + ALTER SERVER s OPTIONS (USER 'Remote'); + DROP SERVER s; + SET v = v + 1; + END WHILE; + SELECT e > 0; +END// +delimiter ;// +connection other; +use test; +send call p1(); +connection master; +call p1(); +connection other; +reap; +drop procedure p1; +drop server if exists s; + + source include/federated_cleanup.inc; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index a0e412afdf0..23bbeb594d5 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -4320,7 +4320,7 @@ create_sp_error: if ((err_code= drop_server(thd, &lex->server_options))) { - if (! lex->drop_if_exists && err_code == ER_FOREIGN_SERVER_EXISTS) + if (! lex->drop_if_exists && err_code == ER_FOREIGN_SERVER_DOESNT_EXIST) { DBUG_PRINT("info", ("problem dropping server %s", lex->server_options.server_name)); diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index 9a5176d9fe3..9b01a1b6889 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -25,14 +25,43 @@ #include "sp_head.h" #include "sp.h" -HASH servers_cache; -pthread_mutex_t servers_cache_mutex; // To init the hash -uint servers_cache_initialised=FALSE; -/* Version of server table. incremented by servers_load */ -static uint servers_version=0; +/* + We only use 1 mutex to guard the data structures - THR_LOCK_servers. + Read locked when only reading data and write-locked for all other access. +*/ + +static HASH servers_cache; static MEM_ROOT mem; static rw_lock_t THR_LOCK_servers; +static bool get_server_from_table_to_cache(TABLE *table); + +/* insert functions */ +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); +/* drop functions */ +static int delete_server_record(TABLE *table, + char *server_name, + int server_name_length); +static int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options); + +/* update functions */ +static void prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, + FOREIGN_SERVER *existing, + FOREIGN_SERVER *altered); +static int update_server(THD *thd, FOREIGN_SERVER *existing, + FOREIGN_SERVER *altered); +static int update_server_record(TABLE *table, FOREIGN_SERVER *server); +static int update_server_record_in_cache(FOREIGN_SERVER *existing, + FOREIGN_SERVER *altered); +/* utility functions */ +static void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to); + + + static byte *servers_cache_get_key(FOREIGN_SERVER *server, uint *length, my_bool not_used __attribute__((unused))) { @@ -45,6 +74,7 @@ static byte *servers_cache_get_key(FOREIGN_SERVER *server, uint *length, DBUG_RETURN((byte*) server->server_name); } + /* Initialize structures responsible for servers used in federated server scheme information for them from the server @@ -64,35 +94,27 @@ static byte *servers_cache_get_key(FOREIGN_SERVER *server, uint *length, 1 Could not initialize servers */ -my_bool servers_init(bool dont_read_servers_table) +bool servers_init(bool dont_read_servers_table) { THD *thd; - my_bool return_val= 0; + bool return_val= FALSE; DBUG_ENTER("servers_init"); /* init the mutex */ - if (pthread_mutex_init(&servers_cache_mutex, MY_MUTEX_INIT_FAST)) - DBUG_RETURN(1); - if (my_rwlock_init(&THR_LOCK_servers, NULL)) - DBUG_RETURN(1); + DBUG_RETURN(TRUE); /* initialise our servers cache */ if (hash_init(&servers_cache, system_charset_info, 32, 0, 0, (hash_get_key) servers_cache_get_key, 0, 0)) { - return_val= 1; /* we failed, out of memory? */ + return_val= TRUE; /* we failed, out of memory? */ goto end; } /* Initialize the mem root for data */ init_alloc_root(&mem, ACL_ALLOC_BLOCK_SIZE, 0); - /* - at this point, the cache is initialised, let it be known - */ - servers_cache_initialised= TRUE; - if (dont_read_servers_table) goto end; @@ -100,7 +122,7 @@ my_bool servers_init(bool dont_read_servers_table) To be able to run this from boot, we allocate a temporary THD */ if (!(thd=new THD)) - DBUG_RETURN(1); + DBUG_RETURN(TRUE); thd->thread_stack= (char*) &thd; thd->store_globals(); /* @@ -130,19 +152,13 @@ end: TRUE Error */ -static my_bool servers_load(THD *thd, TABLE_LIST *tables) +static bool servers_load(THD *thd, TABLE_LIST *tables) { TABLE *table; READ_RECORD read_record_info; - my_bool return_val= TRUE; + bool return_val= TRUE; DBUG_ENTER("servers_load"); - if (!servers_cache_initialised) - DBUG_RETURN(0); - - /* need to figure out how to utilise this variable */ - servers_version++; /* servers updated */ - /* 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)); @@ -156,7 +172,7 @@ static my_bool servers_load(THD *thd, TABLE_LIST *tables) goto end; } - return_val=0; + return_val= FALSE; end: end_read_record(&read_record_info); @@ -183,10 +199,10 @@ end: TRUE Failure */ -my_bool servers_reload(THD *thd) +bool servers_reload(THD *thd) { TABLE_LIST tables[1]; - my_bool return_val= 1; + bool return_val= TRUE; DBUG_ENTER("servers_reload"); if (thd->locked_tables) @@ -196,10 +212,9 @@ my_bool servers_reload(THD *thd) close_thread_tables(thd); } - /* - To avoid deadlocks we should obtain table locks before - obtaining servers_cache->lock mutex. - */ + DBUG_PRINT("info", ("locking servers_cache")); + rw_wrlock(&THR_LOCK_servers); + bzero((char*) tables, sizeof(tables)); tables[0].alias= tables[0].table_name= (char*) "servers"; tables[0].db= (char*) "mysql"; @@ -212,12 +227,6 @@ my_bool servers_reload(THD *thd) goto end; } - DBUG_PRINT("info", ("locking servers_cache")); - VOID(pthread_mutex_lock(&servers_cache_mutex)); - - //old_servers_cache= servers_cache; - //old_mem=mem; - if ((return_val= servers_load(thd, tables))) { // Error. Revert to old list /* blast, for now, we have no servers, discuss later way to preserve */ @@ -226,14 +235,14 @@ my_bool servers_reload(THD *thd) servers_free(); } - DBUG_PRINT("info", ("unlocking servers_cache")); - VOID(pthread_mutex_unlock(&servers_cache_mutex)); - end: close_thread_tables(thd); + DBUG_PRINT("info", ("unlocking servers_cache")); + rw_unlock(&THR_LOCK_servers); DBUG_RETURN(return_val); } + /* Initialize structures responsible for servers used in federated server scheme information for them from the server @@ -260,7 +269,8 @@ end: 1 could not insert server struct into global servers cache */ -my_bool get_server_from_table_to_cache(TABLE *table) +static bool +get_server_from_table_to_cache(TABLE *table) { /* alloc a server struct */ char *ptr; @@ -308,69 +318,6 @@ my_bool get_server_from_table_to_cache(TABLE *table) DBUG_RETURN(FALSE); } -/* - SYNOPSIS - server_exists_in_table() - THD *thd - thread pointer - LEX_SERVER_OPTIONS *server_options - pointer to Lex->server_options - - NOTES - This function takes a LEX_SERVER_OPTIONS struct, which is very much the - same type of structure as a FOREIGN_SERVER, it contains the values parsed - in any one of the [CREATE|DELETE|DROP] SERVER statements. Using the - member "server_name", index_read_idx either founds the record and returns - 1, or doesn't find the record, and returns 0 - - RETURN VALUES - 0 record not found - 1 record found -*/ - -my_bool server_exists_in_table(THD *thd, LEX_SERVER_OPTIONS *server_options) -{ - int result= 1; - int error= 0; - TABLE_LIST tables; - TABLE *table; - DBUG_ENTER("server_exists"); - - bzero((char*) &tables, sizeof(tables)); - tables.db= (char*) "mysql"; - tables.alias= tables.table_name= (char*) "servers"; - - /* need to open before acquiring THR_LOCK_plugin or it will deadlock */ - if (! (table= open_ltable(thd, &tables, TL_WRITE))) - DBUG_RETURN(TRUE); - - table->use_all_columns(); - - rw_wrlock(&THR_LOCK_servers); - VOID(pthread_mutex_lock(&servers_cache_mutex)); - - /* set the field that's the PK to the value we're looking for */ - table->field[0]->store(server_options->server_name, - server_options->server_name_length, - system_charset_info); - - if ((error= table->file->index_read_idx(table->record[0], 0, - (byte *)table->field[0]->ptr, - table->key_info[0].key_length, - HA_READ_KEY_EXACT))) - { - if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE) - { - table->file->print_error(error, MYF(0)); - result= -1; - } - result= 0; - DBUG_PRINT("info",("record for server '%s' not found!", - server_options->server_name)); - } - - VOID(pthread_mutex_unlock(&servers_cache_mutex)); - rw_unlock(&THR_LOCK_servers); - DBUG_RETURN(result); -} /* SYNOPSIS @@ -382,15 +329,18 @@ my_bool server_exists_in_table(THD *thd, LEX_SERVER_OPTIONS *server_options) This function takes a server object that is has all members properly prepared, ready to be inserted both into the mysql.servers table and the servers cache. + + THR_LOCK_servers must be write locked. RETURN VALUES 0 - no error other - error code */ -int insert_server(THD *thd, FOREIGN_SERVER *server) +static int +insert_server(THD *thd, FOREIGN_SERVER *server) { - int error= 0; + int error= -1; TABLE_LIST tables; TABLE *table; @@ -402,13 +352,7 @@ int insert_server(THD *thd, FOREIGN_SERVER *server) /* need to open before acquiring THR_LOCK_plugin or it will deadlock */ if (! (table= open_ltable(thd, &tables, TL_WRITE))) - DBUG_RETURN(TRUE); - - /* lock mutex to make sure no changes happen */ - VOID(pthread_mutex_lock(&servers_cache_mutex)); - - /* lock table */ - rw_wrlock(&THR_LOCK_servers); + goto end; /* insert the server into the table */ if ((error= insert_server_record(table, server))) @@ -419,12 +363,10 @@ int insert_server(THD *thd, FOREIGN_SERVER *server) goto end; end: - /* unlock the table */ - rw_unlock(&THR_LOCK_servers); - VOID(pthread_mutex_unlock(&servers_cache_mutex)); DBUG_RETURN(error); } + /* SYNOPSIS int insert_server_record_into_cache() @@ -434,13 +376,16 @@ end: This function takes a FOREIGN_SERVER pointer to an allocated (root mem) and inserts it into the global servers cache + THR_LOCK_servers must be write locked. + RETURN VALUE 0 - no error >0 - error code */ -int insert_server_record_into_cache(FOREIGN_SERVER *server) +static int +insert_server_record_into_cache(FOREIGN_SERVER *server) { int error=0; DBUG_ENTER("insert_server_record_into_cache"); @@ -461,6 +406,7 @@ int insert_server_record_into_cache(FOREIGN_SERVER *server) DBUG_RETURN(error); } + /* SYNOPSIS store_server_fields() @@ -478,7 +424,8 @@ int insert_server_record_into_cache(FOREIGN_SERVER *server) */ -void store_server_fields(TABLE *table, FOREIGN_SERVER *server) +static void +store_server_fields(TABLE *table, FOREIGN_SERVER *server) { table->use_all_columns(); @@ -539,6 +486,7 @@ void store_server_fields(TABLE *table, FOREIGN_SERVER *server) */ +static int insert_server_record(TABLE *table, FOREIGN_SERVER *server) { int error; @@ -606,7 +554,7 @@ int insert_server_record(TABLE *table, FOREIGN_SERVER *server) int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) { - int error= 0; + int error; TABLE_LIST tables; TABLE *table; @@ -618,28 +566,33 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) tables.db= (char*) "mysql"; tables.alias= tables.table_name= (char*) "servers"; - /* need to open before acquiring THR_LOCK_plugin or it will deadlock */ - if (! (table= open_ltable(thd, &tables, TL_WRITE))) - DBUG_RETURN(TRUE); - rw_wrlock(&THR_LOCK_servers); - VOID(pthread_mutex_lock(&servers_cache_mutex)); - - - if ((error= delete_server_record(table, - server_options->server_name, - server_options->server_name_length))) - goto end; - + /* hit the memory hit first */ if ((error= delete_server_record_in_cache(server_options))) goto end; + if (! (table= open_ltable(thd, &tables, TL_WRITE))) + { + error= my_errno; + goto end; + } + + error= delete_server_record(table, + server_options->server_name, + server_options->server_name_length); + + /* + Perform a reload so we don't have a 'hole' in our mem_root + */ + servers_load(thd, &tables); + end: - VOID(pthread_mutex_unlock(&servers_cache_mutex)); rw_unlock(&THR_LOCK_servers); DBUG_RETURN(error); } + + /* SYNOPSIS @@ -658,10 +611,10 @@ end: */ -int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options) +static int +delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options) { - - int error= 0; + int error= ER_FOREIGN_SERVER_DOESNT_EXIST; FOREIGN_SERVER *server; DBUG_ENTER("delete_server_record_in_cache"); @@ -677,7 +630,7 @@ int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options) DBUG_PRINT("info", ("server_name %s length %d not found!", server_options->server_name, server_options->server_name_length)); - // what should be done if not found in the cache? + goto end; } /* We succeded in deletion of the server to the table, now delete @@ -687,14 +640,15 @@ int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options) server->server_name, server->server_name_length)); - if (server) - VOID(hash_delete(&servers_cache, (byte*) server)); - - servers_version++; /* servers updated */ + VOID(hash_delete(&servers_cache, (byte*) server)); + + error= 0; +end: DBUG_RETURN(error); } + /* SYNOPSIS @@ -714,6 +668,8 @@ int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options) table for the particular server via the call to update_server_record, and in the servers_cache via update_server_record_in_cache. + THR_LOCK_servers must be write locked. + RETURN VALUE 0 - no error >0 - error code @@ -722,7 +678,7 @@ int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options) int update_server(THD *thd, FOREIGN_SERVER *existing, FOREIGN_SERVER *altered) { - int error= 0; + int error; TABLE *table; TABLE_LIST tables; DBUG_ENTER("update_server"); @@ -732,19 +688,26 @@ int update_server(THD *thd, FOREIGN_SERVER *existing, FOREIGN_SERVER *altered) tables.alias= tables.table_name= (char*)"servers"; if (!(table= open_ltable(thd, &tables, TL_WRITE))) - DBUG_RETURN(1); + { + error= my_errno; + goto end; + } - rw_wrlock(&THR_LOCK_servers); if ((error= update_server_record(table, altered))) goto end; - update_server_record_in_cache(existing, altered); + error= update_server_record_in_cache(existing, altered); + + /* + Perform a reload so we don't have a 'hole' in our mem_root + */ + servers_load(thd, &tables); end: - rw_unlock(&THR_LOCK_servers); DBUG_RETURN(error); } + /* SYNOPSIS @@ -761,6 +724,8 @@ end: HASH, then the updated record inserted, in essence replacing the old record. + THR_LOCK_servers must be write locked. + RETURN VALUE 0 - no error 1 - error @@ -791,13 +756,13 @@ int update_server_record_in_cache(FOREIGN_SERVER *existing, { DBUG_PRINT("info", ("had a problem inserting server %s at %lx", altered->server_name, (long unsigned int) altered)); - error= 1; + error= ER_OUT_OF_RESOURCES; } - servers_version++; /* servers updated */ DBUG_RETURN(error); } + /* SYNOPSIS @@ -830,9 +795,9 @@ void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to) to->password= strdup_root(&mem, from->password); if (to->port == -1) to->port= from->port; - if (!to->socket) + if (!to->socket && from->socket) to->socket= strdup_root(&mem, from->socket); - if (!to->scheme) + if (!to->scheme && from->scheme) to->scheme= strdup_root(&mem, from->scheme); if (!to->owner) to->owner= strdup_root(&mem, from->owner); @@ -840,6 +805,7 @@ void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to) DBUG_VOID_RETURN; } + /* SYNOPSIS @@ -862,7 +828,9 @@ void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to) */ -int update_server_record(TABLE *table, FOREIGN_SERVER *server) + +static int +update_server_record(TABLE *table, FOREIGN_SERVER *server) { int error=0; DBUG_ENTER("update_server_record"); @@ -878,10 +846,7 @@ int update_server_record(TABLE *table, FOREIGN_SERVER *server) HA_READ_KEY_EXACT))) { if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE) - { table->file->print_error(error, MYF(0)); - error= 1; - } DBUG_PRINT("info",("server not found!")); error= ER_FOREIGN_SERVER_DOESNT_EXIST; } @@ -901,6 +866,7 @@ end: DBUG_RETURN(error); } + /* SYNOPSIS @@ -916,11 +882,11 @@ end: */ -int delete_server_record(TABLE *table, - char *server_name, - int server_name_length) +static int +delete_server_record(TABLE *table, + char *server_name, int server_name_length) { - int error= 0; + int error; DBUG_ENTER("delete_server_record"); table->use_all_columns(); @@ -933,10 +899,7 @@ int delete_server_record(TABLE *table, HA_READ_KEY_EXACT))) { if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE) - { table->file->print_error(error, MYF(0)); - error= 1; - } DBUG_PRINT("info",("server not found!")); error= ER_FOREIGN_SERVER_DOESNT_EXIST; } @@ -965,28 +928,35 @@ int delete_server_record(TABLE *table, int create_server(THD *thd, LEX_SERVER_OPTIONS *server_options) { - int error; + int error= ER_FOREIGN_SERVER_EXISTS; FOREIGN_SERVER *server; DBUG_ENTER("create_server"); DBUG_PRINT("info", ("server_options->server_name %s", server_options->server_name)); + rw_wrlock(&THR_LOCK_servers); + + /* hit the memory first */ + if (hash_search(&servers_cache, (byte*) server_options->server_name, + server_options->server_name_length)) + goto end; + server= (FOREIGN_SERVER *)alloc_root(&mem, sizeof(FOREIGN_SERVER)); - if ((error= prepare_server_struct_for_insert(server_options, server))) - goto end; + prepare_server_struct_for_insert(server_options, server); - if ((error= insert_server(thd, server))) - goto end; + error= insert_server(thd, server); DBUG_PRINT("info", ("error returned %d", error)); end: + rw_unlock(&THR_LOCK_servers); DBUG_RETURN(error); } + /* SYNOPSIS @@ -1003,37 +973,33 @@ end: int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options) { - int error= 0; + int error= ER_FOREIGN_SERVER_DOESNT_EXIST; FOREIGN_SERVER *altered, *existing; DBUG_ENTER("alter_server"); DBUG_PRINT("info", ("server_options->server_name %s", server_options->server_name)); - altered= (FOREIGN_SERVER *)alloc_root(&mem, - sizeof(FOREIGN_SERVER)); - - VOID(pthread_mutex_lock(&servers_cache_mutex)); + rw_wrlock(&THR_LOCK_servers); if (!(existing= (FOREIGN_SERVER *) hash_search(&servers_cache, (byte*) server_options->server_name, server_options->server_name_length))) - { - error= ER_FOREIGN_SERVER_DOESNT_EXIST; - goto end; - } - - if ((error= prepare_server_struct_for_update(server_options, existing, altered))) goto end; - if ((error= update_server(thd, existing, altered))) - goto end; + altered= (FOREIGN_SERVER *)alloc_root(&mem, + sizeof(FOREIGN_SERVER)); + + prepare_server_struct_for_update(server_options, existing, altered); + + error= update_server(thd, existing, altered); end: DBUG_PRINT("info", ("error returned %d", error)); - VOID(pthread_mutex_unlock(&servers_cache_mutex)); + rw_unlock(&THR_LOCK_servers); DBUG_RETURN(error); } + /* SYNOPSIS @@ -1044,19 +1010,17 @@ end: NOTES RETURN VALUE - 0 - no error + none */ -int prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options, - FOREIGN_SERVER *server) +static void +prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options, + FOREIGN_SERVER *server) { - int error; char *unset_ptr= (char*)""; DBUG_ENTER("prepare_server_struct"); - error= 0; - /* these two MUST be set */ server->server_name= strdup_root(&mem, server_options->server_name); server->server_name_length= server_options->server_name_length; @@ -1086,7 +1050,7 @@ int prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options, server->owner= server_options->owner ? strdup_root(&mem, server_options->owner) : unset_ptr; - DBUG_RETURN(error); + DBUG_VOID_RETURN; } /* @@ -1102,13 +1066,12 @@ int prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options, */ -int prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, - FOREIGN_SERVER *existing, - FOREIGN_SERVER *altered) +static void +prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, + FOREIGN_SERVER *existing, + FOREIGN_SERVER *altered) { - int error; DBUG_ENTER("prepare_server_struct_for_update"); - error= 0; altered->server_name= strdup_root(&mem, server_options->server_name); altered->server_name_length= server_options->server_name_length; @@ -1159,7 +1122,7 @@ int prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, (strcmp(server_options->owner, existing->owner))) ? strdup_root(&mem, server_options->owner) : 0; - DBUG_RETURN(error); + DBUG_VOID_RETURN; } /* @@ -1178,17 +1141,15 @@ int prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, void servers_free(bool end) { DBUG_ENTER("servers_free"); - if (!servers_cache_initialised) + if (!hash_inited(&servers_cache)) DBUG_VOID_RETURN; - VOID(pthread_mutex_destroy(&servers_cache_mutex)); - servers_cache_initialised=0; + rwlock_destroy(&THR_LOCK_servers); free_root(&mem,MYF(0)); hash_free(&servers_cache); DBUG_VOID_RETURN; } - /* SYNOPSIS @@ -1220,7 +1181,7 @@ FOREIGN_SERVER *get_server_by_name(const char *server_name) } DBUG_PRINT("info", ("locking servers_cache")); - VOID(pthread_mutex_lock(&servers_cache_mutex)); + rw_rdlock(&THR_LOCK_servers); if (!(server= (FOREIGN_SERVER *) hash_search(&servers_cache, (byte*) server_name, server_name_length))) @@ -1230,7 +1191,7 @@ FOREIGN_SERVER *get_server_by_name(const char *server_name) server= (FOREIGN_SERVER *) NULL; } DBUG_PRINT("info", ("unlocking servers_cache")); - VOID(pthread_mutex_unlock(&servers_cache_mutex)); + rw_unlock(&THR_LOCK_servers); DBUG_RETURN(server); } diff --git a/sql/sql_servers.h b/sql/sql_servers.h index 23b8cefd5bb..9618374dcab 100644 --- a/sql/sql_servers.h +++ b/sql/sql_servers.h @@ -25,40 +25,18 @@ typedef struct st_federated_server } FOREIGN_SERVER; /* cache handlers */ -my_bool servers_init(bool dont_read_server_table); -my_bool servers_reload(THD *thd); -my_bool get_server_from_table_to_cache(TABLE *table); +bool servers_init(bool dont_read_server_table); +bool servers_reload(THD *thd); void servers_free(bool end=0); /* insert functions */ int create_server(THD *thd, LEX_SERVER_OPTIONS *server_options); -int insert_server(THD *thd, FOREIGN_SERVER *server_options); -int insert_server_record(TABLE *table, FOREIGN_SERVER *server); -int insert_server_record_into_cache(FOREIGN_SERVER *server); -void store_server_fields_for_insert(TABLE *table, FOREIGN_SERVER *server); -void store_server_fields_for_insert(TABLE *table, - FOREIGN_SERVER *existing, - FOREIGN_SERVER *altered); -int prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options, - FOREIGN_SERVER *server); /* drop functions */ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options); -int delete_server_record(TABLE *table, - char *server_name, - int server_name_length); -int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options); /* update functions */ int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options); -int prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options, - FOREIGN_SERVER *existing, - FOREIGN_SERVER *altered); -int update_server(THD *thd, FOREIGN_SERVER *existing, FOREIGN_SERVER *altered); -int update_server_record(TABLE *table, FOREIGN_SERVER *server); -int update_server_record_in_cache(FOREIGN_SERVER *existing, - FOREIGN_SERVER *altered); -/* utility functions */ -void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to); + +/* lookup functions */ FOREIGN_SERVER *get_server_by_name(const char *server_name); -my_bool server_exists_in_table(THD *thd, char *server_name); From ccb9d448f327c78dd86feb369110800814c23549 Mon Sep 17 00:00:00 2001 From: "acurtis/antony@xiphis.org/ltamd64.xiphis.org" <> Date: Sat, 24 Mar 2007 01:18:19 -0700 Subject: [PATCH 09/13] BUG#26257 New Federated Server Functionality Doesn't support differently named tables * Modified Federated memory allocation to use MEM_ROOT * Modified sql_servers and federated to allocate share connection parameters to use MEM_ROOT * Modified Federated to allow tablename in addition to server name * Implicit flushing of tables using altered/dropped server name * Added tests to prove new functionality works Contributors to this patch: Patrick Galbraith, Antony Curtis --- mysql-test/r/federated_server.result | 85 +++++++-- mysql-test/t/federated_server.test | 62 ++++++- sql/mysql_priv.h | 3 + sql/sql_base.cc | 66 +++++++ sql/sql_servers.cc | 110 ++++++++++-- sql/sql_servers.h | 3 +- storage/federated/ha_federated.cc | 258 ++++++++++++++++----------- storage/federated/ha_federated.h | 3 + 8 files changed, 458 insertions(+), 132 deletions(-) diff --git a/mysql-test/r/federated_server.result b/mysql-test/r/federated_server.result index 8faf33581d1..a3e7cd793a6 100644 --- a/mysql-test/r/federated_server.result +++ b/mysql-test/r/federated_server.result @@ -20,6 +20,14 @@ CREATE TABLE first_db.t1 ( `name` varchar(64) NOT NULL default '' ) DEFAULT CHARSET=latin1; +DROP TABLE IF EXISTS first_db.t2; +Warnings: +Note 1051 Unknown table 't2' +CREATE TABLE first_db.t2 ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ) +DEFAULT CHARSET=latin1; use second_db; DROP TABLE IF EXISTS second_db.t1; Warnings: @@ -29,6 +37,14 @@ CREATE TABLE second_db.t1 ( `name` varchar(64) NOT NULL default '' ) DEFAULT CHARSET=latin1; +DROP TABLE IF EXISTS second_db.t2; +Warnings: +Note 1051 Unknown table 't2' +CREATE TABLE second_db.t2 ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ) +DEFAULT CHARSET=latin1; drop server if exists 'server_one'; create server 'server_one' foreign data wrapper 'mysql' options (HOST '127.0.0.1', @@ -60,10 +76,10 @@ CREATE TABLE federated.old ( ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/first_db/t1'; -INSERT INTO federated.old (id, name) values (1, 'federated.old url'); +INSERT INTO federated.old (id, name) values (1, 'federated.old-> first_db.t1, url format'); SELECT * FROM federated.old; id name -1 federated.old url +1 federated.old-> first_db.t1, url format DROP TABLE IF EXISTS federated.old2; Warnings: Note 1051 Unknown table 'old2' @@ -72,8 +88,37 @@ CREATE TABLE federated.old2 ( `name` varchar(64) NOT NULL default '' ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 +CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/first_db/t2'; +INSERT INTO federated.old2 (id, name) values (1, 'federated.old2-> first_db.t2, url format'); +SELECT * FROM federated.old2; +id name +1 federated.old2-> first_db.t2, url format +DROP TABLE IF EXISTS federated.urldb2t1; +Warnings: +Note 1051 Unknown table 'urldb2t1' +CREATE TABLE federated.urldb2t1 ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ) +ENGINE="FEDERATED" DEFAULT CHARSET=latin1 CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/second_db/t1'; -INSERT INTO federated.old2 (id, name) values (1, 'federated.old2 url'); +INSERT INTO federated.urldb2t1 (id, name) values (1, 'federated.urldb2t1 -> second_db.t1, url format'); +SELECT * FROM federated.urldb2t1; +id name +1 federated.urldb2t1 -> second_db.t1, url format +DROP TABLE IF EXISTS federated.urldb2t2; +Warnings: +Note 1051 Unknown table 'urldb2t2' +CREATE TABLE federated.urldb2t2 ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ) +ENGINE="FEDERATED" DEFAULT CHARSET=latin1 +CONNECTION='mysql://root@127.0.0.1:SLAVE_PORT/second_db/t2'; +INSERT INTO federated.urldb2t2 (id, name) values (1, 'federated.urldb2t2 -> second_db.t2, url format'); +SELECT * FROM federated.urldb2t2; +id name +1 federated.urldb2t2 -> second_db.t2, url format DROP TABLE IF EXISTS federated.t1; Warnings: Note 1051 Unknown table 't1' @@ -83,18 +128,38 @@ CREATE TABLE federated.t1 ( ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 CONNECTION='server_one'; -INSERT INTO federated.t1 (id, name) values (1, 'server_one, new scheme'); +INSERT INTO federated.t1 (id, name) values (1, 'server_one, new scheme, first_db.t1'); SELECT * FROM federated.t1; id name -1 federated.old url -1 server_one, new scheme +1 federated.old-> first_db.t1, url format +1 server_one, new scheme, first_db.t1 +DROP TABLE IF EXISTS federated.whatever; +Warnings: +Note 1051 Unknown table 'whatever' +CREATE TABLE federated.whatever ( +`id` int(20) NOT NULL, +`name` varchar(64) NOT NULL default '' + ) +ENGINE="FEDERATED" DEFAULT CHARSET=latin1 +CONNECTION='server_one/t1'; +INSERT INTO federated.whatever (id, name) values (1, 'server_one, new scheme, whatever, first_db.t1'); +SELECT * FROM federated.whatever; +id name +1 federated.old-> first_db.t1, url format +1 server_one, new scheme, first_db.t1 +1 server_one, new scheme, whatever, first_db.t1 ALTER SERVER 'server_one' options(DATABASE 'second_db'); -flush tables; -INSERT INTO federated.t1 (id, name) values (1, 'server_two, new scheme'); +INSERT INTO federated.t1 (id, name) values (1, 'server_two, new scheme, second_db.t1'); SELECT * FROM federated.t1; id name -1 federated.old2 url -1 server_two, new scheme +1 federated.urldb2t1 -> second_db.t1, url format +1 server_two, new scheme, second_db.t1 +INSERT INTO federated.whatever (id, name) values (1, 'server_two, new scheme, whatever, second_db.t1'); +SELECT * FROM federated.whatever; +id name +1 federated.urldb2t1 -> second_db.t1, url format +1 server_two, new scheme, second_db.t1 +1 server_two, new scheme, whatever, second_db.t1 drop table federated.t1; drop server 'server_one'; drop server 'server_two'; diff --git a/mysql-test/t/federated_server.test b/mysql-test/t/federated_server.test index 27dd494e5c5..b2075b8e262 100644 --- a/mysql-test/t/federated_server.test +++ b/mysql-test/t/federated_server.test @@ -17,6 +17,13 @@ CREATE TABLE first_db.t1 ( ) DEFAULT CHARSET=latin1; +DROP TABLE IF EXISTS first_db.t2; +CREATE TABLE first_db.t2 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) + DEFAULT CHARSET=latin1; + use second_db; DROP TABLE IF EXISTS second_db.t1; CREATE TABLE second_db.t1 ( @@ -25,6 +32,13 @@ CREATE TABLE second_db.t1 ( ) DEFAULT CHARSET=latin1; +DROP TABLE IF EXISTS second_db.t2; +CREATE TABLE second_db.t2 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) + DEFAULT CHARSET=latin1; + connection master; drop server if exists 'server_one'; @@ -61,7 +75,7 @@ eval CREATE TABLE federated.old ( ENGINE="FEDERATED" DEFAULT CHARSET=latin1 CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/first_db/t1'; -INSERT INTO federated.old (id, name) values (1, 'federated.old url'); +INSERT INTO federated.old (id, name) values (1, 'federated.old-> first_db.t1, url format'); SELECT * FROM federated.old; @@ -72,9 +86,32 @@ eval CREATE TABLE federated.old2 ( `name` varchar(64) NOT NULL default '' ) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 - CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/second_db/t1'; + CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/first_db/t2'; -INSERT INTO federated.old2 (id, name) values (1, 'federated.old2 url'); +INSERT INTO federated.old2 (id, name) values (1, 'federated.old2-> first_db.t2, url format'); +SELECT * FROM federated.old2; + +DROP TABLE IF EXISTS federated.urldb2t1; +--replace_result $SLAVE_MYPORT SLAVE_PORT +eval CREATE TABLE federated.urldb2t1 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) + ENGINE="FEDERATED" DEFAULT CHARSET=latin1 + CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/second_db/t1'; +INSERT INTO federated.urldb2t1 (id, name) values (1, 'federated.urldb2t1 -> second_db.t1, url format'); +SELECT * FROM federated.urldb2t1; + +DROP TABLE IF EXISTS federated.urldb2t2; +--replace_result $SLAVE_MYPORT SLAVE_PORT +eval CREATE TABLE federated.urldb2t2 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) + ENGINE="FEDERATED" DEFAULT CHARSET=latin1 + CONNECTION='mysql://root@127.0.0.1:$SLAVE_MYPORT/second_db/t2'; +INSERT INTO federated.urldb2t2 (id, name) values (1, 'federated.urldb2t2 -> second_db.t2, url format'); +SELECT * FROM federated.urldb2t2; DROP TABLE IF EXISTS federated.t1; CREATE TABLE federated.t1 ( @@ -84,17 +121,30 @@ CREATE TABLE federated.t1 ( ENGINE="FEDERATED" DEFAULT CHARSET=latin1 CONNECTION='server_one'; -INSERT INTO federated.t1 (id, name) values (1, 'server_one, new scheme'); +INSERT INTO federated.t1 (id, name) values (1, 'server_one, new scheme, first_db.t1'); SELECT * FROM federated.t1; +DROP TABLE IF EXISTS federated.whatever; +CREATE TABLE federated.whatever ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) + ENGINE="FEDERATED" DEFAULT CHARSET=latin1 + CONNECTION='server_one/t1'; +INSERT INTO federated.whatever (id, name) values (1, 'server_one, new scheme, whatever, first_db.t1'); +SELECT * FROM federated.whatever; + ALTER SERVER 'server_one' options(DATABASE 'second_db'); -flush tables; +# FLUSH TABLES is now unneccessary -INSERT INTO federated.t1 (id, name) values (1, 'server_two, new scheme'); +INSERT INTO federated.t1 (id, name) values (1, 'server_two, new scheme, second_db.t1'); SELECT * FROM federated.t1; +INSERT INTO federated.whatever (id, name) values (1, 'server_two, new scheme, whatever, second_db.t1'); +SELECT * FROM federated.whatever; + drop table federated.t1; drop server 'server_one'; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index efcbdf968bf..2b1b310e67e 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -1430,6 +1430,9 @@ void close_open_tables_and_downgrade(ALTER_PARTITION_PARAM_TYPE *lpt); void mysql_wait_completed_table(ALTER_PARTITION_PARAM_TYPE *lpt, TABLE *my_table); bool close_cached_tables(THD *thd, bool wait_for_refresh, TABLE_LIST *tables, bool have_lock = FALSE); +bool close_cached_connection_tables(THD *thd, bool wait_for_refresh, + LEX_STRING *connect_string, + bool have_lock = FALSE); void copy_field_from_tmp_record(Field *field,int offset); bool fill_record(THD *thd, Field **field, List &values, bool ignore_errors); diff --git a/sql/sql_base.cc b/sql/sql_base.cc index e764c498059..44becc77865 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -858,6 +858,7 @@ void free_io_cache(TABLE *table) DBUG_VOID_RETURN; } + /* Close all tables which aren't in use by any thread @@ -969,6 +970,71 @@ bool close_cached_tables(THD *thd, bool if_wait_for_refresh, } +/* + Close all tables which match specified connection string or + if specified string is NULL, then any table with a connection string. +*/ + +bool close_cached_connection_tables(THD *thd, bool if_wait_for_refresh, + LEX_STRING *connection, bool have_lock) +{ + uint idx; + TABLE_LIST tmp, *tables= NULL; + bool result= FALSE; + DBUG_ENTER("close_cached_connections"); + DBUG_ASSERT(thd); + + bzero(&tmp, sizeof(TABLE_LIST)); + + if (!have_lock) + VOID(pthread_mutex_lock(&LOCK_open)); + + for (idx= 0; idx < table_def_cache.records; idx++) + { + TABLE_SHARE *share= (TABLE_SHARE *) hash_element(&table_def_cache, idx); + + /* Ignore if table is not open or does not have a connect_string */ + if (!share->connect_string.length || !share->ref_count) + continue; + + /* Compare the connection string */ + if (connection && + (connection->length > share->connect_string.length || + (connection->length < share->connect_string.length && + (share->connect_string.str[connection->length] != '/' && + share->connect_string.str[connection->length] != '\\')) || + strncasecmp(connection->str, share->connect_string.str, + connection->length))) + continue; + + /* close_cached_tables() only uses these elements */ + tmp.db= share->db.str; + tmp.table_name= share->table_name.str; + tmp.next_local= tables; + + tables= (TABLE_LIST *) memdup_root(thd->mem_root, (char*)&tmp, + sizeof(TABLE_LIST)); + } + + if (tables) + result= close_cached_tables(thd, FALSE, tables, TRUE); + + if (!have_lock) + VOID(pthread_mutex_unlock(&LOCK_open)); + + if (if_wait_for_refresh) + { + pthread_mutex_lock(&thd->mysys_var->mutex); + thd->mysys_var->current_mutex= 0; + thd->mysys_var->current_cond= 0; + thd->proc_info=0; + pthread_mutex_unlock(&thd->mysys_var->mutex); + } + + DBUG_RETURN(result); +} + + /* Mark all tables in the list which were used by current substatement as free for reuse. diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index 9b01a1b6889..71e141b0514 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -16,6 +16,21 @@ /* The servers are saved in the system table "servers" + + Currently, when the user performs an ALTER SERVER or a DROP SERVER + operation, it will cause all open tables which refer to the named + server connection to be flushed. This may cause some undesirable + behaviour with regard to currently running transactions. It is + expected that the DBA knows what s/he is doing when s/he performs + the ALTER SERVER or DROP SERVER operation. + + TODO: + It is desirable for us to implement a callback mechanism instead where + callbacks can be registered for specific server protocols. The callback + will be fired when such a server name has been created/altered/dropped + or when statistics are to be gathered such as how many actual connections. + Storage engines etc will be able to make use of the callback so that + currently running transactions etc will not be disrupted. */ #include "mysql_priv.h" @@ -557,6 +572,8 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) int error; TABLE_LIST tables; TABLE *table; + LEX_STRING name= { server_options->server_name, + server_options->server_name_length }; DBUG_ENTER("drop_server"); DBUG_PRINT("info", ("server name server->server_name %s", @@ -578,14 +595,16 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) goto end; } - error= delete_server_record(table, - server_options->server_name, - server_options->server_name_length); + error= delete_server_record(table, name.str, name.length); - /* - Perform a reload so we don't have a 'hole' in our mem_root - */ - servers_load(thd, &tables); + /* close the servers table before we call closed_cached_connection_tables */ + close_thread_tables(thd); + + if (close_cached_connection_tables(thd, TRUE, &name)) + { + push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, + ER_UNKNOWN_ERROR, "Server connection in use"); + } end: rw_unlock(&THR_LOCK_servers); @@ -975,6 +994,8 @@ int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options) { int error= ER_FOREIGN_SERVER_DOESNT_EXIST; FOREIGN_SERVER *altered, *existing; + LEX_STRING name= { server_options->server_name, + server_options->server_name_length }; DBUG_ENTER("alter_server"); DBUG_PRINT("info", ("server_options->server_name %s", server_options->server_name)); @@ -982,8 +1003,8 @@ int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options) rw_wrlock(&THR_LOCK_servers); if (!(existing= (FOREIGN_SERVER *) hash_search(&servers_cache, - (byte*) server_options->server_name, - server_options->server_name_length))) + (byte*) name.str, + name.length))) goto end; altered= (FOREIGN_SERVER *)alloc_root(&mem, @@ -993,6 +1014,15 @@ int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options) error= update_server(thd, existing, altered); + /* close the servers table before we call closed_cached_connection_tables */ + close_thread_tables(thd); + + if (close_cached_connection_tables(thd, FALSE, &name)) + { + push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_WARN, + ER_UNKNOWN_ERROR, "Server connection in use"); + } + end: DBUG_PRINT("info", ("error returned %d", error)); rw_unlock(&THR_LOCK_servers); @@ -1143,6 +1173,12 @@ void servers_free(bool end) DBUG_ENTER("servers_free"); if (!hash_inited(&servers_cache)) DBUG_VOID_RETURN; + if (!end) + { + free_root(&mem, MYF(MY_MARK_BLOCKS_FREE)); + my_hash_reset(&servers_cache); + DBUG_VOID_RETURN; + } rwlock_destroy(&THR_LOCK_servers); free_root(&mem,MYF(0)); hash_free(&servers_cache); @@ -1150,6 +1186,51 @@ void servers_free(bool end) } +/* + SYNOPSIS + + clone_server(MEM_ROOT *mem_root, FOREIGN_SERVER *orig, FOREIGN_SERVER *buff) + + Create a clone of FOREIGN_SERVER. If the supplied mem_root is of + thd->mem_root then the copy is automatically disposed at end of statement. + + NOTES + + ARGS + MEM_ROOT pointer (strings are copied into this mem root) + FOREIGN_SERVER pointer (made a copy of) + FOREIGN_SERVER buffer (if not-NULL, this pointer is returned) + + RETURN VALUE + FOREIGN_SEVER pointer (copy of one supplied FOREIGN_SERVER) +*/ + +static FOREIGN_SERVER *clone_server(MEM_ROOT *mem, const FOREIGN_SERVER *server, + FOREIGN_SERVER *buffer) +{ + DBUG_ENTER("sql_server.cc:clone_server"); + + if (!buffer) + buffer= (FOREIGN_SERVER *) alloc_root(mem, sizeof(FOREIGN_SERVER)); + + buffer->server_name= strmake_root(mem, server->server_name, + server->server_name_length); + buffer->port= server->port; + buffer->server_name_length= server->server_name_length; + + /* TODO: We need to examine which of these can really be NULL */ + buffer->db= server->db ? strdup_root(mem, server->db) : NULL; + buffer->scheme= server->scheme ? strdup_root(mem, server->scheme) : NULL; + buffer->username= server->username? strdup_root(mem, server->username): NULL; + buffer->password= server->password? strdup_root(mem, server->password): NULL; + buffer->socket= server->socket ? strdup_root(mem, server->socket) : NULL; + buffer->owner= server->owner ? strdup_root(mem, server->owner) : NULL; + buffer->host= server->host ? strdup_root(mem, server->host) : NULL; + + DBUG_RETURN(buffer); +} + + /* SYNOPSIS @@ -1163,11 +1244,11 @@ void servers_free(bool end) */ -FOREIGN_SERVER *get_server_by_name(const char *server_name) +FOREIGN_SERVER *get_server_by_name(MEM_ROOT *mem, const char *server_name, + FOREIGN_SERVER *buff) { - ulong error_num=0; uint server_name_length; - FOREIGN_SERVER *server= 0; + FOREIGN_SERVER *server; DBUG_ENTER("get_server_by_name"); DBUG_PRINT("info", ("server_name %s", server_name)); @@ -1176,7 +1257,6 @@ FOREIGN_SERVER *get_server_by_name(const char *server_name) if (! server_name || !strlen(server_name)) { DBUG_PRINT("info", ("server_name not defined!")); - error_num= 1; DBUG_RETURN((FOREIGN_SERVER *)NULL); } @@ -1190,6 +1270,10 @@ FOREIGN_SERVER *get_server_by_name(const char *server_name) server_name, server_name_length)); server= (FOREIGN_SERVER *) NULL; } + /* otherwise, make copy of server */ + else + server= clone_server(mem, server, buff); + DBUG_PRINT("info", ("unlocking servers_cache")); rw_unlock(&THR_LOCK_servers); DBUG_RETURN(server); diff --git a/sql/sql_servers.h b/sql/sql_servers.h index 9618374dcab..63c691893d1 100644 --- a/sql/sql_servers.h +++ b/sql/sql_servers.h @@ -39,4 +39,5 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options); int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options); /* lookup functions */ -FOREIGN_SERVER *get_server_by_name(const char *server_name); +FOREIGN_SERVER *get_server_by_name(MEM_ROOT *mem, const char *server_name, + FOREIGN_SERVER *server_buffer); diff --git a/storage/federated/ha_federated.cc b/storage/federated/ha_federated.cc index 14ffe5da984..e60dc47f48c 100644 --- a/storage/federated/ha_federated.cc +++ b/storage/federated/ha_federated.cc @@ -43,23 +43,55 @@ The create table will simply create the .frm file, and within the "CREATE TABLE" SQL, there SHALL be any of the following : - comment=scheme://username:password@hostname:port/database/tablename - comment=scheme://username@hostname/database/tablename - comment=scheme://username:password@hostname/database/tablename - comment=scheme://username:password@hostname/database/tablename + connection=scheme://username:password@hostname:port/database/tablename + connection=scheme://username@hostname/database/tablename + connection=scheme://username:password@hostname/database/tablename + connection=scheme://username:password@hostname/database/tablename + + - OR - + + As of 5.1 (See worklog #3031), federated now allows you to use a non-url + format, taking advantage of mysql.servers: + + connection="connection_one" + connection="connection_one/table_foo" An example would be: - comment=mysql://username:password@hostname:port/database/tablename + connection=mysql://username:password@hostname:port/database/tablename - ***IMPORTANT*** + or, if we had: - This is a first release, conceptual release - Only 'mysql://' is supported at this release. + create server 'server_one' foreign data wrapper 'mysql' options + (HOST '127.0.0.1', + DATABASE 'db1', + USER 'root', + PASSWORD '', + PORT 3306, + SOCKET '', + OWNER 'root'); + CREATE TABLE federated.t1 ( + `id` int(20) NOT NULL, + `name` varchar(64) NOT NULL default '' + ) + ENGINE="FEDERATED" DEFAULT CHARSET=latin1 + CONNECTION='server_one'; - This comment connection string is necessary for the handler to be - able to connect to the foreign server. + So, this will have been the equivalent of + + CONNECTION="mysql://root@127.0.0.1:3306/db1/t1" + + Then, we can also change the server to point to a new schema: + + ALTER SERVER 'server_one' options(DATABASE 'db2'); + + All subsequent calls will now be against db2.t1! Guess what? You don't + have to perform an alter table! + + This connecton="connection string" is necessary for the handler to be + able to connect to the foreign server, either by URL, or by server + name. The basic flow is this: @@ -166,7 +198,7 @@ KEY other_key (other)) ENGINE="FEDERATED" DEFAULT CHARSET=latin1 - COMMENT='root@127.0.0.1:9306/federated/test_federated'; + CONNECTION='mysql://root@127.0.0.1:9306/federated/test_federated'; Notice the "COMMENT" and "ENGINE" field? This is where you respectively set the engine type, "FEDERATED" and foreign @@ -263,7 +295,7 @@ To run these tests, go into ./mysql-test (based in the directory you built the server in) - ./mysql-test-run federatedd + ./mysql-test-run federated To run the test, or if you want to run the test and have debug info: @@ -311,7 +343,7 @@ ------------- These were the files that were modified or created for this - Federated handler to work: + Federated handler to work, in 5.0: ./configure.in ./sql/Makefile.am @@ -329,6 +361,13 @@ ./sql/ha_federated.cc ./sql/ha_federated.h + In 5.1 + + my:~/mysql-build/mysql-5.1-bkbits patg$ ls storage/federated/ + CMakeLists.txt Makefile.in ha_federated.h plug.in + Makefile SCCS libfederated.a + Makefile.am ha_federated.cc libfederated_a-ha_federated.o + */ @@ -547,42 +586,39 @@ static int parse_url_error(FEDERATED_SHARE *share, TABLE *table, int error_num) int buf_len; DBUG_ENTER("ha_federated parse_url_error"); - if (share->connection_string) - { - DBUG_PRINT("info", - ("error: parse_url. Returning error code %d \ - freeing share->connection_string %lx", - error_num, (long unsigned int) share->connection_string)); - my_free((gptr) share->connection_string, MYF(0)); - share->connection_string= 0; - } buf_len= min(table->s->connect_string.length, FEDERATED_QUERY_BUFFER_SIZE-1); strmake(buf, table->s->connect_string.str, buf_len); my_error(error_num, MYF(0), buf); DBUG_RETURN(error_num); } + /* retrieve server object which contains server meta-data from the system table given a server's name, set share connection parameter members */ -int get_connection(FEDERATED_SHARE *share) +int get_connection(MEM_ROOT *mem_root, FEDERATED_SHARE *share) { int error_num= ER_FOREIGN_SERVER_DOESNT_EXIST; char error_buffer[FEDERATED_QUERY_BUFFER_SIZE]; - FOREIGN_SERVER *server; + FOREIGN_SERVER *server, server_buffer; DBUG_ENTER("ha_federated::get_connection"); + /* + get_server_by_name() clones the server if exists and allocates + copies of strings in the supplied mem_root + */ if (!(server= - get_server_by_name(share->connection_string))) + get_server_by_name(mem_root, share->connection_string, &server_buffer))) { DBUG_PRINT("info", ("get_server_by_name returned > 0 error condition!")); /* need to come up with error handling */ error_num=1; goto error; } - DBUG_PRINT("info", ("get_server_by_name returned server at %lx", (long unsigned int) server)); + DBUG_PRINT("info", ("get_server_by_name returned server at %lx", + (long unsigned int) server)); /* Most of these should never be empty strings, error handling will @@ -591,29 +627,22 @@ int get_connection(FEDERATED_SHARE *share) except there are errors in the trace file of the share being overrun at the address of the share. */ - if (server->server_name) - share->server_name= server->server_name; - share->server_name_length= server->server_name_length ? - server->server_name_length : 0; - if (server->username) - share->username= server->username; - if (server->password) - share->password= server->password; - if (server->db) - share->database= server->db; - - share->port= server->port ? (ushort) server->port : MYSQL_PORT; - - if (server->host) - share->hostname= server->host; - if (server->socket) - share->socket= server->socket; - else if (strcmp(share->hostname, my_localhost) == 0) - share->socket= my_strdup(MYSQL_UNIX_ADDR, MYF(0)); - if (server->scheme) - share->scheme= server->scheme; - else - share->scheme= NULL; + share->server_name_length= server->server_name_length; + share->server_name= server->server_name; + share->username= server->username; + share->password= server->password; + share->database= server->db; +#ifndef I_AM_PARANOID + share->port= server->port > 0 && server->port < 65536 ? +#else + share->port= server->port > 1023 && server->port < 65536 ? +#endif + (ushort) server->port : MYSQL_PORT; + share->hostname= server->host; + if (!(share->socket= server->socket) && + !strcmp(share->hostname, my_localhost)) + share->socket= (char *) MYSQL_UNIX_ADDR; + share->scheme= server->scheme; DBUG_PRINT("info", ("share->username %s", share->username)); DBUG_PRINT("info", ("share->password %s", share->password)); @@ -636,6 +665,7 @@ error: SYNOPSIS parse_url() + mem_root MEM_ROOT pointer for memory allocation share pointer to FEDERATED share table pointer to current TABLE class table_create_flag determines what error to throw @@ -685,7 +715,7 @@ error: */ -static int parse_url(FEDERATED_SHARE *share, TABLE *table, +static int parse_url(MEM_ROOT *mem_root, FEDERATED_SHARE *share, TABLE *table, uint table_create_flag) { uint error_num= (table_create_flag ? @@ -699,20 +729,19 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, DBUG_PRINT("info", ("Length: %d", table->s->connect_string.length)); DBUG_PRINT("info", ("String: '%.*s'", table->s->connect_string.length, table->s->connect_string.str)); - share->connection_string= my_strndup(table->s->connect_string.str, - table->s->connect_string.length, - MYF(0)); + share->connection_string= strmake_root(mem_root, table->s->connect_string.str, + table->s->connect_string.length); - // Add a null for later termination of table name - share->connection_string[table->s->connect_string.length]= 0; DBUG_PRINT("info",("parse_url alloced share->connection_string %lx", (long unsigned int) share->connection_string)); DBUG_PRINT("info",("share->connection_string %s",share->connection_string)); - /* No delimiters, must be a straight connection name */ - if ( (!strchr(share->connection_string, '/')) && - (!strchr(share->connection_string, '@')) && - (!strchr(share->connection_string, ';'))) + /* + No :// or @ in connection string. Must be a straight connection name of + either "servername" or "servername/tablename" + */ + if ( (!strstr(share->connection_string, "://") && + (!strchr(share->connection_string, '@')))) { DBUG_PRINT("info", @@ -721,17 +750,51 @@ static int parse_url(FEDERATED_SHARE *share, TABLE *table, share->connection_string, (long unsigned int) share->connection_string)); + /* ok, so we do a little parsing, but not completely! */ share->parsed= FALSE; - if ((error_num= get_connection(share))) - goto error; - /* - connection specifies everything but, resort to - expecting remote and foreign table names to match + If there is a single '/' in the connection string, this means the user is + specifying a table name */ - share->table_name= table->s->table_name.str; - share->table_name_length= table->s->table_name.length; - share->table_name[share->table_name_length]= '\0'; + + if ((share->table_name= strchr(share->connection_string, '/'))) + { + share->connection_string[share->table_name - share->connection_string]= '\0'; + share->table_name++; + share->table_name_length= strlen(share->table_name); + + DBUG_PRINT("info", + ("internal format, parsed table_name share->connection_string \ + %s share->table_name %s", + share->connection_string, share->table_name)); + + /* + there better not be any more '/'s ! + */ + if (strchr(share->table_name, '/')) + goto error; + + } + /* + otherwise, straight server name, use tablename of federated table + as remote table name + */ + else + { + /* + connection specifies everything but, resort to + expecting remote and foreign table names to match + */ + share->table_name= strmake_root(mem_root, table->s->table_name.str, + (share->table_name_length= table->s->table_name.length)); + DBUG_PRINT("info", + ("internal format, default table_name share->connection_string \ + %s share->table_name %s", + share->connection_string, share->table_name)); + } + + if ((error_num= get_connection(mem_root, share))) + goto error; } else { @@ -817,7 +880,7 @@ Then password is a null string, so set to NULL if (!share->port) { if (strcmp(share->hostname, my_localhost) == 0) - share->socket= my_strdup(MYSQL_UNIX_ADDR, MYF(0)); + share->socket= (char *) MYSQL_UNIX_ADDR; else share->port= MYSQL_PORT; } @@ -1421,22 +1484,26 @@ err: static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) { - char *select_query; char query_buffer[FEDERATED_QUERY_BUFFER_SIZE]; Field **field; String query(query_buffer, sizeof(query_buffer), &my_charset_bin); FEDERATED_SHARE *share= NULL, tmp_share; + MEM_ROOT mem_root; + DBUG_ENTER("ha_federated.cc::get_share"); + /* In order to use this string, we must first zero it's length, or it will contain garbage */ query.length(0); + init_alloc_root(&mem_root, 256, 0); + pthread_mutex_lock(&federated_mutex); tmp_share.share_key= table_name; tmp_share.share_key_length= strlen(table_name); - if (parse_url(&tmp_share, table, 0)) + if (parse_url(&mem_root, &tmp_share, table, 0)) goto error; /* TODO: change tmp_share.scheme to LEX_STRING object */ @@ -1457,24 +1524,17 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) query.length(query.length() - sizeof_trailing_comma); query.append(STRING_WITH_LEN(" FROM `")); + query.append(tmp_share.table_name, tmp_share.table_name_length); + query.append(STRING_WITH_LEN("`")); + DBUG_PRINT("info", ("calling alloc_root")); - if (!(share= (FEDERATED_SHARE *) - my_multi_malloc(MYF(MY_WME), - &share, sizeof(*share), - &select_query, - query.length()+table->s->connect_string.length+1, - NullS))) + if (!(share= (FEDERATED_SHARE *) memdup_root(&mem_root, (char*)&tmp_share, sizeof(*share))) || + !(share->select_query= (char*) strmake_root(&mem_root, query.ptr(), query.length()))) goto error; - memcpy(share, &tmp_share, sizeof(tmp_share)); - - share->table_name_length= strlen(share->table_name); - /* TODO: share->table_name to LEX_STRING object */ - query.append(share->table_name, share->table_name_length); - query.append(STRING_WITH_LEN("`")); - share->select_query= select_query; - strmov(share->select_query, query.ptr()); share->use_count= 0; + share->mem_root= mem_root; + DBUG_PRINT("info", ("share->select_query %s", share->select_query)); @@ -1483,17 +1543,18 @@ static FEDERATED_SHARE *get_share(const char *table_name, TABLE *table) thr_lock_init(&share->lock); pthread_mutex_init(&share->mutex, MY_MUTEX_INIT_FAST); } + else + free_root(&mem_root, MYF(0)); /* prevents memory leak */ + share->use_count++; pthread_mutex_unlock(&federated_mutex); - return share; + DBUG_RETURN(share); error: pthread_mutex_unlock(&federated_mutex); - my_free((gptr) tmp_share.connection_string, MYF(MY_ALLOW_ZERO_PTR)); - tmp_share.connection_string= 0; - my_free((gptr) share, MYF(MY_ALLOW_ZERO_PTR)); - return NULL; + free_root(&mem_root, MYF(0)); + DBUG_RETURN(NULL); } @@ -1505,23 +1566,16 @@ error: static int free_share(FEDERATED_SHARE *share) { + MEM_ROOT mem_root= share->mem_root; DBUG_ENTER("free_share"); pthread_mutex_lock(&federated_mutex); if (!--share->use_count) { hash_delete(&federated_open_tables, (byte*) share); - if (share->parsed) - my_free((gptr) share->socket, MYF(MY_ALLOW_ZERO_PTR)); - /*if (share->connection_string) - { - */ - my_free((gptr) share->connection_string, MYF(MY_ALLOW_ZERO_PTR)); - share->connection_string= 0; - /*}*/ thr_lock_delete(&share->lock); VOID(pthread_mutex_destroy(&share->mutex)); - my_free((gptr) share, MYF(0)); + free_root(&mem_root, MYF(0)); } pthread_mutex_unlock(&federated_mutex); @@ -1590,6 +1644,8 @@ int ha_federated::open(const char *name, int mode, uint test_if_locked) mysql_options(mysql,MYSQL_SET_CHARSET_NAME, this->table->s->table_charset->csname); + DBUG_PRINT("info", ("calling mysql_real_connect hostname %s user %s", + share->hostname, share->username)); if (!mysql || !mysql_real_connect(mysql, share->hostname, share->username, @@ -2832,15 +2888,13 @@ int ha_federated::create(const char *name, TABLE *table_arg, HA_CREATE_INFO *create_info) { int retval; + THD *thd= current_thd; FEDERATED_SHARE tmp_share; // Only a temporary share, to test the url DBUG_ENTER("ha_federated::create"); - if (!(retval= parse_url(&tmp_share, table_arg, 1))) + if (!(retval= parse_url(thd->mem_root, &tmp_share, table_arg, 1))) retval= check_foreign_data_source(&tmp_share, 1); - /* free this because strdup created it in parse_url */ - my_free((gptr) tmp_share.connection_string, MYF(MY_ALLOW_ZERO_PTR)); - tmp_share.connection_string= 0; DBUG_RETURN(retval); } diff --git a/storage/federated/ha_federated.h b/storage/federated/ha_federated.h index bbc2b2fe9f8..4d2eefdd986 100644 --- a/storage/federated/ha_federated.h +++ b/storage/federated/ha_federated.h @@ -43,6 +43,8 @@ The example implements the minimum of what you will probably need. */ typedef struct st_federated_share { + MEM_ROOT mem_root; + bool parsed; /* this key is unique db/tablename */ const char *share_key; @@ -67,6 +69,7 @@ typedef struct st_federated_share { char *sport; int share_key_length; ushort port; + uint table_name_length, server_name_length, connect_string_length, use_count; pthread_mutex_t mutex; THR_LOCK lock; From 1b64741b9d0e6aa207180e01dc1fac8a640deb65 Mon Sep 17 00:00:00 2001 From: "svoj@mysql.com/april.(none)" <> Date: Sun, 25 Mar 2007 00:24:39 +0400 Subject: [PATCH 10/13] BUG#24566 - Incorrect key file for table ( the size of table is more than 2G) Accessing a file that is bigger than 2G may report that read/write operation failed. This may affect anything that uses my_pread/my_pwrite functions, e.g. MyISAM, ARCHIVE, binary log. For MyISAM INSERT may report that table is crashed when writing to a table that is bigger than 2G. This is fixed by using proper offset type in my_pread/my_pwrite functions on systems that do not have native pread/pwrite calls. Affects systems that do not have native pread/pwrite calls, e.g. Windows. No test case for this fix, since it requires huge table. --- mysys/my_pread.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/mysys/my_pread.c b/mysys/my_pread.c index 7a09c21e039..f8f0fa49c10 100644 --- a/mysys/my_pread.c +++ b/mysys/my_pread.c @@ -37,7 +37,7 @@ uint my_pread(File Filedes, byte *Buffer, uint Count, my_off_t offset, errno=0; /* Linux doesn't reset this */ #endif #ifndef HAVE_PREAD - off_t old_offset; + os_off_t old_offset; pthread_mutex_lock(&my_file_info[Filedes].mutex); /* @@ -45,7 +45,7 @@ uint my_pread(File Filedes, byte *Buffer, uint Count, my_off_t offset, before seeking to the given offset */ - error= (old_offset= (off_t)lseek(Filedes, 0L, MY_SEEK_CUR)) == -1L || + error= (old_offset= lseek(Filedes, 0L, MY_SEEK_CUR)) == -1L || lseek(Filedes, offset, MY_SEEK_SET) == -1L; if (!error) /* Seek was successful */ @@ -116,7 +116,7 @@ uint my_pwrite(int Filedes, const byte *Buffer, uint Count, my_off_t offset, { #ifndef HAVE_PREAD int error= 0; - off_t old_offset; + os_off_t old_offset; writenbytes= (uint) -1; pthread_mutex_lock(&my_file_info[Filedes].mutex); @@ -124,7 +124,7 @@ uint my_pwrite(int Filedes, const byte *Buffer, uint Count, my_off_t offset, As we cannot change the file pointer, we save the old position, before seeking to the given offset */ - error= ((old_offset= (off_t)lseek(Filedes, 0L, MY_SEEK_CUR)) == -1L || + error= ((old_offset= lseek(Filedes, 0L, MY_SEEK_CUR)) == -1L || lseek(Filedes, offset, MY_SEEK_SET) == -1L); if (!error) /* Seek was successful */ From 8934e4f3cc2bdd6c4a24edd0cc6c96ee2325b0dc Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Tue, 27 Mar 2007 10:49:48 +0200 Subject: [PATCH 11/13] Bug#24985 - UTF8 ENUM primary key on MEMORY using BTREE causes incorrect duplicate entries Keys for BTREE indexes on ENUM and SET columns of MEMORY tables with character set UTF8 were computed incorrectly. Many different column values got the same key value. Apart of possible performance problems, it made unique indexes of this type unusable because it rejected many different values as duplicates. The problem was that multibyte character detection was tried on the internal numeric column value. Many values were not identified as characters. Their key value became blank filled. Thanks to Alexander Barkov and Ramil Kalimullin for the patch, which sets the character set of ENUM and SET key segments to the pseudo binary character set. --- mysql-test/r/heap_btree.result | 12 ++++++++++++ mysql-test/t/heap_btree.test | 17 +++++++++++++++++ sql/ha_heap.cc | 5 ++++- 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/heap_btree.result b/mysql-test/r/heap_btree.result index e6492e90b80..512a8a52845 100644 --- a/mysql-test/r/heap_btree.result +++ b/mysql-test/r/heap_btree.result @@ -280,4 +280,16 @@ a 1 1 drop table t1; +CREATE TABLE t1 ( +c1 ENUM('1', '2'), +UNIQUE USING BTREE(c1) +) ENGINE= MEMORY DEFAULT CHARSET= utf8; +INSERT INTO t1 VALUES('1'), ('2'); +DROP TABLE t1; +CREATE TABLE t1 ( +c1 SET('1', '2'), +UNIQUE USING BTREE(c1) +) ENGINE= MEMORY DEFAULT CHARSET= utf8; +INSERT INTO t1 VALUES('1'), ('2'); +DROP TABLE t1; End of 4.1 tests diff --git a/mysql-test/t/heap_btree.test b/mysql-test/t/heap_btree.test index 9aa820becd9..eb4672473f6 100644 --- a/mysql-test/t/heap_btree.test +++ b/mysql-test/t/heap_btree.test @@ -182,4 +182,21 @@ delete from t1 where a >= 2; select a from t1 order by a; drop table t1; +# +# Bug#24985 - UTF8 ENUM primary key on MEMORY using BTREE +# causes incorrect duplicate entries +# +CREATE TABLE t1 ( + c1 ENUM('1', '2'), + UNIQUE USING BTREE(c1) +) ENGINE= MEMORY DEFAULT CHARSET= utf8; +INSERT INTO t1 VALUES('1'), ('2'); +DROP TABLE t1; +CREATE TABLE t1 ( + c1 SET('1', '2'), + UNIQUE USING BTREE(c1) +) ENGINE= MEMORY DEFAULT CHARSET= utf8; +INSERT INTO t1 VALUES('1'), ('2'); +DROP TABLE t1; + --echo End of 4.1 tests diff --git a/sql/ha_heap.cc b/sql/ha_heap.cc index 3e981087df7..dd3a84aaaee 100644 --- a/sql/ha_heap.cc +++ b/sql/ha_heap.cc @@ -549,7 +549,10 @@ int ha_heap::create(const char *name, TABLE *table_arg, seg->start= (uint) key_part->offset; seg->length= (uint) key_part->length; seg->flag = 0; - seg->charset= field->charset(); + if (field->flags & (ENUM_FLAG | SET_FLAG)) + seg->charset= &my_charset_bin; + else + seg->charset= field->charset(); if (field->null_ptr) { seg->null_bit= field->null_bit; From 01be61e30770f64b0095a6aaf9b55db2369f1da8 Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Tue, 27 Mar 2007 12:39:31 +0200 Subject: [PATCH 12/13] Bug#24985 - UTF8 ENUM primary key on MEMORY using BTREE causes incorrect duplicate entries After merge fix --- mysql-test/t/heap_btree.test | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/mysql-test/t/heap_btree.test b/mysql-test/t/heap_btree.test index 14b1779bd1a..d2891943a4e 100644 --- a/mysql-test/t/heap_btree.test +++ b/mysql-test/t/heap_btree.test @@ -182,6 +182,23 @@ delete from t1 where a >= 2; select a from t1 order by a; drop table t1; +# +# Bug#24985 - UTF8 ENUM primary key on MEMORY using BTREE +# causes incorrect duplicate entries +# +CREATE TABLE t1 ( + c1 ENUM('1', '2'), + UNIQUE USING BTREE(c1) +) ENGINE= MEMORY DEFAULT CHARSET= utf8; +INSERT INTO t1 VALUES('1'), ('2'); +DROP TABLE t1; +CREATE TABLE t1 ( + c1 SET('1', '2'), + UNIQUE USING BTREE(c1) +) ENGINE= MEMORY DEFAULT CHARSET= utf8; +INSERT INTO t1 VALUES('1'), ('2'); +DROP TABLE t1; + --echo End of 4.1 tests # @@ -204,22 +221,4 @@ CREATE TABLE t1 (a INT, UNIQUE USING BTREE(a)) ENGINE=MEMORY; INSERT INTO t1 VALUES(NULL),(NULL); DROP TABLE t1; -# -# Bug#24985 - UTF8 ENUM primary key on MEMORY using BTREE -# causes incorrect duplicate entries -# -CREATE TABLE t1 ( - c1 ENUM('1', '2'), - UNIQUE USING BTREE(c1) -) ENGINE= MEMORY DEFAULT CHARSET= utf8; -INSERT INTO t1 VALUES('1'), ('2'); -DROP TABLE t1; -CREATE TABLE t1 ( - c1 SET('1', '2'), - UNIQUE USING BTREE(c1) -) ENGINE= MEMORY DEFAULT CHARSET= utf8; -INSERT INTO t1 VALUES('1'), ('2'); -DROP TABLE t1; - ---echo End of 4.1 tests --echo End of 5.0 tests From 85dce4b8f87590506e72b34d8e4e644a62f0ec65 Mon Sep 17 00:00:00 2001 From: "istruewing@chilla.local" <> Date: Wed, 28 Mar 2007 16:23:44 +0200 Subject: [PATCH 13/13] restored run-time thread lib detection --- include/my_pthread.h | 9 +++++++++ include/thr_alarm.h | 8 +++----- mysys/my_pthread.c | 10 +++++----- mysys/my_thr_init.c | 21 +++++++++++++++++++++ mysys/thr_alarm.c | 44 ++++++++++++++++++++++---------------------- sql/mysqld.cc | 34 +++++++++++++++++++--------------- sql/stacktrace.c | 17 ++--------------- sql/stacktrace.h | 2 -- 8 files changed, 81 insertions(+), 64 deletions(-) diff --git a/include/my_pthread.h b/include/my_pthread.h index 4df105e1b20..340dc32981a 100644 --- a/include/my_pthread.h +++ b/include/my_pthread.h @@ -701,6 +701,15 @@ extern uint my_thread_end_wait_time; Keep track of shutdown,signal, and main threads so that my_end() will not report errors with them */ + +/* Which kind of thread library is in use */ + +#define THD_LIB_OTHER 1 +#define THD_LIB_NPTL 2 +#define THD_LIB_LT 4 + +extern uint thd_lib_detected; + /* statistics_xxx functions are for not essential statistic */ #ifndef thread_safe_increment diff --git a/include/thr_alarm.h b/include/thr_alarm.h index 14dd538c9e4..a2694ba105b 100644 --- a/include/thr_alarm.h +++ b/include/thr_alarm.h @@ -24,11 +24,6 @@ extern "C" { #ifndef USE_ALARM_THREAD #define USE_ONE_SIGNAL_HAND /* One must call process_alarm */ #endif -#ifdef HAVE_LINUXTHREADS -#define THR_CLIENT_ALARM SIGALRM -#else -#define THR_CLIENT_ALARM SIGUSR1 -#endif #ifdef HAVE_rts_threads #undef USE_ONE_SIGNAL_HAND #define USE_ALARM_THREAD @@ -90,6 +85,9 @@ typedef struct st_alarm { my_bool malloced; } ALARM; +extern uint thr_client_alarm; +extern pthread_t alarm_thread; + #define thr_alarm_init(A) (*(A))=0 #define thr_alarm_in_use(A) (*(A)!= 0) void init_thr_alarm(uint max_alarm); diff --git a/mysys/my_pthread.c b/mysys/my_pthread.c index 74ba98c321a..db01602f4ab 100644 --- a/mysys/my_pthread.c +++ b/mysys/my_pthread.c @@ -31,6 +31,8 @@ uint thd_lib_detected= 0; +uint thd_lib_detected; + #ifndef my_pthread_setprio void my_pthread_setprio(pthread_t thread_id,int prior) { @@ -51,8 +53,6 @@ int my_pthread_getprio(pthread_t thread_id) int policy; if (!pthread_getschedparam(thread_id,&policy,&tmp_sched_param)) { - DBUG_PRINT("thread",("policy: %d priority: %d", - policy,tmp_sched_param.sched_priority)); return tmp_sched_param.sched_priority; } #endif @@ -314,8 +314,6 @@ void sigwait_handle_sig(int sig) pthread_mutex_unlock(&LOCK_sigwait); } -extern pthread_t alarm_thread; - void *sigwait_thread(void *set_arg) { sigset_t *set=(sigset_t*) set_arg; @@ -334,7 +332,9 @@ void *sigwait_thread(void *set_arg) sigaction(i, &sact, (struct sigaction*) 0); } } - sigaddset(set,THR_CLIENT_ALARM); + /* Ensure that init_thr_alarm() is called */ + DBUG_ASSERT(thr_client_alarm); + sigaddset(set, thr_client_alarm); pthread_sigmask(SIG_UNBLOCK,(sigset_t*) set,(sigset_t*) 0); alarm_thread=pthread_self(); /* For thr_alarm */ diff --git a/mysys/my_thr_init.c b/mysys/my_thr_init.c index 61e6e640027..464edb77f99 100644 --- a/mysys/my_thr_init.c +++ b/mysys/my_thr_init.c @@ -20,6 +20,7 @@ #include "mysys_priv.h" #include +#include #ifdef THREAD #ifdef USE_TLS @@ -63,6 +64,8 @@ nptl_pthread_exit_hack_handler(void *arg __attribute((unused))) #endif +static uint get_thread_lib(void); + /* initialize thread environment @@ -76,6 +79,8 @@ nptl_pthread_exit_hack_handler(void *arg __attribute((unused))) my_bool my_thread_global_init(void) { + thd_lib_detected= get_thread_lib(); + if (pthread_key_create(&THR_KEY_mysys,0)) { fprintf(stderr,"Can't initialize threads: error %d\n",errno); @@ -395,4 +400,20 @@ const char *my_thread_name(void) } #endif /* DBUG_OFF */ + +static uint get_thread_lib(void) +{ + char buff[64]; + +#ifdef _CS_GNU_LIBPTHREAD_VERSION + confstr(_CS_GNU_LIBPTHREAD_VERSION, buff, sizeof(buff)); + + if (!strncasecmp(buff, "NPTL", 4)) + return THD_LIB_NPTL; + if (!strncasecmp(buff, "linuxthreads", 12)) + return THD_LIB_LT; +#endif + return THD_LIB_OTHER; +} + #endif /* THREAD */ diff --git a/mysys/thr_alarm.c b/mysys/thr_alarm.c index 3fd70790281..57670c9ac14 100644 --- a/mysys/thr_alarm.c +++ b/mysys/thr_alarm.c @@ -34,6 +34,7 @@ #define ETIME ETIMEDOUT #endif +uint thr_client_alarm; static int alarm_aborted=1; /* No alarm thread */ my_bool thr_alarm_inited= 0; volatile my_bool alarm_thread_running= 0; @@ -56,9 +57,7 @@ static void *alarm_handler(void *arg); #define reschedule_alarms() pthread_kill(alarm_thread,THR_SERVER_ALARM) #endif -#if THR_CLIENT_ALARM != SIGALRM || defined(USE_ALARM_THREAD) static sig_handler thread_alarm(int sig __attribute__((unused))); -#endif static int compare_ulong(void *not_used __attribute__((unused)), byte *a_ptr,byte* b_ptr) @@ -77,9 +76,13 @@ void init_thr_alarm(uint max_alarms) sigfillset(&full_signal_set); /* Neaded to block signals */ pthread_mutex_init(&LOCK_alarm,MY_MUTEX_INIT_FAST); pthread_cond_init(&COND_alarm,NULL); -#if THR_CLIENT_ALARM != SIGALRM || defined(USE_ALARM_THREAD) - my_sigset(THR_CLIENT_ALARM,thread_alarm); + thr_client_alarm= thd_lib_detected == THD_LIB_LT ? SIGALRM : SIGUSR1; +#ifndef USE_ALARM_THREAD + if (thd_lib_detected != THD_LIB_LT) #endif + { + my_sigset(thr_client_alarm, thread_alarm); + } sigemptyset(&s); sigaddset(&s, THR_SERVER_ALARM); alarm_thread=pthread_self(); @@ -97,10 +100,11 @@ void init_thr_alarm(uint max_alarms) } #elif defined(USE_ONE_SIGNAL_HAND) pthread_sigmask(SIG_BLOCK, &s, NULL); /* used with sigwait() */ -#if THR_SERVER_ALARM == THR_CLIENT_ALARM - my_sigset(THR_CLIENT_ALARM,process_alarm); /* Linuxthreads */ - pthread_sigmask(SIG_UNBLOCK, &s, NULL); -#endif + if (thd_lib_detected == THD_LIB_LT) + { + my_sigset(thr_client_alarm, process_alarm); /* Linuxthreads */ + pthread_sigmask(SIG_UNBLOCK, &s, NULL); + } #else my_sigset(THR_SERVER_ALARM, process_alarm); pthread_sigmask(SIG_UNBLOCK, &s, NULL); @@ -152,7 +156,7 @@ my_bool thr_alarm(thr_alarm_t *alrm, uint sec, ALARM *alarm_data) now=(ulong) time((time_t*) 0); pthread_sigmask(SIG_BLOCK,&full_signal_set,&old_mask); - pthread_mutex_lock(&LOCK_alarm); /* Lock from threads & alarms */ + pthread_mutex_lock(&LOCK_alarm); /* Lock from threads & alarms */ if (alarm_aborted > 0) { /* No signal thread */ DBUG_PRINT("info", ("alarm aborted")); @@ -273,18 +277,17 @@ sig_handler process_alarm(int sig __attribute__((unused))) This must be first as we can't call DBUG inside an alarm for a normal thread */ -#if THR_SERVER_ALARM == THR_CLIENT_ALARM - if (!pthread_equal(pthread_self(),alarm_thread)) + if (thd_lib_detected == THD_LIB_LT && + !pthread_equal(pthread_self(),alarm_thread)) { #if defined(MAIN) && !defined(__bsdi__) printf("thread_alarm in process_alarm\n"); fflush(stdout); #endif #ifdef DONT_REMEMBER_SIGNAL - my_sigset(THR_CLIENT_ALARM,process_alarm); /* int. thread system calls */ + my_sigset(thr_client_alarm, process_alarm); /* int. thread system calls */ #endif return; } -#endif /* We have to do do the handling of the alarm in a sub function, @@ -328,7 +331,7 @@ static sig_handler process_alarm_part2(int sig __attribute__((unused))) alarm_data=(ALARM*) queue_element(&alarm_queue,i); alarm_data->alarmed=1; /* Info to thread */ if (pthread_equal(alarm_data->thread,alarm_thread) || - pthread_kill(alarm_data->thread, THR_CLIENT_ALARM)) + pthread_kill(alarm_data->thread, thr_client_alarm)) { #ifdef MAIN printf("Warning: pthread_kill couldn't find thread!!!\n"); @@ -352,7 +355,7 @@ static sig_handler process_alarm_part2(int sig __attribute__((unused))) alarm_data->alarmed=1; /* Info to thread */ DBUG_PRINT("info",("sending signal to waiting thread")); if (pthread_equal(alarm_data->thread,alarm_thread) || - pthread_kill(alarm_data->thread, THR_CLIENT_ALARM)) + pthread_kill(alarm_data->thread, thr_client_alarm)) { #ifdef MAIN printf("Warning: pthread_kill couldn't find thread!!!\n"); @@ -488,7 +491,7 @@ void thr_alarm_info(ALARM_INFO *info) ARGSUSED */ -#if THR_CLIENT_ALARM != SIGALRM || defined(USE_ALARM_THREAD) + static sig_handler thread_alarm(int sig) { #ifdef MAIN @@ -498,7 +501,6 @@ static sig_handler thread_alarm(int sig) my_sigset(sig,thread_alarm); /* int. thread system calls */ #endif } -#endif #ifdef HAVE_TIMESPEC_TS_SEC @@ -784,9 +786,7 @@ static void *signal_hand(void *arg __attribute__((unused))) sigaddset(&set,SIGINT); sigaddset(&set,SIGQUIT); sigaddset(&set,SIGTERM); -#if THR_CLIENT_ALARM != SIGHUP sigaddset(&set,SIGHUP); -#endif #ifdef SIGTSTP sigaddset(&set,SIGTSTP); #endif @@ -797,7 +797,7 @@ static void *signal_hand(void *arg __attribute__((unused))) puts("Starting signal handling thread"); #endif printf("server alarm: %d thread alarm: %d\n", - THR_SERVER_ALARM,THR_CLIENT_ALARM); + THR_SERVER_ALARM, thr_client_alarm); DBUG_PRINT("info",("Starting signal and alarm handling thread")); for(;;) { @@ -865,11 +865,11 @@ int main(int argc __attribute__((unused)),char **argv __attribute__((unused))) sigaddset(&set,SIGTSTP); #endif sigaddset(&set,THR_SERVER_ALARM); - sigdelset(&set,THR_CLIENT_ALARM); + sigdelset(&set, thr_client_alarm); (void) pthread_sigmask(SIG_SETMASK,&set,NULL); #ifdef NOT_USED sigemptyset(&set); - sigaddset(&set,THR_CLIENT_ALARM); + sigaddset(&set, thr_client_alarm); VOID(pthread_sigmask(SIG_UNBLOCK, &set, (sigset_t*) 0)); #endif diff --git a/sql/mysqld.cc b/sql/mysqld.cc index a0687929de5..c5933484675 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -195,12 +195,6 @@ inline void reset_floating_point_exceptions() } /* cplusplus */ - -#if defined(HAVE_LINUXTHREADS) -#define THR_KILL_SIGNAL SIGINT -#else -#define THR_KILL_SIGNAL SIGUSR2 // Can't use this with LinuxThreads -#endif #define MYSQL_KILL_SIGNAL SIGTERM #ifdef HAVE_GLIBC2_STYLE_GETHOSTBYNAME_R @@ -605,6 +599,7 @@ pthread_mutex_t LOCK_server_started; pthread_cond_t COND_server_started; int mysqld_server_started= 0; +static uint thr_kill_signal; File_parser_dummy_hook file_parser_dummy_hook; @@ -788,7 +783,7 @@ static void close_connections(void) DBUG_PRINT("info",("Waiting for select thread")); #ifndef DONT_USE_THR_ALARM - if (pthread_kill(select_thread,THR_CLIENT_ALARM)) + if (pthread_kill(select_thread, thr_client_alarm)) break; // allready dead #endif set_timespec(abstime, 2); @@ -2294,7 +2289,9 @@ static void init_signals(void) DBUG_ENTER("init_signals"); if (test_flags & TEST_SIGINT) - my_sigset(THR_KILL_SIGNAL,end_thread_signal); + { + my_sigset(thr_kill_signal, end_thread_signal); + } my_sigset(THR_SERVER_ALARM,print_signal_warning); // Should never be called! if (!(test_flags & TEST_NO_STACKTRACE) || (test_flags & TEST_CORE_ON_SIGNAL)) @@ -2351,8 +2348,12 @@ static void init_signals(void) #endif sigaddset(&set,THR_SERVER_ALARM); if (test_flags & TEST_SIGINT) - sigdelset(&set,THR_KILL_SIGNAL); // May be SIGINT - sigdelset(&set,THR_CLIENT_ALARM); // For alarms + { + // May be SIGINT + sigdelset(&set, thr_kill_signal); + } + // For alarms + sigdelset(&set, thr_client_alarm); sigprocmask(SIG_SETMASK,&set,NULL); pthread_sigmask(SIG_SETMASK,&set,NULL); DBUG_VOID_RETURN; @@ -2415,23 +2416,19 @@ pthread_handler_t signal_hand(void *arg __attribute__((unused))) */ init_thr_alarm(thread_scheduler.max_threads + global_system_variables.max_insert_delayed_threads + 10); -#if SIGINT != THR_KILL_SIGNAL - if (test_flags & TEST_SIGINT) + if (thd_lib_detected != THD_LIB_LT && (test_flags & TEST_SIGINT)) { (void) sigemptyset(&set); // Setup up SIGINT for debug (void) sigaddset(&set,SIGINT); // For debugging (void) pthread_sigmask(SIG_UNBLOCK,&set,NULL); } -#endif (void) sigemptyset(&set); // Setup up SIGINT for debug #ifdef USE_ONE_SIGNAL_HAND (void) sigaddset(&set,THR_SERVER_ALARM); // For alarms #endif #ifndef IGNORE_SIGHUP_SIGQUIT (void) sigaddset(&set,SIGQUIT); -#if THR_CLIENT_ALARM != SIGHUP (void) sigaddset(&set,SIGHUP); -#endif #endif (void) sigaddset(&set,SIGTERM); (void) sigaddset(&set,SIGTSTP); @@ -3626,6 +3623,13 @@ int main(int argc, char **argv) MY_INIT(argv[0]); // init my_sys library & pthreads /* nothing should come before this line ^^^ */ + /* Set signal used to kill MySQL */ +#if defined(SIGUSR2) + thr_kill_signal= thd_lib_detected == THD_LIB_LT ? SIGINT : SIGUSR2; +#else + thr_kill_signal= SIGINT; +#endif + /* Perform basic logger initialization logger. Should be called after MY_INIT, as it initializes mutexes. Log tables are inited later. diff --git a/sql/stacktrace.c b/sql/stacktrace.c index d8e9b7fd883..0dbf4522a71 100644 --- a/sql/stacktrace.c +++ b/sql/stacktrace.c @@ -55,19 +55,6 @@ void safe_print_str(const char* name, const char* val, int max_len) static my_bool is_nptl; -/* Check if we are using NPTL or LinuxThreads on Linux */ -void check_thread_lib(void) -{ - char buf[5]; - -#ifdef _CS_GNU_LIBPTHREAD_VERSION - confstr(_CS_GNU_LIBPTHREAD_VERSION, buf, sizeof(buf)); - is_nptl = !strncasecmp(buf, "NPTL", sizeof(buf)); -#else - is_nptl = 0; -#endif -} - #if defined(__alpha__) && defined(__GNUC__) /* The only way to backtrace without a symbol table on alpha @@ -173,8 +160,8 @@ terribly wrong...\n"); #endif /* __alpha__ */ /* We are 1 frame above signal frame with NPTL and 2 frames above with LT */ - sigreturn_frame_count = is_nptl ? 1 : 2; - + sigreturn_frame_count = thd_lib_detected == THD_LIB_LT ? 2 : 1; + while (fp < (uchar**) stack_bottom) { #if defined(__i386__) || defined(__x86_64__) diff --git a/sql/stacktrace.h b/sql/stacktrace.h index f5c92e54e1c..56b9877180a 100644 --- a/sql/stacktrace.h +++ b/sql/stacktrace.h @@ -27,11 +27,9 @@ extern char* heap_start; #define init_stacktrace() do { \ heap_start = (char*) &__bss_start; \ - check_thread_lib(); \ } while(0); void print_stacktrace(gptr stack_bottom, ulong thread_stack); void safe_print_str(const char* name, const char* val, int max_len); -void check_thread_lib(void); #endif /* (defined (__i386__) || (defined(__alpha__) && defined(__GNUC__))) */ #endif /* TARGET_OS_LINUX */