1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Deprecate where.not working as NOR and will be changed to NAND in Rails 6.1

`where.not` with polymorphic association is partly fixed incidentally at
213796f (refer #33493, #26207, #17010, #16983, #14161), and I've added
test case e9ba12f to avoid lose that fix accidentally in the future.

In Rails 5.2, `where.not(polymorphic: object)` works as expected as
NAND, but `where.not(polymorphic_type: object.class.polymorphic_name,
polymorphic_id: object.id)` still unexpectedly works as NOR.

To will make `where.not` working desiredly as NAND in Rails 6.1, this
deprecates `where.not` working as NOR. If people want to continue NOR
conditions, we'd encourage to them to `where.not` each conditions
manually.

```ruby
all = [treasures(:diamond), treasures(:sapphire), cars(:honda), treasures(:sapphire)]
assert_equal all, PriceEstimate.all.map(&:estimate_of)
```

In Rails 6.0:

```ruby
sapphire = treasures(:sapphire)

nor = all.reject { |e|
  e.estimate_of_type == sapphire.class.polymorphic_name
}.reject { |e|
  e.estimate_of_id == sapphire.id
}
assert_equal [cars(:honda)], nor

without_sapphire = PriceEstimate.where.not(
  estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id
)
assert_equal nor, without_sapphire.map(&:estimate_of)
```

In Rails 6.1:

```ruby
sapphire = treasures(:sapphire)

nand = all - [sapphire]
assert_equal [treasures(:diamond), cars(:honda)], nand

without_sapphire = PriceEstimate.where.not(
  estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id
)
assert_equal nand, without_sapphire.map(&:estimate_of)
```

Resolves #31209.
This commit is contained in:
Ryuta Kamizono 2019-04-09 23:25:30 +09:00
parent 16dae7684e
commit 12a9664ff6
5 changed files with 118 additions and 15 deletions

View file

@ -1,3 +1,44 @@
* Deprecate `where.not` working as NOR and will be changed to NAND in Rails 6.1.
```ruby
all = [treasures(:diamond), treasures(:sapphire), cars(:honda), treasures(:sapphire)]
assert_equal all, PriceEstimate.all.map(&:estimate_of)
```
In Rails 6.0:
```ruby
sapphire = treasures(:sapphire)
nor = all.reject { |e|
e.estimate_of_type == sapphire.class.polymorphic_name
}.reject { |e|
e.estimate_of_id == sapphire.id
}
assert_equal [cars(:honda)], nor
without_sapphire = PriceEstimate.where.not(
estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id
)
assert_equal nor, without_sapphire.map(&:estimate_of)
```
In Rails 6.1:
```ruby
sapphire = treasures(:sapphire)
nand = all - [sapphire]
assert_equal [treasures(:diamond), cars(:honda)], nand
without_sapphire = PriceEstimate.where.not(
estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id
)
assert_equal nand, without_sapphire.map(&:estimate_of)
```
*Ryuta Kamizono*
* Fix dirty tracking after rollback.
Fixes #15018, #30167, #33868.

View file

@ -41,18 +41,31 @@ module ActiveRecord
#
# User.where.not(name: %w(Ko1 Nobu))
# # SELECT * FROM users WHERE name NOT IN ('Ko1', 'Nobu')
#
# User.where.not(name: "Jon", role: "admin")
# # SELECT * FROM users WHERE name != 'Jon' AND role != 'admin'
def not(opts, *rest)
opts = sanitize_forbidden_attributes(opts)
where_clause = @scope.send(:where_clause_factory).build(opts, rest)
@scope.references!(PredicateBuilder.references(opts)) if Hash === opts
if not_behaves_as_nor?(opts)
ActiveSupport::Deprecation.warn(<<~MSG.squish)
NOT conditions will no longer behave as NOR in Rails 6.1.
To continue using NOR conditions, NOT each conditions manually
(`#{ opts.keys.map { |key| ".where.not(#{key.inspect} => ...)" }.join }`).
MSG
@scope.where_clause += where_clause.invert(:nor)
else
@scope.where_clause += where_clause.invert
end
@scope
end
private
def not_behaves_as_nor?(opts)
opts.is_a?(Hash) && opts.size > 1
end
end
FROZEN_EMPTY_ARRAY = [].freeze

View file

@ -70,7 +70,15 @@ module ActiveRecord
predicates == other.predicates
end
def invert
def invert(as = :nand)
if predicates.size == 1
inverted_predicates = [ invert_predicate(predicates.first) ]
elsif as == :nor
inverted_predicates = predicates.map { |node| invert_predicate(node) }
else
inverted_predicates = [ Arel::Nodes::Not.new(ast) ]
end
WhereClause.new(inverted_predicates)
end
@ -115,10 +123,6 @@ module ActiveRecord
node.respond_to?(:operator) && node.operator == :==
end
def inverted_predicates
predicates.map { |node| invert_predicate(node) }
end
def invert_predicate(node)
case node
when NilClass

View file

@ -106,7 +106,7 @@ class ActiveRecord::Relation
Arel::Nodes::Not.new(random_object)
])
assert_equal expected, original.invert
assert_equal expected, original.invert(:nor)
end
test "except removes binary predicates referencing a given column" do

View file

@ -115,13 +115,58 @@ module ActiveRecord
assert_equal expected.to_sql, actual.to_sql
end
def test_polymorphic_shallow_where_not
treasure = treasures(:sapphire)
def test_where_not_polymorphic_association
sapphire = treasures(:sapphire)
expected = [price_estimates(:diamond), price_estimates(:honda)]
actual = PriceEstimate.where.not(estimate_of: treasure)
all = [treasures(:diamond), sapphire, cars(:honda), sapphire]
assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of)
assert_equal expected.sort_by(&:id), actual.sort_by(&:id)
actual = PriceEstimate.where.not(estimate_of: sapphire)
only = PriceEstimate.where(estimate_of: sapphire)
expected = all - [sapphire]
assert_equal expected, actual.sort_by(&:id).map(&:estimate_of)
assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of)
end
def test_where_not_polymorphic_id_and_type_as_nand
sapphire = treasures(:sapphire)
all = [treasures(:diamond), sapphire, cars(:honda), sapphire]
assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of)
actual = PriceEstimate.where.yield_self do |where_chain|
where_chain.stub(:not_behaves_as_nor?, false) do
where_chain.not(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id)
end
end
only = PriceEstimate.where(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id)
expected = all - [sapphire]
assert_equal expected, actual.sort_by(&:id).map(&:estimate_of)
assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of)
end
def test_where_not_polymorphic_id_and_type_as_nor_is_deprecated
sapphire = treasures(:sapphire)
all = [treasures(:diamond), sapphire, cars(:honda), sapphire]
assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of)
message = <<~MSG.squish
NOT conditions will no longer behave as NOR in Rails 6.1.
To continue using NOR conditions, NOT each conditions manually
(`.where.not(:estimate_of_type => ...).where.not(:estimate_of_id => ...)`).
MSG
actual = assert_deprecated(message) do
PriceEstimate.where.not(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id)
end
only = PriceEstimate.where(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id)
expected = all - [sapphire]
# NOT (estimate_of_type = 'Treasure' OR estimate_of_id = sapphire.id) matches only `cars(:honda)` unfortunately.
assert_not_equal expected, actual.sort_by(&:id).map(&:estimate_of)
assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of)
end
def test_polymorphic_nested_array_where