Einträge in taxkeys sind NULL

[Niclas]

In der Tabelle taxkeys gibt es Einträge, bei denen tax_id und taxkey_id NULL sind. Ist das so gewünscht oder wäre es vielleicht besser diese Einträge taxkey_id=0 und entsprechender tax_id zu setzen?

Ich könnte sonst gleich entsprechendes im Upgrade für Konten ohne Eintrag in der taxkeys einbauen (das ist ja was ähnliches). Und dann eben noch ein NOT NULL constraint für beide Spalten, damit der Fehler (falls es einer ist) in Zukunft nicht mehr auftritt.

Ich habe weder in meiner (echt schon alten) Entwicklungsinstallation noch in einer frisch aufgesetzten Einträge, bei denen taxkeys.taxkey_id NULL ist. Es gibt in meiner Entwickler-DB Einträge mit taxkeys.tax_id NULL, was daran liegt, dass diese Einträge (warum auch immer) in Tabelle tax nicht existieren.

Generell sollte für das Schema von taxkeys zwecks Konsistenz allerdings gelten:

• chart_id, tax_id, taxkey_id und startdate müssten NOT NULL sein,
• chart_id muss ein Fremdschlüssel auf chart.id sein,
• tax_id muss ein Fremdschlüssel auf tax.id sein (das gilt bereits, also gut so),
• taxkey_id muss ein Fremdschlüssel auf tax.taxkey sein,
• die Kombination aus chart_id, tax_id, taxkey_id muss eindeutig sein.

Für Tabelle tax sollte gelten:

• chart_id: Fremdschlüssel auf chart.id
• rate: NOT NULL DEFAULT 0 (momentan gibt es Einträge mit NULL, hier durch einfaches Update auf 0 setzen)
• taxkey: NOT NULL
• taxdescription: NOT NULL wäre auch hier der Vollständigkeit halber gut

Zum Update von taxkeys. Hierfür muss unbedingt ein Perl-Script geschrieben werden, das nach Möglichkeit ganz viele der Misstände ausräumt, bevor die verschiedenen Constraints angewandt werden. Warum ein Perl- und kein SQL-Script? Weil das Constraint "die Kombination aus chart_id, tax_id und taxkey_id muss eindeutig sein" erfordert, dass man Duplikate von diesen Zeilen entfernt. Und das geht nicht so einfach mit SQL.

Auf jeden Fall will ich die Update-Scripte begutachten, bevor sie ins offizielle Repository gepusht werden. Entweder, ihr stellt dafür eigene Repos bereit (was mir das Liebste wäre -- gerne auch auf Hostern wie github.com), oder wir machen das über eigene Branches im offiziellen Repo (was aber leicht zu Situtationen wie 'ich pushe in den falschen Branch' führen kann).

Auf jeden Fall ein dickes Danke! für deine Bemühungen hierzu.

Replying to m.bunkus@…:

Ich habe weder in meiner (echt schon alten) Entwicklungsinstallation noch in einer frisch aufgesetzten Einträge, bei denen taxkeys.taxkey_id NULL ist. Es gibt in meiner Entwickler-DB Einträge mit taxkeys.tax_id NULL, was daran liegt, dass diese Einträge (warum auch immer) in Tabelle tax nicht existieren.

Generell sollte für das Schema von taxkeys zwecks Konsistenz allerdings gelten:

• chart_id, tax_id, taxkey_id und startdate müssten NOT NULL sein,

Hierfür habe ich schon was geschrieben, allerdings bin ich mir noch nicht sicher, ob ich in jedem Fall korrekt mit NULL-Einträgen umgehe. Sicher kann man einfach eine Fehlermeldung schmeißen und das Update abbrechen, aber das ist nicht in jedem Fall nötig...

• chart_id muss ein Fremdschlüssel auf chart.id sein,

Hier können Einträge einfach gelöscht werden, da das Konto ohnehin nicht mehr vorhanden ist.

• tax_id muss ein Fremdschlüssel auf tax.id sein (das gilt bereits, also gut so),
• taxkey_id muss ein Fremdschlüssel auf tax.taxkey sein,

Das funktioniert nicht! Denn tax.taxkey ist kein Primärschlüssel! Heißt, man kann keinen Fremdschlüssel auf diese Einträge setzen. Wie ich schon in einer Email geschrieben habe, könnte man hier einen Trigger oder eine Funktion bauen, die überprüft, ob tax.taxkey=taxkeys.taxkey_id ist, wenn tax.id=taxkeys.tax_id. Wie man das genau macht, weiß ich allerdings (noch) nicht.

