From 0f15ab4e750690bdb2b649332d5c3276bf24c440 Mon Sep 17 00:00:00 2001 From: Mark Brand Date: Tue, 29 Mar 2011 10:27:17 -0500 Subject: let generated flag control SQL generation Previously, if QSqlField's QVariant value had type "invalid", this caused the field to be omitted in data-changing SQL statements generated from QSqlRecord edit buffers. Complaints against this mechanism: - It precludes initializing value type to field type, which would otherwise seem sensible and useful, such as when using model.data() in contexts where the field type is not readily available. - QSqlField::clear() does initialize value type to match field type and so is incompatible with this mechanism. - Problems such as described in QTBUG-13211. - Unwanted distinction between "invalid" and "null" when mapping field values to SQL values. - QSqlField's generated flag already provides a mechanism for controlling SQL generation. These complaints are redressed here by replacing this mechanism with reliance on QSqlField's generated flag. The flag is initialized to false in new edit records and set to true when a value is set. Applications can still manipulate generated flags directly to control SQL generation. Generation of SELECT statements already used the generated flag and is unaffected by this change. Merge-request: 1114 Reviewed-by: Marius Storm-Olsen Reviewed-by: Michael Goddard --- src/sql/kernel/qsqldriver.cpp | 4 +- src/sql/kernel/qsqlfield.cpp | 3 ++ src/sql/models/qsqlrelationaltablemodel.cpp | 10 ++-- src/sql/models/qsqltablemodel.cpp | 64 ++++++++++++++---------- src/sql/models/qsqltablemodel_p.h | 4 +- tests/auto/qsqlquery/tst_qsqlquery.cpp | 7 ++- tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp | 6 +-- tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp | 6 +-- 8 files changed, 62 insertions(+), 42 deletions(-) diff --git a/src/sql/kernel/qsqldriver.cpp b/src/sql/kernel/qsqldriver.cpp index c8a16c8..bbec21d 100644 --- a/src/sql/kernel/qsqldriver.cpp +++ b/src/sql/kernel/qsqldriver.cpp @@ -496,7 +496,7 @@ QString QSqlDriver::sqlStatement(StatementType type, const QString &tableName, s.append(QLatin1String("UPDATE ")).append(tableName).append( QLatin1String(" SET ")); for (i = 0; i < rec.count(); ++i) { - if (!rec.isGenerated(i) || !rec.value(i).isValid()) + if (!rec.isGenerated(i)) continue; s.append(prepareIdentifier(rec.fieldName(i), QSqlDriver::FieldName, this)).append(QLatin1Char('=')); if (preparedStatement) @@ -517,7 +517,7 @@ QString QSqlDriver::sqlStatement(StatementType type, const QString &tableName, s.append(QLatin1String("INSERT INTO ")).append(tableName).append(QLatin1String(" (")); QString vals; for (i = 0; i < rec.count(); ++i) { - if (!rec.isGenerated(i) || !rec.value(i).isValid()) + if (!rec.isGenerated(i)) continue; s.append(prepareIdentifier(rec.fieldName(i), QSqlDriver::FieldName, this)).append(QLatin1String(", ")); if (preparedStatement) diff --git a/src/sql/kernel/qsqlfield.cpp b/src/sql/kernel/qsqlfield.cpp index a1ab9e3..b7e58b7 100644 --- a/src/sql/kernel/qsqlfield.cpp +++ b/src/sql/kernel/qsqlfield.cpp @@ -162,6 +162,7 @@ public: QSqlField::QSqlField(const QString& fieldName, QVariant::Type type) { d = new QSqlFieldPrivate(fieldName, type); + val = QVariant(type); } /*! @@ -389,6 +390,8 @@ void QSqlField::setType(QVariant::Type type) { detach(); d->type = type; + if (!val.isValid()) + val = QVariant(type); } diff --git a/src/sql/models/qsqlrelationaltablemodel.cpp b/src/sql/models/qsqlrelationaltablemodel.cpp index a261586..63633e6 100644 --- a/src/sql/models/qsqlrelationaltablemodel.cpp +++ b/src/sql/models/qsqlrelationaltablemodel.cpp @@ -275,6 +275,7 @@ int QSqlRelationalTableModelPrivate::nameToIndex(const QString &name) const void QSqlRelationalTableModelPrivate::clearEditBuffer() { editBuffer = baseRec; + clearGenerated(editBuffer); } /*! @@ -410,13 +411,14 @@ QVariant QSqlRelationalTableModel::data(const QModelIndex &index, int role) cons case OnFieldChange: break; case OnRowChange: - if (index.row() == d->editIndex || index.row() == d->insertIndex) { + if ((index.row() == d->editIndex || index.row() == d->insertIndex) + && d->editBuffer.isGenerated(index.column())) v = d->editBuffer.value(index.column()); - } break; case OnManualSubmit: const QSqlTableModelPrivate::ModifiedRow row = d->cache.value(index.row()); - v = row.rec.value(index.column()); + if (row.op != QSqlTableModelPrivate::None && row.rec.isGenerated(index.column())) + v = row.rec.value(index.column()); break; } if (v.isValid()) @@ -678,8 +680,10 @@ void QSqlRelationalTableModelPrivate::translateFieldNames(int row, QSqlRecord &v int realCol = q->indexInQuery(q->createIndex(row, i)).column(); if (realCol != -1 && relations.value(realCol).isValid()) { QVariant v = values.value(i); + bool gen = values.isGenerated(i); values.replace(i, baseRec.field(realCol)); values.setValue(i, v); + values.setGenerated(i, gen); } } } diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp index bf7c0aa..99b516a 100644 --- a/src/sql/models/qsqltablemodel.cpp +++ b/src/sql/models/qsqltablemodel.cpp @@ -138,6 +138,7 @@ void QSqlTableModelPrivate::revertInsertedRow() void QSqlTableModelPrivate::clearEditBuffer() { editBuffer = rec; + clearGenerated(editBuffer); } void QSqlTableModelPrivate::clearCache() @@ -145,6 +146,18 @@ void QSqlTableModelPrivate::clearCache() cache.clear(); } +void QSqlTableModelPrivate::clearGenerated(QSqlRecord &rec) +{ + for (int i = rec.count() - 1; i >= 0; i--) + rec.setGenerated(i, false); +} + +void QSqlTableModelPrivate::setGeneratedValue(QSqlRecord &rec, int c, QVariant v) +{ + rec.setValue(c, v); + rec.setGenerated(c, true); +} + void QSqlTableModelPrivate::revertCachedRow(int row) { Q_Q(QSqlTableModel); @@ -201,7 +214,7 @@ bool QSqlTableModelPrivate::exec(const QString &stmt, bool prepStatement, } int i; for (i = 0; i < rec.count(); ++i) { - if (rec.isGenerated(i) && rec.value(i).type() != QVariant::Invalid) + if (rec.isGenerated(i)) editQuery.addBindValue(rec.value(i)); } for (i = 0; i < whereValues.count(); ++i) { @@ -435,26 +448,22 @@ QVariant QSqlTableModel::data(const QModelIndex &index, int role) const case OnFieldChange: case OnRowChange: if (index.row() == d->insertIndex) { - QVariant val; if (item.column() < 0 || item.column() >= d->rec.count()) - return val; - val = d->editBuffer.value(index.column()); - if (val.type() == QVariant::Invalid) - val = QVariant(d->rec.field(item.column()).type()); - return val; + return QVariant(); + return d->editBuffer.value(index.column()); } if (d->editIndex == item.row()) { - QVariant var = d->editBuffer.value(item.column()); - if (var.isValid()) - return var; + if (d->editBuffer.isGenerated(item.column())) + return d->editBuffer.value(item.column()); + } + break; + case OnManualSubmit: + if (d->cache.contains(index.row())) { + const QSqlTableModelPrivate::ModifiedRow row = d->cache.value(index.row()); + if (row.rec.isGenerated(item.column()) || row.op == QSqlTableModelPrivate::Insert) + return row.rec.value(item.column()); } break; - case OnManualSubmit: { - const QSqlTableModelPrivate::ModifiedRow row = d->cache.value(index.row()); - const QVariant var = row.rec.value(item.column()); - if (var.isValid() || row.op == QSqlTableModelPrivate::Insert) - return var; - break; } } // We need to handle row mapping here, but not column mapping @@ -503,13 +512,13 @@ bool QSqlTableModel::isDirty(const QModelIndex &index) const case OnFieldChange: return false; case OnRowChange: - return index.row() == d->editIndex && d->editBuffer.value(index.column()).isValid(); + return index.row() == d->editIndex && d->editBuffer.isGenerated(index.column()); case OnManualSubmit: { const QSqlTableModelPrivate::ModifiedRow row = d->cache.value(index.row()); return row.op == QSqlTableModelPrivate::Insert || row.op == QSqlTableModelPrivate::Delete || (row.op == QSqlTableModelPrivate::Update - && row.rec.value(index.column()).isValid()); + && row.rec.isGenerated(index.column())); } } return false; @@ -538,11 +547,11 @@ bool QSqlTableModel::setData(const QModelIndex &index, const QVariant &value, in switch (d->strategy) { case OnFieldChange: { if (index.row() == d->insertIndex) { - d->editBuffer.setValue(index.column(), value); + QSqlTableModelPrivate::setGeneratedValue(d->editBuffer, index.column(), value); return true; } d->clearEditBuffer(); - d->editBuffer.setValue(index.column(), value); + QSqlTableModelPrivate::setGeneratedValue(d->editBuffer, index.column(), value); isOk = updateRowInTable(index.row(), d->editBuffer); if (isOk) select(); @@ -550,7 +559,7 @@ bool QSqlTableModel::setData(const QModelIndex &index, const QVariant &value, in break; } case OnRowChange: if (index.row() == d->insertIndex) { - d->editBuffer.setValue(index.column(), value); + QSqlTableModelPrivate::setGeneratedValue(d->editBuffer, index.column(), value); return true; } if (d->editIndex != index.row()) { @@ -558,7 +567,7 @@ bool QSqlTableModel::setData(const QModelIndex &index, const QVariant &value, in submit(); d->clearEditBuffer(); } - d->editBuffer.setValue(index.column(), value); + QSqlTableModelPrivate::setGeneratedValue(d->editBuffer, index.column(), value); d->editIndex = index.row(); emit dataChanged(index, index); break; @@ -567,9 +576,10 @@ bool QSqlTableModel::setData(const QModelIndex &index, const QVariant &value, in if (row.op == QSqlTableModelPrivate::None) { row.op = QSqlTableModelPrivate::Update; row.rec = d->rec; + QSqlTableModelPrivate::clearGenerated(row.rec); row.primaryValues = d->primaryValues(indexInQuery(index).row()); } - row.rec.setValue(index.column(), value); + QSqlTableModelPrivate::setGeneratedValue(row.rec, index.column(), value); emit dataChanged(index, index); break; } } @@ -1330,6 +1340,7 @@ bool QSqlTableModel::setRecord(int row, const QSqlRecord &record) if (mrow.op == QSqlTableModelPrivate::None) { mrow.op = QSqlTableModelPrivate::Update; mrow.rec = d->rec; + QSqlTableModelPrivate::clearGenerated(mrow.rec); mrow.primaryValues = d->primaryValues(indexInQuery(createIndex(row, 0)).row()); } QString fieldName; @@ -1338,10 +1349,11 @@ bool QSqlTableModel::setRecord(int row, const QSqlRecord &record) if (d->db.driver()->isIdentifierEscaped(fieldName, QSqlDriver::FieldName)) fieldName = d->db.driver()->stripDelimiters(fieldName, QSqlDriver::FieldName); int idx = mrow.rec.indexOf(fieldName); - if (idx == -1) + if (idx == -1) { isOk = false; - else - mrow.rec.setValue(idx, record.value(i)); + } else { + QSqlTableModelPrivate::setGeneratedValue(mrow.rec, idx, record.value(i)); + } } if (isOk) diff --git a/src/sql/models/qsqltablemodel_p.h b/src/sql/models/qsqltablemodel_p.h index f4f3811..322c23b 100644 --- a/src/sql/models/qsqltablemodel_p.h +++ b/src/sql/models/qsqltablemodel_p.h @@ -72,6 +72,8 @@ public: QSqlRecord primaryValues(int index); virtual void clearEditBuffer(); virtual void clearCache(); + static void clearGenerated(QSqlRecord &rec); + static void setGeneratedValue(QSqlRecord &rec, int c, QVariant v); QSqlRecord record(const QVector &values) const; bool exec(const QString &stmt, bool prepStatement, @@ -100,7 +102,7 @@ public: struct ModifiedRow { - ModifiedRow(Op o = None, const QSqlRecord &r = QSqlRecord()): op(o), rec(r) {} + ModifiedRow(Op o = None, const QSqlRecord &r = QSqlRecord()): op(o), rec(r) { clearGenerated(rec);} ModifiedRow(const ModifiedRow &other): op(other.op), rec(other.rec), primaryValues(other.primaryValues) {} Op op; QSqlRecord rec; diff --git a/tests/auto/qsqlquery/tst_qsqlquery.cpp b/tests/auto/qsqlquery/tst_qsqlquery.cpp index 288b29c..be2087d 100644 --- a/tests/auto/qsqlquery/tst_qsqlquery.cpp +++ b/tests/auto/qsqlquery/tst_qsqlquery.cpp @@ -928,13 +928,12 @@ void tst_QSqlQuery::record() QSqlQuery q( db ); QVERIFY( q.record().isEmpty() ); QVERIFY_SQL( q, exec( "select id, t_varchar, t_char from " + qtest + " order by id" ) ); - QSqlRecord rec = q.record(); QCOMPARE( q.record().fieldName( 0 ).toLower(), QString( "id" ) ); QCOMPARE( q.record().fieldName( 1 ).toLower(), QString( "t_varchar" ) ); QCOMPARE( q.record().fieldName( 2 ).toLower(), QString( "t_char" ) ); - QVERIFY( !q.record().value( 0 ).isValid() ); - QVERIFY( !q.record().value( 1 ).isValid() ); - QVERIFY( !q.record().value( 2 ).isValid() ); + QCOMPARE(q.record().value(0), QVariant(q.record().field(0).type())); + QCOMPARE(q.record().value(1), QVariant(q.record().field(1).type())); + QCOMPARE(q.record().value(2), QVariant(q.record().field(2).type())); QVERIFY( q.next() ); QVERIFY( q.next() ); diff --git a/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp b/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp index e876764..9507e54 100644 --- a/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp +++ b/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp @@ -428,9 +428,9 @@ void tst_QSqlQueryModel::record() QCOMPARE(rec.fieldName(0), isToUpper ? QString("ID") : QString("id")); QCOMPARE(rec.fieldName(1), isToUpper ? QString("NAME") : QString("name")); QCOMPARE(rec.fieldName(2), isToUpper ? QString("TITLE") : QString("title")); - QCOMPARE(rec.value(0), QVariant()); - QCOMPARE(rec.value(1), QVariant()); - QCOMPARE(rec.value(2), QVariant()); + QCOMPARE(rec.value(0), QVariant(rec.field(0).type())); + QCOMPARE(rec.value(1), QVariant(rec.field(1).type())); + QCOMPARE(rec.value(2), QVariant(rec.field(2).type())); rec = model.record(0); QCOMPARE(rec.fieldName(0), isToUpper ? QString("ID") : QString("id")); diff --git a/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp b/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp index bf68375..b91d025 100644 --- a/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp +++ b/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp @@ -569,9 +569,9 @@ void tst_QSqlTableModel::insertMultiRecords() QVERIFY(model.insertRow(2)); - QCOMPARE(model.data(model.index(2, 0)), QVariant()); - QCOMPARE(model.data(model.index(2, 1)), QVariant()); - QCOMPARE(model.data(model.index(2, 2)), QVariant()); + QCOMPARE(model.data(model.index(2, 0)), QVariant(model.record().field(0).type())); + QCOMPARE(model.data(model.index(2, 1)), QVariant(model.record().field(1).type())); + QCOMPARE(model.data(model.index(2, 2)), QVariant(model.record().field(2).type())); QVERIFY(model.insertRow(3)); QVERIFY(model.insertRow(0)); -- cgit v0.12