From: Dmitry Lenev
Date: October 5 2010 8:32am
Subject: Re: bzr commit into mysql-5.5-runtime branch (jon.hauglid:3152)
	Bug#57002
List-Archive: http://lists.mysql.com/commits/119951
Message-Id: <20101005083241.GA28253@mockturtle>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Hello Jon Olav!

* Dmitry Lenev <Dmitry.Lenev@stripped> [10/10/05 10:38]:
> * Jon Olav Hauglid <jon.hauglid@stripped> [10/10/04 13:32]:
> > === modified file 'sql/sql_base.cc'
> > --- a/sql/sql_base.cc	2010-09-30 13:29:12 +0000
> > +++ b/sql/sql_base.cc	2010-10-04 09:23:26 +0000
> > @@ -2742,8 +2742,11 @@ bool open_table(THD *thd, TABLE_LIST *ta
> >              distance >  0 - we have lock mode higher then we require
> >              distance == 0 - we have lock mode exactly which we need
> >            */
> > -          if ((best_distance < 0 && distance > best_distance) ||
> > -              (distance >= 0 && distance < best_distance))
> > +          if (((best_distance < 0 && distance > best_distance) ||
> > +               (distance >= 0 && distance < best_distance)) &&
> > +              ((flags & (MYSQL_OPEN_FORCE_SHARED_MDL |
> > +                         MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL)) ||
> > +               table->mdl_ticket->has_stronger_or_equal_type(table_list->mdl_request.type)))
> >            {
> >              best_distance= distance;
> >              best_table= table;
> > 
> 
> The following patch:
> 
> === modified file 'sql/sql_base.cc'
> --- sql/sql_base.cc     2010-09-30 13:29:12 +0000
> +++ sql/sql_base.cc     2010-10-05 05:59:40 +0000
> @@ -2730,6 +2730,12 @@ bool open_table(THD *thd, TABLE_LIST *ta
>            int distance= ((int) table->reginfo.lock_type -
>                           (int) table_list->lock_type);
>  
> +          if (table_list->mdl_request.type >= MDL_SHARED_NO_WRITE &&
> +              ! (flags & (MYSQL_OPEN_FORCE_SHARED_MDL |
> +                          MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL)) &&
> +              ! table->mdl_ticket->is_upgradable_or_exclusive())
> +            distance= -1;
> +
>            /*
>              Find a table that either has the exact lock type requested,
>              or has the best suitable lock. In case there is no locked
> 
> 
> fixes the bug as well.
> 
> OTOH it doesn't change error codes and messages in so many cases.
> 
> So maybe we should follow this approach instead?

I have discussed this bug with Kostja.

He thinks that solution for this MERGE-specific problem should be
localized to MERGE-specific code rather than change code handling
general case.

So here is yet another alternative fix which satisfies this
requirement:

=== modified file 'mysql-test/r/merge.result'
--- mysql-test/r/merge.result   2010-09-30 10:43:43 +0000
+++ mysql-test/r/merge.result   2010-10-05 08:22:56 +0000
@@ -3486,12 +3486,13 @@ ALTER TABLE m1 ADD INDEX (c1);
 UNLOCK TABLES;
 DROP TABLE m1, t1;
 #
-# Locking the merge table will implicitly lock children.
+# Locking the merge table won't implicitly lock children.
 #
 CREATE TABLE t1 (c1 INT);
 CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1);
 LOCK TABLE m1 WRITE;
 ALTER TABLE t1 ADD INDEX (c1);
+ERROR HY000: Table 't1' was locked with a READ lock and can't be updated
 LOCK TABLE m1 WRITE, t1 WRITE;
 ALTER TABLE t1 ADD INDEX (c1);
 UNLOCK TABLES;

=== modified file 'mysql-test/t/merge.test'
--- mysql-test/t/merge.test     2010-09-30 10:43:43 +0000
+++ mysql-test/t/merge.test     2010-10-05 08:22:19 +0000
@@ -2600,11 +2600,12 @@ UNLOCK TABLES;
 DROP TABLE m1, t1;
 
 --echo #
---echo # Locking the merge table will implicitly lock children.
+--echo # Locking the merge table won't implicitly lock children.
 --echo #
 CREATE TABLE t1 (c1 INT);
 CREATE TABLE m1 (c1 INT) ENGINE=MRG_MyISAM UNION=(t1);
 LOCK TABLE m1 WRITE;
+--error ER_TABLE_NOT_LOCKED_FOR_WRITE
 ALTER TABLE t1 ADD INDEX (c1);
 LOCK TABLE m1 WRITE, t1 WRITE;
 ALTER TABLE t1 ADD INDEX (c1);

=== modified file 'storage/myisammrg/ha_myisammrg.cc'
--- storage/myisammrg/ha_myisammrg.cc   2010-09-08 08:25:37 +0000
+++ storage/myisammrg/ha_myisammrg.cc   2010-10-05 08:05:35 +0000
@@ -478,8 +478,10 @@ int ha_myisammrg::add_children_list(void
     /* Set the expected table version, to not cause spurious re-prepare.
 * */
     child_l->set_table_ref_id(mrg_child_def->get_child_table_ref_type(),
                               mrg_child_def->get_child_def_version());
-    /* Use the same metadata lock type for children. */
-    child_l->mdl_request.set_type(parent_l->mdl_request.type);
+
+    if (! thd->locked_tables_mode &&
+        parent_l->mdl_request.type == MDL_SHARED_NO_WRITE)
+      child_l->mdl_request.set_type(MDL_SHARED_NO_WRITE);
     /* Link TABLE_LIST object into the children list. */
     if (this->children_last_l)
       child_l->prev_global= this->children_last_l;

What do you think about this approach?

-- 
Dmitry Lenev, Software Developer
Oracle Development SPB/MySQL, www.mysql.com

Are you MySQL certified?  http://www.mysql.com/certification