diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index c67980173fe3a28de059b848878f637fbb94138f..18cfac1f2fe4bc60ea10a26f7f877291d95ed7ca 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -182,7 +182,8 @@ def find(*ids) # :nodoc: end def find_by(*args) # :nodoc: - return super if scope_attributes? || reflect_on_all_aggregations.any? + return super if scope_attributes? || reflect_on_all_aggregations.any? || + columns_hash.key?(inheritance_column) && !base_class? hash = args.first diff --git a/activerecord/test/cases/bind_parameter_test.rb b/activerecord/test/cases/bind_parameter_test.rb index 8c3fe0437c5cc25dbcff1776bc43e9c15c1c9464..b89f05482188e3f0f28e45ef1f6d97e6d670368d 100644 --- a/activerecord/test/cases/bind_parameter_test.rb +++ b/activerecord/test/cases/bind_parameter_test.rb @@ -2,6 +2,7 @@ require "cases/helper" require "models/topic" +require "models/reply" require "models/author" require "models/post" @@ -53,12 +54,17 @@ def test_statement_cache_with_query_cache @connection.disable_query_cache! end - def test_statement_cache_with_find + def test_statement_cache_with_find_by @connection.clear_cache! - topics = Topic.where(id: 1).limit(1) - assert_equal 1, Topic.find(1).id - assert_includes statement_cache, to_sql_key(topics.arel) + assert_equal 1, Topic.find_by!(id: 1).id + assert_equal 2, Reply.find_by!(id: 2).id + + topic_sql = cached_statement(Topic, [:id]) + assert_includes statement_cache, to_sql_key(topic_sql) + + e = assert_raise { cached_statement(Reply, [:id]) } + assert_equal "Reply has no cached statement by [:id]", e.message end def test_statement_cache_with_in_clause @@ -139,6 +145,13 @@ def to_sql_key(arel) @connection.respond_to?(:sql_key, true) ? @connection.send(:sql_key, sql) : sql end + def cached_statement(klass, key) + cache = klass.send(:cached_find_by_statement, key) do + raise "#{klass} has no cached statement by #{key.inspect}" + end + cache.send(:query_builder).instance_variable_get(:@sql) + end + def statement_cache @connection.instance_variable_get(:@statements).send(:cache) end diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index 19655a2d38df63478815abf39f686a4007612921..629167e9edef7dadb7ce1b4b010228af2e8104ef 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -514,10 +514,12 @@ def test_instantiation_doesnt_try_to_require_corresponding_file # Should fail without FirmOnTheFly in the type condition. assert_raise(ActiveRecord::RecordNotFound) { Firm.find(foo.id) } + assert_raise(ActiveRecord::RecordNotFound) { Firm.find_by!(id: foo.id) } # Nest FirmOnTheFly in the test case where Dependencies won't see it. self.class.const_set :FirmOnTheFly, Class.new(Firm) assert_raise(ActiveRecord::SubclassNotFound) { Firm.find(foo.id) } + assert_raise(ActiveRecord::SubclassNotFound) { Firm.find_by!(id: foo.id) } # Nest FirmOnTheFly in Firm where Dependencies will see it. # This is analogous to nesting models in a migration. @@ -526,6 +528,7 @@ def test_instantiation_doesnt_try_to_require_corresponding_file # And instantiate will find the existing constant rather than trying # to require firm_on_the_fly. assert_nothing_raised { assert_kind_of Firm::FirmOnTheFly, Firm.find(foo.id) } + assert_nothing_raised { assert_kind_of Firm::FirmOnTheFly, Firm.find_by!(id: foo.id) } end end