openjverein / jverein

Open JVerein - Open Source Vereinsverwaltung
https://openjverein.github.io
GNU General Public License v3.0
42 stars 15 forks source link

Fix Update0430 Teil 2 #223

Closed JohannMaierhofer closed 3 months ago

JohannMaierhofer commented 4 months ago

Diese Migration ist für den Fall, dass die korrigierte Migration Update0430 nicht ausgeführt wird weil schon mit der alten Version migriert wurde z.B. mit einem Nightly Build. Weil wegen gelöschter Buchungsklassen die Integrität verletzt sein könnte, muss der Foreign Key mit NOCHECK erzeugt werden.

Der Foreign Key wird nur erzeugt wenn er noch nicht existiert.

Sollte jemand schon bei einem Konto eine Buchungsart gesetzt haben und diese dann gelöscht haben, bleibt die ID bei der Migration in der Kontotabelle erhalten. Dies führt aber zu keinem Fehlverhalten der Software. Beim nächsten setzen eine Buchungsart wird die ID überschrieben.

Die Übergabe muss nach #218 erfolgen wegen der Update Nummer.

lenilsas commented 2 months ago

Ich verwende JVerein mit MariaDb, hier klappt das Update leider nicht:

java.sql.SQLException: exception while executing sql script: (conn=50) You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'IF NOT EXISTS  FOREIGN KEY fkKonto1(buchungsart) REFERENCES buchungsart (id) ON ' at line 1. Current statement: ALTER TABLE konto ADD CONSTRAINT IF NOT EXISTS  FOREIGN KEY fkKonto1(buchungsart) REFERENCES buchungsart (id) ON DELETE SET NULL ON UPDATE NO ACTION NOCHECK
    at de.willuhn.sql.ScriptExecutor.execute(ScriptExecutor.java:195)
    at de.jost_net.JVerein.server.DDLTool.AbstractDDLUpdate.execute(AbstractDDLUpdate.java:89)
    at de.jost_net.JVerein.server.DDLTool.AbstractDDLUpdate.execute(AbstractDDLUpdate.java:76)
    at de.jost_net.JVerein.server.DDLTool.Updates.Update0439.run(Update0439.java:48)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
    at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base/java.lang.reflect.Method.invoke(Method.java:566)
    at de.jost_net.JVerein.server.JVereinUpdateProvider.callMethod2(JVereinUpdateProvider.java:266)
    at de.jost_net.JVerein.server.JVereinUpdateProvider.<init>(JVereinUpdateProvider.java:93)
    at de.jost_net.JVerein.server.DBSupportMySqlImpl.checkConsistency(DBSupportMySqlImpl.java:89)
    at de.jost_net.JVerein.server.JVereinDBServiceImpl.checkConsistency(JVereinDBServiceImpl.java:114)
    at de.jost_net.JVerein.JVereinPlugin$1.call(JVereinPlugin.java:112)
    at de.jost_net.JVerein.JVereinPlugin.call(JVereinPlugin.java:209)
    at de.jost_net.JVerein.JVereinPlugin.init(JVereinPlugin.java:105)
    at de.willuhn.jameica.plugin.PluginLoader.initPlugin(PluginLoader.java:394)
    at de.willuhn.jameica.plugin.PluginLoader.init(PluginLoader.java:239)
    at de.willuhn.jameica.services.PluginService.init(PluginService.java:39)
    at de.willuhn.boot.BootLoader.resolve(BootLoader.java:139)
    at de.willuhn.boot.BootLoader.resolve(BootLoader.java:119)
    at de.willuhn.boot.BootLoader.getBootable(BootLoader.java:70)
    at de.willuhn.jameica.system.Application.init(Application.java:103)
    at de.willuhn.jameica.system.Application.newInstance(Application.java:87)
    at de.willuhn.jameica.Main.main(Main.java:78)
JohannMaierhofer commented 2 months ago

@willuhn , @dippeal ihr kennt euch doch besser mit SQL aus. Ich habe die Methode createForeignKeyIfNotExistsNocheck von createForeignKey kopiert und angepasst. Ich denke aber, dass auch in der createForeignKey ein Fehler ist. Der constraintname müsste doch auch bei MYSQL vor " FOREIGN KEY " stehen so wie bei H2. Wie ich oben sehen kann wird sonst der constraintname als Tabellenname verwendet. Wenn dem so ist würde auch der Update Update0430 bei MySQL nicht richtig funktionieren und möglicherweise frühere Updates die Foreign Keys erzeugen.

