Probleme bei verschachtelten Querys

[Beach]

Hallo,
ich arbeite mit Zeos8 und SQLite3
Ich möchte in meinem Programm 2 Checkboxen anhand von Werten aus einer Datenbank setzen. Wenn diese Checkboxen geändert werden, soll auch der entsprechende Wert in der DB geändert werden.

Die DB Verbindung besteht. Dann SELECTe ich die Werte der Tabelle und durchlaufe die Ergebnisse und setze die Eigenschaften der CheckBox entsprechend. Nun wird aber, wenn ich die Eigenschaft "checked" der Box ändere, auch das Event "OnChange" der Checkbox gefeuert, welches dann ein "UPDATE" auf die Tabelle macht. Wenn ich dann das nächste Ergebnis mit "Query.next" durchlaufen will bekomme ich die Fehlermeldung "Operation cannot be performed on an inactive datase."

Ich habe dann bei der Checkbox die Zuweisung der Prozedur von "OnChange" zu "OnClick" geändert -> Gleicher Fehler
Erst nach dem Einbau einer "Sperrvariable" die ich beim OnCreate der Form auf TRUE setze und erst auf FALSE nachdem die CB auf die Werte der DB angepasst wurden, konnte ich die Verschachtelung auflösen.

Mache ich hier evtl einen grundlegenden Fehler in der Vorgehensweise? Oder ist die Variante mit der "Sperrvariable" ein akzeptable Lösung?

Hoffe das jemand das nachvollziehen kann. Komme mir gerade vor als wenn ich einen Knoten im Hirn habe.

Hier die betreffenden Prozeduren.

Code: Alles auswählen

procedure TForm1.getSetup();
var
  LComponent: TComponent;

begin
  Query.SQL.Text := 'SELECT name,value_integer FROM setup';
  Query.Open;

  while not Query.EOF do
  begin
    LComponent := FindComponent(Query.FieldByName('name').AsString);
    if assigned(LComponent) then
    begin

      if Query.FieldByName('value_integer').AsInteger <> 0 then
        TCheckBox(LComponent).Checked := True;
    end;
    Query.Next;
  end;

  Query.Close;
  Query.SQL.Clear;
  Config.FirstRun := FALSE;
end;

Code: Alles auswählen

procedure TForm1.LimitDishChange(Sender: TObject);
var
  sql: string;
  i: integer;
begin
  if Config.FirstRun then exit;

  i := 0;
  if LimitDish.Checked then i := 1;

  sql := 'UPDATE setup SET value_integer = ' + IntToStr(i) +
    ' WHERE name = ''LimitDish''';
  try
    Query.SQL.Text := sql;
    Query.ExecSQL;
  except
    on E: Exception do
      ShowMessage('1: Fehler beim ändern des SetupWert: ' + E.Message);
  end;
end;

Lazarus 3.6 (rev lazarus_3_6) FPC 3.2.2 x86_64-win64-win32/win64
Win10Pro 22H2

[MmVisual]

Man kann das "OnChange" Event während dem setzen der Checkbox auf NIL setzen und nach dem setzen wieder auf die @Funktionsname. Damit sollte es gehen.

[Bullykiffer]

Moin.

Ich bin jetzt leider nicht der Coder Pro,aber in deinem Source sind mir ein paar Sachen aufgefallen,
davon abgesehen,das ich einiges vielleicht etwas anders gelöst hätte.
Da ich nicht mit ZEOS arbeite,kann es vielleicht sein,das einige meiner Anmerkungen vielleicht nicht ganz zutreffend sein könnten.

Punkt 1: In deinem ersten Codeschnipsel sehe ich zwar,das du Config.FirstRun auf false setzt,aber nirgends das du sie mal auf True gesetzt hättest.
Desweiteren arbeitest du mit Query.next aber es ist sicherlich nicht schlecht,wenn man anfangs auch die Query.First aufrufen würde?
Persönlich setze ich aus Prinzip immer vor einer Zuweisung von Query.sql.text ein Query.close,finde ich persönlich sauberer,aber vielleicht ist es auch nicht (mehr) nötig,

Punkt 2: Am ende der query.execsql sehe ich nirgends das du die Daten wirklich in die Datenbank übernimmst? Stichpunkt: sqltransaction1.Commit;
Mir ist aber jetzt nicht bekannt,wie ZEOS das handhabt...

Cya de Helge