• die Kombination aus chart_id, tax_id, taxkey_id muss eindeutig sein.

Für Tabelle tax sollte gelten:

• chart_id: Fremdschlüssel auf chart.id
• rate: NOT NULL DEFAULT 0 (momentan gibt es Einträge mit NULL, hier durch einfaches Update auf 0 setzen)
• taxkey: NOT NULL
• taxdescription: NOT NULL wäre auch hier der Vollständigkeit halber gut

Zum Update von taxkeys. Hierfür muss unbedingt ein Perl-Script geschrieben werden, das nach Möglichkeit ganz viele der Misstände ausräumt, bevor die verschiedenen Constraints angewandt werden. Warum ein Perl- und kein SQL-Script? Weil das Constraint "die Kombination aus chart_id, tax_id und taxkey_id muss eindeutig sein" erfordert, dass man Duplikate von diesen Zeilen entfernt. Und das geht nicht so einfach mit SQL.

Sicher dass Duplikate bei solchen Einträgen einfach entfernt werden können? Was wenn sie verschiedene UstVA enthalten? Welches Startdatum wählt man aus?

Auf jeden Fall will ich die Update-Scripte begutachten, bevor sie ins offizielle Repository gepusht werden. Entweder, ihr stellt dafür eigene Repos bereit (was mir das Liebste wäre -- gerne auch auf Hostern wie github.com), oder wir machen das über eigene Branches im offiziellen Repo (was aber leicht zu Situtationen wie 'ich pushe in den falschen Branch' führen kann).

Auf jeden Fall ein dickes Danke! für deine Bemühungen hierzu.

Sollte nicht auf jeden Fall auch das Paar taxkeys.startdate und taxkeys.chart_id eindeutig sein?

Replying to Niclas:

• chart_id, tax_id, taxkey_id und startdate müssten NOT NULL sein,

Hierfür habe ich schon was geschrieben, allerdings bin ich mir noch nicht sicher, ob ich in jedem Fall korrekt mit NULL-Einträgen umgehe. Sicher kann man einfach eine Fehlermeldung schmeißen und das Update abbrechen, aber das ist nicht in jedem Fall nötig...

Wenn du mir eine Aufstellung geben kannst von typischen Fällen, dann können wir gerne über eine automatische Bereinigung sprechen. Das ist auch das, was ich meinte, als ich vom Perl-Script gesprochen habe: möglichst viele Probleme automatisiert lösen, dann erst die Constraints definieren.

• chart_id muss ein Fremdschlüssel auf chart.id sein,

Hier können Einträge einfach gelöscht werden, da das Konto ohnehin nicht mehr vorhanden ist.

• tax_id muss ein Fremdschlüssel auf tax.id sein (das gilt bereits, also gut so),
• taxkey_id muss ein Fremdschlüssel auf tax.taxkey sein,

Das funktioniert nicht! Denn tax.taxkey ist kein Primärschlüssel! Heißt, man kann keinen Fremdschlüssel auf diese Einträge setzen. Wie ich schon in einer Email geschrieben habe, könnte man hier einen Trigger oder eine Funktion bauen, die überprüft, ob tax.taxkey=taxkeys.taxkey_id ist, wenn tax.id=taxkeys.tax_id. Wie man das genau macht, weiß ich allerdings (noch) nicht.

Ja, schon richtig. Ich habe mich leicht geirrt und war nicht ganz exakt. Diese Aussage von mir ist falsch:

• die Kombination aus chart_id, tax_id, taxkey_id muss eindeutig sein.

Was statt dessen für taxkeys stimmt:

• die Kombination aus chart_id, taxkey_id und startdate muss eindeutig sein.

Daraus ergibt sich auch gleich eine Antwort zu einer deiner weiteren Fragen:

Sicher dass Duplikate bei solchen Einträgen einfach entfernt werden können? Was wenn sie verschiedene UstVA enthalten? Welches Startdatum wählt man aus?

Duplikate vom Tripel chart_id, taxkey_id und startdate müssen zusammengeschrumpft werden. Hier einfach die erste existierende Zeile überlassen. Ob die sich in UStVA unterscheidet ist egal.

