List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:March 27 2009 7:20pm
Subject:bzr commit into mysql-6.1-fk branch (dlenev:2713) Bug#43520
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-6.1-bg43520/ based on revid:dlenev@stripped

 2713 Dmitry Lenev	2009-03-27
      Fix for bug #43520 "Foreign keys: ignoring triggered action which
      precedes constraint check".
      
      Because we were reporting foreign key violation before executing
      AFTER trigger for a row (and even before executing BEFORE triggers
      for other rows) users were able to see that we perform foreign key
      validation in row-by-row fashion and not at the end of statement
      as prescribed by spec.
      
      This fix solves the problem by enabling usage of EOS buffer for
      foreign  keys checks caused by operations which also fire triggers
      (or when triggers are fired by operations which caused this
      operation).
     @ mysql-test/r/foreign_key_all_engines.result
        Added test for bug #43520 "Foreign keys: ignoring triggered action which
        precedes constraint check".
     @ mysql-test/t/foreign_key_all_engines.test
        Added test for bug #43520 "Foreign keys: ignoring triggered action which
        precedes constraint check".
     @ sql/fk.cc
        To preserve illusion that foreign key validation happens at the end of
        statement, after execution of triggers we have to turn on EOS checking
        if there are some triggers for the operation which caused this foreign
        key check (or one of operations which caused this operation).
     @ sql/fk.h
        Added Fk_constraint_list member to store pointer to TABLE instance on
        which operation which triggered foreign keys is performed.

    modified:
      mysql-test/r/foreign_key_all_engines.result
      mysql-test/t/foreign_key_all_engines.test
      sql/fk.cc
      sql/fk.h
=== modified file 'mysql-test/r/foreign_key_all_engines.result'
--- a/mysql-test/r/foreign_key_all_engines.result	2009-03-25 19:33:45 +0000
+++ b/mysql-test/r/foreign_key_all_engines.result	2009-03-27 19:19:55 +0000
@@ -2116,3 +2116,142 @@ on update restrict on delete restrict) e
 select * from t1;
 s1	s2
 drop tables t1, t2;
+#
+# Test for bug #43520 "Foreign keys: ignoring triggered action which
+#                      precedes constraint check".
+#
+# It should occur to the user that EOS validation of foreign keys
+# really happens after execution of triggers. Therefore EOS checks
+# should be enabled if there are triggers which are bracketing
+# row-by-row checks. Otherwise users will be able to detect that
+# in reality we do foreign key checking in row-by-row fashion,
+# i.e. they could get foreign key violation error before triggers
+# get executed and, for example, emit some other errors or change
+# some user variables.
+drop table if exists t1, t2, t3, t4;
+# Let us begin with cases when without EOS support a wrong error
+# will be emitted.
+create table t1 (pk int primary key);
+create table t2 (fk int references t1 (pk));
+create table t3 (pk int primary key);
+insert into t1 values (1), (2);
+insert into t2 values (1), (2);
+insert into t3 values (1);
+# First coverage for AFTER triggers.
+create trigger t2_ai after insert on t2 for each row
+insert into t3 values (1);
+create trigger t2_au after update on t2 for each row
+insert into t3 values (1);
+create trigger t1_au after update on t1 for each row
+insert into t3 values (1);
+create trigger t1_ad after delete on t1 for each row
+insert into t3 values (1);
+insert into t2 values (3);
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+update t2 set fk= 3 where fk = 1;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+update t1 set pk= 3 where pk = 1;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+delete from t1 where pk = 1;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+drop trigger t2_ai;
+drop trigger t2_au;
+drop trigger t1_au;
+drop trigger t1_ad;
+# Coverage for BEFORE triggers is a bit more tricky.
+create trigger t2_bi before insert on t2 for each row
+begin
+if new.fk > 3 then
+insert into t3 values (1);
+end if;
+end |
+create trigger t2_bu before update on t2 for each row
+begin
+if old.fk > 1 then
+insert into t3 values (1);
+end if;
+end |
+create trigger t1_bu before update on t1 for each row
+begin
+if old.pk > 1 then
+insert into t3 values (1);
+end if;
+end |
+create trigger t1_bd before delete on t1 for each row
+begin
+if old.pk > 1 then
+insert into t3 values (1);
+end if;
+end |
+insert into t2 values (3), (4);
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+update t2 set fk= 3;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+update t1 set pk= pk + 2;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+delete from t1;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+drop trigger t2_bi;
+drop trigger t2_bu;
+drop trigger t1_bu;
+drop trigger t1_bd;
+# Situations in which EOS check will succeed once we will allow
+# to modify tables used for foreign key checks in triggers.
+# Until then an appropriate error should be emitted.
+create trigger t2_ai after insert on t2 for each row
+begin
+if not (select count(*) from t1 where pk = new.fk) then
+insert into t1 values (new.fk);
+end if;
+end |
+create trigger t2_au after update on t2 for each row
+begin
+if not (select count(*) from t1 where pk = new.fk) then
+insert into t1 values (new.fk);
+end if;
+end |
+create trigger t1_au after update on t1 for each row
+begin
+delete from t2 where fk= old.pk;
+end |
+create trigger t1_ad after delete on t1 for each row
+begin
+delete from t2 where fk= old.pk;
+end |
+insert into t2 values (3);
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+update t2 set fk= 3;
+ERROR HY000: Can't update table 't1' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+update t1 set pk= 3 where pk = 1;
+ERROR HY000: Can't update table 't2' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+delete from t1 where pk= 1;
+ERROR HY000: Can't update table 't2' in stored function/trigger because it is already used by statement which invoked this stored function/trigger.
+drop tables t1, t2;
+# Finally, let us also check that in presence of triggers
+# EOS validation is applied for checks which are triggered
+# by cascading actions.
+create table t1 (pk int primary key);
+create table t2 (fk int default 5 references t1 (pk) on delete set default);
+insert into t1 values (1);
+insert into t2 values (1);
+create trigger t1_ad after delete on t1 for each row
+insert into t3 values (1);
+delete from t1;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+drop tables t1, t2;
+create table t1 (pk int primary key);
+create table t2 (fkpk int primary key
+references t1 (pk) on delete cascade on update cascade);
+create table t4 (fk int references t2 (fkpk));
+insert into t1 values (1);
+insert into t2 values (1);
+insert into t4 values (1);
+create trigger t1_au after update on t1 for each row
+insert into t3 values (1);
+create trigger t1_ad after delete on t1 for each row
+insert into t3 values (1);
+update t1 set pk= 2;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+delete from t1 where pk = 1;
+ERROR 23000: Duplicate entry '1' for key 'PRIMARY'
+drop tables t1, t2, t3, t4;

