On 2010-07-02 12.15, Konstantin Osipov wrote:
> * Mattias Jonsson<mattias.jonsson@stripped> [10/07/02 13:01]:
>
> The concept of dd_* API is that it is always protected
> by a metadata lock. Why do you have to do a dirty disk read?
> You realize that this is not reliable, i.e. from binary logging
> point of view, right?
>>
>> 3073 Mattias Jonsson 2010-07-02
>> Bug#49907: ALTER TABLE ... TRUNCATE PARTITION does not wait for locks on
> the table
>
Thank you for looking at this!
I cannot see where the dirty disk read is done, but I see that I missed
to move the DBUG_ASSERT from dd_check_storage_engine_flag to
dd_frm_storage_engine. Is that what you mean?
The code right before the added check for ALTER_ADMIN_PARTITION in
open_and_lock_table_for_truncate (which I assume you mean) is either in
lock_tables_mode or acquired a MDL_SHARED_NO_READ_WRITE. But I now see
that I used the table_ref->table which is wrong.
If I add the following patch, does that make the patch correct?
(Also removes the second dd_frm_type call).
=== modified file 'sql/datadict.cc'
--- sql/datadict.cc 2010-07-02 08:38:18 +0000
+++ sql/datadict.cc 2010-07-02 18:33:42 +0000
@@ -86,6 +86,10 @@
enum legacy_db_type db_type;
LEX_STRING db_name = {(char *) db, strlen(db)};
+ /* There should be at least some lock on the table. */
+ DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, db,
+ table_name, MDL_SHARED));
+
if (check_db_name(&db_name))
{
my_error(ER_WRONG_DB_NAME, MYF(0), db_name.str);
@@ -136,10 +140,6 @@
{
handlerton *table_type;
- /* There should be at least some lock on the table. */
- DBUG_ASSERT(thd->mdl_context.is_lock_owner(MDL_key::TABLE, db,
- table_name, MDL_SHARED));
-
if (dd_frm_storage_engine(thd, db, table_name, &table_type))
return TRUE;
=== modified file 'sql/sql_truncate.cc'
--- sql/sql_truncate.cc 2010-07-02 08:38:18 +0000
+++ sql/sql_truncate.cc 2010-07-02 18:51:50 +0000
@@ -244,6 +244,7 @@
MDL_ticket
**ticket_downgrade)
{
TABLE *table= NULL;
+ handlerton *table_type;
MDL_ticket *mdl_ticket= NULL;
DBUG_ENTER("open_and_lock_table_for_truncate");
@@ -266,7 +267,8 @@
table_ref->table_name,
FALSE)))
DBUG_RETURN(TRUE);
- *hton_can_recreate= ha_check_storage_engine_flag(table->s->db_type(),
+ table_type= table->s->db_type();
+ *hton_can_recreate= ha_check_storage_engine_flag(table_type,
HTON_CAN_RECREATE);
}
else
@@ -290,9 +292,11 @@
mdl_ticket= mdl_request.ticket;
- if (dd_check_storage_engine_flag(thd, table_ref->db,
table_ref->table_name,
- HTON_CAN_RECREATE, hton_can_recreate))
+ if (dd_frm_storage_engine(thd, table_ref->db, table_ref->table_name,
+ &table_type))
DBUG_RETURN(TRUE);
+ *hton_can_recreate= ha_check_storage_engine_flag(table_type,
+ HTON_CAN_RECREATE);
}
#ifdef WITH_PARTITION_STORAGE_ENGINE
@@ -300,22 +304,11 @@
TODO: Add support for TRUNCATE PARTITION for NDB and other engines
supporting native partitioning.
*/
- if (thd->lex->alter_info.flags & ALTER_ADMIN_PARTITION)
+ if (thd->lex->alter_info.flags & ALTER_ADMIN_PARTITION &&
+ table_type != partition_hton)
{
- handlerton *table_type;
- if (table_ref->table)
- table_type= table_ref->table->s->db_type();
- else
- {
- if (dd_frm_storage_engine(thd, table_ref->db, table_ref->table_name,
- &table_type))
- DBUG_RETURN(TRUE);
- }
- if (table_type != partition_hton)
- {
- my_error(ER_PARTITION_MGMT_ON_NONPARTITIONED, MYF(0));
- DBUG_RETURN(TRUE);
- }
+ my_error(ER_PARTITION_MGMT_ON_NONPARTITIONED, MYF(0));
+ DBUG_RETURN(TRUE);
}
#endif
DEBUG_SYNC(thd, "lock_table_for_truncate");
Thank you for spotting this!
Regards
Mattias
