Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix that #exists? can blow up with ThrowResult exception #6521

Merged
merged 1 commit into from

2 participants

Ben Woosley Rafael Mendonça França
Ben Woosley

FinderMethods#construct_limited_ids_condition will raise ThrowResult if the limited reflection comes back empty. The other callers of #construct_limited_ids_condition handle this exception (more specifically, the callers of construct_relation_for*), but #exists? didn't until now.

The included test hits this condition, and after the tests runs green.

Ben Woosley

Can somebody take a look at this? I ran into when running an #exists? against a paginated collection from kaminari in a view.

Rafael Mendonça França

Seems good. Could you squash the commits?

Ben Woosley Empact Fix that #exists? raises ThrowResult when called with an empty limited
reflection.

ActiveRecord::FinderMethods#construct_limited_ids_condition will raise
ThrowResult if the limited reflection comes back empty. The other callers
of #construct_limited_ids_condition handle this exception (more specifically,
the callers of construct_relation_for*), but #exists? didn't until now.
340a93f
Ben Woosley

Done. Also added the complete explanation to the commit message.

Rafael Mendonça França rafaelfranca merged commit f48e767 into from
Ben Woosley

Thanks - would you also take a look at #6523? It's now squashed and such as well.

Rafael Mendonça França

I'll look at it. But I need to discuss with someone with a better knowledge of the active record first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jun 11, 2012
  1. Ben Woosley

    Fix that #exists? raises ThrowResult when called with an empty limited

    Empact authored
    reflection.
    
    ActiveRecord::FinderMethods#construct_limited_ids_condition will raise
    ThrowResult if the limited reflection comes back empty. The other callers
    of #construct_limited_ids_condition handle this exception (more specifically,
    the callers of construct_relation_for*), but #exists? didn't until now.
This page is out of date. Refresh to see the latest.
2  activerecord/lib/active_record/relation/finder_methods.rb
View
@@ -186,6 +186,8 @@ def exists?(id = false)
end
connection.select_value(relation, "#{name} Exists", relation.bind_values)
+ rescue ThrowResult
+ false
end
protected
7 activerecord/test/cases/finder_test.rb
View
@@ -69,7 +69,12 @@ def test_exists_with_order
assert Topic.order(:id).uniq.exists?
end
- def test_does_not_exist_with_empty_table_and_no_args_given
+ def test_exists_with_includes_limit_and_empty_result
+ assert !Topic.includes(:replies).limit(0).exists?
+ assert !Topic.includes(:replies).limit(1).where('0 = 1').exists?
+ end
+
+ def test_exists_with_empty_table_and_no_args_given
Topic.delete_all
assert !Topic.exists?
end
Something went wrong with that request. Please try again.