List:Commits« Previous MessageNext Message »
From:Mattias Jonsson Date:July 2 2010 7:01pm
Subject:Re: bzr commit into mysql-trunk-bugfixing branch (mattias.jonsson:3073)
Bug#49907
View as plain text  
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
Thread
bzr commit into mysql-trunk-bugfixing branch (mattias.jonsson:3073) Bug#49907Mattias Jonsson2 Jul
  • Re: bzr commit into mysql-trunk-bugfixing branch(mattias.jonsson:3073) Bug#49907Konstantin Osipov2 Jul
    • Re: bzr commit into mysql-trunk-bugfixing branch (mattias.jonsson:3073)Bug#49907Mattias Jonsson2 Jul
      • Re: bzr commit into mysql-trunk-bugfixing branch(mattias.jonsson:3073) Bug#49907Konstantin Osipov2 Jul