Handle IN/NOT IN with an empty array

Previously, Sequel may not have handled IN/NOT correctly when given
an empty array.  Also, it may not have handled IN/NOT correctly
for multiple columns and datasets when the database didn't directly
support multiple columns for IN/NOT IN.

This changes the code to handle the empty array as a special case,
using col != col for correct NULL semantics in the IN case, and 1 = 1
for the NOT IN case. The idea for this way of handling things came
from reading SQLAlchemy's documentation.

Sequel should now correctly work with all types of IN/NOT IN and
arrays or datasets. Even the case where the database doesn't support
multiple columns for IN/NOT IN, and a dataset is used instead of an
array is now handled correctly by sending the subquery separately,
getting the results as an array and then handling it like an array.

CHANGELOG

@@ -1,5 +1,9 @@
 === HEAD
 
+* Handle IN/NOT IN with an empty array (jeremyevans)
+
+* Emulate IN/NOT IN with multiple columns where the database doesn't support it and a dataset is given (jeremyevans)
+
 * Add Dataset#unused_table_alias, for generating a table alias that has not yet been used in the query (jeremyevans)
 
 * Support an empty database argument in bin/sequel, useful for testing things without a real database (jeremyevans)

lib/sequel/dataset/sql.rb

@@ -91,11 +91,42 @@ def complex_expression_sql(op, args)
         end
       when :IN, :"NOT IN"
         cols = args.at(0)
-        if !supports_multiple_column_in? && cols.is_a?(Array)
-          expr = SQL::BooleanExpression.new(:OR, *args.at(1).to_a.map{|vals| SQL::BooleanExpression.from_value_pairs(cols.zip(vals).map{|col, val| [col, val]})})
-          literal(op == :IN ? expr : ~expr)
+        vals = args.at(1)
+        col_array = true if cols.is_a?(Array) || cols.is_a?(SQL::SQLArray)
+        if vals.is_a?(Array) || vals.is_a?(SQL::SQLArray)
+          val_array = true
+          empty_val_array = vals.to_a == []
+        end
+        if col_array
+          if empty_val_array
+            if op == :IN
+              literal(SQL::BooleanExpression.from_value_pairs(cols.to_a.map{|x| [x, x]}, :AND, true))
+            else
+              literal(1=>1)
+            end
+          elsif !supports_multiple_column_in?
+            if val_array
+              expr = SQL::BooleanExpression.new(:OR, *vals.to_a.map{|vs| SQL::BooleanExpression.from_value_pairs(cols.to_a.zip(vs).map{|c, v| [c, v]})})
+              literal(op == :IN ? expr : ~expr)
+            else
+              old_vals = vals
+              vals = vals.to_a
+              val_cols = old_vals.columns
+              complex_expression_sql(op, [cols, vals.map!{|x| x.values_at(*val_cols)}])
+            end
+          else
+            "(#{literal(cols)} #{op} #{literal(vals)})"
+          end
         else
-          "(#{literal(cols)} #{op} #{literal(args.at(1))})"
+          if empty_val_array
+            if op == :IN
+              literal(SQL::BooleanExpression.from_value_pairs([[cols, cols]], :AND, true))
+            else
+              literal(1=>1)
+            end
+          else
+            "(#{literal(cols)} #{op} #{literal(vals)})"
+          end
         end
       when *TWO_ARITY_OPERATORS
         "(#{literal(args.at(0))} #{op} #{literal(args.at(1))})"

spec/core/dataset_spec.rb

@@ -446,9 +446,62 @@ def v.values; {}; end
   specify "should accept a subquery" do
     @dataset.filter('gdp > ?', @d1.select(:avg.sql_function(:gdp))).sql.should ==
       "SELECT * FROM test WHERE (gdp > (SELECT avg(gdp) FROM test WHERE (region = 'Asia')))"
