SQLite: Handle identifiers correctly
This change can be described in the following 2 categories: 1. Support 3 ways to escape identifiers mentioned in SQLite Keywords In SQLite Keywords (https://sqlite.org/lang_keywords.html), it shows that there are 3 ways to escape identifiers, i.e., "", [], ``. So, I have overridden "bool isIdentifierEscaped(const QString &, IdentifierType)" to support it. In addition, there was a bug of _q_escapeIdentifier. If there is a field name called length [cm], which uses square brackets to show units, _q_escapeIdentifier will not escape it to "length [cm]". 2. Identify identifiers correctly if identifiers have been escaped There is a bug of QSQLiteDriver::record and QSQLiteDriver::primaryIndex. If we input escaped identifiers with separator, let's say "databaseName"."tableName", both will change the input into databaseName"."tableName, which is incorrect and causes qGetTableInfo cannot get the right results. In addition, I overrode stripDelimiters to strip "databaseName"."tableName" correctly. There are still some assumptions for isIdentifierEscaped, escapeIdentifier, and stripDelimiters, but I think this change it better than what we have now. 1. For isIdentifierEscaped, if identifiers have a dot and the dot is a separator, it is the users' responsibility to escape the pair of schema and table name correctly. For example, "aSchemaName"."aTableName", not "aSchemaName".a"TableName". That's because we don't know whether the dot is just a dot of the name or a separator. 2. For escapeIdentifier, if identifiers have a dot and the parts before and after the dot are not escaped, escapeIdentifier will treat the dot as part of the table name or field name. The same as the item above, it is users' responsibility to do it right. 3. For stripDelimiters, the same as above, it is users' responsibility to do escape if users want to use format schemaName.tableName or tableName.fieldName. Change-Id: I9d036a2a96180f8542436188f75a220a0fe58257 Reviewed-by: Po-Hao Su <supohaosu@gmail.com> Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org> Reviewed-by: Christian Ehrlicher <ch.ehrlicher@gmx.de>bb10
Po-Hao Su
parent
c661dbd42d
commit
7c4aa794ca
|
|
@ -40,22 +40,6 @@ QT_BEGIN_NAMESPACE
|
|||
|
||||
using namespace Qt::StringLiterals;
|
||||
|
||||
static QString _q_escapeIdentifier(const QString &identifier, QSqlDriver::IdentifierType type)
|
||||
{
|
||||
QString res = identifier;
|
||||
// If it contains [ and ] then we assume it to be escaped properly already as this indicates
|
||||
// the syntax is exactly how it should be
|
||||
if (identifier.contains(u'[') && identifier.contains(u']'))
|
||||
return res;
|
||||
if (!identifier.isEmpty() && !identifier.startsWith(u'"') && !identifier.endsWith(u'"')) {
|
||||
res.replace(u'"', "\"\""_L1);
|
||||
if (type == QSqlDriver::TableName)
|
||||
res.replace(u'.', "\".\""_L1);
|
||||
res = u'"' + res + u'"';
|
||||
}
|
||||
return res;
|
||||
}
|
||||
|
||||
static int qGetColumnType(const QString &tpName)
|
||||
{
|
||||
const QString typeName = tpName.toLower();
|
||||
|
|
@ -114,11 +98,64 @@ class QSQLiteDriverPrivate : public QSqlDriverPrivate
|
|||
|
||||
public:
|
||||
inline QSQLiteDriverPrivate() : QSqlDriverPrivate(QSqlDriver::SQLite) {}
|
||||
bool isIdentifierEscaped(QStringView identifier) const;
|
||||
QSqlIndex getTableInfo(QSqlQuery &query, const QString &tableName,
|
||||
bool onlyPIndex = false) const;
|
||||
|
||||
sqlite3 *access = nullptr;
|
||||
QList<QSQLiteResult *> results;
|
||||
QStringList notificationid;
|
||||
};
|
||||
|
||||
bool QSQLiteDriverPrivate::isIdentifierEscaped(QStringView identifier) const
|
||||
{
|
||||
return identifier.size() > 2
|
||||
&& ((identifier.startsWith(u'"') && identifier.endsWith(u'"'))
|
||||
|| (identifier.startsWith(u'`') && identifier.endsWith(u'`'))
|
||||
|| (identifier.startsWith(u'[') && identifier.endsWith(u']')));
|
||||
}
|
||||
|
||||
QSqlIndex QSQLiteDriverPrivate::getTableInfo(QSqlQuery &query, const QString &tableName,
|
||||
bool onlyPIndex) const
|
||||
{
|
||||
Q_Q(const QSQLiteDriver);
|
||||
QString schema;
|
||||
QString table = q->escapeIdentifier(tableName, QSqlDriver::TableName);
|
||||
const auto indexOfSeparator = table.indexOf(u'.');
|
||||
if (indexOfSeparator > -1) {
|
||||
auto leftName = QStringView{table}.first(indexOfSeparator);
|
||||
auto rightName = QStringView{table}.sliced(indexOfSeparator + 1);
|
||||
if (isIdentifierEscaped(leftName) && isIdentifierEscaped(rightName)) {
|
||||
schema = leftName.toString() + u'.';
|
||||
table = rightName.toString();
|
||||
}
|
||||
}
|
||||
|
||||
query.exec("PRAGMA "_L1 + schema + "table_info ("_L1 + table + u')');
|
||||
QSqlIndex ind;
|
||||
while (query.next()) {
|
||||
bool isPk = query.value(5).toInt();
|
||||
if (onlyPIndex && !isPk)
|
||||
continue;
|
||||
QString typeName = query.value(2).toString().toLower();
|
||||
QString defVal = query.value(4).toString();
|
||||
if (!defVal.isEmpty() && defVal.at(0) == u'\'') {
|
||||
const int end = defVal.lastIndexOf(u'\'');
|
||||
if (end > 0)
|
||||
defVal = defVal.mid(1, end - 1);
|
||||
}
|
||||
|
||||
QSqlField fld(query.value(1).toString(), QMetaType(qGetColumnType(typeName)), tableName);
|
||||
if (isPk && (typeName == "integer"_L1))
|
||||
// INTEGER PRIMARY KEY fields are auto-generated in sqlite
|
||||
// INT PRIMARY KEY is not the same as INTEGER PRIMARY KEY!
|
||||
fld.setAutoValue(true);
|
||||
fld.setRequired(query.value(3).toInt() != 0);
|
||||
fld.setDefaultValue(defVal);
|
||||
ind.append(fld);
|
||||
}
|
||||
return ind;
|
||||
}
|
||||
|
||||
class QSQLiteResultPrivate : public QSqlCachedResultPrivate
|
||||
{
|
||||
|
|
@ -913,79 +950,26 @@ QStringList QSQLiteDriver::tables(QSql::TableType type) const
|
|||
return res;
|
||||
}
|
||||
|
||||
static QSqlIndex qGetTableInfo(QSqlQuery &q, const QString &tableName, bool onlyPIndex = false)
|
||||
{
|
||||
QString schema;
|
||||
QString table(tableName);
|
||||
const qsizetype indexOfSeparator = tableName.indexOf(u'.');
|
||||
if (indexOfSeparator > -1) {
|
||||
const qsizetype indexOfCloseBracket = tableName.indexOf(u']');
|
||||
if (indexOfCloseBracket != tableName.size() - 1) {
|
||||
// Handles a case like databaseName.tableName
|
||||
schema = tableName.left(indexOfSeparator + 1);
|
||||
table = tableName.mid(indexOfSeparator + 1);
|
||||
} else {
|
||||
const qsizetype indexOfOpenBracket = tableName.lastIndexOf(u'[', indexOfCloseBracket);
|
||||
if (indexOfOpenBracket > 0) {
|
||||
// Handles a case like databaseName.[tableName]
|
||||
schema = tableName.left(indexOfOpenBracket);
|
||||
table = tableName.mid(indexOfOpenBracket);
|
||||
}
|
||||
}
|
||||
}
|
||||
q.exec("PRAGMA "_L1 + schema + "table_info ("_L1 +
|
||||
_q_escapeIdentifier(table, QSqlDriver::TableName) + u')');
|
||||
QSqlIndex ind;
|
||||
while (q.next()) {
|
||||
bool isPk = q.value(5).toInt();
|
||||
if (onlyPIndex && !isPk)
|
||||
continue;
|
||||
QString typeName = q.value(2).toString().toLower();
|
||||
QString defVal = q.value(4).toString();
|
||||
if (!defVal.isEmpty() && defVal.at(0) == u'\'') {
|
||||
const int end = defVal.lastIndexOf(u'\'');
|
||||
if (end > 0)
|
||||
defVal = defVal.mid(1, end - 1);
|
||||
}
|
||||
|
||||
QSqlField fld(q.value(1).toString(), QMetaType(qGetColumnType(typeName)), tableName);
|
||||
if (isPk && (typeName == "integer"_L1))
|
||||
// INTEGER PRIMARY KEY fields are auto-generated in sqlite
|
||||
// INT PRIMARY KEY is not the same as INTEGER PRIMARY KEY!
|
||||
fld.setAutoValue(true);
|
||||
fld.setRequired(q.value(3).toInt() != 0);
|
||||
fld.setDefaultValue(defVal);
|
||||
ind.append(fld);
|
||||
}
|
||||
return ind;
|
||||
}
|
||||
|
||||
QSqlIndex QSQLiteDriver::primaryIndex(const QString &tblname) const
|
||||
QSqlIndex QSQLiteDriver::primaryIndex(const QString &tablename) const
|
||||
{
|
||||
Q_D(const QSQLiteDriver);
|
||||
if (!isOpen())
|
||||
return QSqlIndex();
|
||||
|
||||
QString table = tblname;
|
||||
if (isIdentifierEscaped(table, QSqlDriver::TableName))
|
||||
table = stripDelimiters(table, QSqlDriver::TableName);
|
||||
|
||||
QSqlQuery q(createResult());
|
||||
q.setForwardOnly(true);
|
||||
return qGetTableInfo(q, table, true);
|
||||
return d->getTableInfo(q, tablename, true);
|
||||
}
|
||||
|
||||
QSqlRecord QSQLiteDriver::record(const QString &tbl) const
|
||||
QSqlRecord QSQLiteDriver::record(const QString &tablename) const
|
||||
{
|
||||
Q_D(const QSQLiteDriver);
|
||||
if (!isOpen())
|
||||
return QSqlRecord();
|
||||
|
||||
QString table = tbl;
|
||||
if (isIdentifierEscaped(table, QSqlDriver::TableName))
|
||||
table = stripDelimiters(table, QSqlDriver::TableName);
|
||||
|
||||
QSqlQuery q(createResult());
|
||||
q.setForwardOnly(true);
|
||||
return qGetTableInfo(q, table);
|
||||
return d->getTableInfo(q, tablename);
|
||||
}
|
||||
|
||||
QVariant QSQLiteDriver::handle() const
|
||||
|
|
@ -996,7 +980,52 @@ QVariant QSQLiteDriver::handle() const
|
|||
|
||||
QString QSQLiteDriver::escapeIdentifier(const QString &identifier, IdentifierType type) const
|
||||
{
|
||||
return _q_escapeIdentifier(identifier, type);
|
||||
Q_D(const QSQLiteDriver);
|
||||
if (identifier.isEmpty() || isIdentifierEscaped(identifier, type))
|
||||
return identifier;
|
||||
|
||||
const auto indexOfSeparator = identifier.indexOf(u'.');
|
||||
if (indexOfSeparator > -1) {
|
||||
auto leftName = QStringView{identifier}.first(indexOfSeparator);
|
||||
auto rightName = QStringView{identifier}.sliced(indexOfSeparator + 1);
|
||||
const QStringView leftEnclose = d->isIdentifierEscaped(leftName) ? u"" : u"\"";
|
||||
const QStringView rightEnclose = d->isIdentifierEscaped(rightName) ? u"" : u"\"";
|
||||
if (leftEnclose.isEmpty() || rightEnclose.isEmpty())
|
||||
return (leftEnclose + leftName + leftEnclose + u'.' + rightEnclose + rightName
|
||||
+ rightEnclose);
|
||||
}
|
||||
return u'"' + identifier + u'"';
|
||||
}
|
||||
|
||||
bool QSQLiteDriver::isIdentifierEscaped(const QString &identifier, IdentifierType type) const
|
||||
{
|
||||
Q_D(const QSQLiteDriver);
|
||||
Q_UNUSED(type);
|
||||
return d->isIdentifierEscaped(QStringView{identifier});
|
||||
}
|
||||
|
||||
QString QSQLiteDriver::stripDelimiters(const QString &identifier, IdentifierType type) const
|
||||
{
|
||||
Q_D(const QSQLiteDriver);
|
||||
const auto indexOfSeparator = identifier.indexOf(u'.');
|
||||
if (indexOfSeparator > -1) {
|
||||
auto leftName = QStringView{identifier}.first(indexOfSeparator);
|
||||
auto rightName = QStringView{identifier}.sliced(indexOfSeparator + 1);
|
||||
const auto leftEscaped = d->isIdentifierEscaped(leftName);
|
||||
const auto rightEscaped = d->isIdentifierEscaped(rightName);
|
||||
if (leftEscaped || rightEscaped) {
|
||||
if (leftEscaped)
|
||||
leftName = leftName.sliced(1).chopped(1);
|
||||
if (rightEscaped)
|
||||
rightName = rightName.sliced(1).chopped(1);
|
||||
return leftName + u'.' + rightName;
|
||||
}
|
||||
}
|
||||
|
||||
if (isIdentifierEscaped(identifier, type))
|
||||
return identifier.mid(1, identifier.size() - 2);
|
||||
|
||||
return identifier;
|
||||
}
|
||||
|
||||
static void handle_sqlite_callback(void *qobj,int aoperation, char const *adbname, char const *atablename,
|
||||
|
|
|
|||
|
|
@ -54,9 +54,12 @@ public:
|
|||
QStringList tables(QSql::TableType) const override;
|
||||
|
||||
QSqlRecord record(const QString& tablename) const override;
|
||||
QSqlIndex primaryIndex(const QString &table) const override;
|
||||
QSqlIndex primaryIndex(const QString &tablename) const override;
|
||||
QVariant handle() const override;
|
||||
QString escapeIdentifier(const QString &identifier, IdentifierType) const override;
|
||||
|
||||
QString escapeIdentifier(const QString &identifier, IdentifierType type) const override;
|
||||
bool isIdentifierEscaped(const QString &identifier, IdentifierType type) const override;
|
||||
QString stripDelimiters(const QString &identifier, IdentifierType type) const override;
|
||||
|
||||
bool subscribeToNotification(const QString &name) override;
|
||||
bool unsubscribeFromNotification(const QString &name) override;
|
||||
|
|
|
|||
|
|
@ -116,6 +116,8 @@ private slots:
|
|||
void submitAllOnInvalidTable();
|
||||
void insertRecordsInLoop_data() { generic_data(); }
|
||||
void insertRecordsInLoop();
|
||||
void sqlite_escaped_delimiters_data() { generic_data("QSQLITE"); }
|
||||
void sqlite_escaped_delimiters();
|
||||
void sqlite_attachedDatabase_data() { generic_data("QSQLITE"); }
|
||||
void sqlite_attachedDatabase(); // For task 130799
|
||||
void tableModifyWithBlank_data() { generic_data(); }
|
||||
|
|
@ -1920,6 +1922,75 @@ void tst_QSqlTableModel::insertRecordsInLoop()
|
|||
QCOMPARE(model.columnCount(), 3);
|
||||
}
|
||||
|
||||
void tst_QSqlTableModel::sqlite_escaped_delimiters()
|
||||
{
|
||||
QFETCH(QString, dbName);
|
||||
QSqlDatabase db = QSqlDatabase::database(dbName);
|
||||
CHECK_DATABASE(db);
|
||||
if (db.databaseName() == ":memory:")
|
||||
QSKIP(":memory: database, skipping test");
|
||||
|
||||
auto attachedDb = QSqlDatabase::cloneDatabase(db, db.driverName() + QLatin1String("attached"));
|
||||
attachedDb.setDatabaseName(db.databaseName() + QLatin1String("attached.dat"));
|
||||
QVERIFY_SQL(attachedDb, open());
|
||||
QSqlQuery q(attachedDb);
|
||||
TableScope tsAttached(attachedDb, "attachedTestTable", __FILE__);
|
||||
QVERIFY_SQL(q,
|
||||
exec("CREATE TABLE attachedTestTable("
|
||||
"id int, \"attachedCol [unit]\" varchar(20))"));
|
||||
QVERIFY_SQL(q,
|
||||
exec("INSERT INTO attachedTestTable VALUES("
|
||||
"1, 'attachTestData')"));
|
||||
|
||||
QSqlQuery q2(db);
|
||||
TableScope ts(db, "testTable", __FILE__);
|
||||
QVERIFY_SQL(q2, exec("CREATE TABLE testTable(id int, \"col [unit]\" varchar(20))"));
|
||||
QVERIFY_SQL(q2, exec("INSERT INTO testTable VALUES(2, 'testData')"));
|
||||
QVERIFY_SQL(q2, exec("ATTACH DATABASE \"" + attachedDb.databaseName() + "\" AS attachedDb"));
|
||||
|
||||
const std::array<std::pair<QLatin1Char, QLatin1Char>, 3> escapingPairs{
|
||||
std::make_pair(QLatin1Char{'"'}, QLatin1Char{'"'}),
|
||||
std::make_pair(QLatin1Char{'`'}, QLatin1Char{'`'}),
|
||||
std::make_pair(QLatin1Char{'['}, QLatin1Char{']'})
|
||||
};
|
||||
|
||||
QSqlTableModel model(nullptr, db);
|
||||
model.setTable("testTable");
|
||||
QVERIFY_SQL(model, select());
|
||||
for (const auto &escapingPair : escapingPairs) {
|
||||
model.setTable(escapingPair.first + "testTable" + escapingPair.second);
|
||||
QVERIFY_SQL(model, select());
|
||||
}
|
||||
|
||||
model.setTable("attachedDb.attachedTestTable");
|
||||
QFAIL_SQL(model, select());
|
||||
for (const auto &escapingPair : escapingPairs) {
|
||||
model.setTable(escapingPair.first + "attachedDb.attachedTestTable" + escapingPair.second);
|
||||
QFAIL_SQL(model, select());
|
||||
model.setTable(escapingPair.first + "attachedDb" + escapingPair.first + ".a"
|
||||
+ escapingPair.second + "ttachedTestTable" + escapingPair.second);
|
||||
QFAIL_SQL(model, select());
|
||||
}
|
||||
|
||||
for (std::size_t i = 0; i <= escapingPairs.size(); ++i) {
|
||||
for (std::size_t j = 0; j <= escapingPairs.size(); ++j) {
|
||||
if (i == escapingPairs.size() && j == escapingPairs.size())
|
||||
continue;
|
||||
|
||||
QString leftName = "attachedDb";
|
||||
if (i != escapingPairs.size())
|
||||
leftName = escapingPairs.at(i).first + leftName + escapingPairs.at(i).second;
|
||||
QString rightName = "attachedTestTable";
|
||||
if (j != escapingPairs.size())
|
||||
rightName = escapingPairs.at(j).first + rightName + escapingPairs.at(j).second;
|
||||
model.setTable(leftName + "." + rightName);
|
||||
QVERIFY_SQL(model, select());
|
||||
}
|
||||
}
|
||||
|
||||
attachedDb.close();
|
||||
}
|
||||
|
||||
void tst_QSqlTableModel::sqlite_attachedDatabase()
|
||||
{
|
||||
QFETCH(QString, dbName);
|
||||
|
|
@ -1948,7 +2019,7 @@ void tst_QSqlTableModel::sqlite_attachedDatabase()
|
|||
|
||||
// This should query the table in the attached database (schema supplied)
|
||||
QSqlTableModel model(0, db);
|
||||
model.setTable("adb.atest");
|
||||
model.setTable("\"adb\".\"atest\"");
|
||||
QVERIFY_SQL(model, select());
|
||||
QCOMPARE(model.rowCount(), 1);
|
||||
QCOMPARE(model.data(model.index(0, 0), Qt::DisplayRole).toInt(), 1);
|
||||
|
|
|
|||
Loading…
Reference in New Issue