From 433b19d7e82263fb78c481576ed0f475a62fde06 Mon Sep 17 00:00:00 2001
From: Arthur Neves <arthurnn@gmail.com>
Date: Thu, 13 Mar 2014 13:05:10 -0400
Subject: [PATCH] Make select_all on query cache accept a Relation without
 binds.

[fixes #14361]
[related #13886]
---
 activerecord/CHANGELOG.md                     |  6 ++++++
 .../abstract/database_statements.rb           | 21 ++++++++-----------
 .../abstract/query_cache.rb                   |  1 +
 activerecord/test/cases/query_cache_test.rb   |  8 +++++++
 4 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index d52c2f711e03f..d6044988f0bb2 100644
--- a/activerecord/CHANGELOG.md
+++ b/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.
 
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
index 6eb59cc398388..dd1962e4d69c8 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb
@@ -20,14 +20,7 @@ def to_sql(arel, binds = [])
 
       # 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 @@ def select_value(arel, name = nil, binds = [])
       # 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 @@ def last_inserted_id(result)
           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
diff --git a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
index adc23a6674912..4a4506c7f5d3c 100644
--- a/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
+++ b/activerecord/lib/active_record/connection_adapters/abstract/query_cache.rb
@@ -63,6 +63,7 @@ def clear_query_cache
 
       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
diff --git a/activerecord/test/cases/query_cache_test.rb b/activerecord/test/cases/query_cache_test.rb
index da8ae672fe628..9d89d6a1e8af1 100644
--- a/activerecord/test/cases/query_cache_test.rb
+++ b/activerecord/test/cases/query_cache_test.rb
@@ -118,6 +118,14 @@ def test_cache_clear_after_close
     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