Added support for limit and offset with eager loading of has_one and belongs_to associations. Using the options with has_many and has_and_belongs_to_many associations will now raise an ActiveRecord::ConfigurationError #1692 [Rick Olsen]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1811 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

4 files changed, 76 insertions, 11 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index 2dca819dfb..94d29ef3b5 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,5 +1,7 @@
 *SVN*
 
+* Added support for limit and offset with eager loading of has_one and belongs_to associations. Using the options with has_many and has_and_belongs_to_many associations will now raise an ActiveRecord::ConfigurationError #1692 [Rick Olsen]
+
 * Fixed that assume_bottom_position (in acts_as_list) could be called on items already last in the list and they would move one position away from the list #1648 [tyler@kianta.com]
 
 * Added ActiveRecord::Base.threaded_connections flag to turn off 1-connection per thread (required for thread safety). By default it's on, but WEBrick in Rails need it off #1685 [Sam Stephenson]
diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 6f49800df6..ffb0d5679b 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -168,7 +168,8 @@ module ActiveRecord
     # catch-all for performance problems, but its a great way to cut down on the number of queries in a situation as the one described above.
     #
     # Please note that because eager loading is fetching both models and associations in the same grab, it doesn't make sense to use the
-    # :limit property and it will be ignored if attempted.
+    # :limit and :offset options on has_many and has_and_belongs_to_many associations and an ConfigurationError exception will be raised 
+    # if attempted. It does, however, work just fine with has_one and belongs_to associations.
     #
     # Also have in mind that since the eager loading is pulling from multiple tables, you'll have to disambiguate any column references
     # in both conditions and orders. So :order => "posts.id DESC" will work while :order => "id DESC" will not. This may require that
@@ -740,12 +741,9 @@ module ActiveRecord
 
         def find_with_associations(options = {})
           reflections  = reflect_on_included_associations(options[:include])
-          reflections.each do |r| 
-            raise(
-              NoMethodError, 
-              "Association was not found; perhaps you misspelled it?  You specified :include=>:#{options[:include].join(', :')}"
-            ) if r.nil? 
-          end
+
+          guard_against_missing_reflections(reflections, options)
+          guard_against_unlimitable_reflections(reflections, options)
           
           schema_abbreviations = generate_schema_abbreviations(reflections)
           primary_key_table    = generate_primary_key_table(reflections, schema_abbreviations)
@@ -788,6 +786,24 @@ module ActiveRecord
           [ associations ].flatten.collect { |association| reflect_on_association(association) }
         end
 
+        def guard_against_missing_reflections(reflections, options)
+          reflections.each do |r| 
+            raise(
+              ConfigurationError, 
+              "Association was not found; perhaps you misspelled it?  You specified :include => :#{options[:include].join(', :')}"
+            ) if r.nil? 
+          end
+        end
+        
+        def guard_against_unlimitable_reflections(reflections, options)
+          if (options[:offset] || options[:limit]) && !using_limitable_reflections?(reflections)
+            raise(
+              ConfigurationError, 
+              "You can not use offset and limit together with has_many or has_and_belongs_to_many associations"
+            )
+          end
+        end
+
         def generate_schema_abbreviations(reflections)
           schema = [ [ table_name, column_names ] ]
           schema += reflections.collect { |r| [ r.table_name, r.klass.column_names ] }
@@ -830,10 +846,14 @@ module ActiveRecord
           add_conditions!(sql, options[:conditions])
           add_sti_conditions!(sql, reflections)
           sql << "ORDER BY #{options[:order]} " if options[:order]
-          
+          add_limit!(sql, options) if using_limitable_reflections?(reflections)
           return sanitize_sql(sql)
         end
 
+        def using_limitable_reflections?(reflections)
+          reflections.reject { |r| [ :belongs_to, :has_one ].include?(r.macro) }.length.zero?
+        end
+
         def add_sti_conditions!(sql, reflections)
           sti_sql = ""
           reflections.each do |reflection|
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index b32843d069..be64c1bbd3 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -26,6 +26,8 @@ module ActiveRecord #:nodoc:
   end
   class StaleObjectError < ActiveRecordError #:nodoc:
   end
