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

[Ruslan U. Zakirov]

	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
>>
>>
> 
>