Hintergrund. Die Logik ist:

• Für jedes Konto (chart_id)...
• ...kann es zu einem Zeitpunkt (startdate)...
• ...für einen bestimmten Steuerschlüssel (taxkey_id)...
• ...nur eine dazugehörige Steuer (tax_id) geben.

Typische Abfragen wissen also die drei Informationen Konto, Zeitpunkt, Steuerschlüssel und müssen dazu die dazugehörige tax_id ermitteln, und das darf nur eine einzige sein. Daher: das Tripel aus chart_id, taxkey_id und startdate muss eindeutig sein.

So, und jetzt bin ich in der Tat unsicher, was mit tax.taxkey überhaupt ist. Wird die streng betrachtet überhaupt benötigt? Die Abfragen laufen ja normalerweise wie folgt:

• In der Oberfläche ist vom Artikel aus über die Buchungsgruppe das Konto bekannt. Zusätzlich ist von der Oberfläche aus das Buchungsdatum bekannt.
• Zum Konto wird in chart der Steuerschlüssel abgefragt.
• Jetzt hat man das übliche Tripel chart_id, startdate (bzw. ein höheres Datum durch das Buchungsdatum) und taxkey_id aus chart, fragt in taxkeys die dazugehörige Steuer taxkeys.tax_id ab.
• Die Steuer bekommt man nun aus tax via taxkeys.tax_id == tax.id. Da ist tax.taxkey also gar nicht involviert.

Aber vielleicht übersehe ich auch momentan etwas...

...ja ich mag den ganzen strukturellen Unterbau auch nicht wirklich.

Sollte nicht auf jeden Fall auch das Paar taxkeys.startdate und taxkeys.chart_id eindeutig sein?

Spontan: nein, in taxkeys muss nur das oben erwähnte Tripel eindeutig sein.

Replying to m.bunkus@…:

Replying to Niclas:

• chart_id, tax_id, taxkey_id und startdate müssten NOT NULL sein,

Hierfür habe ich schon was geschrieben, allerdings bin ich mir noch nicht sicher, ob ich in jedem Fall korrekt mit NULL-Einträgen umgehe. Sicher kann man einfach eine Fehlermeldung schmeißen und das Update abbrechen, aber das ist nicht in jedem Fall nötig...

Wenn du mir eine Aufstellung geben kannst von typischen Fällen, dann können wir gerne über eine automatische Bereinigung sprechen. Das ist auch das, was ich meinte, als ich vom Perl-Script gesprochen habe: möglichst viele Probleme automatisiert lösen, dann erst die Constraints definieren.

chart_id NULL oder ungültig (weist auf ein nicht vorhandenes Konto): Zeile löschen.

startdate NULL: Zeile löschen (im Moment werden solche Einträge von kivitendo nämlich ignoriert)

tax_id NULL, taxkey_id NOT NULL: Fehlermeldung und Update wird abgebrochen.
Variante: erstelle neuen Eintrag in tax mit a) 0%-Steuern und b) angegebener taxkeys.taxkey_id (falls dieser noch nicht existiert). Schreibe die id von diesem Eintrag in taxkey.tax_id. kivitendo verhält sich im Augenblick zumindest so, dass es keine Steuern berechnet, wenn tax_id NULL ist, daher rührt dieser Vorschlag.

tax_id NOT NULL, taxkey_id NULL: Nehme Steuerschlüssel von tax.taxkey (wenn der auch NULL ist, muss abgebrochen werden)
Variante: definiere einen neuen Steuerschlüssel, der noch nicht vorhanden ist.

tax_id NULL und taxkey_id NULL: Setze Steuerschlüssel 0 und entsprechende tax_id.

Duplikate vom Tripel chart_id, taxkey_id und startdate müssen zusammengeschrumpft werden. Hier einfach die erste existierende Zeile überlassen. Ob die sich in UStVA unterscheidet ist egal.

Hintergrund. Die Logik ist:

• Für jedes Konto (chart_id)...
• ...kann es zu einem Zeitpunkt (startdate)...
• ...für einen bestimmten Steuerschlüssel (taxkey_id)...
• ...nur eine dazugehörige Steuer (tax_id) geben.

Typische Abfragen wissen also die drei Informationen Konto, Zeitpunkt, Steuerschlüssel und müssen dazu die dazugehörige tax_id ermitteln, und das darf nur eine einzige sein. Daher: das Tripel aus chart_id, taxkey_id und startdate muss eindeutig sein.

