Permalink
Browse files

Merge Pull Request #16052 Added #or to ActiveRecord::Relation

  • Loading branch information...
sgrif committed Jan 28, 2015
2 parents 96ac14a + ff45b9e commit 9e42cf019f2417473e7dcbfcb885709fa2709f89
View
@@ -1,3 +1,13 @@
+* Added the `#or` method on ActiveRecord::Relation, allowing use of the OR
+ operator to combine WHERE or HAVING clauses.
+
+ Example:
+
+ Post.where('id = 1').or(Post.where('id = 2'))
+ # => SELECT * FROM posts WHERE (id = 1) OR (id = 2)
+
+ *Sean Griffin*, *Matthew Draper*, *Gael Muller*, *Olivier El Mekki*
+
* Don't define autosave association callbacks twice from
`accepts_nested_attributes_for`.
@@ -75,5 +75,9 @@ def calculate(operation, _column_name)
def exists?(_id = false)
false
end
+
+ def or(other)
+ other.spawn
+ end
end
end
@@ -7,7 +7,7 @@ module Querying
delegate :find_by, :find_by!, to: :all
delegate :destroy, :destroy_all, :delete, :delete_all, :update, :update_all, to: :all
delegate :find_each, :find_in_batches, to: :all
- delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins,
+ delegate :select, :group, :order, :except, :reorder, :limit, :offset, :joins, :or,
:where, :rewhere, :preload, :eager_load, :includes, :from, :lock, :readonly,
:having, :create_with, :uniq, :distinct, :references, :none, :unscope, to: :all
delegate :count, :average, :minimum, :maximum, :sum, :calculate, to: :all
@@ -582,6 +582,37 @@ def rewhere(conditions)
unscope(where: conditions.keys).where(conditions)
end
+ # Returns a new relation, which is the logical union 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). Neither relation may have a +limit+, +offset+, or +uniq+ set.
+ #
+ # Post.where("id = 1").or(Post.where("id = 2"))
+ # # SELECT `posts`.* FROM `posts` WHERE (('id = 1' OR 'id = 2'))
+ #
+ def or(other)
+ spawn.or!(other)
+ end
+
+ def or!(other) # :nodoc:
+ unless structurally_compatible_for_or?(other)
+ raise ArgumentError, 'Relation passed to #or must be structurally compatible'
+ end
+
+ self.where_clause = self.where_clause.or(other.where_clause)
+ self.having_clause = self.having_clause.or(other.having_clause)
+
+ self
+ end
+
+ private def structurally_compatible_for_or?(other) # :nodoc:

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jan 29, 2015

Member

Not cool bro 😢

@rafaelfranca

rafaelfranca Jan 29, 2015

Member

Not cool bro 😢

This comment has been minimized.

Show comment
Hide comment
@rafaelfranca

rafaelfranca Jan 29, 2015

Member

Why the nodoc?

@rafaelfranca

rafaelfranca Jan 29, 2015

Member

Why the nodoc?

