Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP
Browse files

Update :in comparison to handle nil values mixed into normal values

*  In DataMapper the assumption is that the following is always true:

    Model.all == Model.all(:name => value) | Model.all(:name.not => value)

  The previous code would not work properly if nil values were mixed
  in with normal values. It would've created a clause like:

    WHERE name IN(NULL, 'Dan')

  Which only works if name can never be NULL. If it can, then those
  will be excluded from the results. What you want to generate is:

    WHERE name IN('Dan') OR name IS NULL

  If the above is negated, then the following can be generated:

    WHERE NOT(name IN('Dan') OR name IS NULL)

  The union of these two queries should equal the original rows.
  • Loading branch information...
commit 5439380b60875401f89405aea98cfde9d267eb10 1 parent 590a881
@dkubb dkubb authored
View
1  dm-do-adapter.gemspec
@@ -59,4 +59,3 @@ Gem::Specification.new do |s|
s.add_dependency(%q<rspec>, ["~> 1.3.2"])
end
end
-
View
73 lib/dm-do-adapter/adapter.rb
@@ -15,6 +15,8 @@ class DataObjectsAdapter < AbstractAdapter
extend Chainable
extend Deprecate
+ SQL_FALSE = '1 = 0'.freeze
+
deprecate :query, :select
# Retrieve results using an SQL SELECT statement
@@ -592,7 +594,7 @@ def subquery_execute(query, source_key, target_key, qualify)
if conditions.valid?
conditions_statement(conditions, qualify)
else
- [ '1 = 0', [] ]
+ [ SQL_FALSE, [] ]
end
end
@@ -690,21 +692,42 @@ def comparison_statement(comparison, qualify)
else
return conditions_statement(comparison.foreign_key_mapping, qualify)
end
- elsif comparison.slug == :in && empty_comparison?(value)
- # An "in" clause with an empty list can be evaluated two ways:
- #
- # * when negated, it means match everything
- # * when not negated, it means match nothing
- #
- # These semantics can be explained with the following ruby examples:
- #
- # * ! [].include?(1) # => true
- # * [].include?(1) # => false
- #
- # In two-valued logic the statement "does the value not match an
- # empty set" is always true. Conversely the statement "does the
- # value match an empty set" is always false.
- return []
+ elsif comparison.slug == :in
+ nil_values, other_values = value.partition { |entry| entry.nil? }
+
+ if nil_values.empty? and other_values.empty?
+ # The "in" clause is an empty list. This can be evaluated two
+ # ways:
+ #
+ # * when not negated, it means: match nothing
+ # * when negated, it means: match everything
+ #
+ # These semantics can be explained with the following ruby examples:
+ #
+ # * [].include?(any_value) # => false
+ # * ! [].include?(any_value) # => true
+ #
+ # In two-valued logic the statement "does the value match an
+ # empty set" is always false. Conversely the statement "does the
+ # value not match an empty set" is always true.
+ #
+ # This returns an SQL statement that is always false, and it may
+ # be negated by an outer negation operator in the case where we
+ # are looking for "a value not in an empty set".
+ return SQL_FALSE, []
+ elsif nil_values.empty?
+ # the "in" clause contains non-nil values, so the normal code path
+ # can handle this condition.
+ else
+ # the "in" clause contains a nil value, so create a query that
+ # handles mixed nil and non-nil values.
+ disjunction = Query::Conditions::Operation.new(:or,
+ Query::Conditions::Comparison.new(:in, subject, other_values),
+ Query::Conditions::Comparison.new(:eql, subject, nil)
+ )
+
+ return conditions_statement(disjunction, qualify)
+ end
end
operator = comparison_operator(comparison)
@@ -765,24 +788,6 @@ def quote_name(name)
end
- # Test if the value is empty
- #
- # If the value contains any object, including "falsy" values like nil or
- # false, it should still return false since it is not empty.
- #
- # Previous code used Enumerable#any? without a block, which returned true
- # if the Enumerable contained falsy values. This allowed for invalid
- # queries to be generated and executed.
- #
- # @param [Enumerable] value
- #
- # @return [Boolean]
- #
- # @api private
- def empty_comparison?(value)
- ! value.any? { true }
- end
-
include SQL
end
View
25 lib/dm-do-adapter/spec/shared_spec.rb
@@ -390,23 +390,36 @@ class ::Author
end
end
- describe 'with an inclusion comparison of falsy values' do
+ context 'with an inclusion comparison of nil values' do
before :all do
5.times do |index|
@article_model.create(:name => "Test #{index}", :parent => @article_model.last).should be_saved
end
- @parents = [nil]
- @query = DataMapper::Query.new(repository, @article_model, :parent => @parents)
- @return = @adapter.read(@query)
+ @query = DataMapper::Query.new(repository, @article_model, :parent_name => [nil])
+ @return = @adapter.read(@query)
end
it 'should return records with matching values' do
- @return.size.should == 1
- @return.to_a.first.should == @article_model.first.attributes(:property)
+ @return.to_a.should == [ @article_model.first.attributes(:property) ]
end
end
+ context 'with an inclusion comparison of nil and actual values' do
+ before :all do
+ 5.times do |index|
+ @article_model.create(:name => "Test #{index}", :parent => @article_model.last).should be_saved
+ end
+
+ @last = @article_model.last
+ @query = DataMapper::Query.new(repository, @article_model, :parent_name => [nil, @last.parent.name])
+ @return = @adapter.read(@query)
+ end
+
+ it 'should return records with matching values' do
+ @return.to_a.should =~ [ @article_model.first.attributes(:property), @last.attributes(:property) ]
+ end
+ end
end
describe 'with a Query Path' do
Please sign in to comment.
Something went wrong with that request. Please try again.