1

I am trying to update two different SQL tables in the same loop using parameterized queries in Delphi XE8. I also want to wrap the whole thing in a transaction, so that if anything in the loop fails, neither table gets updated.

I don't really know what I'm doing, would appreciate some help.

The code below is a simplified version of what I'm trying to achieve, and my best guess as to how to go about it. But I'm not really sure of it at all, particularly the use of two datasets connected to the 'SQL connection' component.

SQL_transaction.TransactionID :=1;
SQL_transaction.IsolationLevel:=xilREADCOMMITTED;
SQL_connection.BeginTransaction;
Try
  { Create connections }

  SQL_dataset1              :=TSQLDataSet.Create(nil);  
  SQL_dataset1.SQLConnection:=SQL_connection;

  SQL_dataset2              :=TSQLDataSet.Create(nil);  
  SQL_dataset2.SQLConnection:=SQL_connection;

  { Create queries }

  SQL_dataset1.CommandType:=ctQuery;
  SQL_dataset1.CommandText:={ some parameterized query updating table A }

  SQL_dataset2.CommandType:=ctQuery;
  SQL_dataset2.CommandText:={ some parameterized query updating table B }

  { Populate parameters and execute }

  For I:=0 to whatever do
  begin
    SQL_dataset1.ParamByName('Table A Field 1').AsString:='Value';
    SQL_dataset1.ExecSQL; 

    SQL_dataset2.ParamByName('Table B Field 1').AsString:='Value';
    SQL_dataset2.ExecSQL; 
  end;

  SQL_connection.Commit(SQL_transaction);
except
  SQL_connection.Rollback(SQL_transaction);  
end;

I am using Delphi XE8, and the database can be either SQL server or SQLite.

Alex
  • 543
  • 1
  • 9
  • 21

2 Answers2

4

