Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Update0430 Teil 2 #223

Merged
merged 2 commits into from
May 29, 2024

Conversation

JohannMaierhofer
Copy link

@JohannMaierhofer JohannMaierhofer commented May 3, 2024

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.

@JohannMaierhofer JohannMaierhofer changed the title Fix update0430teil2 Fix Update0430 Teil 2 May 3, 2024
@dippeal dippeal added the fix This will fix a bug label May 4, 2024
@JohannMaierhofer JohannMaierhofer added the blocked Depends on another feature or request label May 9, 2024
@JohannMaierhofer JohannMaierhofer removed the blocked Depends on another feature or request label May 20, 2024
@willuhn willuhn merged commit d06885f into openjverein:master May 29, 2024
@JohannMaierhofer JohannMaierhofer deleted the fix_update0430teil2 branch May 30, 2024 07:27
@lenilsas
Copy link

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
Copy link
Author

@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
Copy link
Member

willuhn commented Jun 10, 2024

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

@JohannMaierhofer
Copy link
Author

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

@dippeal
Copy link
Member

dippeal commented Jun 10, 2024

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
Copy link
Author

JohannMaierhofer commented Jun 10, 2024

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
Copy link
Author

JohannMaierhofer commented Jun 10, 2024

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
Copy link
Member

dippeal commented Jun 10, 2024

"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
Copy link
Author

JohannMaierhofer commented Jun 10, 2024

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
Copy link
Author

JohannMaierhofer commented Jun 10, 2024

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 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix This will fix a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants