Merge pull request #55907 from ruyrocha/fix/sqlite3-data-loss

[#55866] Fix SQLite3 data loss during table alterations with CASCADE foreign keys.

Author: byroot

activerecord/CHANGELOG.md

@@ -1,3 +1,28 @@
+*   Fix SQLite3 data loss during table alterations with CASCADE foreign keys.
+
+    When altering a table in SQLite3 that is referenced by child tables with
+    `ON DELETE CASCADE` foreign keys, ActiveRecord would silently delete all
+    data from the child tables. This occurred because SQLite requires table
+    recreation for schema changes, and during this process the original table
+    is temporarily dropped, triggering CASCADE deletes on child tables.
+
+    The root cause was incorrect ordering of operations. The original code
+    wrapped `disable_referential_integrity` inside a transaction, but
+    `PRAGMA foreign_keys` cannot be modified inside a transaction in SQLite -
+    attempting to do so simply has no effect. This meant foreign keys remained
+    enabled during table recreation, causing CASCADE deletes to fire.
+
+    The fix reverses the order to follow the official SQLite 12-step ALTER TABLE
+    procedure: `disable_referential_integrity` now wraps the transaction instead
+    of being wrapped by it. This ensures foreign keys are properly disabled
+    before the transaction starts and re-enabled after it commits, preventing
+    CASCADE deletes while maintaining data integrity through atomic transactions.
+
+    *Ruy Rocha*
+
+*   Fix negative scopes for enums to include records with `nil` values.
+
+
 ## Rails 8.1.0.rc1 (October 15, 2025) ##
 
 *   Add replicas to test database parallelization setup.

activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb

@@ -613,8 +613,8 @@ def alter_table(
             yield definition if block_given?
           end
 
-          transaction do
-            disable_referential_integrity do
+          disable_referential_integrity do
+            transaction do
               move_table(table_name, altered_table_name, options.merge(temporary: true))
               move_table(altered_table_name, table_name, &caller)
             end

activerecord/test/cases/adapters/sqlite3/sqlite3_adapter_test.rb

@@ -1095,6 +1095,132 @@ def test_integer_cpk_column_returns_false_for_rowid
         end
       end
 
+      def test_alter_table_with_fk_preserves_rows_when_referenced_table_altered
+        conn = SQLite3Adapter.new(database: ":memory:", adapter: "sqlite3", strict: false)
+
+        conn.create_table :authors do |t|
+          t.string :name, null: false
+        end
+
+        conn.create_table :books do |t|
+          t.string  :title, null: false
+          t.integer :author_id, null: false
+        end
+        conn.add_foreign_key :books, :authors, on_delete: :cascade
+
+        conn.execute("INSERT INTO authors (id, name) VALUES (1, 'Douglas Adams');")
+        conn.execute("INSERT INTO books (id, title, author_id) VALUES (42, 'The Hitchhiker''s Guide', 1);")
+        conn.execute("INSERT INTO books (id, title, author_id) VALUES (43, 'Restaurant at the End', 1);")
+
+        initial_book_count = conn.select_value("SELECT COUNT(*) FROM books")
+        assert_equal 2, initial_book_count
+
+        conn.add_column :authors, :email, :string
+
+        book_count = conn.select_value("SELECT COUNT(*) FROM books")
+        author_count = conn.select_value("SELECT COUNT(*) FROM authors")
+
+        assert_equal 2, book_count, "Books were CASCADE deleted when authors table was altered!"
+        assert_equal 1, author_count, "Authors were lost during table alteration!"
+      ensure
+        conn.disconnect! if conn
+      end
+
+      def test_alter_table_with_fk_preserves_rows_when_adding_fk_to_referenced_table
+        conn = SQLite3Adapter.new(database: ":memory:", adapter: "sqlite3", strict: false)
+
+        conn.create_table :groups do |t|
+          t.string :name, null: false
+        end
+
+        conn.create_table :users do |t|
+          t.string :username, null: false
+        end
+
+        conn.create_table :reports do |t|
+          t.string  :title, null: false
+          t.integer :group_id, null: false
+        end
+        conn.add_foreign_key :reports, :groups, on_delete: :cascade
+
+        conn.execute("INSERT INTO groups (id, name) VALUES (1, 'Admin Group');")
+        conn.execute("INSERT INTO users (id, username) VALUES (1, 'alice');")
+        conn.execute("INSERT INTO reports (id, title, group_id) VALUES (1, 'Report A', 1);")
+        conn.execute("INSERT INTO reports (id, title, group_id) VALUES (2, 'Report B', 1);")
+
+        initial_report_count = conn.select_value("SELECT COUNT(*) FROM reports")
+        assert_equal 2, initial_report_count
+
+        conn.add_column :groups, :owner_id, :integer
+        conn.add_foreign_key :groups, :users, column: :owner_id
+
+        report_count = conn.select_value("SELECT COUNT(*) FROM reports")
+        group_count = conn.select_value("SELECT COUNT(*) FROM groups")
+
+        assert_equal 2, report_count, "Reports were CASCADE deleted when groups table was altered!"
+        assert_equal 1, group_count, "Groups were lost during table alteration!"
+      ensure
+        conn.disconnect! if conn
+      end
+
+      def test_alter_table_with_multiple_cascade_fks_preserves_all_data
+        conn = SQLite3Adapter.new(database: ":memory:", adapter: "sqlite3", strict: false)
+
+        conn.create_table :authors do |t|
+          t.string :name, null: false
+        end
+
+        conn.create_table :books do |t|
+          t.string  :title, null: false
+          t.integer :author_id, null: false
+        end
+        conn.add_foreign_key :books, :authors, on_delete: :cascade
+
+        conn.create_table :articles do |t|
+          t.string  :headline, null: false
+          t.integer :author_id, null: false
+        end
+        conn.add_foreign_key :articles, :authors, on_delete: :cascade
+
+        conn.execute("INSERT INTO authors (id, name) VALUES (1, 'Douglas Adams');")
+        conn.execute("INSERT INTO books (id, title, author_id) VALUES (1, 'HHGTTG', 1);")
+        conn.execute("INSERT INTO articles (id, headline, author_id) VALUES (1, 'Towel Day', 1);")
+
+        conn.add_column :authors, :bio, :text
+
+        book_count = conn.select_value("SELECT COUNT(*) FROM books")
+        article_count = conn.select_value("SELECT COUNT(*) FROM articles")
+
+        assert_equal 1, book_count, "Books were CASCADE deleted when authors table was altered!"
+        assert_equal 1, article_count, "Articles were CASCADE deleted when authors table was altered!"
+      ensure
+        conn.disconnect! if conn
+      end
+
+      def test_rename_table_with_cascade_fk_preserves_referencing_data
+        conn = SQLite3Adapter.new(database: ":memory:", adapter: "sqlite3", strict: false)
+
+        conn.create_table :authors do |t|
+          t.string :name, null: false
+        end
+
+        conn.create_table :books do |t|
+          t.string  :title, null: false
+          t.integer :author_id, null: false
+        end
+        conn.add_foreign_key :books, :authors, on_delete: :cascade
+
+        conn.execute("INSERT INTO authors (id, name) VALUES (1, 'Douglas Adams');")
+        conn.execute("INSERT INTO books (id, title, author_id) VALUES (1, 'HHGTTG', 1);")
+
+        conn.rename_table :authors, :writers
+
+        book_count = conn.select_value("SELECT COUNT(*) FROM books")
+        assert_equal 1, book_count, "Books were CASCADE deleted when authors table was renamed!"
+      ensure
+        conn.disconnect! if conn
+      end
+
       def test_sqlite_extensions_are_constantized_for_the_client_constructor
         mock_adapter = Class.new(SQLite3Adapter) do
           class << self