Löschfunktion verbessern - eigenes Modul

[m-werk]

Hallo, ich hab jetzt in meinem DCA eine Löschroutine eingebaut, die eigentlich funktioniert.

So sieht der Code aus:

    public function deleteCourseRoutine()
    {    
        $resCount = \Database::getInstance()->prepare("SELECT id FROM tl_market_course")
                                          ->execute();
        
        $rows = array();
        while($resCount->next())
        {
            $rows[] = $resCount->row();
        }
            
        $total = count($rows);
        $limit = $total-21;
        $delete = '';
        
        if ($total > 21)
        {
            $result = \Database::getInstance()->prepare("SELECT id, date FROM tl_market_course ORDER BY date ASC LIMIT 0, ?")
                                        ->execute($limit);
            $result_date = array();
            while ($result && $result->next())
            {
                $result_date = $result->date;    
            }
            $delete = \Database::getInstance()->prepare("DELETE FROM tl_market_course WHERE date=?")
                                              ->execute($result_date)
                                              ->affectedRows;
        }

        return $delete;
    }    
} 

Wenn ich im Backend nun auf Löschen gehe, werden alle Daten, wo das Datum übereinstimmt gelöscht.

Das Problem: Wenn ich jetzt 100 Datensätze habe und jweils 5 Dateinsätze von einem Datum habe, muss ich X-Mal Löschen klicken, bis ich auf dem Wert 21 bin. Kann man das auch so machen, dass ich nur 1x Löschen klicke, bis ich auf dem Wert 21 bin?

2. Wenn ich auf löschen klicke, kommt dann im Backend eine Seite ohne Inhalt. Ich würde gerne wieder auf die Übersichtseite gelangen, so wie wenn ich bei einem Datensatz auf "Speichern und schließen" klicke.

Wer kann mir dabei helfen?

[m-werk]

So, nach langer Suche hab ich nun die Lösung für das 2. Problem gefunden.

\System::redirect('contao/main.php?do=market_course'); 

und return habe ich entfernt, da dies dort unnötig ist. Ich gebe ja nichts zurück ;-)

Aber die Optimierung der eigentlichen Löschfunktion steht bei mir leider noch aus.

[fiedsch]

Wenn ich Dich richtig verstanden habe, möchtest Du alle Records bis auf die 21 aktuellsten (bzgl. Datum) löschen.

Dann würde ich es so machen:

Die IDs der aktuellsten 21 holen SELECT id FROM ... ORDER BY date DESC LIMIT 21;

Löschen DELETE FROM ... WHERE id NOT IN (...);

Alles zusammen:

DELETE FROM ... WHERE Id NOT IN (SELECT id FROM ... ORDER BY date DESC LIMIT 21);

[m-werk]

Hallo, danke für deine Info, somit würde ich ja eigentlich einiges an Code mir ersparen. Ich werde das mal testen.

Aber inzwischen hab ich es auch gelöst, nur anders. Ich musste nur die Löschfunktion in die While-Schleife einbauen. Das hab ich total übersehen.

Jetzt tut es was es tun soll.

Zur Information:
Ich gebe jeden Tag Wertpapierkuse für 7 Wertpapiere ein.
Da ich die alten Kurse nicht benötige (Ausgenommen die letzten 3 Tage) möchte ich diese immer wieder bereinigen können. Dies stoße ich mit dieser Funktion jetzt an. Es werden alle alten Wertpapiere, die älter als 3 Tage sind, gelöscht. Da es 7 Wertpapiere sind und ich 3 Tage behalten möchte, zähle ich hier die Gesamten Kurseinträge und ziehe 21 davon ab. Das ist mein limit.

Sicherlich, ich könnte jetzt auch beim Löschen "where ID = ?" nehmen, aber dann habe ich mehr anforderungen an die Datenbank, denn wenn ich jetzt z.B. 2 Tage lösche, sind das 14 mal delete from...
So sind es nur 2.

[m-werk]

Hallo, hab nun deinen Vorschlag genommen und bekomme nun folgende Fehlermeldung

An exception occurred while executing 'DELETE FROM tl_market_course WHERE id NOT IN (SELECT id FROM tl_market_course ORDER BY date DESC LIMIT 21)': SQLSTATE[42000]: Syntax error or access violation: 1235 This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'

So sieht nun mein Code aus:

$limit = 21;
\Database::getInstance()->prepare("DELETE FROM tl_market_course WHERE id NOT IN (SELECT id FROM tl_market_course ORDER BY date DESC LIMIT ?)")
                            ->execute($limit)
                            ->affectRows; 