willuhn commented 2 months ago

Ich glaube, MySQL/MariaDB unterstützt das "IF NOT EXISTS" nicht (oder nicht in allen Versionen).

JohannMaierhofer commented 2 months ago

Ok, ist aber nicht trotzdem auch ein Fehler im Code von createForeignKey für MySQL?

dippeal commented 2 months ago

IF NOT EXISTS gibt es nicht im MYSQL und NOCHECK auch nicht. Du musst erst prüfen ob der key existiert und dann ggf. anlegen.

JohannMaierhofer commented 2 months ago

Ok, ich schaue mal wie das abprüfen geht. Ich bin mir aber auch sicher, dass der Code in createForeignKey für MySQL falsch ist. Der Key Name sollte zwischen " ADD CONSTRAINT " und " FOREIGN KEY stehen so wie das auch im Code für H2 ist. So finde ich es zumindest im Internet.

JohannMaierhofer commented 2 months ago

Da ich kein MySQL habe wäre meine Frage ob der Code unten funktionieren würde. Ich erzeuge einfach den Key im TRY. Wenn er schon existiert geht es in den CATCH und da mache ich nichts weil dann wurde er ja schon mit Update430 erzeugt und braucht nicht neu ezeugt werden. Das nocheck geht wohl nicht. Wenn also die Integrität schon verletzt wäre wird der Key nicht angelegt. Das sollte aber eigentlich nicht der Fall sein wenn man nicht für Test Zwecke schon herumgespielt hat.

  case MYSQL:
  {
    return "BEGIN TRY "
        + createForeignKey(constraintname, table, column, reftable, refcolumn, 
            ondelete, onupdate)
        + " END TRY\n"
        + " BEGIN CATCH "
        + " "
        + " END CATCH;\n";
  }
dippeal commented 2 months ago

"BEGIN TRY" gibt's nicht bei MySQL vielleicht https://dev.mysql.com/doc/refman/8.4/en/commit.html ?

Da ich kein MySQL habe wäre meine Frage ob der Code unten funktionieren würde. Ich erzeuge einfach den Key im TRY. Wenn er schon existiert geht es in den CATCH und da mache ich nichts weil dann wurde er ja schon mit Update430 erzeugt und braucht nicht neu ezeugt werden. Das nocheck geht wohl nicht. Wenn also die Integrität schon verletzt wäre wird der Key nicht angelegt. Das sollte aber eigentlich nicht der Fall sein wenn man nicht für Test Zwecke schon herumgespielt hat.

  case MYSQL:
  {
    return "BEGIN TRY "
        + createForeignKey(constraintname, table, column, reftable, refcolumn, 
            ondelete, onupdate)
        + " END TRY\n"
        + " BEGIN CATCH "
        + " "
        + " END CATCH;\n";
  }
JohannMaierhofer commented 2 months ago

Ich habe mir hier das Create Statement aus der MySQL Dokumentation geholt.

ADD [CONSTRAINT [symbol]] FOREIGN KEY [index_name] (col_name,...)

So wie ich es verstanden habe ist symbol der Contraint Name und index_name der Name des Indexes der auf die Spalte erzeugt wurde.

Der aktuelle Code setzt bei MySQL den übergeben Contraint Namen als index_name ein. Das wäre aber falsch weil der Index einen anderen Namen hat. Für das Löschen des Foreign Key muss man den symbol Namen nehmen.

DROP {CHECK | CONSTRAINT} symbol

Da der bei MySQL nicht angegeben ist wird er wohl automatisch generiert. Dann funktioniert mein Upgrade440 nicht weil zum Löschen der übergebene Contraint Name benutzt wird.

Darum glaube ich, dass die Methode createForeignKey nicht stimmt.

Jetzt habe ich noch etwas gefunden. Die spec sagt: For ALTER TABLE, unlike CREATE TABLE, ADD FOREIGN KEY ignores index_name if given and uses an automatically generated foreign key name. As a workaround, include the CONSTRAINT clause to specify the foreign key name:

Also wird wohl beim aktuellen Code der Contraint Name sowieso ignoriert und einer automatisch generiert. Es wäre aber wohl besser den Request zu ändern und den Namen als Symbol zu übergeben wie bei H2.

JohannMaierhofer commented 2 months ago

Ich weiß jetzt auch nicht ob die Upgrades aus #224 und #225 bei MySQL funktionieren. Ich habe die Contraint Namen benutzt die ich in meiner H2 DB gefunden habe. Sind die Namen dann in MySQL die gleichen ?