So, und jetzt bin ich in der Tat unsicher, was mit tax.taxkey überhaupt ist. Wird die streng betrachtet überhaupt benötigt? Die Abfragen laufen ja normalerweise wie folgt:

• In der Oberfläche ist vom Artikel aus über die Buchungsgruppe das Konto bekannt. Zusätzlich ist von der Oberfläche aus das Buchungsdatum bekannt.
• Zum Konto wird in chart der Steuerschlüssel abgefragt.

Wieso eigentlich? In chart.taxkey_id kann nur ein Steuerschlüssel stehen. Daher werden Einträge in taxkeys, die nicht diesen Steuerschlüssel haben immer ignoriert. Heißt, wenn man das Tripel chart_id, startdate und taxkey_id als unique definiert, so kann man gleich das Tupel taxkeys.startdate und taxkeys.chart_id als unique definieren und die Spalte chart.taxkey_id löschen. Letzteres ist schon von mir auf Geoffry's Vorschlag hin implementiert worden und befindet sich noch in der Testphase.

• Jetzt hat man das übliche Tripel chart_id, startdate (bzw. ein höheres Datum durch das Buchungsdatum) und taxkey_id aus chart, fragt in taxkeys die dazugehörige Steuer taxkeys.tax_id ab.
• Die Steuer bekommt man nun aus tax via taxkeys.tax_id == tax.id. Da ist tax.taxkey also gar nicht involviert.

Aber vielleicht übersehe ich auch momentan etwas...

...ja ich mag den ganzen strukturellen Unterbau auch nicht wirklich.

Sollte nicht auf jeden Fall auch das Paar taxkeys.startdate und taxkeys.chart_id eindeutig sein?

Spontan: nein, in taxkeys muss nur das oben erwähnte Tripel eindeutig sein.

Siehe oben, viele Abfragen benutzen überhaupt gar keinen Steuerschlüssel, sondern nur Datum und Konto, wenn ich mich nicht irre. Ich fände es auch sehr komisch, wenn ein Konto für einen Tag mehrere Steuern haben kann.

Ach ja ein Beispiel, wo tax.taxkey unbedingt gebraucht wird ist beim Steuerschlüssel 0. Das benutze ich zumindest sehr häufig bei den jüngsten Updates (SELECT tax_id FROM tax WHERE taxkey=0 LIMIT 1). Kann natürlich sein, dass es noch andere, bessere Beispiele gibt.

Replying to Niclas:

chart_id NULL oder ungültig (weist auf ein nicht vorhandenes Konto): Zeile löschen.

OK.

startdate NULL: Zeile löschen (im Moment werden solche Einträge von kivitendo nämlich ignoriert)

OK.

tax_id NULL, taxkey_id NOT NULL: Fehlermeldung und Update wird abgebrochen.
Variante: erstelle neuen Eintrag in tax mit a) 0%-Steuern und b) angegebener taxkeys.taxkey_id (falls dieser noch nicht existiert). Schreibe die id von diesem Eintrag in taxkey.tax_id. kivitendo verhält sich im Augenblick zumindest so, dass es keine Steuern berechnet, wenn tax_id NULL ist, daher rührt dieser Vorschlag.

Die Variante ist gut.

tax_id NOT NULL, taxkey_id NULL: Nehme Steuerschlüssel von tax.taxkey (wenn der auch NULL ist, muss abgebrochen werden)
Variante: definiere einen neuen Steuerschlüssel, der noch nicht vorhanden ist.

tax.taxkey darf nicht NULL sein (hatte ich ganz oben initial geschrieben). Das bedeutet, dass Einträge in tax mit taxkey IS NULL zuerst irgendwie behandelt werden müssen. Vielleicht... wirklich löschen. Oder Upgrade abbrechen.

Wenn dann tax.taxkey definitiv nicht NULL sein kann, dann für taxkeys.tax_id NOT NULL und taxkey_id NULL in der Tat tax.taxkey nehmen.

tax_id NULL und taxkey_id NULL: Setze Steuerschlüssel 0 und entsprechende tax_id.

OK.

Hintergrund. Die Logik ist:

...falsch. Du hast Recht.

Die Logik ist so, dass man in der Oberfläche das Belegdatum, den Artikel und die ausgewählte Steuerzone hat. Dann holt sich kivitendo (z.B. SL/IS.pm, retrieve_item):

