Skip to content

Commit 43107ea

Browse files
MKleusbergMKleusberg
committed
Refactor data structures for table constraints
This is a long overdue continuation of some previous refactoring effort. Before this we used to store the columns a table constraint belongs to within the constraint object itself. So for example, a foreign key constraint object would store the referencing as well as the referenced column names. While initially simple, this approach has the downside of duplicating certain data, thus breaking ownership and complicating matters later on. This becomes obvious when renaming the referencing column. The column name clearly is a feature of the table but in the previous approach it also needs to be changed in the foreign key object as well as in any other constraint for this field even though the constraint itself has not been touched. This illustrates how a constraint is not only a property of a table but the field names (a property of the table) are also a property of the constraint, creating a circular ownership. This makes the code hard to maintain. It also invalidates references to constraints in the program needlessly, e.g. when only changing a column name. With this commit the column names are removed from the constraint types. Instead they are now solely a property of the table. This, however, raised another issue. For unique constraints and primary keys it is possible to use expressions and/or sorted keys whereas for foreign keys this is not possible. Additionally check constraints have no columns at all. So when not storing the used columns inside the constraint objects we need to have different storage types for each of them. So in a second step this commit moves the code from a single data structure for storing all table constraints to three data structures, one for PK and unique, one for foreign keys, and one for check constraints. By doing all this, this commit also changes the interface for handling quite a bit. The new interface tends to use more explicit types which makes the usage code easier to read. Please note that this is still far from finished. But future development on this should be a lot easier now.
1 parent e55fed2 commit 43107ea

15 files changed

+1234
-1193
lines changed

‎src/AddRecordDialog.cpp‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,8 +184,8 @@ void AddRecordDialog::populateFields()
184184
ui->treeWidget->setItemDelegateForColumn(kValue, new EditDelegate(this));
185185

186186
sqlb::FieldVector fields;
187-
std::vector<sqlb::ConstraintPtr> fks;
188-
sqlb::StringVector pk;
187+
std::vector<std::shared_ptr<sqlb::ForeignKeyClause>> fks;
188+
sqlb::IndexedColumnVector pk;
189189
bool auto_increment = false;
190190

191191
// Initialize fields, fks and pk differently depending on whether it's a table or a view.
@@ -194,19 +194,19 @@ void AddRecordDialog::populateFields()
194194
if (!table->isView())
195195
{
196196
std::transform(fields.begin(), fields.end(), std::back_inserter(fks), [table](const auto& f) {
197-
return table->constraint({f.name()}, sqlb::Constraint::ForeignKeyConstraintType);
197+
return table->foreignKey({f.name()});
198198
});
199199

200200
const auto pk_constraint = table->primaryKey();
201201
if(pk_constraint)
202202
{
203-
pk = pk_constraint->columnList();
203+
pk = table->primaryKeyColumns();
204204
auto_increment = pk_constraint->autoIncrement();
205205
}
206206
} else {
207207
// It's a view
208-
fks.resize(fields.size(), sqlb::ConstraintPtr(nullptr));
209-
pk = pseudo_pk;
208+
fks.resize(fields.size(), nullptr);
209+
std::transform(pseudo_pk.begin(), pseudo_pk.end(), std::back_inserter(pk), [](const auto& e) { return sqlb::IndexedColumn(e, false); });
210210
}
211211

212212
for(uint i = 0; i < fields.size(); i++)
@@ -244,7 +244,7 @@ void AddRecordDialog::populateFields()
244244
if (!f.check().empty())
245245
toolTip.append(tr("Check constraint:\t %1\n").arg(QString::fromStdString(f.check())));
246246

247-
auto fk = std::dynamic_pointer_cast<sqlb::ForeignKeyClause>(fks[i]);
247+
auto fk = fks[i];
248248
if(fk)
249249
toolTip.append(tr("Foreign key:\t %1\n").arg(QString::fromStdString(fk->toString())));
250250

