Thread is getting a bit long so I've snipped a lot of previous code.
Tim Bunce wrote:
> On Mon, Nov 09, 2009 at 05:05:11PM +, Martin Evans wrote:
>> Martin Evans wrote:
>
>> There was an omission in my addition to Tim's example as I forgot to
>> change DBISTATE_VERSION.
>
> Thanks. Though that's less important than it was now there's also
> DBIXS_REVISION (in dbixs_rev.h) that automatically tracks the svn
> revsion number.
>
>> I've implemented this as it stands in DBD::Oracle and it seems to work
>> out ok and certainly where I was wanting to go (and further).
>
> Ok.
>
>> My own feeling is that if someone asks for something to be bound as an
>> SQL_INTEGER and it cannot due to over/under flow this should be an error
>> and that is how I've implemented it.
>
> The return value of post_fetch_sv() is meant to allow drivers to
> report an error.
>
> I thought about making post_fetch_sv() itself call DBIh_SET_ERR_* to
> report an error but opted to avoid that because, to generate a good
> error more info would need to be passed, like the column number.
I agree and had already output an error containing the column number.
> On the other hand, if post_fetch_sv() doesn't do it then there's a
> greater risk of inconsistency between the drivers.
I think we already have a level of inconsistency as some drivers already
return IVs without being asked for them. Also, number handling in each
database tends to differ quite a bit so I suspect the default may want
to differ per DBD.
>> Perhaps it could have been one of those informationals as the sv is
>> unchanged and still usable but it is not in the requested format so
>> I'd class that an error.
>
> Perhaps we should have $sth->bind_col(..., { LooselyTyped => 1 });
> to allow for those who don't want an error if the type doesn't fit.
I'm happy with that.
> That certainly feels better than overloading SQL_INTEGER vs SQL_NUMERIC
> to achieve the same effect!
agreed.
>> However, I have
>> a very small concern for people who might have been binding columns with
>> a type but no destination SV but their DBD did nothing about it (which I
>> believe is all DBDs up to now). For me, I didn't leave that code in and
>> just documented it as:
>>
>> # I was hoping the following would work (according to DBI, it
>> # might) to ensure the a, b and c
>> # columns are returned as integers instead of strings saving
>> # us from having to add 0 to them below. It does not with
>> # DBD::Oracle.
>> # NOTE: you don't have to pass a var into bind_col to receive
>> # the column data as it works on the underlying column and not
>> # just a particular bound variable.
>> #$cursor->bind_col(4, undef, { TYPE => SQL_INTEGER });
>> #$cursor->bind_col(5, undef, { TYPE => SQL_INTEGER });
>> #$cursor->bind_col(10, undef, { TYPE => SQL_INTEGER });
>>
>> but if those last 3 lines were left uncommented they would have ended up
>> a noop before but not now. However, I'd be surprised if anyone was
>> really doing that as it did nothing.
>
> Does anyone know of any drivers that pay any attention to the type param
> of bind_column?
I did not find one when I was looking a few months ago.
> We could make it default to issuing a warning on overflow, and have
> attributes to specify either an error or ignore.
We could but I think most people would be happy with error or specifying
LooselyTyped. You either know you need LooselyTyped or not and if not
you can leave it off and if it errors then your data was not as you
thought and have to decide if your data is wrong or you need
LooselyTyped. I'd be concerned a warning might just get in the way.
>> I think a MinMemory attribute would be ok but I'd use it as in most of
>> my cases I am retrieving the whole result-set in one go and it can be
>> very large. How would post_fetch_sv know this attribute?
>
> Via the flags argument.
As it turns out I /need/ MinMemory or SvPOKp(sv) returns true and that
ends up being a string again in JSON::XS. i.e., I needed the equivalent
of adding 0 to the sv which does this:
perl -le 'use Devel::Peek;my $a = "5"; Dump($a); $a = $a + 0; Dump($a);'
SV = PV(0x8154b00) at 0x815469c
REFCNT = 1
FLAGS = (PADBUSY,PADMY,POK,pPOK)
PV = 0x816fb48 "5"\0
CUR = 1
LEN = 4
SV = PVIV(0x8155b10) at 0x815469c
REFCNT = 1
FLAGS = (PADBUSY,PADMY,IOK,pIOK)
IV = 5
PV = 0x816fb48 "5"\0
CUR = 1
LEN = 4
as JSON::XS does:
if (SvPOKp (sv))
{
.
}
else if (SvNOKp (sv))
{
.
}
else if (SvIOKp (sv))
{
I want this case.
Of course, I could argue with the JSON::XS maintainer for changing the
order but I don't hold out much hope of that changing.
>> What was the intention of "void *v" argument at the end of post_fetch_sv?
>
> Planning for an uncertain future.
ok.
> After mulling it over some more, and looking at ODBC's SQLBindCol (which
> takes a C type, not an SQL type) I've decided to err on the simple side.
> I've appended a patch for review.
>
> Tim.
>
There were a few small is
