#At file:///opt/local/work/mysql-6.1-fk-stage/
2710 Konstantin Osipov 2008-12-12
WL#148, Milestone 8, Part 1.
Review fixes.
Dont' add cascading keys to check parent list.
Renames.
modified:
sql/fk.cc
sql/fk.h
sql/sql_delete.cc
sql/sql_insert.cc
sql/sql_load.cc
sql/sql_update.cc
=== modified file 'sql/fk.cc'
--- a/sql/fk.cc 2008-12-12 17:33:34 +0000
+++ b/sql/fk.cc 2008-12-12 18:58:51 +0000
@@ -222,13 +222,17 @@ private:
*/
bool
-Fk_constraint_list::prepare_for_insert(THD *thd, TABLE *table,
- MY_BITMAP *changed_column_map)
+Fk_constraint_list::prepare_check_parent(THD *thd, TABLE *table,
+ MY_BITMAP *changed_column_map)
{
List_iterator<Foreign_key_child_share> fk_it(table->s->fkeys_child);
Foreign_key_child_share *fk_share;
- m_fkeys_child.empty();
+ /*
+ The list is emptied in the constructor. Prepare must be called only
+ once.
+ */
+ DBUG_ASSERT(m_fkeys_child.is_empty());
/* No foreign key checks should be performed when FOREIGN_KEY_CHECKS is 0. */
if (thd->options & OPTION_NO_FOREIGN_KEY_CHECKS)
@@ -273,15 +277,15 @@ Fk_constraint_list::prepare_for_insert(T
*/
bool
-Fk_constraint_list::prepare_for_update(THD *thd, TABLE *table,
- bool old_row_is_record1,
- MY_BITMAP *changed_column_map)
+Fk_constraint_list::prepare_check_child(THD *thd, TABLE *table,
+ bool old_row_is_record1,
+ MY_BITMAP *changed_column_map)
{
List_iterator<Foreign_key_parent_share> fk_it(table->s->fkeys_parent);
Foreign_key_parent_share *fk_share;
- m_fkeys_parent.empty();
- m_fkeys_parent_cascade.empty();
+ DBUG_ASSERT(m_fkeys_parent.is_empty());
+ DBUG_ASSERT(m_fkeys_parent_cascade.is_empty());
/* No foreign key checks should be performed when FOREIGN_KEY_CHECKS is 0. */
if (thd->options & OPTION_NO_FOREIGN_KEY_CHECKS)
@@ -300,15 +304,14 @@ Fk_constraint_list::prepare_for_update(T
if (rctx->prepare(old_row_is_record1))
return TRUE;
- if (m_fkeys_parent.push_back(rctx, thd->mem_root))
- return TRUE;
-
- if (rctx->is_cascading() &&
- (rctx->prepare_cascade() ||
- m_fkeys_parent_cascade.push_back(rctx, thd->mem_root)))
+ if (rctx->is_cascading())
{
- return TRUE;
+ if (rctx->prepare_cascade() ||
+ m_fkeys_parent_cascade.push_back(rctx, thd->mem_root))
+ return TRUE;
}
+ else if (m_fkeys_parent.push_back(rctx, thd->mem_root))
+ return TRUE;
}
}
return FALSE;
@@ -342,7 +345,7 @@ Fk_constraint_list::do_check_parent_list
/*
There's a foreign key defined on this table and we are changing at
least one of columns participating in it (this is because only such
- foreign keys were added to the list by prepare_for_insert()).
+ foreign keys were added to the list by prepare_check_parent()).
*/
if (fk_ctx->check_parent_exists())
return TRUE;
@@ -412,52 +415,50 @@ Fk_constraint_list::do_check_child_list(
while ((fk_ctx= fkctx_it++))
{
- if (fk_ctx->ref_action() == FK_OPTION_NO_ACTION ||
- fk_ctx->ref_action() == FK_OPTION_RESTRICT)
- {
- if (fk_ctx->check_children_exist())
- return TRUE;
+ DBUG_ASSERT(! fk_ctx->is_cascading());
+
+ if (fk_ctx->check_children_exist())
+ return TRUE;
#ifdef ONCE_EOS_CHECKS_ARE_IMPLEMENTED
+ /*
+ For this constraint, for this row, there is no foreign-key referencing.
+ That is, the handler returned "no child exists".
+ */
+ continue 'for each constraint in constraint list' loop;
+
+ /* Handler returned "a child exists" */
+
+ if (fk_statement_flags&FK_STATEMENT_FLAG_NO_EOS == 0)
+ {
/*
- For this constraint, for this row, there is no foreign-key referencing.
- That is, the handler returned "no child exists".
+ End-of-statement check is possible. But that doesn't matter
+ unless it's a NO ACTION constraint.
*/
- continue 'for each constraint in constraint list' loop
+ if (UPDATE && constraint->fk_on_update_referential_action==FK_FLAG_NO_ACTION)
+ or (DELETE && constraint->fk_on_delete_referential_action==FK_FLAG_NO_ACTION)
+ need_eos_check= true;
+ continue 'for each constraint in constraint list' loop;
+ else /* referential_action is RESTRICT */
+ error(ER_FK_CHILD_VALUE_EXISTS);
+ }
- /* Handler returned "a child exists" */
+ /* End-of-statement check is not possible. */
- if (fk_statement_flags&FK_STATEMENT_FLAG_NO_EOS == 0)
- {
- /*
- End-of-statement check is possible. But that doesn't matter
- unless it's a NO ACTION constraint.
- */
- if (UPDATE && constraint->fk_on_update_referential_action==FK_FLAG_NO_ACTION)
- or (DELETE && constraint->fk_on_delete_referential_action==FK_FLAG_NO_ACTION)
- need_eos_check= true;
+ if (child->relationship == SELF_REFERENCING_TABLE)
+ if (child->is_self_referencing_relationship)
+ if (fk_check_record_in_memory(record) == TRUE)
+ /* The child row is in memory. That is, it's being deleted too. */
+ /* This is the reverse of the check we could make when inserting. */
continue 'for each constraint in constraint list' loop;
- else /* referential_action is RESTRICT */
- error(ER_FK_CHILD_VALUE_EXISTS);
- }
-
- /* End-of-statement check is not possible. */
-
- if (child->relationship == SELF_REFERENCING_TABLE)
- if (child->is_self_referencing_relationship)
- if (fk_check_record_in_memory(record) == TRUE)
- /* The child row is in memory. That is, it's being deleted too. */
- /* This is the reverse of the check we could make when inserting. */
- continue 'for each constraint in constraint list' loop;
- /*
- End-of-statement check is not possible, and there is no
- self-reference -- so the effect is "as if referential action is
- RESTRICT".
- */
- error(ER_FK_CHILD_VALUE_EXISTS);
+ /*
+ End-of-statement check is not possible, and there is no
+ self-reference -- so the effect is "as if referential action is
+ RESTRICT".
+ */
+ error(ER_FK_CHILD_VALUE_EXISTS);
#endif
- }
}
#ifdef ONCE_EOS_CHECKS_ARE_IMPLEMENTED
if (need_eos_check)
@@ -1046,8 +1047,8 @@ bool Foreign_key_parent_rcontext::prepar
if (! m_cascade_fk_list)
return TRUE;
- if (m_cascade_fk_list->prepare_for_update(m_thd, m_child_table, FALSE,
- &m_child_table->s->all_set))
+ if (m_cascade_fk_list->prepare_check_child(m_thd, m_child_table, FALSE,
+ &m_child_table->s->all_set))
return TRUE;
}
else if (m_ref_action != FK_OPTION_NO_ACTION &&
@@ -1066,10 +1067,10 @@ bool Foreign_key_parent_rcontext::prepar
if (! m_cascade_fk_list)
return TRUE;
- if (m_cascade_fk_list->prepare_for_insert(m_thd, m_child_table,
- m_child_table->write_set) ||
- m_cascade_fk_list->prepare_for_update(m_thd, m_child_table, TRUE,
- m_child_table->write_set))
+ if (m_cascade_fk_list->prepare_check_parent(m_thd, m_child_table,
+ m_child_table->write_set) ||
+ m_cascade_fk_list->prepare_check_child(m_thd, m_child_table, TRUE,
+ m_child_table->write_set))
return TRUE;
}
return FALSE;
=== modified file 'sql/fk.h'
--- a/sql/fk.h 2008-12-11 23:21:56 +0000
+++ b/sql/fk.h 2008-12-12 18:58:51 +0000
@@ -41,12 +41,12 @@ public:
inline trg_event_type action_type() const { return m_action_type; }
- bool prepare_for_insert(THD *thd, TABLE *table,
- MY_BITMAP *changed_column_map);
+ bool prepare_check_parent(THD *thd, TABLE *table,
+ MY_BITMAP *changed_column_map);
- bool prepare_for_update(THD *thd, TABLE *table,
- bool old_row_is_record1,
- MY_BITMAP *changed_column_map);
+ bool prepare_check_child(THD *thd, TABLE *table,
+ bool old_row_is_record1,
+ MY_BITMAP *changed_column_map);
/** Inline wrappers to speed up the most common case. */
bool check_parent_list(TABLE *table, bool use_eos)
=== modified file 'sql/sql_delete.cc'
--- a/sql/sql_delete.cc 2008-12-11 12:26:59 +0000
+++ b/sql/sql_delete.cc 2008-12-12 18:58:51 +0000
@@ -381,7 +381,7 @@ bool mysql_delete(THD *thd, TABLE_LIST *
table->mark_columns_needed_for_delete();
- if (fk_list.prepare_for_update(thd, table, FALSE, &table->s->all_set))
+ if (fk_list.prepare_check_child(thd, table, FALSE, &table->s->all_set))
{
end_read_record(&info);
delete select;
=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc 2008-12-12 17:33:34 +0000
+++ b/sql/sql_insert.cc 2008-12-12 18:58:51 +0000
@@ -684,7 +684,7 @@ bool mysql_insert(THD *thd,TABLE_LIST *t
/* Restore the current context. */
ctx_state.restore_state(context, table_list);
- if (fk_list.prepare_for_insert(thd, table, &table->s->all_set))
+ if (fk_list.prepare_check_parent(thd, table, &table->s->all_set))
goto abort;
/*
@@ -3124,7 +3124,7 @@ select_insert::prepare(List<Item> &value
if (!res)
{
prepare_triggers_for_insert_stmt(table);
- res= fk_list->prepare_for_insert(thd, table, &table->s->all_set);
+ res= fk_list->prepare_check_parent(thd, table, &table->s->all_set);
}
DBUG_RETURN(res);
@@ -3742,7 +3742,7 @@ select_create::prepare(List<Item> &value
DBUG_RETURN(1);
#ifdef WL_148_MILESTONE_12
- if (fk_list->prepare_for_insert(thd, table, &table->s->all_set))
+ if (fk_list->prepare_check_parent(thd, table, &table->s->all_set))
return TRUE;
#endif
=== modified file 'sql/sql_load.cc'
--- a/sql/sql_load.cc 2008-12-10 13:15:01 +0000
+++ b/sql/sql_load.cc 2008-12-12 18:58:51 +0000
@@ -267,7 +267,7 @@ int mysql_load(THD *thd,sql_exchange *ex
prepare_triggers_for_insert_stmt(table);
- if (fk_list.prepare_for_insert(thd, table, &table->s->all_set))
+ if (fk_list.prepare_check_parent(thd, table, &table->s->all_set))
DBUG_RETURN(TRUE);
uint tot_length=0;
=== modified file 'sql/sql_update.cc'
--- a/sql/sql_update.cc 2008-12-11 12:26:59 +0000
+++ b/sql/sql_update.cc 2008-12-12 18:58:51 +0000
@@ -545,8 +545,8 @@ int mysql_update(THD *thd,
table->mark_columns_needed_for_update();
- if (fk_list.prepare_for_insert(thd, table, table->write_set) ||
- fk_list.prepare_for_update(thd, table, TRUE, table->write_set))
+ if (fk_list.prepare_check_parent(thd, table, table->write_set) ||
+ fk_list.prepare_check_child(thd, table, TRUE, table->write_set))
goto err;
/* Check if we are modifying a key that we are used to search with */
| Thread |
|---|
| • bzr commit into mysql-6.1-fk branch (kostja:2710) WL#148 | Konstantin Osipov | 12 Dec |
