Bug in SQLdb...?

[Sieben]

Es scheint da einen hässlichen Bug in TBufDataset zu geben, zu dem es zu meiner Verwunderung aber noch kein Ticket zu geben scheint, daher würde ich mal kurz um Bestätigung bitten, bevor ich eins einreiche. Einfach eine TSQLQuery nehmen, IndexFieldNames auf ein passendes Feld setzen und dann Refresh aufrufen.

[wp_xyz]

Es scheint da einen hässlichen Bug in TBufDataset zu geben, zu dem es zu meiner Verwunderung aber noch kein Ticket zu geben scheint, daher würde ich mal kurz um Bestätigung bitten, bevor ich eins einreiche. Einfach eine TSQLQuery nehmen, IndexFieldNames auf ein passendes Feld setzen und dann Refresh aufrufen.

Und was passiert dann?

[Sieben]

Bei mir gibt es eine allgemeine Schutzverletzung, 'External SIGSEGV'.

RefreshBug.png (49.27 KiB) 2756 mal betrachtet

[af0815]

Kommt mir irgendwie bekannt vor. Ich verwende seit Jahren kein Refresh.

[charlytango]

, zu dem es zu meiner Verwunderung aber noch kein Ticket zu geben scheint,

stimmt nicht ganz:
https://gitlab.com/freepascal.org/fpc/s ... sues/15460

Und hier gibts auch noch etwas:
https://forum.lazarus.freepascal.org/in ... ic=17465.0

Ich wundere mich warum mir IndexFieldNames nie über den Weg lief.
Die Antwort ist, ich hab es nie benutzt und auch nicht vermisst, weil ich Sortierungen immer per SQL (SELECT ... ORDER BY xy asc/desc) gemacht habe.

Offensichtlich hab ich es auch in TBufDataset nie gebraucht ... grübel...

[Sieben]

Danke für den Hinweis, aber das ist ein anderes Problem und offenbar auch behoben und geschlossen. Trotzdem merkwürdig, dass ich den nicht gefunden habe:

RefreshBug2.png (37.71 KiB) 2734 mal betrachtet

EDIT: Auch der zweite Hinweis trifft es nicht ganz. Ich persönlich finde den CustomIndex über IndexFieldNames ausgesprochen hilfreich, gerade auch weil er eine erneute Anfrage an die Datenbank vermeidet. Und ein Workaround für diesen Bug ist auch schnell geschrieben. Testen mag das aber offenbar niemand mal kurz...?

[wp_xyz]

Das beigefügte Projekt ist eine abgespeckte Version einer sqlite3-Demo, die ich mal im Internet gefunden habe. Damit kann Ich den Fehler weiter eingrenzen. Und zwar erfolgt der Absturz in TCustomBufDataset.ProcessFieldsToCompareStruct (unit fcl-db/src/BufDataset.pas)

Code: Alles auswählen

procedure TCustomBufDataset.ProcessFieldsToCompareStruct(const AFields, ADescFields, ACInsFields: TList;
      const AIndexOptions: TIndexOptions; const ALocateOptions: TLocateOptions; out ACompareStruct: TDBCompareStruct);
begin
  SetLength(ACompareStruct, AFields.Count);
  for i:=0 to high(ACompareStruct) do
    begin
      [...]
      ACompareRec.Off:=BufferOffset + FFieldBufPositions[AField.FieldNo-1];  // Absturz hier
      [...]  

Das LongInt-Array FFieldBufPositions hat hier nach dem Aufruf von Refresh die Länge 0, damit knallt es natürlich bei der Abfrage von FFieldBufPositions[AField.FieldNo-1].

Zur Bedienung des Testprogramms noch: Ein Klick auf einem Spalten-Header weist dessen Feld den IndexFieldNames der SQLQuery zu. Das funktioniert mit jeder Spalte - an den IndexFieldNames allein kann der Fehler nicht liegen, und daher meine ich auch, die die schon genannten Bugreport ein anderes Problem beschreiben. Klickt man aber nach dem Setzen der IndexFieldNames auf den Refresh-Button, hat man den Absturz.

Für die weitere Fehlersuche müsste man das Projekt vereinfachen, denn das permanente Neuübersetzen von FPC nach dem Einbauen von Test-Code in the BufDataset-Unit nervt gewaltig. Ich denke, man müsste in dem Test-Projekt SQLQuery durch einen BufDataset ersetzen (damit man weniger Abhängigkeiten hat). Wenn dann der Fehler noch auftritt, würde ich die benötigen Datenbank-Units (db, BufDataset, ...) ins Projektverzeichnis kopieren, so dass man Änderungen daran gleich nach einem Compilier-Lauf beurteilen kann.

[Sieben]

Ich denke auch dass der Fehler in TCustomBufDataset liegt, TSQLQuery 'überschreibt' zwar auch InternalRefresh, fügt aber nichts in dieser Hinsicht relevantes hinzu.

