8 files changed, 65 insertions, 9 deletions
diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index af436232f2..6847755e6e 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Fix has_many :through delete with custom foreign keys.  #6466 [naffis]
+
 * Foxy fixtures, from rathole (http://svn.geeksomnia.com/rathole/trunk/README)
     - stable, autogenerated IDs
     - specify associations (belongs_to, has_one, has_many) by label, not ID
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index b64b339781..e542e8d85a 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -49,6 +49,12 @@ module ActiveRecord
     end
   end
 
+  class HasManyThroughCantDissociateNewRecords < ActiveRecordError #:nodoc:
+    def initialize(owner, reflection)
+      super("Cannot dissociate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to delete the has_many :through record associating them.")
+    end
+  end
+
   class EagerLoadPolymorphicError < ActiveRecordError #:nodoc:
     def initialize(reflection)
       super("Can not eagerly load the polymorphic association #{reflection.name.inspect}")
diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb
index 5bb306c060..5ff0e3d0d7 100644
--- a/activerecord/lib/active_record/associations/has_many_through_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_through_association.rb
@@ -71,18 +71,24 @@ module ActiveRecord
       def delete(*records)
         records = flatten_deeper(records)
         records.each { |associate| raise_on_type_mismatch(associate) }
-        records.reject! { |associate| @target.delete(associate) if associate.new_record? }
-        return if records.empty?
-        
-        @delete_join_finder ||= "find_all_by_#{@reflection.source_reflection.association_foreign_key}"
+
         through = @reflection.through_reflection
-        through.klass.transaction do
-          records.each do |associate|
-            joins = @owner.send(through.name).send(@delete_join_finder, associate.id)
-            @owner.send(through.name).delete(joins)
+        raise ActiveRecord::HasManyThroughCantDissociateNewRecords.new(@owner, through) if @owner.new_record?
+
+        load_target
+
+        klass = through.klass
+        klass.transaction do
+          flatten_deeper(records).each do |associate|
+            raise_on_type_mismatch(associate)
+            raise ActiveRecord::HasManyThroughCantDissociateNewRecords.new(@owner, through) unless associate.respond_to?(:new_record?) && !associate.new_record?
+
+            @owner.send(through.name).proxy_target.delete(klass.delete_all(construct_join_attributes(associate)))
             @target.delete(associate)
           end
         end
+
+        self
       end
 
       def build(attrs = nil)
diff --git a/activerecord/test/associations/join_model_test.rb b/activerecord/test/associations/join_model_test.rb
index 6b67b15664..0e146f74c4 100644
--- a/activerecord/test/associations/join_model_test.rb
+++ b/activerecord/test/associations/join_model_test.rb
@@ -9,10 +9,12 @@ require 'fixtures/category'
 require 'fixtures/categorization'
 require 'fixtures/vertex'
 require 'fixtures/edge'
+require 'fixtures/book'
+require 'fixtures/citation'
 
 class AssociationsJoinModelTest < Test::Unit::TestCase
   self.use_transactional_fixtures = false
-  fixtures :posts, :authors, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items
+  fixtures :posts, :authors, :categories, :categorizations, :comments, :tags, :taggings, :author_favorites, :vertices, :items, :books
 
   def test_has_many
     assert authors(:david).categories.include?(categories(:general))
@@ -476,6 +478,20 @@ class AssociationsJoinModelTest < Test::Unit::TestCase
     assert_equal tags, posts(:thinking).tags.push(tags(:general))
   end
 
+  def test_delete_associate_when_deleting_from_has_many_through_with_nonstandard_id
+    count = books(:awdr).references.count
+    references_before = books(:awdr).references
+    book = Book.create!(:name => 'Getting Real')
+    book_awdr = books(:awdr)
+    book_awdr.references << book
+    assert_equal(count + 1, book_awdr.references(true).size)
+
+    assert_nothing_raised { book_awdr.references.delete(book) }
+    assert_equal(count, book_awdr.references.size)
+    assert_equal(count, book_awdr.references(true).size)
+    assert_equal(references_before.sort, book_awdr.references.sort)
+  end
+
   def test_delete_associate_when_deleting_from_has_many_through
     count = posts(:thinking).tags.count
     tags_before = posts(:thinking).tags
diff --git a/activerecord/test/fixtures/book.rb b/activerecord/test/fixtures/book.rb
new file mode 100644
index 0000000000..cfd07abddc
--- /dev/null
+++ b/activerecord/test/fixtures/book.rb
@@ -0,0 +1,4 @@
+class Book < ActiveRecord::Base
+  has_many :citations, :foreign_key => 'book1_id'
+  has_many :references, :through => :citations, :source => :reference_of, :uniq => true
+end
diff --git a/activerecord/test/fixtures/books.yml b/activerecord/test/fixtures/books.yml
new file mode 100644
index 0000000000..473663ff5b
--- /dev/null
+++ b/activerecord/test/fixtures/books.yml
@@ -0,0 +1,7 @@
+awdr:
+  id: 1
+  name: "Agile Web Development with Rails"
+
+rfr:
+  id: 2
+  name: "Ruby for Rails"
diff --git a/activerecord/test/fixtures/citation.rb b/activerecord/test/fixtures/citation.rb
new file mode 100644
index 0000000000..545aa8110d
--- /dev/null
+++ b/activerecord/test/fixtures/citation.rb
@@ -0,0 +1,6 @@
+class Citation < ActiveRecord::Base
+  belongs_to :reference_of, :class_name => "Book", :foreign_key => :book2_id
+
+  belongs_to :book1, :class_name => "Book", :foreign_key => :book1_id
+  belongs_to :book2, :class_name => "Book", :foreign_key => :book2_id
+end
diff --git a/activerecord/test/fixtures/db_definitions/schema.rb b/activerecord/test/fixtures/db_definitions/schema.rb
index 9cadc5bdb8..5bde5126d5 100644
--- a/activerecord/test/fixtures/db_definitions/schema.rb
+++ b/activerecord/test/fixtures/db_definitions/schema.rb
@@ -290,6 +290,15 @@ ActiveRecord::Schema.define do
     t.column :developer_id, :integer, :null=>false
   end
 
+  create_table :books, :force => true do |t|
+    t.column :name, :string
+  end
+
+  create_table :citations, :force => true do |t|
+    t.column :book1_id, :integer
+    t.column :book2_id, :integer
+  end
+
   create_table :inept_wizards, :force => true do |t|
     t.column :name, :string, :null => false
     t.column :city, :string, :null => false