Tim Bunce wrote:
> On Thu, Nov 26, 2009 at 07:47:45PM +0000, Martin J. Evans wrote:
>> Martin Evans wrote:
>>> Tim,
>>>
>>> I'm not sure if you are bothered by this but there appears to be a small
>>> inconsistency between SQL_INTEGER/SQL_NUMERIC and SQL_DOUBLE handling in
>>> sql_type_cast:
>>>
>>> sql_type_cast("aa", SQL_INTEGER, 0)
>>> returns 1 (no cast not strict)
>>> sql_type_casr("aa", SQL_INTEGER, DBIstcf_STRICT)
>>> returns 0 (no cast, strict)
>>> SQL_NUMERIC works as above (with a fix to the grok stuff)
>>>
>>> but
>>>
>>> sql_type_cast("aa", SQL_DOUBLE, 0)
>>> returns 2 (cast ok) I expected 1
>>> sql_type_cast("aa", SQL_DOUBLE, DBIstcf_STRICT)
>>> returns 2 (cast ok) I expected 0
>>>
>>> As you point out in the code if warnings are enabled you get a warning
>>> but you don't get the expected return.
>> Would you have any objections to me changing:
>>
>> case SQL_DOUBLE:
>> sv_2nv(sv);
>> /* SvNOK should be set but won't if sv is not numeric (in which
>> * case perl would have warn'd already if -w or warnings are in
>> effect)
>> */
>> cast_ok = SvNOK(sv);
>> break;
>>
>> to
>>
>> case SQL_DOUBLE:
>> if (looks_like_number(sv)) {
>> sv_2nv(sv);
>> /* SvNOK should be set but won't if sv is not numeric (in which
>> * case perl would have warn'd already if -w or warnings are in
>> effect)
>> */
>> cast_ok = SvNOK(sv);
>> } else {
>> cast_ok = 0;
>> }
>> break;
>>
>> as this fixes the inconsistency I mentioned above i.e., sv's cast to
>> doubles which are not numbers return 0 or 1 (depending on STRICT)
>> instead of always returning 2 (cast ok). I worried a little about this
>> as you end up with 0 in the NV for a non-numeric and a return of 2 which
>> looked like the cast worked.
>
> What does
> perl -V:.*|grep nv_preserves_uv
> say for you?
$ perl -V:.*|grep nv_preserves_uv
d_nv_preserves_uv='define';
nv_preserves_uv_bits='32';
> My reading of Perl_sv_2nv() in sv.c is that ifdef NV_PRESERVES_UV
> then SvNOK is not set (but SvNOKp is) if grok_number() returns 0
> into numtype. The else NV_PRESERVES_UV branch ends with
> if (!numtype)
> SvFLAGS(sv) &= ~(SVf_IOK|SVf_NOK);
> So either way, if grok_number() returns 0 then SvNOK() should be false.
>
> And since looks_like_number is just a wrapper around grok_number I'm not
> sure what's going on.
>
> Perhaps check it in without the change above (so with a failing test)
> and I might get a change to dig into it.
ok, I'll check it in as you described later this afternoon and if you
get a chance to look at it that will be good but in the mean time I'll
let you know if I get any further with it.
>>> Also, there are a couple of bugs in sql_type_cast which I will fix when
>>> the dbi repository is available again - it is locked by some problem at
>>> svn.perl.org. I mailed s...@perl.org but no response or resolution as yet.
>> I fixed these.
>
> Thanks.
checked in now.
>>> My test code so far which demonstrates the above attached in just in
>>> case (not tested on a system without JSON::XS or Data::Peek as yet and
>>> not using DBI::neat because it is too clever at recognising numbers -
>>> SvNIOK).
>
> The "/* already has string version of the value, so use it */" block?
yes indeed.
>> I have now tested this on a system without JSON::XS and Data::Peek and
>> fixed the problems and added more tests.
>>
>> If you are ok with this I'll commit the change to DBI.xs, add the test
>> and document sql_type_cast_svpv in DBI::DBD then move on to making this
>> work in DBD::Oracle again.
>
> That would be great. Many thanks Martin.
DBI::DBD pod changes checked in now although I may come back to it once
I've re-implemented this in DBD::Oracle.
>> BTW, not sure whether you prefer a mail to you or to continue this type
>> of discussion on the dbi-dev list - let me know and I'll do so in the
>> future.
>
> dbi-dev is best - CC'd - as it keeps other driver developers in the loop
> and more eyeballs can help spot/fix problems more quickly.
>
> Tim.
>
>
Martin
--
Martin J. Evans
Easysoft Limited
http://www.easysoft.com