Kleiner Nachtrag: Das du den Fehler mit Inaktiver Datenbank erhälst liegt vermutlich auch daran,das du für beide Aufrufe die gleiche Query benutzt,daher ist die Sperrvariable auch nötig. Duch den Change aufruf überschreibst du die Query deines ersten Aufrufes. Da im 2. Aufruf die Query geschlossen ist,ist die (gleiche) Query auch in deinem ersten Aufruf
geschlossen worden (und der Inhalt gelöscht).

[Zvoni]

mWn nicht anderst lösbar als mit ner Sperrvariablen

[charlytango]

Hoffe das jemand das nachvollziehen kann. Komme mir gerade vor als wenn ich einen Knoten im Hirn habe.

Ja, leider geht es mir hier genauso beim nach-denken deines Problems.
Ein kleines Beispielprojekt würde da entscheidend helfen.

Denn bei DB-Anwendungen kann das Problem an sehr vielen Stellen entstehen. Vom DB Design über die DB-Anbindung, dem Transaction-Handling, der Query, dem Code etc.
Hier isoliert eine Lösung anzubieten die auf Vermutungen angewiesen ist wird dir schwer weiter helfen.

Was mir aufgefallen ist:

• Du benutzt ZEOS, also ein Framework das recht ausgefeilte Komponenten zur Anbindung datensensitiver Controls bereitstellt.
  Warum nutzt du keine datensensitiven Controls, sondern versuchst das was ZEOS längst kann, nochmal "per Hand" zu codieren?
  
  Datenbankanwendungen sind eine Stärke von Lazarus, also musst du einen Grund haben warum du die entsprechenden Controls und Komponenten nicht nutzt?
  Mit ein paar Komponenten und datensensitiven Checkboxen lässt sich deine Anforderung zusammen klicken. ich denke, dazu ist gar kein eigener Code nötig

• Es ist lobenswert dass du kein SELECT * benutzt und die Daten per Feldnamen holst. Mir fehlt in dem SELECT nur ein Primary Key (außer NAME wäre dieser, was ich bezweifle)
  Wenn ZEOS/SQLDB einen Primary Key entdeckt, klappen viele Funktionen einfach auf Anhieb. (wenn man DataControls benutzt)

• Du benutzt keine Parameter beim Zusammenbau der SQL Statements. Das ist für dich selbst eine Fehlerquelle und ein Sicherheitsthema

• Leider gibt es auch keine Infos über das Transaction management.

• ZEOS hat eine eigene Komponente für das Monitoring des SQL-Verkehrs.
  Du kannst dir alles was von und zur Datenbank geht samt den Statements ausgeben lassen. Ein für mich unverzichtbare Funktion bei DB-Verbindungen.

Noch eine Anmerkung - Als Funktion zum Einstellen von Setup-Werten wird es ja nicht bei der einen Checkbox bleiben. Dann frage ich mich, ob es überhaupt Sinn macht jede Änderung per OnChange sofort in die DB zu schreiben, Damit ist ein Abbruch ja nicht mehr möglich.

Pack zwei Buttons (Speichern, Abbrechen) auf die Form und du hast das Problem mit der Sperrvariablen nicht mehr.

[Zvoni]

• Es ist lobenswert dass du kein SELECT * benutzt und die Daten per Feldnamen holst. Mir fehlt in dem SELECT nur ein Primary Key (außer NAME wäre dieser, was ich bezweifle)
  Wenn ZEOS/SQLDB einen Primary Key entdeckt, klappen viele Funktionen einfach auf Anhieb. (wenn man DataControls benutzt)

"Name" (was übrigens äusserst unglücklich gewählt ist als Feldname) MUSS ein Primary Key sein (bzw. zumindest UNIQUE), da er FindComponent seiner Form1 verwendet.

• Du benutzt keine Parameter beim Zusammenbau der SQL Statements. Das ist für dich selbst eine Fehlerquelle und ein Sicherheitsthema

Yepp. Parameter ist die Wahl der Waffen
Was ich nicht verstehe, warum er value_integer auf <>0 prüft.
Einfach
CheckBox.Checked:=Query.FieldByName('value_integer').ToBoolean;
und gut ist (Vorausgesetzt er hat SysUtils in Uses)

[charlytango]

"Name" (was übrigens äusserst unglücklich gewählt ist als Feldname) MUSS ein Primary Key sein (bzw. zumindest UNIQUE), da er FindComponent seiner Form1 verwendet.

Der Feldname "NAME" ist für mich schon ok. Da werden andere Werte folgen.
Aber ein sauberer Primary Key fehlt -- meinetwegen eine IDSETUP auf Integer und autoincrement.