[fiedsch]

Das

\Database::getInstance()->prepare("DELETE FROM tl_market_course WHERE id NOT IN (SELECT id FROM tl_market_course ORDER BY date DESC LIMIT ?)")
                            ->execute($limit)
                            ->affectRows; 

schaut für mich schon korrekt aus; beim ->execute($limit) bin ich mir nicht 100% sicher, ob das passt

ABER

This version of MySQL doesn't yet support 'LIMIT & IN/ALL/ANY/SOME subquery'

ist eigentlich recht eindeutig: Die MySQL-Version unterstützt das Konstrukt nicht. Ob es eine aktuellere gibt, die das tut, keine Ahnung, aber schau vielleicht mal hier: https://stackoverflow.com/questions/...subquery-limit

[mvo]

Kein Grund eine Subquery zu nehmen:

DELETE FROM tl_market_course ORDER BY date DESC LIMIT 21 

Aber wenn du Sachen löschen möchtest, die älter als 3 Tage sind, dann ist das der falsche Ansatz. Vergleiche in diesem Fall das Datum, statt eine generische Abfrage zu limiten.

[m-werk]

Hm, mit diesem Code hat es nun funktioniert:

            \Database::getInstance()->prepare("DELETE FROM tl_market_course WHERE id NOT IN (SELECT * FROM (SELECT id FROM tl_market_course ORDER BY date DESC LIMIT 21) AS derivedTable)")
                                    ->execute()
                                    ->affectRows; 

Leider funktioniert es nicht, wenn ich ->execute($limit) eingebe und bei Limit ? schreibe. Ich könnte aber sehrwohl im DELETE das '.$limit.' einbinden, oder?

NACHTRAG: ja, wenn ich im Delete hinten bei LIMIT $limit eintrage, funktioniert es auch.

Danke für den TIPP.

[m-werk]

Kein Grund eine Subquery zu nehmen:

DELETE FROM tl_market_course ORDER BY date DESC LIMIT 21 

Aber wenn du Sachen löschen möchtest, die älter als 3 Tage sind, dann ist das der falsche Ansatz. Vergleiche in diesem Fall das Datum, statt eine generische Abfrage zu limiten.

Das mit dem Datum hatte ich auch vor, aber z.B. am Wochenende werden keine Kurse eingegeben.

[fiedsch]

Leider funktioniert es nicht, wenn ich ->execute($limit) eingebe und bei Limit ? schreibe. Ich könnte aber sehrwohl im DELETE das '.$limit.' einbinden, oder?

Kannst Du machen und da Du das $limit unter Kontrolle hast (=selbst festlegst) sollte das auch kein Sicherheitsproblem sein.

