[Rt-devel] [PATCH] DBIx-SB: new tests, fixes and cleanups

[Ruslan U. Zakirov]

as usual :)

David Glasser wrote:
> no patch attached :)
> 
> --dave
> 
> On 5/24/05, Ruslan U. Zakirov <Ruslan.Zakirov at miet.ru> wrote:
> 
>>        Heya, guys.
>>Patch attached that fixes issue on Pg as suggested and uncomment tests.
>>
>>Now I have system where I can test firebird, mysql, pg and sqlite, so
>>next time I would be testing my patches with all backends.
>>
>>--
>>Best regards, Ruslan.
>>
>>
>>Ruslan U. Zakirov wrote:
>>
>>>David Glasser wrote:
>>>
>>>
>>>>On 5/23/05, Ruslan U. Zakirov <Ruslan.Zakirov at miet.ru> wrote:
>>>>
>>>>
>>>>>       Heya!
>>>>>Changes:
>>>>>* rewrite LoadByPrimaryKeys args handling to consistent with other Load*
>>>>>methods
>>>>>* report error when PK filed is missing in LoadByPrimaryKeys
>>>>>* fix warning in __Set methods when newvalue is undef
>>>>>* small code cleanups
>>>>>
>>>>>Tests:
>>>>>* coverage grows from 75.2% to 84.7% for Record.pm
>>>>
>>>>
>>>>
>>>>Hi Ruslan.  Thanks for all the great new tests!  I am going to apply
>>>>your patch, but one of the tests fails under Postgres (so I commented
>>>>out, since doing a TODO block for only one driver seems tough).
>>>>Specifically:
>>>>
>>>># LoadByCols and empty or NULL values
>>>>    $rec = TestApp::Address->new($handle);
>>>>    $id = $rec->Create( Name => 'Obra', Phone => undef );
>>>>    ok( $id, "new record");
>>>>    $rec = TestApp::Address->new($handle);
>>>>    $rec->LoadByCols( Name => 'Obra', Phone => undef, EmployeeId => '' );
>>>>       is( $rec->id, $id, "loaded record by empty value" );
>>>>
>>>>The problem is that in Pg, '' and 0 are not the same thing.  That
>>>>means that the SQL generated for the EmployeeId, which is:
>>>>  (EmployeeId IS NULL OR EmployeeID = '')
>>>>makes an error, since  EmployeeID (an integer) cannot be compared to
>>>>'' (a string).
>>>
>>>Heh, I have sense that it fails on some DB. I think it fails also on
>>>InterBase 6.0(don't have it), but firebird use dinamic cast here.
>>>
>>>
>>>>I guess the current status for what LoadByCols does is:
>>>>   0 or '0' means search for 0, though on mysql and Sqlite it gets '' too
>>>>   '' or undef means search for NULL or '', though on mysql and Sqlite
>>>>it gets 0 too
>>>>
>>>>Jesse's suggestion is to patch LoadByCols to check if the column
>>>>appears to be an integer type (based on _ClassAccessible), and if so
>>>>put in the code
>>>>  (EmployeeId IS NULL OR EmployeeID = 0)
>>>>instead.
>>>
>>>I prefer bind_values, I think it should work with bind_values, because
>>>DBI or DB itself must do right thing.
>>>
>>>
>>>>Would you like to try to get that working?
>>>
>>>Yes, I have installed PostgreSQL, so I can test it too. I'll fix this bug.
>>>
>>>
>>>>--dave
>>>>
>>>>
>>>
>>>
>>
> 
> 


-------------- next part --------------
=== t/01records.t
==================================================================
--- t/01records.t   (/mirrors/DBIx-SearchBuilder/trunk)   (revision 1676)
+++ t/01records.t   (/DBIx-SearchBuilder/local)   (revision 1676)
@@ -8,7 +8,7 @@
 BEGIN { require "t/utils.pl" }
 our (@AvailableDrivers);
 
-use constant TESTS_PER_DRIVER => 64;
+use constant TESTS_PER_DRIVER => 65;
 
 my $total = scalar(@AvailableDrivers) * TESTS_PER_DRIVER;
 plan tests => $total;
@@ -169,9 +169,8 @@
 	$id = $rec->Create( Name => 'Obra', Phone => undef );
 	ok( $id, "new record");
 	$rec = TestApp::Address->new($handle);
-#	$rec->LoadByCols( Name => 'Obra', Phone => undef, EmployeeId => '' );
-#	Fails under Pg
-#    is( $rec->id, $id, "loaded record by empty value" );
+	$rec->LoadByCols( Name => 'Obra', Phone => undef, EmployeeId => '' );
+	is( $rec->id, $id, "loaded record by empty value" );
 
 # __Set error paths
 	$rec = TestApp::Address->new($handle);
=== SearchBuilder/Record.pm
==================================================================
--- SearchBuilder/Record.pm   (/mirrors/DBIx-SearchBuilder/trunk)   (revision 1676)
+++ SearchBuilder/Record.pm   (/DBIx-SearchBuilder/local)   (revision 1676)
@@ -986,7 +986,15 @@
 		push @bind, $value;
 	}
 	else {
-		push @phrases, "($key IS NULL OR $key = '')";
+		push @phrases, "($key IS NULL OR $key = ?)";
+		my $meta = $self->_ClassAccessible->{$key};
+		$meta->{'type'} ||= '';
+		# TODO: type checking should be done in generic way
+		if ( $meta->{'is_numeric'} || $meta->{'type'} =~ /INT|NUMERIC|DECIMAL|REAL|DOUBLE|FLOAT/i  ) {
+		     push @bind, 0;
+		} else {
+		     push @bind, '';
+		}
 	}
     }
     

Property changes on: 
___________________________________________________________________
Name: svk:merge
  4c7c5507-10e6-0310-a638-9a99c59afa75:/local/sb:9860
  a51291e0-c2ea-0310-847b-fbb8d8170edb:/local/sb:16978
  d53a6f7f-efd6-0310-930d-faa3dfc79e01:/local/SearchBuilder:1719
 +e417ac7c-1bcc-0310-8ffa-8f5827389a85:/DBIx-SearchBuilder/trunk:2939