-
-    @dataset.filter(:id => @d1.select(:id)).sql.should ==
-      "SELECT * FROM test WHERE (id IN (SELECT id FROM test WHERE (region = 'Asia')))"
+  end
+
+  specify "should handle all types of IN/NOT IN queries" do
+    @dataset.filter(:id => @d1.select(:id)).sql.should == "SELECT * FROM test WHERE (id IN (SELECT id FROM test WHERE (region = 'Asia')))"
+    @dataset.filter(:id => []).sql.should == "SELECT * FROM test WHERE (id != id)"
+    @dataset.filter(:id => [1, 2]).sql.should == "SELECT * FROM test WHERE (id IN (1, 2))"
+    @dataset.filter([:id1, :id2] => @d1.select(:id1, :id2)).sql.should == "SELECT * FROM test WHERE ((id1, id2) IN (SELECT id1, id2 FROM test WHERE (region = 'Asia')))"
+    @dataset.filter([:id1, :id2] => []).sql.should == "SELECT * FROM test WHERE ((id1 != id1) AND (id2 != id2))"
+    @dataset.filter([:id1, :id2] => [[1, 2], [3,4]].sql_array).sql.should == "SELECT * FROM test WHERE ((id1, id2) IN ((1, 2), (3, 4)))"
+
+    @dataset.exclude(:id => @d1.select(:id)).sql.should == "SELECT * FROM test WHERE (id NOT IN (SELECT id FROM test WHERE (region = 'Asia')))"
+    @dataset.exclude(:id => []).sql.should == "SELECT * FROM test WHERE (1 = 1)"
+    @dataset.exclude(:id => [1, 2]).sql.should == "SELECT * FROM test WHERE (id NOT IN (1, 2))"
+    @dataset.exclude([:id1, :id2] => @d1.select(:id1, :id2)).sql.should == "SELECT * FROM test WHERE ((id1, id2) NOT IN (SELECT id1, id2 FROM test WHERE (region = 'Asia')))"
+    @dataset.exclude([:id1, :id2] => []).sql.should == "SELECT * FROM test WHERE (1 = 1)"
+    @dataset.exclude([:id1, :id2] => [[1, 2], [3,4]].sql_array).sql.should == "SELECT * FROM test WHERE ((id1, id2) NOT IN ((1, 2), (3, 4)))"
+  end
+
+  specify "should handle IN/NOT IN queries with multiple columns and an array where the database doesn't support it" do
+    @dataset.meta_def(:supports_multiple_column_in?){false}
+    @dataset.filter([:id1, :id2] => []).sql.should == "SELECT * FROM test WHERE ((id1 != id1) AND (id2 != id2))"
+    @dataset.filter([:id1, :id2] => [[1, 2], [3,4]].sql_array).sql.should == "SELECT * FROM test WHERE (((id1 = 1) AND (id2 = 2)) OR ((id1 = 3) AND (id2 = 4)))"
+    @dataset.exclude([:id1, :id2] => []).sql.should == "SELECT * FROM test WHERE (1 = 1)"
+    @dataset.exclude([:id1, :id2] => [[1, 2], [3,4]].sql_array).sql.should == "SELECT * FROM test WHERE (((id1 != 1) OR (id2 != 2)) AND ((id1 != 3) OR (id2 != 4)))"
+  end
+
+  specify "should handle IN/NOT IN queries with multiple columns and a dataset where the database doesn't support it" do
+    @dataset.meta_def(:supports_multiple_column_in?){false}
+    d1 = @d1.select(:id1, :id2)
+    def d1.fetch_rows(sql)
+      @sql_used = sql
+      @columns = [:id1, :id2]
+      yield(:id1=>1, :id2=>2)
+      yield(:id1=>3, :id2=>4)
+    end
+    d1.instance_variable_get(:@sql_used).should == nil
+    @dataset.filter([:id1, :id2] => d1).sql.should == "SELECT * FROM test WHERE (((id1 = 1) AND (id2 = 2)) OR ((id1 = 3) AND (id2 = 4)))"
+    d1.instance_variable_get(:@sql_used).should == "SELECT id1, id2 FROM test WHERE (region = 'Asia')"
+    d1.instance_variable_set(:@sql_used, nil)
+    @dataset.exclude([:id1, :id2] => d1).sql.should == "SELECT * FROM test WHERE (((id1 != 1) OR (id2 != 2)) AND ((id1 != 3) OR (id2 != 4)))"
+    d1.instance_variable_get(:@sql_used).should == "SELECT id1, id2 FROM test WHERE (region = 'Asia')"
+  end
+
+  specify "should handle IN/NOT IN queries with multiple columns and an empty dataset where the database doesn't support it" do
+    @dataset.meta_def(:supports_multiple_column_in?){false}
+    d1 = @d1.select(:id1, :id2)
+    def d1.fetch_rows(sql)
+      @sql_used = sql
+      @columns = [:id1, :id2]
+    end
+    d1.instance_variable_get(:@sql_used).should == nil
+    @dataset.filter([:id1, :id2] => d1).sql.should == "SELECT * FROM test WHERE ((id1 != id1) AND (id2 != id2))"
+    d1.instance_variable_get(:@sql_used).should == "SELECT id1, id2 FROM test WHERE (region = 'Asia')"
+    d1.instance_variable_set(:@sql_used, nil)
+    @dataset.exclude([:id1, :id2] => d1).sql.should == "SELECT * FROM test WHERE (1 = 1)"
+    d1.instance_variable_get(:@sql_used).should == "SELECT id1, id2 FROM test WHERE (region = 'Asia')"
   end
 
   specify "should accept a subquery for an EXISTS clause" do

