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