diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/where_clause.rb | 25 | ||||
| -rw-r--r-- | activerecord/test/cases/relation/where_clause_test.rb | 33 |
3 files changed, 53 insertions, 9 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1ff7010c2f..aa581ed1e6 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,7 @@ +* When using #or, extract the common conditions and put them before the OR condition. + + *Maxime Handfield Lapointe* + * `Relation#or` now accepts two relations who have different values for `references` only, as `references` can be implicitly called by `where`. diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb index ef2bca9155..5b68c25bdd 100644 --- a/activerecord/lib/active_record/relation/where_clause.rb +++ b/activerecord/lib/active_record/relation/where_clause.rb @@ -15,6 +15,12 @@ module ActiveRecord ) end + def -(other) + WhereClause.new( + predicates - other.predicates, + ) + end + def merge(other) WhereClause.new( predicates_unreferenced_by(other) + other.predicates, @@ -26,15 +32,16 @@ module ActiveRecord end def or(other) - if empty? - self - elsif other.empty? - other - else - WhereClause.new( - [ast.or(other.ast)], - ) - end + left = self - other + common = self - left + right = other - common + + return common if left.empty? || right.empty? + + or_clause = WhereClause.new( + [left.ast.or(right.ast)] + ) + common + or_clause end def to_h(table_name = nil) diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb index f3a81f3c70..42cf35ac9c 100644 --- a/activerecord/test/cases/relation/where_clause_test.rb +++ b/activerecord/test/cases/relation/where_clause_test.rb @@ -181,6 +181,39 @@ class ActiveRecord::Relation assert_equal WhereClause.empty, WhereClause.empty.or(where_clause) end + test "or places common conditions before the OR" do + wcs = (0..6).map do |i| + WhereClause.new([table["col_#{i}"].eq(bind_param(i))]) + end + + actual = wcs[0] + expected = wcs[0] + + actual = (actual + wcs[1]).or(actual + wcs[2]) + expected = expected + wcs[1].or(wcs[2]) + + actual = (actual + wcs[3] + wcs[4]).or(actual + wcs[5] + wcs[6]) + expected = expected + (wcs[3] + wcs[4]).or(wcs[5] + wcs[6]) + + assert_equal expected, actual + end + + test "or will use common conditions only if one side only has common conditions" do + common = WhereClause.new( + [table["id"].eq(bind_param(1)), "foo = bar"] + ) + + extra = WhereClause.new([table["extra"].eq(bind_param("pluto"))]) + + actual_extra_only_on_left = (common + extra).or(common) + actual_extra_only_on_right = (common).or(common + extra) + + expected = common + + assert_equal expected, actual_extra_only_on_left + assert_equal expected, actual_extra_only_on_right + end + private def table |