From 5953304bc5f50eca77f78fd0623479de6bebacbe Mon Sep 17 00:00:00 2001 From: Mark Brand Date: Mon, 6 Feb 2012 15:03:25 +0100 Subject: [PATCH] QSqlTableModel::setRecord(): do not try to detect value changes In an apparent attempt to be economical with emitting dataChanged() and submitting SQL to the databse, setRecord() compares each field value of the record with the old value, taking action only when a difference is detected. Several complaints against this code are: -The comparision does not work on float type. -It is really up to the application and database to decide this. The model should make few assumptions. The application has the option to omit fields from the record that should be ignored. -The current behavior seems to assume that the "old" values are the current state of the database, but the database may have changed since the model was last refreshed. -The code compares the value from record(), which probably corresponds to the EditRole, with the DisplayRole value from data(). Change-Id: I11477c185eb411d442144dc682893d0df12d03d5 Reviewed-by: Oswald Buddenhagen --- src/sql/models/qsqltablemodel.cpp | 11 ++--------- .../sql/models/qsqltablemodel/tst_qsqltablemodel.cpp | 6 +++--- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp index 1746883d34..7210ddce5b 100644 --- a/src/sql/models/qsqltablemodel.cpp +++ b/src/sql/models/qsqltablemodel.cpp @@ -1217,15 +1217,8 @@ bool QSqlTableModel::setRecord(int row, const QSqlRecord &record) } else if (d->strategy != OnManualSubmit) { // historical bug: this could all be simple like OnManualSubmit, but isn't const QModelIndex cIndex = createIndex(row, idx); - // historical bug: comparing EditRole with DisplayRole values here - const QVariant oldValue = data(cIndex); - const QVariant value = record.value(i); - // historical bug: it's a bad idea to check for change here - // historical bug: should test oldValue.isNull() != value.isNull() - if (oldValue.isNull() || oldValue != value) { - mrow.setValue(idx, record.value(i)); - emit dataChanged(cIndex, cIndex); - } + mrow.setValue(idx, record.value(i)); + emit dataChanged(cIndex, cIndex); } else { mrow.setValue(idx, record.value(i)); } diff --git a/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp b/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp index 6a827e65c5..518c6b6d25 100644 --- a/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp +++ b/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp @@ -412,10 +412,10 @@ void tst_QSqlTableModel::setRecord() model.submit(); else { // dataChanged() is not emitted when submitAll() is called - QCOMPARE(spy.count(), 2); + QCOMPARE(spy.count(), model.columnCount()); QCOMPARE(spy.at(0).count(), 2); - QCOMPARE(qvariant_cast(spy.at(0).at(0)), model.index(i, 1)); - QCOMPARE(qvariant_cast(spy.at(0).at(1)), model.index(i, 1)); + QCOMPARE(qvariant_cast(spy.at(1).at(0)), model.index(i, 1)); + QCOMPARE(qvariant_cast(spy.at(1).at(1)), model.index(i, 1)); } }