#At file:///home/dlenev/src/bzr/mysql-6.1-mil13/ based on revid:dlenev@stripped
2725 Dmitry Lenev 2009-06-28
WL#148 "Foreign keys".
Milestone 13 "DDL checks and changes: ALTER, CREATE INDEX, DROP INDEX words".
Work in progress.
Added descriptions for several functions and methods.
modified:
sql/fk.cc
sql/sql_class.h
sql/sql_table.cc
=== modified file 'sql/fk.cc'
--- a/sql/fk.cc 2009-06-25 08:43:04 +0000
+++ b/sql/fk.cc 2009-06-28 06:37:40 +0000
@@ -523,7 +523,22 @@ Fk_constraint_list::prepare_check_parent
/**
- TODO/FIXME Add comment.
+ For ALTER TABLE operation on the table, prepare runtime contexts for foreign
+ keys which are added by this statement and which may require checking.
+
+ @param thd Thread context.
+ @param db Name of database for table being altered.
+ @param table_name Name of table being altered.
+ @param table TABLE object representing new version of table being
+ altered.
+ @param table_2 Second instance of TABLE object for new version of table
+ being altered (required for checking self-referencing
+ foreign keys).
+ @param fk_list List of foreign keys in new version of table, with new
+ foreign keys marked.
+
+ @retval FALSE Success.
+ @retval TRUE Error, out of resources.
*/
bool
@@ -537,7 +552,6 @@ Fk_constraint_list::prepare_alter_check_
List_iterator<Foreign_key_child> fk_def_it(fk_list);
Foreign_key_child *fk_def;
-
/*
The list is emptied in the constructor. Prepare must be called only
once.
@@ -1246,7 +1260,14 @@ bool Foreign_key_child_rcontext::prepare
/**
- FIXME/TODO: Add comment.
+ Prepare a foreign key runtime context for checks performed when a
+ statement modifies a child table.
+
+ @param parent_table TABLE instance for the parent table to be used
+ for checks.
+
+ @retval FALSE Success.
+ @retval TRUE Error (data-dictionary inconsistency, OOM, etc).
*/
bool Foreign_key_child_rcontext::prepare(TABLE *parent_table)
=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h 2009-06-25 08:43:04 +0000
+++ b/sql/sql_class.h 2009-06-28 06:37:40 +0000
@@ -228,6 +228,10 @@ public:
enum drop_type {KEY, COLUMN, FOREIGN_KEY };
const char *name;
enum drop_type type;
+ /**
+ Indicates that this Alter_drop instance represents deprecated
+ DROP FOREIGN KEY clause.
+ */
bool is_fkey_clause;
Alter_drop(enum drop_type par_type,const char *par_name, bool par_fkey_clause)
:name(par_name), type(par_type), is_fkey_clause(par_fkey_clause) {}
@@ -437,7 +441,17 @@ public:
Tells if constraint name was generated rather than specified explicitly.
*/
bool is_name_generated;
- /** TODO/FIXME Add comment. */
+ /**
+ Indicates whether ALTER TABLE statement going to replace an existing
+ foreign key with this foreign key (i.e. this ALTER TABLE drops old
+ foreign key and creates new one with the same name).
+
+ Code that checks if name of new foreign key conflicts with name of
+ existing constraint does not produce error in such case even although
+ at the momement when this check is performed there still exists .CNS
+ file for the name of foreign key being created which corresponds to
+ the old foreign key.
+ */
bool replaces_existing;
/**
Pointer to string with name of unique constraint in the parent
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2009-06-26 09:38:32 +0000
+++ b/sql/sql_table.cc 2009-06-28 06:37:40 +0000
@@ -5232,7 +5232,31 @@ prepare_create_table(TABLE_LIST *create_
/**
- TODO/FIXME: Add comment.
+ Prepare context for ALTER TABLE operation by preparing Parent_info
+ objects for related tables with removed descriptions of foreign keys
+ to be dropped and with added descriptions of foreign keys to be created.
+
+ @param[in] alter_table Table list element for table to be
+ altered.
+ @param[in] fkey_tables List of parent tables for foreign keys
+ to be created.
+ @param[in] drop_list List of Alter_drop objects some of which
+ contain names of foreign keys to be
+ dropped.
+ @param[in] foreign_key_list List of foreign keys to be created.
+ @param[in] alter_table_parent_info Parent_info for table being altered.
+ @param[out] parent_tables On return contains unique list of parent
+ tables for foreign keys to be dropped
+ which are not among parent tables for
+ foreign keys being created.
+
+ @note For self-referencing foreign keys final steps of preparing Parent_info
+ for table being altered happen in mysql_prepare_alter_table(), since
+ at the point where this method is called we don't have description of
+ table being altered yet.
+
+ @retval FALSE Success.
+ @retval TRUE Failure (e.g. OOM or absence of one of the parent tables).
*/
bool Foreign_key_ddl_rcontext::
@@ -7921,13 +7945,29 @@ blob_length_by_type(enum_field_types typ
/**
- TODO/FIXME: Add comment.
+ Check that column which is going to be changed or deleted by ALTER TABLE
+ participates in existing foreign key (as one of child or parent columns)
+ and if it is true emit error that such statement is illegal.
+
+ @param col_name Name of column.
+ @param fk_list List of foreign keys in which table being altered
+ participates as child.
+ @param parent_fk_list List of foreign keys in which table being altered
+ participates as parent.
+ @param statement Statement which to be used in error message.
+ @param table_name Name of table being altered to be used in error
+ message.
+
+ @retval FALSE Column does not participate in any existing foreign key.
+ @retval TRUE Column participates in one of foreign keys, error was
+ emitted.
*/
static bool
column_participates_in_fk(const char *col_name,
List<Foreign_key_child> &fk_list,
- List<Foreign_key_parent> &parent_fk_list)
+ List<Foreign_key_parent> &parent_fk_list,
+ const char *statement, const char *table_name)
{
List_iterator<Foreign_key_child> fk_c_it(fk_list);
List_iterator<Foreign_key_parent> fk_p_it(parent_fk_list);
@@ -7943,7 +7983,11 @@ column_participates_in_fk(const char *co
while ((col= col_it++))
if (! my_strcasecmp(system_charset_info, col_name, col->field_name.str))
+ {
+ /* QQ: should not we use a nicer error message? */
+ my_error(ER_FK_STATEMENT_ILLEGAL, MYF(0), statement, table_name);
return TRUE;
+ }
}
}
@@ -7956,7 +8000,10 @@ column_participates_in_fk(const char *co
while ((col= col_it++))
if (! my_strcasecmp(system_charset_info, col_name, col->field_name.str))
+ {
+ my_error(ER_FK_STATEMENT_ILLEGAL, MYF(0), statement, table_name);
return TRUE;
+ }
}
}
@@ -7965,7 +8012,17 @@ column_participates_in_fk(const char *co
/**
- TODO/FIXME: Add comment.
+ Check that column for which we are going to drop or set default value
+ is a child column in existing foreign key with SET DEFAULT referential
+ action and if it is true emit error that doing so is illegal.
+
+ @param col_name Name of column.
+ @param fk_list List of foreign keys in which table being altered
+ participates as child.
+
+ @retval FALSE Column does not participate in such foreign key.
+ @retval TRUE Column is a child column in such foreign key, an
+ appropriate error was emitted.
*/
static bool
@@ -8214,7 +8271,12 @@ mysql_prepare_alter_table(THD *thd, TABL
*/
new_foreign_key_list.swap(alter_info->foreign_key_list);
new_parent_foreign_key_list.swap(alter_info->parent_foreign_key_list);
- /* TODO/FIXME Add comment why we do this. */
+ /*
+ Also to be compatible with pre-WL#148 server in old mode server have to
+ ignore DROP FOREIGN KEY clauses on SQL-layer (they are processed by
+ engines). To achieve this we remove all Alter_drop instances which
+ correspond to such clauses from the 'drop_list'.
+ */
drop_it.rewind();
while ((drop= drop_it++))
if (drop->type == Alter_drop::FOREIGN_KEY)
@@ -8241,14 +8303,10 @@ mysql_prepare_alter_table(THD *thd, TABL
in foreign key (unless this foreign key is created by this ALTER).
*/
if (column_participates_in_fk(drop->name, new_foreign_key_list,
- new_parent_foreign_key_list))
- {
- /* QQ: should not we use a nicer error message? */
- my_error(ER_FK_STATEMENT_ILLEGAL, MYF(0),
- "ALTER TABLE ... DROP COLUMN",
- table_list->table_name);
+ new_parent_foreign_key_list,
+ "ALTER TABLE ... DROP COLUMN",
+ table_list->table_name))
goto err;
- }
/* Reset auto_increment value if it was dropped */
if (MTYP_TYPENR(field->unireg_check) == Field::NEXT_NUMBER &&
@@ -8280,17 +8338,10 @@ mysql_prepare_alter_table(THD *thd, TABL
in foreign key (unless this foreign key is created by this ALTER).
*/
if (column_participates_in_fk(def->change, new_foreign_key_list,
- new_parent_foreign_key_list))
- {
- /*
- QQ: should not we use a nicer error message ?
- Also how about moving my_error inside function ?
- */
- my_error(ER_FK_STATEMENT_ILLEGAL, MYF(0),
- "ALTER TABLE ... CHANGE/MODIFY COLUMN",
- table_list->table_name);
+ new_parent_foreign_key_list,
+ "ALTER TABLE ... CHANGE/MODIFY COLUMN",
+ table_list->table_name))
goto err;
- }
def->field=field;
if (!def->after)
@@ -8680,7 +8731,15 @@ err:
/**
- TODO/FIXME: Add comment.
+ Check if table being altered participates in some foreign key as parent
+ which will be preserved in its new version.
+
+ @param alter_info Alter_info object describing new version of table.
+
+ @retval FALSE Table was not referenced by foreign keys or all such
+ foreign keys are dropped by this ALTER TABLE.
+ @retval TRUE Table is parent in some foreign key which will be
+ preserved in its new version.
*/
static bool has_previosly_existing_referencing_fks(Alter_info *alter_info)
@@ -8938,7 +8997,11 @@ view_err:
}
}
- /* TODO/FIXME Add comment. */
+ /*
+ Code below can handle only base tables so ensure that we won't open a view.
+ Note that RENAME TABLE the only ALTER clause which is supported for views
+ has been already processed.
+ */
table_list->required_type= FRMTYPE_TABLE;
error= open_and_lock_tables_derived(thd, table_list, FALSE,
Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20090628063740-etho2ve342sfqmq4.bundle
| Thread |
|---|
| • bzr commit into mysql-6.1-fk branch (dlenev:2725) WL#148 | Dmitry Lenev | 28 Jun |