Magst du dann das Ticket einreichen, wenn du schon die Fehlerstelle gefunden hast...? Wenn es da so etwas wie eine Verteilerliste gibt, wäre ich aber gern mit drauf - ich habe dort zwar auch einen Account unter demselben Namen wie hier, habe aber seit Mantis nichts mehr selbst eingereicht.

Und gibt es vielleicht irgendwo eine Kurzanleitung für das Kompilieren der FCL mit Debug-Informationen? Wobei ich auf den Trichter, einfach die Unit in's Projektverzeichnis zu holen, leider auch nicht gekommen bin.

[wp_xyz]

Hier nun die debugger-freundliche Version, ein einfaches Kommandozeilen-Programm, das einen TBufDataset erzeugt, ihn mit Dummy-Daten füllt, dann nach dem Öffnen die IndexFieldNames auf einen der Feldnamen setzt und schließlich Refresh aufruft. Damit man leicht Debuggen kann, habe ich die FCL-Datenbank-Units aus dem Ordner fcl-db/src/base sowie zwei benötigte aus fcl-db/src/dbase in den Projekt-Ordner fcl kopiert, den ich in den Unit-Pfad aufgenommen habe. Dadurch werden diese lokal verfügbaren Units beim Compilieren und Debuggen verwendet, statt der Original-FCL-Units.

Ich habe vor dem Kopieren nach das FPC-Repository aktualisert, so dass man gleich mit den aktuellen Main-Units arbeitet. Beim Kompilieren gab es ein paar Inkompatibilitäten mit meinem FPC 3.2.2, diese nicht relevanten Zeilen habe ich auskommentiert.

Startet man das Testprojekt hat man tatsächlich einen Absturz in der Refresh-Anweisung. Allerdings ist die Fehlermeldung anders: "Must apply updates before refreshing data". Das kommt aus TBufDataset.InternalRefresh, das am Anfang die Anweisung

Code: Alles auswählen

  if length(FUpdateBuffer)>0 then
    DatabaseError(SErrApplyUpdBeforeRefresh,Self);

enthält. Das im ersten Test-Projekt verwendete TSQLQuery löscht den UpdateBuffer in der dort überschriebenen InternalRefresh-Methode. Daher habe ich diese Zeilen auch auskommentiert, und nun bekomme ich dieselbe Fehlermeldung wie in dem sqlite3-Testprogramm.

Nun kann ich direkt verfolgen, wo FFieldBufPositions gelöscht wird. Das passiert in dem von InternalRefresh aufgerufenen InternalClose - irgendwie logisch. Allerdings bleibt FFieldBufPositions bis zur Crash-Stelle gelöscht.

Erzeugt wird FFieldBufPositions - natürlich - in InternalOpen, in der Routine CalcRecordSize. Aber leider erfolgt der Absturz eine Zeile früher in BuildCustomIndex, das im Fall von nicht-leeren IndexFieldDefs angespringen wird. Und damit bin ich mit meinem Latein am Ende. Aber vielleicht findet ja jemand anders, wie es weiter geht....

---

Den Bugreport kann ich gerne einreichen. Aber ich würde noch etwas warten, vielleicht finden wir noch einen Fix. Denn meiner Erfahrung nach werden Bugreports ohne Patch oft nicht bearbeitet.

[Sieben]

Besten Dank für das Test-Projekt, ich habe mir das jetzt nochmal angesehen und zuerst gedacht, man könnte doch CalcRecordSize einfach vor dem Index-Block aufrufen, wenn der das offensichtlich voraussetzt und CalcRecordSize keine weiteren Abhängigkeiten hat und einfach nur diesen FieldPosBuffer setzt. Crasht dann aber in Zeile 1194 beim ersten Zugriff auf PCurRecLinkItem nach der Zuweisung von Index0.FFirstRecBuf. Ich habe aber noch einen anderen groben Versuch unternommen, und die Zeilen 1471/1472 auskommentiert und den Aufruf hinter die Zeile FOpen := True kopiert, da mir das schon den Zustand zu versprechen scheint, den auch eine Zuweisung zur Laufzeit vorfindet. Das ganze sieht dann so aus:

Code: Alles auswählen

    ...
    InitDefaultIndexes;
    InitUserIndexes;
    If FIndexName<>'' then
      FCurrentIndexDef:=TBufDatasetIndex(FIndexes.Find(FIndexName));
    //else if (FIndexFieldNames<>'') then
      //BuildCustomIndex;

    CalcRecordSize;

    FBRecordCount := 0;

    for IndexNr:=0 to FIndexes.Count-1 do
      if Assigned(BufIndexdefs[IndexNr]) then
        With BufIndexes[IndexNr] do
          InitialiseSpareRecord(IntAllocRecordBuffer);

    FAllPacketsFetched := False;

    FOpen:=True;

    if (FIndexName='') and (FIndexFieldNames<>'') then //cf else above
      BuildCustomIndex;

    // parse filter expression
    ParseFilter(Filter);
    ...

