From 787b991c940532e1ef4b5036e26ac9dc97e514a8 Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Wed, 27 May 2020 09:18:00 +0900 Subject: [PATCH] `rewhere` allow to overwrite non-attribute nodes Follow up to #39415. --- .../lib/active_record/relation/query_methods.rb | 2 +- .../lib/active_record/relation/where_clause.rb | 16 ++++++---------- activerecord/test/cases/relation/merging_test.rb | 6 ++++++ 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 4ca72b0cf9..89704be34a 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1399,7 +1399,7 @@ def order_column(field) def resolve_arel_attributes(attrs) attrs.flat_map do |attr| case attr - when Arel::Attributes::Attribute + when Arel::Predications attr when Hash attr.flat_map do |table, columns| diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index a558fa298f..dd92beb136 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -104,7 +104,7 @@ def extract_attributes attrs = [] predicates.each do |node| Arel.fetch_attribute(node) { |attr| attrs << attr } || begin - attrs << node.left if node.equality? && node.left.is_a?(Arel::Nodes::Node) + attrs << node.left if node.equality? && node.left.is_a?(Arel::Predications) end end attrs @@ -157,18 +157,14 @@ def invert_predicate(node) end def except_predicates(columns) - non_attrs = columns.extract! { |node| node.is_a?(Arel::Nodes::Node) } + attrs = columns.extract! { |node| node.is_a?(Arel::Attribute) } + non_attrs = columns.extract! { |node| node.is_a?(Arel::Predications) } + predicates.reject do |node| - if !non_attrs.empty? && node.equality? && node.left.is_a?(Arel::Nodes::Node) + if !non_attrs.empty? && node.equality? && node.left.is_a?(Arel::Predications) non_attrs.include?(node.left) end || Arel.fetch_attribute(node) do |attr| - columns.any? do |column| - if column.is_a?(Arel::Attributes::Attribute) - attr == column - else - attr.name.to_s == column.to_s - end - end + attrs.include?(attr) || columns.include?(attr.name.to_s) end end end diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 9507a351b9..fc7b4de0c0 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -137,6 +137,9 @@ def test_relation_merging_with_arel_equalities_keeps_last_equality devs = Developer.where(salary_attr.eq(80000)).merge(Developer.where(salary_attr.eq(9000)), rewhere: true) assert_equal [developers(:poor_jamis)], devs.to_a + + devs = Developer.where(salary_attr.eq(80000)).rewhere(salary_attr.eq(9000)) + assert_equal [developers(:poor_jamis)], devs.to_a end def test_relation_merging_with_arel_equalities_keeps_last_equality_with_non_attribute_left_hand @@ -148,6 +151,9 @@ def test_relation_merging_with_arel_equalities_keeps_last_equality_with_non_attr devs = Developer.where(abs_salary.eq(80000)).merge(Developer.where(abs_salary.eq(9000)), rewhere: true) assert_equal [developers(:poor_jamis)], devs.to_a + + devs = Developer.where(abs_salary.eq(80000)).rewhere(abs_salary.eq(9000)) + assert_equal [developers(:poor_jamis)], devs.to_a end def test_relation_merging_with_eager_load -- GitLab