From 2fcafee250ee24224b8fb8c1d884a48770fe08b3 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 19:24:56 +0200 Subject: Fix that #pluck wasn't rescuing ThrowResult, meaning it would blow up when failing to construct_limited_ids_condition. --- activerecord/lib/active_record/relation/calculations.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 7239270c4d..308db8227f 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -189,6 +189,8 @@ module ActiveRecord end columns.one? ? result.map!(&:first) : result end + rescue ThrowResult + [] end # Pluck all the ID's for the relation using the table's primary key -- cgit v1.2.3 From 86cc141ed5fd51ec752c956febfa436d1c816532 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 20:35:59 +0200 Subject: No point in memoizing a simple literal string. --- activerecord/lib/active_record/null_relation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 711fc8b883..5d801fa705 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -39,7 +39,7 @@ module ActiveRecord end def to_sql - @to_sql ||= "" + "" end def where_values_hash -- cgit v1.2.3 From a2e607e1055dcede27970ccd7f5b89a1ddee8c32 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 20:45:09 +0200 Subject: Make NullRelation a bit more like a real relation by returning 0 for #calculate(:count) --- activerecord/lib/active_record/null_relation.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/null_relation.rb b/activerecord/lib/active_record/null_relation.rb index 5d801fa705..d166f0dd66 100644 --- a/activerecord/lib/active_record/null_relation.rb +++ b/activerecord/lib/active_record/null_relation.rb @@ -55,7 +55,11 @@ module ActiveRecord end def calculate(_operation, _column_name, _options = {}) - nil + if _operation == :count + 0 + else + nil + end end def exists?(_id = false) -- cgit v1.2.3 From 118147af53bebf71bf2a1d3ded4fc6491a708697 Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Fri, 10 May 2013 20:57:12 +0200 Subject: Rather than raising ThrowResult when construct_limited_ids_conditions comes up empty, set the relation to NullRelation and rely on its results. This will help avoid errors like 2fcafee250ee2, because in most cases NullRelation will do the right thing. Minor bonus is avoiding the use of exceptions for flow control. --- activerecord/lib/active_record/errors.rb | 4 ---- .../lib/active_record/relation/calculations.rb | 4 ---- .../lib/active_record/relation/finder_methods.rb | 28 ++++++++++++---------- 3 files changed, 15 insertions(+), 21 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index cd31147414..34bfad2d62 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -69,10 +69,6 @@ module ActiveRecord end end - # Raised when SQL statement is invalid and the application gets a blank result. - class ThrowResult < ActiveRecordError - end - # Defunct wrapper class kept for compatibility. # +StatementInvalid+ wraps the original exception now. class WrappedDatabaseException < StatementInvalid diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 308db8227f..7371fe370e 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -114,8 +114,6 @@ module ActiveRecord else relation.calculate(operation, column_name, options) end - rescue ThrowResult - 0 end # Use pluck as a shortcut to select one or more attributes without @@ -189,8 +187,6 @@ module ActiveRecord end columns.one? ? result.map!(&:first) : result end - rescue ThrowResult - [] end # Pluck all the ID's for the relation using the table's primary key diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index ba222aac93..f0edef58bf 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -160,8 +160,9 @@ module ActiveRecord conditions = conditions.id if Base === conditions return false if !conditions - join_dependency = construct_join_dependency - relation = construct_relation_for_association_find(join_dependency) + relation = construct_relation_for_association_find(construct_join_dependency) + return false if ActiveRecord::NullRelation === relation + relation = relation.except(:select, :order).select("1 AS one").limit(1) case conditions @@ -172,8 +173,6 @@ module ActiveRecord end connection.select_value(relation, "#{name} Exists", relation.bind_values) - rescue ThrowResult - false end # This method is called whenever no records are found with either a single @@ -203,10 +202,12 @@ module ActiveRecord def find_with_associations join_dependency = construct_join_dependency relation = construct_relation_for_association_find(join_dependency) - rows = connection.select_all(relation, 'SQL', relation.bind_values.dup) - join_dependency.instantiate(rows) - rescue ThrowResult - [] + if ActiveRecord::NullRelation === relation + [] + else + rows = connection.select_all(relation, 'SQL', relation.bind_values.dup) + join_dependency.instantiate(rows) + end end def construct_join_dependency(joins = []) @@ -230,21 +231,22 @@ module ActiveRecord if using_limitable_reflections?(join_dependency.reflections) relation else - relation.where!(construct_limited_ids_condition(relation)) if relation.limit_value + if relation.limit_value + limited_ids = limited_ids_for(relation) + limited_ids.empty? ? relation.none! : relation.where!(table[primary_key].in(limited_ids)) + end relation.except(:limit, :offset) end end - def construct_limited_ids_condition(relation) + def limited_ids_for(relation) values = @klass.connection.columns_for_distinct( "#{quoted_table_name}.#{quoted_primary_key}", relation.order_values) relation = relation.except(:select).select(values).distinct! id_rows = @klass.connection.select_all(relation.arel, 'SQL', relation.bind_values) - ids_array = id_rows.map {|row| row[primary_key]} - - ids_array.empty? ? raise(ThrowResult) : table[primary_key].in(ids_array) + id_rows.map {|row| row[primary_key]} end def find_with_ids(*ids) -- cgit v1.2.3