[Zvoni]

"Name" (was übrigens äusserst unglücklich gewählt ist als Feldname) MUSS ein Primary Key sein (bzw. zumindest UNIQUE), da er FindComponent seiner Form1 verwendet.

Der Feldname "NAME" ist für mich schon ok. Da werden andere Werte folgen.
Aber ein sauberer Primary Key fehlt -- meinetwegen eine IDSETUP auf Integer und autoincrement.

Hab ich ne andere Meinung dazu.
Es gibt sowas wie "sauberer Primärschlüssel" nicht. Punkt!
Ein Primärschlüssel hat die Eigenschaft UNIQUE und NOT NULL auf die Entität/Tabelle bezogen.
Das kann ein anonymer Integer-Wert sein (was du meinst! ein sogenannter "surrogate Primary Key"), kann aber auch ein String, ein DateTime oder was auch immer sein, solange er UNIQUE und NOT NULL für die Tabelle ist.

Denk mal an GUID's, welche immer mehr als Primary Key verwendet werden.

Der einzige Vorteil, wenn man einer Spalte das Attribut "Primärschlüssel" gibt, ist das automatisch das Attribute UNIQUE und NOT NULL gesetzt wird, als auch, dass im Hintergrund ein Index dafür angelegt wird, welcher in den meisten Fällen sogar "unsichtbar" für den user ist

Der einzige Vorteil mMn bei surrogate Primary Keys (sogenannte "Auto-ID" --> AutoIncrement), ist dass man bei INSERTS keinen Feldwert mit übergeben muss, da der Wert vom DBMS dann selbst errechnet wird.
Bei "natürlichen" Primary Keys muss man diesen mitgeben, sowie auch eine Logik parat haben, sofern es zu einer Kollision kommt ("Primary Key existiert bereits")

[Beach]

[...]

• Du benutzt ZEOS, also ein Framework das recht ausgefeilte Komponenten zur Anbindung datensensitiver Controls bereitstellt.
  Warum nutzt du keine datensensitiven Controls, sondern versuchst das was ZEOS längst kann, nochmal "per Hand" zu codieren?
  
  Datenbankanwendungen sind eine Stärke von Lazarus, also musst du einen Grund haben warum du die entsprechenden Controls und Komponenten nicht nutzt?
  Mit ein paar Komponenten und datensensitiven Checkboxen lässt sich deine Anforderung zusammen klicken. ich denke, dazu ist gar kein eigener Code nötig

• Es ist lobenswert dass du kein SELECT * benutzt und die Daten per Feldnamen holst. Mir fehlt in dem SELECT nur ein Primary Key (außer NAME wäre dieser, was ich bezweifle)
  Wenn ZEOS/SQLDB einen Primary Key entdeckt, klappen viele Funktionen einfach auf Anhieb. (wenn man DataControls benutzt)

• Du benutzt keine Parameter beim Zusammenbau der SQL Statements. Das ist für dich selbst eine Fehlerquelle und ein Sicherheitsthema

• Leider gibt es auch keine Infos über das Transaction management.

• ZEOS hat eine eigene Komponente für das Monitoring des SQL-Verkehrs.
  Du kannst dir alles was von und zur Datenbank geht samt den Statements ausgeben lassen. Ein für mich unverzichtbare Funktion bei DB-Verbindungen.

Noch eine Anmerkung - Als Funktion zum Einstellen von Setup-Werten wird es ja nicht bei der einen Checkbox bleiben. Dann frage ich mich, ob es überhaupt Sinn macht jede Änderung per OnChange sofort in die DB zu schreiben, Damit ist ein Abbruch ja nicht mehr möglich.

Pack zwei Buttons (Speichern, Abbrechen) auf die Form und du hast das Problem mit der Sperrvariablen nicht mehr.

Ich merke das ich wohl langsam zu alt werde für sowas und vor allem zu selten sowas mache. Ich habe viele einfache Sachen vergessen und muß viel Syntax erstmal wieder googlen.

- Datensensitive Controls..... Jetzt wo du es erwähnst... Da war was. Aber habe ich mich noch allgemein wenig damit beschäftigt.
- Es gibt bei mir in jeder Tabelle eine Spalte ID die als Primary Key fungiert. Ich weis das sowas heute nicht mehr unbedingt praxis ist. Aber es hat für mich bis heute immer den Zweck erfüllt. Abfragen tue ich den nicht da ich den in der Anwendung hier nicht brauche.
- Ich nutze Parameter. Aber in diesem Fall hier sah ich keine Notwendigkeit.
- Der Zeos SQL monitor. Danke. Das Teil hatte ich komplett nicht mehr auf dem Schirm.
- Die Funktionen für das Setup sollten noch erweitert werden. Aber ich denke dafür taugt die Basis aktuell nicht. Deshalb nur die beiden CB für eine Funktion die ich noch chnell einbauen wollte.

