aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
authorMaxime Lapointe <hunter_spawn@hotmail.com>2017-07-25 14:00:39 -0400
committerMaxime Lapointe <hunter_spawn@hotmail.com>2017-07-25 22:17:47 -0400
commit110e0e1fcceab68716e0c75d87baffb14403b288 (patch)
treea41c218554b33a6aca16e66965d6c8d061e0c6d3 /activerecord
parent6f9b01c056cd2f3a4761baf78df207e1154f1b06 (diff)
downloadrails-110e0e1fcceab68716e0c75d87baffb14403b288.tar.gz
rails-110e0e1fcceab68716e0c75d87baffb14403b288.tar.bz2
rails-110e0e1fcceab68716e0c75d87baffb14403b288.zip
Avoid duplicate clauses when using #or
Condenses the clauses that are common to both sides of the OR and put them outside, before the OR This fix the current behavior where the number of conditions is exponential based on the number of times #or is used.
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md4
-rw-r--r--activerecord/lib/active_record/relation/where_clause.rb25
-rw-r--r--activerecord/test/cases/relation/where_clause_test.rb33
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