QSqlTableModel/QSqlQueryModel and insertColumns problem.

After inserting a column, fetching data through QSqlTableModel was
off by one or more, since it passed the indexInQuery through to
QSQM.  Also, the headerData would sometimes return a blank string
for an inserted column, and sometimes the column number.

The autotests have been beefed up a little to check insertRows and
insertColumns play nicely.

Change-Id: I7399d4c4d94f958884b67ab9b39b5cf2485d8416
Task-number: QTBUG-12626
Reviewed-by: Charles Yin

5 files changed, 223 insertions, 14 deletions

diff --git a/src/sql/models/qsqlquerymodel.cpp b/src/sql/models/qsqlquerymodel.cpp
index 8730192..9800e67 100644
--- a/src/sql/models/qsqlquerymodel.cpp
+++ b/src/sql/models/qsqlquerymodel.cpp
@@ -279,7 +279,11 @@ QVariant QSqlQueryModel::headerData(int section, Qt::Orientation orientation, in
             val = d->headers.value(section).value(Qt::EditRole);
         if (val.isValid())
             return val;
-        if (role == Qt::DisplayRole && d->rec.count() > section)
+
+        // See if it's an inserted column (iiq.column() != -1)
+        QModelIndex dItem = indexInQuery(createIndex(0, section));
+
+        if (role == Qt::DisplayRole && d->rec.count() > section && dItem.column() != -1)
             return d->rec.fieldName(section);
     }
     return QAbstractItemModel::headerData(section, orientation, role);
diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp
index 3bb46cc..6f9c284 100644
--- a/src/sql/models/qsqltablemodel.cpp
+++ b/src/sql/models/qsqltablemodel.cpp
@@ -423,6 +423,10 @@ QVariant QSqlTableModel::data(const QModelIndex &index, int role) const
     if (!index.isValid() || (role != Qt::DisplayRole && role != Qt::EditRole))
         return QVariant();
 
+    // Problem.. we need to use QSQM::indexInQuery to handle inserted columns
+    // but inserted rows we need to handle
+    // and indexInQuery is not virtual (grrr) so any values we pass to QSQM need
+    // to handle the insertedRows
     QModelIndex item = indexInQuery(index);
 
     switch (d->strategy) {
@@ -450,7 +454,9 @@ QVariant QSqlTableModel::data(const QModelIndex &index, int role) const
             return var;
         break; }
     }