+ Relation::SINGLE_VALUE_METHODS.all? { |m| send("#{m}_value") == other.send("#{m}_value") } &&
+ (Relation::MULTI_VALUE_METHODS - [:extending]).all? { |m| send("#{m}_values") == other.send("#{m}_values") } &&
+ (Relation::CLAUSE_METHODS - [:having, :where]).all? { |m| send("#{m}_clause") != other.send("#{m}_clause") }
+ end
+
# Allows to specify a HAVING clause. Note that you can't use HAVING
# without also specifying a GROUP clause.
#
@@ -31,6 +31,19 @@ def except(*columns)
)
end
+ def or(other)
+ if empty?
+ other
+ elsif other.empty?
+ self
+ else
+ WhereClause.new(
+ [ast.or(other.ast)],
+ binds + other.binds
+ )
+ end
+ end
+
def to_h(table_name = nil)
equalities = predicates.grep(Arel::Nodes::Equality)
if table_name
@@ -0,0 +1,84 @@
+require "cases/helper"
+require 'models/post'
+
+module ActiveRecord
+ class OrTest < ActiveRecord::TestCase
+ fixtures :posts
+
+ def test_or_with_relation
+ expected = Post.where('id = 1 or id = 2').to_a
+ assert_equal expected, Post.where('id = 1').or(Post.where('id = 2')).to_a
+ end
+
+ def test_or_identity
+ expected = Post.where('id = 1').to_a
+ assert_equal expected, Post.where('id = 1').or(Post.where('id = 1')).to_a
+ end
+
+ def test_or_with_null_left
+ expected = Post.where('id = 1').to_a
+ assert_equal expected, Post.none.or(Post.where('id = 1')).to_a
+ end
+
+ def test_or_with_null_right
+ expected = Post.where('id = 1').to_a
+ assert_equal expected, Post.where('id = 1').or(Post.none).to_a
+ end
+
+ def test_or_with_bind_params
+ assert_equal Post.find([1, 2]), Post.where(id: 1).or(Post.where(id: 2)).to_a
+ end
+
+ def test_or_with_null_both
+ expected = Post.none.to_a
+ assert_equal expected, Post.none.or(Post.none).to_a
+ end
+
+ def test_or_without_left_where
+ expected = Post.where('id = 1')
+ assert_equal expected, Post.or(Post.where('id = 1')).to_a
+ end
+
+ def test_or_without_right_where
+ expected = Post.where('id = 1')
+ assert_equal expected, Post.where('id = 1').or(Post.all).to_a
+ end
+
+ def test_or_preserves_other_querying_methods
+ expected = Post.where('id = 1 or id = 2 or id = 3').order('body asc').to_a
+ partial = Post.order('body asc')
+ assert_equal expected, partial.where('id = 1').or(partial.where(:id => [2, 3])).to_a
+ assert_equal expected, Post.order('body asc').where('id = 1').or(Post.order('body asc').where(:id => [2, 3])).to_a
+ end
+
+ def test_or_with_incompatible_relations
+ assert_raises ArgumentError do
+ Post.order('body asc').where('id = 1').or(Post.order('id desc').where(:id => [2, 3])).to_a
+ end
+ end
+
+ def test_or_when_grouping
+ groups = Post.where('id < 10').group('body').select('body, COUNT(*) AS c')
+ expected = groups.having("COUNT(*) > 1 OR body like 'Such%'").to_a.map {|o| [o.body, o.c] }
+ assert_equal expected, groups.having('COUNT(*) > 1').or(groups.having("body like 'Such%'")).to_a.map {|o| [o.body, o.c] }
+ end
+
+ def test_or_with_named_scope
+ expected = Post.where("id = 1 or body LIKE '\%a\%'").to_a
+ assert_equal expected, Post.where('id = 1').or(Post.containing_the_letter_a)
+ end
+
+ def test_or_inside_named_scope
+ expected = Post.where("body LIKE '\%a\%' OR title LIKE ?", "%'%").order('id DESC').to_a
+ assert_equal expected, Post.order(id: :desc).typographically_interesting
+ end
+
+ def test_or_on_loaded_relation
+ expected = Post.where('id = 1 or id = 2').to_a
+ p = Post.where('id = 1')
+ p.load
+ assert_equal p.loaded?, true
+ assert_equal expected, p.or(Post.where('id = 2')).to_a
+ end
+ end
+end
@@ -145,6 +145,26 @@ class WhereClauseTest < ActiveRecord::TestCase
assert_equal where_clause.ast, where_clause_with_empty.ast
end
+ test "or joins the two clauses using OR" do
+ where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)])
+ other_clause = WhereClause.new([table["name"].eq(bind_param)], [attribute("name", "Sean")])
+ expected_ast =
+ Arel::Nodes::Grouping.new(
+ Arel::Nodes::Or.new(table["id"].eq(bind_param), table["name"].eq(bind_param))
+ )
+ expected_binds = where_clause.binds + other_clause.binds
+
+ assert_equal expected_ast.to_sql, where_clause.or(other_clause).ast.to_sql
+ assert_equal expected_binds, where_clause.or(other_clause).binds
+ end
+
+ test "or does nothing with an empty where clause" do
+ where_clause = WhereClause.new([table["id"].eq(bind_param)], [attribute("id", 1)])
+
+ assert_equal where_clause, where_clause.or(WhereClause.empty)
+ assert_equal where_clause, WhereClause.empty.or(where_clause)
+ end
+
private
def table
@@ -18,6 +18,7 @@ def greeting
end
scope :containing_the_letter_a, -> { where("body LIKE '%a%'") }
+ scope :titled_with_an_apostrophe, -> { where("title LIKE '%''%'") }
scope :ranked_by_comments, -> { order("comments_count DESC") }
scope :limit_by, lambda {|l| limit(l) }
@@ -43,6 +44,8 @@ def first_comment
scope :tagged_with, ->(id) { joins(:taggings).where(taggings: { tag_id: id }) }
scope :tagged_with_comment, ->(comment) { joins(:taggings).where(taggings: { comment: comment }) }
+ scope :typographically_interesting, -> { containing_the_letter_a.or(titled_with_an_apostrophe) }
+
has_many :comments do
def find_most_recent
order("id DESC").first

