Skip to content

Commit

Permalink
Support relation.and for intersection as Set theory
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kamipo committed Jun 7, 2020
1 parent 159cc34 commit 7219eb2
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 12 deletions.
14 changes: 14 additions & 0 deletions 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)`.
Expand Down
7 changes: 4 additions & 3 deletions activerecord/lib/active_record/querying.rb
Expand Up @@ -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)
Expand Down
38 changes: 35 additions & 3 deletions activerecord/lib/active_record/relation/query_methods.rb
Expand Up @@ -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.
#
Expand All @@ -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:
Expand Down
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/relation/where_clause.rb
Expand Up @@ -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)
Expand Down
30 changes: 24 additions & 6 deletions activerecord/test/cases/relation/merging_test.rb
Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 7219eb2

Please sign in to comment.