From 0ea798a47b498664bce0edf2b7e80757943a4f4d Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Mon, 15 Jun 2020 16:31:41 +0900 Subject: [PATCH] Allow difference for `and`/`or` unless other values are set explicitly I often hit the inconvenience (recently in #39558). `and`/`or` are focused to combine where-like clauses, so we often use one-time relation which has only where clause. But if the receiver has values other than where clause (e.g. `order`, `select`, `includes`, etc), `and`/`or` will raise structurally incompatible error. Since it is harder to predict the receiver has which values other than where clause before doing `and`/`or`, that restriction is a little annoying to me. So I'd like to relax that restriction, at least unless other values are set explicitly for multiple values. --- activerecord/lib/active_record/relation/query_methods.rb | 7 +++++-- activerecord/test/cases/relation/merging_test.rb | 6 +++--- activerecord/test/cases/relation/or_test.rb | 3 ++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index c80f573f01..110cfc6d0b 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1513,8 +1513,11 @@ module ActiveRecord values = other.values STRUCTURAL_OR_METHODS.reject do |method| v1, v2 = @values[method], values[method] - v1 = v1.uniq if v1.is_a?(Array) - v2 = v2.uniq if v2.is_a?(Array) + if v1.is_a?(Array) + next true unless v2.is_a?(Array) + v1 = v1.uniq + v2 = v2.uniq + end v1 == v2 || (!v1 || v1.empty?) && (!v2 || v2.empty?) end end diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index d7f6b43cec..0da5dd7cbc 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -43,7 +43,7 @@ class RelationMergingTest < ActiveRecord::TestCase assert_equal [mary], david_and_mary.and(mary_and_bob) assert_equal authors, david_and_mary.or(mary_and_bob) - david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")).order(:id) + david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")) assert_equal [david], david_and_mary.merge(david_and_bob) assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true) @@ -96,7 +96,7 @@ class RelationMergingTest < ActiveRecord::TestCase assert_equal [mary], david_and_mary.and(mary_and_bob) assert_equal authors, david_and_mary.or(mary_and_bob) - david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")).order(:id) + david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")) assert_equal [david], david_and_mary.merge(david_and_bob) assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true) @@ -149,7 +149,7 @@ class RelationMergingTest < ActiveRecord::TestCase assert_equal [mary], david_and_mary.and(mary_and_bob) assert_equal authors, david_and_mary.or(mary_and_bob) - david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")).order(:id) + david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")) assert_equal [david], david_and_mary.merge(david_and_bob) assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true) diff --git a/activerecord/test/cases/relation/or_test.rb b/activerecord/test/cases/relation/or_test.rb index 4e6a78e283..b3dbbc4d6d 100644 --- a/activerecord/test/cases/relation/or_test.rb +++ b/activerecord/test/cases/relation/or_test.rb @@ -84,11 +84,12 @@ module ActiveRecord def test_or_with_unscope_order expected = Post.where("id = 1 or id = 2") assert_equal expected, Post.order("body asc").where("id = 1").unscope(:order).or(Post.where("id = 2")).to_a + assert_equal expected, Post.order(:id).where("id = 1").or(Post.order(:id).where("id = 2").unscope(:order)).to_a end def test_or_with_incompatible_unscope error = assert_raises ArgumentError do - Post.order("body asc").where("id = 1").or(Post.order("body asc").where("id = 2").unscope(:order)).to_a + Post.order("body asc").where("id = 1").unscope(:order).or(Post.order("body asc").where("id = 2")).to_a end assert_equal "Relation passed to #or must be structurally compatible. Incompatible values: [:order]", error.message