Allow named bind values to be used multiple times per query

Prepared queries should be able to use a name parameter more than
once. Currently this will result in undefined behavior and crashes.
This patch fixes the bug and implements the needed test case.

Task-number: QTBUG-6420
Change-Id: I07d6537e432a9b2781e9ef3d9f597bceb054527e
Reviewed-by: Andy Shaw <andy.shaw@digia.com>
Reviewed-by: Honglei Zhang <honglei.zhang@nokia.com>

diff --git a/src/sql/kernel/qsqlresult.cpp b/src/sql/kernel/qsqlresult.cpp
index 16df1b8..f9dbae3 100644 (file)
--- a/src/sql/kernel/qsqlresult.cpp
+++ b/src/sql/kernel/qsqlresult.cpp
@@ -114,16 +114,18 @@ public:
     QString executedQuery;
     QHash<int, QSql::ParamType> types;
     QVector<QVariant> values;
-    typedef QHash<QString, int> IndexMap;
+    typedef QHash<QString, QList<int> > IndexMap;
     IndexMap indexes;
 
     typedef QVector<QHolder> QHolderVector;
     QHolderVector holders;
 };
 
+static QString qFieldSerial(int);
+
 QString QSqlResultPrivate::holderAt(int index) const
 {
-    return indexes.key(index);
+    return holders.size() > index ? holders.at(index).holderName : qFieldSerial(index);
 }
 
 // return a unique id for bound names
@@ -159,6 +161,8 @@ QString QSqlResultPrivate::positionalToNamedBinding()
     for (int i = 0; i < n; ++i) {
         QChar ch = sql.at(i);
         if (ch == QLatin1Char('?') && !inQuote) {
+            // Update the holder position since we are changing the holder name lengths
+            holders[count].holderPos = result.size();
             result += qFieldSerial(count++);
         } else {
             if (ch == QLatin1Char('\''))
@@ -188,7 +192,9 @@ QString QSqlResultPrivate::namedToPositionalBinding()
             int pos = i + 2;
             while (pos < n && qIsAlnum(sql.at(pos)))
                 ++pos;
-            indexes[sql.mid(i, pos - i)] = count++;
+            QString holder(sql.mid(i, pos - i));
+            indexes[holder].append(count++);
+            holders.append(QHolder(holder, i));
             result += QLatin1Char('?');
             i = pos;
         } else {
@@ -199,6 +205,7 @@ QString QSqlResultPrivate::namedToPositionalBinding()
         }
     }
     result.squeeze();
+    values.resize(holders.size());
     return result;
 }
 