=== modified file 'mysql-test/t/foreign_key_all_engines.test'
--- a/mysql-test/t/foreign_key_all_engines.test	2009-03-25 19:33:45 +0000
+++ b/mysql-test/t/foreign_key_all_engines.test	2009-03-27 19:19:55 +0000
@@ -1995,3 +1995,150 @@ create table t2 (s1 int references t1 (s
                         on update restrict on delete restrict) engine=myisam;
 select * from t1;
 drop tables t1, t2;
+
+
+--echo #
+--echo # Test for bug #43520 "Foreign keys: ignoring triggered action which
+--echo #                      precedes constraint check".
+--echo #
+--echo # It should occur to the user that EOS validation of foreign keys
+--echo # really happens after execution of triggers. Therefore EOS checks
+--echo # should be enabled if there are triggers which are bracketing
+--echo # row-by-row checks. Otherwise users will be able to detect that
+--echo # in reality we do foreign key checking in row-by-row fashion,
+--echo # i.e. they could get foreign key violation error before triggers
+--echo # get executed and, for example, emit some other errors or change
+--echo # some user variables.
+--disable_warnings
+drop table if exists t1, t2, t3, t4;
+--enable_warnings
+--echo # Let us begin with cases when without EOS support a wrong error
+--echo # will be emitted.
+create table t1 (pk int primary key);
+create table t2 (fk int references t1 (pk));
+create table t3 (pk int primary key);
+insert into t1 values (1), (2);
+insert into t2 values (1), (2);
+insert into t3 values (1);
+--echo # First coverage for AFTER triggers.
+create trigger t2_ai after insert on t2 for each row
+  insert into t3 values (1);
+create trigger t2_au after update on t2 for each row
+  insert into t3 values (1);
+create trigger t1_au after update on t1 for each row
+  insert into t3 values (1);
+create trigger t1_ad after delete on t1 for each row
+  insert into t3 values (1);
+--error ER_DUP_ENTRY
+insert into t2 values (3);
+--error ER_DUP_ENTRY
+update t2 set fk= 3 where fk = 1;
+--error ER_DUP_ENTRY
+update t1 set pk= 3 where pk = 1;
+--error ER_DUP_ENTRY
+delete from t1 where pk = 1;
+drop trigger t2_ai;
+drop trigger t2_au;
+drop trigger t1_au;
+drop trigger t1_ad;
+--echo # Coverage for BEFORE triggers is a bit more tricky.
+delimiter |;
+create trigger t2_bi before insert on t2 for each row
+begin
+  if new.fk > 3 then
+    insert into t3 values (1);
+  end if;
+end |
+create trigger t2_bu before update on t2 for each row
+begin
+  if old.fk > 1 then
+    insert into t3 values (1);
+  end if;
+end |
+create trigger t1_bu before update on t1 for each row
+begin
+  if old.pk > 1 then
+    insert into t3 values (1);
+  end if;
+end |
+create trigger t1_bd before delete on t1 for each row
+begin
+  if old.pk > 1 then
+    insert into t3 values (1);
+  end if;
+end |
+delimiter ;|
+--error ER_DUP_ENTRY
+insert into t2 values (3), (4);
+--error ER_DUP_ENTRY
+update t2 set fk= 3;
+--error ER_DUP_ENTRY
+update t1 set pk= pk + 2;
+--error ER_DUP_ENTRY
+delete from t1;
+drop trigger t2_bi;
+drop trigger t2_bu;
+drop trigger t1_bu;
+drop trigger t1_bd;
+--echo # Situations in which EOS check will succeed once we will allow
+--echo # to modify tables used for foreign key checks in triggers.
+--echo # Until then an appropriate error should be emitted.
+delimiter |;
+create trigger t2_ai after insert on t2 for each row
+begin
+  if not (select count(*) from t1 where pk = new.fk) then
+    insert into t1 values (new.fk);
+  end if;
+end |
+create trigger t2_au after update on t2 for each row
+begin
+  if not (select count(*) from t1 where pk = new.fk) then
+    insert into t1 values (new.fk);
+  end if;
+end |
+create trigger t1_au after update on t1 for each row
+begin
+  delete from t2 where fk= old.pk;
+end |
+create trigger t1_ad after delete on t1 for each row
+begin
+  delete from t2 where fk= old.pk;
+end |
+delimiter ;|
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+insert into t2 values (3);
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+update t2 set fk= 3;
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+update t1 set pk= 3 where pk = 1;
+--error ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG
+delete from t1 where pk= 1;
+drop tables t1, t2;
+--echo # Finally, let us also check that in presence of triggers
+--echo # EOS validation is applied for checks which are triggered
+--echo # by cascading actions.
+create table t1 (pk int primary key);
+create table t2 (fk int default 5 references t1 (pk) on delete set default);
+insert into t1 values (1);
+insert into t2 values (1);
+create trigger t1_ad after delete on t1 for each row
+  insert into t3 values (1);
+--error ER_DUP_ENTRY
+delete from t1;
+drop tables t1, t2;
+create table t1 (pk int primary key);
+create table t2 (fkpk int primary key
+                 references t1 (pk) on delete cascade on update cascade);
+create table t4 (fk int references t2 (fkpk));
+insert into t1 values (1);
+insert into t2 values (1);
+insert into t4 values (1);
+create trigger t1_au after update on t1 for each row
+  insert into t3 values (1);
+create trigger t1_ad after delete on t1 for each row
+  insert into t3 values (1);
+--error ER_DUP_ENTRY
+update t1 set pk= 2;
+--error ER_DUP_ENTRY
+delete from t1 where pk = 1;
+drop tables t1, t2, t3, t4;

=== modified file 'sql/fk.cc'
--- a/sql/fk.cc	2009-03-26 06:08:50 +0000
+++ b/sql/fk.cc	2009-03-27 19:19:55 +0000
@@ -19,6 +19,7 @@
 #include "sql_select.h"
 #include "sp.h"
 #include "my_bitmap.h"
+#include "sql_trigger.h"
 
 
 class Foreign_key_eos_buffer;
@@ -491,6 +492,9 @@ Fk_constraint_list::prepare_check_parent
   if (thd->options & OPTION_NO_FOREIGN_KEY_CHECKS)
     return FALSE;
 
+  DBUG_ASSERT(m_table == 0 || m_table == table);
+  m_table= table;
+
   /**
     Iterate over all foreign keys defined on the table.
     Create runtime context for checks and actions on modified columns.
@@ -546,6 +550,9 @@ Fk_constraint_list::prepare_check_child(
   if (thd->options & OPTION_NO_FOREIGN_KEY_CHECKS)
     return FALSE;
 
+  DBUG_ASSERT(m_table == 0 || m_table == table);
+  m_table= table;
+
   /*
     Create contexts only for foreign keys that are involved in the operation.
   */
@@ -753,6 +760,21 @@ void Fk_constraint_list::do_invalidate_q
 
 
 /**
+  Check if triggers should be fired for this operation or one of operations which
+  caused this operations (when this operation is a cascading change).
+*/
+
+bool
+Fk_constraint_list::has_triggers()
+{
+  return ((m_table->triggers &&
+           (m_table->triggers->has_triggers(m_action_type, TRG_ACTION_BEFORE) ||
+            m_table->triggers->has_triggers(m_action_type, TRG_ACTION_AFTER))) ||
+          (m_parent_fk_list && m_parent_fk_list->has_triggers()));
+}
+
+
+/**
    Find a key (index) in a table by name.
 
    @param table  TABLE which index should be found.
@@ -922,15 +944,14 @@ prepare_key_copy(THD *thd, KEY *key,
      be added by the main statement: e.g. as a result of the
      update or of some other, perhaps indirect, cascading action
      of the main statement.
-  6) There are AFTER triggers. Currently triggers can not
-     change tables that are used in foreign key checks.
-     On such an attempt, we produce
-     ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG.
-     But unless we turn on EOS checks when there are AFTER
-     triggers, it would look to the user that FK checks are done
-     before AFTER triggers are fired, whereas our spec prescribes
-     otherwise (in other words, we'll produce a wrong error
-     message, see Bug#43520).
+  6) There are BEFORE or AFTER triggers. Currently triggers
+     can not change tables that are used in foreign key checks
+     (we produce ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG on such
+     an attempt) so they can't change outcome of EOS foreign key
+     validation. But unless we turn on EOS checks triggers, it
+     would look to the user that FK checks are done before triggers
+     are fired, whereas our spec prescribes otherwise (for example,
+     we'll produce a wrong error message, see Bug#43520).
 
   The code below is not trying to find out exactly what
   case we're dealing with this time:
@@ -1068,6 +1089,27 @@ can_eos_check_help(bool *can_eos_check_i
     an rcontext.
   */
   DBUG_ASSERT(m_fk_list->action_type() != TRG_EVENT_DELETE);
+
+  if (m_fk_list->has_triggers())
+  {
+    /*
+      5) There are some triggers which should be fired for operation
+      which caused this check or one of actions which triggered this
+      operation.
+      To preserve illusion that foreign key validation happens at the
+      end of statement after execution of triggers we should turn on
+      EOS checking.
+
+      Example (ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG should be
+      produced by INSERT into t2. In future it might even succeed):
+      CREATE TABLE t1 (pk INT PRIMARY KEY);
+      CREATE TABLE t2 (fk INT REFERENCES t1 (pk));
+      CREATE TRIGGER t2_ai AFTER INSERT ON t2 FOR EACH ROW
+        INSERT IGNORE INTO t1 VALUES (NEW.fk);
+      INSERT INTO t2 VALUES (1);
+    */
+    *can_eos_check_in_parent_help= TRUE;
+  }
 }
 
 
