On 20/07/15 14:15, Tim Bunce wrote:
On Mon, Jul 20, 2015 at 08:55:40AM +0100, Martin J. Evans wrote:
On 19/07/15 15:41, Tim Bunce wrote:
Please also see the issue I reported in DBI back in 2012:
https://rt.cpan.org/Ticket/Display.html?id=81911
I had to add various workarounds and a warning to DBD::ODBC.
Ah, thanks for the reminder Martin! I'll add a comment on that case.
Any thoughts about the general principle of changing the XS execute to
return the value of the DBIc_ROW_COUNT IV if the int returned by
dbd_st_execute is > 0 and DBIc_ROW_COUNT > 0?
Tim.
I think that would work for me - I'm happy to test it our here if you want to
give it a go.
IIRC, when this was last discussed the problem is that some drivers might not
set DBIc_ROW_COUNT so you can't just use DBIc_ROW_COUNT. In fact, I just
checked, and DBD::ODBC does not seem to call DBIc_ROW_COUNT other than to set
it to 0 in ODBC.xsi (which is code from DBI anyway). Does that sound right?
If a driver is supposed to set DBIc_ROW_COUNT I'd rather change the drivers I
maintain to do that especially since in ODBC and 64bit SQLRowCount already
returns a 64 bit value. Is there some docs on that or perhaps you could just
tell me or point me at a driver that does it correctly.
I'm having a frustrating day so far so perhaps have lost the ability to read
diffs and C but in your change at
https://github.com/perl5-dbi/dbi/commit/29f6b9b76e9c637be31cb80f1a262ff68b42ef43#diff-cb6af96fe009d6f8d9d682415e1ab755
"if retval>0 (checked above) " I don't see where the "checked above" bit is.
it looks like:
if (retval == 0)
..
else if (retval == -1)
..
else if (retval <= -2)
..
else
new stuff here
retval could still be negative just not -1 or -2
Also, maybe a little picky but the comment "and DBIc_ROW_COUNT>0" does not
match the code.
If no DBDs use DBIc_ROW_COUNT then that warning you've put in will do nothing.
I'd like to see a driver which does use DBIc_ROW_COUNT and if there are none
I'm happy to change DBD::ODBC initially to a) test the diff you just applied
and b) test the suggested fix.
Martin