• über die Buchungsgruppe der ausgewählten Steuerzone...
• ...über die inventory_accno_id den Eintrag aus chart...
• ...von diesem die accno, new_chart_id (Folgekonto) und valid_from...
• ...folgt der Kette von new_chart_id und valid_from solange, bis es den letzten Eintrag in dieser Kette gefunden hat. Nun hat die Funktion also das Belegdatum, die dazugehörige Konto-ID chart_id und -Nummer accno. Dazu...
• ...holt sie aus taxkeys die Einträge für chart_id, deren Startdatum kleiner gleich dem Belegdatum ist (also den "gültigen taxkeys-Eintrag")...
• ...nimmt sich dann alle Einträge aus tax alle Einträge, deren id denjenigen gleicht, die im Schritt davor in taxkeys gefunden wurden (SELECT FROM tax ... WHERE tax.id IN (SELECT taxkeys.tax_id ... WHERE chart_id = ? AND startdate < ?))...
• ...und hat dann alle möglichen Steuern dafür.

Und der Steuerschlüssel (also tax.taxkey oder taxkey.taxkey_id) ist dabei gar nicht beteiligt (such mal in SL/IS.pm, sub retrieve_item, nach taxkey).

Der dort verwendete Algorithmus wäre in der Tat in der Lage, mit einer Situation umzugehen, in der es für ein Tupel (chart_id, startdate) mehrere Einträge in taxkeys gäbe, wodurch potenziell für ein Konto für einen Zeitpunkt gleichzeitig mehrere Steuern gelten würden. Das ist aber logischer Unfug und sollte eigentlich genau verhindert werden (sowohl datenbankseitig als auch beim Bearbeiten/Speichern? von Konten und den dazugehörigen Steuereinträgen).

Du hast also Recht mit dem hier:

Siehe oben, viele Abfragen benutzen überhaupt gar keinen Steuerschlüssel, sondern nur Datum und Konto, wenn ich mich nicht irre. Ich fände es auch sehr komisch, wenn ein Konto für einen Tag mehrere Steuern haben kann.

Genau. Das Tupel (chart_id, startdate) muss eindeutig sein.

Lass tax.taxkey und taxkeys.taxkey_id erst einmal aus der Betrachtung heraus, ich habe mich da bei meiner initialen Auflistung zu gewünschten Fremdschlüsseln schlicht zu sehr darin geirrt, wie die Algorithmen funktionieren.

Ok, ich hab hier mal ne kleine Übersicht gemacht, von dem, was jetzt gemacht wird (bin schon dran und fast fertig)

Für die Tabelle tax:

Spaltenname	Constraints, die gesetzt werden	Verhalten bei ungültigen Einträgen
chart_id	Fremdschlüssel auf chart.id	Abbrechen
rate	NOT NULL + DEFAULT 0	Update auf 0%
taxkey	NOT NULL	Abbrechen
taxdescription	NOT NULL	Update auf "-"

Für die Tabelle taxkeys:

Spaltenname	Constraints, die gesetzt werden	Verhalten bei ungültigen Einträgen
chart_id	NOT NULL + Fremdschlüssel auf chart.id	Zeile löschen
tax_id	NOT NULL + Fremdschlüssel auf tax.id	Auf Eintrag mit 0% Steuern referenzieren
taxkey_id	NOT NULL	Update mit tax.taxkey
startdate	NOT NULL	Zeile löschen
chart_id+startdate	UNIQUE	Doppelte Zeilen löschen

Was noch zu überlegen wäre ist eine Konsistenzprüfung für tax.taxkey und taxkey.taxkey_id. Diese Felder müssen ja eigentlich übereinstimmen. Diese Information ist redundant und eine Spalte sollte vielleicht (wie der Eintrag chart.taxkey_id) gelöscht werden.

Das Löschen erfordert im Vorfeld eine gute Untersuchung, wo welche Spalte noch angesprochen wird -- und gutes Testen, nachdem man die entsprechenden Queries entsprechend angepasst hat. Aber ansonsten gebe ich dir Recht, ja, das wäre schon gut.

Rest sieht soweit gut aus.

Diese Schlüssel werden im Update-Script tax_constraints gesetzt. Das Ticket ist somit schon implementiert in Commit 18931692ebe56ef5a2afa471182d3845914979c2.