Permalink
Browse files

Handle nil values when formatting bound variable arguments in the pg_…

…row extension (Fixes #548)

Basically, pg_row piggy-backs on pg_array bound variable
formatting, but array types use NULL to represent NULL,
while row types use the empty string to represent NULL.

While here, add tests for nil values in bound variables for
arrays and hstore, even though the behavior there was already
correct.
  • Loading branch information...
1 parent ed3b8a5 commit a8b135dddbb7c71060499d44272f7aa78b95c768 @jeremyevans committed Sep 13, 2012
Showing with 35 additions and 4 deletions.
  1. +2 −0 CHANGELOG
  2. +4 −4 lib/sequel/extensions/pg_row.rb
  3. +22 −0 spec/adapters/postgres_spec.rb
  4. +7 −0 spec/extensions/pg_row_spec.rb
View
2 CHANGELOG
@@ -1,5 +1,7 @@
=== HEAD
+* Handle nil values when formatting bound variable arguments in the pg_row extension (jeremyevans) (#548)
+
* Handle nil values when parsing composite types in the pg_row extension (jeremyevans) (#548)
* Add :disconnect=>:retry option to Database#transaction, for automatically retrying the transaction on disconnect (jeremyevans)
View
8 lib/sequel/extensions/pg_row.rb
@@ -379,10 +379,10 @@ def self.extended(db)
def bound_variable_arg(arg, conn)
case arg
when ArrayRow
- "(#{arg.map{|v| bound_variable_array(v)}.join(COMMA)})"
+ "(#{arg.map{|v| bound_variable_array(v) if v}.join(COMMA)})"
when HashRow
arg.check_columns!
- "(#{arg.values_at(*arg.columns).map{|v| bound_variable_array(v)}.join(COMMA)})"
+ "(#{arg.values_at(*arg.columns).map{|v| bound_variable_array(v) if v}.join(COMMA)})"
else
super
end
@@ -528,10 +528,10 @@ def schema_column_type(db_type)
def bound_variable_array(arg)
case arg
when ArrayRow
- "\"(#{arg.map{|v| bound_variable_array(v)}.join(COMMA).gsub(ESCAPE_RE, ESCAPE_REPLACEMENT)})\""
+ "\"(#{arg.map{|v| bound_variable_array(v) if v}.join(COMMA).gsub(ESCAPE_RE, ESCAPE_REPLACEMENT)})\""
when HashRow
arg.check_columns!
- "\"(#{arg.values_at(*arg.columns).map{|v| bound_variable_array(v)}.join(COMMA).gsub(ESCAPE_RE, ESCAPE_REPLACEMENT)})\""
+ "\"(#{arg.values_at(*arg.columns).map{|v| bound_variable_array(v) if v}.join(COMMA).gsub(ESCAPE_RE, ESCAPE_REPLACEMENT)})\""
else
super
end
View
22 spec/adapters/postgres_spec.rb
@@ -1592,6 +1592,11 @@ def @ds.check_return
@ds.filter(:i=>:$i).call(:first, :i=>[1,2]).should == {:i=>[1,2]}
@ds.filter(:i=>:$i).call(:first, :i=>[1,3]).should == nil
+ # NULL values
+ @ds.delete
+ @ds.call(:insert, {:i=>[nil,nil]}, {:i=>:$i})
+ @ds.first.should == {:i=>[nil, nil]}
+
@db.create_table!(:items) do
column :i, 'text[]'
end
@@ -1752,6 +1757,10 @@ def @ds.check_return
@ds.filter(:i=>:$i).call(:first, :i=>Sequel.hstore({})).should == nil
@ds.delete
+ @ds.call(:insert, {:i=>Sequel.hstore('a'=>nil)}, {:i=>:$i})
+ @ds.get(:i).should == Sequel.hstore('a'=>nil)
+
+ @ds.delete
@ds.call(:insert, {:i=>@h}, {:i=>:$i})
@ds.get(:i).should == @h
@ds.filter(:i=>:$i).call(:first, :i=>@h).should == {:i=>@h}
@@ -2014,6 +2023,10 @@ def left_item_id
@ds.filter(Sequel.cast(:i, String)=>:$i).call(:first, :i=>Sequel.pg_json(@a)).should == {:i=>@a}
@ds.filter(Sequel.cast(:i, String)=>:$i).call(:first, :i=>Sequel.pg_json([])).should == nil
+ @ds.delete
+ @ds.call(:insert, {:i=>Sequel.pg_json('a'=>nil)}, {:i=>:$i})
+ @ds.get(:i).should == Sequel.pg_json('a'=>nil)
+
@db.create_table!(:items){column :i, 'json[]'}
j = Sequel.pg_array([Sequel.pg_json('a'=>1), Sequel.pg_json(['b', 2])], :text)
@ds.call(:insert, {:i=>j}, {:i=>:$i})
@@ -2499,6 +2512,10 @@ def left_item_id
@ds.get(:address).should == {:street=>'123 Sesame St', :city=>'Somewhere', :zip=>'12345'}
@ds.filter(:address=>Sequel.cast(:$address, :address)).call(:first, :address=>Sequel.pg_row(['123 Sesame St', 'Somewhere', '12345']))[:id].should == 1
@ds.filter(:address=>Sequel.cast(:$address, :address)).call(:first, :address=>Sequel.pg_row(['123 Sesame St', 'Somewhere', '12356'])).should == nil
+
+ @ds.delete
+ @ds.call(:insert, {:address=>Sequel.pg_row([nil, nil, nil])}, {:address=>:$address, :id=>1})
+ @ds.get(:address).should == {:street=>nil, :city=>nil, :zip=>nil}
end if POSTGRES_DB.adapter_scheme == :postgres && SEQUEL_POSTGRES_USES_PG
specify 'use arrays of row types in bound variables' do
@@ -2507,6 +2524,11 @@ def left_item_id
@ds.get(:company).should == {:id=>1, :employees=>[{:id=>1, :address=>{:street=>'123 Sesame St', :city=>'Somewhere', :zip=>'12345'}}]}
@ds.filter(:employees=>Sequel.cast(:$employees, 'person[]')).call(:first, :employees=>Sequel.pg_array([@db.row_type(:person, [1, Sequel.pg_row(['123 Sesame St', 'Somewhere', '12345'])])]))[:id].should == 1
@ds.filter(:employees=>Sequel.cast(:$employees, 'person[]')).call(:first, :employees=>Sequel.pg_array([@db.row_type(:person, [1, Sequel.pg_row(['123 Sesame St', 'Somewhere', '12356'])])])).should == nil
+
+
+ @ds.delete
+ @ds.call(:insert, {:employees=>Sequel.pg_array([@db.row_type(:person, [1, Sequel.pg_row([nil, nil, nil])])])}, {:employees=>:$employees, :id=>1})
+ @ds.get(:employees).should == [{:address=>{:city=>nil, :zip=>nil, :street=>nil}, :id=>1}]
end if POSTGRES_DB.adapter_scheme == :postgres && SEQUEL_POSTGRES_USES_PG
specify 'operations/functions with pg_row_ops' do
View
7 spec/extensions/pg_row_spec.rb
@@ -158,6 +158,13 @@
@db.bound_variable_arg([@m::HashRow.subclass(nil, [:a, :b]).call(:a=>"1", :b=>"abc\\'\",")], nil).should == '{"(\\"1\\",\\"abc\\\\\\\\\'\\\\\\",\\")"}'
end
+ it "should handle nils in bound variables" do
+ @db.bound_variable_arg(@m::ArrayRow.call([nil, nil]), nil).should == '(,)'
+ @db.bound_variable_arg(@m::HashRow.subclass(nil, [:a, :b]).call(:a=>nil, :b=>nil), nil).should == '(,)'
+ @db.bound_variable_arg([@m::ArrayRow.call([nil, nil])], nil).should == '{"(,)"}'
+ @db.bound_variable_arg([@m::HashRow.subclass(nil, [:a, :b]).call(:a=>nil, :b=>nil)], nil).should == '{"(,)"}'
+ end
+
it "should allow registering row type parsers by introspecting system tables" do
@db.conversion_procs[4] = p4 = proc{|s| s.to_i}
@db.conversion_procs[5] = p5 = proc{|s| s * 2}

0 comments on commit a8b135d

Please sign in to comment.