Trotzdem haben alle Antworten hier den Knoten in meinem Kopf etwas gelöst.

Der ganze Aufbau des Programm ist, aus meiner aktuellen Sicht, einfach nur gruselig und gehört komplett anders aufgebaut. Auch damit es etwas variabler wird.
Anfangs wurde nur ein paar Sachen zusammengeklickt und das Ergebnis ausgedruckt, dann wollte ich noch eine Statistik haben. -> Anbinden an eine SQLite DB. Dann habe ich gemerkt das die Konfiguration direkt im Quellcode unhandlich und unübersichtlich ist wenn die Sachen sich die man zusammenklickt sich ändern.
Aber wie es immer so ist, ich brauche das Programm 4x im Jahr. Und es ist jetzt natürlich wieder kurz vor Ultimo.....

[af0815]

Ich finde man muss sich entscheiden - Datensensitive Komponenten - oder alles zu Fuss. Mischt man das, gibt es meistens Probleme und die Wartbarkeit sinkt. Eine schnelle Änderung und es kracht irgendwo.

Wenn man datensensitive Komponenten = RAD Design verwendet, dann auch beachten welche Locks auf den Tabellen vom System gelegt werden können und wie lange Queries offen sein müssen. Auch sollte man damit rechnen, das einmal eine Buchung daneben geht, weil der Kollege halt bei der Buchung schneller war. Autoinc, Löschweitergaben etc. was die DBs so halt an Gimmicks haben, sollte msn hinterfragen ob sie eine Verbesserung oder Verschlimmerung sind.

Das mit den GUID als PK hat was für sich, besonders wenn man Daten über Server repliziert. Amsonsten hat man den Vorteil, das man den PK nicht für normale Daten missbraucht

[Zvoni]

Zurück zu OP's Problem:
So wie ich das verstehe, hat er Konfigurationsdaten in einer Datenbank, und auf seiner Form diverse Checkboxes ("FindComponent" und danach der Cast per TCheckBox):
Heisst: Er sucht explizit nach dem Namen seiner Checkbox, und setzt dann True/False, und das ganze in einer Schleife.
Wobei er in das Problem rennt, dass eine "Änderung" der Checked-Eigenschaft aus dem Code heraus das Change-Event auslöst.

Hätte mir wahrscheinlich ein Array mit Referenzen zu den jeweiligen Checkboxen gemacht, und dann einfach durch das Array laufen, und man nutzt den Array-Eintrag (was der Name der Checkbox ist), und nutzt das als Filter auf das TDataset des Querys.

Absoluter Aircode

Code: Alles auswählen

Var
   MeineCheckboxen:Array[0..12] Of TCheckBox; //Oder wieviel Checkboxen das auch sind
   T:TCheckBox;
.....
//In FormCreate
i:=0;
For T In Form1.Components Do
Begin
  T.Tag:=1;  //Oder halt schon bei Design setzen
  MeineCheckBoxen[i]:=T;
  Inc(i);
End;
.....
//Führe SELECT name,value_integer FROM setup aus
For i:=0 To 12 Do 
Begin
  Query.Dataset.Filtered:=False;
  Query.Dataset.Filter:='name='+QuoteStr(MeineCheckboxen[i].Name);
  Query.Dataset.Filtered:=True;
  MeineCheckBoxen[i].Checked:=Query.FieldByName('value_integer').ToBoolean;
  MeineCheckBoxen[i].Tag:=0;
End;
.....
procedure TForm1.MyGenericOnChange(Sender: TObject);
var
  sql: string;  
begin
  if Sender.Tag.ToBoolean then exit;
  
  sql := 'UPDATE setup SET value_integer = ' + Sender.Checked.AsInteger+' WHERE name = '+QuoteStr(Sender.name);
  try
    Query.SQL.Text := sql;
    Query.ExecSQL;
  except
    on E: Exception do
      ShowMessage('1: Fehler beim ändern des SetupWert: ' + E.Message);
  end;
end;

Da weiterer Code in OnChange der jeweiligen CheckBox steht (WIESO jeweils eigenes OnChange-Event??? Benutze eine einzige Prozedur, und nutze das "Sender"-Argument), kann das NUR per Sperrvariable abgefangen werden.

