From 94924dc32baf78f13e289172534c2e71c9c8cade Mon Sep 17 00:00:00 2001 From: Jon Leighton Date: Fri, 28 Jun 2013 13:32:54 +0100 Subject: Simplify/fix implementation of default scopes The previous implementation was necessary in order to support stuff like: class Post < ActiveRecord::Base default_scope where(published: true) scope :ordered, order("created_at") end If we didn't evaluate the default scope at the last possible moment before sending the SQL to the database, it would become impossible to do: Post.unscoped.ordered This is because the default scope would already be bound up in the "ordered" scope, and therefore wouldn't be removed by the "Post.unscoped" part. In 4.0, we have deprecated all "eager" forms of scopes. So now you must write: class Post < ActiveRecord::Base default_scope { where(published: true) } scope :ordered, -> { order("created_at") } end This prevents the default scope getting bound up inside the "ordered" scope, which means we can now have a simpler/better/more natural implementation of default scoping. A knock on effect is that some things that didn't work properly now do. For example it was previously impossible to use #except to remove a part of the default scope, since the default scope was evaluated after the call to #except. --- activerecord/lib/active_record/relation/merger.rb | 4 ---- 1 file changed, 4 deletions(-) (limited to 'activerecord/lib/active_record/relation/merger.rb') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index c114ea0c0d..6d047e0331 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -42,10 +42,6 @@ module ActiveRecord attr_reader :relation, :values, :other def initialize(relation, other) - if other.default_scoped? && other.klass != relation.klass - other = other.with_default_scope - end - @relation = relation @values = other.values @other = other -- cgit v1.2.3 From 9f85c41eec2ffcb2477bd4d66854d168634c5eaf Mon Sep 17 00:00:00 2001 From: Ankit Gupta Date: Fri, 12 Jul 2013 21:29:31 +0100 Subject: Typo fix [skip ci] --- activerecord/lib/active_record/relation/merger.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record/relation/merger.rb') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 6d047e0331..f5d668dae8 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -109,7 +109,7 @@ module ActiveRecord # override any order specified in the original relation relation.reorder! values[:order] elsif values[:order] - # merge in order_values from r + # merge in order_values from relation relation.order! values[:order] end -- cgit v1.2.3 From 9675c7da28758432ec7bd6e7ca1814801b2591f5 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Mon, 15 Jul 2013 16:29:01 -0700 Subject: reorder bind parameters when merging relations --- activerecord/lib/active_record/relation/merger.rb | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) (limited to 'activerecord/lib/active_record/relation/merger.rb') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index f5d668dae8..da13152e01 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -97,13 +97,29 @@ module ActiveRecord def merge_multi_values lhs_wheres = relation.where_values rhs_wheres = values[:where] || [] + lhs_binds = relation.bind_values rhs_binds = values[:bind] || [] removed, kept = partition_overwrites(lhs_wheres, rhs_wheres) - relation.where_values = kept + rhs_wheres - relation.bind_values = filter_binds(lhs_binds, removed) + rhs_binds + where_values = kept + rhs_wheres + bind_values = filter_binds(lhs_binds, removed) + rhs_binds + + conn = relation.klass.connection + bviter = bind_values.each.with_index + where_values.map! do |node| + if Arel::Nodes::Equality === node && Arel::Nodes::BindParam === node.right + (column, _), i = bviter.next + substitute = conn.substitute_at column, i + Arel::Nodes::Equality.new(node.left, substitute) + else + node + end + end + + relation.where_values = where_values + relation.bind_values = bind_values if values[:reordering] # override any order specified in the original relation -- cgit v1.2.3 From f38b5444428f418c4e6377bbb40d7518ea0c61a7 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Tue, 23 Jul 2013 14:43:17 -0700 Subject: add a specific factory method rather than using new --- activerecord/lib/active_record/relation/merger.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record/relation/merger.rb') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index da13152e01..c08158d38b 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -22,7 +22,7 @@ module ActiveRecord # build a relation to merge in rather than directly merging # the values. def other - other = Relation.new(relation.klass, relation.table) + other = Relation.create(relation.klass, relation.table) hash.each { |k, v| if k == :joins if Hash === v -- cgit v1.2.3 From 7db835c9f98ee6cc536d0304a5f7a9965efc86cb Mon Sep 17 00:00:00 2001 From: Nicholas Jakobsen Date: Fri, 30 Aug 2013 13:02:45 -0700 Subject: Don't create fibers just to iterate --- activerecord/lib/active_record/relation/merger.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'activerecord/lib/active_record/relation/merger.rb') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index c08158d38b..530c47d0d0 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -107,11 +107,11 @@ module ActiveRecord bind_values = filter_binds(lhs_binds, removed) + rhs_binds conn = relation.klass.connection - bviter = bind_values.each.with_index + bv_index = 0 where_values.map! do |node| if Arel::Nodes::Equality === node && Arel::Nodes::BindParam === node.right - (column, _), i = bviter.next - substitute = conn.substitute_at column, i + substitute = conn.substitute_at(bind_values[bv_index].first, bv_index) + bv_index += 1 Arel::Nodes::Equality.new(node.left, substitute) else node -- cgit v1.2.3 From 5cbed2bc606750c71ee231d2b816471fe241459e Mon Sep 17 00:00:00 2001 From: thedarkone Date: Wed, 11 Sep 2013 14:18:31 +0200 Subject: Relation#merge should not lose readonly(false) flag. The original code ignores the `false` value because `false.blank? # => true`. --- activerecord/lib/active_record/relation/merger.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'activerecord/lib/active_record/relation/merger.rb') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index 530c47d0d0..c05632e688 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -58,7 +58,11 @@ module ActiveRecord def merge normal_values.each do |name| value = values[name] - relation.send("#{name}!", *value) unless value.blank? + # The unless clause is here mostly for performance reasons (since the `send` call might be moderately + # expensive), most of the time the value is going to be `nil` or `.blank?`, the only catch is that + # `false.blank?` returns `true`, so there needs to be an extra check so that explicit `false` values + # don't fall through the cracks. + relation.send("#{name}!", *value) unless value.nil? || (value.blank? && false != value) end merge_multi_values -- cgit v1.2.3 From d20ccb7e632665b6661e82ae450e8180e3c085f9 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 10 Oct 2013 14:12:04 -0700 Subject: stuff the join dependency object in the "anything goes" hash. --- activerecord/lib/active_record/relation/merger.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib/active_record/relation/merger.rb') diff --git a/activerecord/lib/active_record/relation/merger.rb b/activerecord/lib/active_record/relation/merger.rb index c05632e688..182b9ed89c 100644 --- a/activerecord/lib/active_record/relation/merger.rb +++ b/activerecord/lib/active_record/relation/merger.rb @@ -94,7 +94,7 @@ module ActiveRecord []) relation.joins! rest - @relation = join_dependency.join_relation(relation) + @relation = relation.joins join_dependency end end -- cgit v1.2.3