Hi,
I see some pretty old gotchas are being fixed :-)
* Davi Arnaut <Davi.Arnaut@stripped> [10/03/08 21:49]:
> === modified file 'mysql-test/r/read_only_innodb.result'
> --- a/mysql-test/r/read_only_innodb.result 2009-12-09 15:56:34 +0000
> +++ b/mysql-test/r/read_only_innodb.result 2010-03-08 18:43:02 +0000
> @@ -46,3 +46,73 @@ UNLOCK TABLES;
> DROP TABLE t1;
> DROP USER test@localhost;
> echo End of 5.1 tests
> +#
> +# Bug#33669: Transactional temporary tables do not work under --read-only
> +#
> +DROP DATABASE IF EXISTS db1;
> +# Setup user and tables
> +CREATE USER bug33669@localhost;
> +CREATE DATABASE db1;
> +CREATE TABLE db1.t1 (a INT) ENGINE=INNODB;
> +CREATE TABLE db1.t2 (a INT) ENGINE=INNODB;
> +INSERT INTO db1.t1 VALUES (1);
> +INSERT INTO db1.t2 VALUES (2);
> +GRANT CREATE TEMPORARY TABLES, DROP, INSERT,
> +SELECT, LOCK TABLES ON db1.* TO bug33669@localhost;
> +SET GLOBAL READ_ONLY = ON;
> +# Connection con1 (user bug33669):
> +# Create, insert and drop temporary table:
> +CREATE TEMPORARY TABLE temp (a INT) ENGINE=INNODB;
> +INSERT INTO temp VALUES (1);
> +DROP TABLE temp;
> +# Lock base tables and use temporary table:
> +CREATE TEMPORARY TABLE temp (a INT) ENGINE=INNODB;
> +LOCK TABLES t1 READ, t2 READ;
> +SELECT * FROM t1;
> +a
> +1
> +INSERT INTO temp values (1);
> +SELECT * FROM t2;
> +a
> +2
> +UNLOCK TABLES;
> +DROP TABLE temp;
> +# Transaction
> +BEGIN;
> +SELECT * FROM t1;
> +a
> +1
> +CREATE TEMPORARY TABLE temp (a INT) ENGINE=INNODB;
> +INSERT INTO t1 VALUES (1);
> +ERROR HY000: The MySQL server is running with the --read-only option so it cannot
> execute this statement
> +INSERT INTO temp VALUES (1);
> +SELECT * FROM t2;
> +a
> +2
> +ROLLBACK;
> +SELECT * FROM temp;
> +a
> +DROP TABLE temp;
> +# Lock base table as READ and temporary table as WRITE:
> +CREATE TEMPORARY TABLE temp (a INT) ENGINE=INNODB;
> +LOCK TABLES t1 READ, temp WRITE;
> +SELECT * FROM t1;
> +a
> +1
> +SELECT * FROM temp;
> +a
> +INSERT INTO t1 VALUES (1);
> +ERROR HY000: The MySQL server is running with the --read-only option so it cannot
> execute this statement
> +INSERT INTO temp VALUES (1);
> +DROP TABLE temp;
> +UNLOCK TABLES;
> +# Lock temporary table that shadows a base table.
> +CREATE TEMPORARY TABLE t1 (a INT) ENGINE=INNODB;
> +LOCK TABLES t1 WRITE;
> +DROP TABLE t1;
> +SELECT * FROM t1;
> +ERROR HY000: Table 't1' was not locked with LOCK TABLES
> +# Disconnect and cleanup
> +SET GLOBAL READ_ONLY = OFF;
> +DROP USER bug33669@localhost;
> +DROP DATABASE db1;
Please add a test case for multi-insert into two temporary tables,
a mix of tmp and base tables,
multi-update, insert-select (select from a non-temp), an insert
that uses a subquery that is not using a temp table,
an insert <tmp> ... select <base> from (subquery <base>).
> -int mysql_lock_tables_check(THD *thd, TABLE **tables, uint count, uint flags)
> +static int
> +lock_tables_check(THD *thd, TABLE **tables, uint count,
> + bool *write_lock_used, uint flags)
> {
> + bool superuser;
Please consider calling the variable is_superuser.
> bool log_table_write_query;
> uint system_count;
> uint i;
OK, I haven't yet removed the loop from mysql_lock_tables(),
but it currently almost never works in 5.5, so moving the check
outside the loop is OK.
> - DBUG_ENTER("mysql_lock_tables_check");
> + DBUG_ENTER("lock_tables_check");
>
> system_count= 0;
> + *write_lock_used= FALSE;
> + superuser= thd->security_ctx->master_access & SUPER_ACL;
> log_table_write_query= (is_log_table_write_query(thd->lex->sql_command)
> || ((flags & MYSQL_LOCK_PERF_SCHEMA) != 0));
>
> @@ -148,10 +153,18 @@ int mysql_lock_tables_check(THD *thd, TA
> }
> }
>
> - if ((t->s->table_category == TABLE_CATEGORY_SYSTEM) &&
> - (t->reginfo.lock_type >= TL_WRITE_ALLOW_WRITE))
> + if (t->reginfo.lock_type >= TL_WRITE_ALLOW_WRITE)
> {
> - system_count++;
> + *write_lock_used= TRUE;
> +
> + if (t->s->table_category == TABLE_CATEGORY_SYSTEM)
> + system_count++;
> +
> + if (t->db_stat & HA_READ_ONLY)
> + {
> + my_error(ER_OPEN_AS_READONLY, MYF(0), t->alias);
> + DBUG_RETURN(1);
> + }
> }
>
> /*
> @@ -172,6 +185,20 @@ int mysql_lock_tables_check(THD *thd, TA
> thd->mdl_context.is_lock_owner(MDL_key::TABLE,
> t->s->db.str, t->s->table_name.str,
> MDL_SHARED)));
> +
> + /*
> + Prevent modifications to base tables if READ_ONLY is activated.
> + In any case, read only does not apply to temporary tables.
> + */
> + if (!(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) &&
> !t->s->tmp_table)
> + {
> + if (t->reginfo.lock_type >= TL_WRITE_ALLOW_WRITE &&
> + !superuser && opt_readonly && !thd->slave_thread)
> + {
> + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
> + DBUG_RETURN(1);
> + }
> + }
> }
>
> /*
> @@ -267,15 +294,15 @@ static void reset_lock_data_and_free(MYS
> MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count,
> uint flags, bool *need_reopen)
> {
> - MYSQL_LOCK *sql_lock;
> - TABLE *write_lock_used;
> int rc;
> + MYSQL_LOCK *sql_lock;
> + bool write_lock_used;
>
> DBUG_ENTER("mysql_lock_tables");
>
> *need_reopen= FALSE;
>
> - if (mysql_lock_tables_check(thd, tables, count, flags))
> + if (lock_tables_check(thd, tables, count, &write_lock_used, flags))
> DBUG_RETURN (NULL);
>
> ulong timeout= (flags & MYSQL_LOCK_IGNORE_TIMEOUT) ?
> @@ -283,8 +310,7 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd,
>
> for (;;)
> {
> - if (! (sql_lock= get_lock_data(thd, tables, count, GET_LOCK_STORE_LOCKS,
> - &write_lock_used)))
> + if (! (sql_lock= get_lock_data(thd, tables, count, GET_LOCK_STORE_LOCKS)))
> break;
>
> if (global_read_lock && write_lock_used &&
> @@ -308,21 +334,6 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd,
> }
> }
>
> - if (!(flags & MYSQL_LOCK_IGNORE_GLOBAL_READ_ONLY) &&
> - write_lock_used &&
> - opt_readonly &&
> - !(thd->security_ctx->master_access & SUPER_ACL) &&
> - !thd->slave_thread)
> - {
> - /*
> - Someone has issued SET GLOBAL READ_ONLY=1 and we want a write lock.
> - We do not wait for READ_ONLY=0, and fail.
> - */
> - reset_lock_data_and_free(&sql_lock);
> - my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only");
> - break;
> - }
> -
> thd_proc_info(thd, "System lock");
> DBUG_PRINT("info", ("thd->proc_info %s", thd->proc_info));
> if (sql_lock->table_count && lock_external(thd, sql_lock->table,
> @@ -459,9 +470,7 @@ void mysql_unlock_tables(THD *thd, MYSQL
> void mysql_unlock_some_tables(THD *thd, TABLE **table,uint count)
> {
> MYSQL_LOCK *sql_lock;
> - TABLE *write_lock_used;
> - if ((sql_lock= get_lock_data(thd, table, count, GET_LOCK_UNLOCK,
> - &write_lock_used)))
> + if ((sql_lock= get_lock_data(thd, table, count, GET_LOCK_UNLOCK)))
> mysql_unlock_tables(thd, sql_lock);
> }
>
> @@ -603,9 +612,7 @@ void mysql_lock_downgrade_write(THD *thd
> thr_lock_type new_lock_type)
> {
> MYSQL_LOCK *locked;
> - TABLE *write_lock_used;
> - if ((locked = get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK,
> - &write_lock_used)))
> + if ((locked = get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK)))
> {
> for (uint i=0; i < locked->lock_count; i++)
> thr_downgrade_write_lock(locked->locks[i], new_lock_type);
> @@ -619,11 +626,9 @@ void mysql_lock_downgrade_write(THD *thd
> void mysql_lock_abort(THD *thd, TABLE *table, bool upgrade_lock)
> {
> MYSQL_LOCK *locked;
> - TABLE *write_lock_used;
> DBUG_ENTER("mysql_lock_abort");
>
> - if ((locked= get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK,
> - &write_lock_used)))
> + if ((locked= get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK)))
> {
> for (uint i=0; i < locked->lock_count; i++)
> thr_abort_locks(locked->locks[i]->lock, upgrade_lock);
> @@ -648,12 +653,10 @@ void mysql_lock_abort(THD *thd, TABLE *t
> bool mysql_lock_abort_for_thread(THD *thd, TABLE *table)
> {
> MYSQL_LOCK *locked;
> - TABLE *write_lock_used;
> bool result= FALSE;
> DBUG_ENTER("mysql_lock_abort_for_thread");
>
> - if ((locked= get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK,
> - &write_lock_used)))
> + if ((locked= get_lock_data(thd, &table, 1, GET_LOCK_UNLOCK)))
> {
> for (uint i=0; i < locked->lock_count; i++)
> {
> @@ -848,11 +851,10 @@ static int unlock_external(THD *thd, TAB
> @param flags One of:
> - GET_LOCK_UNLOCK : If we should send TL_IGNORE to store lock
> - GET_LOCK_STORE_LOCKS : Store lock info in TABLE
> - @param write_lock_used Store pointer to last table with WRITE_ALLOW_WRITE
> */
>
> static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count,
> - uint flags, TABLE **write_lock_used)
> + uint flags)
> {
> uint i,tables,lock_count;
> MYSQL_LOCK *sql_lock;
> @@ -861,9 +863,8 @@ static MYSQL_LOCK *get_lock_data(THD *th
> DBUG_ENTER("get_lock_data");
>
> DBUG_ASSERT((flags == GET_LOCK_UNLOCK) || (flags == GET_LOCK_STORE_LOCKS));
> -
> DBUG_PRINT("info", ("count %d", count));
> - *write_lock_used=0;
> +
> for (i=tables=lock_count=0 ; i < count ; i++)
> {
> TABLE *t= table_ptr[i];
> @@ -895,24 +896,12 @@ static MYSQL_LOCK *get_lock_data(THD *th
> {
> TABLE *table;
> enum thr_lock_type lock_type;
> + THR_LOCK_DATA **org_locks = locks;
>
> if ((table=table_ptr[i])->s->tmp_table == NON_TRANSACTIONAL_TMP_TABLE)
> continue;
> lock_type= table->reginfo.lock_type;
> DBUG_ASSERT(lock_type != TL_WRITE_DEFAULT && lock_type !=
> TL_READ_DEFAULT);
> - if (lock_type >= TL_WRITE_ALLOW_WRITE)
> - {
> - *write_lock_used=table;
> - if (table->db_stat & HA_READ_ONLY)
> - {
> - my_error(ER_OPEN_AS_READONLY,MYF(0),table->alias);
> - /* Clear the lock type of the lock data that are stored already. */
> - sql_lock->lock_count= (uint) (locks - sql_lock->locks);
> - reset_lock_data_and_free(&sql_lock);
> - DBUG_RETURN(0);
> - }
> - }
> - THR_LOCK_DATA **org_locks = locks;
> locks_start= locks;
> locks= table->file->store_lock(thd, locks,
> (flags & GET_LOCK_UNLOCK) ? TL_IGNORE :
The patch is OK to push after fixing the above.
You should push into trunk-bugfixing, the new mdl is in the trunk.