+  class ConfigurationError < StandardError #:nodoc:
+  end
 
   class AttributeAssignmentError < ActiveRecordError #:nodoc:
     attr_reader :exception, :attribute
@@ -336,7 +338,7 @@ module ActiveRecord #:nodoc:
 
         case args.first
           when :first
-            find(:all, options.merge({ :limit => 1 })).first
+            find(:all, options.merge(options[:include] ? { } : { :limit => 1 })).first
           when :all
             options[:include] ? find_with_associations(options) : find_by_sql(construct_finder_sql(options))
           else
diff --git a/activerecord/test/associations_go_eager_test.rb b/activerecord/test/associations_go_eager_test.rb
index 892a879283..cecc41ab50 100644
--- a/activerecord/test/associations_go_eager_test.rb
+++ b/activerecord/test/associations_go_eager_test.rb
@@ -48,10 +48,51 @@ class EagerAssociationTest < Test::Unit::TestCase
 
   def test_eager_association_loading_with_belongs_to
     comments = Comment.find(:all, :include => :post)
+    assert_equal 9, comments.length
     titles = comments.map { |c| c.post.title }
     assert titles.include?(posts(:welcome).title)
     assert titles.include?(posts(:sti_post_and_comments).title)
   end
+  
+  def test_eager_association_loading_with_belongs_to_and_limit
+    comments = Comment.find(:all, :include => :post, :limit => 5)
+    assert_equal 5, comments.length
+    assert_equal [1,2,3,5,6], comments.collect { |c| c.id }
+  end
+
+  def test_eager_association_loading_with_belongs_to_and_limit_and_conditions
+    comments = Comment.find(:all, :include => :post, :conditions => 'post_id = 4', :limit => 3)
+    assert_equal 3, comments.length
+    assert_equal [5,6,7], comments.collect { |c| c.id }
+  end
+
+  def test_eager_association_loading_with_belongs_to_and_limit_and_offset
+    comments = Comment.find(:all, :include => :post, :limit => 3, :offset => 2)
+    assert_equal 3, comments.length
+    assert_equal [3,5,6], comments.collect { |c| c.id }
+  end
+
+  def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_conditions
+    comments = Comment.find(:all, :include => :post, :conditions => 'post_id = 4', :limit => 3, :offset => 1)
+    assert_equal 3, comments.length
+    assert_equal [6,7,8], comments.collect { |c| c.id }
+  end
+  
+  def test_eager_association_loading_with_belongs_to_and_limit_and_multiple_associations
+    posts = Post.find(:all, :include => [:author, :very_special_comment], :limit => 1)
+    assert_equal 1, posts.length
+    assert_equal [4], posts.collect { |p| p.id }
+  end
+  
+  def test_eager_association_loading_with_belongs_to_and_limit_and_offset_and_multiple_associations
+    posts = Post.find(:all, :include => [:author, :very_special_comment], :limit => 1, :offset => 1)
+    assert_equal 0, posts.length
+    assert_equal [], posts
+  end
+  
+  def test_eager_association_raise_on_limit
+    assert_raises(ActiveRecord::ConfigurationError) { Post.find(:all, :include => [:author, :comments], :limit => 1) }
+  end
 
   def test_eager_association_loading_with_habtm
     posts = Post.find(:all, :include => :categories, :order => "posts.id")
@@ -95,10 +136,10 @@ class EagerAssociationTest < Test::Unit::TestCase
   end
 
   def test_eager_with_invalid_association_reference
-    assert_raises(NoMethodError, "Association was not found; perhaps you misspelled it?  You specified :include=>:monkeys") {
+    assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it?  You specified :include => :monkeys") {
       post = Post.find(6, :include=>[ :monkeys ])
     }
-    assert_raises(NoMethodError, "Association was not found; perhaps you misspelled it?  You specified :include=>:monkeys, :elephants") {
+    assert_raises(ActiveRecord::ConfigurationError, "Association was not found; perhaps you misspelled it?  You specified :include => :monkeys, :elephants") {
       post = Post.find(6, :include=>[ :monkeys, :elephants ])
     }
   end