Fix cases where delete_records on a has_many association caused errors

because of an ambiguous column name. This happened if the association
model had a default scope that referenced a third table, and the third
table also referenced the original table (with an identical
foreign_key).

Mysql requires that ambiguous columns are deambiguated by using the full
table.column syntax. Postgresql and Sqlite use a different syntax for
updates altogether (and don't tolerate table.name syntax), so the fix
requires always including the full table.column and discarding it later
for Sqlite and Postgresql.

7 files changed, 49 insertions, 5 deletions

diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
index 60a9eee7c7..c97138dfd7 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb
@@ -93,6 +93,18 @@ module ActiveRecord
         quote_column_name(table_name)
       end
 
+      # Override to return the quoted table name for assignment. Defaults to
+      # table quoting.
+      #
+      # This works for mysql and mysql2 where table.column can be used to
+      # resolve ambiguity.
+      #
+      # We override this in the sqlite and postgresql adaptors to use only
+      # the column name (as per syntax requirements).
+      def quote_table_name_for_assignment(table, attr)
+        quote_table_name("#{table}.#{attr}")
+      end
+
       def quoted_true
         "'t'"
       end
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
index 791b032023..47e2e3928f 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb
@@ -130,6 +130,10 @@ module ActiveRecord
           end
         end
 
+        def quote_table_name_for_assignment(table, attr)
+          quote_column_name(attr)
+        end
+
         # Quotes column names for use in SQL queries.
         def quote_column_name(name) #:nodoc:
           PGconn.quote_ident(name.to_s)
diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
index 11e8197293..7bead4bde9 100644
--- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb
@@ -229,6 +229,10 @@ module ActiveRecord
         @connection.class.quote(s)
       end
 
+      def quote_table_name_for_assignment(table, attr)
+        quote_column_name(attr)
+      end
+
       def quote_column_name(name) #:nodoc:
         %Q("#{name.to_s.gsub('"', '""')}")
       end
diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb
index 2dad1dc177..3c5b871e99 100644
--- a/activerecord/lib/active_record/sanitization.rb
+++ b/activerecord/lib/active_record/sanitization.rb
@@ -33,10 +33,10 @@ module ActiveRecord
       # Accepts an array, hash, or string of SQL conditions and sanitizes
       # them into a valid SQL fragment for a SET clause.
       #   { name: nil, group_id: 4 }  returns "name = NULL , group_id='4'"
-      def sanitize_sql_for_assignment(assignments)
+      def sanitize_sql_for_assignment(assignments, default_table_name = self.table_name)
         case assignments
         when Array; sanitize_sql_array(assignments)
-        when Hash;  sanitize_sql_hash_for_assignment(assignments)
+        when Hash;  sanitize_sql_hash_for_assignment(assignments, default_table_name)
         else        assignments
         end
       end
@@ -98,9 +98,9 @@ module ActiveRecord
       # Sanitizes a hash of attribute/value pairs into SQL conditions for a SET clause.
       #   { status: nil, group_id: 1 }
       #     # => "status = NULL , group_id = 1"
-      def sanitize_sql_hash_for_assignment(attrs)
+      def sanitize_sql_hash_for_assignment(attrs, table)
         attrs.map do |attr, value|
-          "#{connection.quote_column_name(attr)} = #{quote_bound_value(value)}"
+          "#{connection.quote_table_name_for_assignment(table, attr)} = #{quote_bound_value(value)}"
         end.join(', ')
       end
 
diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb
index d42630e1b7..fd6d531645 100644
--- a/activerecord/test/cases/associations/has_many_associations_test.rb
+++ b/activerecord/test/cases/associations/has_many_associations_test.rb
@@ -19,6 +19,7 @@ require 'models/line_item'
 require 'models/car'
 require 'models/bulb'
 require 'models/engine'
+require 'models/categorization'
 
 class HasManyAssociationsTestForCountWithFinderSql < ActiveRecord::TestCase
   class Invoice < ActiveRecord::Base
@@ -108,7 +109,8 @@ end
 class HasManyAssociationsTest < ActiveRecord::TestCase
   fixtures :accounts, :categories, :companies, :developers, :projects,
            :developers_projects, :topics, :authors, :comments,
-           :people, :posts, :readers, :taggings, :cars, :essays
+           :people, :posts, :readers, :taggings, :cars, :essays,
+           :categorizations
 
   def setup
     Client.destroyed_client_ids.clear
@@ -1729,4 +1731,20 @@ class HasManyAssociationsTest < ActiveRecord::TestCase
     assert_equal "lifo",  post.comments_with_extend_2.author
     assert_equal "hello", post.comments_with_extend_2.greeting
   end
+
+  test "delete record with complex joins" do
+    david = authors(:david)
+
+    post = david.posts.first
+    post.type = 'PostWithSpecialCategorization'
+    post.save
+
+    categorization = post.categorizations.first
+    categorization.special = true
+    categorization.save
+
+    assert_not_equal [], david.posts_with_special_categorizations
+    david.posts_with_special_categorizations = []
+    assert_equal [], david.posts_with_special_categorizations
+  end
 end
diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb
index 83904cd850..8423411474 100644
--- a/activerecord/test/models/author.rb
+++ b/activerecord/test/models/author.rb
@@ -9,6 +9,7 @@ class Author < ActiveRecord::Base
   has_many :posts_with_categories, -> { includes(:categories) }, :class_name => "Post"
   has_many :posts_with_comments_and_categories, -> { includes(:comments, :categories).order("posts.id") }, :class_name => "Post"
   has_many :posts_containing_the_letter_a, :class_name => "Post"
+  has_many :posts_with_special_categorizations, :class_name => 'PostWithSpecialCategorization'
   has_many :posts_with_extension, :class_name => "Post"
   has_one  :post_about_thinking, -> { where("posts.title like '%thinking%'") }, :class_name => 'Post'
   has_one  :post_about_thinking_with_last_comment, -> { where("posts.title like '%thinking%'").includes(:last_comment) }, :class_name => 'Post'
diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb
index 73ffc0de38..93a7a2073c 100644
--- a/activerecord/test/models/post.rb
+++ b/activerecord/test/models/post.rb
@@ -178,6 +178,11 @@ class PostWithDefaultInclude < ActiveRecord::Base
   has_many :comments, :foreign_key => :post_id
 end
 
+class PostWithSpecialCategorization < Post
+  has_many :categorizations, :foreign_key => :post_id
+  default_scope { where(:type => 'PostWithSpecialCategorization').joins(:categorizations).where(:categorizations => { :special => true }) }
+end
+
 class PostWithDefaultScope < ActiveRecord::Base
   self.table_name = 'posts'
   default_scope { order(:title) }