Allerdings würde ich mir auch den Kommentar von @mvo (#7) anschauen, und die Abfrage abhängig vom Datum machen. Es gibt aber wahrscheinlich noch mehr zu bedenken, wie z.B. was soll gelöscht werden, wenn Du -- warum auch immer -- einige Tage keine neuen Kurse für ein oder mehrere Wertpapiere eingegeben hast.

[mvo]

Das kannst du problemlos in einer einzigen Query machen.

Ich geb dir ein bisschen Hilfestellung, damit du es dir zusammensuchen kannst:

Du musst nur eine einfache Query ohne Parameter ausführen - ich nehme an du bist unter Contao 3.5:

 \Database::getInstance()->query("...") 

Ein einfacher Ansatz wäre es alles zu löschen, außer die Elemente der 3 jüngsten Tage.

DELETE FROM tl_market_course WHERE id NOT IN(...) 

Die Subquery muss dann alle Ids der letzten 3 Tage zurückgeben.

SELECT id FROM tl_market_course WHERE ... 

Dir helfen dabei die folgenden MySQL Funktionen für die Bedingung:

FROM_UNIXTIME() 

, falls du Timestamps verwendest

DATE_ADD() 

und etwas wie

INTERVAL 3 DAYS 

Das ist jetzt eine gute Gelegenheit sich mit der MySQL Dokumentation vertraut zu machen. Zum Entwickeln und Testen empfiehlt es sich eine MySQL Konsole aufzumachen und darin erst einmal die Subquery zu testen, bis diese passt.

[m-werk]

Hm, ok, ich werde mir das mal jetzt in ruhe ansehen.

Übrigens, ich verwende Contao 4.4

[mvo]

Übrigens, ich verwende Contao 4.4

Dann nutze

$connection->executeQuery("..."); 

$connection ist in diesem Fall eine Instanz von Doctrine\DBAL\Connection, die du dir z.B. in einem Service mit '@database_connection' injecten kannst.

[m-werk]

Ok, ich hab aber überall bei mir \database::....

Soll ich das überall ändern auf $connection?

[mvo]

Der richtige Ansatz wäre es die vorhandene Datenbankverbindung aus dem DI Container zu nehmen. Hier mal ein Minimalbeispiel für einen einfachen Service:

Erstelle dir einen Service, mit der Datenbankverbindung als Argument (--> Constructor-Injection):

#services.yml

services:
  my_bundle.my.service:
    class: 'MyBundle\PathToMy\SuperClass'
    arguments:
      - '@database_connection' 

Schreibe deine Klasse:

#SuperClass.php

namespace MyBundle\PathToMy;

use Doctrine\DBAL\Connection;

class SuperClass
{
    /** @var Connection */
    private $connection;

    public function __construct(Connection $connection)
    {
        $this->connection = $connection;
    }

    public function pruneCourses() 
    {
        // ...
    }
} 

An jeder Stelle in Contao, wo du Callbacks der Sorte ['Klasse', 'Methode'] angeben kannst, kannst du stattdessen auch die Service-Definition verwenden. Hier z.B. ['my_bundle.my.service', 'pruneCourses'].

Bonus: Hooks lassen sich direkt als 'tagged service' definieren und sind damit dann automatisch registriert. Praxisbeispiel zum Nachvollziehen aus einem Repository von mir, bei dem ich im letzten Change genau das geändert habe (alte Hook Definition > direkt als tagged service): https://github.com/m-vo/contao-neste...g/services.yml

Google-Suchbegriffe: Symfony DI Container, Service und ggf. Tagged Service

[m-werk]

So, ich muss mich da wirklich mal durchlesen. Ich hab das alles auch nicht gelernt. Alles was ich weiß ist Selbststudium via Internet (Foren usw.)

Ich hab es jetzt mal geschafft, mein SELECT bzw. mein Delete zu verfeinern.

Hiermit sollte es möglich sein, dass alles gelöscht wird, was älter als 3 Tage sich in der DB befindet: (Das ist jetzt nur zu Testzwecken ein SELECT, damit ich sehen kann, ob auch das richtige Ergebnis angezeigt wird)

SELECT id, FROM_UNIXTIME(DATE) FROM `tl_market_course` WHERE (SELECT FROM_UNIXTIME(date) FROM tl_market_course LIMIT 1) >= DATE_SUB(FROM_UNIXTIME(date), INTERVAL 3 DAY)

Wenn ich diesen SELECT bzw DELETE FROM ausführe, benötige ich den Code gar nicht

DELETE FROM tl_market_course WHERE id NOT IN (...

. Es werden mit diesem SELECT sowieso nur jene angezeigt, die in der DB älter als 3 Tage sind. Oder sehe ich das falsch?


Weiters hab ich noch folgende änderungen durchgeführt.

Ich hab nun anstelle von

\Database::getInstance()->prepare 

nun

$this->Database->prepare 

eingegeben. Ist eigentlich jetzt die richtige Schreibweise für Contao 4, oder? Wobei ja das 1. auch funktioniert.

[m-werk]

Mensch, mein oben genanntes SELECT funktioniert auch nicht richtig. Aber so sollte es gehen:

SELECT id, FROM_UNIXTIME(date) FROM tl_market_course WHERE id NOT IN (SELECT id FROM `tl_market_course` WHERE FROM_UNIXTIME(date) >= DATE_SUB(CURDATE(), INTERVAL 3 DAY))

Hier werden mir jetzt schon mal die alten Daten angezeigt.

Mir war dar Curdate() noch nicht klar. Wenn ich das richtig verstanden habe, nimmt Curdate() das Datum aus der Datenbank, oder, denn wenn ich NOW() eingebe wird ja das aktuelle Datum genommen.

[m-werk]

Ich komm einfach mit dem SELECT nicht klar. Aber jetzt dürfte es passen. Nun:

SELECT id, FROM_UNIXTIME(date) FROM tl_market_course WHERE id NOT IN (SELECT id FROM `tl_market_course` WHERE FROM_UNIXTIME(date) > DATE_SUB((SELECT FROM_UNIXTIME(date) FROM tl_market_course ORDER BY date DESC LIMIT 1), INTERVAL 3 DAY))

Hier werden nun genau jene Datensätze angezeigt, die in der DB älter als 3 Tage sind. Bitte kann mir das jemand bestätigen?
Ich hab zu Testzwecken mal mehrere Tage in der DB angelegt und einzelln manuel diese gelöscht. Das Ergebnis mit dieser Abfrage war immer richtig.

Wenn ich jetzt anstellen vom 1. Select ein Delete nehme, werden mir dann ja auch die Daten gelöscht, die ich mit dem SELECT sehe oder?

[mvo]

Programmieren ist für die allermeisten Selbststudium. Es tut mir leid, aber du wirst dich da 'durchquälen' müssen. Die Community kann dir bei konkreten Problemen helfen, aber die Basics musst du selbst mitbringen. Es gibt zahlreiche gute Tutorials im Web und es ist auch gut ein konkretes Problem zu haben, an dem arbeiten kann. Bei dir ist es jetzt erst einmal weniger PHP und die Symfony Umgebung (das kommt im zweiten Schritt) als vielmehr MySQL.

Schau dir an, wie eine Abfrage zum Löschen allgemein aussieht:
https://dev.mysql.com/doc/refman/5.7/en/delete.html

Darin gibt es viele Optionen, du die nicht benötigst, aber aufs wesentliche reduziert steht da:

DELETE FROM table WHERE condition ORDER BY field LIMIT row_count 

Übersetzt:

Lösche alle Einträge aus Tabelle 'table', auf die die Bedingung 'condition' zutrifft, allerdings nur die nach Feld 'field' sortiert ersten 'row_count' Einträge.

Wenn du eine Subquery (= irgendwo ein SELECT ... innerhalb deiner Abfrage) vermeiden kannst, ohne dass es die Query unendlich kompliziert und unlesbar macht, dann solltest du das tun. Ich empfehle dir fürs Lernen: Verzichte ganz drauf, du wirst das zu 99.9% nie brauchen und wenn, dann merkst du wo es Sinn macht. :-)

Zurück zur Delete Query. Du willst alles löschen, was älter als 3 Tage ist - wie lässt sich das im oberen Schema sprachlich ausdrücken?

Versuch:

Lösche alle Einträge aus der 'Wertpapier-Tabelle', auf die die Bedingung 'WertpapierDatum älter als 3 Tage' zutrifft.

Du brauchst dafür also noch nicht einmal die Sortierung oder die Limitierung auf die ersten n Einträge. Prima - je einfacher, desto besser.

Wie lässt sich jetzt 'WertpapierDatum älter als 3 Tage' ausdrücken? Älter als was? In der sprachlichen Formulierung fehlt der Vergleich: 'WertpapierDatum mehr als 3 Tage vor dem heutigen Datum'. In einer Gleichung ausgedrückt liest sich 'älter als 3 Tage' also so:

WertpapierDatum < HeutigesDatum - 3 Tage

Das Problem reduziert sich jetzt also auf die Fragen: Wie bekomme ich 'HeutigesDatum'? und: Wie ziehe ich davon 3 Tage ab? und: Wie kann man Daten vergleichen?
zu Frage 1: https://dev.mysql.com/doc/refman/5.7...nction_curdate
zu Frage 2: https://dev.mysql.com/doc/refman/5.7...ction_date-add
zu Frage 3: wie Zahlen! <, <=, ==, >=, >

Du hast eine Spalte, die Zeitinformation enthält, allerdings nicht als Darum, sondern als Timestamp. Google das, falls dir nicht klar ist, was der Unterschied ist. Hier kommt jetzt etwas Programmierer-Erfahrung ins Spiel: Du kannst zwar mit Timestamps rechnen, allerdings hat das viele Tücken (von Schaltsekunden bis Zeitzonen), drum empfiehlt es sich die in MySQL vorhandenen Datumsfunktionen zu nutzen.

Wie lässt sich ein Timestamp in ein Datum umwandeln?
zu Frage 4: https://dev.mysql.com/doc/refman/5.7..._from-unixtime

-----------------------

Setz das mal zusammen: Wie sieht jetzt deine Query aus? Wenn du testen willst, ohne dass die Einträge immer gelöscht werden kannst du erst einmal das DELETE durch ein SELECT * ersetzen.

[m-werk]

Hallo, danke für deine Infos.

Ganz so ist es nicht, dass ich nicht weiß, wie man einen Datensatz anlegt, löscht oder ein Update macht. In meinem Beispiel von oben hab ich deshalb SELECT stehen, da ich nicht anhand Tests alles Löschen möchte.

Zu meinem Fall:
"Heute -3 Tage" kann ich bei mir nicht verwenden.

Ich speichere ja die Kurse Täglich in die DB "AUSGENOMMEN WOCHENENDE" Da gibt's keine Kurse.

Daher kann ich nicht "Heute -3 Tage" nehmen. Ich muss aus der Datenbank den letzten Kurs heranziehen, den ich eingetragen habe. Von dem aus muss ich ausgenen.

Daher finde ich jetzt keinen Kürzeren Weg, als diesen Code. Der User klickt ja nicht jeden Tag auf diesen Button, sondern macht es, wann er möchte. Jetzt kann schon sein, dass in der DB Kurse von über 30 Tagen existieren. Dies dient ja nur zur reinhaltung der DB. Ich hab deshalb 3 Tage genommen (Es können ja auch 5 sein), da es eine Berechnung vom aktuellen Kurs und dem Vortagkurs gibt.

Ich hab vorige Woche sehr viel gelesen und herumgetüftelt. Mir ist aber kein kürzerer Code eingefallen, der genau das macht, was er machen soll.

DELETE FROM tl_market_course WHERE id NOT IN (SELECT id FROM `tl_market_course` WHERE FROM_UNIXTIME(date) > DATE_SUB((SELECT FROM_UNIXTIME(date) FROM tl_market_course ORDER BY date DESC LIMIT 1), INTERVAL 3 DAY))

Ich hab auch in der Datenbank mich herungespielt und alle möglichen Szenarien eingegeben. (Kurse über 8 Tage, nur den letzen Tag usw.) Das Ergebnis war immer richtig.

Nachtrag:
Ich könnte ja auch hergehen und mehrere SQL-Statements machen, und einfach beim Delete die entsprechenden Werte eintragen. Dann hab ich eben ein Select für den letzen eingetragenen Datensatz, ein Select für die Werte, die kommen, wenn ich die letzten 3 Tage nehme und dann ein Delete.

Soll das nicht besser alles in einem passieren?

[BugBuster]

SELECT FROM_UNIXTIME(date) FROM tl_market_course ORDER BY date DESC LIMIT 1

ginge da nicht auch sowas in der Art:

SELECT MAX(FROM_UNIXTIME(date)) FROM tl_market_course

Bringt das neuste Datum zurück. (habs nicht getestet)

Nachtrag: Noch besser, dann braucht nicht jedes "date" umgewandelt werden:

SELECT FROM_UNIXTIME(MAX(date)) FROM tl_market_course

[m-werk]

Ja, das habe ich auch schon probiert. Dabei ist der Code etwas kürzer, aber um das letzte Select komm ich trotzdem nicht herum. Es entfällt lediglich das ORDER und LIMIT.

[BugBuster]

Eben. "order" muss erst alle Datensätze sortieren um dir dann nur einens zu geben per "limit".
Bei wenigen Datensätzen ist das vielleicht noch OK, bei vielen kommt es dann schnell zum internen "order in temporary table... / filesort", und das verzögert die Sache.

Ich bin da sehr vorsichtig, ich versuche "order" zu vermeiden wo es geht. Muss aber dazu sagen, ich arbeite hier auf Arbeit mit Millionen von Datensätzen :-)

