`where.not` now generates NAND predicates instead of NOR

This commit is contained in:
Rafael Mendonça França 2020-05-07 00:00:27 -04:00
parent b6d62491e4
commit 688a1c9d1d
No known key found for this signature in database
GPG Key ID: FC23B6D0F1EEE948
6 changed files with 52 additions and 79 deletions

View File

@ -1,3 +1,17 @@
* `where.not` now generates NAND predicates instead of NOR.
Before:
User.where.not(name: "Jon", role: "admin")
# SELECT * FROM users WHERE name != 'Jon' AND role != 'admin'
After:
User.where.not(name: "Jon", role: "admin")
# SELECT * FROM users WHERE NOT (name == 'Jon' AND role == 'admin')
*Rafael Mendonça França*
* Remove deprecated `ActiveRecord::Result#to_hash` method. * Remove deprecated `ActiveRecord::Result#to_hash` method.
*Rafael Mendonça França* *Rafael Mendonça França*

View File

@ -39,27 +39,13 @@ module ActiveRecord
# #
# User.where.not(name: %w(Ko1 Nobu)) # User.where.not(name: %w(Ko1 Nobu))
# # SELECT * FROM users WHERE name NOT IN ('Ko1', 'Nobu') # # SELECT * FROM users WHERE name NOT IN ('Ko1', 'Nobu')
#
# User.where.not(name: "Jon", role: "admin")
# # SELECT * FROM users WHERE NOT (name == 'Jon' AND role == 'admin')
def not(opts, *rest) def not(opts, *rest)
where_clause = @scope.send(:build_where_clause, opts, rest) where_clause = @scope.send(:build_where_clause, opts, rest)
if not_behaves_as_nor?(opts) @scope.where_clause += where_clause.invert
ActiveSupport::Deprecation.warn(<<~MSG.squish)
NOT conditions will no longer behave as NOR in Rails 6.1.
To continue using NOR conditions, NOT each condition individually
(`#{
opts.flat_map { |key, value|
if value.is_a?(Hash) && value.size > 1
value.map { |k, v| ".where.not(#{key.inspect} => { #{k.inspect} => ... })" }
else
".where.not(#{key.inspect} => ...)"
end
}.join
}`).
MSG
@scope.where_clause += where_clause.invert(:nor)
else
@scope.where_clause += where_clause.invert
end
@scope @scope
end end
@ -92,14 +78,6 @@ module ActiveRecord
@scope @scope
end end
private
def not_behaves_as_nor?(opts)
return false unless opts.is_a?(Hash)
opts.any? { |k, v| v.is_a?(Hash) && v.size > 1 } ||
opts.size > 1
end
end end
FROZEN_EMPTY_ARRAY = [].freeze FROZEN_EMPTY_ARRAY = [].freeze

View File

@ -77,11 +77,9 @@ module ActiveRecord
predicates == other.predicates predicates == other.predicates
end end
def invert(as = :nand) def invert
if predicates.size == 1 if predicates.size == 1
inverted_predicates = [ invert_predicate(predicates.first) ] inverted_predicates = [ invert_predicate(predicates.first) ]
elsif as == :nor
inverted_predicates = predicates.map { |node| invert_predicate(node) }
else else
inverted_predicates = [ Arel::Nodes::Not.new(ast) ] inverted_predicates = [ Arel::Nodes::Not.new(ast) ]
end end

View File

@ -87,7 +87,7 @@ class ActiveRecord::Relation
end end
end end
test "invert replaces each part of the predicate with its inverse" do test "invert wraps the ast inside a NAND node" do
original = WhereClause.new([ original = WhereClause.new([
table["id"].in([1, 2, 3]), table["id"].in([1, 2, 3]),
table["id"].not_in([1, 2, 3]), table["id"].not_in([1, 2, 3]),
@ -102,20 +102,24 @@ class ActiveRecord::Relation
"sql literal" "sql literal"
]) ])
expected = WhereClause.new([ expected = WhereClause.new([
table["id"].not_in([1, 2, 3]), Arel::Nodes::Not.new(
table["id"].in([1, 2, 3]), Arel::Nodes::And.new([
table["id"].not_eq(1), table["id"].in([1, 2, 3]),
table["id"].eq(2), table["id"].not_in([1, 2, 3]),
table["id"].lteq(1), table["id"].eq(1),
table["id"].lt(2), table["id"].not_eq(2),
table["id"].gteq(1), table["id"].gt(1),
table["id"].gt(2), table["id"].gteq(2),
table["id"].is_distinct_from(1), table["id"].lt(1),
table["id"].is_not_distinct_from(2), table["id"].lteq(2),
Arel::Nodes::Not.new(Arel::Nodes::SqlLiteral.new("sql literal")) table["id"].is_not_distinct_from(1),
table["id"].is_distinct_from(2),
Arel::Nodes::Grouping.new("sql literal")
])
)
]) ])
assert_equal expected, original.invert(:nor) assert_equal expected, original.invert
end end
test "except removes binary predicates referencing a given column" do test "except removes binary predicates referencing a given column" do

View File

@ -158,11 +158,7 @@ module ActiveRecord
all = [treasures(:diamond), sapphire, cars(:honda), sapphire] all = [treasures(:diamond), sapphire, cars(:honda), sapphire]
assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of) assert_equal all, PriceEstimate.all.sort_by(&:id).map(&:estimate_of)
actual = PriceEstimate.where.yield_self do |where_chain| actual = PriceEstimate.where.not(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id)
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) only = PriceEstimate.where(estimate_of_type: sapphire.class.polymorphic_name, estimate_of_id: sapphire.id)
expected = all - [sapphire] expected = all - [sapphire]
@ -170,43 +166,14 @@ module ActiveRecord
assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of) assert_equal all - expected, only.sort_by(&:id).map(&:estimate_of)
end end
def test_where_not_polymorphic_id_and_type_as_nor_is_deprecated def test_where_not_association_as_nand
sapphire = treasures(:sapphire) 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 condition individually
(`.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_where_not_association_as_nor_is_deprecated
treasure = Treasure.create!(name: "my_treasure") treasure = Treasure.create!(name: "my_treasure")
PriceEstimate.create!(estimate_of: treasure, price: 2, currency: "USD") PriceEstimate.create!(estimate_of: treasure, price: 2, currency: "USD")
PriceEstimate.create!(estimate_of: treasure, price: 2, currency: "EUR")
message = <<~MSG.squish result = Treasure.joins(:price_estimates).where.not(price_estimates: { price: 2, currency: "USD" })
NOT conditions will no longer behave as NOR in Rails 6.1.
To continue using NOR conditions, NOT each condition individually
(`.where.not(:price_estimates => { :price => ... }).where.not(:price_estimates => { :currency => ... })`).
MSG
assert_deprecated(message) do
result = Treasure.joins(:price_estimates).where.not(price_estimates: { price: 2, currency: "USD" })
assert_predicate result, :empty? assert_equal [treasures(:diamond), sapphire, sapphire], result
end
end end
def test_polymorphic_nested_array_where def test_polymorphic_nested_array_where

View File

@ -159,6 +159,18 @@ Please refer to the [Changelog][active-record] for detailed changes.
### Notable changes ### Notable changes
* `where.not` now generates NAND predicates instead of NOR.
Before:
User.where.not(name: "Jon", role: "admin")
# SELECT * FROM users WHERE name != 'Jon' AND role != 'admin'
After:
User.where.not(name: "Jon", role: "admin")
# SELECT * FROM users WHERE NOT (name == 'Jon' AND role == 'admin')
Active Storage Active Storage
-------------- --------------