From 7219eb2cd21ac9c9c985fbf98dbfd30d79bfff1b Mon Sep 17 00:00:00 2001 From: Ryuta Kamizono Date: Sun, 7 Jun 2020 06:49:42 +0900 Subject: [PATCH] Support `relation.and` for intersection as Set theory As described at #39328, `relation.merge` behaves inspired as Hash-like merge for where clause. In other words, currently there is no official way to intersect the result by both relation conditions (i.e. there is no official way to maintain both relation conditions). To resolve that issue, I'd like to support a way to intersect relations as `relation.and`. ```ruby david_and_mary = Author.where(id: [david, mary]) mary_and_bob = Author.where(id: [mary, bob]) # => [bob] david_and_mary.merge(mary_and_bob) # => [mary, bob] david_and_mary.and(mary_and_bob) # => [mary] david_and_mary.or(mary_and_bob) # => [david, mary, bob] ``` Fixes #39232. --- activerecord/CHANGELOG.md | 14 +++++++ activerecord/lib/active_record/querying.rb | 7 ++-- .../active_record/relation/query_methods.rb | 38 +++++++++++++++++-- .../active_record/relation/where_clause.rb | 4 ++ .../test/cases/relation/merging_test.rb | 30 ++++++++++++--- 5 files changed, 81 insertions(+), 12 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 68cdcd29a9de7..41e05f47d54b2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,17 @@ +* Support `relation.and` for intersection as Set theory. + + ```ruby + david_and_mary = Author.where(id: [david, mary]) + mary_and_bob = Author.where(id: [mary, bob]) # => [bob] + + david_and_mary.merge(mary_and_bob) # => [mary, bob] + + david_and_mary.and(mary_and_bob) # => [mary] + david_and_mary.or(mary_and_bob) # => [david, mary, bob] + ``` + + *Ryuta Kamizono* + * Merging conditions on the same column no longer maintain both conditions, and will be consistently replaced by the latter condition in Rails 6.2. To migrate to Rails 6.2's behavior, use `relation.merge(other, rewhere: true)`. diff --git a/activerecord/lib/active_record/querying.rb b/activerecord/lib/active_record/querying.rb index d8b0b50811937..5e0669e5ec7f1 100644 --- a/activerecord/lib/active_record/querying.rb +++ b/activerecord/lib/active_record/querying.rb @@ -13,9 +13,10 @@ module Querying :destroy_all, :delete_all, :update_all, :touch_all, :destroy_by, :delete_by, :find_each, :find_in_batches, :in_batches, :select, :reselect, :order, :reorder, :group, :limit, :offset, :joins, :left_joins, :left_outer_joins, - :where, :rewhere, :preload, :extract_associated, :eager_load, :includes, :from, :lock, :readonly, :extending, :or, - :having, :create_with, :distinct, :references, :none, :unscope, :optimizer_hints, :merge, :except, :only, - :count, :average, :minimum, :maximum, :sum, :calculate, :annotate, + :where, :rewhere, :preload, :extract_associated, :eager_load, :includes, :from, :lock, :readonly, + :and, :or, :annotate, :optimizer_hints, :extending, + :having, :create_with, :distinct, :references, :none, :unscope, :merge, :except, :only, + :count, :average, :minimum, :maximum, :sum, :calculate, :pluck, :pick, :ids, :strict_loading ].freeze # :nodoc: delegate(*QUERYING_METHODS, to: :all) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 7e18238f4c81a..c80f573f01659 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -699,6 +699,38 @@ def rewhere(conditions) scope end + # Returns a new relation, which is the logical intersection of this relation and the one passed + # as an argument. + # + # The two relations must be structurally compatible: they must be scoping the same model, and + # they must differ only by #where (if no #group has been defined) or #having (if a #group is + # present). + # + # Post.where(id: [1, 2]).and(Post.where(id: [2, 3])) + # # SELECT `posts`.* FROM `posts` WHERE `posts`.`id` IN (1, 2) AND `posts`.`id` IN (2, 3) + # + def and(other) + if other.is_a?(Relation) + spawn.and!(other) + else + raise ArgumentError, "You have passed #{other.class.name} object to #and. Pass an ActiveRecord::Relation object instead." + end + end + + def and!(other) # :nodoc: + incompatible_values = structurally_incompatible_values_for_or(other) + + unless incompatible_values.empty? + raise ArgumentError, "Relation passed to #and must be structurally compatible. Incompatible values: #{incompatible_values}" + end + + self.where_clause |= other.where_clause + self.having_clause |= other.having_clause + self.references_values |= other.references_values + + self + end + # Returns a new relation, which is the logical union of this relation and the one passed as an # argument. # @@ -710,11 +742,11 @@ def rewhere(conditions) # # SELECT `posts`.* FROM `posts` WHERE ((id = 1) OR (author_id = 3)) # def or(other) - unless other.is_a? Relation + if other.is_a?(Relation) + spawn.or!(other) + else raise ArgumentError, "You have passed #{other.class.name} object to #or. Pass an ActiveRecord::Relation object instead." end - - spawn.or!(other) end def or!(other) # :nodoc: diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index 02e14fd9b9c14..f6c9100e6dc0e 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -19,6 +19,10 @@ def -(other) WhereClause.new(predicates - other.predicates) end + def |(other) + WhereClause.new(predicates | other.predicates) + end + def merge(other, rewhere = nil) predicates = if rewhere except_predicates(other.extract_attributes) diff --git a/activerecord/test/cases/relation/merging_test.rb b/activerecord/test/cases/relation/merging_test.rb index 8b58f2a6a92e2..a7bf2959ced50 100644 --- a/activerecord/test/cases/relation/merging_test.rb +++ b/activerecord/test/cases/relation/merging_test.rb @@ -14,7 +14,7 @@ class RelationMergingTest < ActiveRecord::TestCase fixtures :developers, :comments, :authors, :author_addresses, :posts def test_merge_in_clause - david, mary, bob = authors(:david, :mary, :bob) + david, mary, bob = authors = authors(:david, :mary, :bob) david_and_mary = Author.where(id: [david, mary]).order(:id) mary_and_bob = Author.where(id: [mary, bob]).order(:id) @@ -35,18 +35,24 @@ def test_merge_in_clause assert_equal [mary, bob], david_and_mary.merge(mary_and_bob) assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true) + assert_equal [mary], david_and_mary.and(mary_and_bob) + assert_equal authors, david_and_mary.or(mary_and_bob) assert_equal [david, mary], mary_and_bob.merge(david_and_mary) assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true) + 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")) + david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")).order(:id) assert_equal [david], david_and_mary.merge(david_and_bob) assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true) + assert_equal [david], david_and_mary.and(david_and_bob) + assert_equal authors, david_and_mary.or(david_and_bob) end def test_merge_between_clause - david, mary, bob = authors(:david, :mary, :bob) + david, mary, bob = authors = authors(:david, :mary, :bob) david_and_mary = Author.where(id: david.id..mary.id).order(:id) mary_and_bob = Author.where(id: mary.id..bob.id).order(:id) @@ -80,20 +86,26 @@ def test_merge_between_clause assert_equal [mary], david_and_mary.merge(mary_and_bob) end assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true) + assert_equal [mary], david_and_mary.and(mary_and_bob) + assert_equal authors, david_and_mary.or(mary_and_bob) assert_deprecated(message) do assert_equal [mary], mary_and_bob.merge(david_and_mary) end assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true) + 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")) + david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")).order(:id) assert_equal [david], david_and_mary.merge(david_and_bob) assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true) + assert_equal [david], david_and_mary.and(david_and_bob) + assert_equal authors, david_and_mary.or(david_and_bob) end def test_merge_or_clause - david, mary, bob = authors(:david, :mary, :bob) + david, mary, bob = authors = authors(:david, :mary, :bob) david_and_mary = Author.where(id: david).or(Author.where(id: mary)).order(:id) mary_and_bob = Author.where(id: mary).or(Author.where(id: bob)).order(:id) @@ -127,16 +139,22 @@ def test_merge_or_clause assert_equal [mary], david_and_mary.merge(mary_and_bob) end assert_equal [mary, bob], david_and_mary.merge(mary_and_bob, rewhere: true) + assert_equal [mary], david_and_mary.and(mary_and_bob) + assert_equal authors, david_and_mary.or(mary_and_bob) assert_deprecated(message) do assert_equal [mary], mary_and_bob.merge(david_and_mary) end assert_equal [david, mary], mary_and_bob.merge(david_and_mary, rewhere: true) + 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")) + david_and_bob = Author.where(id: david).or(Author.where(name: "Bob")).order(:id) assert_equal [david], david_and_mary.merge(david_and_bob) assert_equal [david], david_and_mary.merge(david_and_bob, rewhere: true) + assert_equal [david], david_and_mary.and(david_and_bob) + assert_equal authors, david_and_mary.or(david_and_bob) end def test_merge_not_in_clause