[m-werk]

Danke für den Tipp.

Mit so vielen Datensätzen habe ich bis dato noch nicht zu tun gehabt. In diesem Beispiel hier handelt es sich lediglich um einige Datensätze. Hier komm ich nicht einmal an die 1000, wenn es viel ist.

Aber gut zu wissen.

[mvo]

Warum sollte ein ORDER BY + LIMIT ein Performance Problem geben? Man muss halt einen Index verwenden. :-)

Du brauchst trotzdem nur eine Subquery, die dir entweder den letzten noch gültigen Timestamp oder alle gültigen selected und löschst dann alle älteren bzw. anderen.

Alle gültigen sind z.B. eine DISTINCT oder GROUP BY Selektion des Datums mit ORDER + LIMIT 3. Analog bekommst du den letzten gültigen Wert, wenn du das dritte eindeutige Element selektierst.

Was die Performance angeht: Lieber Lesbarkeit und
sauberen Code. Weil meinen: erst darum kümmern, wenn es ein Problem ist. Bei 1000 Datensätzen ist das wohl vorerst egal.

Was du machen willst klingt ein bisschen wie ein Pruning-Job, der auch automatisiert abkaufen könnte? Wenn dem so ist, solltest du evtl. einen Cronjob nutzen oder am besten gleich dafür sorgen, dass die Struktur konsistent bleibt.