Merge pull request #9388 from senny/9367_wrong_schema_after_remove_co…

…lumn

Sqlite preserves primary keys when copying/altering tables.

activerecord/CHANGELOG.md

@@ -1,5 +1,11 @@
 ## unreleased ##
 
+*   Sqlite now preserves custom primary keys when copying or altering tables.
+    Fixes #9367.
+    Backport #2312.
+
+    *Sean Scally + Yves Senn*
+
 *   Preloading `has_many :through` associations with conditions won't
     cache the `:through` association. This will prevent invalid
     subsets to be cached.

activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb

@@ -490,7 +490,11 @@ def move_table(from, to, options = {}, &block) #:nodoc:
         end
 
         def copy_table(from, to, options = {}) #:nodoc:
-          options = options.merge(:id => (!columns(from).detect{|c| c.name == 'id'}.nil? && 'id' == primary_key(from).to_s))
+          from_primary_key = primary_key(from)
+          options[:primary_key] = from_primary_key if from_primary_key != 'id'
+          unless options[:primary_key]
+            options[:id] = !columns(from).detect{|c| c.name == 'id'}.nil? && 'id' == from_primary_key
+          end
           create_table(to, options) do |definition|
             @definition = definition
             columns(from).each do |column|
@@ -504,7 +508,7 @@ def copy_table(from, to, options = {}) #:nodoc:
                 :precision => column.precision, :scale => column.scale,
                 :null => column.null)
             end
-            @definition.primary_key(primary_key(from)) if primary_key(from)
+            @definition.primary_key(from_primary_key) if from_primary_key
             yield @definition if block_given?
           end
 

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

@@ -57,6 +57,14 @@ def test_copy_table_with_id_col_that_is_not_primary_key
     end
   end
 
+  def test_copy_table_with_unconventional_primary_key
+    test_copy_table('owners', 'owners_unconventional') do |from, to, options|
+      original_pk = @connection.primary_key('owners')
+      copied_pk = @connection.primary_key('owners_unconventional')
+      assert_equal original_pk, copied_pk
+    end
+  end
+
 protected
   def copy_table(from, to, options = {})
     @connection.copy_table(from, to, {:temporary => true}.merge(options))

activerecord/test/cases/migration_test.rb

@@ -889,6 +889,20 @@ def test_remove_column_with_array_as_an_argument_is_deprecated
       ActiveRecord::Base.connection.drop_table(:hats) rescue nil
     end
 
+    def test_removing_and_renaming_column_preserves_custom_primary_key
+      ActiveRecord::Base.connection.create_table "my_table", :primary_key => "my_table_id", :force => true do |t|
+        t.integer "col_one"
+        t.string "col_two", :limit => 128, :null => false
+      end
+
+      ActiveRecord::Base.connection.remove_column("my_table", "col_two")
+      ActiveRecord::Base.connection.rename_column("my_table", "col_one", "col_three")
+
+      assert_equal 'my_table_id', ActiveRecord::Base.connection.primary_key('my_table')
+    ensure
+      ActiveRecord::Base.connection.drop_table(:my_table) rescue nil
+    end
+
     def test_change_type_of_not_null_column
       assert_nothing_raised do
         Topic.connection.change_column "topics", "written_on", :datetime, :null => false