1

The following function takes the cuits (the cuit is like social security number) from a grid and inserts them into a temporary table. I'm using Delphi XE7 and Firebird 2.5.

function TfImportFileARBARetenPercep.fxListClientFromGrid(
      pboClient: Boolean): WideString;
var
   wsAux : WideString;
   stTable, stCuit: String;
   qCliProv, qCuitsExcl, qCuit : TFXQuery;
   niRow : Integer;
begin
  wsAux := '';

  qCuitsExcl.SQL.Text := 'Create global temporary table TempExcl (cuitExcl varchar(13)) on commit delete rows';
  qCuitsExcl.ExecSQL;

  if pboClient then
  begin
    stTable := 'Client'
  end
  else
  begin
    stTable := 'Prov';

  end;
  for niRow := 1 to gDetails.RowCount - 1 do
    if Trim(gDetails.Cells[3,niRow]) <> '' then
    Begin
      stCuit := QuotedStr(Trim(gDetails.Cells[3,niRow]));
      qCuit.SQL.Text := 'Insert into TempExcl(:cuitExcl)';
      qCuit.ParamByName('cuitExcl').AsString := stCuit;
      qCuit.ExecSQL;//←←←←←←←←←←←←←←←←this line throws the exception
    End;

  qCuitsExcl.SQL.Text :='';
  qCuitsExcl.SQL.Text := 'commit;';//to commit previous inserts
  qCuitsExcl.SQL.Add('drop table TempExcl;');//in case the table is not deleted when the connection is closed
  qCuitsExcl.SQL.Add('commit;');//commit previous drop
  qCuitsExcl.ExecSQL;

//the rest of the code only returns the list of cuits that were loaded 
↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓↓

  try
    qCliProv.SQL.Text :=
      ' Select Code' +
      '   From ' + stTable +
      '  Where Active = 1 ';

    if gDetails.RowCount  > 0 then
      begin
        qCliProv.SQL.Add('And Cuit Not In (Select cuitExcl from TempExcl)');
      end;

    qCliProv.Open;

    wsAux := '';

    while not qCliProv.Eof do
    begin
      wsAux := wsAux + iif((wsAux = ''), '', ',') + qCliProv.FieldByName('Code').AsString;
      qCliProv.Next;
    end;

    Result := wsAux;
  finally
    FreeAndNil(qCliProv);
    FreeAndNil(qCuit);
    FreeAndNil(qCuitsExcluidos);
  end;
end;

When trying to execute the line qCuit.ExecSQL; the following exception is thrown:

Project ERP.exe raised exception class EIBNativeException with message '[FireDAC][Phys][FB] Dynamic SQL Error SQL error code = -104 Token unknown - line 1, column 27 ?'.

I don't know why it throws this exception

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
Franco Torres
  • 253
  • 1
  • 6
  • You have tagged question with delphi-7 and inside question you are saying you are using XE7. Those are two very different versions. Winch one is it? – Dalija Prasnikar Mar 05 '21 at 12:08
  • Please post the error message as **text**. – Mark Rotteveel Mar 05 '21 at 12:12
  • Dalija Prasnikar sorry. I really did not know that it was 2 different versions. I'm new to pascal. Deleted tag. Now Change error message. – Franco Torres Mar 05 '21 at 12:41
  • @FrancoTorres and you also seem new to Interbase/Firebird and maybe to SQL as well. `And Cuit Not In (Select cuitExcl from TempExcl)` - this is quite a no-no-no on IB/FB unless data amount is really small (like 100 rows or less) – Arioch 'The Mar 05 '21 at 16:56
  • @FrancoTorres did you wrote this code from scratch, or did you inherited some code and now trying to update it? Because the code you show is not badly inconsistent, it seems to be dangerously so... Or at least, very very smelly... Frankly, unless Dmitry Ariefiev did some really dark voodoo your code should not even run once, less so run twice! So i expect your code there is not real. – Arioch 'The Mar 05 '21 at 17:03

1 Answers1

2

The problem is that this is wrong:

qCuit.SQL.Text := 'Insert into TempExcl(:cuitExcl)';

This will result in a generated query that is:

Insert into TempExcl(?)

Which is a syntax error (the "Token unknown"), because Firebird doesn't expect a question mark (parameter placeholder) in this position, it expects an identifier (column name).

A valid query would be either:

Insert into TempExcl (cuitExcl) values (?)

or:

Insert into TempExcl values (?)

The first form is preferred, as you explicitly specify the columns, though in this case not really necessary as the table has one column anyway.

In other words, you need to use:

qCuit.SQL.Text := 'Insert into TempExcl (cuitExcl) values (:cuitExcl)';

As an aside, I'm not familiar with Delphi or how and when queries are prepared, but it might be better to move this statement out of the loop, so it is prepared only once (in case Delphi prepares the statement again any time the qCuit.SQL.Text is assigned a value).

The comment "// in case the table is not deleted when the connection is closed" seems to indicate a lack of understanding of Global Temporary Tables. They are intended to be permanent objects, available for re-use by your application. That is on commit delete rows makes the content available only to your current transaction, while on commit preserve rows will make the content available for the remainder of your connection (and only your connection), deleting the data when the connection is closed. Creating and dropping a GTT in something that is executed more than once is an indication something is wrong in your design.

Mark Rotteveel
  • 100,966
  • 191
  • 140
  • 197
  • Also creation of the temporary table is better to be moved out of the application. Or better yet completely removed and SELECT in "the rest of code" to use parameter value from gDetails directly. – user13964273 Mar 05 '21 at 12:41
  • @user13964273 You're right, I added another comment to that effect. – Mark Rotteveel Mar 05 '21 at 12:49
  • With your code I corrected that error but now it throws me another error. The error message: "SQL error code = -204 Table unknown TEMPEXCL At line 1, column 13". How is this possible if the table was created before. – Franco Torres Mar 05 '21 at 13:39
  • @FrancoTorres You do not commit after creating the GTT, objects are not available in the transaction that created it. And you also drop it immediately after populating it, so it no longer exists when you try to use it in the select. This is why you shouldn't create GTTs on the fly, but instead create them once, and reuse them just like normal tables. – Mark Rotteveel Mar 05 '21 at 14:17