From 8a2fc0862f99a54fc3a6cf9db81355a0829c6f5a Mon Sep 17 00:00:00 2001 From: David Rosca Date: Tue, 20 Dec 2016 14:19:58 +0100 Subject: [PATCH] Fix performance when querying icons from database Make use of the index on icons column by using GLOB instead of LIKE and handle the escaping ourselves. Closes #1679 --- src/lib/other/iconchooser.cpp | 5 ++--- src/lib/tools/iconprovider.cpp | 11 ++++------- src/lib/tools/qztools.cpp | 12 ++++++------ src/lib/tools/qztools.h | 2 +- tests/autotests/qztoolstest.cpp | 27 +++++++++++++++++++++++++++ tests/autotests/qztoolstest.h | 3 +++ 6 files changed, 43 insertions(+), 17 deletions(-) diff --git a/src/lib/other/iconchooser.cpp b/src/lib/other/iconchooser.cpp index 3682c8d5a..0dae18702 100644 --- a/src/lib/other/iconchooser.cpp +++ b/src/lib/other/iconchooser.cpp @@ -65,9 +65,8 @@ void IconChooser::searchIcon(const QString &string) ui->iconList->clear(); QSqlQuery query; - query.prepare(QSL("SELECT icon FROM icons WHERE url LIKE ? ESCAPE ? LIMIT 20")); - query.bindValue(0, QString(QL1S("%%1%")).arg(QzTools::escapeSqlString(string))); - query.bindValue(1, QL1S("!")); + query.prepare(QSL("SELECT icon FROM icons WHERE url GLOB ? LIMIT 20")); + query.addBindValue(QString(QL1S("*%1*")).arg(QzTools::escapeSqlGlobString(string))); query.exec(); while (query.next()) { diff --git a/src/lib/tools/iconprovider.cpp b/src/lib/tools/iconprovider.cpp index 1b12fbd6e..43f5b5451 100644 --- a/src/lib/tools/iconprovider.cpp +++ b/src/lib/tools/iconprovider.cpp @@ -185,10 +185,8 @@ QImage IconProvider::imageForUrl(const QUrl &url, bool allowEmpty) } QSqlQuery query; - query.prepare(QSL("SELECT icon FROM icons WHERE url LIKE ? ESCAPE ? LIMIT 1")); - - query.addBindValue(QString("%1%").arg(QzTools::escapeSqlString(QString::fromUtf8(url.toEncoded(QUrl::RemoveFragment))))); - query.addBindValue(QL1S("!")); + query.prepare(QSL("SELECT icon FROM icons WHERE url GLOB ? LIMIT 1")); + query.addBindValue(QString("%1*").arg(QzTools::escapeSqlGlobString(QString::fromUtf8(url.toEncoded(QUrl::RemoveFragment))))); SqlDatabase::instance()->exec(query); if (query.next()) { @@ -216,10 +214,9 @@ QImage IconProvider::imageForDomain(const QUrl &url, bool allowEmpty) } QSqlQuery query; - query.prepare(QSL("SELECT icon FROM icons WHERE url LIKE ? ESCAPE ? LIMIT 1")); + query.prepare(QSL("SELECT icon FROM icons WHERE url GLOB ? LIMIT 1")); - query.addBindValue(QString("%%1%").arg(QzTools::escapeSqlString(url.host()))); - query.addBindValue(QL1S("!")); + query.addBindValue(QString("*%1*").arg(QzTools::escapeSqlGlobString(url.host()))); query.exec(); if (query.next()) { diff --git a/src/lib/tools/qztools.cpp b/src/lib/tools/qztools.cpp index 9e64293fa..9cc2631a0 100644 --- a/src/lib/tools/qztools.cpp +++ b/src/lib/tools/qztools.cpp @@ -198,13 +198,13 @@ QString QzTools::fromPunycode(const QString &str) return decoded.left(decoded.size() - 4); } -QString QzTools::escapeSqlString(QString urlString) +QString QzTools::escapeSqlGlobString(QString urlString) { - const static QString &escapeString = QL1S("!"); - urlString.replace(escapeString, escapeString + escapeString); - urlString.replace(QL1S("_"), escapeString + QL1S("_")); - urlString.replace(QL1S("%"), escapeString + QL1S("%")); - + urlString.replace(QL1C('['), QStringLiteral("[[")); + urlString.replace(QL1C(']'), QStringLiteral("[]]")); + urlString.replace(QStringLiteral("[["), QStringLiteral("[[]")); + urlString.replace(QL1C('*'), QStringLiteral("[*]")); + urlString.replace(QL1C('?'), QStringLiteral("[?]")); return urlString; } diff --git a/src/lib/tools/qztools.h b/src/lib/tools/qztools.h index 6e015c58c..1aca74ab9 100644 --- a/src/lib/tools/qztools.h +++ b/src/lib/tools/qztools.h @@ -48,7 +48,7 @@ public: static QString samePartOfStrings(const QString &one, const QString &other); static QString urlEncodeQueryString(const QUrl &url); static QString fromPunycode(const QString &str); - static QString escapeSqlString(QString urlString); + static QString escapeSqlGlobString(QString urlString); static QString ensureUniqueFilename(const QString &name, const QString &appendFormat = QString("(%1)")); static QString getFileNameFromUrl(const QUrl &url); diff --git a/tests/autotests/qztoolstest.cpp b/tests/autotests/qztoolstest.cpp index 88d55bb4d..52d908e88 100644 --- a/tests/autotests/qztoolstest.cpp +++ b/tests/autotests/qztoolstest.cpp @@ -134,6 +134,33 @@ void QzToolsTest::splitCommandArguments() QCOMPARE(QzTools::splitCommandArguments(command), result); } +void QzToolsTest::escapeSqlGlobString_data() +{ + QTest::addColumn("input"); + QTest::addColumn("result"); + + QTest::newRow("NothingToEscape") << "http://test" << "http://test"; + QTest::newRow("Escape *") << "http://test*/heh" << "http://test[*]/heh"; + QTest::newRow("Escape **") << "http://test**/he*h" << "http://test[*][*]/he[*]h"; + QTest::newRow("Escape ?") << "http://test?/heh" << "http://test[?]/heh"; + QTest::newRow("Escape ??") << "http://t??est?/heh" << "http://t[?][?]est[?]/heh"; + QTest::newRow("Escape [") << "http://[test/heh" << "http://[[]test/heh"; + QTest::newRow("Escape [[") << "http://[[te[st/heh" << "http://[[][[]te[[]st/heh"; + QTest::newRow("Escape ]") << "http://]test/heh" << "http://[]]test/heh"; + QTest::newRow("Escape ]]") << "http://]]te]st/heh" << "http://[]][]]te[]]st/heh"; + QTest::newRow("Escape []") << "http://[]test/heh" << "http://[[][]]test/heh"; + QTest::newRow("Escape [][[]][]") << "http://t[][[]][]est/heh" << "http://t[[][]][[][[][]][]][[][]]est/heh"; + QTest::newRow("Escape [?]][[*]") << "http://t[?]][[*]est/heh" << "http://t[[][?][]][]][[][[][*][]]est/heh"; +} + +void QzToolsTest::escapeSqlGlobString() +{ + QFETCH(QString, input); + QFETCH(QString, result); + + QCOMPARE(QzTools::escapeSqlGlobString(input), result); +} + class TempFile { QString name; diff --git a/tests/autotests/qztoolstest.h b/tests/autotests/qztoolstest.h index 1aa1f730f..d633a27a0 100644 --- a/tests/autotests/qztoolstest.h +++ b/tests/autotests/qztoolstest.h @@ -37,6 +37,9 @@ private slots: void splitCommandArguments_data(); void splitCommandArguments(); + void escapeSqlGlobString_data(); + void escapeSqlGlobString(); + void ensureUniqueFilename(); private: