aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorSantiago Pastorino <santiago@wyeworks.com>2010-11-04 20:48:02 -0200
committerAaron Patterson <aaron.patterson@gmail.com>2010-11-05 11:27:34 -0700
commit3146aa68fd03ea4392b45f1c8771675a9c850471 (patch)
tree6f229814d49e9c9da4bb4a276a5a291b093c50d8 /activerecord
parentd5e45931a0f3d36515d6048fd9cb13f641018571 (diff)
downloadrails-3146aa68fd03ea4392b45f1c8771675a9c850471.tar.gz
rails-3146aa68fd03ea4392b45f1c8771675a9c850471.tar.bz2
rails-3146aa68fd03ea4392b45f1c8771675a9c850471.zip
Fixes queries using limits and punctuation in order, removes order("col1, col2") usage in favor of order(["col1", "col2"})
[#4597 state:committed]
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb6
-rw-r--r--activerecord/lib/active_record/relation/finder_methods.rb2
-rw-r--r--activerecord/test/cases/associations/cascaded_eager_loading_test.rb2
-rw-r--r--activerecord/test/cases/associations/eager_test.rb4
-rw-r--r--activerecord/test/cases/relations_test.rb23
5 files changed, 29 insertions, 8 deletions
diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
index 426360b470..942ea9b297 100644
--- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
+++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb
@@ -932,12 +932,12 @@ module ActiveRecord
# requires that the ORDER BY include the distinct column.
#
# distinct("posts.id", "posts.created_at desc")
- def distinct(columns, order_by) #:nodoc:
- return "DISTINCT #{columns}" if order_by.blank?
+ def distinct(columns, orders) #:nodoc:
+ return "DISTINCT #{columns}" if orders.empty?
# Construct a clean list of column names from the ORDER BY clause, removing
# any ASC/DESC modifiers
- order_columns = order_by.split(',').collect { |s| s.split.first }
+ order_columns = orders.collect { |s| s.split.first }
order_columns.delete_if { |c| c.blank? }
order_columns = order_columns.zip((0...order_columns.size).to_a).map { |s,i| "#{s} AS alias_#{i}" }
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index fe1ef2e2e3..ab9f10b380 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -222,7 +222,7 @@ module ActiveRecord
end
def construct_limited_ids_condition(relation)
- orders = relation.order_values.join(", ")
+ orders = relation.order_values
values = @klass.connection.distinct("#{@klass.connection.quote_table_name @klass.table_name}.#{@klass.primary_key}", orders)
ids_array = relation.select(values).collect {|row| row[@klass.primary_key]}
diff --git a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
index 271bb92ee8..37c6f354a8 100644
--- a/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
+++ b/activerecord/test/cases/associations/cascaded_eager_loading_test.rb
@@ -137,7 +137,7 @@ class CascadedEagerLoadingTest < ActiveRecord::TestCase
end
def test_eager_association_loading_with_multiple_stis_and_order
- author = Author.find(:first, :include => { :posts => [ :special_comments , :very_special_comment ] }, :order => 'authors.name, comments.body, very_special_comments_posts.body', :conditions => 'posts.id = 4')
+ author = Author.find(:first, :include => { :posts => [ :special_comments , :very_special_comment ] }, :order => ['authors.name', 'comments.body', 'very_special_comments_posts.body'], :conditions => 'posts.id = 4')
assert_equal authors(:david), author
assert_no_queries do
author.posts.first.special_comments
diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb
index 2d8e02e398..b559c9ddad 100644
--- a/activerecord/test/cases/associations/eager_test.rb
+++ b/activerecord/test/cases/associations/eager_test.rb
@@ -584,8 +584,8 @@ class EagerAssociationTest < ActiveRecord::TestCase
end
def test_limited_eager_with_multiple_order_columns
- assert_equal posts(:thinking, :sti_comments), Post.find(:all, :include => [:author, :comments], :conditions => "authors.name = 'David'", :order => 'UPPER(posts.title), posts.id', :limit => 2, :offset => 1)
- assert_equal posts(:sti_post_and_comments, :sti_comments), Post.find(:all, :include => [:author, :comments], :conditions => "authors.name = 'David'", :order => 'UPPER(posts.title) DESC, posts.id', :limit => 2, :offset => 1)
+ assert_equal posts(:thinking, :sti_comments), Post.find(:all, :include => [:author, :comments], :conditions => "authors.name = 'David'", :order => ['UPPER(posts.title)', 'posts.id'], :limit => 2, :offset => 1)
+ assert_equal posts(:sti_post_and_comments, :sti_comments), Post.find(:all, :include => [:author, :comments], :conditions => "authors.name = 'David'", :order => ['UPPER(posts.title) DESC', 'posts.id'], :limit => 2, :offset => 1)
end
def test_limited_eager_with_numeric_in_association
diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb
index f4f3dc4d5a..fbc257195c 100644
--- a/activerecord/test/cases/relations_test.rb
+++ b/activerecord/test/cases/relations_test.rb
@@ -1,4 +1,5 @@
require "cases/helper"
+require 'models/tag'
require 'models/tagging'
require 'models/post'
require 'models/topic'
@@ -17,7 +18,7 @@ require 'models/tyre'
class RelationTest < ActiveRecord::TestCase
fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts, :comments,
- :taggings, :cars
+ :tags, :taggings, :cars
def test_bind_values
relation = Post.scoped
@@ -144,6 +145,26 @@ class RelationTest < ActiveRecord::TestCase
assert_equal entrants(:first).name, entrants.first.name
end
+ def test_finding_with_complex_order_and_limit
+ if current_adapter?(:SQLite3Adapter)
+ tags = Tag.includes(:taggings).order("MIN(1,2)").limit(1).to_a
+ else
+ tags = Tag.includes(:taggings).order("LEAST(1,COS(1)*COS(-1)*COS(RADIANS(taggings.super_tag_id)))").limit(1).to_a
+ end
+
+ assert_equal 1, tags.length
+ end
+
+ def test_finding_with_complex_order
+ if current_adapter?(:SQLite3Adapter)
+ tags = Tag.includes(:taggings).order("MIN(1,2)").to_a
+ else
+ tags = Tag.includes(:taggings).order("LEAST(1,COS(1)*COS(-1)*COS(RADIANS(taggings.super_tag_id)))").to_a
+ end
+
+ assert_equal 2, tags.length
+ end
+
def test_finding_with_order_limit_and_offset
entrants = Entrant.order("id ASC").limit(2).offset(1)