summaryrefslogtreecommitdiff
path: root/storage/myisammrg
diff options
context:
space:
mode:
authorDmitry Lenev <Dmitry.Lenev@oracle.com>2011-07-22 16:31:10 +0400
committerDmitry Lenev <Dmitry.Lenev@oracle.com>2011-07-22 16:31:10 +0400
commit0b5b1dd197a7e93b6b2aea151b15a4aa563cc209 (patch)
treebd7bd2798108c490f31052385cf4c683f6a831e3 /storage/myisammrg
parentbcb4a861fdd08a0b8f1d97cca415d2c30a38f6e4 (diff)
downloadmariadb-git-0b5b1dd197a7e93b6b2aea151b15a4aa563cc209.tar.gz
Fix for bug #11754210 - "45777: CHECK TABLE DOESN'T
SHOW ALL PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1". The problem was that CHECK/REPAIR TABLE for a MERGE table which had several children missing or in wrong engine reported only issue with the first such table in its result-set. While in 5.0 this statement returned the whole list of problematic tables. Ability to report problems for all children was lost during significant refactorings of MERGE code which were done as part of work on 5.1 and 5.5 releases. This patch restores status quo ante refactorings by changing code in such a way that: 1) Failure to open child table due to its absence during CHECK/ REPAIR TABLE for a MERGE table is not reported immediately when its absence is discovered in open_tables(). Instead handling/error reporting in such a situation is postponed until the moment when children are attached. 2) Code performing attaching of children no longer stops when it encounters first problem with one of the children during CHECK/REPAIR TABLE. Instead it continues iteration through the child list until all problems caused by child absence/ wrong engine are reported. Note that even after this change problem with mismatch of child/parent definition won't be reported if there is also another child missing, but this is how it was in 5.0 as well. mysql-test/r/merge.result: Added test case for bug #11754210 - "45777: CHECK TABLE DOESN'T SHOW ALL PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1". Adjusted results of existing tests to the fact that CHECK/REPAIR TABLE statements now try to report problems about missing table/ wrong engine for all underlying tables, and to the fact that mismatch of parent/child definitions is always reported as an error and not a warning. mysql-test/t/merge.test: Added test case for bug #11754210 - "45777: CHECK TABLE DOESN'T SHOW ALL PROBLEMS FOR MERGE TABLE COMPLIANCE IN 5.1". sql/sql_base.cc: Changed code responsible for opening tables to ignore the fact that underlying tables of a MERGE table are missing, if this table is opened for CHECK/REPAIR TABLE. The absence of underlying tables in this case is now detected and appropriate error is reported at the point when child tables are attached. At this point we can produce full list of problematic child tables/errors to be returned as part of CHECK/REPAIR TABLE result-set. storage/myisammrg/ha_myisammrg.cc: Changed myisammrg_attach_children_callback() to handle new situation, when during CHECK/REPAIR TABLE we do not report error about missing child immediately when this fact is discovered during open_tables() but postpone error-reporting till the time when children are attached. Also this callback is now responsible for pushing an error mentioning problematic child table to the list of errors to be reported by CHECK/REPAIR TABLE statements. Finally, since now myrg_attach_children() no longer relies on return value from callback to determine the end of the children list, callback no longer needs to set my_errno value and can be simplified. Changed myrg_print_wrong_table() to always report a problem with child table as an error and not as a warning. This makes reporting for different types of issues with child tables more consistent and compatible with 5.0 behavior. storage/myisammrg/myrg_open.c: Changed code in myrg_attach_children() not to abort on the first problem with a child table when attaching children to parent MERGE table during CHECK/REPAIR TABLE statement execution. This allows CHECK/REPAIR TABLE to report problems about absence/wrong engine for all underlying tables as part of their result-set.
Diffstat (limited to 'storage/myisammrg')
-rw-r--r--storage/myisammrg/ha_myisammrg.cc70
-rw-r--r--storage/myisammrg/myrg_open.c24
2 files changed, 65 insertions, 29 deletions
diff --git a/storage/myisammrg/ha_myisammrg.cc b/storage/myisammrg/ha_myisammrg.cc
index 233f63ba680..c76e9ee0bfe 100644
--- a/storage/myisammrg/ha_myisammrg.cc
+++ b/storage/myisammrg/ha_myisammrg.cc
@@ -159,9 +159,14 @@ extern "C" void myrg_print_wrong_table(const char *table_name)
buf[db.length]= '.';
memcpy(buf + db.length + 1, name.str, name.length);
buf[db.length + name.length + 1]= 0;
- push_warning_printf(current_thd, MYSQL_ERROR::WARN_LEVEL_WARN,
- ER_ADMIN_WRONG_MRG_TABLE, ER(ER_ADMIN_WRONG_MRG_TABLE),
- buf);
+ /*
+ Push an error to be reported as part of CHECK/REPAIR result-set.
+ Note that calling my_error() from handler is a hack which is kept
+ here to avoid refactoring. Normally engines should report errors
+ through return value which will be interpreted by caller using
+ handler::print_error() call.
+ */
+ my_error(ER_ADMIN_WRONG_MRG_TABLE, MYF(0), buf);
}
@@ -593,8 +598,7 @@ public:
@return pointer to open MyISAM table structure
@retval !=NULL OK, returning pointer
- @retval NULL, my_errno == 0 Ok, no more child tables
- @retval NULL, my_errno != 0 error
+ @retval NULL, Error.
@detail
This function retrieves the MyISAM table handle from the
@@ -614,17 +618,33 @@ extern "C" MI_INFO *myisammrg_attach_children_callback(void *callback_param)
MI_INFO *myisam= NULL;
DBUG_ENTER("myisammrg_attach_children_callback");
- if (!child_l)
- {
- DBUG_PRINT("myrg", ("No more children to attach"));
- my_errno= 0; /* Ok, no more child tables. */
- goto end;
- }
+ /*
+ Number of children in the list and MYRG_INFO::tables_count,
+ which is used by caller of this function, should always match.
+ */
+ DBUG_ASSERT(child_l);
+
child= child_l->table;
/* Prepare for next child. */
param->next();
/*
+ When MERGE table is opened for CHECK or REPAIR TABLE statements,
+ failure to open any of underlying tables is ignored until this moment
+ (this is needed to provide complete list of the problematic underlying
+ tables in CHECK/REPAIR TABLE output).
+ Here we detect such a situation and report an appropriate error.
+ */
+ if (! child)
+ {
+ DBUG_PRINT("error", ("failed to open underlying table '%s'.'%s'",
+ child_l->db, child_l->table_name));
+ /* This should only happen inside of CHECK/REPAIR TABLE. */
+ DBUG_ASSERT(current_thd->open_options & HA_OPEN_FOR_REPAIR);
+ goto end;
+ }
+
+ /*
Do a quick compatibility check. The table def version is set when
the table share is created. The child def version is copied
from the table def version after a successful compatibility check.
@@ -653,7 +673,6 @@ extern "C" MI_INFO *myisammrg_attach_children_callback(void *callback_param)
{
DBUG_PRINT("error", ("temporary table mismatch parent: %d child: %d",
parent->s->tmp_table, child->s->tmp_table));
- my_errno= HA_ERR_WRONG_MRG_TABLE_DEF;
goto end;
}
@@ -664,12 +683,27 @@ extern "C" MI_INFO *myisammrg_attach_children_callback(void *callback_param)
DBUG_PRINT("error", ("no MyISAM handle for child table: '%s'.'%s' 0x%lx",
child->s->db.str, child->s->table_name.str,
(long) child));
- my_errno= HA_ERR_WRONG_MRG_TABLE_DEF;
}
- DBUG_PRINT("myrg", ("MyISAM handle: 0x%lx my_errno: %d",
- my_errno ? 0L : (long) myisam, my_errno));
+
+ DBUG_PRINT("myrg", ("MyISAM handle: 0x%lx", (long) myisam));
end:
+
+ if (!myisam &&
+ (current_thd->open_options & HA_OPEN_FOR_REPAIR))
+ {
+ char buf[2*NAME_LEN + 1 + 1];
+ strxnmov(buf, sizeof(buf) - 1, child_l->db, ".", child_l->table_name, NULL);
+ /*
+ Push an error to be reported as part of CHECK/REPAIR result-set.
+ Note that calling my_error() from handler is a hack which is kept
+ here to avoid refactoring. Normally engines should report errors
+ through return value which will be interpreted by caller using
+ handler::print_error() call.
+ */
+ my_error(ER_ADMIN_WRONG_MRG_TABLE, MYF(0), buf);
+ }
+
DBUG_RETURN(myisam);
}
@@ -783,12 +817,6 @@ int ha_myisammrg::attach_children(void)
/* Must call this with children list in place. */
DBUG_ASSERT(this->table->pos_in_table_list->next_global == this->children_l);
- /*
- 'my_errno' is set by myisammrg_attach_children_callback() in
- case of an error.
- */
- my_errno= 0;
-
if (myrg_attach_children(this->file, this->test_if_locked |
current_thd->open_options,
myisammrg_attach_children_callback, &param,
diff --git a/storage/myisammrg/myrg_open.c b/storage/myisammrg/myrg_open.c
index 533e5d13cdd..d51036fc69d 100644
--- a/storage/myisammrg/myrg_open.c
+++ b/storage/myisammrg/myrg_open.c
@@ -385,6 +385,7 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking,
uint UNINIT_VAR(key_parts);
uint min_keys;
my_bool bad_children= FALSE;
+ my_bool first_child= TRUE;
DBUG_ENTER("myrg_attach_children");
DBUG_PRINT("myrg", ("handle_locking: %d", handle_locking));
@@ -399,16 +400,26 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking,
errpos= 0;
file_offset= 0;
min_keys= 0;
- child_nr= 0;
- while ((myisam= (*callback)(callback_param)))
+ for (child_nr= 0; child_nr < m_info->tables; child_nr++)
{
+ if (! (myisam= (*callback)(callback_param)))
+ {
+ if (handle_locking & HA_OPEN_FOR_REPAIR)
+ {
+ /* An appropriate error should've been already pushed by callback. */
+ bad_children= TRUE;
+ continue;
+ }
+ goto bad_children;
+ }
+
DBUG_PRINT("myrg", ("child_nr: %u table: '%s'",
child_nr, myisam->filename));
- DBUG_ASSERT(child_nr < m_info->tables);
/* Special handling when the first child is attached. */
- if (!child_nr)
+ if (first_child)
{
+ first_child= FALSE;
m_info->reclength= myisam->s->base.reclength;
min_keys= myisam->s->base.keys;
key_parts= myisam->s->base.key_parts;
@@ -456,14 +467,11 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking,
for (idx= 0; idx < key_parts; idx++)
m_info->rec_per_key_part[idx]+= (myisam->s->state.rec_per_key_part[idx] /
m_info->tables);
- child_nr++;
}
if (bad_children)
goto bad_children;
- /* Note: callback() resets my_errno, so it is safe to check it here */
- if (my_errno == HA_ERR_WRONG_MRG_TABLE_DEF)
- goto err;
+
if (sizeof(my_off_t) == 4 && file_offset > (ulonglong) (ulong) ~0L)
{
my_errno= HA_ERR_RECORD_FILE_FULL;