diff options
10 files changed, 42 insertions, 26 deletions
diff --git a/activemodel/lib/active_model/serializers/json.rb b/activemodel/lib/active_model/serializers/json.rb index 9d984b7a18..05e2e089e5 100644 --- a/activemodel/lib/active_model/serializers/json.rb +++ b/activemodel/lib/active_model/serializers/json.rb @@ -109,7 +109,7 @@ module ActiveModel # # def attributes=(hash) # hash.each do |key, value| - # instance_variable_set("@#{key}", value) + # send("#{key}=", value) # end # end # diff --git a/activemodel/test/cases/railtie_test.rb b/activemodel/test/cases/railtie_test.rb index d44a3448df..0643fa775d 100644 --- a/activemodel/test/cases/railtie_test.rb +++ b/activemodel/test/cases/railtie_test.rb @@ -7,8 +7,12 @@ class RailtieTest < ActiveModel::TestCase def setup require 'active_model/railtie' + # Set a fake logger to avoid creating the log directory automatically + fake_logger = mock() + @app ||= Class.new(::Rails::Application) do config.eager_load = false + config.logger = fake_logger end end diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 017b8bace6..7e38719811 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -69,10 +69,6 @@ module ActiveRecord end end - # Raised when SQL statement is invalid and the application gets a blank result. - class ThrowResult < ActiveRecordError - end - # Defunct wrapper class kept for compatibility. # +StatementInvalid+ wraps the original exception now. class WrappedDatabaseException < StatementInvalid diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 711fc8b883..d166f0dd66 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -39,7 +39,7 @@ module ActiveRecord end def to_sql - @to_sql ||= "" + "" end def where_values_hash @@ -55,7 +55,11 @@ module ActiveRecord end def calculate(_operation, _column_name, _options = {}) - nil + if _operation == :count + 0 + else + nil + end end def exists?(_id = false) diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index e234e02032..ccb48247b7 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -106,8 +106,6 @@ module ActiveRecord else relation.calculate(operation, column_name, options) end - rescue ThrowResult - 0 end # Use <tt>pluck</tt> as a shortcut to select one or more attributes without diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index ba222aac93..f0edef58bf 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -160,8 +160,9 @@ module ActiveRecord conditions = conditions.id if Base === conditions return false if !conditions - join_dependency = construct_join_dependency - relation = construct_relation_for_association_find(join_dependency) + relation = construct_relation_for_association_find(construct_join_dependency) + return false if ActiveRecord::NullRelation === relation + relation = relation.except(:select, :order).select("1 AS one").limit(1) case conditions @@ -172,8 +173,6 @@ module ActiveRecord end connection.select_value(relation, "#{name} Exists", relation.bind_values) - rescue ThrowResult - false end # This method is called whenever no records are found with either a single @@ -203,10 +202,12 @@ module ActiveRecord def find_with_associations join_dependency = construct_join_dependency relation = construct_relation_for_association_find(join_dependency) - rows = connection.select_all(relation, 'SQL', relation.bind_values.dup) - join_dependency.instantiate(rows) - rescue ThrowResult - [] + if ActiveRecord::NullRelation === relation + [] + else + rows = connection.select_all(relation, 'SQL', relation.bind_values.dup) + join_dependency.instantiate(rows) + end end def construct_join_dependency(joins = []) @@ -230,21 +231,22 @@ module ActiveRecord if using_limitable_reflections?(join_dependency.reflections) relation else - relation.where!(construct_limited_ids_condition(relation)) if relation.limit_value + if relation.limit_value + limited_ids = limited_ids_for(relation) + limited_ids.empty? ? relation.none! : relation.where!(table[primary_key].in(limited_ids)) + end relation.except(:limit, :offset) end end - def construct_limited_ids_condition(relation) + def limited_ids_for(relation) values = @klass.connection.columns_for_distinct( "#{quoted_table_name}.#{quoted_primary_key}", relation.order_values) relation = relation.except(:select).select(values).distinct! id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values) - ids_array = id_rows.map {|row| row[primary_key]} - - ids_array.empty? ? raise(ThrowResult) : table[primary_key].in(ids_array) + id_rows.map {|row| row[primary_key]} end def find_with_ids(*ids) diff --git a/activerecord/test/cases/calculations_test.rb b/activerecord/test/cases/calculations_test.rb index 095b78c6c8..c8ac77984f 100644 --- a/activerecord/test/cases/calculations_test.rb +++ b/activerecord/test/cases/calculations_test.rb @@ -6,6 +6,7 @@ require 'models/edge' require 'models/organization' require 'models/possession' require 'models/topic' +require 'models/reply' require 'models/minivan' require 'models/speedometer' require 'models/ship_part' @@ -472,6 +473,11 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal [1,2,3,4], Topic.order(:id).pluck(:id) end + def test_pluck_without_column_names + assert_equal [[1, "Firm", 1, nil, "37signals", nil, 1, nil, ""]], + Company.order(:id).limit(1).pluck + end + def test_pluck_type_cast topic = topics(:first) relation = Topic.where(:id => topic.id) @@ -533,6 +539,11 @@ class CalculationsTest < ActiveRecord::TestCase assert_equal Company.all.map(&:id).sort, Company.ids.sort end + def test_pluck_with_includes_limit_and_empty_result + assert_equal [], Topic.includes(:replies).limit(0).pluck(:id) + assert_equal [], Topic.includes(:replies).limit(1).where('0 = 1').pluck(:id) + end + def test_pluck_not_auto_table_name_prefix_if_column_included Company.create!(:name => "test", :contracts => [Contract.new(:developer_id => 7)]) ids = Company.includes(:contracts).pluck(:developer_id) diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index b64ff13d29..e746ca2805 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -278,8 +278,9 @@ class RelationTest < ActiveRecord::TestCase def test_null_relation_calculations_methods assert_no_queries do - assert_equal 0, Developer.none.count - assert_equal nil, Developer.none.calculate(:average, 'salary') + assert_equal 0, Developer.none.count + assert_equal 0, Developer.none.calculate(:count, nil, {}) + assert_equal nil, Developer.none.calculate(:average, 'salary') end end diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 6d42667e97..3dde87ac2e 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -164,7 +164,7 @@ class Module # # Reason is twofold: On one hand doing less calls is in general better. # On the other hand it could be that the target has side-effects, - # whereas conceptualy, from the user point of view, the delegator should + # whereas conceptually, from the user point of view, the delegator should # be doing one call. if allow_nil module_eval(<<-EOS, file, line - 3) diff --git a/activesupport/lib/active_support/file_update_checker.rb b/activesupport/lib/active_support/file_update_checker.rb index 20136dd1b0..d6918bede2 100644 --- a/activesupport/lib/active_support/file_update_checker.rb +++ b/activesupport/lib/active_support/file_update_checker.rb @@ -115,7 +115,7 @@ module ActiveSupport end def compile_glob(hash) - hash.freeze # Freeze so changes aren't accidently pushed + hash.freeze # Freeze so changes aren't accidentally pushed return if hash.empty? globs = hash.map do |key, value| |