From 63cf8710172ed7075c2d8f37bc226fb46669c25c Mon Sep 17 00:00:00 2001 From: Martin Kleusberg Date: Mon, 3 Jun 2013 20:15:29 +0200 Subject: [PATCH] SqliteTypes: Move PK flag from table to field Store the primary key flag(s) inside the sqlb::Field objects instead of the sqlb::Table object. Technically this doesn't make a lot of sense but then again it makes things a lot easier for us. So this should fix quite a few issues in the entire program, especially - again - in renameColumn(). It also magically fixes createColumn() which had no chance of guessing which column should be a PK prior to this. To benefit from these changes the EditTableDialog has changes as well. - It should now be possible to set and unset PK and AI flags and they are actually saved. - Setting the AI flag automatically sets the data type to Integer because that's the only type SQLite can handle in an autoincrement field. - Clicking on the entry in the data type combobox which is currently selected doesn't update the DB anymore. This commit also makes some changes to the unit tests to reflect the API changes made. But it also adds missing quote characters in some verification strings. I hope this is a worthy 500th commit - at least it's got a long commit message... --- src/EditTableDialog.cpp | 31 +++++++--------- src/sqlitetypes.cpp | 69 ++++++++++-------------------------- src/sqlitetypes.h | 17 ++++----- src/tests/testsqlobjects.cpp | 39 ++++++++++---------- 4 files changed, 60 insertions(+), 96 deletions(-) diff --git a/src/EditTableDialog.cpp b/src/EditTableDialog.cpp index 165d5920..10e33302 100644 --- a/src/EditTableDialog.cpp +++ b/src/EditTableDialog.cpp @@ -75,7 +75,6 @@ void EditTableDialog::populateFields() tbitem->setText(kName, f->name()); QComboBox* typeBox = new QComboBox(ui->treeWidget); typeBox->setProperty("column", f->name()); - connect(typeBox, SIGNAL(activated(int)), this, SLOT(updateTypes())); typeBox->setEditable(false); typeBox->addItems(sqlb::Field::Datatypes); int index = typeBox->findText(f->type(), Qt::MatchExactly); @@ -86,10 +85,11 @@ void EditTableDialog::populateFields() index = typeBox->count() - 1; } typeBox->setCurrentIndex(index); + connect(typeBox, SIGNAL(currentIndexChanged(int)), this, SLOT(updateTypes())); ui->treeWidget->setItemWidget(tbitem, kType, typeBox); tbitem->setCheckState(kNotNull, f->notnull() ? Qt::Checked : Qt::Unchecked); - tbitem->setCheckState(kPrimaryKey, m_table.primarykey().contains(f) ? Qt::Checked : Qt::Unchecked); + tbitem->setCheckState(kPrimaryKey, f->primaryKey() ? Qt::Checked : Qt::Unchecked); tbitem->setCheckState(kAutoIncrement, f->autoIncrement() ? Qt::Checked : Qt::Unchecked); tbitem->setText(kDefault, f->defaultValue()); tbitem->setText(kCheck, f->check()); @@ -204,28 +204,21 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) break; case kPrimaryKey: { - sqlb::FieldVector pks = m_table.primarykey(); + field->setPrimaryKey(item->checkState(column) == Qt::Checked); if(item->checkState(column) == Qt::Checked) { - pks.append(field); - // this will unset any set autoincrement + // this will unset any other autoincrement for(int i = 0; i < ui->treeWidget->topLevelItemCount(); ++i) { QTreeWidgetItem* tbitem = ui->treeWidget->topLevelItem(i); if(tbitem != item) - { tbitem->setCheckState(kAutoIncrement, Qt::Unchecked); - } } - } - else - { + } else { item->setCheckState(kAutoIncrement, Qt::Unchecked); - int index = pks.indexOf(field); - if(index != -1) - pks.remove(index); } - m_table.setPrimaryKey(pks); + if(!m_bNewTable) + callRenameColumn = true; } break; case kNotNull: @@ -255,13 +248,14 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) case kAutoIncrement: { bool ischecked = item->checkState(column) == Qt::Checked; - field->setAutoIncrement(ischecked); if(ischecked) { - item->setCheckState(kPrimaryKey, Qt::Checked); + QComboBox* comboType = qobject_cast(ui->treeWidget->itemWidget(item, kType)); + comboType->setCurrentIndex(comboType->findText("INTEGER")); + item->setCheckState(kPrimaryKey, Qt::Checked); - // this will reset all other primary keys unset - // there can't be more then one autoincrement pk + // this will unset all other primary keys + // there can't be more than one autoincrement pk for(int i = 0; i < ui->treeWidget->topLevelItemCount(); ++i) { QTreeWidgetItem* tbitem = ui->treeWidget->topLevelItem(i); @@ -272,6 +266,7 @@ void EditTableDialog::itemChanged(QTreeWidgetItem *item, int column) } } } + field->setAutoIncrement(ischecked); if(!m_bNewTable) callRenameColumn = true; diff --git a/src/sqlitetypes.cpp b/src/sqlitetypes.cpp index da26a4a4..8d8d8d0c 100644 --- a/src/sqlitetypes.cpp +++ b/src/sqlitetypes.cpp @@ -55,7 +55,6 @@ bool Field::isInteger() const void Table::clear() { m_fields.clear(); - m_primarykey.clear(); } Table::~Table() @@ -95,31 +94,6 @@ int Table::findField(const QString &sname) return -1; } -bool Table::setPrimaryKey(const FieldVector& pk) -{ - foreach(FieldPtr f, pk) { - if(!m_fields.contains(f)) - return false; - } - - m_primarykey = pk; - return true; -} - -bool Table::setPrimaryKey(FieldPtr pk, bool autoincrement) -{ - if(m_fields.contains(pk)) - { - if(pk->isInteger() && autoincrement) - pk->setAutoIncrement(true); - m_primarykey.clear(); - m_primarykey.append(pk); - return true; - } - - return false; -} - QStringList Table::fieldList() const { QStringList sl; @@ -175,12 +149,10 @@ QString Table::emptyInsertStmt() const if(f->notnull()) { fields << f->name(); - if( m_primarykey.contains(f) && f->isInteger() ) + if( f->primaryKey() && f->isInteger() ) { vals << "NULL"; - } - else - { + } else { if(f->isInteger()) vals << "0"; else @@ -220,18 +192,21 @@ QString Table::sql() const sql += fieldList().join(",\n"); // primary key - if( m_primarykey.size() > 0 && !hasAutoIncrement()) + if(!hasAutoIncrement()) { - sql += ",\n\tPRIMARY KEY("; - for(FieldVector::ConstIterator it = m_primarykey.constBegin(); - it != m_primarykey.constEnd(); - ++it) + QString pk = ",\n\tPRIMARY KEY("; + bool pks_found = false; + foreach(FieldPtr f, m_fields) { - sql += (*it)->name(); - if(*it != m_primarykey.last()) - sql += ","; + if(f->primaryKey()) + { + pk += f->name() + ","; + pks_found = true; + } } - sql += ")"; + pk.chop(1); + if(pks_found) + sql += pk + ")"; } return sql + "\n);"; @@ -281,16 +256,12 @@ Table CreateTableWalker::table() while(column != antlr::nullAST && column->getType() == sqlite3TokenTypes::COLUMNDEF) { FieldPtr f; - bool pk = parsecolumn(f, column->getFirstChild()); + parsecolumn(f, column->getFirstChild()); tab.addField(f); - if(pk) - pks.append(FieldPtr(f)); column = column->getNextSibling(); //COMMA or RPAREN column = column->getNextSibling(); //null or tableconstraint } - tab.setPrimaryKey(pks); - // now we are finished or it is a tableconstraint while(s != antlr::nullAST) { @@ -305,18 +276,15 @@ Table CreateTableWalker::table() { tc = tc->getNextSibling()->getNextSibling(); // skip primary and key tc = tc->getNextSibling(); // skip LPAREN - pks.clear(); do { QString col = identifier(tc); int fieldindex = tab.findField(col); if(fieldindex != -1) - pks.append(tab.fields().at(fieldindex)); + tab.fields().at(fieldindex)->setPrimaryKey(true); tc = tc->getNextSibling(); // skip ident and comma tc = tc->getNextSibling(); } while(tc != antlr::nullAST && tc->getType() != sqlite3TokenTypes::RPAREN); - - tab.setPrimaryKey(pks); } break; default: break; @@ -330,7 +298,7 @@ Table CreateTableWalker::table() return tab; } -bool CreateTableWalker::parsecolumn(FieldPtr& f, antlr::RefAST c) +void CreateTableWalker::parsecolumn(FieldPtr& f, antlr::RefAST c) { QString columnname; QString type = "TEXT"; @@ -396,9 +364,8 @@ bool CreateTableWalker::parsecolumn(FieldPtr& f, antlr::RefAST c) c = c->getNextSibling(); } - f = FieldPtr( new Field(columnname, type, notnull, defaultvalue, check)); + f = FieldPtr( new Field(columnname, type, notnull, defaultvalue, check, primarykey)); f->setAutoIncrement(autoincrement); - return primarykey; } } //namespace sqlb diff --git a/src/sqlitetypes.h b/src/sqlitetypes.h index 199f177b..fac7c707 100644 --- a/src/sqlitetypes.h +++ b/src/sqlitetypes.h @@ -18,13 +18,16 @@ public: const QString& type, bool notnull = false, const QString& defaultvalue = "", - const QString& check = "") + const QString& check = "", + bool pk = false) : m_name(name) , m_type(type) , m_notnull(notnull) , m_check(check) , m_defaultvalue(defaultvalue) - , m_autoincrement(false) {} + , m_autoincrement(false) + , m_primaryKey(pk) + {} QString toString(const QString& indent = "\t", const QString& sep = "\t") const; @@ -34,6 +37,7 @@ public: void setCheck(const QString& check) { m_check = check; } void setDefaultValue(const QString& defaultvalue) { m_defaultvalue = defaultvalue; } void setAutoIncrement(bool autoinc) { m_autoincrement = autoinc; } + void setPrimaryKey(bool pk) { m_primaryKey = pk; } bool isText() const; bool isInteger() const; @@ -44,6 +48,7 @@ public: const QString& check() const { return m_check; } const QString& defaultValue() const { return m_defaultvalue; } bool autoIncrement() const { return m_autoincrement; } + bool primaryKey() const { return m_primaryKey; } static QStringList Datatypes; private: @@ -53,6 +58,7 @@ private: QString m_check; QString m_defaultvalue; bool m_autoincrement; //! this is stored here for simplification + bool m_primaryKey; }; typedef QSharedPointer FieldPtr; @@ -67,7 +73,6 @@ public: const QString& name() const { return m_name; } const FieldVector& fields() const { return m_fields; } - const FieldVector& primarykey() const { return m_primarykey; } /** * @brief Creates an empty insert statement. @@ -94,9 +99,6 @@ public: */ int findField(const QString& sname); - bool setPrimaryKey(const FieldVector& pk); - bool setPrimaryKey(FieldPtr pk, bool autoincrement = false); - static Table parseSQL(const QString& sSQL); private: QStringList fieldList() const; @@ -105,7 +107,6 @@ private: private: QString m_name; FieldVector m_fields; - FieldVector m_primarykey; }; /** @@ -121,7 +122,7 @@ public: Table table(); private: - bool parsecolumn(FieldPtr& f, antlr::RefAST c); + void parsecolumn(FieldPtr& f, antlr::RefAST c); private: antlr::RefAST m_root; diff --git a/src/tests/testsqlobjects.cpp b/src/tests/testsqlobjects.cpp index f79e0926..cdb1cb6f 100644 --- a/src/tests/testsqlobjects.cpp +++ b/src/tests/testsqlobjects.cpp @@ -22,19 +22,17 @@ void TestTable::sqlOutput() { Table tt("testtable"); FieldPtr f = FieldPtr(new Field("id", "integer")); + f->setPrimaryKey(true); FieldPtr fkm = FieldPtr(new Field("km", "integer", false, "", "km > 1000")); + fkm->setPrimaryKey(true); tt.addField(f); tt.addField(FieldPtr(new Field("car", "text"))); tt.addField(fkm); - FieldVector pk; - pk.append(f); - pk.append(fkm); - tt.setPrimaryKey(pk); QCOMPARE(tt.sql(), QString("CREATE TABLE `testtable` (\n" - "\tid\tinteger,\n" - "\tcar\ttext,\n" - "\tkm\tinteger CHECK(km > 1000),\n" + "\t`id`\tinteger,\n" + "\t`car`\ttext,\n" + "\t`km`\tinteger CHECK(km > 1000),\n" "\tPRIMARY KEY(id,km)\n" ");")); } @@ -43,16 +41,17 @@ void TestTable::autoincrement() { Table tt("testtable"); FieldPtr f = FieldPtr(new Field("id", "integer")); + f->setPrimaryKey(true); + f->setAutoIncrement(true); FieldPtr fkm = FieldPtr(new Field("km", "integer")); tt.addField(f); tt.addField(FieldPtr(new Field("car", "text"))); tt.addField(fkm); - tt.setPrimaryKey(f, true); QCOMPARE(tt.sql(), QString("CREATE TABLE `testtable` (\n" - "\tid\tinteger PRIMARY KEY AUTOINCREMENT,\n" - "\tcar\ttext,\n" - "\tkm\tinteger\n" + "\t`id`\tinteger PRIMARY KEY AUTOINCREMENT,\n" + "\t`car`\ttext,\n" + "\t`km`\tinteger\n" ");")); } @@ -60,16 +59,17 @@ void TestTable::notnull() { Table tt("testtable"); FieldPtr f = FieldPtr(new Field("id", "integer")); + f->setPrimaryKey(true); + f->setAutoIncrement(true); FieldPtr fkm = FieldPtr(new Field("km", "integer")); tt.addField(f); tt.addField(FieldPtr(new Field("car", "text", true))); tt.addField(fkm); - tt.setPrimaryKey(f, true); QCOMPARE(tt.sql(), QString("CREATE TABLE `testtable` (\n" - "\tid\tinteger PRIMARY KEY AUTOINCREMENT,\n" - "\tcar\ttext NOT NULL,\n" - "\tkm\tinteger\n" + "\t`id`\tinteger PRIMARY KEY AUTOINCREMENT,\n" + "\t`car`\ttext NOT NULL,\n" + "\t`km`\tinteger\n" ");")); } @@ -93,12 +93,13 @@ void TestTable::parseSQL() QVERIFY(tab.fields().at(2)->type() == "VARCHAR(255)"); QVERIFY(tab.fields().at(0)->autoIncrement()); + QVERIFY(tab.fields().at(0)->primaryKey()); QVERIFY(tab.fields().at(1)->notnull()); QCOMPARE(tab.fields().at(1)->defaultValue(), QString("'xxxx'")); QCOMPARE(tab.fields().at(1)->check(), QString("")); QCOMPARE(tab.fields().at(2)->check(), QString("info==x")); - QVERIFY(tab.primarykey().contains(tab.fields().at(0))); + } void TestTable::parseSQLdefaultexpr() @@ -124,7 +125,7 @@ void TestTable::parseSQLdefaultexpr() QCOMPARE(tab.fields().at(2)->defaultValue(), QString("")); QCOMPARE(tab.fields().at(2)->check(), QString("")); - QVERIFY(tab.primarykey().contains(tab.fields().at(0))); + QVERIFY(tab.fields().at(0)->primaryKey()); } void TestTable::parseSQLMultiPk() @@ -145,8 +146,8 @@ void TestTable::parseSQLMultiPk() QVERIFY(tab.fields().at(0)->type() == "integer"); QVERIFY(tab.fields().at(1)->type() == "integer"); - QVERIFY(tab.primarykey().contains(tab.fields().at(0))); - QVERIFY(tab.primarykey().contains(tab.fields().at(1))); + QVERIFY(tab.fields().at(0)->primaryKey()); + QVERIFY(tab.fields().at(1)->primaryKey()); } void TestTable::parseSQLForeignKey()