On 10/05/2010 10:32 AM, Dmitry Lenev wrote:
Hello!
> 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.
In general, I agree with this line of thinking.
Especially if not checking MDL lock type when finding
the TABLE instance to use, will always remain a MERGE-specific issue.
> === 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?
My concern is that this will make MERGE tables conceptually harder to
understand by adding a special case.
I also think that the code for selecting a correct TABLE instance
could use a slight refactoring along the lines you mentioned.
But this is maybe not the time and place for it?
--- Jon Olav
