Skip Menu |

This queue is for tickets about the DBD-ODBC CPAN distribution.

Report information
The Basics
Id: 75687
Status: resolved
Priority: 0/
Queue: DBD-ODBC

People
Owner: Nobody in particular
Requestors: ribasushi [...] leporine.io
Cc:
AdminCc:

Bug Information
Severity: Important
Broken in: 1.35
Fixed in: 1.36_2



Subject: Regression in 1.35 execute_for_fetch/execute_array with freetds 0.82 (current Debian stable)
Damn it, I should have tested this more :((( Somewhat reduced test case: https://github.com/dbsrgits/dbix-class/blob/8c4e88c67d11ba11847e801e03a96d384ff616a5/t/746mssql.t Available in two incarnations - one is the original execute_array() based code (8c4e88c67) and the other the current master including the new execute_for_fetch() based code (1d1906158). I can try to reduce it more, let me know! Attached is a run capture, and full level 5 traces of all 4 runs: rabbit@Thesaurus:~/devel/dbic/dbgit$ perl Makefile.PL &>/dev/null ; git rev-parse HEAD 8c4e88c67d11ba11847e801e03a96d384ff616a5 rabbit@Thesaurus:~/devel/dbic/dbgit$ DBI_TRACE=5=ODBC_1.33_execute_array_PASS prove -l t/746mssql.t t/746mssql.t .. ok Result: PASS rabbit@Thesaurus:~/devel/dbic/dbgit$ perl Makefile.PL &>/dev/null ; git rev-parse HEAD 1d190615808b90d101ee6c2bd6931458b861d344 rabbit@Thesaurus:~/devel/dbic/dbgit$ DBI_TRACE=5=ODBC_1.33_execute_for_fetch_PASS prove -l t/746mssql.t Result: PASS rabbit@Thesaurus:~/devel/dbic/dbgit$ PERL_CPANM_OPT= cpanm DBD::ODBC --> Working on DBD::ODBC ... Successfully installed DBD-ODBC-1.35 (upgraded from 1.33) rabbit@Thesaurus:~/devel/dbic/dbgit$ DBI_TRACE=5=ODBC_1.35_execute_for_fetch_FAIL prove -l t/746mssql.t t/746mssql.t .. 3/? # Failed test 'populate with PKs supplied ok' # at t/746mssql.t line 101. # died: DBIx::Class::Exception (DBIx::Class::Schema::populate(): execute_for_fetch() aborted with 'failed to retrieve diags' at populate slice: # { # id => 2, # name => "woggle" # } at t/746mssql.t line 83 # ) t/746mssql.t .. 4/? # Failed test 'populate with PKs supplied ok' # at t/746mssql.t line 101. # died: DBIx::Class::Exception (DBIx::Class::Schema::populate(): execute_for_fetch() aborted with 'failed to retrieve diags' at populate slice: # { # id => 3, # name => "boggle" # } at t/746mssql.t line 83 # ) # Looks like you failed 2 tests of 5. t/746mssql.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/5 subtests (less 2 skipped subtests: 1 okay) Test Summary Report ------------------- t/746mssql.t (Wstat: 512 Tests: 5 Failed: 2) Failed tests: 3-4 Non-zero exit status: 2 Files=1, Tests=5, 9 wallclock secs ( 0.02 usr 0.00 sys + 0.30 cusr 0.00 csys = 0.32 CPU) Result: FAIL rabbit@Thesaurus:~/devel/dbic/dbgit$ perl Makefile.PL &>/dev/null ; git rev-parse HEAD 8c4e88c67d11ba11847e801e03a96d384ff616a5 rabbit@Thesaurus:~/devel/dbic/dbgit$ DBI_TRACE=5=ODBC_1.35_execute_array_FAIL prove -l t/746mssql.t t/746mssql.t .. 3/? # Failed test 'populate with PKs supplied ok' # at t/746mssql.t line 100. # died: DBIx::Class::Exception (DBIx::Class::Schema::populate(): execute_array() aborted with 'failed to retrieve diags' at populate slice: # { # id => 2, # name => "woggle" # } at t/746mssql.t line 82 # ) t/746mssql.t .. 4/? # Failed test 'populate with PKs supplied ok' # at t/746mssql.t line 100. # died: DBIx::Class::Exception (DBIx::Class::Schema::populate(): execute_array() aborted with 'failed to retrieve diags' at populate slice: # { # id => 2, # name => "woggle" # } at t/746mssql.t line 82 # ) t/746mssql.t .. 5/? # Looks like you failed 2 tests of 5. t/746mssql.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/5 subtests (less 2 skipped subtests: 1 okay) Test Summary Report ------------------- t/746mssql.t (Wstat: 512 Tests: 5 Failed: 2) Failed tests: 3-4 Non-zero exit status: 2 Files=1, Tests=5, 10 wallclock secs ( 0.02 usr 0.00 sys + 0.30 cusr 0.02 csys = 0.34 CPU) Result: FAIL
Subject: ODBC_1.35_execute_for_fetch_FAIL
Download ODBC_1.35_execute_for_fetch_FAIL
application/octet-stream 102.5k

Message body not shown because it is not plain text.

Subject: ODBC_1.35_execute_array_FAIL
Download ODBC_1.35_execute_array_FAIL
application/octet-stream 105.6k

Message body not shown because it is not plain text.

Subject: ODBC_1.33_execute_array_PASS
Download ODBC_1.33_execute_array_PASS
application/octet-stream 148.2k

Message body not shown because it is not plain text.

Subject: ODBC_1.33_execute_for_fetch_PASS
Download ODBC_1.33_execute_for_fetch_PASS
application/octet-stream 145.4k

Message body not shown because it is not plain text.

On Sun Mar 11 06:04:43 2012, RIBASUSHI wrote: Show quoted text
> Damn it, I should have tested this more :(((
Thanks Peter. I know some ODBC drivers have bugs which prevent the native ODBC execute_for_fetch working and I've been trying to persuade people to fix them. I've not looked at your case yet as I'm away this weekend so it could just as easily be DBD::ODBC in this case. There is an attribute to turn off the ODBC native execute_for_fetch (which will work as a workaround for now). In the end, even though I knew some drivers would break I decided it was better for the default to be on and it would hopefully force driver writers to fix the issues, and have a switch to return to the old behaviour. I'll look at this properly when I get home. As always, I scoured the net/irc etc for people to test but as always, I had little feedback. I cannot test all of the 100's (and there are easily over 100) of ODBC drivers. Smokers don't help in this respect as no one has DBI set up although book (I think) worte Test::Database to help with this - perhaps you can speak to him at QA hackathon and see if we can move it on a bit - it would be really worth while. Martin -- Martin J. Evans Wetherby, UK
On Sun Mar 11 06:32:57 2012, MJEVANS wrote: Show quoted text
> On Sun Mar 11 06:04:43 2012, RIBASUSHI wrote:
> > Damn it, I should have tested this more :(((
> > Thanks Peter. > > I know some ODBC drivers have bugs which prevent the native ODBC > execute_for_fetch working and I've been trying to persuade people to fix > them. I've not looked at your case yet as I'm away this weekend so it > could just as easily be DBD::ODBC in this case. There is an attribute to > turn off the ODBC native execute_for_fetch (which will work as a > workaround for now). > > In the end, even though I knew some drivers would break I decided it was > better for the default to be on and it would hopefully force driver > writers to fix the issues, and have a switch to return to the old
behaviour. Show quoted text
> > I'll look at this properly when I get home. > > As always, I scoured the net/irc etc for people to test but as always, I > had little feedback. I cannot test all of the 100's (and there are > easily over 100) of ODBC drivers. Smokers don't help in this respect as > no one has DBI set up although book (I think) worte Test::Database to > help with this - perhaps you can speak to him at QA hackathon and see if > we can move it on a bit - it would be really worth while. > > Martin
"failed to retrieve diags" - same old same old issue with freeTDS. It returns an error from an ODBC call and then does not provide any error text - I guess. Should get a chance to look at it this evening or tomorrow. Martin -- Martin J. Evans Wetherby, UK
Subject: Re: [rt.cpan.org #75687] Regression in 1.35 execute_for_fetch/execute_array with freetds 0.82 (current Debian stable)
Date: Sun, 11 Mar 2012 11:39:46 +0100
To: bug-DBD-ODBC [...] rt.cpan.org
From: Peter Rabbitson <ribasushi [...] cpan.org>
Martin J Evans via RT wrote: Show quoted text
> <URL: https://rt.cpan.org/Ticket/Display.html?id=75687 > > > On Sun Mar 11 06:04:43 2012, RIBASUSHI wrote:
>> Damn it, I should have tested this more :(((
> > Thanks Peter. > > I know some ODBC drivers have bugs which prevent the native ODBC > execute_for_fetch working and I've been trying to persuade people to fix > them. I've not looked at your case yet as I'm away this weekend so it > could just as easily be DBD::ODBC in this case. There is an attribute to > turn off the ODBC native execute_for_fetch (which will work as a > workaround for now). > > In the end, even though I knew some drivers would break I decided it was > better for the default to be on and it would hopefully force driver > writers to fix the issues, and have a switch to return to the old behaviour. > > I'll look at this properly when I get home. > > As always, I scoured the net/irc etc for people to test but as always, I > had little feedback. I cannot test all of the 100's (and there are > easily over 100) of ODBC drivers. Smokers don't help in this respect as > no one has DBI set up although book (I think) worte Test::Database to > help with this - perhaps you can speak to him at QA hackathon and see if > we can move it on a bit - it would be really worth while. >
Yes, this is definitely on the roadmap. Also note that my sentence quoted above laments *me* not testing more. I am in no way accusing you of not having your due diligence ;)
Subject: Re: [rt.cpan.org #75687] Regression in 1.35 execute_for_fetch/execute_array with freetds 0.82 (current Debian stable)
Date: Sun, 11 Mar 2012 11:41:26 +0100
To: bug-DBD-ODBC [...] rt.cpan.org
From: Peter Rabbitson <ribasushi [...] cpan.org>
Martin J Evans via RT wrote: Show quoted text
> > "failed to retrieve diags" - same old same old issue with freeTDS. It > returns an error from an ODBC call and then does not provide any error > text - I guess. Should get a chance to look at it this evening or tomorrow. >
This is a little different - with 1.33 tests pass fully, and with 1.35 something that should insert just fine does mysteriously fail. So, while yes, freetds is a \x{1F4A9}, I think we do have a genuine D::O bug here.
On Sun Mar 11 06:41:36 2012, RIBASUSHI wrote: Show quoted text
> Martin J Evans via RT wrote: >
> > > > "failed to retrieve diags" - same old same old issue with freeTDS.
> It
> > returns an error from an ODBC call and then does not provide any
> error
> > text - I guess. Should get a chance to look at it this evening or
> tomorrow.
> >
> > This is a little different - with 1.33 tests pass fully, and with 1.35 > something that should insert just fine does mysteriously fail. So, > while > yes, freetds is a \x{1F4A9}, I think we do have a genuine D::O bug > here.
The standard 70execute_array.t test fails with freeTDS 9 as it returns rows affected as 1 instead of 5. Although this may not be the problem in this case it is an error in the driver. Still to look at your exact example. Martin -- Martin J. Evans Wetherby, UK
My interpretation of the DBIC code example (I've not got all the required stuff to run the example as is and I'm DBIC illiterate) is: use DBI; use strict; use warnings; my $h = DBI->connect('dbi:ODBC:freetds8','sa','easysoft', {RaiseError => 1}); eval { $h->do(q/drop table owners/); }; $h->do(q/create table owners (id int identity (1,1) not null, name varchar(100))/); my $s = $h->prepare_cached(q/SET IDENTITY_INSERT owners ON insert into owners (id, name) values(?,?)/); my @data = ([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15], ['wiggle', 'woggle', 'boggle', 'fRIOUX', 'fRUE', 'fREW', 'fROOH', 'fISMBoC', 'station', 'mirror', 'dimly', 'face_to_face', 'icarus', 'dream', 'dyrstyggyr']); $s->execute_array(undef, @data); and it appears to work with freeTDS 0.91 and 0.8something although the latter outputs: Describe failed during DBI::st=HASH(0x9cda9b8)->FETCH(NUM_OF_PARAMS,0) at /home/ martin/perl5/perlbrew/perls/perl-5.13.6/lib/site_perl/5.13.6/i686-linux/DBI.pm l ine 1896. Martin -- Martin J. Evans Wetherby, UK
It appears Caelum has made my example fail by adding SELECT SCOPE_IDENTITY() to the end of the prepared statement. I'm not sure about this now. Also Caelum said something that suggested the select has been taken out of dbic but only in his branch. I still don't like: row=0 p1 ind=-3 /1/ row=0 p2 ind=-3 /wiggle/ row=1 p1 ind=-3 /2/ row=1 p2 ind=-3 /woggle/ row=2 p1 ind=-3 /3/ row=2 p2 ind=-3 /boggle/ row=3 p1 ind=-3 /4/ row=3 p2 ind=-3 /fRIOUX/ row=4 p1 ind=-3 /5/ row=4 p2 ind=-3 /fRUE/ row=5 p1 ind=-3 /6/ row=5 p2 ind=-3 /fREW/ row=6 p1 ind=-3 /7/ row=6 p2 ind=-3 /fROOH/ row=7 p1 ind=-3 /8/ row=7 p2 ind=-3 /fISMBoC/ row=8 p1 ind=-3 /9/ row=8 p2 ind=-3 /station/ row=9 p1 ind=-3 /10/ row=9 p2 ind=-3 /mirror/ params processed = 1 row 0, parameter status = 0 row 1, parameter status = 2778 +get_row_diag for row 2 row 2, parameter status = 30062 +get_row_diag for row 3 row 3, parameter status = 8301 +get_row_diag for row 4 row 4, parameter status = 24944 +get_row_diag for row 5 row 5, parameter status = 24946 +get_row_diag for row 6 row 6, parameter status = 29549 +get_row_diag for row 7 row 7, parameter status = 61 +get_row_diag for row 8 row 8, parameter status = 10344 +get_row_diag for row 9 row 9, parameter status = 0 The only parameter status which looks right is the first - 0 = SQL_PARAM_SUCCESS. The others are very suspicious suggesting either they were never set of this freeTDS and DBD::ODBC are built with different size SQLULEN. Lastly, Caelum found my example above + the select works with MS native client: https://gist.github.com/2063477 #!/usr/bin/env perl use DBI; use strict; use warnings; my $h = DBI->connect(@ENV{map "DBICTEST_MSSQL_ODBC_$_", qw/DSN USER PASS/}, {RaiseError => 1, PrintError => 0} ); eval { $h->do(q/drop table owners/); }; $h->do(q/create table owners (id int identity (1,1) not null, name varchar(100))/); my $s = $h->prepare_cached(<<"EOF"); SET IDENTITY_INSERT owners ON insert into owners (id, name) values (?,?) SET IDENTITY_INSERT owners OFF SELECT SCOPE_IDENTITY() EOF my @data = ([1,2,3,4,5,6,7,8,9,10,11,12,13,14,15], ['wiggle', 'woggle', 'boggle', 'fRIOUX', 'fRUE', 'fREW', 'fROOH', 'fISMBoC', 'station', 'mirror', 'dimly', 'face_to_face', 'icarus', 'dream', 'dyrstyggyr']); $s->execute_array(undef, @data); eval { $h->do(q/drop table owners/); }; So, where to go now? Martin -- Martin J. Evans Wetherby, UK
On Sat Mar 17 14:04:40 2012, MJEVANS wrote: Show quoted text
> It appears Caelum has made my example fail by adding SELECT > SCOPE_IDENTITY() to the end of the prepared statement. I'm not sure > about this now. Also Caelum said something that suggested the select has > been taken out of dbic but only in his branch. I still don't like: > > row=0 p1 ind=-3 /1/ > row=0 p2 ind=-3 /wiggle/ > row=1 p1 ind=-3 /2/ > row=1 p2 ind=-3 /woggle/ > row=2 p1 ind=-3 /3/ > row=2 p2 ind=-3 /boggle/ > row=3 p1 ind=-3 /4/ > row=3 p2 ind=-3 /fRIOUX/ > row=4 p1 ind=-3 /5/ > row=4 p2 ind=-3 /fRUE/ > row=5 p1 ind=-3 /6/ > row=5 p2 ind=-3 /fREW/ > row=6 p1 ind=-3 /7/ > row=6 p2 ind=-3 /fROOH/ > row=7 p1 ind=-3 /8/ > row=7 p2 ind=-3 /fISMBoC/ > row=8 p1 ind=-3 /9/ > row=8 p2 ind=-3 /station/ > row=9 p1 ind=-3 /10/ > row=9 p2 ind=-3 /mirror/ > params processed = 1 > row 0, parameter status = 0 > row 1, parameter status = 2778 > +get_row_diag for row 2 > row 2, parameter status = 30062 > +get_row_diag for row 3 > row 3, parameter status = 8301 > +get_row_diag for row 4 > row 4, parameter status = 24944 > +get_row_diag for row 5 > row 5, parameter status = 24946 > +get_row_diag for row 6 > row 6, parameter status = 29549 > +get_row_diag for row 7 > row 7, parameter status = 61 > +get_row_diag for row 8 > row 8, parameter status = 10344 > +get_row_diag for row 9 > row 9, parameter status = 0 > > The only parameter status which looks right is the first - 0 = > SQL_PARAM_SUCCESS. The others are very suspicious suggesting either they > were never set of this freeTDS and DBD::ODBC are built with different > size SQLULEN.
Nonesense but possible. Long answer: The actual problem in this case is that freeTDS (or MS SQL Server ODBC driver) actually only processes the first row of parameters (i.e., params processed is 1) but DBD::ODBC assumes all values in the parameter status array will be set (incorrectly, only params processed rows in param status array are valid). That is why the parameter status array after row 1 is nonesense. I've fixed this (not checked in yet) although it won't make any difference to the case in point as issuing a select in the SQL with array bound parameters is a HELL-WHAT-TO-DO-WITH-THIS situation. Think about it. If you bind a row of parameters to an insert followed by a select at what point does the select get processed - the ODBC driver is simply pumping rows of parameters after the SQL and there is nothing in the spec to say how you break in to this to process the select i.e., in ODBC terms, once the rows of parameters are bound all that happens is SQLExecute is called and the driver is supposed to pass all the arrays of parameters in perhaps in a single block - there is nothing to interrupt this and handle the select. Short answer: Don't run what appears like multiple statements i.e, sql_do_this; sql_now_something_else; because a) the handling of it is driver dependent (note 1) and b) some drivers batch the sql up. note 1: in particular, don't mix insert/update/delete and a select as few ODBC drivers will handle this in a reasonable way and if you bind arrays of parameters (execute_for_fetch/execute_array) there is no way to interrupt the parameters and retrieve the result-set. Succinctly, there is no way DBD::ODBC is going to support multiple statements passed to prepare as the way it is handled in ODBC drivers is too diverse, unpredicatable and not defined clearly enough in ODBC spec. Consider this one: a procedure which takes an input parameter and returns one output parameter. In ODBC, the output parameter cannot be valid until the procedure is finished since anything in the proc could change the output parameter. Also, a proc can issue multiple inserts/updates/deletes/selects. As a result, calling a procedure that does insert this, select this, select that, insert this is only meaningful when SQLMoreResults is called as the result-set changes repeatedly during the procedure so you do: prepare({call proc fred (?,?)}); bind_param(1, input_param) bind_param(2, output_param) execute; do { fetch rows from different result sets of possibly different columns } while odbc_more_results; output param is valid now Now imagine what happens when you do this with execute_array - i.e., the procedure is called multiple times with each row of params but it all kicks off with one ODBC call SQLExecute - I daren't even think about how the ODBC driver never mind DBD::ODBC would handle this. In fact when this came up with the author of our SQL Server ODBC driver it was a walk away and never come back mmoment. Martin -- Martin J. Evans Wetherby, UK
I fixed the only true ODBC issue in this rt in 1.36_1. There is no way DBD::ODBC is going to support mixing inserts/deletes/updates/selects in the same statement - reason above. 1.36_2 will revert default to no array operations although strictly speaking all that defaulting it to on, in the case of this rt, is highlight issues in freeTDS. It is a shame I'm having to do this. I knew there would be issues in some ODBC drivers but I thought highlighting them would hopefully put pressure on people to fix them. Instead it seems I've just added to my work by having to keep a list of broken drivers and disable array operations for each of them to hide the bugs in them - sigh. I've attempted to find some sort of middle ground by pulling the common code out of the 70execute_array.t test and splitting it in to 2 - one which does the default DBI execute* and one which forces internal DBD::ODBC array operations. In this way if you run the tests to a real driver it still shows the bugs in the driver. However, as no one bothers to set up a real scenario when installing DBD::ODBC the test suite is useless in this respect. BTW, Peter, above comment is not directed at you - just my moaning about the sad state of some open source drivers. Will write this off when 1.36_2 appears on rt unless you come back to me. Martin -- Martin J. Evans Wetherby, UK
On Sat Mar 31 08:32:02 2012, MJEVANS wrote: Show quoted text
> I fixed the only true ODBC issue in this rt in 1.36_1. > > There is no way DBD::ODBC is going to support mixing > inserts/deletes/updates/selects in the same statement - reason above. > > 1.36_2 will revert default to no array operations although strictly > speaking all that defaulting it to on, in the case of this rt, is > highlight issues in freeTDS.
Above wraps this up a long with a 1.37 release which reverts default to no array operations. Martin -- Martin J. Evans Wetherby, UK