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 <[email protected]>
Reviewed-by: Michael Goddard

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)

src/sql/kernel/qsqlfield.cpp

@@ -162,6 +162,7 @@ class QSqlFieldPrivate
 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);
 }
 
 

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);
         }
     }
 }

src/sql/models/qsqltablemodel.cpp

@@ -138,13 +138,26 @@ void QSqlTableModelPrivate::revertInsertedRow()
 void QSqlTableModelPrivate::clearEditBuffer()
 {
     editBuffer = rec;
+    clearGenerated(editBuffer);
 }
 
 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,27 +547,27 @@ 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();
         emit dataChanged(index, index);
         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()) {
             if (d->editIndex != -1)
                 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)

src/sql/models/qsqltablemodel_p.h

@@ -72,6 +72,8 @@ class QSqlTableModelPrivate: public QSqlQueryModelPrivate
     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<QVariant> &values) const;
 
     bool exec(const QString &stmt, bool prepStatement,
@@ -100,7 +102,7 @@ class QSqlTableModelPrivate: public QSqlQueryModelPrivate
 
     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;

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() );

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"));

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));