pashagolub / postgresdac

Delphi/C++Builder direct access components for PostgreSQL (and derivatives)
MIT License
19 stars 12 forks source link

A lot of operations without checking _PQexecute result #16

Closed macc2010 closed 4 months ago

macc2010 commented 7 months ago

Hello,

I have seen that there are several _PQexecute that are not checking the result of the operation.

For example :

image

pashagolub commented 7 months ago

Sorry, I don't understand. Would you please use code instead of screenshots?

macc2010 commented 7 months ago

Sorry,

When this code is executed :

procedure TNativeDataSet.AddIndex(var IdxDesc: IDXDesc; pszKeyviolName : string);
var
  Result : PPGResult;

 function CreateSQLForAddIndex: String;
 var
   Fld : String;
   PSQLIdxs : TPSQLIndexes;
 begin
   Result := '';
   PSQLIdxs := TPSQLIndexes.Create(nil);
   TPSQLIndex.CreateIndex(PSQLIdxs,@IdxDesc);
   Fld := SQLCreateIdxStr(PSQLIdxs[1],TableName,Fields);
   Result := Result+Fld;
   PSQLIdxs.Free;
 end;

begin
  if not Assigned(FConnect) or not (FConnect.FLoggin) then  Exit;
  Result := _PQexecute(FConnect, CreateSQLForAddIndex);
  if Result <> nil  then
  begin
    PQclear(Result);
  end else
    FConnect.CheckResult;
end;

As the result of calling TPSQLTable.AddIndex

If an error is returned by the server, the function does not raise the error because this code :

  Result := _PQexecute(FConnect, CreateSQLForAddIndex);
  if Result <> nil  then
  begin
    PQclear(Result);
  end else
    FConnect.CheckResult;

Should be changed to :

  if not Assigned(FConnect) or not (FConnect.FLoggin) then  Exit;
  Result := _PQexecute(FConnect, CreateSQLForAddIndex);
  if Result <> nil  then
  begin
    try
      FConnect.CheckResult( Result ); // --> added
    finally
      PQclear(Result);
    end;
  end else
    FConnect.CheckResult;

to raise the error returned by the server.

pashagolub commented 7 months ago

The logic here is simple. If PQexecute returned result, that means operation was successful and we can PQclear it without investigations.

pashagolub commented 7 months ago

But probably you're right. There is a sense to check for error, just in case. Would you like to create pull request?

github-actions[bot] commented 5 months ago

📅 This issue has been automatically marked as stale because lack of recent activity. It will be closed if no further activity occurs. ♻️ If you think there is new information allowing us to address the issue, please reopen it and provide us with updated details. 🤝 Thank you for your contributions.