mirror of
https://github.com/rails/rails.git
synced 2022-11-09 12:12:34 -05:00
Handle Relations with having clauses in #include?
- skips optimised exists? query for relations that have a having clause Relations that have aliased select values AND a having clause that references an aliased select value would generate an error when #include? was called, due to an optimisation that would generate call #exists? on the relation instead, which effectively alters the select values of the query (and thus removes the aliased select values), but leaves the having clause intact. Because the having clause is then referencing an aliased column that is no longer present in the simplified query, an ActiveRecord::InvalidStatement error was raised. An sample query affected by this problem: Author.select('COUNT(*) as total_posts', 'authors.*') .joins(:posts) .group(:id) .having('total_posts > 2') .include?(Author.first) This change adds an addition check to the condition that skips the simplified #exists? query, which simply checks for the presence of a having clause.
This commit is contained in:
parent
5b0fe373f3
commit
644d694dea
3 changed files with 41 additions and 1 deletions
|
@ -1,3 +1,34 @@
|
||||||
|
* Skip optimised #exist? query when #include? is called on a relation
|
||||||
|
with a having clause
|
||||||
|
|
||||||
|
Relations that have aliased select values AND a having clause that
|
||||||
|
references an aliased select value would generate an error when
|
||||||
|
#include? was called, due to an optimisation that would generate
|
||||||
|
call #exists? on the relation instead, which effectively alters
|
||||||
|
the select values of the query (and thus removes the aliased select
|
||||||
|
values), but leaves the having clause intact. Because the having
|
||||||
|
clause is then referencing an aliased column that is no longer
|
||||||
|
present in the simplified query, an ActiveRecord::InvalidStatement
|
||||||
|
error was raised.
|
||||||
|
|
||||||
|
An sample query affected by this problem:
|
||||||
|
|
||||||
|
```ruby
|
||||||
|
Author.select('COUNT(*) as total_posts', 'authors.*')
|
||||||
|
.joins(:posts)
|
||||||
|
.group(:id)
|
||||||
|
.having('total_posts > 2')
|
||||||
|
.include?(Author.first)
|
||||||
|
```
|
||||||
|
|
||||||
|
This change adds an addition check to the condition that skips the
|
||||||
|
simplified #exists? query, which simply checks for the presence of
|
||||||
|
a having clause.
|
||||||
|
|
||||||
|
Fixes #41417
|
||||||
|
|
||||||
|
*Michael Smart*
|
||||||
|
|
||||||
* Increment postgres prepared statement counter before making a prepared statement, so if the statement is aborted
|
* Increment postgres prepared statement counter before making a prepared statement, so if the statement is aborted
|
||||||
without Rails knowledge (e.g., if app gets kill -9d during long-running query or due to Rack::Timeout), app won't end
|
without Rails knowledge (e.g., if app gets kill -9d during long-running query or due to Rack::Timeout), app won't end
|
||||||
up in perpetual crash state for being inconsistent with Postgres.
|
up in perpetual crash state for being inconsistent with Postgres.
|
||||||
|
|
|
@ -352,7 +352,7 @@ module ActiveRecord
|
||||||
# compared to the records in memory. If the relation is unloaded, an
|
# compared to the records in memory. If the relation is unloaded, an
|
||||||
# efficient existence query is performed, as in #exists?.
|
# efficient existence query is performed, as in #exists?.
|
||||||
def include?(record)
|
def include?(record)
|
||||||
if loaded? || offset_value || limit_value
|
if loaded? || offset_value || limit_value || having_clause.any?
|
||||||
records.include?(record)
|
records.include?(record)
|
||||||
else
|
else
|
||||||
record.is_a?(klass) && exists?(record.id)
|
record.is_a?(klass) && exists?(record.id)
|
||||||
|
|
|
@ -412,6 +412,15 @@ class FinderTest < ActiveRecord::TestCase
|
||||||
assert_equal true, Customer.order(id: :desc).limit(2).include?(mary)
|
assert_equal true, Customer.order(id: :desc).limit(2).include?(mary)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def test_include_on_unloaded_relation_with_having_referencing_aliased_select
|
||||||
|
skip if current_adapter?(:PostgreSQLAdapter)
|
||||||
|
bob = authors(:bob)
|
||||||
|
mary = authors(:mary)
|
||||||
|
|
||||||
|
assert_equal false, Author.select("COUNT(*) as total_posts", "authors.*").joins(:posts).group(:id).having("total_posts > 2").include?(bob)
|
||||||
|
assert_equal true, Author.select("COUNT(*) as total_posts", "authors.*").joins(:posts).group(:id).having("total_posts > 2").include?(mary)
|
||||||
|
end
|
||||||
|
|
||||||
def test_include_on_loaded_relation_with_match
|
def test_include_on_loaded_relation_with_match
|
||||||
customers = Customer.where(name: "David").load
|
customers = Customer.where(name: "David").load
|
||||||
david = customers(:david)
|
david = customers(:david)
|
||||||
|
|
Loading…
Reference in a new issue