Hoffe ich war mit meinem Ansatz verständlich

EDIT: Hmm....anstatt ner separaten Sperrvariablen hätte ich die Tag-Eigenschaft der Checkboxen im OnChange abgefragt.
Tag ist initial eins ("1"), also schon beim Design.
In obiger For i:=0 To 12-Schleife als letzte Zeile vor dem end; noch ein MeineCheckboxen[ i].Tag:=0;
.... und im OnChange als erste Zeile
If Sender.Tag.ToBoolean Then Exit;

[charlytango]

Jetzt geben ihm 5 Leute Stoff -- der Arme

Der ganze Aufbau des Programm ist, aus meiner aktuellen Sicht, einfach nur gruselig und gehört komplett anders aufgebaut.

Dann ist auch noch zu überlegen ob man "nur" ein paar Tausend Datensätze irgendwo in eine DB schreibt, ob das ein Mehrbenutzersystem wird oder gar eines mit Server-Replikation. Danach würde ich auch die Art der Primary Keys wählen.

PKs unique per PC zu generieren hatte ich auch schon mal in einer Applikation weil wir uns damals nicht auf eine saubwere Vergabe durch die DB verlassen haben - Hat Zusatzaufwand auch beim Installieren gebracht.

Bei "natürlichen" Primary Keys muss man diesen mitgeben, sowie auch eine Logik parat haben, sofern es zu einer Kollision kommt ("Primary Key existiert bereits")

Ja eh
Was wieder den Aufwand steigert -- Klar kann man das alles lösen, aber dieser Check muss dann in jedes INSERT Statenment rein.

Das mit den GUID als PK hat was für sich, besonders wenn man Daten über Server repliziert. Amsonsten hat man den Vorteil, das man den PK nicht für normale Daten missbraucht

Stimmt, dafür ist ein Integer leichter lesbar wenn man irgendwas in der DB frickeln muss.

Eine RICHTIGE Anwort wird es darauf nicht geben -- It Depends

Wenn man es nur 4x im Jahr braucht, würde ich besondere Sorgfalt darauf legen, dass alles einfach, wartbar und stabil ist und bleibt. Denn Fehler fallen halt auch nur 4x im Jahr auf.


@Beach Fall du ein generisches System zur Ablage von Einstellungs-Daten brauchst, kann ich dir etwas überlassen

[Zvoni]

Übrigens:
Sofern keine anderen Daten in dieser SQLite-DB sind, dann ist das eh overkill.
Stumpfe Textdatei im Format
LimitDish = 1 //Soll wohl der Name einer CheckBox sein
CheckBox1 = 1
CheckBox2 = 0
CheckBox3 = 0
usw.....
Dann einfach ne Stringlist mit LoadFromFile, dann kann man sich das Query-Gedöns sparen
Aircode

Code: Alles auswählen

For i:=0 To 12 Do 
Begin
  MeineCheckBoxen[i].Checked:=MeineStringList.Values[MeineCheckBoxen[i].Name].ToBoolean;
  MeineCheckBoxen[i].Tag:=0;
End;

[Beach]

Zuerst wollte ich alles in INI Dateien packen.
Aber da ich dann eh die DB eingebaut habe wegen der Statistik (Selbst dafür ist die SQLite noch 'fast' Overkill) wollte ich die natürlich auch dafür nutzen.

Allerdings habe ich mir jetzt noch irgendetwas bei ner DB Abfrage verfrickelt und finde es einfach nicht. Durch das Upgrade von Zeos7 auf Zeos8 kommen eh noch ein paar Widrigkeiten dazu....
Bin im Moment dringend am Überlegen ob ich nicht doch komplett von vorne Anfange.

@charlytango : Würde gerne auf das Angebot zurück kommen und mir zumindest mal zeigen lassen was du da hast. Ich kann nur lernen daraus

[charlytango]

@charlytango : Würde gerne auf das Angebot zurück kommen und mir zumindest mal zeigen lassen was du da hast. Ich kann nur lernen daraus

Mach ich gerne, aber das wird schon noch bis Mo,Di dauern. Bin im Moment nur mobil unterwegs.
Ist auch noch längst nicht fertig und stabil, man sieht aber wohin es gehen soll.

Und wenn wir schon von Overkill reden -- DAS ist dann sicher einer, denn dabei wird die GUI generisch aus der DB generiert. Aber ich lasse es dir zukommen.