From: Date: December 26 2008 6:19pm Subject: bzr commit into mysql-6.1-fk branch (dlenev:2694) WL#148 List-Archive: http://lists.mysql.com/commits/62354 Message-Id: <20081226171948.BA1DA6ECF87@mockturtle.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7BIT #At file:///home/dlenev/src/bzr/mysql-6.1-mil9/ 2694 Dmitry Lenev 2008-12-26 WL#148 "Foreign keys". Draft patch for milestone #9 "DML words: DML words: INSERT, UPDATE, DELETE, EOS checks". Work in progress. Questions for reviewer are marked by QQ. modified: mysql-test/r/foreign_key_acceptance.result mysql-test/r/foreign_key_all_engines.result mysql-test/t/foreign_key_all_engines.test sql/fk.cc sql/fk.h sql/sql_delete.cc sql/sql_insert.cc sql/sql_load.cc sql/sql_select.cc sql/sql_update.cc === modified file 'mysql-test/r/foreign_key_acceptance.result' --- a/mysql-test/r/foreign_key_acceptance.result 2008-12-13 23:33:23 +0000 +++ b/mysql-test/r/foreign_key_acceptance.result 2008-12-26 17:19:38 +0000 @@ -93,7 +93,7 @@ FOREIGN KEY (s2) REFERENCES t1 (s2) ON U INSERT INTO t1 VALUES (1,1),(2,2); INSERT INTO t2 VALUES (1,1),(2,2); DELETE FROM t1 WHERE s1 = 1; -ERROR 23000: Foreign key error: constraint 'fk_t2_81x6f': cannot change because foreign key refers to value '1' +ERROR 23000: Foreign key error: constraint 'fk_t2_r114w': cannot change because foreign key refers to value '1' SELECT COUNT(*) FROM t2 WHERE s1 = 1; COUNT(*) 1 === modified file 'mysql-test/r/foreign_key_all_engines.result' --- a/mysql-test/r/foreign_key_all_engines.result 2008-12-13 23:33:23 +0000 +++ b/mysql-test/r/foreign_key_all_engines.result 2008-12-26 17:19:38 +0000 @@ -1011,6 +1011,197 @@ on update set default); ERROR 42000: Foreign key error: Constraint 'fk_t2_m5zjv': SET DEFAULT for a column 'fk' which has no DEFAULT value set @@sql_mode= @old_sql_mode; drop tables t1; +# +# Basic coverage for the 9th milestone of WL#148 "Foreign keys" +# ("DML words: INSERT, UPDATE, DELETE, EOS checks"). +# +# Ensure that automatically generated names are stable +set @@rand_seed1=10000000,@@rand_seed2=1000000; +# Check that end-of-statement checks work as expected for various +# simple statements. Create table with a self-reference first. +create table t1 (pk int primary key, fk int references t1(pk)); +# Let us begin with checks on parent table +# This INSERT should succeed since at EOS all references are satisfied. +insert into t1 values (2, 1), (1, NULL); +select * from t1; +pk fk +2 1 +1 NULL +# And this should still fail +insert into t1 values (3, 4); +ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': no matching key for value '4', it is not in parent table +# This UPDATE should succeed +update t1 set pk= if(pk = 2, 2, 3), fk= if(fk = 1, 3, NULL); +select * from t1; +pk fk +2 3 +3 NULL +# And this should fail (and print error about failed check for value 4!) +update t1 set pk= if(pk = 2, 2, 3), fk= if(fk = 3, 4, 2); +ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': no matching key for value '4', it is not in parent table +# Now let us test EOS checks on child tables. +delete from t1; +insert into t1 values (1, NULL), (2, 1); +select * from t1; +pk fk +1 NULL +2 1 +# This UPDATE should succeed +update t1 set pk= if(pk = 1, 3, 2), fk= if(fk = 1, 3, NULL); +select * from t1; +pk fk +3 NULL +2 3 +# And this should fail +update t1 set pk= if(pk = 3, 4, 2), fk= if(fk = 3, 3, NULL); +ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '3' +# This DELETE should fail +delete from t1 where pk = 3; +ERROR 23000: Foreign key error: constraint 'fk_t1_11f06': cannot change because foreign key refers to value '3' +# And this should succeed +delete from t1; +select * from t1; +pk fk +# Finally example which emphasizes that is important to +# check both parent and child tables during EOS checks. +insert into t1 values (1, 3), (2, 1), (3, 3), (4, 4); +update t1 set +pk= case pk when 1 then 5 +when 2 then 2 +when 3 then 3 +when 4 then 1 +end, +fk= case pk when 1 then 3 +when 2 then 1 +when 3 then 3 +when 4 then 3 +end; +select * from t1; +pk fk +5 NULL +2 1 +3 3 +1 3 +drop table t1; +# Test that shows that EOS checks are needed even +# for cases without self-referential constraints. +# Also it is another test case which shows that EOS +# checks should look into both child and parent. +create table t1 (pk int primary key); +create table t2 (fk int references t1 (pk)); +insert into t1 values (1), (2); +insert into t2 values (1); +update t1 set pk= case pk when 1 then 3 when 2 then 1 end; +drop tables t1, t2; +# Test case that emphasizes that during EOS checks we should +# look into both parent and child even when we perform checks +# caused by modifications of child table. +create table t1 (pk int primary key); +create table t2 (fk1 int default 10 references t1 (pk) on delete set default, +fk2 int references t1 (pk) on delete cascade); +insert into t1 values (1), (2); +insert into t2 values (1, 2); +# This statement should succeed. Note that fact that update +# row and then delete it doesn't qualify as double update. +delete from t1; +drop tables t1, t2; +# Check that EOS checks properly work in case +# of RESTRICT as referential action. +create table t1 (pk int primary key, +fk int references t1 (pk) on delete restrict); +# EOS checks should be applied for INSERT +insert into t1 values (1, 2), (2, 2), (3, 2); +# And for UPDATE +update t1 set pk= case pk when 2 then 5 else pk end, fk= 5; +# But not for DELETE ! +delete from t1; +ERROR 23000: Foreign key error: constraint 'fk_t1_5jjs5': cannot change because foreign key refers to value '5' +drop table t1; +create table t1 (pk int primary key, +fk int references t1 (pk) on update restrict); +# As in previous case EOS checks should be applied for INSERT +insert into t1 values (1, 2), (2, 2), (3, 2); +# But UPDATE should emit an error +update t1 set pk= case pk when 2 then 5 else pk end, fk= 5; +ERROR 23000: Foreign key error: constraint 'fk_t1_dvssn': cannot change because foreign key refers to value '2' +# While for DELETE EOS check should work as well +delete from t1; +drop table t1; +# Test for EOS checks and ON DELETE CASCADE. They also +# should work for checks caused by cascading deletions. +create table t1 (pk int primary key); +create table t2 (pk int primary key, +fk int references t1(pk) on delete cascade); +create table t3 (fk1 int references t2 (pk) on delete no action, +fk2 int references t1 (pk) on delete cascade); +insert into t1 values (1), (2), (3), (4); +insert into t2 values (1, 1), (3, 3); +insert into t3 values (1, 2), (3, 4); +# This delete should succeed +delete from t1 where pk in (1, 2); +# And this should fail +delete from t1 where pk = 3; +ERROR 23000: Foreign key error: constraint 'fk_t3_6acsr': cannot change because foreign key refers to value '3' +drop tables t1, t2, t3; +# Test for EOS checks and ON UPDATE CASCADE. Again such +# checks should for checks caused by cascading updates +# both in child and parent direction. +create table t1 (pk int primary key); +create table t2 (a int, b int references t1 (pk) on update cascade, +primary key (a,b)); +create table t3 (fk1 int, fk2 int, foreign key (fk1,fk2) references t2 (a,b)); +insert into t1 values (1), (2), (3); +insert into t2 values (1, 1), (1, 3), (2, 2); +insert into t3 values (1, 1); +# This statement should succeed as by EOS there +# are no dangling references +update t1 set pk= case pk when 1 then 4 when 3 then 1 else pk end; +update t1 set pk= 5 where pk = 1; +ERROR 23000: Foreign key error: constraint 'fk_t3_tclwo': cannot change because foreign key refers to value '1-1' +# To enforce lookup from child updated by cascading +# action to parent we have to fiddle with trigger. +create trigger t2_bu before update on t2 for each row set new.b:= 7; +insert into t1 values (5); +# Again this wicked statement should succeed +update t1 set pk= case pk when 2 then 6 when 5 then 7 else pk end; +update t1 set pk= 8 where pk = 7; +ERROR 23000: Foreign key error: constraint 'fk_t2_uik15': no matching key for value '7', it is not in parent table +drop tables t1, t2, t3; +# Now similar tests for SET DEFAULT referential action +# First that EOS checks work for lookups in child of +# rows being updated by cascading action. +create table t1 (pk int primary key); +create table t2 (a int, +b int default 7 references t1 (pk) on delete set default, +primary key (a,b)); +create table t3 (fk1 int, fk2 int, fk3 int, +foreign key (fk1,fk2) references t2 (a,b), +foreign key (fk3) references t1 (pk) on delete cascade); +insert into t1 values (1), (2), (7); +insert into t2 values (1, 1); +insert into t3 values (1, 1, 2); +delete from t1 where pk = 1; +ERROR 23000: Foreign key error: constraint 'fk_t3_7z1hz': cannot change because foreign key refers to value '1-1' +delete from t1 where pk in (1, 2); +drop tables t1, t2, t3; +# Then check that they also work for lookups from in the parent +create table t1 (pk int primary key); +create table t2 (fk int default 42 references t1 (pk) on update set default); +insert into t1 values (1), (2); +insert into t2 values (1); +update t1 set pk= 3 where pk = 1; +ERROR 23000: Foreign key error: constraint 'fk_t2_vcbc1': no matching key for value '42', it is not in parent table +update t1 set pk= case pk when 1 then 3 when 2 then 42 end; +drop tables t1, t2; +# Check that EOS checks are not applied for non-transactional tables. +create table t1 (pk int primary key, +fk int references t1 (pk) on update restrict on delete restrict) +engine=myisam; +insert into t1 values (1, 2), (2, 2); +ERROR 23000: Foreign key error: constraint 'fk_t1_tyb6c': no matching key for value '2', it is not in parent table +# There is no sense in test for DELETE and UPDATE since non-transactional +# tables support only foreign keys with RESTRICT as referential action. +drop table t1; set @@rand_seed1=10000000,@@rand_seed2=1000000; drop tables if exists t1, t2; create table t2 (a int primary key); === modified file 'mysql-test/t/foreign_key_all_engines.test' --- a/mysql-test/t/foreign_key_all_engines.test 2008-12-13 23:33:23 +0000 +++ b/mysql-test/t/foreign_key_all_engines.test 2008-12-26 17:19:38 +0000 @@ -835,6 +835,186 @@ set @@sql_mode= @old_sql_mode; drop tables t1; +--echo # +--echo # Basic coverage for the 9th milestone of WL#148 "Foreign keys" +--echo # ("DML words: INSERT, UPDATE, DELETE, EOS checks"). +--echo # +--echo # Ensure that automatically generated names are stable +set @@rand_seed1=10000000,@@rand_seed2=1000000; +--echo # Check that end-of-statement checks work as expected for various +--echo # simple statements. Create table with a self-reference first. +create table t1 (pk int primary key, fk int references t1(pk)); +--echo # Let us begin with checks on parent table +--echo # This INSERT should succeed since at EOS all references are satisfied. +insert into t1 values (2, 1), (1, NULL); +select * from t1; +--echo # And this should still fail +--error ER_FK_CHILD_NO_MATCH +insert into t1 values (3, 4); +--echo # This UPDATE should succeed +update t1 set pk= if(pk = 2, 2, 3), fk= if(fk = 1, 3, NULL); +select * from t1; +--echo # And this should fail (and print error about failed check for value 4!) +--error ER_FK_CHILD_NO_MATCH +update t1 set pk= if(pk = 2, 2, 3), fk= if(fk = 3, 4, 2); +--echo # Now let us test EOS checks on child tables. +delete from t1; +insert into t1 values (1, NULL), (2, 1); +select * from t1; +--echo # This UPDATE should succeed +update t1 set pk= if(pk = 1, 3, 2), fk= if(fk = 1, 3, NULL); +select * from t1; +--echo # And this should fail +--error ER_FK_CHILD_VALUE_EXISTS +update t1 set pk= if(pk = 3, 4, 2), fk= if(fk = 3, 3, NULL); +--echo # This DELETE should fail +--error ER_FK_CHILD_VALUE_EXISTS +delete from t1 where pk = 3; +--echo # And this should succeed +delete from t1; +select * from t1; +--echo # Finally example which emphasizes that is important to +--echo # check both parent and child tables during EOS checks. +insert into t1 values (1, 3), (2, 1), (3, 3), (4, 4); +update t1 set + pk= case pk when 1 then 5 + when 2 then 2 + when 3 then 3 + when 4 then 1 + end, + fk= case pk when 1 then 3 + when 2 then 1 + when 3 then 3 + when 4 then 3 + end; +select * from t1; +drop table t1; +--echo # Test that shows that EOS checks are needed even +--echo # for cases without self-referential constraints. +--echo # Also it is another test case which shows that EOS +--echo # checks should look into both child and parent. +create table t1 (pk int primary key); +create table t2 (fk int references t1 (pk)); +insert into t1 values (1), (2); +insert into t2 values (1); +update t1 set pk= case pk when 1 then 3 when 2 then 1 end; +drop tables t1, t2; +--echo # Test case that emphasizes that during EOS checks we should +--echo # look into both parent and child even when we perform checks +--echo # caused by modifications of child table. +create table t1 (pk int primary key); +create table t2 (fk1 int default 10 references t1 (pk) on delete set default, + fk2 int references t1 (pk) on delete cascade); +insert into t1 values (1), (2); +insert into t2 values (1, 2); +--echo # This statement should succeed. Note that fact that update +--echo # row and then delete it doesn't qualify as double update. +delete from t1; +drop tables t1, t2; + +--echo # Check that EOS checks properly work in case +--echo # of RESTRICT as referential action. +create table t1 (pk int primary key, + fk int references t1 (pk) on delete restrict); +--echo # EOS checks should be applied for INSERT +insert into t1 values (1, 2), (2, 2), (3, 2); +--echo # And for UPDATE +update t1 set pk= case pk when 2 then 5 else pk end, fk= 5; +--echo # But not for DELETE ! +--error ER_FK_CHILD_VALUE_EXISTS +delete from t1; +drop table t1; +create table t1 (pk int primary key, + fk int references t1 (pk) on update restrict); +--echo # As in previous case EOS checks should be applied for INSERT +insert into t1 values (1, 2), (2, 2), (3, 2); +--echo # But UPDATE should emit an error +--error ER_FK_CHILD_VALUE_EXISTS +update t1 set pk= case pk when 2 then 5 else pk end, fk= 5; +--echo # While for DELETE EOS check should work as well +delete from t1; +drop table t1; + +--echo # Test for EOS checks and ON DELETE CASCADE. They also +--echo # should work for checks caused by cascading deletions. +create table t1 (pk int primary key); +create table t2 (pk int primary key, + fk int references t1(pk) on delete cascade); +create table t3 (fk1 int references t2 (pk) on delete no action, + fk2 int references t1 (pk) on delete cascade); +insert into t1 values (1), (2), (3), (4); +insert into t2 values (1, 1), (3, 3); +insert into t3 values (1, 2), (3, 4); +--echo # This delete should succeed +delete from t1 where pk in (1, 2); +--echo # And this should fail +--error ER_FK_CHILD_VALUE_EXISTS +delete from t1 where pk = 3; +drop tables t1, t2, t3; + +--echo # Test for EOS checks and ON UPDATE CASCADE. Again such +--echo # checks should for checks caused by cascading updates +--echo # both in child and parent direction. +create table t1 (pk int primary key); +create table t2 (a int, b int references t1 (pk) on update cascade, + primary key (a,b)); +create table t3 (fk1 int, fk2 int, foreign key (fk1,fk2) references t2 (a,b)); +insert into t1 values (1), (2), (3); +insert into t2 values (1, 1), (1, 3), (2, 2); +insert into t3 values (1, 1); +--echo # This statement should succeed as by EOS there +--echo # are no dangling references +update t1 set pk= case pk when 1 then 4 when 3 then 1 else pk end; +--error ER_FK_CHILD_VALUE_EXISTS +update t1 set pk= 5 where pk = 1; +--echo # To enforce lookup from child updated by cascading +--echo # action to parent we have to fiddle with trigger. +create trigger t2_bu before update on t2 for each row set new.b:= 7; +insert into t1 values (5); +--echo # Again this wicked statement should succeed +update t1 set pk= case pk when 2 then 6 when 5 then 7 else pk end; +--error ER_FK_CHILD_NO_MATCH +update t1 set pk= 8 where pk = 7; +drop tables t1, t2, t3; + +--echo # Now similar tests for SET DEFAULT referential action +--echo # First that EOS checks work for lookups in child of +--echo # rows being updated by cascading action. +create table t1 (pk int primary key); +create table t2 (a int, + b int default 7 references t1 (pk) on delete set default, + primary key (a,b)); +create table t3 (fk1 int, fk2 int, fk3 int, + foreign key (fk1,fk2) references t2 (a,b), + foreign key (fk3) references t1 (pk) on delete cascade); +insert into t1 values (1), (2), (7); +insert into t2 values (1, 1); +insert into t3 values (1, 1, 2); +--error ER_FK_CHILD_VALUE_EXISTS +delete from t1 where pk = 1; +delete from t1 where pk in (1, 2); +drop tables t1, t2, t3; +--echo # Then check that they also work for lookups from in the parent +create table t1 (pk int primary key); +create table t2 (fk int default 42 references t1 (pk) on update set default); +insert into t1 values (1), (2); +insert into t2 values (1); +--error ER_FK_CHILD_NO_MATCH +update t1 set pk= 3 where pk = 1; +update t1 set pk= case pk when 1 then 3 when 2 then 42 end; +drop tables t1, t2; + +--echo # Check that EOS checks are not applied for non-transactional tables. +create table t1 (pk int primary key, + fk int references t1 (pk) on update restrict on delete restrict) + engine=myisam; +--error ER_FK_CHILD_NO_MATCH +insert into t1 values (1, 2), (2, 2); +--echo # There is no sense in test for DELETE and UPDATE since non-transactional +--echo # tables support only foreign keys with RESTRICT as referential action. +drop table t1; + + # # Test for bug #35522 "Foreign keys: 'foreign key without name' errors" # In --foreign-key-all-engines mode we should not replace names of FKs === modified file 'sql/fk.cc' --- a/sql/fk.cc 2008-12-23 20:29:54 +0000 +++ b/sql/fk.cc 2008-12-26 17:19:38 +0000 @@ -20,6 +20,11 @@ #include "sp.h" +class Foreign_key_eos_buffer; +static KEY* find_supporting_key(TABLE *table, LEX_STRING *columns, + uint column_count); + + /** Report a foreign key violation error by emitting an error message that has the value of the foreign key which caused the violation. @@ -88,11 +93,15 @@ public: : m_thd(thd_arg), m_child_table(child_table_arg), m_fk_share(fk_share_arg), - m_column_count(fk_share_arg->column_count) + m_column_count(fk_share_arg->column_count), + m_do_eos_checks(FALSE), m_eos_buffer(NULL) { } + ~Foreign_key_child_rcontext(); + bool prepare(); bool check_parent_exists(); + bool do_eos_check(); private: THD *m_thd; @@ -122,6 +131,14 @@ private: columns to appropriate positions in the key buffer. */ store_key_field **m_key_copy; + + /** Indicates that EOS checks are applicable for this foreign key. */ + bool m_do_eos_checks; + /** + Buffer where we accumulate dangling foreign key values for re-checking + them at the end-of-statement. + */ + Foreign_key_eos_buffer *m_eos_buffer; }; @@ -140,12 +157,15 @@ 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_do_eos_checks(FALSE), m_eos_buffer(NULL) { m_ref_action= ((m_fk_list->action_type() == TRG_EVENT_UPDATE) ? m_fk_share->update_opt : m_fk_share->delete_opt); } + ~Foreign_key_parent_rcontext(); + bool prepare(bool old_row_is_record1); bool prepare_cascade(); bool check_children_exist(); @@ -164,6 +184,9 @@ public: inline enum_fk_option ref_action() const { return m_ref_action; } + bool do_eos_check(); + bool do_eos_checks_for_cascade(); + private: THD *m_thd; TABLE *m_parent_table; @@ -211,9 +234,101 @@ private: these modifications. */ Fk_constraint_list *m_cascade_fk_list; + + /** Indicates that EOS checks are applicable for this foreign key. */ + bool m_do_eos_checks; + /** + Buffer where we accumulate dangling foreign key values for re-checking + them at the end-of-statement. + */ + Foreign_key_eos_buffer *m_eos_buffer; }; +/** + Buffer which is used for accumulating values of the foreign key + for which no matching parent key value were found during statement + execution and for which additional check should be performed at the + end of statement. + + Standard semantics assumes that for NO ACTION foreign keys and for + INSERT statement all that matters is referential integrity at the + end of statement and temporary violations during statement execution + should be ignored. To implement this semantics we do not report an + error when we discover foreign key value without matching parent key + value during main phase of statement execution and add this dangling + value to this buffer instead. At the end of statement execution we + iterate through all values in this buffer and report error if only + one of offending foreign key values is still present in child table + and no matching parent key value were added for it. +*/ + +class Foreign_key_eos_buffer : public Sql_alloc +{ +public: + Foreign_key_eos_buffer(THD *thd, Foreign_key_share *fk_share, + Field **src_columns, int error, + TABLE *child_table, KEY *child_key, + TABLE *parent_table, KEY *parent_key) + : m_thd(thd), m_fk_share(fk_share), m_src_columns(src_columns), + m_column_count(fk_share->column_count), m_error(error), + m_eos_buffer(NULL), + m_child_table(child_table), m_child_key(child_key), + m_child_key_idx(child_key - child_table->key_info), + m_parent_table(parent_table), m_parent_key(parent_key), + m_parent_key_idx(parent_key - parent_table->key_info) + { } + + ~Foreign_key_eos_buffer() + { + if (m_eos_buffer) + free_tmp_table(m_thd, m_eos_buffer); + } + + bool prepare(); + bool add_fk_value(); + bool do_eos_check(); + +private: + THD *m_thd; + Foreign_key_share *m_fk_share; + + /** + Field objects for columns from which values should be copied + when foreign key value is added to the EOS buffer. + */ + Field **m_src_columns; + uint m_column_count; + + /** + Error code for error to be reported when foreign key violation + is discovered during EOS check. + */ + int m_error; + + /** + Internal temporary table which serves as buffer where offending + foreign key values are accumulated and TMP_TABLE_PARAM which is + used for its creation. + */ + TABLE *m_eos_buffer; + TMP_TABLE_PARAM m_eos_buffer_param; + + TABLE *m_child_table; + KEY *m_child_key; + uint m_child_key_idx; + uchar *m_child_key_buff; + store_key_field **m_child_key_copy; + + TABLE *m_parent_table; + KEY *m_parent_key; + uint m_parent_key_idx; + uchar *m_parent_key_buff; + store_key_field **m_parent_key_copy; + + bool check_fk_value(); +}; + /** For INSERT or UPDATE operation on the table, prepare runtime contexts for all @@ -256,11 +371,12 @@ Fk_constraint_list::prepare_check_parent Foreign_key_child_rcontext *rctx= new (thd->mem_root) Foreign_key_child_rcontext(thd, table, fk_share); - if (rctx == NULL || rctx->prepare()) - return TRUE; /* No need to call destructor for rctx */ - - if (m_fkeys_child.push_back(rctx, thd->mem_root)) - return TRUE; /* No need to call destructor or cleanup rctx */ + if (rctx == NULL || rctx->prepare() || + m_fkeys_child.push_back(rctx, thd->mem_root)) + { + delete rctx; + return TRUE; + } } } return FALSE; @@ -308,17 +424,26 @@ Fk_constraint_list::prepare_check_child( Foreign_key_parent_rcontext *rctx= new (thd->mem_root) Foreign_key_parent_rcontext(thd, table, fk_share, this); - if (rctx->prepare(old_row_is_record1)) + if (rctx == NULL || rctx->prepare(old_row_is_record1)) + { + delete rctx; return TRUE; + } if (rctx->is_cascading()) { if (rctx->prepare_cascade() || m_fkeys_parent_cascade.push_back(rctx, thd->mem_root)) + { + delete rctx; return TRUE; + } } else if (m_fkeys_parent.push_back(rctx, thd->mem_root)) + { + delete rctx; return TRUE; + } } } return FALSE; @@ -326,24 +451,41 @@ Fk_constraint_list::prepare_check_child( /** + Release resources associated with runtime context for foreign key + checks in operation. +*/ + +Fk_constraint_list::~Fk_constraint_list() +{ + List_iterator child_it(m_fkeys_child); + List_iterator parent_it(m_fkeys_parent); + List_iterator cascade_it(m_fkeys_parent_cascade); + Foreign_key_child_rcontext *child_ctx; + Foreign_key_parent_rcontext *parent_ctx; + + while ((child_ctx= child_it++)) + delete child_ctx; + + while ((parent_ctx= parent_it++)) + delete parent_ctx; + + while ((parent_ctx= cascade_it++)) + delete parent_ctx; +} + + +/** Check that for all foreign keys in which the currently modified or inserted row serves as a child there is a corresponding row in the parent table, or that such parent row is not required. - @param table TABLE object representing table being modified. - @param use_eos Indicates if it is allowed to postpone check - till the end of statement. - @retval FALSE Success. @retval TRUE Error, a constraint violation. */ bool -Fk_constraint_list::do_check_parent_list(TABLE *table, bool use_eos) +Fk_constraint_list::do_check_parent_list() { -#ifdef ONCE_EOS_CHECKS_ARE_IMPLEMENTED - bool need_eos_check= FALSE; -#endif List_iterator fkctx_it(m_fkeys_child); Foreign_key_child_rcontext *fk_ctx; @@ -356,44 +498,7 @@ Fk_constraint_list::do_check_parent_list */ if (fk_ctx->check_parent_exists()) return TRUE; - -#ifdef ONCE_EOS_CHECKS_ARE_IMPLEMENTED - /* Handler returned "the parent does not exist" */ - if (fk_statement_flags&FK_STATEMENT_FLAG_NO_EOS == 0) - /* End-of-statement check is possible. So set flag + continue. */ - need_eos_check= true; - continue 'for each constraint in constraint list' loop - else - /* End-of-statement check is not possible. */ - if (parent->relationship == SELF_REFERENCING_TABLE) - /* - Lack of end-of-statement checks limits the set of supported - foreign key operations to non-recursive relationships, and - recursive relationships for which record consistency may be - verified using the current snapshot of the table at the time - of checking plus the new value of the subject record. - InnoDB currently operates with a similar limitation. - */ - if (fk_check_record_in_memory(record) == TRUE) - /* Self-reference and it's in memory. */ - continue 'for each constraint in constraint list' loop - error(ER_FK_CHILD_NO_MATCH); -#endif } - -#ifdef ONCE_EOS_CHECKS_ARE_IMPLEMENTED - if (need_eos_check) - /* - Foreign key needs to match, and at least one foreign-key-column - changed, and the new value was not found in the parent table, - and end-of-statement check is possible. - So save record contents to be checked at end-of-statement. - WE SAVE ONLY ONCE PER ROW -- we do not record which foreign key - failed, so if there are many foreign keys the end-of-statement - validity checker will check them all. - */ - fk_put_record_to_eos_buffer(table, record, changed_column_map, ACTION_INSERT); -#endif return FALSE; } @@ -403,20 +508,13 @@ Fk_constraint_list::do_check_parent_list or deleted row serves as a parent there are no corresponding rows in any child table. - @param table TABLE object representing table being modified. - @param use_eos Indicates if it is allowed to postpone check - till the end of statement. - @retval FALSE Success. There are no child rows. @retval TRUE Error, a constraint violation. There is a child row. */ bool -Fk_constraint_list::do_check_child_list(TABLE *table, bool use_eos) +Fk_constraint_list::do_check_child_list() { -#ifdef ONCE_EOS_CHECKS_ARE_IMPLEMENTED - bool need_eos_check= FALSE; -#endif List_iterator fkctx_it(m_fkeys_parent); Foreign_key_parent_rcontext *fk_ctx; @@ -426,60 +524,7 @@ Fk_constraint_list::do_check_child_list( 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) - { - /* - 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; - 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); -#endif } -#ifdef ONCE_EOS_CHECKS_ARE_IMPLEMENTED - if (need_eos_check) - /* - At least one parent-key-column changed, and the change was significant, - and the new value was not found in the child table, and end-of-statement - check is possible, and the constraint is NO ACTION rather than RESTRICT. - So save record contents to be checked at end-of-statement. - WE SAVE ONLY ONCE PER ROW -- we do not record which foreign key failed, - so if there are many NO ACTION foreign keys the end-of-statement - validity checker will check them all. - */ - fk_put_record_to_eos_buffer(table, record, changed_column_map, ACTION_DELETE); -#endif return FALSE; } @@ -489,15 +534,12 @@ Fk_constraint_list::do_check_child_list( row serves as a parent perform cascading actions on corresponding rows in child tables. - @param table TABLE object representing table being modified. - @param fk_op_ctx Foreign key context of operation. - @retval FALSE Success. @retval TRUE Error. */ bool -Fk_constraint_list::do_cascading_action_child_list(TABLE *table) +Fk_constraint_list::do_cascading_action_child_list() { #ifdef ONCE_RECURSION_STACK_IS_IMPLEMENTED if (fk_recursion_stack.find_record(oldrecord, @@ -534,6 +576,40 @@ Fk_constraint_list::do_cascading_action_ /** + Check if at the end of statement foreign keys which were made + dangling while performing this operation are still dangling + and report appropriate error. + + @retval FALSE Success. There are no dangling references. + @retval TRUE Failure. Some dangling references still present, + error was reported. +*/ + +bool Fk_constraint_list::do_eos_checks() +{ + List_iterator fkctx_c_it(m_fkeys_child); + List_iterator fkctx_p_it(m_fkeys_parent); + List_iterator fkctx_a_it(m_fkeys_parent_cascade); + Foreign_key_child_rcontext *fk_c_ctx; + Foreign_key_parent_rcontext *fk_p_ctx; + + while ((fk_c_ctx= fkctx_c_it++)) + if (fk_c_ctx->do_eos_check()) + return TRUE; + + while ((fk_p_ctx= fkctx_p_it++)) + if (fk_p_ctx->do_eos_check()) + return TRUE; + + while ((fk_p_ctx= fkctx_a_it++)) + if (fk_p_ctx->do_eos_checks_for_cascade()) + return TRUE; + + return FALSE; +} + + +/** Find a key (index) in a table by name. @param table TABLE which index should be found. @@ -744,10 +820,118 @@ 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 (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 end-of-statement buffer and lookup structures. ***/ + + if (m_child_table->file->has_transactions()) + { + /* + Performing data-change operation on transactional table is + a necessary prerequisite for doing end-of-statement checks. + + QQ/TODO: Think about case of multi-table operations. + + Conditions which describe when it _makes sense_ to do such + checking for Foreign_key_child_rcontext, i.e. when result + with EOS checks will be any better than without them, can + be formulated as: "It makes sense to perform EOS checks when + there are chances that parent table will be modified before + EOS or offending row in child table will be modified before + EOS (e.g. due to REPLACE/INSERT ... ON DUPLICATE KEY UPDATE + or cascading delete). + + TODO: implement the second set of conditions as an optimization. + */ + KEY *child_key; + + m_do_eos_checks= TRUE; + + if (!(child_key= find_supporting_key(m_child_table, + m_fk_share->child_columns, + m_column_count))) + { + my_error(ER_FK_CHILD_NO_INDEX, MYF(0), m_fk_share->name.str); + return TRUE; + } + + if (!(m_eos_buffer= new (m_thd->mem_root) Foreign_key_eos_buffer(m_thd, + m_fk_share, m_child_columns, + ER_FK_CHILD_NO_MATCH, + m_child_table, child_key, + m_parent_table, m_key))) + return TRUE; + + if (m_eos_buffer->prepare()) + return TRUE; + } + return FALSE; +} + + +/** + Release resources associated with this runtime context for + foreign key checks performed when child table is updated. +*/ + +Foreign_key_child_rcontext::~Foreign_key_child_rcontext() +{ + delete m_eos_buffer; +} + + +/** + Result of check if table contains any rows with particular key value. +*/ + +enum enum_fk_lookup_result +{ + FK_LOOKUP_NO_ROWS, FK_LOOKUP_ROW_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 column_count Number of columns participating in this key. + @param copy Array of auxiliary objects to be used for + constructing key value for lookup. + + @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. + @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, uint column_count, + store_key_field **copy) +{ + int error; + uint idx; + + for (idx= 0 ; idx < column_count; idx++) + copy[idx]->copy(); + + error= key_table->file->index_read_idx_map(key_table->record[0], + key_idx, key_buff, + HA_WHOLE_KEY, + HA_READ_KEY_EXACT); + switch (error) { + case 0: + return FK_LOOKUP_ROW_EXIST; + case HA_ERR_END_OF_FILE: + case HA_ERR_KEY_NOT_FOUND: + return FK_LOOKUP_NO_ROWS; + default: + key_table->file->print_error(error, MYF(0)); + return FK_LOOKUP_ERROR; + } } @@ -762,8 +946,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. @@ -803,6 +986,7 @@ bool Foreign_key_child_rcontext::check_p if (has_null_columns && has_non_null_columns) { + /* QQ: Should not we apply EOS check in this case as well? */ /* MATCH FULL: One of columns has a non-NULL value while there is at least one column with NULL value. This is an error. @@ -826,30 +1010,56 @@ bool Foreign_key_child_rcontext::check_p Handler interaction point #1: look up child value in parent table. */ - for (idx= 0 ; idx < m_key->key_parts; idx++) - m_key_copy[idx]->copy(); - 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: + result= fk_lookup(m_parent_table, m_key_idx, m_key_buff, m_column_count, + m_key_copy); + + switch (result) { + case FK_LOOKUP_ROW_EXIST: /* Matching primary key value was found in parent table. */ 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. */ - fk_report_error(ER_FK_CHILD_NO_MATCH, m_fk_share->name.str, m_child_table, - m_child_columns); - return TRUE; + case FK_LOOKUP_NO_ROWS: + /* + No matching key was found. If we can and it makes sense to + perform EOS checks add value of offending foreign key to EOS + buffer. Otherwise report a foreign key violation error. + */ + if (m_do_eos_checks) + { + return m_eos_buffer->add_fk_value(); + } + else + { + 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 engine was already reported by fk_lookup(). */ return TRUE; } } /** + Check if at the end of statement values of the foreign key which + were added to the EOS buffer as dangling still are dangling and + report appropriate error. + + @retval FALSE Success. There are no dangling references. + @retval TRUE Failure. Some dangling references still present, + error was reported. +*/ + +bool Foreign_key_child_rcontext::do_eos_check() +{ + if (m_do_eos_checks) + return m_eos_buffer->do_eos_check(); + else + return FALSE; +} + + +/** Find table's index which contains all columns from the list and nothing else. @@ -1014,10 +1224,62 @@ 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); + 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->file->has_transactions() && + m_ref_action == FK_OPTION_NO_ACTION) + { + /* + Performing data-change operation on transactional table is + a necessary prerequisite for doing end-of-statement checks. + + QQ/TODO: Think about case of multi-table operations. + + Also standard says that EOS checks should not be done after + updating parent table if referential action is RESTRICT. + + Conditions which describe when it _makes sense_ to do such + checking for Foreign_key_parent_rcontext, i.e. when result + with EOS checks will be any better than without them, can + be formulated as: "It makes sense to perform EOS checks when + it is possible that child table will be modified before EOS + in such wayt that offending foreign key value will be replaced + or removed or that parent table will be modifed before EOS and + matching parent key value will be added to it". + + TODO: implement the second set of conditions as an optimization. + */ + KEY *parent_key; + + m_do_eos_checks= TRUE; + + /* + QQ: Alternatively we can also store name of parent key in .FRM of + parent table and use find_key_by_name here... ? + */ + if (!(parent_key= find_supporting_key(m_parent_table, + m_fk_share->parent_columns, + m_column_count))) + { + my_error(ER_FK_PARENT_KEY_NO_PK_OR_UNIQUE, MYF(0), m_fk_share->name.str); + return TRUE; + } + + if (!(m_eos_buffer= new (m_thd->mem_root) Foreign_key_eos_buffer(m_thd, + m_fk_share, m_parent_columns_old, + ER_FK_CHILD_VALUE_EXISTS, + m_child_table, m_key, + m_parent_table, parent_key))) + return TRUE; + + if (m_eos_buffer->prepare()) + return TRUE; + } + + return FALSE; } @@ -1130,6 +1392,18 @@ bool Foreign_key_parent_rcontext::prepar /** + Release resources associated with this runtime context for foreign + key checks/actions performed when parent table is updated. +*/ + +Foreign_key_parent_rcontext::~Foreign_key_parent_rcontext() +{ + delete m_cascade_fk_list; + delete m_eos_buffer; +} + + +/** Check if child table contains any records referencing to old values of foreign key in record being modified. @@ -1139,36 +1413,80 @@ 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. + + TODO/FIXME: Add comment about visibility and RESTRICT. */ - 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: + enum_fk_lookup_result result= fk_lookup(m_child_table, m_key_idx, + m_key_buff, m_column_count, + m_key_copy); + + switch (result) { + case FK_LOOKUP_NO_ROWS: /* No matching children exist. Everything is OK. */ return FALSE; - case 0: - /* Child rows exist. 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_ROW_EXIST: + /* + Child rows exist. If we can do EOS checks and it makes sense to + do them add offending foreign key value to EOS buffer. Otherwise + report a foreign key violation error. + */ + if (m_do_eos_checks) + { + DBUG_ASSERT(m_ref_action == FK_OPTION_NO_ACTION); + return m_eos_buffer->add_fk_value(); + } + else + { + 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 engine was already reported by fk_lookup(). */ return TRUE; } } /** + Check if at the end of statement values of the foreign key which + were added to the EOS buffer as dangling are still dangling and + report appropriate error. + + @retval FALSE Success. There are no dangling references. + @retval TRUE Failure. Some dangling references still present, + error was reported. +*/ + +bool Foreign_key_parent_rcontext::do_eos_check() +{ + if (m_do_eos_checks) + return m_eos_buffer->do_eos_check(); + else + return FALSE; +} + + +/** + Perform end-of-statement validation for all foreign keys which + were affected by cascading changes caused by this foreign key. + + @retval FALSE Success. There are no dangling references. + @retval TRUE Failure. Some dangling references still present, + error was reported. +*/ + +bool Foreign_key_parent_rcontext::do_eos_checks_for_cascade() +{ + DBUG_ASSERT(is_cascading()); + + return m_cascade_fk_list->do_eos_checks(); +} + + +/** For UPDATE check if old and new versions of values for columns participating in the foreign key differ significantly enough to cause a cascading action. @@ -1237,21 +1555,27 @@ bool Foreign_key_parent_rcontext::do_cas Tell the handler: "we want to find (for UPDATE or DELETE purposes) all the rows in child table where values = (oldrecord values). Please set up a cursor." + + TODO/FIXME: Add explanation about visibility rules and double updates. + Add explanation why stop-gap for infinite cycle (e.g. + because of set default/triggers) is not necessary. */ for (i= 0 ; i < m_key->key_parts; i++) m_key_copy[i]->copy(); - if ((error= m_child_table->file->ha_index_init(m_key_idx, FALSE))) + if ((error= file->ha_index_init(m_key_idx, FALSE))) { - m_child_table->file->print_error(error, MYF(0)); + file->print_error(error, MYF(0)); return TRUE; } - while (!(error= file->index_read_map(m_child_table->record[0], - m_key_buff, HA_WHOLE_KEY, - HA_READ_KEY_EXACT))) - { + error= file->index_read_map(m_child_table->record[0], m_key_buff, + HA_WHOLE_KEY, HA_READ_KEY_EXACT); + + /* QQ: Does it make sense to make this operation interruptible ? */ + while (!error && !m_thd->killed) + { /* Since we always know that below we modify transactional table we should not bother about THD::transaction.stmt.modified_non_trans_table. @@ -1309,6 +1633,9 @@ bool Foreign_key_parent_rcontext::do_cas m_cascade_fk_list)) goto err_with_index_end; } + + error= file->index_next_same(m_child_table->record[0], m_key_buff, + m_key->key_length); } if (error == HA_ERR_KEY_NOT_FOUND || error == HA_ERR_END_OF_FILE) @@ -1318,10 +1645,230 @@ bool Foreign_key_parent_rcontext::do_cas return FALSE; } - file->print_error(error, MYF(0)); + /* We can have error == 0 if our statement or connection was killed. */ + if (error) + file->print_error(error, MYF(0)); err_with_index_end: file->ha_index_end(); return TRUE; } + +/** + Prepare end-of-statement buffer for accumulating values of dangling + foreign keys and setup structures for validating them at the EOS. + + @retval FALSE Success. + @retval TRUE Error. +*/ + +bool Foreign_key_eos_buffer::prepare() +{ + List eos_buffer_fields; + Item_field *item; + uint idx; + + m_eos_buffer_param.init(); + m_eos_buffer_param.field_count= m_column_count; + + for (idx= 0; idx < m_column_count; ++idx) + { + if (!(item= new (m_thd->mem_root) Item_field(m_src_columns[idx])) || + eos_buffer_fields.push_back(item, m_thd->mem_root)) + return TRUE; + } + + if (!(m_eos_buffer= create_tmp_table(m_thd, &m_eos_buffer_param, + eos_buffer_fields, (ORDER*)0, 0, 0, + TMP_TABLE_ALL_COLUMNS, HA_POS_ERROR, + (char *) ""))) + return TRUE; + + if (prepare_key_copy(m_thd, m_child_key, + m_eos_buffer->field, m_fk_share->child_columns, + m_column_count, &m_child_key_buff, &m_child_key_copy)) + return TRUE; + + if (prepare_key_copy(m_thd, m_parent_key, + m_eos_buffer->field, m_fk_share->parent_columns, + m_column_count, &m_parent_key_buff, &m_parent_key_copy)) + return TRUE; + + m_eos_buffer->file->extra(HA_EXTRA_WRITE_CACHE); + + return FALSE; +} + + +/** + Add foreign key value to the buffer of dangling foreign key + values which should be validated at the end of statement. + + @retval FALSE Success. + @retval TRUE Failure. +*/ + +bool Foreign_key_eos_buffer::add_fk_value() +{ + Copy_field *copy; + int error; + + for (copy= m_eos_buffer_param.copy_field; + copy < m_eos_buffer_param.copy_field_end; + copy++) + (*copy->do_copy)(copy); + + if ((error= m_eos_buffer->file->ha_write_row(m_eos_buffer->record[0]))) + { + /* + If it is a "table is full" error convert in-memory temporary + table to on-disk temporary table. Report an error otherwise. + */ + if (create_internal_tmp_table_from_heap(m_thd, m_eos_buffer, + m_eos_buffer_param.start_recinfo, + &m_eos_buffer_param.recinfo, + error, 0)) + return TRUE; + } + return FALSE; +} + + +/** + Check if foreign key value which just have been read from EOS buffer + is still dangling and report appropriate error. + + @retval FALSE There is no row with such foreign key or corresponding + parent key was added. I.e. we no longer have dangling + foreign key. + @retval TRUE Foreign key value is still dangling. Report appropriate + error. +*/ + +bool Foreign_key_eos_buffer::check_fk_value() +{ + enum_fk_lookup_result result; + + /* + When current statement (also) inserts or updates rows in the parent + table there is chance that we have introduced new parent key for + foreign key value from EOS buffer. + + QQ/FIXME: Again how can we detect this situation to avoid these + lookups on other cases? + Note the diamond-shaped relationships... + */ + result= fk_lookup(m_parent_table, m_parent_key_idx, m_parent_key_buff, + m_column_count, m_parent_key_copy); + + switch (result) { + case FK_LOOKUP_ROW_EXIST: + /* Matching parent key value was found in parent table. */ + return FALSE; + case FK_LOOKUP_NO_ROWS: + /* + No parent key was found. Still there is a chance that row + of the child table which contained offending foreign key + was deleted (e.g. in case of REPLACE or cascading change) or + updated (e.g. in cased of INSERT ON DUPLICATE KEY UPDATE). + So do further checking instead of reporting error. + */ + break; + default: + /* Error from engine was already reported by fk_lookup(). */ + return TRUE; + } + + /* + When there is a chance that current statement will update or + delete rows with dangling references it makes sense to check + if these offending foreign keys values are still present in the + child table by the EOS. + + QQ/FIXME: Add condition which will ensure that this lookup + is done only if such possibility really exists + (e.g. is not done for normal INSERT statements). + */ + result= fk_lookup(m_child_table, m_child_key_idx, m_child_key_buff, + m_column_count, m_child_key_copy); + + switch (result) { + case FK_LOOKUP_NO_ROWS: + /* Row with offending foreign key value no longer exists. Excellent! */ + return FALSE; + case FK_LOOKUP_ROW_EXIST: + /* + Offending foreign key value is still there. + Fallthrough to error reporting. + */ + break; + default: + /* Error from engine was already reported by fk_lookup(). */ + return TRUE; + } + + /* + We have a row in child table which references to non-existing value + of parent key. Report an appropriate foreign key violation error. + */ + fk_report_error(m_error, m_fk_share->name.str, m_eos_buffer, + m_eos_buffer->field); + return TRUE; +} + + +/** + At the end of statement for all values of foreign keys in EOS buffer + check if they are still dangling and report appropriate error. + + @retval FALSE Success. There are no dangling references. + @retval TRUE Failure. Some dangling references still present, + error was reported. +*/ + +bool Foreign_key_eos_buffer::do_eos_check() +{ + int error; + + /* Switch to read caching. */ + m_eos_buffer->file->extra(HA_EXTRA_CACHE); + + if ((error= m_eos_buffer->file->ha_rnd_init(1))) + { + m_eos_buffer->file->print_error(error, MYF(0)); + return TRUE; + } + + /* QQ: Does it make sense to make this operation interruptible ? */ + + while (!m_thd->killed && + !(error= m_eos_buffer->file->rnd_next(m_eos_buffer->record[0]))) + { + if (check_fk_value()) + { + m_eos_buffer->file->ha_rnd_end(); + return TRUE; + } + } + + switch (error) { + case HA_ERR_END_OF_FILE: + /* + We have performed EOS checks for all foreign keys in buffer. + And found that all previously un-matched values have match + now or were removed from the child table. + */ + m_eos_buffer->file->ha_rnd_end(); + return FALSE; + case 0: + /* Our statement or connection were killed. */ + DBUG_ASSERT(m_thd->killed); + m_eos_buffer->file->ha_rnd_end(); + return TRUE; + default: + m_eos_buffer->file->print_error(error, MYF(0)); + m_eos_buffer->file->ha_rnd_end(); + return TRUE; + } +} === modified file 'sql/fk.h' --- a/sql/fk.h 2008-12-13 23:33:23 +0000 +++ b/sql/fk.h 2008-12-26 17:19:38 +0000 @@ -39,6 +39,8 @@ public: : m_action_type(action_type_arg) { } + ~Fk_constraint_list(); + inline trg_event_type action_type() const { return m_action_type; } bool prepare_check_parent(THD *thd, TABLE *table, @@ -49,32 +51,41 @@ public: MY_BITMAP *changed_column_map); /** Inline wrappers to speed up the most common case. */ - bool check_parent_list(TABLE *table, bool use_eos) + bool check_parent_list() { return (m_fkeys_child.elements && - do_check_parent_list(table, use_eos)); + do_check_parent_list()); } - bool check_child_list(TABLE *table, bool use_eos) + bool check_child_list() { return (m_fkeys_parent.elements && - do_check_child_list(table, use_eos)); + do_check_child_list()); } - bool cascading_action_child_list(TABLE *table) + bool cascading_action_child_list() { return (m_fkeys_parent_cascade.elements && - do_cascading_action_child_list(table)); + do_cascading_action_child_list()); + } + + bool eos_checks() + { + return ((m_fkeys_child.elements || m_fkeys_parent.elements || + m_fkeys_parent_cascade.elements) && do_eos_checks()); } + bool do_eos_checks(); + private: /** Implementation. */ - bool do_check_parent_list(TABLE *table, bool use_eos); + bool do_check_parent_list(); + + bool do_check_child_list(); - bool do_check_child_list(TABLE *table, bool use_eos); + bool do_cascading_action_child_list(); - bool do_cascading_action_child_list(TABLE *table); private: /** Type of performed operation: INSERT, UPDATE or DELETE. Defines === modified file 'sql/sql_delete.cc' --- a/sql/sql_delete.cc 2008-12-23 20:29:54 +0000 +++ b/sql/sql_delete.cc 2008-12-26 17:19:38 +0000 @@ -67,7 +67,7 @@ bool delete_record(THD *thd, TABLE *tabl tables are barred from participating in foreign keys requiring such actions. */ - if (fk_list->check_child_list(table, FALSE)) + if (fk_list->check_child_list()) return TRUE; } @@ -93,8 +93,7 @@ bool delete_record(THD *thd, TABLE *tabl If we modify transactional table we perform checks and cascading actions on child table after deleting the row. */ - if (fk_list->check_child_list(table, FALSE) || - fk_list->cascading_action_child_list(table)) + if (fk_list->check_child_list() || fk_list->cascading_action_child_list()) return TRUE; } @@ -411,6 +410,20 @@ bool mysql_delete(THD *thd, TABLE_LIST * else table->file->unlock_row(); // Row failed selection, release lock on it } + + if (error < 0) + { + /* + There is no sense in doing potentially expensive EOS check if statement + will result in error and thus will be rolled back anyway. + + Since bulk deletion is disabled in the presence of foreign keys it + doesn't matter that this check is performed before end_bulk_delete(). + */ + if (fk_list.eos_checks()) + error= 1; + } + killed_status= thd->killed; if (killed_status != THD::NOT_KILLED || thd->is_error()) error= 1; // Aborted === modified file 'sql/sql_insert.cc' --- a/sql/sql_insert.cc 2008-12-13 23:33:23 +0000 +++ b/sql/sql_insert.cc 2008-12-26 17:19:38 +0000 @@ -876,6 +876,17 @@ bool mysql_insert(THD *thd,TABLE_LIST *t if (duplic != DUP_ERROR || ignore) table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); + if (error <= 0) + { + /* + There is no sense in doing potentially expensive EOS check if + statement will result in error and thus will be rolled back + anyway. + */ + if (fk_list.eos_checks()) + error= 1; + } + transactional_table= table->file->has_transactions(); if ((changed= (info.copied || info.deleted || info.updated))) @@ -1628,7 +1639,7 @@ int write_record(THD *thd, TABLE *table, such check does not takes any locks on parent rows in parent table (e.g. as it happens for Falcon). */ - if (fk_list->check_parent_list(table, FALSE)) + if (fk_list->check_parent_list()) goto before_trg_err; } @@ -1641,8 +1652,7 @@ int write_record(THD *thd, TABLE *table, goto ok_or_after_trg_err; } - if (has_transactions && - fk_list->check_parent_list(table, FALSE)) + if (has_transactions && fk_list->check_parent_list()) { info->copied++; thd->record_first_successful_insert_id_in_cur_stmt(table->file->insert_id_for_cur_row); @@ -3177,6 +3187,8 @@ select_insert::~select_insert() table->auto_increment_field_not_null= FALSE; table->file->ha_reset(); } + /* QQ: is it proper place ? when this thing is called ? */ + delete fk_list; thd->count_cuted_fields= CHECK_FIELD_IGNORE; thd->abort_on_warning= 0; DBUG_VOID_RETURN; @@ -3272,16 +3284,32 @@ bool select_insert::send_eof() bool const trans_table= table->file->has_transactions(); ulonglong id; bool changed; - THD::killed_state killed_status= thd->killed; + THD::killed_state killed_status; DBUG_ENTER("select_insert::send_eof"); DBUG_PRINT("enter", ("trans_table=%d, table_type='%s'", trans_table, table->file->table_type())); - error= ((thd->locked_tables_mode <= LTM_LOCK_TABLES) ? - table->file->ha_end_bulk_insert(0) : 0); + if ((error= ((thd->locked_tables_mode <= LTM_LOCK_TABLES) ? + table->file->ha_end_bulk_insert(0) : 0))) + { + table->file->print_error(error, MYF(0)); + } + table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); + if (!error) + { + error= fk_list->eos_checks(); + } + + /* + For purproses of binary logging after this point we can ignore the + fact statement was killed as it won't affect result of statement + execution. + */ + killed_status= thd->killed; + if (changed= (info.copied || info.deleted || info.updated)) { /* @@ -3313,7 +3341,6 @@ bool select_insert::send_eof() if (error) { - table->file->print_error(error,MYF(0)); MYSQL_INSERT_SELECT_DONE(error, 0); DBUG_RETURN(1); } === modified file 'sql/sql_load.cc' --- a/sql/sql_load.cc 2008-12-13 23:33:23 +0000 +++ b/sql/sql_load.cc 2008-12-26 17:19:38 +0000 @@ -450,6 +450,12 @@ int mysql_load(THD *thd,sql_exchange *ex table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY); table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE); table->next_number_field=0; + + if (!error) + { + if (fk_list.eos_checks()) + error= 1; + } } if (file >= 0) my_close(file,MYF(0)); === modified file 'sql/sql_select.cc' --- a/sql/sql_select.cc 2008-12-10 11:42:31 +0000 +++ b/sql/sql_select.cc 2008-12-26 17:19:38 +0000 @@ -12156,12 +12156,21 @@ create_tmp_table(THD *thd,TMP_TABLE_PARA default_field[i] is set only in the cases when 'field' can inherit the default value that is defined for the field referred by the Item_field object from which 'field' has been created. + + Note that we can't directly use default_field[i] for getting default + value in code below since some in some cases source Field objects + are bound to record[1] buffer instead of record[0] (e.g. in code + handling foreign keys). + + QQ: This is a bit hackish. Maybe it is better to have separate flag + for this in TMP_TABLE_PARAM? */ my_ptrdiff_t diff; - Field *orig_field= default_field[i]; + TABLE *orig_table= default_field[i]->table; + Field *orig_field= orig_table->field[default_field[i]->field_index]; /* Get the value from default_values */ - diff= (my_ptrdiff_t) (orig_field->table->s->default_values- - orig_field->table->record[0]); + diff= (my_ptrdiff_t) (orig_table->s->default_values - + orig_table->record[0]); orig_field->move_field_offset(diff); // Points now at default_values if (orig_field->is_real_null()) field->set_null(); === modified file 'sql/sql_update.cc' --- a/sql/sql_update.cc 2008-12-23 20:29:54 +0000 +++ b/sql/sql_update.cc 2008-12-26 17:19:38 +0000 @@ -219,8 +219,7 @@ update_record(THD *thd, TABLE_LIST *tabl tables are barred from participating in foreign keys requiring such actions. */ - if (fk_list->check_parent_list(table, FALSE) || - fk_list->check_child_list(table, FALSE)) + if (fk_list->check_parent_list() || fk_list->check_child_list()) return TRUE; } @@ -292,9 +291,8 @@ update_record(THD *thd, TABLE_LIST *tabl If we modify transactional table we perform checks and cascading actions on child table after updating the row. */ - if (fk_list->check_parent_list(table, FALSE) || - fk_list->check_child_list(table, FALSE) || - fk_list->cascading_action_child_list(table)) + if (fk_list->check_parent_list() || fk_list->check_child_list() || + fk_list->cascading_action_child_list()) return TRUE; } } @@ -828,6 +826,21 @@ int mysql_update(THD *thd, } } dup_key_found= 0; + + /* + QQ: Is it proper place for EOS check? I dislike breach of the symmetry... + OTOH it seems that we should do this before playing with thd->killed. + */ + if (error < 0) + { + /* + There is no sense in doing potentially expensive EOS check if statement + will result in error and thus will be rolled back anyway. + */ + if (fk_list.eos_checks()) + error= 1; + } + /* Caching the killed status to pass as the arg to query event constuctor; The cached value can not change whereas the killed status can