33 comments on commit 9e42cf0

@gabebw

This comment has been minimized.

Show comment
Hide comment
@gabebw

gabebw Jan 29, 2015

Contributor

<3

Contributor

gabebw replied Jan 29, 2015

<3

@jonatack

This comment has been minimized.

Show comment
Hide comment
@jonatack

jonatack Jan 29, 2015

Contributor

Thanks Gael, Olivier, Matthew and Sean for your work on this 🎉 looking forward to giving it a try with scopes/class methods.

Contributor

jonatack replied Jan 29, 2015

Thanks Gael, Olivier, Matthew and Sean for your work on this 🎉 looking forward to giving it a try with scopes/class methods.

@mcmire

This comment has been minimized.

Show comment
Hide comment
@mcmire

mcmire Jan 29, 2015

Contributor

I am surprised at what a small commit this is!

Contributor

mcmire replied Jan 29, 2015

I am surprised at what a small commit this is!

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Jan 29, 2015

Member

That's because I've been refactoring to make changes like this trivial for a few weeks.

Member

sgrif replied Jan 29, 2015

That's because I've been refactoring to make changes like this trivial for a few weeks.

@mdespuits

This comment has been minimized.

Show comment
Hide comment
@mdespuits

mdespuits Jan 29, 2015

Contributor

👍 😄 Been waiting for this for a few years. Thanks guys!

Contributor

mdespuits replied Jan 29, 2015

👍 😄 Been waiting for this for a few years. Thanks guys!

@nshoes

This comment has been minimized.

Show comment
Hide comment

👍

@kelvinst

This comment has been minimized.

Show comment
Hide comment

👍 😍

@teknull

This comment has been minimized.

Show comment
Hide comment
@teknull

teknull Jan 29, 2015

This is great!

This is great!

@mechanicles

This comment has been minimized.

Show comment
Hide comment
@mechanicles

mechanicles Jan 30, 2015

Contributor

Awesome 😍 One of the most required features in active_record :)

Contributor

mechanicles replied Jan 30, 2015

Awesome 😍 One of the most required features in active_record :)

@mentero

This comment has been minimized.

Show comment
Hide comment
@JanStevens

This comment has been minimized.

Show comment
Hide comment
@JanStevens

JanStevens Jan 30, 2015

How about combining and and or ? How should one do that? Can we nest or's and and's togheter?

How about combining and and or ? How should one do that? Can we nest or's and and's togheter?

@emaiax

This comment has been minimized.

Show comment
Hide comment
@emaiax

emaiax Jan 30, 2015

completely awesome! 😍 😎

completely awesome! 😍 😎

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Jan 30, 2015

Member

@JanStevens Yes, you can combine as many operations as you'd like, just like all the other methods on Relation

Member

sgrif replied Jan 30, 2015

@JanStevens Yes, you can combine as many operations as you'd like, just like all the other methods on Relation

@seuros

This comment has been minimized.

Show comment
Hide comment
@seuros

seuros Jan 30, 2015

Member

💚 💚 💚 💚 💚 💚

Member

seuros replied Jan 30, 2015

💚 💚 💚 💚 💚 💚

@jordpo

This comment has been minimized.

Show comment
Hide comment
@jordpo

