#At file:///home/dlenev/src/bzr/mysql-6.1-mil13/ based on revid:dlenev@stripped
2742 Dmitry Lenev 2009-07-20
WL#148 "Foreign keys".
Milestone 13 "DDL checks and changes: ALTER, CREATE INDEX, DROP INDEX words".
Work in progress. Avoid creation of extra TABLE instance for performing
integrity checks when handling ALTER TABLE ADD CONSTRAINT when it is not
necessary.
modified:
sql/fk.cc
sql/fk.h
sql/sql_table.cc
=== modified file 'sql/fk.cc'
--- a/sql/fk.cc 2009-07-17 06:30:13 +0000
+++ b/sql/fk.cc 2009-07-20 12:52:17 +0000
@@ -526,22 +526,23 @@ Fk_constraint_list::prepare_check_parent
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 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.
+ @param thd Thread context.
+ @param table TABLE object representing new version of table being
+ altered.
+ @param table_tmp_name Temporary name for new version of table being altered
+ (used for creation of additional TABLE object for
+ table being altered if some self-referencing foreign
+ keys need to be checked).
+ @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
-Fk_constraint_list::prepare_alter_check_parent(THD *thd,
- TABLE *table, TABLE *table_2,
+Fk_constraint_list::prepare_alter_check_parent(THD *thd, TABLE *table,
+ const char *table_tmp_name,
List<Foreign_key_child> &fk_list)
{
List_iterator<Foreign_key_child_share> fk_s_it(table->s->fkeys.fkeys_child);
@@ -580,7 +581,40 @@ Fk_constraint_list::prepare_alter_check_
new (thd->mem_root) Foreign_key_child_rcontext(thd, table, fk_s, this);
if (fk_s->is_self_reference)
- parent_table= table_2;
+ {
+ /*
+ To perform checks for self-referencing foreign key constraints on
+ table being altered we have to create an additional TABLE object
+ for its new version. To do this we approach similar to one used
+ in open_temporary_table() with the exception that here we already
+ have properly loaded TABLE_SHARE object for table.
+
+ TODO: At some point in future we will probably use a cursor created
+ through cursor API instead of full-blown TABLE instance.
+ */
+ if (! m_extra_table)
+ {
+ if (! (m_extra_table= (TABLE*) my_malloc(sizeof(*m_extra_table),
+ MYF(MY_WME))))
+ return TRUE;
+
+ if (open_table_from_share(thd, table->s, table_tmp_name,
+ (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE |
+ HA_GET_INDEX),
+ (READ_KEYINFO | COMPUTE_TYPES |
+ EXTRA_RECORD),
+ ha_open_options, m_extra_table, OTM_OPEN))
+ {
+ my_free((char*) m_extra_table, MYF(0));
+ m_extra_table= 0;
+ return TRUE;
+ }
+
+ m_extra_table->reginfo.lock_type= TL_READ_NO_INSERT;
+ m_extra_table->pos_in_table_list= 0;
+ }
+ parent_table= m_extra_table;
+ }
else
parent_table= fk_def->parent_table->table;
@@ -597,6 +631,60 @@ Fk_constraint_list::prepare_alter_check_
/**
+ Set external lock for extra TABLE instance which was created for performing
+ checks for self-referencing foreign keys during ALTER TABLE.
+
+ @param thd Thread context.
+
+ @note Some engines use handler::ha_external_lock() called by this method as
+ a mark of statement boundary and to set up some structures needed for
+ statement execution.
+
+ @retval FALSE Success.
+ @retval TRUE Error.
+*/
+
+bool Fk_constraint_list::lock_extra_table(THD *thd)
+{
+ int error;
+
+ if (m_extra_table &&
+ (error= m_extra_table->file->ha_external_lock(thd, F_RDLCK)))
+ {
+ m_extra_table->file->print_error(error, MYF(0));
+ return TRUE;
+ }
+ return FALSE;
+}
+
+
+/**
+ "Unlock" extra TABLE instance which was created for performing checks for
+ self-referencing foreign keys during ALTER TABLE.
+
+ @param thd Thread context.
+
+ @sa Fk_constraint_list::lock_extra_table()
+
+ @retval FALSE Success.
+ @retval TRUE Error.
+*/
+
+bool Fk_constraint_list::unlock_extra_table(THD *thd)
+{
+ int error;
+
+ if (m_extra_table &&
+ (error= m_extra_table->file->ha_external_lock(thd, F_UNLCK)))
+ {
+ m_extra_table->file->print_error(error, MYF(0));
+ return TRUE;
+ }
+ return FALSE;
+}
+
+
+/**
For UPDATE or DELETE operation on the table prepare runtime contexts
for all foreign keys for which this table is a parent and which may
require checks or cascading actions.
@@ -687,6 +775,12 @@ Fk_constraint_list::~Fk_constraint_list(
while ((parent_ctx= cascade_it++))
delete parent_ctx;
+
+ if (m_extra_table)
+ {
+ close_temporary(m_extra_table, FALSE, FALSE);
+ my_free((char*) m_extra_table, MYF(0));
+ }
}
=== modified file 'sql/fk.h'
--- a/sql/fk.h 2009-07-17 06:30:13 +0000
+++ b/sql/fk.h 2009-07-20 12:52:17 +0000
@@ -38,7 +38,7 @@ public:
Fk_constraint_list(trg_event_type action_type_arg,
Fk_constraint_list *parent_fk_list)
: m_action_type(action_type_arg),
- m_table(NULL),
+ m_table(NULL), m_extra_table(NULL),
m_parent_fk_list(parent_fk_list)
{ }
@@ -49,8 +49,8 @@ public:
bool prepare_check_parent(THD *thd, TABLE *table,
MY_BITMAP *changed_column_map);
- bool prepare_alter_check_parent(THD *thd,
- TABLE *table, TABLE *table_2,
+ bool prepare_alter_check_parent(THD *thd, TABLE *table,
+ const char *table_tmp_name,
List<Foreign_key_child> &fk_list);
bool prepare_check_child(THD *thd, TABLE *table,
@@ -111,6 +111,9 @@ public:
bool has_triggers();
+ bool lock_extra_table(THD *thd);
+ bool unlock_extra_table(THD *thd);
+
private:
/** Implementation. */
@@ -135,6 +138,13 @@ private:
*/
TABLE *m_table;
/**
+ Additional TABLE instance which is created for performing checks for
+ self-referencing foreign keys during ALTER TABLE. Unlike instance
+ from 'm_table' member it is created within method of this class and
+ it is responsibility of this class to destroy it at its end-of-life.
+ */
+ TABLE *m_extra_table;
+ /**
List of foreign key constraints in which the subject table
participates as a child.
*/
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2009-07-17 06:30:13 +0000
+++ b/sql/sql_table.cc 2009-07-20 12:52:17 +0000
@@ -9352,7 +9352,7 @@ bool mysql_alter_table(THD *thd,char *ne
Alter_info *alter_info,
uint order_num, ORDER *order, bool ignore)
{
- TABLE *table, *new_table= 0, *new_table_2= 0;
+ TABLE *table, *new_table= 0;
MDL_request target_mdl_request;
bool has_target_mdl_lock= FALSE;
int error= 0;
@@ -10105,29 +10105,6 @@ view_err:
FN_IS_TMP);
/* Open our intermediate table */
new_table= open_temporary_table(thd, path, new_db, tmp_name, 1, OTM_OPEN);
-
- /*
- TODO/FIXME: - Do this only if needed...
- - Move code to separate function...
- - Find out what should be done re locking...
- - Clarify what happens in case of error...
-
- A death-defying stunt - creation of second instance of TABLE object
- for a temporary table.
-
- QQ: What is the correct approach to this ? May be we should rely on the
- new interface for cursors ? Should we call mysql_lock_tables() and
- such?
- */
- if (! (new_table_2= (TABLE*) my_malloc(sizeof(*new_table_2), MYF(MY_WME))) ||
- open_table_from_share(thd, new_table->s, tmp_name,
- (uint) (HA_OPEN_KEYFILE | HA_OPEN_RNDFILE | HA_GET_INDEX),
- (READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD),
- ha_open_options, new_table_2, OTM_OPEN))
- goto err_new_table_cleanup;
-
- new_table_2->reginfo.lock_type= TL_WRITE;
- new_table_2->pos_in_table_list= 0;
}
if (!new_table)
goto err_new_table_cleanup;
@@ -10157,7 +10134,7 @@ view_err:
{
Fk_constraint_list fk_list(TRG_EVENT_INSERT, NULL);
- if (fk_list.prepare_alter_check_parent(thd, new_table, new_table_2,
+ if (fk_list.prepare_alter_check_parent(thd, new_table, tmp_name,
alter_info->foreign_key_list))
{
error= 1;
@@ -10213,11 +10190,6 @@ view_err:
not delete it! Even altough MERGE tables do not have their children
attached here it is safe to call close_temporary_table().
*/
- if (new_table_2)
- {
- close_temporary(new_table_2, 0, 0);
- new_table_2= 0;
- }
close_temporary_table(thd, new_table, 1, 0);
new_table= 0;
@@ -10396,11 +10368,6 @@ end_temporary:
DBUG_RETURN(FALSE);
err_new_table_cleanup:
- if (new_table_2)
- {
- close_temporary(new_table_2, 0, 0);
- new_table_2= 0;
- }
if (new_table)
{
/* close_temporary_table() frees the new_table pointer. */
@@ -10541,6 +10508,10 @@ copy_data_between_tables(TABLE *from,TAB
goto err;
errpos= 2;
+ if (fk_list->lock_extra_table(thd))
+ goto err;
+ errpos= 3;
+
/* We need external lock before we can disable/enable keys */
alter_table_manage_keys(to, from->file->indexes_are_disabled(), keys_onoff);
@@ -10551,7 +10522,7 @@ copy_data_between_tables(TABLE *from,TAB
from->file->info(HA_STATUS_VARIABLE | HA_STATUS_NO_LOCK);
to->file->ha_start_bulk_insert(from->file->stats.records);
- errpos= 3;
+ errpos= 4;
copy_end=copy;
for (Field **ptr=to->field ; *ptr ; ptr++)
@@ -10614,7 +10585,7 @@ copy_data_between_tables(TABLE *from,TAB
/* Tell handler that we have values for all columns in the to table */
to->use_all_columns();
init_read_record(&info, thd, from, (SQL_SELECT *) 0, 1, 1, FALSE);
- errpos= 4;
+ errpos= 5;
if (ignore)
to->file->extra(HA_EXTRA_IGNORE_DUP_KEY);
thd->warning_info->reset_current_row_for_warning();
@@ -10683,14 +10654,14 @@ copy_data_between_tables(TABLE *from,TAB
}
err:
- if (errpos >= 4)
+ if (errpos >= 5)
end_read_record(&info);
free_io_cache(from);
delete [] copy;
if (error > 0)
to->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
- if (errpos >= 3 && to->file->ha_end_bulk_insert(error > 1) && error <= 0)
+ if (errpos >= 4 && to->file->ha_end_bulk_insert(error > 1) && error <= 0)
{
to->file->print_error(my_errno,MYF(0));
error= 1;
@@ -10717,6 +10688,9 @@ err:
*copied= found_count;
*deleted=delete_count;
to->file->ha_release_auto_increment();
+
+ if (errpos >= 3 && fk_list->unlock_extra_table(thd))
+ error= 1;
if (errpos >= 2 && to->file->ha_external_lock(thd,F_UNLCK))
error=1;
if (error < 0 && to->file->extra(HA_EXTRA_PREPARE_FOR_RENAME))
Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20090720125217-ovx8p56rqcowdgv4.bundle
| Thread |
|---|
| • bzr commit into mysql-6.1-fk branch (dlenev:2742) WL#148 | Dmitry Lenev | 20 Jul |
