#At file:///home/dlenev/src/bzr/mysql-6.1-bg41761/
2703 Dmitry Lenev 2009-01-30
Patch that fixes bug #41761 "Foreign keys: integrity breach if
UPDATE and non-transactional+self-referencing" and implements
support of insertion/removal of self-referencing rows to
non-transactional table.
The problem was that in --foreign-key-all-engines mode one was
able to create row with a dangling reference in a
non-transactional table with a self-referencing foreign key.
This has happened when one updated existing row in such table
and its primary key was set to some new value while row's
foreign key was set to the old value of primary key.
This problem stemmed from the fact that foreign key checks for
such tables, which happen before operation on the row is executed,
didn't take into account contents of the row which is about to be
changed.
This fix implements extra checks which do so and also allow to
support addition/removal of self-referencing rows to such tables.
modified:
mysql-test/r/foreign_key_all_engines.result
mysql-test/t/foreign_key_all_engines.test
sql/fk.cc
sql/table.cc
per-file messages:
mysql-test/r/foreign_key_all_engines.result
Added test coverage for bug #41761 "Foreign keys: integrity
breach if UPDATE and non-transactional+self-referencing" and
some other aspects of handling of self-referencing FKs for
non-transactional tables.
mysql-test/t/foreign_key_all_engines.test
Added test coverage for bug #41761 "Foreign keys: integrity
breach if UPDATE and non-transactional+self-referencing" and
some other aspects of handling of self-referencing FKs for
non-transactional tables.
sql/fk.cc
Ensure that we properly preserve integrity and support addition/
removal of self-referencing rows to non-transactional tables with
self-referencing foreign keys. To do this when performing FK
checks we have to take into account not only contents of rows
which are already in the table but also the contents of the row
about to be inserted/updated/deleted (note the fact that for
non-transactional tables foreign key checks happen before
performing actual operation on a row).
Introduced code which performs these extra checks when they are
necessary. Added couple of auxiliary functions in order to
simplify their implementation. Also moved code marking columns
used by foreign key checks to Foreign_key_child/parent_rcontext
methods to make it closer to place where results of such marking
are used.
sql/table.cc
Moved code which is responsible for marking columns used by foreign key checks from TABLE::mark_columns_needed_for_*() to
Foreign_key_child/parent_rcontext::prepare(). This should make
code in the latter easier to understand as now we mark columns
closer to the place where values from them are used.
=== modified file 'mysql-test/r/foreign_key_all_engines.result'
--- a/mysql-test/r/foreign_key_all_engines.result 2009-01-29 09:00:19 +0000
+++ b/mysql-test/r/foreign_key_all_engines.result 2009-01-30 13:41:12 +0000
@@ -1671,3 +1671,73 @@ Level Code Message
Error 1292 Incorrect date value: '2009-02-30' for column 'fk1' at row 3
drop tables t1, t2;
set sql_mode= @old_sql_mode;
+#
+# Test for bug #41761 "Foreign keys: integrity breach if UPDATE and
+# non-transactional+self-referencing" and some other aspects of
+# handling of self-referencing FKs for non-transactional tables.
+#
+set @@rand_seed1=10000000,@@rand_seed2=1000000;
+drop table if exists t1;
+create table t1 (pk int primary key,
+fk int references t1 (pk)
+on update restrict on delete restrict) engine=myisam;
+# Coverage for lookups in parent tables.
+#
+# This should succeed as we can detect that PK will be
+# provided by the row containing FK value.
+insert into t1 values (1, 1);
+# This should fail as it requires real end of statement check
+insert into t1 values (2, 3), (3, 3);
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': no matching key for value '3', it is not in parent table
+# Now similar test for UPDATE
+insert into t1 values (2, 1);
+# Again providing PK for FK in the same row should succeed
+update t1 set pk= 3, fk= 3 where pk= 2;
+insert into t1 values (4, NULL);
+update t1 set pk= case pk when 3 then 3 when 4 then 5 end,
+fk= case fk when 3 then 5 else fk end
+where pk >= 3;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': no matching key for value '5', it is not in parent table
+# Now coverage for lookups in child table.
+#
+# UPDATE that changes PK with no matching FKs but
+# adds FK referencing to its old value.
+update t1 set pk= 5, fk= 4 where pk= 4;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '4'
+# UPDATE that changes PK with only one matching row
+# which is row being updated itself, but fails since
+# it does not change FK value.
+update t1 set pk= 5 where pk = 3;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '3'
+# Same UPDATE which changes FK insignificantly.
+update t1 set pk= 5, fk= 3 where pk = 3;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '3'
+# Similar UPDATE which changes FK significantly should succeed.
+update t1 set pk= 5, fk= 4 where pk = 3;
+# Now case with UPDATE changing PK with only one
+# matching row, but the row is not the one which
+# is being updated.
+update t1 set pk= 6, fk= NULL where pk = 4;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '4'
+# Now case with UPDATE and more than one matching row
+insert into t1 values (2, 1);
+update t1 set pk= 6, fk= 6 where pk = 1;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '1'
+# DELETE and more than one matching row
+delete from t1 where pk = 1;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '1'
+# DELETE with one matching row but which is not row being
+# deleted.
+delete from t1 where pk = 4;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '4'
+# Finally DELETE with only one matching row and which is
+# row being deleted.
+update t1 set fk= 5 where pk = 5;
+delete from t1 where pk = 5;
+# Now UPDATE and DELETE which should fail since to pass
+# they have to use true end-of-statement checking.
+update t1 set pk = case pk when 1 then 5 else 2 end, fk= 5 where pk <= 2;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '1'
+delete from t1 where fk = 1;
+ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '1'
+drop table t1;
=== modified file 'mysql-test/t/foreign_key_all_engines.test'
--- a/mysql-test/t/foreign_key_all_engines.test 2009-01-29 09:00:19 +0000
+++ b/mysql-test/t/foreign_key_all_engines.test 2009-01-30 13:41:12 +0000
@@ -1535,3 +1535,77 @@ update t1 set pk2= 6 where pk2 = 3;
show warnings;
drop tables t1, t2;
set sql_mode= @old_sql_mode;
+
+
+--echo #
+--echo # Test for bug #41761 "Foreign keys: integrity breach if UPDATE and
+--echo # non-transactional+self-referencing" and some other aspects of
+--echo # handling of self-referencing FKs for non-transactional tables.
+--echo #
+set @@rand_seed1=10000000,@@rand_seed2=1000000;
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+create table t1 (pk int primary key,
+ fk int references t1 (pk)
+ on update restrict on delete restrict) engine=myisam;
+--echo # Coverage for lookups in parent tables.
+--echo #
+--echo # This should succeed as we can detect that PK will be
+--echo # provided by the row containing FK value.
+insert into t1 values (1, 1);
+--echo # This should fail as it requires real end of statement check
+--error ER_FK_CHILD_NO_MATCH
+insert into t1 values (2, 3), (3, 3);
+--echo # Now similar test for UPDATE
+insert into t1 values (2, 1);
+--echo # Again providing PK for FK in the same row should succeed
+update t1 set pk= 3, fk= 3 where pk= 2;
+insert into t1 values (4, NULL);
+--error ER_FK_CHILD_NO_MATCH
+update t1 set pk= case pk when 3 then 3 when 4 then 5 end,
+ fk= case fk when 3 then 5 else fk end
+ where pk >= 3;
+--echo # Now coverage for lookups in child table.
+--echo #
+--echo # UPDATE that changes PK with no matching FKs but
+--echo # adds FK referencing to its old value.
+--error ER_FK_CHILD_VALUE_EXISTS
+update t1 set pk= 5, fk= 4 where pk= 4;
+--echo # UPDATE that changes PK with only one matching row
+--echo # which is row being updated itself, but fails since
+--echo # it does not change FK value.
+--error ER_FK_CHILD_VALUE_EXISTS
+update t1 set pk= 5 where pk = 3;
+--echo # Same UPDATE which changes FK insignificantly.
+--error ER_FK_CHILD_VALUE_EXISTS
+update t1 set pk= 5, fk= 3 where pk = 3;
+--echo # Similar UPDATE which changes FK significantly should succeed.
+update t1 set pk= 5, fk= 4 where pk = 3;
+--echo # Now case with UPDATE changing PK with only one
+--echo # matching row, but the row is not the one which
+--echo # is being updated.
+--error ER_FK_CHILD_VALUE_EXISTS
+update t1 set pk= 6, fk= NULL where pk = 4;
+--echo # Now case with UPDATE and more than one matching row
+insert into t1 values (2, 1);
+--error ER_FK_CHILD_VALUE_EXISTS
+update t1 set pk= 6, fk= 6 where pk = 1;
+--echo # DELETE and more than one matching row
+--error ER_FK_CHILD_VALUE_EXISTS
+delete from t1 where pk = 1;
+--echo # DELETE with one matching row but which is not row being
+--echo # deleted.
+--error ER_FK_CHILD_VALUE_EXISTS
+delete from t1 where pk = 4;
+--echo # Finally DELETE with only one matching row and which is
+--echo # row being deleted.
+update t1 set fk= 5 where pk = 5;
+delete from t1 where pk = 5;
+--echo # Now UPDATE and DELETE which should fail since to pass
+--echo # they have to use true end-of-statement checking.
+--error ER_FK_CHILD_VALUE_EXISTS
+update t1 set pk = case pk when 1 then 5 else 2 end, fk= 5 where pk <= 2;
+--error ER_FK_CHILD_VALUE_EXISTS
+delete from t1 where fk = 1;
+drop table t1;
=== modified file 'sql/fk.cc'
--- a/sql/fk.cc 2009-01-28 17:25:41 +0000
+++ b/sql/fk.cc 2009-01-30 13:41:12 +0000
@@ -91,7 +91,8 @@ public:
m_child_table(child_table_arg),
m_fk_share(fk_share_arg),
m_column_count(fk_share_arg->column_count),
- m_fk_list(fk_list_arg)
+ m_fk_list(fk_list_arg),
+ m_check_record_in_memory(FALSE)
{ }
bool prepare();
@@ -127,6 +128,34 @@ private:
store_key_field **m_key_copy;
/** Our constraint list, the one we are part of. */
Fk_constraint_list *m_fk_list;
+ /**
+ Indicates that this a self-referencing key on a non-transactional
+ table, and an extra step is required when performing checks for it.
+ When adding or modifying a self-referencing row to a table
+ with a self-referencing foreign key, we need to take into
+ account contents of the row about to be inserted/updated and
+ not only contents of the rows which are already in table.
+ Imagine we INSERT INTO t1 (pk, fk) VALUES (1, 1);
+ or UPDATE t1 SET pk=2, fk=2 WHERE pk=1 and FK=1;
+ Constraint checks for non transactional tables are done
+ before a change is applied; end-of-statement checks are
+ impossible -- that's why this special logic is called for.
+ @sa Foreign_key_parent_rcontext where such checks are vital
+ for integrity.
+ */
+ bool m_check_record_in_memory;
+ /**
+ In case when extra steps described above are necessary, contains a
+ list of Field objects pointing at a new value of the parent.
+ Remember, even though we have a self-referencing key, i.e. operate
+ on the same physical table, still two TABLE instances are used: one
+ for the operation itself, and another (m_parent_table) to look up
+ the parent.
+ Thus we can't use "m_key" member for in-memory check, since it points
+ at the other instance (m_parent_table), whereas the new parent value
+ is in m_child_table::record[0].
+ */
+ Field **m_new_parent_columns_in_child;
};
@@ -145,7 +174,8 @@ public:
m_parent_table(parent_table_arg),
m_fk_share(fk_share_arg),
m_column_count(fk_share_arg->column_count),
- m_fk_list(fk_list_arg), m_cascade_fk_list(NULL)
+ m_fk_list(fk_list_arg), m_cascade_fk_list(NULL),
+ m_check_record_in_memory(FALSE)
{
m_ref_action= ((m_fk_list->action_type() == TRG_EVENT_UPDATE) ?
m_fk_share->update_opt : m_fk_share->delete_opt);
@@ -222,6 +252,49 @@ private:
these modifications.
*/
Fk_constraint_list *m_cascade_fk_list;
+ /**
+ Indicates that this is a self-referencing foreign key on a
+ non-transactional table and the operation at hand will
+ require an extra step while performing foreign key checks
+ for it.
+ In case of non-transactional tables the end of statement
+ buffer can not be used, and checks are performed before
+ a change is applied.
+ That's why, in order to preserve integrity and support
+ removal/addition of self-referencing rows we must take
+ into account not only contents of rows which are already
+ in the table, but also the contents of the row about to
+ be updated/deleted.
+ Imagine we have:
+ CREATE TABLE t1 (pk char, fk char references t1(pk)) SELECT ('a', 'a');
+ And do:
+ DELETE FROM t1 WHERE pk='a'
+ Without an in-memory check we can never tell that this
+ operation is OK as PK has no children in the end.
+ A trickier case is possible with UPDATE:
+ CREATE TABLE t1 (pk char, fk char references t1(pk)) SELECT ('a', NULL);
+ UPDATE t1 SET pk='b', fk='A';
+ Without an in-memory check we would never be able to
+ discover that FK becomes an orphan.
+ */
+ bool m_check_record_in_memory;
+ /**
+ In case this is an UPDATE and extra steps described above
+ are necessary, contains list of Field objects representing the
+ new value of child key in the parent TABLE instance (i.e. one
+ that used for updating the rows in the table).
+ @sa explanation for why we need a separate list of Field pointers
+ in a similar member of class Foreign_key_child_rcontext.
+ */
+ Field **m_new_child_columns_in_parent;
+ /**
+ In case this is UPDATE or DELETE and extra steps described
+ above are necessary, contains a list of Field objects
+ representing old values of child key columns in the parent
+ TABLE instance (i.e. one that used for updating/deleting rows
+ from the table).
+ */
+ Field **m_old_child_columns_in_parent;
};
@@ -752,10 +825,231 @@ bool Foreign_key_child_rcontext::prepare
}
m_child_columns[idx]= NULL; /* NULL-terminate for fk_report_error(). */
- return prepare_key_copy(m_thd, m_key,
- m_child_columns, m_fk_share->parent_columns,
- m_column_count,
- &m_key_buff, &m_key_copy);
+ if (m_fk_list->action_type() == TRG_EVENT_UPDATE)
+ {
+ /*
+ In case of UPDATE we also have to ensure that all child columns
+ are read so we have enough information for checking of MATCH rule
+ and constructing key for lookup in the parent table. There is no
+ need. There is no need for this in case of INSERT as it always
+ provides values for all columns.
+ */
+ bitmap_union(m_child_table->read_set, &m_fk_share->column_set);
+ m_child_table->file->column_bitmaps_signal();
+ }
+
+ if (prepare_key_copy(m_thd, m_key, m_child_columns,
+ m_fk_share->parent_columns, m_column_count,
+ &m_key_buff, &m_key_copy))
+ return TRUE;
+
+ /*
+ Step 4: Prepare for an extra check when non-transactional +
+ self-referencing.
+ */
+ if (m_parent_table->s == m_child_table->s &&
+ ! m_child_table->file->has_transactions())
+ {
+ /*
+ When we are performing a check for self-referencing foreign key on
+ a non-transactional table it is possible that impending INSERT or
+ UPDATE of a row will add or remove a matching parent value.
+ Enable extra checks which take such possibility into account.
+ */
+ if (m_fk_list->action_type() == TRG_EVENT_INSERT)
+ m_check_record_in_memory= TRUE;
+ else
+ {
+ DBUG_ASSERT(m_fk_list->action_type() == TRG_EVENT_UPDATE);
+ /*
+ In case of UPDATE this extra check is needed only if the parent
+ key is going to be modified. Notice that since in this case we
+ will read all parent key columns, our check will have all data
+ it needs.
+ */
+ for (idx= 0; idx < m_key->key_parts; idx++)
+ {
+ if (bitmap_is_set(m_child_table->write_set,
+ m_key->key_part[idx].field->field_index))
+ {
+ m_check_record_in_memory= TRUE;
+ break;
+ }
+ }
+ }
+ }
+
+ if (m_check_record_in_memory)
+ {
+ /*
+ Fill the list of Fields which correspond to new values of PK columns
+ in child TABLE instance (i.e. the isntance used for INSERT/UPDATE
+ operation).
+ */
+ if (! (m_new_parent_columns_in_child= (Field **)
+ alloc_root(m_thd->mem_root, sizeof(Field*) * (m_column_count+1))))
+ return TRUE;
+ for (idx= 0; idx < m_key->key_parts; idx++)
+ {
+ uint field_index= m_key->key_part[idx].field->field_index;
+ m_new_parent_columns_in_child[idx]= m_child_table->field[field_index];
+ }
+ /** Null-terminate for fk_lookup(). */
+ m_new_parent_columns_in_child[idx]= NULL;
+ }
+
+ return FALSE;
+}
+
+
+/**
+ Compare two sets of columns: one represented by a key, another by a
+ corresponding list of fields objects.
+ Doesn't take into account "insignificant" differences. NULLs have
+ to match exactly -- a NULL cmp not NULL yields FALSE.
+ This is exactly how you look up a child for a parent and vice versa,
+ MATCH rules put aside.
+
+ @param columns NULL-ended array of columns to be compared with key
+ (Field objects should go in the same order as in key
+ but can belong to a different TABLE instance and
+ be bound to record other than TABLE::record[0]).
+ @param key KEY object describing key.
+ @param key_buff Buffer containing key value.
+
+ @retval FALSE Values in columns are equal to key.
+ @retval TRUE Values in columns are different from the key value.
+*/
+
+static bool fk_key_cmp(Field **columns, KEY *key, uchar *key_buff)
+{
+ uint i;
+ uchar *key_col_pos;
+
+ for (i= 0, key_col_pos= key_buff ; i < key->key_parts;
+ key_col_pos+= key->key_part[i].store_length, i++)
+ {
+ uint maybe_null= test(key->key_part[i].null_bit);
+
+ if (maybe_null)
+ {
+ if (*key_col_pos)
+ {
+ if (!columns[i]->is_null())
+ return TRUE;
+ /* Both columns and key have NULL for this position. */
+ continue;
+ }
+ else if (columns[i]->is_null())
+ return TRUE;
+ }
+
+ if (columns[i]->key_cmp(key_col_pos + maybe_null,
+ key->key_part[i].length))
+ return TRUE;
+ }
+ return FALSE;
+}
+
+
+/**
+ Result of check if table contains rows with particular key value.
+*/
+
+enum enum_fk_lookup_result
+{
+ FK_LOOKUP_NO_ROWS, FK_LOOKUP_ROW_EXIST,
+ FK_LOOKUP_SEVERAL_ROWS_EXIST, FK_LOOKUP_ERROR
+};
+
+
+/**
+ Check if table contains any rows with particular key value.
+
+ @param key_table Table in which lookup should be performed.
+ @param key_idx Index of key which should be used for lookup.
+ @param key_buff Buffer for constructing key value for lookup.
+ @param copy Array of auxiliary objects to be used for
+ constructing key value for lookup.
+ @param notify_if_several_rows TRUE - If we also need to know if there
+ exists more than one row.
+ FALSE - Otherwise.
+
+
+ @retval FK_LOOKUP_NO_ROWS - There are now rows for this key value.
+ @retval FK_LOOKUP_ROW_EXIST - At least one row with this key exists,
+ or if notify_if_several_rows is TRUE
+ there is only one row with this key.
+ @retval FK_LOOKUP_SEVERAL_ROWS_EXIST - Returned when notify_if_several_rows
+ is TRUE, indicates that more than one
+ row exists with this key value.
+ @retval FK_LOOKUP_ERROR - Table engine returned an error.
+*/
+
+static enum_fk_lookup_result
+fk_lookup(TABLE *key_table, uint key_idx, uchar *key_buff,
+ store_key_field **copy, bool notify_if_several_rows)
+{
+ enum_fk_lookup_result result= FK_LOOKUP_ERROR;
+ store_key_field **copy_it;
+ int error;
+
+ for (copy_it= copy ; *copy_it; copy_it++)
+ (*copy_it)->copy();
+
+ if ((error= key_table->file->ha_index_init(key_idx, FALSE)))
+ {
+ key_table->file->print_error(error, MYF(0));
+ return FK_LOOKUP_ERROR;
+ }
+
+ error= key_table->file->index_read_map(key_table->record[0],
+ key_buff,
+ HA_WHOLE_KEY,
+ HA_READ_KEY_EXACT);
+
+ switch (error) {
+ case 0:
+ /* There is at least one row. */
+ if (notify_if_several_rows)
+ {
+ /* We need to know if there are more rows. */
+ uint key_length= key_table->key_info[key_idx].key_length;
+
+ error= key_table->file->index_next_same(key_table->record[0],
+ key_buff,
+ key_length);
+
+ switch (error) {
+ case 0:
+ /* There is one more row. */
+ result= FK_LOOKUP_SEVERAL_ROWS_EXIST;
+ break;
+ case HA_ERR_END_OF_FILE:
+ case HA_ERR_KEY_NOT_FOUND:
+ /* The first row found is the only one. */
+ result= FK_LOOKUP_ROW_EXIST;
+ break;
+ default:
+ key_table->file->print_error(error, MYF(0));
+ break;
+ }
+ }
+ else
+ result= FK_LOOKUP_ROW_EXIST;
+ break;
+ case HA_ERR_END_OF_FILE:
+ case HA_ERR_KEY_NOT_FOUND:
+ /* No rows exist. */
+ result= FK_LOOKUP_NO_ROWS;
+ break;
+ default:
+ key_table->file->print_error(error, MYF(0));
+ break;
+ }
+
+ key_table->file->ha_index_end();
+ return result;
}
@@ -770,8 +1064,7 @@ bool Foreign_key_child_rcontext::prepare
bool Foreign_key_child_rcontext::check_parent_exists()
{
- int error;
- uint idx;
+ enum_fk_lookup_result result;
/*
Foreign key / match simple doesn't need to match if any column is NULL.
@@ -848,30 +1141,52 @@ bool Foreign_key_child_rcontext::check_p
old_map= dbug_tmp_use_all_columns(m_child_table, m_child_table->read_set);
#endif
- for (idx= 0 ; idx < m_key->key_parts; idx++)
- m_key_copy[idx]->copy();
+ result= fk_lookup(m_parent_table, m_key_idx, m_key_buff, m_key_copy, FALSE);
#ifndef DBUG_OFF
if (m_fk_list->action_type() == TRG_EVENT_INSERT)
dbug_tmp_restore_column_map(m_child_table->read_set, old_map);
#endif
- error= m_parent_table->file->index_read_idx_map(m_parent_table->record[0],
- m_key_idx, m_key_buff,
- HA_WHOLE_KEY,
- HA_READ_KEY_EXACT);
- switch (error) {
- case 0:
- /* Matching primary key value was found in parent table. */
+ switch (result) {
+ case FK_LOOKUP_ROW_EXIST:
+ /*
+ A matching parent key value was found in the parent table.
+
+ If it's an UPDATE, a non-transactional table (so that the check
+ is performed before the update and the EOS buffer is unused)
+ and a self-reference foreign key, the parent may get deleted
+ in the course of the very UPDATE operation that is in progress:
+ CREATE TABLE t1 (pk INT, fk INT REFERENCES t1(pk)) SELECT (1, NULL);
+ UPDATE t1 SET pk=2, fk=1;
+ We handle this situation by having an appropriate check in
+ Foreign_key_parent_rcontext::check_children_exist(), so a dangling
+ reference won't be left around. It is possible to do this as to cause
+ this problem the new value of PK has to differ 'significantly' from
+ the old value of PK.
+ */
return FALSE;
- case HA_ERR_END_OF_FILE:
- case HA_ERR_KEY_NOT_FOUND:
- /* No matching key was found. Report a foreign key violation error. */
+ case FK_LOOKUP_NO_ROWS:
+ /* No matching key was found. */
+ if (m_check_record_in_memory)
+ {
+ /*
+ If this is a self-referencing FK for non-transactional table we
+ might be about to create a matching PK by impending INSERT or
+ UPDATE of the row. Check if this is the case.
+ We do this by comparing PK in the row about to be inserted/changed
+ with the value of key we constructed earlier for lookups in parent
+ (i.e. with the new value of FK).
+ */
+ if (! fk_key_cmp(m_new_parent_columns_in_child, m_key, m_key_buff))
+ return FALSE;
+ }
+ /* Report a foreign key violation error. */
fk_report_error(ER_FK_CHILD_NO_MATCH, m_fk_share->name.str, m_child_table,
m_child_columns);
return TRUE;
default:
- m_parent_table->file->print_error(error, MYF(0));
+ /* Error from the engine was already reported by fk_lookup(). */
return TRUE;
}
}
@@ -1042,10 +1357,112 @@ bool Foreign_key_parent_rcontext::prepar
}
m_parent_columns_old[idx]= NULL; /* NULL-terminate. */
- return prepare_key_copy(m_thd, m_key,
- m_parent_columns_old, m_fk_share->child_columns,
- m_column_count,
- &m_key_buff, &m_key_copy);
+ /*
+ Let us also ensure that all columns of parent key are read so we have
+ enough information for constructing key for lookup in the child table.
+ */
+ bitmap_union(m_parent_table->read_set, &m_fk_share->column_set);
+ m_parent_table->file->column_bitmaps_signal();
+
+ if (prepare_key_copy(m_thd, m_key, m_parent_columns_old,
+ m_fk_share->child_columns, m_column_count,
+ &m_key_buff, &m_key_copy))
+ return TRUE;
+
+ if (m_parent_table->s == m_child_table->s &&
+ ! m_parent_table->file->has_transactions())
+ {
+ /*
+ When we are performing a check for self-referencing foreign key on
+ non-transactional table it is possible that an impending UPDATE or
+ DELETE of a row will add or remove matching foreign key value.
+ Enable extra checks which take such possibility into account if
+ neccessary.
+ */
+ if (m_fk_list->action_type() == TRG_EVENT_DELETE)
+ m_check_record_in_memory= TRUE;
+ else
+ {
+ DBUG_ASSERT(m_fk_list->action_type() == TRG_EVENT_UPDATE);
+ /*
+ In case of UPDATE a matching FK value might be added or go away
+ only when we are updating at least one of its columns. So we
+ need to do this extra check only in this case. Also since in
+ this case all FK columns are going to be read we will have all
+ the necessary information for it.
+ */
+ for (idx= 0; idx < m_key->key_parts; idx++)
+ {
+ if (bitmap_is_set(m_parent_table->write_set,
+ m_key->key_part[idx].field->field_index))
+ {
+ m_check_record_in_memory= TRUE;
+ break;
+ }
+ }
+ }
+ }
+
+ if (m_check_record_in_memory)
+ {
+ if (m_fk_list->action_type() == TRG_EVENT_UPDATE)
+ {
+ /*
+ Fill the list of Field objects which correspond to new
+ values of FK columns in parent TABLE instance (i.e. one
+ that used for update or delete).
+ */
+ if (! (m_new_child_columns_in_parent= (Field **)
+ alloc_root(m_thd->mem_root, sizeof(Field*)*(m_column_count+1))))
+ return TRUE;
+
+ for (idx= 0; idx < m_key->key_parts; idx++)
+ {
+ uint field_index= m_key->key_part[idx].field->field_index;
+ m_new_child_columns_in_parent[idx]= m_parent_table->field[field_index];
+ }
+ m_new_child_columns_in_parent[idx]= NULL;
+ }
+
+ /*
+ Fill the list of Field objects which correspond to old values
+ of FK columns in parent TABLE instance (i.e. one that used for
+ UPDATE or DELETE).
+ */
+ if (! (m_old_child_columns_in_parent= (Field **)
+ alloc_root(m_thd->mem_root, sizeof(Field*) * (m_column_count+1))))
+ return TRUE;
+
+ for (idx= 0; idx < m_key->key_parts; idx++)
+ {
+ uint field_index= m_key->key_part[idx].field->field_index;
+ Field *old_child_col= m_parent_table->field[field_index];
+
+ if (old_row_is_record1)
+ {
+ if (!(old_child_col= old_child_col->new_field(m_thd->mem_root,
+ m_parent_table, TRUE)))
+ return TRUE;
+ old_child_col->move_field_offset(new_offset);
+ }
+ m_old_child_columns_in_parent[idx]= old_child_col;
+ }
+ m_old_child_columns_in_parent[idx]= NULL;
+
+ if (m_fk_list->action_type() == TRG_EVENT_DELETE)
+ {
+ /*
+ To be able to perform checks taking into account impending
+ DELETE operations we also need to know values of FK columns.
+ */
+ for (idx= 0; idx < m_key->key_parts; idx++)
+ bitmap_set_bit(m_parent_table->read_set,
+ m_key->key_part[idx].field->field_index);
+ m_parent_table->file->column_bitmaps_signal();
+ }
+ }
+
+ return FALSE;
}
@@ -1202,24 +1619,38 @@ bool Foreign_key_parent_rcontext::prepar
bool Foreign_key_parent_rcontext::check_children_exist()
{
- uint i;
- int error;
-
/*
Handler interaction point #2: look up parent value in child table.
*/
- for (i= 0 ; i < m_key->key_parts; i++)
- m_key_copy[i]->copy();
- error= m_child_table->file->index_read_idx_map(m_child_table->record[0],
- m_key_idx, m_key_buff,
- HA_WHOLE_KEY,
- HA_READ_KEY_EXACT);
- switch (error) {
- case HA_ERR_END_OF_FILE:
- case HA_ERR_KEY_NOT_FOUND:
- /* No matching children exist. Everything is OK. */
+
+ enum_fk_lookup_result result= fk_lookup(m_child_table, m_key_idx,
+ m_key_buff, m_key_copy,
+ m_check_record_in_memory);
+
+ switch (result) {
+ case FK_LOOKUP_NO_ROWS:
+ /* No matching children exist. */
+ if (m_check_record_in_memory &&
+ m_fk_list->action_type() == TRG_EVENT_UPDATE)
+ {
+ /*
+ It is a self-referencing FK on a non-transactional table and we might
+ be about to create an orphaned child, referencing to the old (changed)
+ PK value.
+ Check if this is the case. We do this by comparing the new FK value
+ with the key we constructed earlier, for lookups in the child (i.e.
+ with the old value of PK).
+ */
+ if (! fk_key_cmp(m_new_child_columns_in_parent, m_key, m_key_buff))
+ {
+ fk_report_error(ER_FK_CHILD_VALUE_EXISTS, m_fk_share->name.str,
+ m_parent_table, m_parent_columns_old);
+ return TRUE;
+ }
+ }
+ /* Everything is OK. */
return FALSE;
- case 0:
+ case FK_LOOKUP_ROW_EXIST:
/*
Child rows exist.
@@ -1240,12 +1671,85 @@ bool Foreign_key_parent_rcontext::check_
! has_changed_columns())
return FALSE;
+ if (m_check_record_in_memory)
+ {
+ /*
+ If it is a self-referencing FK for non-transactional table we might
+ be about to remove offending FK by impending row UPDATE or DELETE.
+ So additional check is needed.
+
+ Note that if we get FK_LOOKUP_ROW_EXIST when m_check_record_in_memory
+ is TRUE we know that there is only one matching row.
+
+ So let us check that we about to update or delete exactly this row.
+ This can be done in a variety of ways. What we do here is check that
+ in the parent instance of table OLD.FK is the same as key used for
+ lookup in the child instance (i.e. OLD.PK).
+ Also let us check that in case of UPDATE we change FK value to
+ something 'significantly' different.
+
+ It is a self-referencing FK on a non-transactional table and we might
+ be about to remove an offending child. An additional check is needed,
+ to not wrongly report an error.
+
+ Note that when m_check_record_in_memory is true, FK_LOOKUP_ROW_EXIST
+ means there is one and only one matching row.
+
+ So let us check that we are about to UPDATE or DELETE exactly this
+ row. In order to do that, we check that in the parent TABLE instance
+ OLD.FK is the same as the key used for lookup in the child instance
+ (i.e. OLD.PK).
+
+ If this is an UPDATE, we must also check that we change the child
+ value to something 'significantly' different, since otherwise it
+ still going to reference OLD.PK and thus should cause an error.
+ */
+ if ((! fk_key_cmp(m_old_child_columns_in_parent, m_key, m_key_buff)) &&
+ (m_fk_list->action_type() == TRG_EVENT_DELETE ||
+ fk_key_cmp(m_new_child_columns_in_parent, m_key, m_key_buff)))
+ {
+ /* Fine. Integrity will be preserved. */
+ return FALSE;
+ }
+ else
+ {
+ /*
+ Either the new version of row contains a child matching
+ the old (updated/deleted) parent, or we are about to delete
+ or update a row other than the one containing our child.
+ Fallthrough to constraint violation reporting.
+ */
+ }
+ }
/* Report a foreign key violation error. */
fk_report_error(ER_FK_CHILD_VALUE_EXISTS, m_fk_share->name.str,
m_parent_table, m_parent_columns_old);
return TRUE;
+ case FK_LOOKUP_SEVERAL_ROWS_EXIST:
+ /*
+ There are several rows with the same FK value.
+
+ Still it is OK if we are performing an UPDATE and parent key value
+ is changed 'insignificantly' enough so a new value of parent key can
+ serve as matching for the rows of child table which were corresponding
+ to an old value of parent key.
+
+ @sa Comment for similar situation in FK_LOOKUP_ROW_EXIST branch.
+ */
+ if (m_fk_list->action_type() == TRG_EVENT_UPDATE &&
+ ! has_changed_columns())
+ return FALSE;
+
+ /*
+ Since the impending UPDATE/DELETE won't affect all children,
+ some child will be surely left around. Report a foreign key
+ violation error.
+ */
+ fk_report_error(ER_FK_CHILD_VALUE_EXISTS, m_fk_share->name.str,
+ m_parent_table, m_parent_columns_old);
+ return TRUE;
default:
- m_child_table->file->print_error(error, MYF(0));
+ /* Error from the engine was already reported by fk_lookup(). */
return TRUE;
}
}
=== modified file 'sql/table.cc'
--- a/sql/table.cc 2009-01-26 13:40:43 +0000
+++ b/sql/table.cc 2009-01-30 13:41:12 +0000
@@ -4819,6 +4819,9 @@ void TABLE::mark_auto_increment_column()
mark all key columns as 'to-be-read'. This allows the engine to
loop over the given record to find all keys and doesn't have to
retrieve the row again.
+
+ Note that columns which are needed for performing foreign key checks
+ are marked in Foreign_key_parent_rcontext::prepare().
*/
void TABLE::mark_columns_needed_for_delete()
@@ -4826,20 +4829,6 @@ void TABLE::mark_columns_needed_for_dele
if (triggers)
triggers->mark_fields_used(TRG_EVENT_DELETE);
- if (!s->fkeys.fkeys_parent.is_empty())
- {
- /*
- If we have foreign keys that are referencing to this table we need
- to read values in foreign key columns as they will be required in
- order to perform checks or cascading actions on child tables.
- */
- List_iterator<Foreign_key_parent_share> fk_it(s->fkeys.fkeys_parent);
- Foreign_key_parent_share *fk;
- while ((fk= fk_it++))
- bitmap_union(read_set, &fk->column_set);
- file->column_bitmaps_signal();
- }
-
if (file->ha_table_flags() & HA_REQUIRES_KEY_COLUMNS_FOR_DELETE)
{
Field **reg_field;
@@ -4887,6 +4876,9 @@ void TABLE::mark_columns_needed_for_dele
mark all USED key columns as 'to-be-read'. This allows the engine to
loop over the given record to find all changed keys and doesn't have to
retrieve the row again.
+
+ Note that columns which are needed for performing foreign key checks
+ are marked in Foreign_key_child/parent_rcontext::prepare().
*/
void TABLE::mark_columns_needed_for_update()
@@ -4894,36 +4886,6 @@ void TABLE::mark_columns_needed_for_upda
DBUG_ENTER("mark_columns_needed_for_update");
if (triggers)
triggers->mark_fields_used(TRG_EVENT_UPDATE);
- if (!s->fkeys.fkeys_parent.is_empty())
- {
- /*
- If we are going to update any column which participates in a foreign
- key which references this table we need to read values for the rest
- of columns of this foreign key as they will be required in order to
- perform checks and cascading actions on the child table.
- */
- List_iterator<Foreign_key_parent_share> fk_it(s->fkeys.fkeys_parent);
- Foreign_key_parent_share *fk;
- while ((fk= fk_it++))
- if (bitmap_is_overlapping(write_set, &fk->column_set))
- bitmap_union(read_set, &fk->column_set);
- file->column_bitmaps_signal();
- }
- if (!s->fkeys.fkeys_child.is_empty())
- {
- /*
- If we are going to update any column which participates in a foreign
- key defined on this table we need to read values for the rest of
- columns of this foreign key as they will be required in order to
- perform checks on the parent table.
- */
- List_iterator<Foreign_key_child_share> fk_it(s->fkeys.fkeys_child);
- Foreign_key_child_share *fk;
- while ((fk= fk_it++))
- if (bitmap_is_overlapping(write_set, &fk->column_set))
- bitmap_union(read_set, &fk->column_set);
- file->column_bitmaps_signal();
- }
if (file->ha_table_flags() & HA_REQUIRES_KEY_COLUMNS_FOR_DELETE)
{
/* Mark all used key columns for read */
@@ -4973,8 +4935,6 @@ void TABLE::mark_columns_needed_for_inse
INSERT ... ON DUPLICATE KEY UPDATE, since before doing actual
row replacement or update write_record() will mark all table
fields as used.
- For the same reason we don't need to mark columns of foreign
- keys which might be affected by those clauses.
*/
if (triggers)
triggers->mark_fields_used(TRG_EVENT_INSERT);
| Thread |
|---|
| • bzr commit into mysql-6.1-fk branch (dlenev:2703) Bug#41761 | Dmitry Lenev | 30 Jan |