spec/integration/dataset_test.rb

@@ -715,6 +715,14 @@
     @ds.exclude(:a=>nil).all.should == [{:a=>20, :b=>nil}]
   end
 
+  it "should work with arrays as hash values" do
+    @ds.insert(20, 10)
+    @ds.filter(:a=>[10]).all.should == []
+    @ds.filter(:a=>[20, 10]).all.should == [{:a=>20, :b=>10}]
+    @ds.exclude(:a=>[10]).all.should == [{:a=>20, :b=>10}]
+    @ds.exclude(:a=>[20, 10]).all.should == []
+  end
+
   it "should work with ranges as hash values" do
     @ds.insert(20, 10)
     @ds.filter(:a=>(10..30)).all.should == [{:a=>20, :b=>10}]
@@ -733,7 +741,7 @@
     @ds.filter({15=>20}.case(0, :a) > 0).all.should == []
   end
 
-  it "should work multiple value arrays" do
+  it "should work with multiple value arrays" do
     @ds.insert(20, 10)
     @ds.quote_identifiers = false
     @ds.filter([:a, :b]=>[[20, 10]].sql_array).all.should == [{:a=>20, :b=>10}]
@@ -746,6 +754,40 @@
     @ds.exclude([:a, :b]=>[[20, 10], [1, 2]].sql_array).all.should == []
     @ds.exclude([:a, :b]=>[[10, 10], [20, 20]].sql_array).all.should == [{:a=>20, :b=>10}]
   end
+
+  it "should work with IN/NOT in with datasets" do
+    @ds.insert(20, 10)
+    @ds.quote_identifiers = false
+
+    @ds.filter(:a=>@ds.select(:a)).all.should == [{:a=>20, :b=>10}]
+    @ds.filter(:a=>@ds.select(:b)).all.should == []
+    @ds.exclude(:a=>@ds.select(:a)).all.should == []
+    @ds.exclude(:a=>@ds.select(:b)).all.should == [{:a=>20, :b=>10}]
+
+    @ds.filter([:a, :b]=>@ds.select(:a, :b)).all.should == [{:a=>20, :b=>10}]
+    @ds.filter([:a, :b]=>@ds.select(:b, :a)).all.should == []
+    @ds.exclude([:a, :b]=>@ds.select(:a, :b)).all.should == []
+    @ds.exclude([:a, :b]=>@ds.select(:b, :a)).all.should == [{:a=>20, :b=>10}]
+
+    @ds.filter([:a, :b]=>@ds.select(:a, :b).where(:a=>15)).all.should == []
+    @ds.exclude([:a, :b]=>@ds.select(:a, :b).where(:a=>15)).all.should == [{:a=>20, :b=>10}]
+  end
+
+  specify "should work empty arrays" do
+    @ds.insert(20, 10)
+    @ds.filter(:a=>[]).all.should == []
+    @ds.exclude(:a=>[]).all.should == [{:a=>20, :b=>10}]
+    @ds.filter([:a, :b]=>[]).all.should == []
+    @ds.exclude([:a, :b]=>[]).all.should == [{:a=>20, :b=>10}]
+  end
+
+  specify "should work empty arrays with nulls" do
+    @ds.insert(nil, nil)
+    @ds.filter(:a=>[]).all.should == []
+    @ds.exclude(:a=>[]).all.should == [{:a=>nil, :b=>nil}]
+    @ds.filter([:a, :b]=>[]).all.should == []
+    @ds.exclude([:a, :b]=>[]).all.should == [{:a=>nil, :b=>nil}]
+  end
 
   it "should work multiple conditions" do
     @ds.insert(20, 10)