From d98338ce89089755aef305f8c04ce22ac2509ee9 Mon Sep 17 00:00:00 2001 From: Martin Kleusberg Date: Fri, 19 Aug 2016 20:01:25 +0200 Subject: [PATCH] Move all foreign key handling form the fields to the tables Foreign keys used to be stored along with the column information even though it's more or less a table constraint. However, as we only support single column foreign keys it was easier to store it inside that single column. Now with multi-column foreign keys coming, a mechanism has been introduced to store those multi-column foreign keys in the table data. This lead to two different storing places for foreign key information: inside the field for one-column foreign keys and inside the table for multi-column foreign keys. This commit deletes the foreign key storage inside fields and changes all code to use the table storage. --- src/EditTableDialog.cpp | 19 +++++++++---- src/sqlitetablemodel.cpp | 2 +- src/sqlitetypes.cpp | 53 ++++++++++++++++++++++++------------ src/sqlitetypes.h | 15 ++++++---- src/tests/testsqlobjects.cpp | 6 ++-- 5 files changed, 62 insertions(+), 33 deletions(-) diff --git a/src/EditTableDialog.cpp b/src/EditTableDialog.cpp index 84c459b7..af3d0127 100644 --- a/src/EditTableDialog.cpp +++ b/src/EditTableDialog.cpp @@ -115,7 +115,7 @@ void EditTableDialog::populateFields() tbitem->setText(kDefault, f->defaultValue()); tbitem->setText(kCheck, f->check()); - tbitem->setText(kForeignKey, f->foreignKey().toString()); + tbitem->setText(kForeignKey, m_table.foreignKey(f).toString()); ui->treeWidget->addTopLevelItem(tbitem); } @@ -233,14 +233,19 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) { foreach(const DBBrowserObject& fkobj, pdb->objMap.values("table")) { - foreach(const sqlb::FieldPtr& fkfield, fkobj.table.fields()) + sqlb::ForeignKeyMap::ConstIterator it = fkobj.table.foreignKeys().constBegin(); + while(it != fkobj.table.foreignKeys().constEnd()) { - if(fkfield->foreignKey().table() == m_table.name()) + const sqlb::FieldVector& fkfields = it.key(); + const sqlb::ForeignKeyClause& fk = it.value(); + if(fk.table() == m_table.name()) { - if(fkfield->foreignKey().columns().contains(field->name()) || field->primaryKey()) + if(fk.columns().contains(field->name()) || field->primaryKey()) { QMessageBox::warning(this, qApp->applicationName(), tr("This column is referenced in a foreign key in table %1, column %2 and thus " - "its name cannot be changed.").arg(fkobj.getname()).arg(fkfield->name())); + "its name cannot be changed.") + .arg(fkobj.getname()) + .arg(sqlb::fieldVectorToFieldNames(fkfields).join(","))); // Reset the name to the old value but avoid calling this method again for that automatic change ui->treeWidget->blockSignals(true); item->setText(column, oldFieldName); @@ -248,6 +253,8 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) return; } } + + ++it; } } } @@ -418,7 +425,7 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) case kForeignKey: sqlb::ForeignKeyClause fk; fk.setFromString(item->text(column)); - field->setForeignKey(fk); + m_table.addForeignKey(field, fk); if(!m_bNewTable) callRenameColumn = true; break; diff --git a/src/sqlitetablemodel.cpp b/src/sqlitetablemodel.cpp index f4169bb9..3edfe0f7 100644 --- a/src/sqlitetablemodel.cpp +++ b/src/sqlitetablemodel.cpp @@ -262,7 +262,7 @@ sqlb::ForeignKeyClause SqliteTableModel::getForeignKeyClause(int column) const // foreign key on that column. if (column >= 0 && column < obj.table.fields().count()) { - return obj.table.fields().at(column)->foreignKey(); + return obj.table.foreignKey(obj.table.fields().at(column)); } else { return sqlb::ForeignKeyClause(); } diff --git a/src/sqlitetypes.cpp b/src/sqlitetypes.cpp index b74b61a4..86672c7b 100644 --- a/src/sqlitetypes.cpp +++ b/src/sqlitetypes.cpp @@ -157,7 +157,7 @@ void Table::setField(int index, FieldPtr f) } // Foreign keys - QMap::iterator it = m_foreignKeyClauses.begin(); + ForeignKeyMap::iterator it = m_foreignKeyClauses.begin(); while(it != m_foreignKeyClauses.end()) { // Loop through all fields mentioned in a foreign key @@ -298,12 +298,7 @@ QString Table::sql() const } // foreign keys - foreach(FieldPtr f, m_fields) - { - if(f->foreignKey().isSet()) - sql += QString(",\n\tFOREIGN KEY(%1) REFERENCES %2").arg(escapeIdentifier(f->name())).arg(f->foreignKey().toString()); - } - QMap::const_iterator it = m_foreignKeyClauses.constBegin(); + ForeignKeyMap::const_iterator it = m_foreignKeyClauses.constBegin(); while(it != m_foreignKeyClauses.constEnd()) { sql += QString(",\n\tFOREIGN KEY(%1) REFERENCES %2").arg(fieldVectorToFieldNames(it.key()).join(",")).arg(it.value().toString()); @@ -324,11 +319,36 @@ void Table::addUniqueConstraint(FieldVector fields) m_uniqueConstraints.push_back(fields); } +void Table::addForeignKey(FieldPtr field, ForeignKeyClause fk) +{ + FieldVector v; + v.push_back(field); + addForeignKey(v, fk); +} + void Table::addForeignKey(FieldVector fields, ForeignKeyClause fk) { m_foreignKeyClauses.insert(fields, fk); } +const ForeignKeyClause& Table::foreignKey(FieldPtr field) const +{ + FieldVector v; + v.push_back(field); + return foreignKey(v); +} + +const ForeignKeyClause& Table::foreignKey(FieldVector fields) const +{ + static ForeignKeyClause empty_foreign_key; + + ForeignKeyMap::ConstIterator it = m_foreignKeyClauses.find(fields); + if(it != m_foreignKeyClauses.constEnd()) + return it.value(); + else + return empty_foreign_key; +} + namespace { QString identifier(antlr::RefAST ident) @@ -409,9 +429,7 @@ Table CreateTableWalker::table() // loop columndefs while(column != antlr::nullAST && column->getType() == sqlite3TokenTypes::COLUMNDEF) { - FieldPtr f; - parsecolumn(f, column->getFirstChild()); - tab.addField(f); + parsecolumn(tab, column->getFirstChild()); column = column->getNextSibling(); //COMMA or RPAREN column = column->getNextSibling(); //null or tableconstraint } @@ -541,11 +559,7 @@ Table CreateTableWalker::table() } fk.setConstraint(concatTextAST(tc, true)); - - if(fields.size() == 1) - fields[0]->setForeignKey(fk); - else - tab.addForeignKey(fields, fk); + tab.addForeignKey(fields, fk); } break; default: @@ -572,7 +586,7 @@ Table CreateTableWalker::table() return tab; } -void CreateTableWalker::parsecolumn(FieldPtr& f, antlr::RefAST c) +void CreateTableWalker::parsecolumn(Table& table, antlr::RefAST c) { QString colname; QString type = "TEXT"; @@ -682,9 +696,12 @@ void CreateTableWalker::parsecolumn(FieldPtr& f, antlr::RefAST c) c = c->getNextSibling(); } - f = FieldPtr( new Field(colname, type, notnull, defaultvalue, check, primarykey, unique)); + FieldPtr f = FieldPtr(new Field(colname, type, notnull, defaultvalue, check, primarykey, unique)); f->setAutoIncrement(autoincrement); - f->setForeignKey(foreignKey); + table.addField(f); + + if(foreignKey.isSet()) + table.addForeignKey(f, foreignKey); } } //namespace sqlb diff --git a/src/sqlitetypes.h b/src/sqlitetypes.h index 9be6c156..08d9163d 100644 --- a/src/sqlitetypes.h +++ b/src/sqlitetypes.h @@ -77,7 +77,6 @@ public: void setAutoIncrement(bool autoinc) { m_autoincrement = autoinc; } void setPrimaryKey(bool pk) { m_primaryKey = pk; } void setUnique(bool u) { m_unique = u; } - void setForeignKey(const ForeignKeyClause& key) { m_foreignKey = key; } bool isText() const; bool isInteger() const; @@ -90,7 +89,6 @@ public: bool autoIncrement() const { return m_autoincrement; } bool primaryKey() const { return m_primaryKey; } bool unique() const { return m_unique; } - const ForeignKeyClause& foreignKey() const { return m_foreignKey; } static QStringList Datatypes; private: @@ -99,7 +97,6 @@ private: bool m_notnull; QString m_check; QString m_defaultvalue; - ForeignKeyClause m_foreignKey; // Even though this information is a table constraint it's easier for accessing and processing to store it here bool m_autoincrement; //! this is stored here for simplification bool m_primaryKey; bool m_unique; @@ -107,6 +104,7 @@ private: typedef QSharedPointer FieldPtr; typedef QVector< FieldPtr > FieldVector; +typedef QMap ForeignKeyMap; #if QT_VERSION_MAJOR < 5 inline bool operator<(const FieldVector&, const FieldVector&) @@ -144,7 +142,12 @@ public: void clear(); void addUniqueConstraint(FieldVector fields); + + void addForeignKey(FieldPtr field, ForeignKeyClause fk); void addForeignKey(FieldVector fields, ForeignKeyClause fk); + const ForeignKeyClause& foreignKey(FieldPtr field) const; + const ForeignKeyClause& foreignKey(FieldVector fields) const; + const ForeignKeyMap& foreignKeys() const { return m_foreignKeyClauses; } /** * @brief findField Finds a field and returns the index. @@ -173,7 +176,7 @@ private: FieldVector m_fields; QString m_rowidColumn; QVector m_uniqueConstraints; - QMap m_foreignKeyClauses; + ForeignKeyMap m_foreignKeyClauses; }; /** @@ -193,13 +196,15 @@ public: bool modifysupported() const { return m_bModifySupported; } private: - void parsecolumn(FieldPtr& f, antlr::RefAST c); + void parsecolumn(Table& table, antlr::RefAST c); private: antlr::RefAST m_root; bool m_bModifySupported; }; +QStringList fieldVectorToFieldNames(const sqlb::FieldVector& vector); + } //namespace sqlb #endif // SQLITETYPES_H diff --git a/src/tests/testsqlobjects.cpp b/src/tests/testsqlobjects.cpp index a6a108db..446ae4bd 100644 --- a/src/tests/testsqlobjects.cpp +++ b/src/tests/testsqlobjects.cpp @@ -82,8 +82,8 @@ void TestTable::foreignKeys() { Table tt("testtable"); FieldPtr f = FieldPtr(new Field("a", "integer")); - f->setForeignKey(sqlb::ForeignKeyClause("b", QStringList("c"))); tt.addField(f); + tt.addForeignKey(f, sqlb::ForeignKeyClause("b", QStringList("c"))); QCOMPARE(tt.sql(), QString("CREATE TABLE `testtable` (\n" "\t`a`\tinteger,\n" @@ -271,10 +271,10 @@ void TestTable::parseSQLForeignKeys() QCOMPARE(tab.name(), QString("foreign_key_test")); QCOMPARE(tab.fields().at(0)->name(), QString("a")); QCOMPARE(tab.fields().at(0)->type(), QString("int")); - QCOMPARE(tab.fields().at(0)->foreignKey().table(), QString("x")); + QCOMPARE(tab.foreignKey(tab.fields().at(0)).table(), QString("x")); QCOMPARE(tab.fields().at(1)->name(), QString("b")); QCOMPARE(tab.fields().at(1)->type(), QString("int")); - QCOMPARE(tab.fields().at(1)->foreignKey().toString(), QString("`w`(`y`,`z`) on delete set null")); + QCOMPARE(tab.foreignKey(tab.fields().at(1)).toString(), QString("`w`(`y`,`z`) on delete set null")); } void TestTable::parseSQLCheckConstraint()