The logic of your transaction handling is correct (except the missing exception re-raise mentioned by @whosrdaddy). What is wrong are missing try..finally blocks for your dataset instances. Except that you should stop using TSQLConnection deprecated methods that are using the TTransactinDesc record (always check the compiler warnings when you're building your app.). And you can also switch to TSQLQuery component. Try something like this instead:

var
  I: Integer;
  Query1: TSQLQuery;
  Query2: TSQLQuery;
  Connection: TSQLConnection;
  Transaction: TDBXTransaction;
begin
  ...  
  Query1 := TSQLQuery.Create(nil);
  try
    Query1.SQLConnection := Connection;
    Query1.SQL.Text := '...';

    Query2 := TSQLQuery.Create(nil);
    try
      Query2.SQLConnection := Connection;
      Query2.SQL.Text := '...';

      Transaction := Connection.BeginTransaction;
      try
        // fill params here and execute the commands
        for I := 0 to 42 to
        begin
          Query1.ExecSQL;
          Query2.ExecSQL;
        end;
        // commit if everything went right
        Connection.CommitFreeAndNil(Transaction);
      except
        // rollback at failure, and re-raise the exception
        Connection.RollbackFreeAndNil(Transaction);
        raise;
      end;
    finally
      Query2.Free;
    end;
  finally
    Query1.Free;
  end;
end;
TLama
  • 75,147
  • 17
  • 214
  • 392
  • If you initialize the datasets to nil before the try finally block, you only need one try finally block to free the datasets. – House of Dexter Jul 13 '15 at 14:55
  • @House, you should still assume that releasing the inner one fails on exception. – TLama Jul 13 '15 at 14:57
  • Free checks to see if they are nil before freeing the object...if something happens on the inner...and the outer never gets set...its ref. will be nil and calling free on it will not do anything... – House of Dexter Jul 13 '15 at 15:08
  • @House, I know what `Free` does. But this is not about initialization. It's about releasing. If any of your destructors raise an exception, following objects will leak. – TLama Jul 14 '15 at 06:35
  • If you have destructors that will raise an exception...you have more serious problems...The only time I wouldn't use the initialization try finally is if I had code that fired Destructor event code...In 15 years of Delphi programming I can say I've done less than I can count on 1 hand... – House of Dexter Jul 14 '15 at 13:50
  • Try Try Finally Finally to me is more a Component Writer setup...where you have no idea how the user will use your component...and so you have to safeguard your resources...but for an application writer you should know what your code is doing. – House of Dexter Jul 14 '15 at 13:57
  • @House, yes, it is a safeguard. And of course you should know what your app. is doing. But I'm not afraid of *performance penalty* here. Why ? Because if some objects are supposed to be re-used, I won't free them but put back into an object pool (where possible). If this is the case, I don't know. – TLama Jul 14 '15 at 14:08
  • See...if you get in the object pool...then you can't guarantee the freeing of list objects...anything that uses for i to count -1do mylist[i].free is prone to the same problem. Because if any of the objects in the list raise an exception during the destruction...then rest of the objects will not be freed... – House of Dexter Jul 14 '15 at 16:11
-1

I prefer try finally over try except

here's how to make it work in a try finally block

var
  a_Error: boolean;
begin
a_Error := True;//set in error state...
SQL_dataset1 := nil;
SQL_dataset2 := nil;
SQL_transaction.TransactionID :=1;
SQL_transaction.IsolationLevel:=xilREADCOMMITTED;
SQL_connection.BeginTransaction;

Try
  { Create connections }

  SQL_dataset1              :=TSQLDataSet.Create(nil);  
  SQL_dataset1.SQLConnection:=SQL_connection;

  SQL_dataset2              :=TSQLDataSet.Create(nil);  
  SQL_dataset2.SQLConnection:=SQL_connection;

  { Create queries }

  SQL_dataset1.CommandType:=ctQuery;
  SQL_dataset1.CommandText:={ some parameterized query updating table A }

  SQL_dataset2.CommandType:=ctQuery;
  SQL_dataset2.CommandText:={ some parameterized query updating table B }

  { Populate parameters and execute }

  For I:=0 to whatever do
  begin
    SQL_dataset1.ParamByName('Table A Field 1').AsString:='Value';
    SQL_dataset1.ExecSQL; 

    SQL_dataset2.ParamByName('Table B Field 1').AsString:='Value';
    SQL_dataset2.ExecSQL; 
  end;

  a_Error := False;//if you don't get here you had a problem
finally
  if a_Error then
    SQL_connection.Rollback(SQL_transaction)
  else
    SQL_connection.Commit(SQL_transaction);
  SQL_dataset1.Free;
  SQL_dataset2.Free;

end;    
end;

I added some code on how Try Finally works with init objects to nil

  TMyObject = class(TObject)
    Name: string;
  end;

procedure TForm11.Button1Click(Sender: TObject);
var
  a_MyObject1, a_MyObject2: TMyObject;
begin
  a_MyObject1 := nil;
  a_MyObject2 := nil;
  try
    a_MyObject1 := TMyObject.Create;
    a_MyObject1.Name := 'Object1';
    if Sender = Button1 then    
      raise exception.Create('Object 2 not created');
    ShowMessage('We will not see this');
    a_MyObject2 := TMyObject.Create;
    a_MyObject2.Name := 'Object2';
  finally
    a_MyObject2.Free;
    ShowMessage('We will see this even though we called a_MyObject2.free on a nil object');
    a_MyObject1.Free;
  end;
end;
House of Dexter
  • 386
  • 1
  • 7
  • 2
    Ouch, that's nasty. You are using flags to convert a try-finally into a try-except. Why on earth would you do that? The language already has a language construct that has been added specifically to do that. It's the try-except block that you are replacing. – Graymatter Jul 10 '15 at 22:47
  • Actually why is it nasty...Try Except should be the exceptions not the norm, no worrying about reraising the exception...You handle your Transactions whether it's good or bad correctly...You should read Danny Thorpes book...in that app writers very seldom should write Try Except...and should write Try Finally... – House of Dexter Jul 13 '15 at 03:48
  • It also handles the freeing of the datasets correctly. It will be faster than wrapping the code in Try Try Try Except Finally Finally...You should read Page 112-114 of Delphi Component Design by Danny Thorpe. – House of Dexter Jul 13 '15 at 14:50