From 3237e9d2f5a6f7ca29942a08f85aee0f2c0eb1e0 Mon Sep 17 00:00:00 2001 From: Martin Kleusberg Date: Fri, 6 Sep 2019 13:44:47 +0200 Subject: [PATCH] grammar: Rework how PK and unique constraints work This commit changes the class hierarchy to make primary key constraints a type of unique constraints. This fits nicely with reality because primary key columns do not allow duplicate values. It also makes our life easier as the other changes which are introduced here add some code required by both unique and primary key constraints and which now can be shared. Move the auto increment flag from the field class to the primary key class. This changes how auto increment fields work and look and might be a bit unfamiliar but it simplifies things a lot for us because an auto increment field is always a primary key. So before we had to maintain two places: the field with the auto increment flag and the primary key which belongs to it. Now it is all in one place in the primary key. Add support for storing and manipulating sort order for columns in primary key and unique constraints. It does not add support for them to the grammar parser though. Finally add a way to store and manipulate on conflict clauses for unique and primary key constraints. Again, parser support for them is not added. --- src/AddRecordDialog.cpp | 11 +- src/DbStructureModel.cpp | 7 +- src/EditTableDialog.cpp | 67 ++++++------ src/ExtendedTableWidget.cpp | 2 +- src/ForeignKeyEditorDelegate.cpp | 2 +- src/TableBrowser.cpp | 2 +- src/sql/sqlitetypes.cpp | 177 +++++++++++++++++++++---------- src/sql/sqlitetypes.h | 56 ++++++---- src/sqlitetablemodel.cpp | 7 +- src/tests/testsqlobjects.cpp | 52 ++++----- 10 files changed, 237 insertions(+), 146 deletions(-) diff --git a/src/AddRecordDialog.cpp b/src/AddRecordDialog.cpp index f659878a..2a6bcf9e 100644 --- a/src/AddRecordDialog.cpp +++ b/src/AddRecordDialog.cpp @@ -180,6 +180,7 @@ void AddRecordDialog::populateFields() sqlb::FieldVector fields; std::vector fks; sqlb::StringVector pk; + bool auto_increment = false; // Initialize fields, fks and pk differently depending on whether it's a table or a view. const sqlb::ObjectPtr obj = pdb.getObjectByName(curTable); @@ -189,7 +190,13 @@ void AddRecordDialog::populateFields() fields = m_table->fields; for(const sqlb::Field& f : fields) fks.push_back(m_table->constraint({f.name()}, sqlb::Constraint::ForeignKeyConstraintType)); - pk = m_table->primaryKey(); + + const auto pk_constraint = m_table->primaryKey(); + if(pk_constraint) + { + pk = pk_constraint->columnList(); + auto_increment = pk_constraint->autoIncrement(); + } } else { sqlb::ViewPtr m_view = pdb.getObjectByName(curTable); fields = m_view->fields; @@ -224,7 +231,7 @@ void AddRecordDialog::populateFields() QString defaultValue = QString::fromStdString(f.defaultValue()); QString toolTip; - if (f.autoIncrement()) + if (auto_increment && contains(pk, f.name())) toolTip.append(tr("Auto-increment\n")); if (f.unique()) diff --git a/src/DbStructureModel.cpp b/src/DbStructureModel.cpp index 32e31dab..2dca5835 100644 --- a/src/DbStructureModel.cpp +++ b/src/DbStructureModel.cpp @@ -364,7 +364,12 @@ void DbStructureModel::buildTree(QTreeWidgetItem* parent, const std::string& sch { sqlb::StringVector pk_columns; if(it->type() == sqlb::Object::Types::Table) - pk_columns = std::dynamic_pointer_cast(it)->primaryKey(); + { + const auto pk = std::dynamic_pointer_cast(it)->primaryKey(); + if(pk) + pk_columns = pk->columnList(); + } + for(const sqlb::FieldInfo& field : fieldList) { QTreeWidgetItem *fldItem = new QTreeWidgetItem(item); diff --git a/src/EditTableDialog.cpp b/src/EditTableDialog.cpp index 22985ae0..d4ac98bf 100644 --- a/src/EditTableDialog.cpp +++ b/src/EditTableDialog.cpp @@ -114,7 +114,7 @@ EditTableDialog::EditTableDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier& return; // Show the select items popup dialog - SelectItemsPopup* dialog = new SelectItemsPopup(m_table.fieldNames(), item->data(Qt::UserRole).value()->column_list, this); + SelectItemsPopup* dialog = new SelectItemsPopup(m_table.fieldNames(), item->data(Qt::UserRole).value()->columnList(), this); QRect item_rect = ui->tableConstraints->visualItemRect(item); dialog->move(ui->tableConstraints->mapToGlobal(QPoint(ui->tableConstraints->x() + item_rect.x(), ui->tableConstraints->y() + item_rect.y() + item_rect.height() / 2))); @@ -124,11 +124,11 @@ EditTableDialog::EditTableDialog(DBBrowserDB& db, const sqlb::ObjectIdentifier& connect(dialog, &SelectItemsPopup::accepted, [this, dialog, constraint]() { // Check if column selection changed at all sqlb::StringVector new_columns = dialog->selectedItems(); - if(constraint->column_list != new_columns) + if(constraint->columnList() != new_columns) { // Remove the constraint with the old columns and add a new one with the new columns m_table.removeConstraint(constraint); - constraint->column_list = new_columns; + constraint->setColumnList(new_columns); m_table.addConstraint(constraint); // Update the UI @@ -185,7 +185,7 @@ void EditTableDialog::populateFields() ui->treeWidget->clear(); const auto& fields = m_table.fields; - sqlb::StringVector pk = m_table.primaryKey(); + const auto pk = m_table.primaryKey(); for(const sqlb::Field& f : fields) { QTreeWidgetItem *tbitem = new QTreeWidgetItem(ui->treeWidget); @@ -208,8 +208,8 @@ void EditTableDialog::populateFields() ui->treeWidget->setItemWidget(tbitem, kType, typeBox); tbitem->setCheckState(kNotNull, f.notnull() ? Qt::Checked : Qt::Unchecked); - tbitem->setCheckState(kPrimaryKey, contains(pk, f.name()) ? Qt::Checked : Qt::Unchecked); - tbitem->setCheckState(kAutoIncrement, f.autoIncrement() ? Qt::Checked : Qt::Unchecked); + tbitem->setCheckState(kPrimaryKey, pk && contains(pk->columnList(), f.name()) ? Qt::Checked : Qt::Unchecked); + tbitem->setCheckState(kAutoIncrement, pk && pk->autoIncrement() && contains(pk->columnList(), f.name()) ? Qt::Checked : Qt::Unchecked); tbitem->setCheckState(kUnique, f.unique() ? Qt::Checked : Qt::Unchecked); // For the default value check if it is surrounded by parentheses and if that's the case @@ -258,7 +258,7 @@ void EditTableDialog::populateConstraints() int row = 0; for(const auto& constraint : constraints) { - const auto& columns = constraint->column_list; + const auto columns = constraint->columnList(); // Columns QTableWidgetItem* column = new QTableWidgetItem(QString::fromStdString(sqlb::joinStringVector(columns, ","))); @@ -278,7 +278,7 @@ void EditTableDialog::populateConstraints() // Only the column list and the name can be migrated to the new constraint. // Make sure there is only one primary key at a time - if(index == 0 && !m_table.primaryKey().empty()) + if(index == 0 && m_table.primaryKey()) { QMessageBox::warning(this, qApp->applicationName(), tr("There can only be one primary key for each table. Please modify the existing primary " "key instead.")); @@ -293,7 +293,7 @@ void EditTableDialog::populateConstraints() // Create new constraint depending on selected type sqlb::ConstraintPtr new_constraint = sqlb::Constraint::makeConstraint(static_cast(index)); new_constraint->setName(constraint->name()); - new_constraint->column_list = constraint->column_list; + new_constraint->setColumnList(constraint->columnList()); // Replace old by new constraint m_table.replaceConstraint(constraint, new_constraint); @@ -473,20 +473,20 @@ void EditTableDialog::fieldItemChanged(QTreeWidgetItem *item, int column) // When editing an exiting table, check if any foreign keys would cause trouble in case this name is edited if(!m_bNewTable) { - sqlb::StringVector pk = m_table.primaryKey(); + const auto pk = m_table.primaryKey(); const auto tables = pdb.schemata[curTable.schema()].equal_range("table"); for(auto it=tables.first;it!=tables.second;++it) { const sqlb::ObjectPtr& fkobj = it->second; - auto fks = std::dynamic_pointer_cast(fkobj)->constraints(sqlb::StringVector(), sqlb::Constraint::ForeignKeyConstraintType); + auto fks = std::dynamic_pointer_cast(fkobj)->constraints({}, sqlb::Constraint::ForeignKeyConstraintType); for(const sqlb::ConstraintPtr& fkptr : fks) { auto fk = std::dynamic_pointer_cast(fkptr); if(fk->table() == m_table.name()) { - if(contains(fk->columns(), field.name()) || contains(pk, field.name())) + if(contains(fk->columns(), field.name()) || (pk && contains(pk->columnList(), field.name()))) { QMessageBox::warning(this, qApp->applicationName(), tr("This column is referenced in a foreign key in table %1 and thus " "its name cannot be changed.") @@ -526,18 +526,18 @@ void EditTableDialog::fieldItemChanged(QTreeWidgetItem *item, int column) case kPrimaryKey: { // Check if there already is a primary key - if(m_table.constraint(sqlb::StringVector(), sqlb::Constraint::PrimaryKeyConstraintType)) + auto pk = m_table.primaryKey(); + if(pk) { // There already is a primary key for this table. So edit that one as there always can only be one primary key anyway. - sqlb::StringVector& pk = m_table.primaryKeyRef(); if(item->checkState(column) == Qt::Checked) { - pk.push_back(field.name()); + pk->addToColumnList(field.name()); } else { - pk.erase(std::remove(pk.begin(), pk.end(), field.name()), pk.end()); + pk->removeFromColumnList(field.name()); // If this is now a primary key constraint without any columns, remove it entirely - if(pk.empty()) + if(pk->columnList().empty()) m_table.removeConstraints({}, sqlb::Constraint::PrimaryKeyConstraintType); } } else if(item->checkState(column) == Qt::Checked) { @@ -639,7 +639,8 @@ void EditTableDialog::fieldItemChanged(QTreeWidgetItem *item, int column) } } } - field.setAutoIncrement(ischecked); + if(m_table.primaryKey()) + m_table.primaryKey()->setAutoIncrement(ischecked); } break; case kUnique: @@ -901,24 +902,20 @@ void EditTableDialog::setWithoutRowid(bool without_rowid) if(without_rowid) { // Before setting the without rowid flag, first perform a check to see if the table meets all the required criteria for without rowid tables - auto pks = m_table.primaryKey(); - for(const auto& pk_name : pks) + const auto pk = m_table.primaryKey(); + if(!pk || pk->autoIncrement()) { - auto pk = sqlb::findField(m_table, pk_name); - if(pk == m_table.fields.end() || pk->autoIncrement()) - { - QMessageBox::information(this, QApplication::applicationName(), - tr("Please add a field which meets the following criteria before setting the without rowid flag:\n" - " - Primary key flag set\n" - " - Auto increment disabled")); + QMessageBox::information(this, QApplication::applicationName(), + tr("Please add a field which meets the following criteria before setting the without rowid flag:\n" + " - Primary key flag set\n" + " - Auto increment disabled")); - // Reset checkbox state to unchecked. Block any signals while doing this in order to avoid an extra call to - // this function being triggered. - ui->checkWithoutRowid->blockSignals(true); - ui->checkWithoutRowid->setChecked(false); - ui->checkWithoutRowid->blockSignals(false); - return; - } + // Reset checkbox state to unchecked. Block any signals while doing this in order to avoid an extra call to + // this function being triggered. + ui->checkWithoutRowid->blockSignals(true); + ui->checkWithoutRowid->setChecked(false); + ui->checkWithoutRowid->blockSignals(false); + return; } // If it does, set the without rowid flag of the table @@ -962,7 +959,7 @@ void EditTableDialog::addConstraint(sqlb::Constraint::ConstraintTypes type) // There can only be one primary key if(type == sqlb::Constraint::PrimaryKeyConstraintType) { - if(!m_table.primaryKey().empty()) + if(m_table.primaryKey()) { QMessageBox::information(this, qApp->applicationName(), tr("There can only be one primary key for each table. Please modify the existing primary " "key instead.")); diff --git a/src/ExtendedTableWidget.cpp b/src/ExtendedTableWidget.cpp index dcdf5955..bb6856d8 100644 --- a/src/ExtendedTableWidget.cpp +++ b/src/ExtendedTableWidget.cpp @@ -137,7 +137,7 @@ QWidget* ExtendedTableWidgetEditorDelegate::createEditor(QWidget* parent, const // If no column name is set, assume the primary key is meant if(fk.columns().empty()) { sqlb::TablePtr obj = m->db().getObjectByName(foreignTable); - column = obj->primaryKey().front(); + column = obj->primaryKey()->columnList().front(); } else column = fk.columns().at(0); diff --git a/src/ForeignKeyEditorDelegate.cpp b/src/ForeignKeyEditorDelegate.cpp index a5e750a7..971dcd4a 100644 --- a/src/ForeignKeyEditorDelegate.cpp +++ b/src/ForeignKeyEditorDelegate.cpp @@ -153,7 +153,7 @@ void ForeignKeyEditorDelegate::setModelData(QWidget* editor, QAbstractItemModel* const QString clause = fkEditor->clauseEdit->text(); fk->setTable(table.toStdString()); - fk->column_list = { field.name() }; + fk->setColumnList({ field.name() }); if (!id.empty()) fk->setColumns({id}); diff --git a/src/TableBrowser.cpp b/src/TableBrowser.cpp index f41ca16c..08eb0677 100644 --- a/src/TableBrowser.cpp +++ b/src/TableBrowser.cpp @@ -1098,7 +1098,7 @@ void TableBrowser::jumpToRow(const sqlb::ObjectIdentifier& table, QString column // If no column name is set, assume the primary key is meant if(!column.size()) - column = QString::fromStdString(obj->primaryKey().front()); + column = QString::fromStdString(obj->primaryKey()->columnList().front()); // If column doesn't exist don't do anything auto column_index = sqlb::findField(obj, column.toStdString()); diff --git a/src/sql/sqlitetypes.cpp b/src/sql/sqlitetypes.cpp index 624a84f8..f41d7908 100644 --- a/src/sql/sqlitetypes.cpp +++ b/src/sql/sqlitetypes.cpp @@ -149,6 +149,16 @@ ConstraintPtr Constraint::makeConstraint(ConstraintTypes type) } } +void Constraint::replaceInColumnList(const std::string& from, const std::string& to) +{ + std::replace(column_list.begin(), column_list.end(), from, to); +} + +void Constraint::removeFromColumnList(const std::string& key) +{ + column_list.erase(std::remove(column_list.begin(), column_list.end(), key), column_list.end()); +} + bool ForeignKeyClause::isSet() const { return m_override.size() || m_table.size(); @@ -188,22 +198,100 @@ std::string ForeignKeyClause::toSql() const return result; } +UniqueConstraint::UniqueConstraint(const IndexedColumnVector& columns) : + m_columns(columns) +{ + // Extract column names and give them to the column list in the base class + for(const auto& c : columns) + column_list.push_back(c.name()); +} + +UniqueConstraint::UniqueConstraint(const StringVector& columns) : + Constraint(columns) +{ + setColumnList(columns); +} + +void UniqueConstraint::setColumnList(const StringVector& list) +{ + Constraint::setColumnList(list); + + // Create our own column list without sort orders etc + m_columns.clear(); + for(const auto& c : list) + m_columns.push_back(IndexedColumn(c, false)); +} + +void UniqueConstraint::addToColumnList(const std::string& key) +{ + Constraint::addToColumnList(key); + + // Also add to our own column list + m_columns.push_back(IndexedColumn(key, false)); +} + +void UniqueConstraint::replaceInColumnList(const std::string& from, const std::string& to) +{ + Constraint::replaceInColumnList(from, to); + + for(auto& c : m_columns) + { + if(c.name() == from) + c.setName(to); + } +} + +void UniqueConstraint::removeFromColumnList(const std::string& key) +{ + Constraint::removeFromColumnList(key); + + m_columns.erase(std::remove_if(m_columns.begin(), m_columns.end(), [key](const IndexedColumn& c) { + if(c.name() == key) + return true; + else + return false; + }), m_columns.end()); +} + std::string UniqueConstraint::toSql() const { std::string result; if(!m_name.empty()) result = "CONSTRAINT " + escapeIdentifier(m_name) + " "; - result += "UNIQUE(" + joinStringVector(escapeIdentifier(column_list), ",") + ")"; + + std::vector u_columns; + for(const auto& c : m_columns) + u_columns.push_back(c.toString("", " ")); + result += "UNIQUE(" + joinStringVector(u_columns, ",") + ")"; + + if(!m_conflictAction.empty()) + result += " ON CONFLICT " + m_conflictAction; return result; } +PrimaryKeyConstraint::PrimaryKeyConstraint(const IndexedColumnVector& columns) : + UniqueConstraint(columns), + m_auto_increment(false) +{ +} + +PrimaryKeyConstraint::PrimaryKeyConstraint(const StringVector& columns) : + UniqueConstraint(columns), + m_auto_increment(false) +{ +} + std::string PrimaryKeyConstraint::toSql() const { std::string result; if(!m_name.empty()) result = "CONSTRAINT " + escapeIdentifier(m_name) + " "; - result += "PRIMARY KEY(" + joinStringVector(escapeIdentifier(column_list), ",") + ")"; + + std::vector pk_columns; + for(const auto& c : m_columns) + pk_columns.push_back(c.toString("", " ")); + result += "PRIMARY KEY(" + joinStringVector(pk_columns, ",") + (m_auto_increment ? " AUTOINCREMENT" : "") + ")"; if(!m_conflictAction.empty()) result += " ON CONFLICT " + m_conflictAction; @@ -233,8 +321,6 @@ bool Field::operator==(const Field& rhs) const return false; if(m_defaultvalue != rhs.m_defaultvalue) return false; - if(m_autoincrement != rhs.m_autoincrement) - return false; if(m_unique != rhs.m_unique) return false; if(m_collation != rhs.m_collation) @@ -252,8 +338,6 @@ std::string Field::toString(const std::string& indent, const std::string& sep) c str += " DEFAULT " + m_defaultvalue; if(!m_check.empty()) str += " CHECK(" + m_check + ")"; - if(m_autoincrement) - str += " PRIMARY KEY AUTOINCREMENT"; if(m_unique) str += " UNIQUE"; if(!m_collation.empty()) @@ -395,7 +479,7 @@ StringVector Table::rowidColumns() const { // For WITHOUT ROWID tables this function returns the names of the primary key column. For ordinary tables with a rowid column, it returns "_rowid_" if(m_withoutRowid) - return primaryKey(); + return const_cast(this)->primaryKey()->columnList(); else return {"_rowid_"}; } @@ -408,11 +492,6 @@ FieldInfoList Table::fieldInformation() const return result; } -bool Table::hasAutoIncrement() const -{ - return std::any_of(fields.begin(), fields.end(), [](const Field& f) {return f.autoIncrement(); }); -} - TablePtr Table::parseSQL(const std::string& sSQL) { SetLocaleToC locale; @@ -464,18 +543,13 @@ std::string Table::sql(const std::string& schema, bool ifNotExists) const sql += joinStringVector(fieldList(), ",\n"); // Constraints - bool autoincrement = hasAutoIncrement(); for(const auto& it : m_constraints) { - // Ignore auto increment primary key constraint - if((!autoincrement || it->type() != Constraint::PrimaryKeyConstraintType)) + // Ignore all constraints without any fields, except for check constraints which don't rely on a field vector + if(!it->columnList().empty() || it->type() == Constraint::CheckConstraintType) { - // Ignore all constraints without any fields, except for check constraints which don't rely on a field vector - if(!it->column_list.empty() || it->type() == Constraint::CheckConstraintType) - { - sql += ",\n\t"; - sql += it->toSql(); - } + sql += ",\n\t"; + sql += it->toSql(); } } @@ -496,7 +570,7 @@ void Table::addConstraint(ConstraintPtr constraint) void Table::setConstraint(ConstraintPtr constraint) { // Delete any old constraints of this type for these fields - removeConstraints(constraint->column_list, constraint->type()); + removeConstraints(constraint->columnList(), constraint->type()); // Add the new constraint to the table, effectively overwriting all old constraints for that fields/type combination addConstraint(constraint); @@ -520,7 +594,7 @@ void Table::removeConstraints(const StringVector& vStrFields, Constraint::Constr { for(auto it = m_constraints.begin();it!=m_constraints.end();) { - if((*it)->column_list == vStrFields && (*it)->type() == type) + if((*it)->columnList() == vStrFields && (*it)->type() == type) m_constraints.erase(it++); else ++it; @@ -541,7 +615,7 @@ std::vector Table::constraints(const StringVector& vStrFields, Co std::vector clist; for(const auto& it : m_constraints) { - if((type == Constraint::NoType || it->type() == type) && (vStrFields.empty() || it->column_list == vStrFields)) + if((type == Constraint::NoType || it->type() == type) && (vStrFields.empty() || it->columnList() == vStrFields)) clist.push_back(it); } return clist; @@ -562,21 +636,13 @@ void Table::replaceConstraint(ConstraintPtr from, ConstraintPtr to) m_constraints.insert(to); // Insert new constraint } -StringVector& Table::primaryKeyRef() +std::shared_ptr Table::primaryKey() { - return const_cast(static_cast(this)->primaryKey()); -} - -const StringVector& Table::primaryKey() const -{ - for(const auto& it : m_constraints) - { - if(it->type() == Constraint::PrimaryKeyConstraintType) - return it->column_list; - } - - static StringVector emptyFieldVector; - return emptyFieldVector; + const auto c = constraint({}, Constraint::PrimaryKeyConstraintType); + if(c) + return std::dynamic_pointer_cast(c); + else + return nullptr; } void Table::removeKeyFromAllConstraints(const std::string& key) @@ -585,13 +651,13 @@ void Table::removeKeyFromAllConstraints(const std::string& key) for(auto it=m_constraints.begin();it!=m_constraints.end();) { // Check if they contain the old key name - if(contains((*it)->column_list, key)) + if(contains((*it)->columnList(), key)) { // If so, remove it from the column list - (*it)->column_list.erase(std::remove((*it)->column_list.begin(), (*it)->column_list.end(), key), (*it)->column_list.end()); + (*it)->removeFromColumnList(key); // If the column list is empty now, remove the entire constraint. Otherwise save the updated column list - if((*it)->column_list.empty()) + if((*it)->columnList().empty()) it = m_constraints.erase(it); else ++it; @@ -610,8 +676,8 @@ void Table::renameKeyInAllConstraints(const std::string& key, const std::string& // Find all occurrences of the key and change it to the new one for(auto& it : m_constraints) { - if(contains(it->column_list, key)) - std::replace(it->column_list.begin(), it->column_list.end(), key, to); + if(contains(it->columnList(), key)) + it->replaceInColumnList(key, to); } } @@ -829,7 +895,7 @@ TablePtr CreateTableWalker::table() antlr::RefAST indexed_column = tc->getFirstChild(); std::string col = columnname(indexed_column); - pk->column_list.push_back(col); + pk->addToColumnList(col); indexed_column = indexed_column->getNextSibling(); if(indexed_column != antlr::nullAST @@ -851,8 +917,7 @@ TablePtr CreateTableWalker::table() if(indexed_column != antlr::nullAST && indexed_column->getType() == sqlite3TokenTypes::AUTOINCREMENT) { - auto field = findField(tab, col); - field->setAutoIncrement(true); + pk->setAutoIncrement(true); indexed_column = indexed_column->getNextSibling(); } @@ -884,7 +949,7 @@ TablePtr CreateTableWalker::table() std::string col = columnname(indexed_column); auto field = findField(tab, col); - unique->column_list.push_back(field->name()); + unique->addToColumnList(field->name()); indexed_column = indexed_column->getNextSibling(); if(indexed_column != antlr::nullAST @@ -912,9 +977,9 @@ TablePtr CreateTableWalker::table() } } while(tc != antlr::nullAST && tc->getType() != sqlite3TokenTypes::RPAREN); - if(unique->column_list.size() == 1 && constraint_name.empty()) + if(unique->columnList().size() == 1 && constraint_name.empty()) { - findField(tab, unique->column_list[0])->setUnique(true); + findField(tab, unique->columnList()[0])->setUnique(true); delete unique; } else { tab->addConstraint(ConstraintPtr(unique)); @@ -933,7 +998,7 @@ TablePtr CreateTableWalker::table() do { std::string col = columnname(tc); - fk->column_list.push_back(findField(tab, col)->name()); + fk->addToColumnList(findField(tab, col)->name()); tc = tc->getNextSibling(); @@ -1008,7 +1073,6 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c) { std::string colname; std::string type = "TEXT"; - bool autoincrement = false; bool notnull = false; bool unique = false; std::string defaultvalue; @@ -1071,7 +1135,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c) delete primaryKey; primaryKey = new PrimaryKeyConstraint; - primaryKey->column_list = { colname }; + primaryKey->setColumnList({ colname }); primaryKey->setName(constraint_name); con = con->getNextSibling()->getNextSibling(); // skip KEY @@ -1085,7 +1149,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c) primaryKey->setConflictAction(parseConflictClause(con)); if(con != antlr::nullAST && con->getType() == sqlite3TokenTypes::AUTOINCREMENT) - autoincrement = true; + primaryKey->setAutoIncrement(true); } break; case sqlite3TokenTypes::NOT: @@ -1114,7 +1178,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c) { CheckConstraint* check_constraint = new CheckConstraint(check); check_constraint->setName(constraint_name); - check_constraint->column_list = { colname }; + check_constraint->setColumnList({ colname }); table->addConstraint(ConstraintPtr(check_constraint)); check.clear(); } @@ -1144,7 +1208,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c) con = con->getNextSibling(); // REFERENCES sqlb::ForeignKeyClause* foreignKey = new ForeignKeyClause; - foreignKey->column_list = { colname }; + foreignKey->setColumnList({ colname }); foreignKey->setTable(identifier(con)); foreignKey->setName(constraint_name); con = con->getNextSibling(); // identifier @@ -1187,7 +1251,6 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c) } Field f(colname, type, notnull, defaultvalue, check, unique, collation); - f.setAutoIncrement(autoincrement); table->fields.push_back(f); for(sqlb::ForeignKeyClause* fk : foreignKeys) @@ -1197,7 +1260,7 @@ void CreateTableWalker::parsecolumn(Table* table, antlr::RefAST c) StringVector v; if(table->constraint(v, Constraint::PrimaryKeyConstraintType)) { - table->primaryKeyRef().push_back(f.name()); + table->primaryKey()->addToColumnList(f.name()); // Delete useless primary key constraint. There already is a primary key object for this table, we // don't need another one. diff --git a/src/sql/sqlitetypes.h b/src/sql/sqlitetypes.h index 2c362af0..cf4d4d71 100644 --- a/src/sql/sqlitetypes.h +++ b/src/sql/sqlitetypes.h @@ -141,11 +141,16 @@ public: void setName(const std::string& name) { m_name = name; } const std::string& name() const { return m_name; } + StringVector columnList() const { return column_list; } + virtual void setColumnList(const StringVector& list) { column_list = list; } + virtual void addToColumnList(const std::string& key) { column_list.push_back(key); } + virtual void replaceInColumnList(const std::string& from, const std::string& to); + virtual void removeFromColumnList(const std::string& key); + virtual std::string toSql() const = 0; - StringVector column_list; - protected: + StringVector column_list; std::string m_name; }; @@ -187,31 +192,47 @@ private: class UniqueConstraint : public Constraint { public: - explicit UniqueConstraint(const StringVector& columns = {}) : - Constraint (columns) - {} + explicit UniqueConstraint(const IndexedColumnVector& columns = {}); + explicit UniqueConstraint(const StringVector& columns); + + void setConflictAction(const std::string& conflict) { m_conflictAction = conflict; } + const std::string& conflictAction() const { return m_conflictAction; } + + // We override these because we maintain our own copy of the column_list variable in m_columns. + // This needs to be done because in a unique constraint we can add expressions, sort order, etc. to the + // list of columns. + void setColumnList(const StringVector& list) override; + void addToColumnList(const std::string& key) override; + void replaceInColumnList(const std::string& from, const std::string& to) override; + void removeFromColumnList(const std::string& key) override; std::string toSql() const override; ConstraintTypes type() const override { return UniqueConstraintType; } + +protected: + IndexedColumnVector m_columns; + std::string m_conflictAction; }; -class PrimaryKeyConstraint : public Constraint +class PrimaryKeyConstraint : public UniqueConstraint { -public: - explicit PrimaryKeyConstraint(const StringVector& columns = {}) : - Constraint (columns) - {} + // Primary keys are a sort of unique constraint for us. This matches quite nicely as both can have a conflict action + // and both need to maintain a copy of the column list with sort order information etc. - void setConflictAction(const std::string& conflict) { m_conflictAction = conflict; } - const std::string& conflictAction() const { return m_conflictAction; } +public: + explicit PrimaryKeyConstraint(const IndexedColumnVector& columns = {}); + explicit PrimaryKeyConstraint(const StringVector& columns); + + void setAutoIncrement(bool ai) { m_auto_increment = ai; } + bool autoIncrement() const { return m_auto_increment; } std::string toSql() const override; ConstraintTypes type() const override { return PrimaryKeyConstraintType; } private: - std::string m_conflictAction; + bool m_auto_increment; }; class CheckConstraint : public Constraint @@ -238,7 +259,6 @@ class Field public: Field() : m_notnull(false), - m_autoincrement(false), m_unique(false) {} @@ -254,7 +274,6 @@ public: , m_notnull(notnull) , m_check(check) , m_defaultvalue(defaultvalue) - , m_autoincrement(false) , m_unique(unique) , m_collation(collation) {} @@ -268,7 +287,6 @@ public: void setNotNull(bool notnull = true) { m_notnull = notnull; } void setCheck(const std::string& check) { m_check = check; } void setDefaultValue(const std::string& defaultvalue) { m_defaultvalue = defaultvalue; } - void setAutoIncrement(bool autoinc) { m_autoincrement = autoinc; } void setUnique(bool u) { m_unique = u; } void setCollation(const std::string& collation) { m_collation = collation; } @@ -286,7 +304,6 @@ public: bool notnull() const { return m_notnull; } const std::string& check() const { return m_check; } const std::string& defaultValue() const { return m_defaultvalue; } - bool autoIncrement() const { return m_autoincrement; } bool unique() const { return m_unique; } const std::string& collation() const { return m_collation; } @@ -296,7 +313,6 @@ private: bool m_notnull; std::string m_check; std::string m_defaultvalue; - bool m_autoincrement; //! this is stored here for simplification bool m_unique; std::string m_collation; }; @@ -343,8 +359,7 @@ public: ConstraintSet allConstraints() const { return m_constraints; } void setConstraints(const ConstraintSet& constraints); void replaceConstraint(ConstraintPtr from, ConstraintPtr to); - StringVector& primaryKeyRef(); - const StringVector& primaryKey() const; + std::shared_ptr primaryKey(); void removeKeyFromAllConstraints(const std::string& key); void renameKeyInAllConstraints(const std::string& key, const std::string& to); @@ -356,7 +371,6 @@ public: static TablePtr parseSQL(const std::string& sSQL); private: StringVector fieldList() const; - bool hasAutoIncrement() const; private: bool m_withoutRowid; diff --git a/src/sqlitetablemodel.cpp b/src/sqlitetablemodel.cpp index 44654f94..81f404d0 100644 --- a/src/sqlitetablemodel.cpp +++ b/src/sqlitetablemodel.cpp @@ -462,7 +462,8 @@ bool SqliteTableModel::setTypedData(const QModelIndex& index, bool isBlob, const if(table) { auto field = sqlb::findField(table, m_headers.at(column)); - if(contains(table->primaryKey(), field->name()) && field->isInteger()) + const auto pk = table->primaryKey(); + if(pk && contains(pk->columnList(), field->name()) && field->isInteger()) newValue = "0"; } } @@ -681,9 +682,9 @@ QModelIndex SqliteTableModel::dittoRecord(int old_row) sqlb::TablePtr t = m_db.getObjectByName(m_query.table()); - sqlb::StringVector pk = t->primaryKey(); + const auto pk = t->primaryKey(); for (size_t col = 0; col < t->fields.size(); ++col) { - if(!contains(pk, t->fields.at(col).name())) { + if(!pk || !contains(pk->columnList(), t->fields.at(col).name())) { if (!firstEditedColumn) firstEditedColumn = col + 1; diff --git a/src/tests/testsqlobjects.cpp b/src/tests/testsqlobjects.cpp index c5aeb0db..8af6edd9 100644 --- a/src/tests/testsqlobjects.cpp +++ b/src/tests/testsqlobjects.cpp @@ -86,17 +86,19 @@ void TestTable::autoincrement() { Table tt("testtable"); Field f("id", "integer"); - f.setAutoIncrement(true); Field fkm("km", "integer"); tt.fields.push_back(f); tt.fields.emplace_back("car", "text"); tt.fields.push_back(fkm); - tt.addConstraint(ConstraintPtr(new PrimaryKeyConstraint({f.name()}))); + PrimaryKeyConstraint pk({f.name()}); + pk.setAutoIncrement(true); + tt.addConstraint(ConstraintPtr(new PrimaryKeyConstraint(pk))); QCOMPARE(tt.sql(), "CREATE TABLE \"testtable\" (\n" - "\t\"id\"\tinteger PRIMARY KEY AUTOINCREMENT,\n" + "\t\"id\"\tinteger,\n" "\t\"car\"\ttext,\n" - "\t\"km\"\tinteger\n" + "\t\"km\"\tinteger,\n" + "\tPRIMARY KEY(\"id\" AUTOINCREMENT)\n" ");"); } @@ -104,17 +106,19 @@ void TestTable::notnull() { Table tt("testtable"); Field f("id", "integer"); - f.setAutoIncrement(true); Field fkm("km", "integer"); tt.fields.push_back(f); tt.fields.emplace_back("car", "text", true); tt.fields.push_back(fkm); - tt.addConstraint(ConstraintPtr(new PrimaryKeyConstraint({f.name()}))); + PrimaryKeyConstraint pk({f.name()}); + pk.setAutoIncrement(true); + tt.addConstraint(ConstraintPtr(new PrimaryKeyConstraint(pk))); QCOMPARE(tt.sql(), "CREATE TABLE \"testtable\" (\n" - "\t\"id\"\tinteger PRIMARY KEY AUTOINCREMENT,\n" + "\t\"id\"\tinteger,\n" "\t\"car\"\ttext NOT NULL,\n" - "\t\"km\"\tinteger\n" + "\t\"km\"\tinteger,\n" + "\tPRIMARY KEY(\"id\" AUTOINCREMENT)\n" ");"); } @@ -122,15 +126,15 @@ void TestTable::withoutRowid() { Table tt("testtable"); Field f("a", "integer"); - f.setAutoIncrement(true); tt.fields.push_back(f); tt.fields.emplace_back("b", "integer"); tt.setWithoutRowidTable(true); tt.addConstraint(ConstraintPtr(new PrimaryKeyConstraint({f.name()}))); QCOMPARE(tt.sql(), "CREATE TABLE \"testtable\" (\n" - "\t\"a\"\tinteger PRIMARY KEY AUTOINCREMENT,\n" - "\t\"b\"\tinteger\n" + "\t\"a\"\tinteger,\n" + "\t\"b\"\tinteger,\n" + "\tPRIMARY KEY(\"a\")\n" ") WITHOUT ROWID;"); } @@ -140,7 +144,7 @@ void TestTable::foreignKeys() Field f("a", "integer"); tt.fields.push_back(f); sqlb::ConstraintPtr fk = sqlb::ConstraintPtr(new sqlb::ForeignKeyClause("b", sqlb::StringVector{"c"})); - fk->column_list = {f.name()}; + fk->setColumnList({f.name()}); tt.addConstraint(fk); QCOMPARE(tt.sql(), "CREATE TABLE \"testtable\" (\n" @@ -189,10 +193,10 @@ void TestTable::parseSQL() QCOMPARE(tab.fields.at(1).type(), "text"); QCOMPARE(tab.fields.at(2).type(), "VARCHAR(255)"); - sqlb::StringVector pk = tab.primaryKey(); - QVERIFY(tab.fields.at(0).autoIncrement()); - QCOMPARE(pk.size(), 1); - QCOMPARE(pk.at(0), tab.fields.at(0).name()); + auto pk = tab.primaryKey(); + QVERIFY(pk->autoIncrement()); + QCOMPARE(pk->columnList().size(), 1); + QCOMPARE(pk->columnList().at(0), tab.fields.at(0).name()); QVERIFY(tab.fields.at(1).notnull()); QCOMPARE(tab.fields.at(1).defaultValue(), "'xxxx'"); QCOMPARE(tab.fields.at(1).check(), ""); @@ -227,9 +231,9 @@ void TestTable::parseSQLdefaultexpr() QCOMPARE(tab.fields.at(3).defaultValue(), ""); QCOMPARE(tab.fields.at(3).check(), ""); - sqlb::StringVector pk = tab.primaryKey(); - QCOMPARE(pk.size(), 1); - QCOMPARE(pk.at(0), tab.fields.at(0).name()); + auto pk = tab.primaryKey(); + QCOMPARE(pk->columnList().size(), 1); + QCOMPARE(pk->columnList().at(0), tab.fields.at(0).name()); } void TestTable::parseSQLMultiPk() @@ -250,10 +254,10 @@ void TestTable::parseSQLMultiPk() QCOMPARE(tab.fields.at(0).type(), "integer"); QCOMPARE(tab.fields.at(1).type(), "integer"); - sqlb::StringVector pk = tab.primaryKey(); - QCOMPARE(pk.size(), 2); - QCOMPARE(pk.at(0), tab.fields.at(0).name()); - QCOMPARE(pk.at(1), tab.fields.at(1).name()); + auto pk = tab.primaryKey(); + QCOMPARE(pk->columnList().size(), 2); + QCOMPARE(pk->columnList().at(0), tab.fields.at(0).name()); + QCOMPARE(pk->columnList().at(1), tab.fields.at(1).name()); } void TestTable::parseSQLForeignKey() @@ -321,7 +325,7 @@ void TestTable::parseSQLWithoutRowid() Table tab(*(std::dynamic_pointer_cast(Table::parseSQL(sSQL)))); - QCOMPARE(tab.primaryKey(), {"a"}); + QCOMPARE(tab.primaryKey()->columnList(), {"a"}); QCOMPARE(tab.rowidColumns(), {"a"}); }