Und das scheint klaglos und ohne bislang bemerkte Nebenwirkungen durchzulaufen, erfordert aber ganz sicher noch weitere Tests. Leider unterscheidet sich die Unit in deinem Projekt offenbar in Teilen signifikant von der mit 3.2.2 ausgelieferten Version und ich habe bislang nicht versucht, sie in meine Installation zu integrieren.

[Sieben]

Also, ich habe jetzt mal ein LCL-Projekt mit dem geänderten BufDataset kompiliert bekommen (eine ziemliche Kopierorgie, bevor man auch alle abhängigen LCL-Units im Projekt selbst hat) - und der Schnellschuß von gestern Nacht war dann wohl doch nix. Es knallt zwar nicht mehr, aber nach einem Refresh ist der Dataset leer, da FetchAll mit einem FPacketRecords von -1 angelaufen wird. Nicht hilfreich...

EDIT - Stimmt so auch wieder nicht, Suche geht weiter...

[Sieben]

Also noch mal von vorn: dass TBufDataset bei meinen ersten Tests die Records verloren hat, lag natürlich nicht an FPacketRecords = -1, sondern daran, dass ich zunächst ebenfalls auf einem InMemory-Dataset getestet habe und damit am Rückgabewert der Funktion Fetch - und letztlich natürlich daran, dass ein Refresh auf einem InMemory-Dataset schlicht keinen Sinn macht. Mit allen Ableitungen von TBufDataset - die Fetch brav und tunlichst überschreiben - scheint der Schnellschuß hingegen tatsächlich und ohne Nebenwirkungen zu funktionieren.

[wp_xyz]

Wie bist du mit dem in der von mir veränderten Version von TCustomBufDataset.InternalRefresh auskommentierten if-Block am Anfang umgegangen? Das war nur eine Notlösung, um weiterarbeiten zu können, aber eigentlich sollte dieser Block schon bleiben. Aus Ausweg könnte man bei TBufDataset eine eigene Version von InternalRefresh einführen, die so ähnlich arbeitet wie die bei TCustomSQLQuery:

Code: Alles auswählen

procedure TCustomBufDataset.InternalRefresh;
begin
  if (ChangeCount>0) then
    CancelUpdates;
  inherited InternalRefresh;
end;

Das wäre natürlich hinfällig, falls man bei TBufDataset prinzipiell gar kein Refresh machen kann. Oder hast du das mit FPacketRecords = -1 irgendwie behoben?

Mit allen Ableitungen von TBufDataset - die Fetch brav und tunlichst überschreiben - scheint der Schnellschuß hingegen tatsächlich und ohne Nebenwirkungen zu funktionieren.

Ich habe nur TSQLQuery und TCSVDataset als Nachfahren von TCustomBufDataset gefunden. Welche gibt es denn noch? Damit man dort spicken könnte...

[Sieben]

Wie bist du mit dem in der von mir veränderten Version von TCustomBufDataset.InternalRefresh auskommentierten if-Block am Anfang umgegangen?

Habe ich erstmal so gelassen, sehe aber auch nicht, dass das Probleme machen würde. EDIT - das würde allerdings bei InMemory-Verwendung dann immer den Error auslösen (ChangeCount wird ja nie zurückgesetzt nach der Bestückung mit Daten) - und würde so dann doch effektiv einen versehentlichen Refresh verhindern... Vielleicht könnte man die Fehlermeldung noch anpassen und dezent daran erinnern, dass es InMemory ist.

Das wäre natürlich hinfällig, falls man bei TBufDataset prinzipiell gar kein Refresh machen kann. Oder hast du das mit FPacketRecords = -1 irgendwie behoben?

Das mit den FPacketRecords war ein Hirnschwurbel meinerseits - das soll so und funktioniert auch so. Und einen Refresh auf einem 'puren' TBufDataset kann man schon machen - man verliert aber - logischerweise - alle Records beim InternalClose, und es gibt - normalerweise - keine Quelle, um sie nachzuladen. Das irgendwie abzufangen, dürfte schwierig bis unmöglich sein, insbesondere wenn man selbst eine Ableitung mit ein paar Spezialfunktionen für interne InMemory-Zwecke erstellt hat. In TBufDataset selbst könnte man dazu ClassName abfragen, aber das geht mit einer Ableitung dann auch nicht mehr, es sei denn, man implementiert das jedes mal. Oder man müsste mit RTTI oder so schauen, ob die Funktion Fetch überschrieben wurde...

Weitere Ableitungen in der LCL-Familie fallen mir jetzt ehrlich gesagt auch nicht ein, nur eben meine eigenen.

[charlytango]

Ich weiß jetzt nicht ob es zum Thread passt, aber in alten Zeiten wenn ich eine "lokale" (SQL-)Abfrage auf eine TDataset (also eben eine TSQLQuery haben wollte, habe ich TxQuery verwendet.
https://github.com/nakijun/txquery

Damit kann man tatsächlich mit einem SELECT gegen ein TSQLQuery gehen ohne nochmal fetchen zu müssen.

Mittlerweile scheint das auch OpenSource zu sein, einen aktuellen Lazarus Port hab ich noch nicht gefunden.