QSqlTableModel::selectRow(): don't expand cache if there is no change

Test added.

Change-Id: Ibd72ef2aeee482abbd22991573460e55dc577457
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
Reviewed-by: David Faure (fixes for KDE) <faure@kde.org>

src/sql/models/qsqltablemodel.cpp

@@ -439,18 +439,40 @@ bool QSqlTableModel::selectRow(int row)
     if (stmt.isEmpty())
         return false;
 
+    bool exists;
+    QSqlRecord newValues;
+
     {
         QSqlQuery q(d->db);
         q.setForwardOnly(true);
         if (!q.exec(stmt))
             return false;
 
-        bool exists = q.next();
-        d->cache[row].refresh(exists, q.record());
+        exists = q.next();
+        newValues = q.record();
     }
 
-    emit headerDataChanged(Qt::Vertical, row, row);
-    emit dataChanged(createIndex(row, 0), createIndex(row, columnCount() - 1));
+    bool needsAddingToCache = !exists || d->cache.contains(row);
+
+    if (!needsAddingToCache) {
+        const QSqlRecord curValues = record(row);
+        needsAddingToCache = curValues.count() != newValues.count();
+        if (!needsAddingToCache) {
+            // Look for changed values. Primary key fields are customarily first
+            // and probably change less often than other fields, so start at the end.
+            for (int f = curValues.count() - 1; f >= 0; --f) {
+                if (curValues.value(f) != newValues.value(f))
+                    needsAddingToCache = true;
+                    break;
+            }
+        }
+    }
+
+    if (needsAddingToCache) {
+        d->cache[row].refresh(exists, newValues);
+        emit headerDataChanged(Qt::Vertical, row, row);
+        emit dataChanged(createIndex(row, 0), createIndex(row, columnCount() - 1));
+    }
 
     return true;
 }

tests/auto/sql/models/qsqltablemodel/qsqltablemodel.pro

@@ -2,7 +2,7 @@ CONFIG += testcase
 TARGET = tst_qsqltablemodel
 SOURCES  += tst_qsqltablemodel.cpp
 
-QT = core sql testlib
+QT = core core-private sql sql-private testlib
 
 wince*: {
    plugFiles.files = ../../../plugins/sqldrivers

tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp

@@ -43,6 +43,7 @@
 #include <QtTest/QtTest>
 #include "../../kernel/qsqldatabase/tst_databases.h"
 #include <QtSql>
+#include <QtSql/private/qsqltablemodel_p.h>
 
 const QString test(qTableName("test", __FILE__)),
                    test2(qTableName("test2", __FILE__)),
@@ -319,6 +320,19 @@ void tst_QSqlTableModel::select()
     QCOMPARE(model.data(model.index(3, 3)), QVariant());
 }
 
+class SelectRowModel: public QSqlTableModel
+{
+    Q_OBJECT
+    Q_DECLARE_PRIVATE(QSqlTableModel)
+public:
+    SelectRowModel(QObject *parent, QSqlDatabase db): QSqlTableModel(parent, db) {}
+    bool cacheEmpty() const
+    {
+        Q_D(const QSqlTableModel);
+        return d->cache.isEmpty();
+    }
+};
+
 void tst_QSqlTableModel::selectRow()
 {
     QFETCH(QString, dbName);
@@ -332,7 +346,7 @@ void tst_QSqlTableModel::selectRow()
     q.exec("INSERT INTO " + tbl + " (id, a) VALUES (1, 'b')");
     q.exec("INSERT INTO " + tbl + " (id, a) VALUES (2, 'c')");
 
-    QSqlTableModel model(0, db);
+    SelectRowModel model(0, db);
     model.setEditStrategy(QSqlTableModel::OnFieldChange);
     model.setTable(tbl);
     model.setSort(0, Qt::AscendingOrder);
@@ -343,6 +357,11 @@ void tst_QSqlTableModel::selectRow()
 
     QModelIndex idx = model.index(1, 1);
 
+    // selectRow should not make the cache grow if there is no change.
+    model.selectRow(1);
+    QCOMPARE(model.data(idx).toString(), QString("b"));
+    QVERIFY_SQL(model, cacheEmpty());
+
     // Check if selectRow() refreshes an unchanged row.
     // Row is not in cache yet.
     q.exec("UPDATE " + tbl + " SET a = 'Qt' WHERE id = 1");