From 45881b0a64b4039e3b0d27e2c4d959dc2d744b9d Mon Sep 17 00:00:00 2001
From: Yasuo Honda <yasuo.honda@gmail.com>
Date: Mon, 7 May 2018 09:22:23 +0000
Subject: Disable foreign keys during `alter_table` for sqlite3 adapter

Unlike other databases, changing SQLite3 table definitions need to create a temporary table.
While changing table operations, the original table needs dropped which caused
`SQLite3::ConstraintException: FOREIGN KEY constraint failed` if the table is referenced by foreign keys.
This pull request disables foreign keys by `disable_referential_integrity`.

Also `disable_referential_integrity` method needs to execute `defer_foreign_keys = ON`
to defer re-enabling foreign keys until the transaction is committed.

https://www.sqlite.org/pragma.html#pragma_defer_foreign_keys

Fixes #31988

- This `defer_foreign_keys = ON` has been supported since SQLite 3.8.0
https://www.sqlite.org/releaselog/3_8_0.html and Rails 6 requires SQLite 3.8 #32923 now

- <Models>.reset_column_information added to address `ActiveModel::UnknownAttributeError`

```
Error:
ActiveRecord::Migration::ForeignKeyChangeColumnTest#test_change_column_of_parent_table:
ActiveModel::UnknownAttributeError: unknown attribute 'name' for ActiveRecord::Migration::ForeignKeyChangeColumnTest::Post.
```
---
 .../connection_adapters/sqlite3_adapter.rb         | 15 ++++---
 .../test/cases/migration/foreign_key_test.rb       | 46 ++++++++++++++++++++++
 2 files changed, 56 insertions(+), 5 deletions(-)

(limited to 'activerecord')

diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 32e2ed6fc6..544374586c 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -187,13 +187,16 @@ module ActiveRecord
       # REFERENTIAL INTEGRITY ====================================
 
       def disable_referential_integrity # :nodoc:
-        old = query_value("PRAGMA foreign_keys")
+        old_foreign_keys = query_value("PRAGMA foreign_keys")
+        old_defer_foreign_keys = query_value("PRAGMA defer_foreign_keys")
 
         begin
+          execute("PRAGMA defer_foreign_keys = ON")
           execute("PRAGMA foreign_keys = OFF")
           yield
         ensure
-          execute("PRAGMA foreign_keys = #{old}")
+          execute("PRAGMA defer_foreign_keys = #{old_defer_foreign_keys}")
+          execute("PRAGMA foreign_keys = #{old_foreign_keys}")
         end
       end
 
@@ -407,9 +410,11 @@ module ActiveRecord
           caller = lambda { |definition| yield definition if block_given? }
 
           transaction do
-            move_table(table_name, altered_table_name,
-              options.merge(temporary: true))
-            move_table(altered_table_name, table_name, &caller)
+            disable_referential_integrity do
+              move_table(table_name, altered_table_name,
+                options.merge(temporary: true))
+              move_table(altered_table_name, table_name, &caller)
+            end
           end
         end
 
diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb
index 50f5696ad1..c471dd1106 100644
--- a/activerecord/test/cases/migration/foreign_key_test.rb
+++ b/activerecord/test/cases/migration/foreign_key_test.rb
@@ -19,6 +19,52 @@ if ActiveRecord::Base.connection.supports_foreign_keys_in_create?
           assert_equal "fk_name", fk.name unless current_adapter?(:SQLite3Adapter)
         end
       end
+
+      class ForeignKeyChangeColumnTest < ActiveRecord::TestCase
+        self.use_transactional_tests = false
+
+        class Rocket < ActiveRecord::Base
+          has_many :astronauts
+        end
+
+        class Astronaut < ActiveRecord::Base
+          belongs_to :rocket
+        end
+
+        setup do
+          @connection = ActiveRecord::Base.connection
+          @connection.create_table "rockets", force: true do |t|
+            t.string :name
+          end
+
+          @connection.create_table "astronauts", force: true do |t|
+            t.string :name
+            t.references :rocket, foreign_key: true
+          end
+          Rocket.reset_column_information
+          Astronaut.reset_column_information
+        end
+
+        teardown do
+          @connection.drop_table "astronauts", if_exists: true
+          @connection.drop_table "rockets", if_exists: true
+          Rocket.reset_column_information
+          Astronaut.reset_column_information
+        end
+
+        def test_change_column_of_parent_table
+          foreign_keys = ActiveRecord::Base.connection.foreign_keys("astronauts")
+          rocket = Rocket.create!(name: "myrocket")
+          rocket.astronauts << Astronaut.create!
+
+          @connection.change_column_null :rockets, :name, false
+
+          fk = foreign_keys.first
+          assert_equal "myrocket", Rocket.first.name
+          assert_equal "astronauts", fk.from_table
+          assert_equal "rockets", fk.to_table
+        end
+      end
     end
   end
 end
-- 
cgit v1.2.3