Don't cache `find_by` statements on STI subclasses

Caching `find_by` statements on STI subclasses is unsafe, since
`type IN (?,?,?,?)` part is dynamic, and we don't have SQL statements
cache invalidation when a STI subclass is created or removed for now.

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
 

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

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