Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Added logic to associations.rb to make sure select_for_limited_ids

includes joins that are needed to reach tables listed in the :order
or :conditions options if they are not joined directly to the main
active_record table.

Signed-off-by: Michael Koziarski <michael@koziarski.com>
[#109 state:resolved]
  • Loading branch information...
commit 8ded457b1b31b157d6fe89b553749579e5ac4a27 1 parent fbebdb0
@jdevine jdevine authored NZKoz committed
View
25 activerecord/lib/active_record/associations.rb 100755 → 100644
@@ -1446,6 +1446,12 @@ def construct_finder_sql_for_association_limiting(options, join_dependency)
tables_from_conditions = conditions_tables(options)
tables_from_order = order_tables(options)
all_tables = tables_from_conditions + tables_from_order
+ distinct_join_associations = all_tables.uniq.map{|table|
+ join_dependency.joins_for_table_name(table)
+ }.flatten.compact.uniq
+
+
+
is_distinct = !options[:joins].blank? || include_eager_conditions?(options, tables_from_conditions) || include_eager_order?(options, tables_from_order)
sql = "SELECT "
@@ -1457,7 +1463,7 @@ def construct_finder_sql_for_association_limiting(options, join_dependency)
sql << " FROM #{connection.quote_table_name table_name} "
if is_distinct
- sql << join_dependency.join_associations.reject{ |ja| !all_tables.include?(ja.table_name) }.collect(&:association_join).join
+ sql << distinct_join_associations.collect(&:association_join).join
add_joins!(sql, options, scope)
end
@@ -1617,6 +1623,23 @@ def remove_duplicate_results!(base, records, associations)
end
end
+ def join_for_table_name(table_name)
+ @joins.select{|j|j.aliased_table_name == table_name.gsub(/^\"(.*)\"$/){$1} }.first rescue nil
+ end
+
+ def joins_for_table_name(table_name)
+ join = join_for_table_name(table_name)
+ result = nil
+ if join && join.is_a?(JoinAssociation)
+ result = [join]
+ if join.parent && join.parent.is_a?(JoinAssociation)
+ result = joins_for_table_name(join.parent.aliased_table_name) +
+ result
+ end
+ end
+ result
+ end
+
protected
def build(associations, parent = nil)
parent ||= @joins.last
View
9 activerecord/test/cases/finder_test.rb
@@ -8,6 +8,7 @@
require 'models/developer'
require 'models/post'
require 'models/customer'
+require 'models/job'
class FinderTest < ActiveRecord::TestCase
fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers
@@ -857,6 +858,14 @@ def test_select_rows
Company.connection.select_rows("SELECT id, name FROM companies WHERE id IN (1,2,3) ORDER BY id").map! {|i| i.map! {|j| j.to_s unless j.nil?}}
end
+ def test_find_with_order_on_included_associations_with_construct_finder_sql_for_association_limiting_and_is_distinct
+ assert_equal 2, Post.find(:all,:include=>{:authors=>:author_address},:order=>' author_addresses.id DESC ', :limit=>2).size
+
+ assert_equal 3, Post.find(:all,:include=>{:author=>:author_address,:authors=>:author_address},
+ :order=>' author_addresses_authors.id DESC ', :limit=>3).size
+ end
+
+
protected
def bind(statement, *vars)
if vars.first.is_a?(Hash)
Please sign in to comment.
Something went wrong with that request. Please try again.