Skip to content

Commit

Permalink
Make select_all on query cache accept a Relation without binds.
Browse files Browse the repository at this point in the history
[fixes rails#14361]
[related rails#13886]
  • Loading branch information
arthurnn committed Mar 13, 2014
1 parent d35f003 commit 433b19d
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 12 deletions.
6 changes: 6 additions & 0 deletions 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.

Expand Down
Expand Up @@ -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

Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions activerecord/test/cases/query_cache_test.rb
Expand Up @@ -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
Expand Down

0 comments on commit 433b19d

Please sign in to comment.