jordpo Feb 1, 2015

awesome - excited to use this!

awesome - excited to use this!

@classyPimp

This comment has been minimized.

Show comment
Hide comment
@classyPimp

classyPimp Feb 1, 2015

i'm newb, please someboy tell me how can i use this feature? shall i update smthing (im newb to Rails). thanks! p.s. i was just astonished that Rails didnt have OR operator for ORM/ and that's a really great commit!

i'm newb, please someboy tell me how can i use this feature? shall i update smthing (im newb to Rails). thanks! p.s. i was just astonished that Rails didnt have OR operator for ORM/ and that's a really great commit!

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Feb 1, 2015

Member

@classyPimp This will be part of Rails 5.0, which will ship in late 2015.

Member

sgrif replied Feb 1, 2015

@classyPimp This will be part of Rails 5.0, which will ship in late 2015.

@korny

This comment has been minimized.

Show comment
Hide comment
@korny

korny Feb 2, 2015

Contributor

The definition of scope :typographically_interesting using two other scopes is really interesting. Is there a way to combine two scopes using AND? If not, an .and method would be useful, too…

Contributor

korny replied Feb 2, 2015

The definition of scope :typographically_interesting using two other scopes is really interesting. Is there a way to combine two scopes using AND? If not, an .and method would be useful, too…

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Feb 2, 2015

Member

@korny Post.typographcially_interesting.containing_the_letter_a would combine them with AND. Or to be more specific, where adds conditions using AND.

Member

sgrif replied Feb 2, 2015

@korny Post.typographcially_interesting.containing_the_letter_a would combine them with AND. Or to be more specific, where adds conditions using AND.

@korny

This comment has been minimized.

Show comment
Hide comment
@korny

korny Feb 2, 2015

Contributor

Okay, I was missing the obvious 😊

Contributor

korny replied Feb 2, 2015

Okay, I was missing the obvious 😊

@dkonayuki

This comment has been minimized.

Show comment
Hide comment
@dkonayuki

dkonayuki Mar 7, 2015

👍 definitely gonna use this.

👍 definitely gonna use this.

@keepcosmos

This comment has been minimized.

Show comment
Hide comment
@keepcosmos

keepcosmos Mar 7, 2015

Contributor

👍

Contributor

keepcosmos replied Mar 7, 2015

👍

@anandhak

This comment has been minimized.

Show comment
Hide comment

👍

@rmrgh

This comment has been minimized.

Show comment
Hide comment

👍

@andre1810

This comment has been minimized.

Show comment
Hide comment

👍

@kyuden

This comment has been minimized.

Show comment
Hide comment
@kyuden

kyuden Sep 1, 2015

Contributor

👍

Contributor

kyuden replied Sep 1, 2015

👍

@bf4

This comment has been minimized.

Show comment
Hide comment
@bf4

bf4 Sep 1, 2015

Contributor

FWIW, I've backported this to Rails 4.2.3 and am sharing since it was non-trivial https://gist.github.com/bf4/84cff9cc6ac8489d769e

Contributor

bf4 replied Sep 1, 2015

FWIW, I've backported this to Rails 4.2.3 and am sharing since it was non-trivial https://gist.github.com/bf4/84cff9cc6ac8489d769e

@ClaudioFloreani

This comment has been minimized.

Show comment
Hide comment
@mcmire

This comment has been minimized.

Show comment
Hide comment
@mcmire

mcmire Sep 24, 2015

Contributor

No need to 👍 this commit, folks -- everyone who has commented or who has starred this repo gets a notification.

Contributor

mcmire replied Sep 24, 2015

No need to 👍 this commit, folks -- everyone who has commented or who has starred this repo gets a notification.

@sgrif

This comment has been minimized.

Show comment
Hide comment
@sgrif

sgrif Sep 24, 2015

Member

👍 :trollface:

Member

sgrif replied Sep 24, 2015

👍 :trollface:

@dogyearm

This comment has been minimized.

Show comment
Hide comment

👍

@unused

This comment has been minimized.

Show comment
Hide comment

👍

@sampatbadhe

This comment has been minimized.

Show comment
Hide comment

👍

Please sign in to comment.