Proposed change to disconnect in DBD::ODBC

2011-03-16 Thread Martin J. Evans

DBI does not define what happens wrt transactions when disconnect is called.

Up to DBD::ODBC 1.29, DBD::ODBC always called SQLEndTran(SQL_ROLLBACK) to roll 
back any outstanding transactions (if AutoCommit was off) before calling 
SQLDisconnect. This means the outstanding transaction that is rolled back is 
hidden and that there is a difference between:

my $h = DBI-connect(dbi:ODBC:test,test,test, {AutoCommit = 0, 
RaiseError=1, PrintWarn = 1});
$h-do(q/insert into mje values(?, ?)/, undef, 1, fred);
# disconnect not called

and

my $h = DBI-connect(dbi:ODBC:test,test,test, {AutoCommit = 0, 
RaiseError=1, PrintWarn = 1});
$h-do(q/insert into mje values(?, ?)/, undef, 1, fred);
$h-disconnect or die $DBI::errstr;

because in the first case DBI catches the possible outstanding txn in DESTROY 
and issues:

Issuing rollback() due to DESTROY without explicit disconnect() of 
DBD::ODBC::db handle test.

but in the second case DBD::ODBC will rollback the transaction before 
disconnecting and there is no error or warning.

I don't really like this behaviour and would like to change it so in the second 
case above, DBD::ODBC issues a warning before rolling the txn back so you'd get 
something like:

DBD::ODBC::db disconnect warning: Disconnect with transaction in progress - 
rolling back

Obviously if you have PrintWarn turned off or are not using warnings it won't 
make any difference to you. Anyone have any fundamental objections to this 
change.

Martin
--
Martin J. Evans
Easysoft Limited
http://www.easysoft.com


Re: Proposed change to disconnect in DBD::ODBC

2011-03-16 Thread Martin J. Evans

On 16/03/11 16:07, Greg Sabino Mullane wrote:

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160



but in the second case DBD::ODBC will rollback the transaction before
disconnecting and there is no error or warning.
I don't really like this behaviour and would like to change it so in the second
case above, DBD::ODBC issues a warning before rolling the txn back so you'd get
something like:

DBD::ODBC::db disconnect warning: Disconnect with transaction in progress - 
rolling back


I don't know - it seems that someone issuing an explicit $dbh-disconnect() is 
far
different from a script just randomly exiting for one reason or another.


oh, yes, I'd agree with that.


I'd
also like to keep DBI consistent as much as possible, and I don't know of any
other DBDs that issue a warning on disconnect. So a -1 on this proposal, or at
least maybe a very weak +1 to putting such a thing inside DBI rather than
DBD::ODBC if others agree on this new behavior.



Thanks for the response Greg.

This area is already inconsistent. From DBI under disconnect:

The transaction behaviour of the disconnect method is, sadly, undefined. Some 
database systems (such as Oracle and Ingres) will automatically commit any outstanding 
changes, but others (such as Informix) will rollback any outstanding changes.

I tried it with DBD::Oracle using the same code as my DBD::ODBC example and in 
case 1 the transaction is not committed and in case 2 it is. That differs from 
the way DBD::ODBC has always been which is to rollback on disconnect no matter 
what. Note, I'm not suggesting a change to DBD::ODBC rolling back but I could 
just as easily make it commit like DBD::Oracle.

I'm not sure how we'd get DBI to control this I don't think it knows - all it 
does on DESTROY right now is to call rollback if disconnect has not been 
called. However, DBD::ODBC knows when disconnect is called and it calls 
SQLDisconnect which errors with state 25000 that a txn was in progress.

The reason I was putting this forward is that I got caught out by this and I 
thought I'd rather see at least a warning.

DBI also says
Generally, if you want your changes to be committed or rolled back when you disconnect, then you should 
explicitly call commit or rollback before disconnecting.

I'm still of the feeling that calling disconnect without committing or rolling 
back is probably a mistake (and almost definitely a mistake if your code could 
use multiple DBDs).

Martin
--
Martin J. Evans
Easysoft Limited
http://www.easysoft.com


Re: Proposed change to disconnect in DBD::ODBC

2011-03-16 Thread David Nicol
A non-fatal warning is the right thing here.


   do nothing   |warn |die
notifies in new dev no  |  yes |   yes
darkpan safeyes |   probably |no





-- 
This is not a 'bug'. Being documented, it is merely a 'restriction'. --
Intercal manual


Re: Proposed change to disconnect in DBD::ODBC

2011-03-16 Thread Greg Sabino Mullane

-BEGIN PGP SIGNED MESSAGE-
Hash: RIPEMD160


 This area is already inconsistent. From DBI under disconnect:

 The transaction behaviour of the disconnect method is, sadly, 
 undefined. Some database systems (such as Oracle and Ingres) 
 will automatically commit any outstanding changes,

Aside: COMMIT is an insane default. :)

 but others (such as Informix) will rollback any outstanding changes.

Right, I'm not saying the rollback or commit can be standardized, I'm 
saying we need to standardize on whether we throw a warning or not. 
Right now DBI does *not* give any warnings, and I presume this was 
done on purpose. But see below...

 I'm not sure how we'd get DBI to control this I don't think it knows 
 - all it does on DESTROY right now is to call rollback if disconnect 
 has not been called. However, DBD::ODBC knows when disconnect is 
 called and it calls SQLDisconnect which errors with state 25000 
 that a txn was in progress.

Right, it's completely up to the drivers to control the behavior, 
but DBI can certainly handle the warning/notification: maybe 
something generic enough that could be overriden by a driver? Maybe 
that's why nothing was ever printed, as it would be Disconnect 
called with an open transaction, so we committed it OR rolled it 
back (consult your DBD docs). Hmmm...I wonder if rather than overriding 
the message, the driver could provide some sort of method such that 
DBI knows the commit/rollback behavior. Which could in theory be 
useful in other places.

 DBI also says
 Generally, if you want your changes to be committed or rolled 
 back when you disconnect, then you should explicitly call 
 commit or rollback before disconnecting.

 I'm still of the feeling that calling disconnect without 
 committing or rolling back is probably a mistake (and almost 
 definitely a mistake if your code could use multiple DBDs).

With multiple, yes, but with a single one that does something sane 
(in other words, a rollback) I think it's a valid shortcut. But 
I would certainly be okay with adding a warning.

- -- 
Greg Sabino Mullane g...@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201103162039
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-BEGIN PGP SIGNATURE-

iEYEAREDAAYFAk2BWHoACgkQvJuQZxSWSsjC8gCgu9YVyeCH6K2I3VkS09KMilFE
qHMAniearHW1omu0aA2ZPmLasklFNPiT
=ftn+
-END PGP SIGNATURE-