Rails Lighthouse ticket #3165

exists-2-3-stable.patch

From ccc8aee0aeac0213cf15abd67ed5dab577899007 Mon Sep 17 00:00:00 2001
From: Rob Olson <[email protected]>
Date: Fri, 23 Oct 2009 19:20:48 -0700
Subject: [PATCH 1/2] Add option to skip record initialization for sql queries.

This fixes an exception caused by queries generated by #exists?
when there is an after_initialize which accesses an attribute
on the record.
---
 activerecord/lib/active_record/base.rb |   19 ++++++++++---------
 activerecord/test/cases/finder_test.rb |    4 ++++
 activerecord/test/models/entrant.rb    |    4 ++++
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 2ec2f73..13cd762 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -657,8 +657,8 @@ module ActiveRecord #:nodoc:
       #   # You can use the same string replacement techniques as you can with ActiveRecord#find
       #   Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date]
       #   > [#<Post:0x36bff9c @attributes={"first_name"=>"The Cheap Man Buys Twice"}>, ...]
-      def find_by_sql(sql)
-        connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) }
+      def find_by_sql(sql, skip_instantiation = false)
+        connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| skip_instantiation ? record : instantiate(record) }
       end
 
       # Returns true if a record exists in the table that matches the +id+ or
@@ -687,9 +687,10 @@ module ActiveRecord #:nodoc:
       #   Person.exists?(['name LIKE ?', "%#{query}%"])
       #   Person.exists?
       def exists?(id_or_conditions = {})
-        find_initial(
+        find_initial({
           :select => "#{quoted_table_name}.#{primary_key}",
-          :conditions => expand_id_conditions(id_or_conditions)) ? true : false
+          :conditions => expand_id_conditions(id_or_conditions)},
+          :skip_instantiation) ? true : false
       end
 
       # Creates an object (or multiple objects) and saves it to the database, if validations pass.
@@ -1500,9 +1501,9 @@ module ActiveRecord #:nodoc:
       end
 
       private
-        def find_initial(options)
+        def find_initial(options, skip_instantiation = false)
           options.update(:limit => 1)
-          find_every(options).first
+          find_every(options, skip_instantiation).first
         end
 
         def find_last(options)
@@ -1539,14 +1540,14 @@ module ActiveRecord #:nodoc:
           }.join(',')
         end
 
-        def find_every(options)
+        def find_every(options, skip_instantiation = false)
           include_associations = merge_includes(scope(:find, :include), options[:include])
 
           if include_associations.any? && references_eager_loaded_tables?(options)
             records = find_with_associations(options)
           else
-            records = find_by_sql(construct_finder_sql(options))
-            if include_associations.any?
+            records = find_by_sql(construct_finder_sql(options), skip_instantiation)
+            if include_associations.any? && !skip_instantiation
               preload_associations(records, include_associations)
             end
           end
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index f1bede9..6e9f759 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -125,6 +125,10 @@ class FinderTest < ActiveRecord::TestCase
     end
   end
 
+  def test_exists_on_model_with_after_initialize_method_should_not_blow_up
+    assert Entrant.exists?
+  end
+
   def test_find_by_array_of_one_id
     assert_kind_of(Array, Topic.find([ 1 ]))
     assert_equal(1, Topic.find([ 1 ]).length)
diff --git a/activerecord/test/models/entrant.rb b/activerecord/test/models/entrant.rb
index 4682ce4..dfec275 100644
--- a/activerecord/test/models/entrant.rb
+++ b/activerecord/test/models/entrant.rb
@@ -1,3 +1,7 @@
 class Entrant < ActiveRecord::Base
   belongs_to :course
+
+  after_initialize do
+    self.name ||= ""
+  end
 end
-- 
1.6.3.3


From cdc43f185161bda7abf2385957be9bb2cf1e4ea2 Mon Sep 17 00:00:00 2001
From: Rob Olson <[email protected]>
Date: Mon, 9 Nov 2009 19:31:53 -0800
Subject: [PATCH 2/2] updated test_exists_on_model_with_after_initialize_method_should_not_blow_up to fail properly

---
 activerecord/test/cases/finder_test.rb |    2 +-
 activerecord/test/models/entrant.rb    |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 6e9f759..fd43065 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -126,7 +126,7 @@ class FinderTest < ActiveRecord::TestCase
   end
 
   def test_exists_on_model_with_after_initialize_method_should_not_blow_up
-    assert Entrant.exists?
+    assert_nothing_raised { assert Entrant.exists? }
   end
 
   def test_find_by_array_of_one_id
