From e3cb994e0e633421e195ccf19669bd0a2e9567bc Mon Sep 17 00:00:00 2001 From: ispyisail Date: Mon, 24 Aug 2026 22:51:39 +1200 Subject: [PATCH] Cascade-delete a diagram's element/element_info/terminal/conductor rows removeDiagram() only ever deleted the diagram's own row. No foreign key in this schema is declared ON DELETE CASCADE (and SQLite foreign-key enforcement is never turned on for this connection anyway), so removing a diagram left every element, element_info, terminal and conductor row that belonged to it behind in the database -- silently, since nothing reads them until the next full updateDB() rebuild papers over it. Traced why this had never crashed anything: Diagram::~Diagram() explicitly walks and deletes its top-level items through removeItem() (which does call dataBase()->removeElement() correctly), but deliberately skips conductors -- because a conductor's destructor touches both of its terminals (terminal1->removeConductor(this)), and those terminals may belong to an element already destroyed earlier in the same sweep. Conductors are instead destroyed as a side effect of Terminal::~Terminal()'s qDeleteAll() on its own conductor list, which is a plain C++ delete that never goes through Diagram::removeItem() and therefore never calls dataBase()->removeConductor() at all. So the object graph is torn down safely, but the database is never told about the conductors or their terminals. Fixed by adding the missing bulk deletes to projectDataBase::removeDiagram() itself, run while the diagram (and its live scene) still exist -- verified that QETProject::detachDiagram() emits diagramRemoved() (which this class's constructor connects to this slot) synchronously, before the Diagram object is scheduled for destruction via deleteLater(), so nothing here races the C++ teardown described above. Order matters: element_info and terminal have no diagram_uuid column of their own, so both are scoped through a subquery on element and must run before element itself is deleted. Verified against examples/industrial.qet (50 diagrams) by calling projectDataBase::removeDiagram() directly and comparing table counts before and after, with no intervening updateDB() call to mask a gap: element=354->335 element_info=354->335 terminal=1087->1033 conductor=671->626 diagram=50->49 Every delta matches a direct SQL count for that diagram's own rows exactly (19 elements, 54 terminals), and both "orphan rows still referencing the removed diagram" checks read 0 afterward -- so the cascade is complete and, just as importantly, scoped: nothing belonging to the other 49 diagrams moved. Separate finding, not fixed here: QETProject::removeDiagram(Diagram*) (the synchronous, non-undoable variant, not the usual GUI ProjectView::removeDiagram() path) segfaults if the enclosing QETProject is destroyed before an event loop iteration lets its pending deleteLater() run -- reproduces identically on unmodified master, so it predates and is unrelated to this change. Worth its own report; a headless caller is the only realistic way to hit it, which is how this surfaced. Co-Authored-By: Claude Sonnet 5 --- sources/dataBase/projectdatabase.cpp | 57 +++++++++++++++++++++++++++- sources/dataBase/projectdatabase.h | 6 ++- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/sources/dataBase/projectdatabase.cpp b/sources/dataBase/projectdatabase.cpp index 37bb3d2e6..1f4770230 100644 --- a/sources/dataBase/projectdatabase.cpp +++ b/sources/dataBase/projectdatabase.cpp @@ -225,7 +225,40 @@ void projectDataBase::addDiagram(Diagram *diagram) void projectDataBase::removeDiagram(Diagram *diagram) { - m_remove_diagram_query.bindValue(":uuid", diagram->uuid().toString()); + const QString uuid_str = diagram->uuid().toString(); + + //Order matters: element_info and terminal are scoped through a + //subquery on element, so they must run before element itself is + //deleted below. Without this, a removed diagram left its rows + //behind in every one of these tables until the next full + //updateDB() rebuild -- invisible day to day, since nothing reads + //them meanwhile, but a real inconsistency between the live scene + //and the database in between. + m_cascade_remove_element_info_query.bindValue(":uuid", uuid_str); + if (!m_cascade_remove_element_info_query.exec()) { + qDebug() << "projectDataBase::removeDiagram element_info cascade error : " + << m_cascade_remove_element_info_query.lastError(); + } + + m_cascade_remove_terminal_query.bindValue(":uuid", uuid_str); + if (!m_cascade_remove_terminal_query.exec()) { + qDebug() << "projectDataBase::removeDiagram terminal cascade error : " + << m_cascade_remove_terminal_query.lastError(); + } + + m_cascade_remove_conductor_query.bindValue(":uuid", uuid_str); + if (!m_cascade_remove_conductor_query.exec()) { + qDebug() << "projectDataBase::removeDiagram conductor cascade error : " + << m_cascade_remove_conductor_query.lastError(); + } + + m_cascade_remove_element_query.bindValue(":uuid", uuid_str); + if (!m_cascade_remove_element_query.exec()) { + qDebug() << "projectDataBase::removeDiagram element cascade error : " + << m_cascade_remove_element_query.lastError(); + } + + m_remove_diagram_query.bindValue(":uuid", uuid_str); if (!m_remove_diagram_query.exec()) { qDebug() << "projectDataBase::removeDiagram delete error : " << m_remove_diagram_query.lastError(); } else { @@ -772,7 +805,27 @@ void projectDataBase::prepareQuery() m_insert_diagram_query = QSqlQuery(m_data_base); m_insert_diagram_query.prepare("INSERT INTO diagram (uuid, pos) VALUES (:uuid, :pos)"); - //REMOVE DIAGRAM + //REMOVE DIAGRAM (cascade first: element_info and terminal have no + //diagram_uuid column of their own, so both are scoped through + //element while the element rows for this diagram still exist). + m_cascade_remove_element_info_query = QSqlQuery(m_data_base); + m_cascade_remove_element_info_query.prepare( + "DELETE FROM element_info WHERE element_uuid IN " + "(SELECT uuid FROM element WHERE diagram_uuid = :uuid)"); + + m_cascade_remove_terminal_query = QSqlQuery(m_data_base); + m_cascade_remove_terminal_query.prepare( + "DELETE FROM terminal WHERE element_uuid IN " + "(SELECT uuid FROM element WHERE diagram_uuid = :uuid)"); + + m_cascade_remove_conductor_query = QSqlQuery(m_data_base); + m_cascade_remove_conductor_query.prepare( + "DELETE FROM conductor WHERE diagram_uuid = :uuid"); + + m_cascade_remove_element_query = QSqlQuery(m_data_base); + m_cascade_remove_element_query.prepare( + "DELETE FROM element WHERE diagram_uuid = :uuid"); + m_remove_diagram_query = QSqlQuery(m_data_base); m_remove_diagram_query.prepare("DELETE FROM diagram WHERE uuid=:uuid"); diff --git a/sources/dataBase/projectdatabase.h b/sources/dataBase/projectdatabase.h index cddf53468..9b5505b4c 100644 --- a/sources/dataBase/projectdatabase.h +++ b/sources/dataBase/projectdatabase.h @@ -106,7 +106,11 @@ class projectDataBase : public QObject m_insert_terminal_query, m_insert_conductor_query, m_update_conductor_query, - m_remove_conductor_query; + m_remove_conductor_query, + m_cascade_remove_element_info_query, + m_cascade_remove_terminal_query, + m_cascade_remove_conductor_query, + m_cascade_remove_element_query; #ifdef QET_EXPORT_PROJECT_DB public: