Merge pull request #14372 from arthurnn/fix_query_cache_binds

Make select_all on query cache accept a Relation without binds.

activerecord/CHANGELOG.md

@@ -1,3 +1,9 @@
+*   Add support for `Relation` be passed as parameter on `QueryCache#select_all`.
+
+    Fixes #14361.
+
+    *arthurnn*
+
 * Passing an Active Record object to `find` is now deprecated.  Call `.id`
   on the object first.
 

activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb

@@ -20,14 +20,7 @@ module ActiveRecord
 
       # Returns an ActiveRecord::Result instance.
       def select_all(arel, name = nil, binds = [])
-        if arel.is_a?(Relation)
-          relation = arel
-          arel = relation.arel
-          if !binds || binds.empty?
-            binds = relation.bind_values
-          end
-        end
-
+        arel, binds = binds_from_relation arel, binds
         select(to_sql(arel, binds), name, binds)
       end
 
@@ -47,10 +40,7 @@ module ActiveRecord
       # Returns an array of the values of the first column in a select:
       #   select_values("SELECT id FROM companies LIMIT 3") => [1,2,3]
       def select_values(arel, name = nil)
-        binds = []
-        if arel.is_a?(Relation)
-          arel, binds = arel.arel, arel.bind_values
-        end
+        arel, binds = binds_from_relation arel
         select_rows(to_sql(arel, binds), name, binds).map(&:first)
       end
 
@@ -389,6 +379,13 @@ module ActiveRecord
           row = result.rows.first
           row && row.first
         end
+
+        def binds_from_relation(relation, binds = [])
+          if relation.is_a?(Relation) && binds.blank?
+            relation, binds = relation.arel, relation.bind_values
+          end
+          [relation, binds]
+        end
     end
   end
 end

activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb

@@ -63,6 +63,7 @@ module ActiveRecord
 
       def select_all(arel, name = nil, binds = [])
         if @query_cache_enabled && !locked?(arel)
+          arel, binds = binds_from_relation arel, binds
           sql = to_sql(arel, binds)
           cache_sql(sql, binds) { super(sql, name, binds) }
         else

activerecord/test/cases/query_cache_test.rb

@@ -118,6 +118,14 @@ class QueryCacheTest < ActiveRecord::TestCase
     assert ActiveRecord::Base.connection.query_cache.empty?, 'cache should be empty'
   end
 
+  def test_cache_passing_a_relation
+    post = Post.first
+    Post.cache do
+      query = post.categories.select(:post_id)
+      assert Post.connection.select_all(query).is_a?(ActiveRecord::Result)
+    end
+  end
+
   def test_find_queries
     assert_queries(2) { Task.find(1); Task.find(1) }
   end