| List: | Commits | « Previous MessageNext Message » | |
| From: | dlenev | Date: | May 22 2007 3:43pm |
| Subject: | bk commit into 5.0 tree (dlenev:1.2490) BUG#23667 | ||
| View as plain text | |||
Below is the list of changes that have just been committed into a local 5.0 repository of dlenev. When dlenev does a push these changes will be propagated to the main repository and, within 24 hours after the push, to the public repository. For information on how to access the public repository see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html ChangeSet@stripped, 2007-05-22 17:43:40+04:00, dlenev@stripped +7 -0 5.0 version of fix for: Bug #23667 "CREATE TABLE LIKE is not isolated from alteration by other connections" Bug #18950 "CREATE TABLE LIKE does not obtain LOCK_open" As well as: Bug #25578 "CREATE TABLE LIKE does not require any privileges on source table". The first and the second bugs resulted in various errors and wrong binary log order when one tried to execute concurrently CREATE TABLE LIKE statement and DDL statements on source table or DML/DDL statements on its target table. The problem was caused by incomplete protection/table-locking against concurrent statements implemented in mysql_create_like_table() routine. We solve it by simply implementing such protection in proper way (see comment for sql_table.cc for details). The third bug allowed user who didn't have any privileges on table create its copy and therefore circumvent privilege check for SHOW CREATE TABLE. This patch solves this problem by adding privilege check, which was missing. Finally it also removes some duplicated code from mysql_create_like_table(). mysql-test/r/grant2.result@stripped, 2007-05-22 17:43:35+04:00, dlenev@stripped +24 -0 Added test for bug#25578 "CREATE TABLE LIKE does not require any privileges on source table". mysql-test/t/grant2.test@stripped, 2007-05-22 17:43:35+04:00, dlenev@stripped +44 -0 Added test for bug#25578 "CREATE TABLE LIKE does not require any privileges on source table". sql/handler.h@stripped, 2007-05-22 17:43:35+04:00, dlenev@stripped +1 -0 Introduced new flag for HA_CREATE_INFO::options in order to be able to distinguish CREATE TABLE ... LIKE from other types of CREATE TABLE. sql/mysql_priv.h@stripped, 2007-05-22 17:43:35+04:00, dlenev@stripped +2 -3 mysql_create_like_table() now takes source table name not as a Table_ident object but as regular table list element. sql/sql_parse.cc@stripped, 2007-05-22 17:43:35+04:00, dlenev@stripped +29 -8 CREATE TABLE ... LIKE implementation now uses statement's table list for storing information about the source table. We also use flag in LEX::create_info.options for distinguishing it from other types of CREATE TABLE. Finally CREATE TABLE ... LIKE now requires the same privileges on the source tables as SHOW CREATE TABLE. Moved this privilege check to check_show_create_table_access() function. sql/sql_table.cc@stripped, 2007-05-22 17:43:36+04:00, dlenev@stripped +43 -49 mysql_create_like_table(): - Provided proper protection from concurrent statements. This is achieved by keeping name-lock on the source table and holding LOCK_open mutex during whole operation. This gives protection against concurrent DDL on source table. Also holding this mutex makes copying of .frm file, call to ha_create_table() and binlogging atomic against concurrent DML and DDL operations on target table. - Get rid of duplicated code related to source database/table name handling. All these operations are already done in st_select_lex::add_table_to_list(), so we achieve the same effect by including source table into the statement's table list. sql/sql_yacc.yy@stripped, 2007-05-22 17:43:36+04:00, dlenev@stripped +4 -17 Now we use special flag in LEX::create_info::options for distinguishing CREATE TABLE ... LIKE from other types of CREATE TABLE and store name of source table as regular element in statement's table list. # This is a BitKeeper patch. What follows are the unified diffs for the # set of deltas contained in the patch. The rest of the patch, the part # that BitKeeper cares about, is below these diffs. # User: dlenev # Host: mockturtle.local # Root: /home/dlenev/src/mysql-5.0-like --- 1.183/sql/handler.h 2007-05-22 17:43:50 +04:00 +++ 1.184/sql/handler.h 2007-05-22 17:43:50 +04:00 @@ -162,6 +162,7 @@ #define HA_LEX_CREATE_TMP_TABLE 1 #define HA_LEX_CREATE_IF_NOT_EXISTS 2 +#define HA_LEX_CREATE_TABLE_LIKE 4 #define HA_OPTION_NO_CHECKSUM (1L << 17) #define HA_OPTION_NO_DELAY_KEY_WRITE (1L << 18) #define HA_MAX_REC_LENGTH 65535 --- 1.451/sql/mysql_priv.h 2007-05-22 17:43:51 +04:00 +++ 1.452/sql/mysql_priv.h 2007-05-22 17:43:51 +04:00 @@ -816,9 +816,8 @@ bool mysql_alter_table(THD *thd, char *n Alter_info *alter_info, uint order_num, ORDER *order, bool ignore); bool mysql_recreate_table(THD *thd, TABLE_LIST *table_list); -bool mysql_create_like_table(THD *thd, TABLE_LIST *table, - HA_CREATE_INFO *create_info, - Table_ident *src_table); +bool mysql_create_like_table(THD *thd, TABLE_LIST *table, TABLE_LIST *src_table, + HA_CREATE_INFO *create_info); bool mysql_rename_table(enum db_type base, const char *old_db, const char * old_name, --- 1.621/sql/sql_parse.cc 2007-05-22 17:43:51 +04:00 +++ 1.622/sql/sql_parse.cc 2007-05-22 17:43:51 +04:00 @@ -75,6 +75,7 @@ static bool check_db_used(THD *thd,TABLE static void remove_escape(char *name); static bool append_file_to_dir(THD *thd, const char **filename_ptr, const char *table_name); +static bool check_show_create_table_access(THD *thd, TABLE_LIST *table); const char *any_db="*any*"; // Special symbol for check_access @@ -3080,9 +3081,9 @@ mysql_execute_command(THD *thd) else { /* regular create */ - if (lex->name) - res= mysql_create_like_table(thd, create_table, &create_info, - (Table_ident *)lex->name); + if (lex->create_info.options & HA_LEX_CREATE_TABLE_LIKE) + res= mysql_create_like_table(thd, create_table, select_tables, + &create_info); else { res= mysql_create_table(thd, create_table->db, @@ -3319,11 +3320,7 @@ end_with_restore_list: first_table->skip_temporary= 1; if (check_db_used(thd, all_tables) || - check_access(thd, SELECT_ACL | EXTRA_ACL, first_table->db, - &first_table->grant.privilege, 0, 0, - test(first_table->schema_table))) - goto error; - if (grant_option && check_grant(thd, SELECT_ACL, all_tables, 2, UINT_MAX, 0)) + check_show_create_table_access(thd, first_table)) goto error; res= mysqld_show_create(thd, first_table); break; @@ -7519,6 +7516,25 @@ bool insert_precheck(THD *thd, TABLE_LIS } +/** + @brief Check privileges for SHOW CREATE TABLE statement. + + @param thd Thread context + @param table Target table + + @retval TRUE Failure + @retval FALSE Success +*/ + +static bool check_show_create_table_access(THD *thd, TABLE_LIST *table) +{ + return check_access(thd, SELECT_ACL | EXTRA_ACL, table->db, + &table->grant.privilege, 0, 0, + test(table->schema_table)) || + grant_option && check_grant(thd, SELECT_ACL, table, 2, UINT_MAX, 0); +} + + /* CREATE TABLE query pre-check @@ -7581,6 +7597,11 @@ bool create_table_precheck(THD *thd, TAB } #endif if (tables && check_table_access(thd, SELECT_ACL, tables,0)) + goto err; + } + else if (lex->create_info.options & HA_LEX_CREATE_TABLE_LIKE) + { + if (check_show_create_table_access(thd, tables)) goto err; } error= FALSE; --- 1.343/sql/sql_table.cc 2007-05-22 17:43:51 +04:00 +++ 1.344/sql/sql_table.cc 2007-05-22 17:43:51 +04:00 @@ -2718,7 +2718,8 @@ bool mysql_preload_keys(THD* thd, TABLE_ SYNOPSIS mysql_create_like_table() thd Thread object - table Table list (one table only) + table Table list element for target table + src_table Table list element for source table create_info Create info table_ident Src table_ident @@ -2727,61 +2728,53 @@ bool mysql_preload_keys(THD* thd, TABLE_ TRUE error */ -bool mysql_create_like_table(THD* thd, TABLE_LIST* table, - HA_CREATE_INFO *create_info, - Table_ident *table_ident) +bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST *src_table, + HA_CREATE_INFO *create_info) { TABLE **tmp_table; char src_path[FN_REFLEN], dst_path[FN_REFLEN]; char *db= table->db; char *table_name= table->table_name; - char *src_db; - char *src_table= table_ident->table.str; int err; bool res= TRUE; db_type not_used; - - TABLE_LIST src_tables_list; DBUG_ENTER("mysql_create_like_table"); - DBUG_ASSERT(table_ident->db.str); /* Must be set in the parser */ - src_db= table_ident->db.str; /* - Validate the source table + By taking name-lock on source table and holding LOCK_open mutex we + ensure that no concurrent DDL operation will mess with it. Note that + holding only name-lock is not enough for this because it won't block + other DDL statements that only take name-locks on the table and don't + open it (simple name-locks are not exclusive between each other). + + Unfortunately, simply opening table (i.e. acquiring shared meta-data + lock instead of exclusive meta-data lock) is not enough for our + purproses since in 5.0 ALTER TABLE may change .FRM files on disk even + if there are connections that still have old version of table open. + This 'optimization' is removed in 5.1 so there we open source table + instead of name-locking it. + + By acquiring LOCK_open we also make copying of .frm file, call to + ha_create_table() and binlogging atomic against concurrent DML + and DDL operations on target table. */ - if (table_ident->table.length > NAME_LEN || - (table_ident->table.length && - check_table_name(src_table,table_ident->table.length))) - { - my_error(ER_WRONG_TABLE_NAME, MYF(0), src_table); - DBUG_RETURN(TRUE); - } - if (!src_db || check_db_name(src_db)) - { - my_error(ER_WRONG_DB_NAME, MYF(0), src_db ? src_db : "NULL"); - DBUG_RETURN(-1); - } - - bzero((gptr)&src_tables_list, sizeof(src_tables_list)); - src_tables_list.db= src_db; - src_tables_list.table_name= src_table; - - if (lock_and_wait_for_table_name(thd, &src_tables_list)) + if (lock_and_wait_for_table_name(thd, src_table)) goto err; - if ((tmp_table= find_temporary_table(thd, src_db, src_table))) + pthread_mutex_lock(&LOCK_open); + + if ((tmp_table= find_temporary_table(thd, src_table->db, + src_table->table_name))) strxmov(src_path, (*tmp_table)->s->path, reg_ext, NullS); else { - strxmov(src_path, mysql_data_home, "/", src_db, "/", src_table, - reg_ext, NullS); + strxmov(src_path, mysql_data_home, "/", src_table->db, "/", + src_table->table_name, reg_ext, NullS); /* Resolve symlinks (for windows) */ fn_format(src_path, src_path, "", "", MYF(MY_UNPACK_FILENAME)); - if (lower_case_table_names) - my_casedn_str(files_charset_info, src_path); if (access(src_path, F_OK)) { - my_error(ER_BAD_TABLE_ERROR, MYF(0), src_table); + my_error(ER_BAD_TABLE_ERROR, MYF(0), src_table->table_name); goto err; } } @@ -2791,10 +2784,13 @@ bool mysql_create_like_table(THD* thd, T */ if (mysql_frm_type(thd, src_path, ¬_used) != FRMTYPE_TABLE) { - my_error(ER_WRONG_OBJECT, MYF(0), src_db, src_table, "BASE TABLE"); + my_error(ER_WRONG_OBJECT, MYF(0), src_table->db, src_table->table_name, + "BASE TABLE"); goto err; } + DBUG_EXECUTE_IF("sleep_create_like_before_check_if_exists", my_sleep(6000000);); + /* Validate the destination table @@ -2810,27 +2806,22 @@ bool mysql_create_like_table(THD* thd, T } else { - bool exists; strxmov(dst_path, mysql_data_home, "/", db, "/", table_name, reg_ext, NullS); fn_format(dst_path, dst_path, "", "", MYF(MY_UNPACK_FILENAME)); /* - Note that this critical section should actually cover most - of mysql_create_like_table() function. See bugs #18950 and - #23667 for more information. - Also note that starting from 5.1 we obtain name-lock on - target table instead of inspecting table cache for presence + Note that starting from 5.1 we obtain name-lock on target + table instead of inspecting table cache for presence of open placeholders (see comment in mysql_create_table()). */ - pthread_mutex_lock(&LOCK_open); - exists= (table_cache_has_open_placeholder(thd, db, table_name) || - !access(dst_path, F_OK)); - pthread_mutex_unlock(&LOCK_open); - if (exists) + if (table_cache_has_open_placeholder(thd, db, table_name) || + !access(dst_path, F_OK)) goto table_exists; } + DBUG_EXECUTE_IF("sleep_create_like_before_copy", my_sleep(6000000);); + /* Create a new table by copying from source table */ @@ -2843,6 +2834,8 @@ bool mysql_create_like_table(THD* thd, T goto err; } + DBUG_EXECUTE_IF("sleep_create_like_before_ha_create", my_sleep(6000000);); + /* As mysql_truncate don't work on a new table at this stage of creation, instead create the table directly (for both normal @@ -2867,6 +2860,8 @@ bool mysql_create_like_table(THD* thd, T goto err; /* purecov: inspected */ } + DBUG_EXECUTE_IF("sleep_create_like_before_binlogging", my_sleep(6000000);); + // Must be written before unlock if (mysql_bin_log.is_open()) { @@ -2891,8 +2886,7 @@ table_exists: my_error(ER_TABLE_EXISTS_ERROR, MYF(0), table_name); err: - pthread_mutex_lock(&LOCK_open); - unlock_table_name(thd, &src_tables_list); + unlock_table_name(thd, src_table); pthread_mutex_unlock(&LOCK_open); DBUG_RETURN(res); } --- 1.519/sql/sql_yacc.yy 2007-05-22 17:43:51 +04:00 +++ 1.520/sql/sql_yacc.yy 2007-05-22 17:43:51 +04:00 @@ -1483,7 +1483,6 @@ create: lex->create_info.options=$2 | $4; lex->create_info.db_type= (enum db_type) lex->thd->variables.table_type; lex->create_info.default_table_charset= NULL; - lex->name=0; } create2 { Lex->current_select= &Lex->select_lex; } @@ -2763,27 +2762,15 @@ create2: | opt_create_table_options create3 {} | LIKE table_ident { - LEX *lex=Lex; - THD *thd= lex->thd; - if (!(lex->name= (char *)$2)) + Lex->create_info.options|= HA_LEX_CREATE_TABLE_LIKE; + if (!Lex->select_lex.add_table_to_list(YYTHD, $2, NULL, 0, TL_READ)) MYSQL_YYABORT; - if ($2->db.str == NULL && - thd->copy_db_to(&($2->db.str), &($2->db.length))) - { - MYSQL_YYABORT; - } } | '(' LIKE table_ident ')' { - LEX *lex=Lex; - THD *thd= lex->thd; - if (!(lex->name= (char *)$3)) + Lex->create_info.options|= HA_LEX_CREATE_TABLE_LIKE; + if (!Lex->select_lex.add_table_to_list(YYTHD, $3, NULL, 0, TL_READ)) MYSQL_YYABORT; - if ($3->db.str == NULL && - thd->copy_db_to(&($3->db.str), &($3->db.length))) - { - MYSQL_YYABORT; - } } ; --- 1.33/mysql-test/r/grant2.result 2007-05-22 17:43:51 +04:00 +++ 1.34/mysql-test/r/grant2.result 2007-05-22 17:43:51 +04:00 @@ -380,3 +380,27 @@ drop function f2; drop table t2; REVOKE ALL PRIVILEGES, GRANT OPTION FROM `a@`@localhost; drop user `a@`@localhost; +drop database if exists mysqltest_1; +drop database if exists mysqltest_2; +drop user mysqltest_u1@localhost; +create database mysqltest_1; +create database mysqltest_2; +grant all on mysqltest_1.* to mysqltest_u1@localhost; +use mysqltest_2; +create table t1 (i int); +show create table mysqltest_2.t1; +ERROR 42000: SELECT command denied to user 'mysqltest_u1'@'localhost' for table 't1' +create table t1 like mysqltest_2.t1; +ERROR 42000: SELECT command denied to user 'mysqltest_u1'@'localhost' for table 't1' +grant select on mysqltest_2.t1 to mysqltest_u1@localhost; +show create table mysqltest_2.t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `i` int(11) default NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +create table t1 like mysqltest_2.t1; +use test; +drop database mysqltest_1; +drop database mysqltest_2; +drop user mysqltest_u1@localhost; +End of 5.0 tests --- 1.38/mysql-test/t/grant2.test 2007-05-22 17:43:51 +04:00 +++ 1.39/mysql-test/t/grant2.test 2007-05-22 17:43:51 +04:00 @@ -513,3 +513,47 @@ disconnect bug13310; connection default; REVOKE ALL PRIVILEGES, GRANT OPTION FROM `a@`@localhost; drop user `a@`@localhost; + + +# +# Bug#25578 "CREATE TABLE LIKE does not require any privileges on source table" +# +--disable_warnings +drop database if exists mysqltest_1; +drop database if exists mysqltest_2; +--enable_warnings +--error 0,ER_CANNOT_USER +drop user mysqltest_u1@localhost; + +create database mysqltest_1; +create database mysqltest_2; +grant all on mysqltest_1.* to mysqltest_u1@localhost; +use mysqltest_2; +create table t1 (i int); + +# Connect as user with all rights on mysqltest_1 but with no rights on mysqltest_2. +connect (user1,localhost,mysqltest_u1,,mysqltest_1); +connection user1; +# As expected error is emitted +--error ER_TABLEACCESS_DENIED_ERROR +show create table mysqltest_2.t1; +# This should emit error as well +--error ER_TABLEACCESS_DENIED_ERROR +create table t1 like mysqltest_2.t1; + +# Now let us check that SELECT privilege on the source is enough +connection default; +grant select on mysqltest_2.t1 to mysqltest_u1@localhost; +connection user1; +show create table mysqltest_2.t1; +create table t1 like mysqltest_2.t1; + +# Clean-up +connection default; +use test; +drop database mysqltest_1; +drop database mysqltest_2; +drop user mysqltest_u1@localhost; + +--echo End of 5.0 tests +
| Thread | ||
|---|---|---|
| • bk commit into 5.0 tree (dlenev:1.2490) BUG#23667 | dlenev | 22 May |
© 1995-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc.
Page generated in 0.430 sec. using MySQL 5.1.14-beta-log