From: Matt Newell Date: Mon, 26 Mar 2012 17:46:22 +0000 (-0700) Subject: Allow named bind values to be used multiple times per query X-Git-Tag: 071012110112~1939 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=cff46983a8823fda13cafa2c8774153525f0d4d1;p=profile%2Fivi%2Fqtbase.git 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 Reviewed-by: Honglei Zhang --- diff --git a/src/sql/kernel/qsqlresult.cpp b/src/sql/kernel/qsqlresult.cpp index 16df1b8..f9dbae3 100644 --- a/src/sql/kernel/qsqlresult.cpp +++ b/src/sql/kernel/qsqlresult.cpp @@ -114,16 +114,18 @@ public: QString executedQuery; QHash types; QVector values; - typedef QHash IndexMap; + typedef QHash > IndexMap; IndexMap indexes; typedef QVector 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 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 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 --- 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 }