‎src/DbStructureModel.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -334,18 +334,18 @@ void DbStructureModel::buildTree(QTreeWidgetItem* parent, const std::string& sch
334334
// Add an extra node for the browsable section
335335
addNode(schema, obj.second->name(), obj.second->isView() ? "view" : "table", obj.second->originalSql(), browsablesRootItem);
336336

337-
sqlb::StringVector pk_columns;
337+
sqlb::IndexedColumnVector pk_columns;
338338
if(!obj.second->isView())
339339
{
340340
const auto pk = obj.second->primaryKey();
341341
if(pk)
342-
pk_columns = pk->columnList();
342+
pk_columns = obj.second->primaryKeyColumns();
343343
}
344344

345345
for(const auto& field : obj.second->fields)
346346
{
347347
bool isPK = contains(pk_columns, field.name());
348-
bool isFK = obj.second->constraint({field.name()}, sqlb::Constraint::ForeignKeyConstraintType) != nullptr;
348+
bool isFK = obj.second->foreignKey({field.name()}) != nullptr;
349349

350350
addNode(schema, field.name(), "field", field.toString(" ", " "), item, field.type(), isPK ? "_key" : (isFK ? "_fk" : std::string{}));
351351
}

‎src/EditTableDialog.cpp‎

Lines changed: 89 additions & 53 deletions
Large diffs are not rendered by default.

‎src/ExtendedTableWidget.cpp‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -131,19 +131,18 @@ QWidget* ExtendedTableWidgetEditorDelegate::createEditor(QWidget* parent, const
131131
emit dataAboutToBeEdited(index);
132132

133133
SqliteTableModel* m = qobject_cast<SqliteTableModel*>(const_cast<QAbstractItemModel*>(index.model()));
134-
sqlb::ForeignKeyClause fk = m->getForeignKeyClause(static_cast<size_t>(index.column()-1));
134+
auto fk = m->getForeignKeyClause(static_cast<size_t>(index.column()-1));
135135

136-
if(fk.isSet()) {
137-
138-
sqlb::ObjectIdentifier foreignTable = sqlb::ObjectIdentifier(m->currentTableName().schema(), fk.table());
136+
if(fk) {
137+
sqlb::ObjectIdentifier foreignTable = sqlb::ObjectIdentifier(m->currentTableName().schema(), fk->table());
139138

140139
std::string column;
141140
// If no column name is set, assume the primary key is meant
142-
if(fk.columns().empty()) {
141+
if(fk->columns().empty()) {
143142
sqlb::TablePtr obj = m->db().getTableByName(foreignTable);
144-
column = obj->primaryKey()->columnList().front();
143+
column = obj->primaryKeyColumns().front().name();
145144
} else
146-
column = fk.columns().at(0);
145+
column = fk->columns().at(0);
147146

148147
sqlb::TablePtr currentTable = m->db().getTableByName(m->currentTableName());
149148
QString query = QString("SELECT %1 FROM %2").arg(QString::fromStdString(sqlb::escapeIdentifier(column)), QString::fromStdString(foreignTable.toString()));
@@ -1057,11 +1056,11 @@ void ExtendedTableWidget::cellClicked(const QModelIndex& index)
10571056
if(qApp->keyboardModifiers().testFlag(Qt::ControlModifier) && qApp->keyboardModifiers().testFlag(Qt::ShiftModifier) && model())
10581057
{
10591058
SqliteTableModel* m = qobject_cast<SqliteTableModel*>(model());
1060-
sqlb::ForeignKeyClause fk = m->getForeignKeyClause(static_cast<size_t>(index.column()-1));
1059+
auto fk = m->getForeignKeyClause(static_cast<size_t>(index.column()-1));
10611060

1062-
if(fk.isSet())
1063-
emit foreignKeyClicked(sqlb::ObjectIdentifier(m->currentTableName().schema(), fk.table()),
1064-
fk.columns().size() ? fk.columns().at(0) : "",
1061+
if(fk)
1062+
emit foreignKeyClicked(sqlb::ObjectIdentifier(m->currentTableName().schema(), fk->table()),
1063+
fk->columns().size() ? fk->columns().at(0) : "",
10651064
m->data(index, Qt::EditRole).toByteArray());
10661065
else {
10671066
// If this column does not have a foreign-key, try to interpret it as a filename/URL and open it in external application.

‎src/ForeignKeyEditorDelegate.cpp‎

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@ class ForeignKeyEditor : public QWidget
7171
QComboBox* tablesComboBox;
7272
QComboBox* idsComboBox;
7373
QLineEdit* clauseEdit; // for ON CASCADE and such
74+
std::shared_ptr<sqlb::ForeignKeyClause> foreignKey; // This can be used for storing a pointer to the foreign key object currently edited
7475

7576
private:
7677
QPushButton* m_btnReset;
@@ -138,12 +139,12 @@ void ForeignKeyEditorDelegate::setEditorData(QWidget* editor, const QModelIndex&
138139

139140
size_t column = static_cast<size_t>(index.row()); // weird? I know right
140141
const sqlb::Field& field = m_table.fields.at(column);
141-
auto fk = std::dynamic_pointer_cast<sqlb::ForeignKeyClause>(m_table.constraint({field.name()}, sqlb::Constraint::ForeignKeyConstraintType));
142-
if (fk) {
143-
fkEditor->tablesComboBox->setCurrentText(QString::fromStdString(fk->table()));
144-
fkEditor->clauseEdit->setText(QString::fromStdString(fk->constraint()));
145-
if (!fk->columns().empty())
146-
fkEditor->idsComboBox->setCurrentText(QString::fromStdString(fk->columns().front()));
142+
fkEditor->foreignKey = m_table.foreignKey({field.name()});
143+
if (fkEditor->foreignKey) {
144+
fkEditor->tablesComboBox->setCurrentText(QString::fromStdString(fkEditor->foreignKey->table()));
145+
fkEditor->clauseEdit->setText(QString::fromStdString(fkEditor->foreignKey->constraint()));
146+
if (!fkEditor->foreignKey->columns().empty())
147+
fkEditor->idsComboBox->setCurrentText(QString::fromStdString(fkEditor->foreignKey->columns().front()));
147148
} else {
148149
fkEditor->tablesComboBox->setCurrentIndex(-1);
149150
}
@@ -156,28 +157,27 @@ void ForeignKeyEditorDelegate::setModelData(QWidget* editor, QAbstractItemModel*
156157

157158
size_t column = static_cast<size_t>(index.row());
158159
const sqlb::Field& field = m_table.fields.at(column);
159-
if (sql.isEmpty()) {
160+
if (sql.isEmpty() && fkEditor->foreignKey) {
160161
// Remove the foreign key
161-
m_table.removeConstraints({field.name()}, sqlb::Constraint::ConstraintTypes::ForeignKeyConstraintType);
162+
m_table.removeConstraint(fkEditor->foreignKey);
162163
} else {
163-
// Set the foreign key
164-
sqlb::ForeignKeyClause* fk = new sqlb::ForeignKeyClause;
165-
166-
const QString table = fkEditor->tablesComboBox->currentText();
167-
const std::string id = fkEditor->idsComboBox->currentText().toStdString();
168-
const QString clause = fkEditor->clauseEdit->text();
169-
170-
fk->setTable(table.toStdString());
171-
fk->setColumnList({ field.name() });
172-
173-
if (!id.empty())
174-
fk->setColumns({id});
175-
176-
if (!clause.trimmed().isEmpty()) {
177-
fk->setConstraint(clause.toStdString());
164+
// Create a new foreign key object if required
165+
if(!fkEditor->foreignKey)
166+
{
167+
fkEditor->foreignKey = std::make_shared<sqlb::ForeignKeyClause>();
168+
m_table.addConstraint({field.name()}, fkEditor->foreignKey);
178169
}
179170

180-
m_table.setConstraint(sqlb::ConstraintPtr(fk));
171+
// Set data
172+
const auto table = fkEditor->tablesComboBox->currentText().toStdString();
173+
const auto id = fkEditor->idsComboBox->currentText().toStdString();
174+
const auto clause = fkEditor->clauseEdit->text().trimmed().toStdString();
175+
fkEditor->foreignKey->setTable(table);
176+
if(id.empty())
177+
fkEditor->foreignKey->setColumns({});
178+
else
179+
fkEditor->foreignKey->setColumns({id});
180+
fkEditor->foreignKey->setConstraint(clause);
181181
}
182182

183183
model->setData(index, sql);

‎src/TableBrowser.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1502,7 +1502,7 @@ void TableBrowser::jumpToRow(const sqlb::ObjectIdentifier& table, std::string co
15021502

15031503
// If no column name is set, assume the primary key is meant
15041504
if(!column.size())
1505-
column = obj->primaryKey()->columnList().front();
1505+
column = obj->primaryKeyColumns().front().name();
15061506

15071507
// If column doesn't exist don't do anything
15081508
auto column_it = sqlb::findField(obj, column);

0 commit comments

Comments
 (0)