-    return QSqlQueryModel::data(item, role);
+
+    // We need to handle row mapping here, but not column mapping
+    return QSqlQueryModel::data(index.sibling(item.row(), index.column()), role);
 }
 
 /*!
@@ -1225,7 +1231,7 @@ int QSqlTableModel::rowCount(const QModelIndex &parent) const
 QModelIndex QSqlTableModel::indexInQuery(const QModelIndex &item) const
 {
     Q_D(const QSqlTableModel);
-    const QModelIndex it = QSqlQueryModel::indexInQuery(item);
+    const QModelIndex it = QSqlQueryModel::indexInQuery(item); // this adjusts columns only
     if (d->strategy == OnManualSubmit) {
         int rowOffset = 0;
         QSqlTableModelPrivate::CacheMap::ConstIterator i = d->cache.constBegin();
diff --git a/tests/auto/qsqldatabase/tst_databases.h b/tests/auto/qsqldatabase/tst_databases.h
index 5dcf754..350e0d0 100644
--- a/tests/auto/qsqldatabase/tst_databases.h
+++ b/tests/auto/qsqldatabase/tst_databases.h
@@ -51,6 +51,7 @@
 #include <QDir>
 #include <QVariant>
 #include <QDebug>
+#include <QSqlTableModel>
 
 #include <QtTest/QtTest>
 
@@ -166,6 +167,29 @@ public:
         return count;
     }
 
+    int fillTestTableWithStrategies( const QString& driverPrefix = QString() ) const
+    {
+        QTest::addColumn<QString>( "dbName" );
+        QTest::addColumn<int>("submitpolicy_i");
+        int count = 0;
+
+        for ( int i = 0; i < dbNames.count(); ++i ) {
+            QSqlDatabase db = QSqlDatabase::database( dbNames.at( i ) );
+
+            if ( !db.isValid() )
+                continue;
+
+            if ( driverPrefix.isEmpty() || db.driverName().startsWith( driverPrefix ) ) {
+                QTest::newRow( QString("%1 [field]").arg(dbNames.at( i )).toLatin1() ) << dbNames.at( i ) << (int)QSqlTableModel::OnFieldChange;
+                QTest::newRow( QString("%1 [row]").arg(dbNames.at( i )).toLatin1() ) << dbNames.at( i ) << (int)QSqlTableModel::OnRowChange;
+                QTest::newRow( QString("%1 [manual]").arg(dbNames.at( i )).toLatin1() ) << dbNames.at( i ) << (int)QSqlTableModel::OnManualSubmit;
+                ++count;
+            }
+        }
+
+        return count;
+    }
+
     void addDb( const QString& driver, const QString& dbName,
                 const QString& user = QString(), const QString& passwd = QString(),
                 const QString& host = QString(), int port = -1, const QString params = QString() )
diff --git a/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp b/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp
index 0ca1417..e876764 100644
--- a/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp
+++ b/tests/auto/qsqlquerymodel/tst_qsqlquerymodel.cpp
@@ -318,6 +318,11 @@ void tst_QSqlQueryModel::insertColumn()
     model.setQuery(QSqlQuery("select * from " + qTableName("test", __FILE__), db));
     model.fetchMore(); // necessary???
 
+    bool isToUpper = db.driverName().startsWith("QIBASE") || db.driverName().startsWith("QOCI") || db.driverName().startsWith("QDB2");
+    const QString idColumn(isToUpper ? "ID" : "id");
+    const QString nameColumn(isToUpper ? "NAME" : "name");
+    const QString titleColumn(isToUpper ? "TITLE" : "title");
+
     QSignalSpy spy(&model, SIGNAL(columnsInserted(QModelIndex, int, int)));
 
     QCOMPARE(model.data(model.index(0, 0)).toInt(), 1);
@@ -325,6 +330,11 @@ void tst_QSqlQueryModel::insertColumn()
     QCOMPARE(model.data(model.index(0, 2)).toInt(), 1);
     QCOMPARE(model.data(model.index(0, 3)), QVariant());
 
+    QCOMPARE(model.headerData(0, Qt::Horizontal).toString(), idColumn);
+    QCOMPARE(model.headerData(1, Qt::Horizontal).toString(), nameColumn);
+    QCOMPARE(model.headerData(2, Qt::Horizontal).toString(), titleColumn);
+    QCOMPARE(model.headerData(3, Qt::Horizontal).toString(), QString("4"));
+
     QVERIFY(model.insertColumn(1));
 
     QCOMPARE(spy.count(), 1);
@@ -344,6 +354,12 @@ void tst_QSqlQueryModel::insertColumn()
     QCOMPARE(model.data(model.index(0, 3)).toInt(), 1);
     QCOMPARE(model.data(model.index(0, 4)), QVariant());
 
+    QCOMPARE(model.headerData(0, Qt::Horizontal).toString(), idColumn);
+    QCOMPARE(model.headerData(1, Qt::Horizontal).toString(), QString("2"));
+    QCOMPARE(model.headerData(2, Qt::Horizontal).toString(), nameColumn);
+    QCOMPARE(model.headerData(3, Qt::Horizontal).toString(), titleColumn);
+    QCOMPARE(model.headerData(4, Qt::Horizontal).toString(), QString("5"));
+
     QVERIFY(!model.insertColumn(-1));
     QVERIFY(!model.insertColumn(100));
     QVERIFY(!model.insertColumn(1, model.index(1, 1)));
@@ -378,14 +394,21 @@ void tst_QSqlQueryModel::insertColumn()
     QCOMPARE(model.indexInQuery(model.index(0, 5)).column(), -1);
     QCOMPARE(model.indexInQuery(model.index(0, 6)).column(), -1);
 
-    bool isToUpper = db.driverName().startsWith("QIBASE") || db.driverName().startsWith("QOCI") || db.driverName().startsWith("QDB2");
     QCOMPARE(model.record().field(0).name(), QString());
-    QCOMPARE(model.record().field(1).name(), isToUpper ? QString("ID") : QString("id"));
+    QCOMPARE(model.record().field(1).name(), idColumn);
     QCOMPARE(model.record().field(2).name(), QString());
-    QCOMPARE(model.record().field(3).name(), isToUpper ? QString("NAME") : QString("name"));
-    QCOMPARE(model.record().field(4).name(), isToUpper ? QString("TITLE") : QString("title"));
+    QCOMPARE(model.record().field(3).name(), nameColumn);
+    QCOMPARE(model.record().field(4).name(), titleColumn);
     QCOMPARE(model.record().field(5).name(), QString());
     QCOMPARE(model.record().field(6).name(), QString());
+
+    QCOMPARE(model.headerData(0, Qt::Horizontal).toString(), QString("1"));
+    QCOMPARE(model.headerData(1, Qt::Horizontal).toString(), idColumn);
+    QCOMPARE(model.headerData(2, Qt::Horizontal).toString(), QString("3"));
+    QCOMPARE(model.headerData(3, Qt::Horizontal).toString(), nameColumn);
+    QCOMPARE(model.headerData(4, Qt::Horizontal).toString(), titleColumn);
+    QCOMPARE(model.headerData(5, Qt::Horizontal).toString(), QString("6"));
+    QCOMPARE(model.headerData(6, Qt::Horizontal).toString(), QString("7"));
 }
 
 void tst_QSqlQueryModel::record()
diff --git a/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp b/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp
index f9fcaf0..1821d9c 100644
--- a/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp
+++ b/tests/auto/qsqltablemodel/tst_qsqltablemodel.cpp
@@ -78,11 +78,13 @@ private slots:
 
     void select_data() { generic_data(); }
     void select();
+    void insertColumns_data() { generic_data_with_strategies(); }
+    void insertColumns();
     void submitAll_data() { generic_data(); }
     void submitAll();
     void setRecord_data()  { generic_data(); }
     void setRecord();
-    void insertRow_data() { generic_data(); }
+    void insertRow_data() { generic_data_with_strategies(); }
     void insertRow();
     void insertRecord_data() { generic_data(); }
     void insertRecord();
@@ -130,6 +132,7 @@ private slots:
     void insertBeforeDelete();
 private:
     void generic_data(const QString& engine=QString());
+    void generic_data_with_strategies(const QString& engine=QString());
 };
 
 tst_QSqlTableModel::tst_QSqlTableModel()
@@ -227,7 +230,17 @@ void tst_QSqlTableModel::recreateTestTables()
 void tst_QSqlTableModel::generic_data(const QString &engine)
 {
     if ( dbs.fillTestTable(engine) == 0 ) {
-        if(engine.isEmpty())
+        if (engine.isEmpty())
+           QSKIP( "No database drivers are available in this Qt configuration", SkipAll );
+        else
+           QSKIP( (QString("No database drivers of type %1 are available in this Qt configuration").arg(engine)).toLocal8Bit(), SkipAll );
+    }
+}
+
+void tst_QSqlTableModel::generic_data_with_strategies(const QString &engine)
+{
+    if ( dbs.fillTestTableWithStrategies(engine) == 0 ) {
+        if (engine.isEmpty())
            QSKIP( "No database drivers are available in this Qt configuration", SkipAll );
         else
            QSKIP( (QString("No database drivers of type %1 are available in this Qt configuration").arg(engine)).toLocal8Bit(), SkipAll );
@@ -289,6 +302,81 @@ void tst_QSqlTableModel::select()
     QCOMPARE(model.data(model.index(3, 3)), QVariant());
 }
 
+void tst_QSqlTableModel::insertColumns()
+{
+    // Just like the select test, with extra stuff
+    QFETCH(QString, dbName);
+    QFETCH(int, submitpolicy_i);
+    QSqlTableModel::EditStrategy submitpolicy = (QSqlTableModel::EditStrategy) submitpolicy_i;
+    QSqlDatabase db = QSqlDatabase::database(dbName);
+    CHECK_DATABASE(db);
+
+    QSqlTableModel model(0, db);
+    model.setTable(test);
+    model.setSort(0, Qt::AscendingOrder);
+    model.setEditStrategy(submitpolicy);
+
+    QVERIFY_SQL(model, select());
+
+    QCOMPARE(model.rowCount(), 3);
+    QCOMPARE(model.columnCount(), 3);
+
+    QCOMPARE(model.data(model.index(0, 0)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry"));
+    QCOMPARE(model.data(model.index(0, 2)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 3)), QVariant());
+
+    QCOMPARE(model.data(model.index(1, 0)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond"));
+    QCOMPARE(model.data(model.index(1, 2)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 3)), QVariant());
+
+    QCOMPARE(model.data(model.index(2, 0)).toInt(), 3);
+    QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi"));
+    QCOMPARE(model.data(model.index(2, 2)).toInt(), 3);
+    QCOMPARE(model.data(model.index(2, 3)), QVariant());
+
+    QCOMPARE(model.data(model.index(3, 0)), QVariant());
+    QCOMPARE(model.data(model.index(3, 1)), QVariant());
+    QCOMPARE(model.data(model.index(3, 2)), QVariant());
+    QCOMPARE(model.data(model.index(3, 3)), QVariant());
+
+    // Now add a column at 0 and 2
+    model.insertColumn(0);
+    model.insertColumn(2);
+
+    QCOMPARE(model.rowCount(), 3);
+    QCOMPARE(model.columnCount(), 5);
+
+    QCOMPARE(model.data(model.index(0, 0)), QVariant());
+    QCOMPARE(model.data(model.index(0, 1)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 2)), QVariant());
+    QCOMPARE(model.data(model.index(0, 3)).toString(), QString("harry"));
+    QCOMPARE(model.data(model.index(0, 4)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 5)), QVariant());
+
+    QCOMPARE(model.data(model.index(1, 0)), QVariant());
+    QCOMPARE(model.data(model.index(1, 1)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 2)), QVariant());
+    QCOMPARE(model.data(model.index(1, 3)).toString(), QString("trond"));
+    QCOMPARE(model.data(model.index(1, 4)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 5)), QVariant());
+
+    QCOMPARE(model.data(model.index(2, 0)), QVariant());
+    QCOMPARE(model.data(model.index(2, 1)).toInt(), 3);
+    QCOMPARE(model.data(model.index(2, 2)), QVariant());
+    QCOMPARE(model.data(model.index(2, 3)).toString(), QString("vohi"));
+    QCOMPARE(model.data(model.index(2, 4)).toInt(), 3);
+    QCOMPARE(model.data(model.index(2, 5)), QVariant());
+
+    QCOMPARE(model.data(model.index(3, 0)), QVariant());
+    QCOMPARE(model.data(model.index(3, 1)), QVariant());
+    QCOMPARE(model.data(model.index(3, 2)), QVariant());
+    QCOMPARE(model.data(model.index(3, 3)), QVariant());
+    QCOMPARE(model.data(model.index(3, 4)), QVariant());
+    QCOMPARE(model.data(model.index(3, 5)), QVariant());
+}
+
 void tst_QSqlTableModel::setRecord()
 {
     QFETCH(QString, dbName);
@@ -339,26 +427,90 @@ void tst_QSqlTableModel::setRecord()
 void tst_QSqlTableModel::insertRow()
 {
     QFETCH(QString, dbName);
+    QFETCH(int, submitpolicy_i);
+    QSqlTableModel::EditStrategy submitpolicy = (QSqlTableModel::EditStrategy) submitpolicy_i;
     QSqlDatabase db = QSqlDatabase::database(dbName);
     CHECK_DATABASE(db);
 
     QSqlTableModel model(0, db);
-    model.setEditStrategy(QSqlTableModel::OnRowChange);
+    model.setEditStrategy(submitpolicy);
     model.setTable(test);
     model.setSort(0, Qt::AscendingOrder);
     QVERIFY_SQL(model, select());
 
+    QCOMPARE(model.data(model.index(0, 0)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry"));
+    QCOMPARE(model.data(model.index(0, 2)).toInt(), 1);
+    QCOMPARE(model.data(model.index(1, 0)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond"));
+    QCOMPARE(model.data(model.index(1, 2)).toInt(), 2);
+    QCOMPARE(model.data(model.index(2, 0)).toInt(), 3);
+    QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi"));
+    QCOMPARE(model.data(model.index(2, 2)).toInt(), 3);
+
     QVERIFY(model.insertRow(2));
+
+    QCOMPARE(model.data(model.index(0, 0)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry"));
+    QCOMPARE(model.data(model.index(0, 2)).toInt(), 1);
+    QCOMPARE(model.data(model.index(1, 0)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond"));
+    QCOMPARE(model.data(model.index(1, 2)).toInt(), 2);
+    QCOMPARE(model.data(model.index(2, 0)).toInt(), 0);
+    QCOMPARE(model.data(model.index(2, 1)).toString(), QString());
+    QCOMPARE(model.data(model.index(2, 2)).toInt(), 0);
+    QCOMPARE(model.data(model.index(3, 0)).toInt(), 3);
+    QCOMPARE(model.data(model.index(3, 1)).toString(), QString("vohi"));
+    QCOMPARE(model.data(model.index(3, 2)).toInt(), 3);
+
     QSqlRecord rec = model.record(1);
     rec.setValue(0, 42);
-    rec.setValue(1, QString("vohi"));
+    rec.setValue(1, QString("francis"));
+
+    // FieldChange updates immediately and resorts
+    // Row/Manual submit does not resort
     QVERIFY(model.setRecord(2, rec));
 
-    QCOMPARE(model.data(model.index(2, 0)).toInt(), 42);
-    QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi"));
-    QCOMPARE(model.data(model.index(2, 2)).toInt(), 2);
+    QCOMPARE(model.data(model.index(0, 0)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry"));
+    QCOMPARE(model.data(model.index(0, 2)).toInt(), 1);
+    QCOMPARE(model.data(model.index(1, 0)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond"));
+    QCOMPARE(model.data(model.index(1, 2)).toInt(), 2);
+
+    // See comment above setRecord
+    if (submitpolicy == QSqlTableModel::OnFieldChange) {
+        QCOMPARE(model.data(model.index(2, 0)).toInt(), 3);
+        QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi"));
+        QCOMPARE(model.data(model.index(2, 2)).toInt(), 3);
+        QCOMPARE(model.data(model.index(3, 0)).toInt(), 42);
+        QCOMPARE(model.data(model.index(3, 1)).toString(), QString("francis"));
+        QCOMPARE(model.data(model.index(3, 2)).toInt(), 2);
+    } else {
+        QCOMPARE(model.data(model.index(2, 0)).toInt(), 42);
+        QCOMPARE(model.data(model.index(2, 1)).toString(), QString("francis"));
+        QCOMPARE(model.data(model.index(2, 2)).toInt(), 2);
+        QCOMPARE(model.data(model.index(3, 0)).toInt(), 3);
+        QCOMPARE(model.data(model.index(3, 1)).toString(), QString("vohi"));
+        QCOMPARE(model.data(model.index(3, 2)).toInt(), 3);
+    }
 
     QVERIFY(model.submitAll());
+
+    // After the submit we should have the resorted view
+    QCOMPARE(model.data(model.index(0, 0)).toInt(), 1);
+    QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry"));
+    QCOMPARE(model.data(model.index(0, 2)).toInt(), 1);
+    QCOMPARE(model.data(model.index(1, 0)).toInt(), 2);
+    QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond"));
+    QCOMPARE(model.data(model.index(1, 2)).toInt(), 2);
+    QCOMPARE(model.data(model.index(2, 0)).toInt(), 3);
+    QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi"));
+    QCOMPARE(model.data(model.index(2, 2)).toInt(), 3);
+    QCOMPARE(model.data(model.index(3, 0)).toInt(), 42);
+    QCOMPARE(model.data(model.index(3, 1)).toString(), QString("francis"));
+    QCOMPARE(model.data(model.index(3, 2)).toInt(), 2);
+
 }
 
 void tst_QSqlTableModel::insertRecord()