@@ -610,27 +617,32 @@ bool QSqlResult::savePrepare(const QString& query)
 */
 bool QSqlResult::prepare(const QString& query)
 {
-    int n = query.size();
+    if (d->holders.isEmpty()) {
+        int n = query.size();
 
-    bool inQuote = false;
-    int i = 0;
-
-    while (i < n) {
-        QChar ch = query.at(i);
-        if (ch == QLatin1Char(':') && !inQuote
-                && (i == 0 || query.at(i - 1) != QLatin1Char(':'))
-                && (i + 1 < n && qIsAlnum(query.at(i + 1)))) {
-            int pos = i + 2;
-            while (pos < n && qIsAlnum(query.at(pos)))
-                ++pos;
+        bool inQuote = false;
+        int i = 0;
 
-            d->holders.append(QHolder(query.mid(i, pos - i), i));
-            i = pos;
-        } else {
-            if (ch == QLatin1Char('\''))
-                inQuote = !inQuote;
-            ++i;
+        while (i < n) {
+            QChar ch = query.at(i);
+            if (ch == QLatin1Char(':') && !inQuote
+                    && (i == 0 || query.at(i - 1) != QLatin1Char(':'))
+                    && (i + 1 < n && qIsAlnum(query.at(i + 1)))) {
+                int pos = i + 2;
+                while (pos < n && qIsAlnum(query.at(pos)))
+                    ++pos;
+
+                QString holder(query.mid(i, pos - i));
+                d->indexes[holder].append(d->holders.size());
+                d->holders.append(QHolder(holder, i));
+                i = pos;
+            } else {
+                if (ch == QLatin1Char('\''))
+                    inQuote = !inQuote;
+                ++i;
+            }
         }
+        d->values.resize(d->holders.size());
     }
     d->sql = query;
     return true; // fake prepares should always succeed
@@ -653,7 +665,7 @@ bool QSqlResult::exec()
         QString holder;
         for (i = d->holders.count() - 1; i >= 0; --i) {
             holder = d->holders.at(i).holderName;
-            val = d->values.value(d->indexes.value(holder));
+            val = d->values.value(d->indexes.value(holder).value(0,-1));
             QSqlField f(QLatin1String(""), val.type());
             f.setValue(val);
             query = query.replace(d->holders.at(i).holderPos,
@@ -697,7 +709,7 @@ bool QSqlResult::exec()
 void QSqlResult::bindValue(int index, const QVariant& val, QSql::ParamType paramType)
 {
     d->binds = PositionalBinding;
-    d->indexes[qFieldSerial(index)] = index;
+    d->indexes[qFieldSerial(index)].append(index);
     if (d->values.count() <= index)
         d->values.resize(index + 1);
     d->values[index] = val;
@@ -727,19 +739,14 @@ void QSqlResult::bindValue(const QString& placeholder, const QVariant& val,
     d->binds = NamedBinding;
     // if the index has already been set when doing emulated named
     // bindings - don't reset it
-    int idx = d->indexes.value(placeholder, -1);
-    if (idx >= 0) {
+    QList<int> indexes = d->indexes.value(placeholder);
+    foreach (int idx, indexes) {
         if (d->values.count() <= idx)
             d->values.resize(idx + 1);
         d->values[idx] = val;
-    } else {
-        d->values.append(val);
-        idx = d->values.count() - 1;
-        d->indexes[placeholder] = idx;
+        if (paramType != QSql::In || !d->types.isEmpty())
+            d->types[idx] = paramType;
     }
-
-    if (paramType != QSql::In || !d->types.isEmpty())
-        d->types[idx] = paramType;
 }
 
 /*!
@@ -776,8 +783,8 @@ QVariant QSqlResult::boundValue(int index) const
 */
 QVariant QSqlResult::boundValue(const QString& placeholder) const
 {
-    int idx = d->indexes.value(placeholder, -1);
-    return d->values.value(idx);
+    QList<int> indexes = d->indexes.value(placeholder);
+    return d->values.value(indexes.value(0,-1));
 }
 
 /*!
@@ -798,7 +805,7 @@ QSql::ParamType QSqlResult::bindValueType(int index) const
 */
 QSql::ParamType QSqlResult::bindValueType(const QString& placeholder) const
 {
-    return d->types.value(d->indexes.value(placeholder, -1), QSql::In);
+    return d->types.value(d->indexes.value(placeholder).value(0,-1), QSql::In);
 }
 
 /*!

diff --git a/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp b/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp
index 584fcb0..dab34a8 100644 (file)
--- a/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp
+++ b/tests/auto/sql/kernel/qsqlquery/tst_qsqlquery.cpp
@@ -1683,11 +1683,11 @@ void tst_QSqlQuery::prepare_bind_exec()
             QVERIFY_SQL( q, exec("set client_min_messages='warning'"));
 
         if ( tst_Databases::isSqlServer( db ) || db.driverName().startsWith( "QTDS" ) )
-            createQuery = "create table " + qtest_prepare + " (id int primary key, name nvarchar(200) null)";
+            createQuery = "create table " + qtest_prepare + " (id int primary key, name nvarchar(200) null, name2 nvarchar(200) null)";
         else if ( tst_Databases::isMySQL(db) && useUnicode )
-            createQuery = "create table " + qtest_prepare + " (id int not null primary key, name varchar(200) character set utf8)";
+            createQuery = "create table " + qtest_prepare + " (id int not null primary key, name varchar(200) character set utf8, name2 varchar(200) character set utf8)";
         else
-            createQuery = "create table " + qtest_prepare + " (id int not null primary key, name varchar(200))";
+            createQuery = "create table " + qtest_prepare + " (id int not null primary key, name varchar(200), name2 varchar(200))";
 
         QVERIFY_SQL( q, exec( createQuery ) );
 
@@ -1756,7 +1756,7 @@ void tst_QSqlQuery::prepare_bind_exec()
             QCOMPARE( q.value( 0 ).toInt(), i );
             QCOMPARE( q.value( 1 ).toString().trimmed(), values[ i ] );
             QSqlRecord rInf = q.record();
-            QCOMPARE(( int )rInf.count(), 2 );
+            QCOMPARE(( int )rInf.count(), 3 );
             QCOMPARE( rInf.field( 0 ).name().toUpper(), QString( "ID" ) );
             QCOMPARE( rInf.field( 1 ).name().toUpper(), QString( "NAME" ) );
             QVERIFY( !q.next() );
@@ -1839,6 +1839,20 @@ void tst_QSqlQuery::prepare_bind_exec()
 
         QVERIFY( !q.isActive() );
 
+        QVERIFY( q.prepare( "insert into " + qtest_prepare + " (id, name, name2) values (:id, :name, :name)" ) );
+        for ( i = 101; i < 103; ++i ) {
+            q.bindValue( ":id", i );
+            q.bindValue( ":name", "name" );
+            QVERIFY( q.exec() );
+        }
+
+        // Test for QTBUG-6420
+        QVERIFY( q.exec( "select * from " + qtest_prepare + " where id > 100 order by id" ) );
+        QVERIFY( q.next() );
+        QCOMPARE( q.value(0).toInt(), 101 );
+        QCOMPARE( q.value(1).toString(), QString("name") );
+        QCOMPARE( q.value(2).toString(), QString("name") );
+
     } // end of SQLite scope
 }