diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 6 | ||||
| -rw-r--r-- | activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb | 5 | ||||
| -rw-r--r-- | activerecord/lib/active_record/querying.rb | 2 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/finder_methods.rb | 56 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 32 |
5 files changed, 92 insertions, 9 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 6f0b5bfce6..a661a44f1f 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 4.0.0 (unreleased) ## +* Added default order to `first` to assure consistent results among + diferent database engines. Introduced `take` as a replacement to + the old behavior of `first`. + + *Marcelo Silveira* + * Added an :index option to automatically create indexes for references and belongs_to statements in migrations. diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 273c165084..68cf495025 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -1233,7 +1233,10 @@ module ActiveRecord # Construct a clean list of column names from the ORDER BY clause, removing # any ASC/DESC modifiers - order_columns = orders.collect { |s| s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '') } + order_columns = orders.collect do |s| + s = s.to_sql unless s.is_a?(String) + s.gsub(/\s+(ASC|DESC)\s*(NULLS\s+(FIRST|LAST)\s*)?/i, '') + end 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/querying.rb b/activerecord/lib/active_record/querying.rb index 29b8b2fb73..4d8283bcff 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -3,7 +3,7 @@ require 'active_support/deprecation' module ActiveRecord module Querying - delegate :find, :first, :first!, :last, :last!, :all, :exists?, :any?, :many?, :to => :scoped + delegate :find, :take, :take!, :first, :first!, :last, :last!, :all, :exists?, :any?, :many?, :to => :scoped delegate :first_or_create, :first_or_create!, :first_or_initialize, :to => :scoped delegate :find_by, :find_by!, :to => :scoped delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, :to => :scoped diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 1ceb1949a4..3c9c9c4e84 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -60,6 +60,28 @@ module ActiveRecord where(*args).first! end + # Gives a record (or N records if a parameter is supplied) without any implied + # order. The order will depend on the database implementation. + # If an order is supplied it will be respected. + # + # Examples: + # + # Person.take # returns an object fetched by SELECT * FROM people + # Person.take(5) # returns 5 objects fetched by SELECT * FROM people LIMIT 5 + # Person.where(["name LIKE '%?'", name]).take + def take(limit = nil) + limit ? limit(limit).to_a : find_take + end + + # Same as +take+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record + # is found. Note that <tt>take!</tt> accepts no arguments. + def take! + take or raise RecordNotFound + end + + # Find the first record (or first N records if a parameter is supplied). + # If no order is defined it will order by primary key. + # # Examples: # # Person.first # returns the first object fetched by SELECT * FROM people @@ -67,7 +89,15 @@ module ActiveRecord # Person.where(["user_name = :u", { :u => user_name }]).first # Person.order("created_on DESC").offset(5).first def first(limit = nil) - limit ? limit(limit).to_a : find_first + if limit + if order_values.empty? && primary_key + order(arel_table[primary_key].asc).limit(limit).to_a + else + limit(limit).to_a + end + else + find_first + end end # Same as +first+ but raises <tt>ActiveRecord::RecordNotFound</tt> if no record @@ -76,6 +106,9 @@ module ActiveRecord first or raise RecordNotFound end + # Find the last record (or last N records if a parameter is supplied). + # If no order is defined it will order by primary key. + # # Examples: # # Person.last # returns the last object fetched by SELECT * FROM people @@ -83,8 +116,8 @@ module ActiveRecord # Person.order("created_on DESC").offset(5).last def last(limit = nil) if limit - if order_values.empty? - order("#{primary_key} DESC").limit(limit).reverse + if order_values.empty? && primary_key + order(arel_table[primary_key].desc).limit(limit).reverse else to_a.last(limit) end @@ -315,11 +348,24 @@ module ActiveRecord end end + def find_take + if loaded? + @records.take(1).first + else + @take ||= limit(1).to_a.first + end + end + def find_first if loaded? @records.first else - @first ||= limit(1).to_a[0] + @first ||= + if order_values.empty? && primary_key + order(arel_table[primary_key].asc).limit(1).to_a.first + else + limit(1).to_a.first + end end end @@ -331,7 +377,7 @@ module ActiveRecord if offset_value || limit_value to_a.last else - reverse_order.limit(1).to_a[0] + reverse_order.limit(1).to_a.first end end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 29469c42ed..54801bd101 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -143,6 +143,26 @@ class FinderTest < ActiveRecord::TestCase assert_equal [Account], accounts.collect(&:class).uniq end + def test_take + assert_equal topics(:first), Topic.take + end + + def test_take_failing + assert_nil Topic.where("title = 'This title does not exist'").take + end + + def test_take_bang_present + assert_nothing_raised do + assert_equal topics(:second), Topic.where("title = 'The Second Topic of the day'").take! + end + end + + def test_take_bang_missing + assert_raises ActiveRecord::RecordNotFound do + Topic.where("title = 'This title does not exist'").take! + end + end + def test_first assert_equal topics(:second).title, Topic.where("title = 'The Second Topic of the day'").first.title end @@ -163,6 +183,12 @@ class FinderTest < ActiveRecord::TestCase end end + def test_first_have_primary_key_order_by_default + expected = topics(:first) + expected.touch # PostgreSQL changes the default order if no order clause is used + assert_equal expected, Topic.first + end + def test_model_class_responds_to_first_bang assert Topic.first! Topic.delete_all @@ -191,7 +217,8 @@ class FinderTest < ActiveRecord::TestCase end end - def test_first_and_last_with_integer_should_use_sql_limit + def test_take_and_first_and_last_with_integer_should_use_sql_limit + assert_sql(/LIMIT 3|ROWNUM <= 3/) { Topic.take(3).entries } assert_sql(/LIMIT 2|ROWNUM <= 2/) { Topic.first(2).entries } assert_sql(/LIMIT 5|ROWNUM <= 5/) { Topic.last(5).entries } end @@ -212,7 +239,8 @@ class FinderTest < ActiveRecord::TestCase assert_no_match(/LIMIT/, query.first) end - def test_first_and_last_with_integer_should_return_an_array + def test_take_and_first_and_last_with_integer_should_return_an_array + assert_kind_of Array, Topic.take(5) assert_kind_of Array, Topic.first(5) assert_kind_of Array, Topic.last(5) end |