diff --git a/activerecord/test/models/entrant.rb b/activerecord/test/models/entrant.rb
index dfec275..1d96617 100644
--- a/activerecord/test/models/entrant.rb
+++ b/activerecord/test/models/entrant.rb
@@ -1,7 +1,7 @@
 class Entrant < ActiveRecord::Base
   belongs_to :course
 
-  after_initialize do
+  def after_initialize
     self.name ||= ""
   end
 end
-- 
1.6.3.3

exists.patch

Index: activerecord/lib/active_record/base.rb
===================================================================
--- a/rails/activerecord/lib/active_record/base.rb
+++ b/rails/activerecord/lib/active_record/base.rb
@@ -689,5 +689,4 @@
       def exists?(id_or_conditions = {})
         find_initial(
-          :select => "#{quoted_table_name}.#{primary_key}",
           :conditions => expand_id_conditions(id_or_conditions)) ? true : false
       end

skip_initialization_on_exists.patch

From 81904d95af9f6c7922d54ab43ed4147433172e86 Mon Sep 17 00:00:00 2001
From: Rob Olson <[email protected]>
Date: Fri, 23 Oct 2009 19:20:48 -0700
Subject: [PATCH] Add option to skip record initialization for sql queries.

This fixes an exception caused by queries generated by #exists?
when there is an after_initialize which accesses an attribute
on the record.
---
 activerecord/lib/active_record/base.rb |   19 ++++++++++---------
 activerecord/test/cases/finder_test.rb |    4 ++++
 activerecord/test/models/entrant.rb    |    4 ++++
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4e60904..831f0bb 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -707,8 +707,8 @@ module ActiveRecord #:nodoc:
       #   # You can use the same string replacement techniques as you can with ActiveRecord#find
       #   Post.find_by_sql ["SELECT title FROM posts WHERE author = ? AND created > ?", author_id, start_date]
       #   > [#<Post:0x36bff9c @attributes={"first_name"=>"The Cheap Man Buys Twice"}>, ...]
-      def find_by_sql(sql)
-        connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| instantiate(record) }
+      def find_by_sql(sql, skip_instantiation = false)
+        connection.select_all(sanitize_sql(sql), "#{name} Load").collect! { |record| skip_instantiation ? record : instantiate(record) }
       end
 
       # Returns true if a record exists in the table that matches the +id+ or
@@ -737,9 +737,10 @@ module ActiveRecord #:nodoc:
       #   Person.exists?(['name LIKE ?', "%#{query}%"])
       #   Person.exists?
       def exists?(id_or_conditions = {})
-        find_initial(
+        find_initial({
           :select => "#{quoted_table_name}.#{primary_key}",
-          :conditions => expand_id_conditions(id_or_conditions)) ? true : false
+          :conditions => expand_id_conditions(id_or_conditions)},
+          :skip_instantiation) ? true : false
       end
 
       # Creates an object (or multiple objects) and saves it to the database, if validations pass.
@@ -1502,9 +1503,9 @@ module ActiveRecord #:nodoc:
       end
 
       private
-        def find_initial(options)
+        def find_initial(options, skip_instantiation = false)
           options.update(:limit => 1)
-          find_every(options).first
+          find_every(options, skip_instantiation).first
         end
 
         def find_last(options)
@@ -1541,14 +1542,14 @@ module ActiveRecord #:nodoc:
           }.join(',')
         end
 
-        def find_every(options)
+        def find_every(options, skip_instantiation = false)
           include_associations = merge_includes(scope(:find, :include), options[:include])
 
           if include_associations.any? && references_eager_loaded_tables?(options)
             records = find_with_associations(options)
           else
-            records = find_by_sql(construct_finder_sql(options))
-            if include_associations.any?
+            records = find_by_sql(construct_finder_sql(options), skip_instantiation)
+            if include_associations.any? && !skip_instantiation
               preload_associations(records, include_associations)
             end
           end
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 3de0779..3fdab03 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -125,6 +125,10 @@ class FinderTest < ActiveRecord::TestCase
     end
   end
 
+  def test_exists_on_model_with_after_initialize_method_should_not_blow_up
+    assert Entrant.exists?
+  end
+
   def test_find_by_array_of_one_id
     assert_kind_of(Array, Topic.find([ 1 ]))
     assert_equal(1, Topic.find([ 1 ]).length)
diff --git a/activerecord/test/models/entrant.rb b/activerecord/test/models/entrant.rb
index 4682ce4..dfec275 100644
--- a/activerecord/test/models/entrant.rb
+++ b/activerecord/test/models/entrant.rb
@@ -1,3 +1,7 @@
 class Entrant < ActiveRecord::Base
   belongs_to :course
+
+  after_initialize do
+    self.name ||= ""
+  end
 end
-- 
1.6.3.3