@@ -1948,6 +1990,29 @@ can_eos_check_help(bool *can_eos_check_i
       *can_eos_check_in_child_help= TRUE;
     *can_eos_check_in_parent_help= TRUE;
   }
+
+  if (m_fk_list->has_triggers())
+  {
+    /*
+      6) There are some triggers which should be fired for operation
+      which caused this check or one of actions which triggered this
+      operation.
+      To preserve illusion that foreign key validation happens at the
+      end of statement after execution of triggers we should turn on
+      EOS checking.
+
+      Example (ER_CANT_UPDATE_USED_TABLE_IN_SF_OR_TRG should be
+      produced by DELETE from t1. In future it might even succeed):
+      CREATE TABLE t1 (pk INT PRIMARY KEY);
+      CREATE TABLE t2 (fk INT REFERENCES t1 (pk));
+      INSERT INTO t1 VALUES (1);
+      INSERT INTO t2 VALUES (1);
+      CREATE TRIGGER t1_ad AFTER DELETE ON t1 FOR EACH ROW
+        DELETE FROM t2 WHERE fk= OLD.pk;
+      DELETE FROM t1;
+    */
+    *can_eos_check_in_child_help= TRUE;
+  }
 }
 
 

=== modified file 'sql/fk.h'
--- a/sql/fk.h	2009-03-25 19:33:45 +0000
+++ b/sql/fk.h	2009-03-27 19:19:55 +0000
@@ -38,6 +38,7 @@ public:
   Fk_constraint_list(trg_event_type action_type_arg,
                      Fk_constraint_list *parent_fk_list)
     : m_action_type(action_type_arg),
+      m_table(NULL),
       m_parent_fk_list(parent_fk_list)
   { }
 
@@ -104,6 +105,8 @@ public:
       do_invalidate_query_cache();
   }
 
+  bool has_triggers();
+
 private:
   /** Implementation. */
 
@@ -124,6 +127,10 @@ private:
   */
   trg_event_type                    m_action_type;
   /**
+    TABLE object on which operation is performed.
+  */
+  TABLE *m_table;
+  /**
     List of foreign key constraints in which the subject table
     participates as a child.
   */


Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20090327191955-t1vfuwfb0e70do0r.bundle
Thread
bzr commit into mysql-6.1-fk branch (dlenev:2713) Bug#43520Dmitry Lenev27 Mar