aboutsummaryrefslogtreecommitdiffstats
path: root/activerecord
diff options
context:
space:
mode:
Diffstat (limited to 'activerecord')
-rw-r--r--activerecord/CHANGELOG.md15
-rw-r--r--activerecord/lib/active_record/relation/where_clause.rb3
-rw-r--r--activerecord/test/cases/relation/where_clause_test.rb18
3 files changed, 34 insertions, 2 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 1bd0199e4c..7eca9c2e2e 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,18 @@
+* Fix `unscoped(where: [columns])` removing the wrong bind values
+
+ When the `where` is called on a relation after a `or`, unscoping the column of that later `where` removed
+ bind values used by the `or` instead. (possibly other cases too)
+
+ ```
+ Post.where(id: 1).or(Post.where(id: 2)).where(foo: 3).unscope(where: :foo).to_sql
+ # Currently:
+ # SELECT "posts".* FROM "posts" WHERE ("posts"."id" = 2 OR "posts"."id" = 3)
+ # With fix:
+ # SELECT "posts".* FROM "posts" WHERE ("posts"."id" = 1 OR "posts"."id" = 2)
+ ```
+
+ *Maxime Handfield Lapointe*
+
* Values constructed using multi-parameter assignment will now use the
post-type-cast value for rendering in single-field form inputs.
diff --git a/activerecord/lib/active_record/relation/where_clause.rb b/activerecord/lib/active_record/relation/where_clause.rb
index 0feb2c6cb2..731f161eb1 100644
--- a/activerecord/lib/active_record/relation/where_clause.rb
+++ b/activerecord/lib/active_record/relation/where_clause.rb
@@ -136,10 +136,11 @@ module ActiveRecord
binds_index = 0
predicates = self.predicates.reject do |node|
+ binds_contains = node.grep(Arel::Nodes::BindParam).size if node.is_a?(Arel::Nodes::Node)
+
except = \
case node
when Arel::Nodes::Between, Arel::Nodes::In, Arel::Nodes::NotIn, Arel::Nodes::Equality, Arel::Nodes::NotEqual, Arel::Nodes::LessThan, Arel::Nodes::LessThanOrEqual, Arel::Nodes::GreaterThan, Arel::Nodes::GreaterThanOrEqual
- binds_contains = node.grep(Arel::Nodes::BindParam).size
subrelation = (node.left.kind_of?(Arel::Attributes::Attribute) ? node.left : node.right)
columns.include?(subrelation.name.to_s)
end
diff --git a/activerecord/test/cases/relation/where_clause_test.rb b/activerecord/test/cases/relation/where_clause_test.rb
index f8eb0dee91..57d7b315b0 100644
--- a/activerecord/test/cases/relation/where_clause_test.rb
+++ b/activerecord/test/cases/relation/where_clause_test.rb
@@ -97,7 +97,7 @@ class ActiveRecord::Relation
assert_equal expected, original.invert
end
- test "accept removes binary predicates referencing a given column" do
+ test "except removes binary predicates referencing a given column" do
where_clause = WhereClause.new([
table["id"].in([1, 2, 3]),
table["name"].eq(bind_param),
@@ -111,6 +111,22 @@ class ActiveRecord::Relation
assert_equal expected, where_clause.except("id", "name")
end
+ test "except jumps over unhandled binds (like with OR) correctly" do
+ wcs = (0..9).map do |i|
+ WhereClause.new([table["id#{i}"].eq(bind_param)], [bind_attribute("id#{i}", i)])
+ end
+
+ wc = wcs[0] + wcs[1] + wcs[2].or(wcs[3]) + wcs[4] + wcs[5] + wcs[6].or(wcs[7]) + wcs[8] + wcs[9]
+
+ expected = wcs[0] + wcs[2].or(wcs[3]) + wcs[5] + wcs[6].or(wcs[7]) + wcs[9]
+ actual = wc.except("id1", "id2", "id4", "id7", "id8")
+
+ # Easier to read than the inspect of where_clause
+ assert_equal expected.ast.to_sql, actual.ast.to_sql
+ assert_equal expected.binds.map(&:value), actual.binds.map(&:value)
+ assert_equal expected, actual
+ end
+
test "ast groups its predicates with AND" do
predicates = [
table["id"].in([1, 2, 3]),