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

Jesse Vincent jesse at bestpractical.com
Tue May 24 11:15:23 EDT 2005




On Tue, May 24, 2005 at 03:33:55PM +0400, Ruslan U. Zakirov wrote:
> 	Heya, guys.
> Patch attached that fixes issue on Pg as suggested and uncomment tests.

Except it's not :(

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

-- 


More information about the Rt-devel mailing list