Zeoslib Portal

bug report: AutoCommit issues in ADO

[problem]
there is a set of broblems with transactioning using ADO.
it would be clear to quote and comment source rather then
to describe those bugs.

======== [dbc\ZDbcAdo.pas] ========
// everything is OK here
procedure TZAdoConnection.ReStartTransactionSupport;
begin
if Closed then Exit;
if not (AutoCommit or (GetTransactionIsolation = tiNone)) then
StartTransaction;
end;

//here goes the bug
procedure TZAdoConnection.SetAutoCommit(AutoCommit: Boolean);
begin
if GetAutoCommit = AutoCommit then Exit;
if not Closed and AutoCommit then
begin
if FAdoConnection.State = adStateOpen then

// fails if GetTransactionIsolation = tiNone

FAdoConnection.CommitTrans;

// fails because no transaction had been started

DriverManager.LogMessage(lcExecute, FPLainDriver.GetProtocol, 'COMMIT');
end;
inherited;
ReStartTransactionSupport;
end;

so the problem is that we do not hanlde AutoCommit in the right way
with GetTransactionIsolation = tiNone (and this is the default value of the property)

[suggested solution]

the solution is clear.

(origin)
{
if FAdoConnection.State = adStateOpen then
FAdoConnection.CommitTrans;
DriverManager.LogMessage(lcExecute, FPLainDriver.GetProtocol, 'COMMIT');
}
(changes into)
{
if FAdoConnection.State = adStateOpen then
if GetTransactionIsolation <> tiNone then
begin
FAdoConnection.CommitTrans;
DriverManager.LogMessage(lcExecute, FPLainDriver.GetProtocol, 'COMMIT');
end;
}

BUT !!

there is still a way to make it even better.

the first issue is the FAdoConnection.CommitTrans expression.
its presence means the following:

- if we had AutoCommit=false and did some posts into session and then we
set AutoCommit into True - all our changes become commited (just watch the code)

but i think it's better to rollback changes when we change AutoCommit into True.
and it should be the application logic which will execute commit before it changes
the autocommit value.

the second issue is the format of TZAdoConnection.SetAutoCommit method call.
THIS CODE STINKS !!!
we have a property named AutoCommit and pass a parameter into method with the
same name. and then we heroically resolve this naming collision accessing
AutoCommit property value with GetAutoCommit method.
what the property paradigm has beed developed for ?

so the final suggestion is:

(scope)
======== [dbc\ZDbcConnection.pas] ========
procedure TZAbstractConnection.SetAutoCommit(AutoCommit: Boolean);
- change format to make code nice
(origin)
{
procedure TZAbstractConnection.SetAutoCommit(AutoCommit: Boolean);
begin
FAutoCommit := AutoCommit;
end;
}
(change into)
{
procedure TZAbstractConnection.SetAutoCommit(AValue: Boolean);
begin
FAutoCommit := AValue;
end;
}


(scope)
======== [dbc\ZDbcAdo.pas] ========
procedure TZAdoConnection.SetAutoCommit(AutoCommit: Boolean);
(origin)
{
procedure TZAdoConnection.SetAutoCommit(AutoCommit: Boolean);
begin
if GetAutoCommit = AutoCommit then Exit;
if not Closed and AutoCommit then
begin
if FAdoConnection.State = adStateOpen then
FAdoConnection.CommitTrans;
DriverManager.LogMessage(lcExecute, FPLainDriver.GetProtocol, 'COMMIT');
end;
inherited;
ReStartTransactionSupport;
end;
}
(change into)
{
procedure TZAdoConnection.SetAutoCommit(AValue: Boolean);
begin
if AutoCommit = AValue then Exit;
if not Closed and AValue then
begin
if FAdoConnection.State = adStateOpen then
if GetTransactionIsolation <> tiNone then
begin
FAdoConnection.RollBackTrans;
// maybe FAdoConnection.CommitTrans; if you like.
// but i think it is better to rollback it changing AutoCommit property.
DriverManager.LogMessage(lcExecute, FPLainDriver.GetProtocol, 'ROLLBACK');
end;
end;
inherited SetAutoCommit(AValue);
ReStartTransactionSupport;
end;
}

[platform notes]
zeosdbo-6.6.1-beta
protocol: ado
delphi7
Microsoft SQL server 2000

best wishes.

Zx,

I understand the only bug is the missing 'if GetTransactionIsolation <> tiNone then'.
The two other topics are
- 'coding style' : agreed, but i think this naming looks quite consistent in implementations of this function. So change them all and send me a svn diff, please. I do not change it now for ADO only.
- 'commit or rollback when setting autocommit' : for me (personally) commit sounds more logic when I set autocommit to true. I agree it would be smarter if everybody decides himself before changing the Autocommit modus. This is a neverending discussion, so I won't change it because of this report.

I hope you can live with this decision. I did change the code a little, but think it's still OK.
SVN Rev. 261.