Finalize query execution, query iterator and add a test

Includes some minor API fixups discovered while writing the test.

autotests/server/CMakeLists.txt

@@ -105,4 +105,5 @@ add_server_test(searchtest.cpp akonadiprivate)
 add_server_test(relationhandlertest.cpp akonadiprivate)
 add_server_test(taghandlertest.cpp akonadiprivate)
 add_server_test(fetchhandlertest.cpp akonadiprivate)
+add_server_test(queryexectest.cpp)
 endif()

autotests/server/queryexectest.cpp

+/*
+    Copyright (c) 2019 Daniel Vrátil <dvratil@kde.org>
+
+    This library is free software; you can redistribute it and/or modify it
+    under the terms of the GNU Library General Public License as published by
+    the Free Software Foundation; either version 2 of the License, or (at your
+    option) any later version.
+
+    This library is distributed in the hope that it will be useful, but WITHOUT
+    ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+    FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Library General Public
+    License for more details.
+
+    You should have received a copy of the GNU Library General Public License
+    along with this library; see the file COPYING.LIB.  If not, write to the
+    Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+    02110-1301, USA.
+*/
+
+#include <QObject>
+#include <QTest>
+#include <QMap>
+#include <QString>
+
+#include "storage/qb/selectquery.h"
+#include "storage/qb/insertquery.h"
+#include "storage/qb/updatequery.h"
+#include "storage/qb/deletequery.h"
+#include "storage/qb/aggregatefuncs.h"
+
+#include "fakeakonadiserver.h"
+#include "fakedatastore.h"
+#include "dbinitializer.h"
+#include "shared/aktest.h"
+#include "entities.h"
+
+using namespace Akonadi::Server;
+
+class QueryExecTest : public QObject
+{
+    Q_OBJECT
+public:
+    explicit QueryExecTest()
+        : QObject()
+    {
+        FakeAkonadiServer::instance()->init();
+    }
+
+    ~QueryExecTest()
+    {
+        FakeAkonadiServer::instance()->quit();
+    }
+
+private Q_SLOTS:
+    void testSelect()
+    {
+        auto &db = *DataStore::self();
+        auto query = Qb::Select(db)
+                .from(Collection::tableName())
+                .columns(Collection::nameColumn(), Collection::parentIdColumn())
+                .where({Collection::parentIdColumn(), Qb::Compare::Is, QVariant{}});
+        QVERIFY(query.exec());
+        QVERIFY(!query.error().isValid());
+        int count = 0;
+        for (const auto &result: query) {
+            QVERIFY(result.at<QString>(1).isNull());
+            count++;
+        }
+        QCOMPARE(count, 3);
+    }
+
+    void testInsert()
+    {
+        auto &db = *DataStore::self();
+        auto insert = Qb::Insert(db)
+                .into(MimeType::tableName())
+                .value(MimeType::nameColumn(), QStringLiteral("foo/bar"));
+        QVERIFY(insert.exec());
+        QVERIFY(!insert.error().isValid());
+        const auto insertId = insert.insertId();
+        QVERIFY(insertId.has_value());
+
+        auto select = Qb::Select(db)
+                .from(MimeType::tableName())
+                .column(MimeType::nameColumn())
+                .where({MimeType::idColumn(), Qb::Compare::Equals, *insertId});
+        QVERIFY(select.exec());
+        QVERIFY(!select.error().isValid());
+        QCOMPARE(select.begin()->at<QString>(0), QStringLiteral("foo/bar"));
+    }
+
+    void testUpdate()
+    {
+        auto &db = *DataStore::self();
+
+        auto insert = Qb::Insert(db)
+                .into(MimeType::tableName())
+                .value(MimeType::nameColumn(), QStringLiteral("bar/foo"));
+        QVERIFY(insert.exec());
+        QVERIFY(!insert.error().isValid());
+        const auto insertId = insert.insertId();
+        QVERIFY(insertId.has_value());
+
+        auto update = Qb::Update(db)
+                .table(MimeType::tableName())
+                .value(MimeType::nameColumn(), QStringLiteral("baz/blah"))
+                .where({MimeType::idColumn(), Qb::Compare::Equals, *insertId});
+        QVERIFY(update.exec());
+        QVERIFY(!update.error().isValid());
+
+        auto select = Qb::Select(db)
+                .from(MimeType::tableName())
+                .column(MimeType::nameColumn())
+                .where({MimeType::idColumn(), Qb::Compare::Equals, *insertId});
+        QVERIFY(select.exec());
+        QVERIFY(!select.error().isValid());
+        QCOMPARE(select.begin()->at<QString>(0), QStringLiteral("baz/blah"));
+    }
+
+    void testDelete()
+    {
+        auto &db = *DataStore::self();
+
+        auto insert = Qb::Insert(db)
+                .into(MimeType::tableName())
+                .value(MimeType::nameColumn(), QStringLiteral("bla/bla"));
+        QVERIFY(insert.exec());
+        QVERIFY(!insert.error().isValid());
+        const auto insertId = insert.insertId();
+        QVERIFY(insertId.has_value());
+
+        auto select = Qb::Select(db)
+                .from(MimeType::tableName())
+                .column(Qb::Count())
+                .where({MimeType::idColumn(), Qb::Compare::Equals, *insertId});
+        QVERIFY(select.exec());
+        QCOMPARE(select.begin()->at<int>(0), 1);
+
+        auto del = Qb::Delete(db)
+                .from(MimeType::tableName())
+                .where({MimeType::idColumn(), Qb::Compare::Equals, *insertId});
+        QVERIFY(del.exec());
+        QVERIFY(!del.error().isValid());
+
+        QVERIFY(select.exec());
+        QCOMPARE(select.begin()->at<int>(0), 0);
+    }
+};
+
+AKTEST_FAKESERVER_MAIN(QueryExecTest)
+
+#include "queryexectest.moc"
+
+

src/server/storage/qb/deletequery.cpp

@@ -28,7 +28,7 @@ DeleteQuery::DeleteQuery(DataStore &db)
     : Query(db)
 {}
 
-DeleteQuery &DeleteQuery::table(const QString &table)
+DeleteQuery &DeleteQuery::from(const QString &table)
 {
     mTable = table;
     return *this;

src/server/storage/qb/deletequery.h

@@ -40,7 +40,7 @@ public:
 #endif
     explicit DeleteQuery(DataStore &db);
 
-    DeleteQuery &table(const QString &table);
+    DeleteQuery &from(const QString &table);
 
     DeleteQuery &where(const ConditionStmt &condition);
     DeleteQuery &where(ConditionStmt &&condition);
@@ -53,7 +53,7 @@ private:
     akOptional<ConditionStmt> mCondition;
 };
 
-DeleteQuery Delete(DataStore &db)
+inline DeleteQuery Delete(DataStore &db)
 {
     return DeleteQuery(db);
 }

src/server/storage/qb/insertquery.h

@@ -90,10 +90,9 @@ private:
     akOptional<QString> mTable;
     QString mReturning = QStringLiteral("id");
     QVariantMap mValues;
-    qint64 mInsertId = -1;
 };
 
-InsertQuery Insert(DataStore &db)
+inline InsertQuery Insert(DataStore &db)
 {
     return InsertQuery(db);
 }

src/server/storage/qb/query.cpp

@@ -65,6 +65,31 @@ QSqlQuery &Query::query()
     return mQuery;
 }
 
+QSqlError Query::error() const
+{
+    return mQuery.lastError();
+}
+
+int Query::size() const
+{
+    return mQuery.size();
+}
+
+void Query::finish()
+{
+    return mQuery.finish();
+}
+
+QueryResultIterator Query::begin() const
+{
+    return QueryResultIterator(mQuery);
+}
+
+QueryResultIterator Query::end() const
+{
+    return QueryResultIterator(QueryResultIterator::End(true));
+}
+
 #ifdef QUERYBUILDER_UNITTEST
 bool Query::exec()
 {
@@ -76,13 +101,13 @@ bool Query::exec()
 namespace
 {
 
-akOptional<QSqlQuery> prepareQuery(const QString &statement)
+akOptional<QSqlQuery> prepareQuery(const QSqlDatabase &db, const QString &statement)
 {
     auto query = QueryCache::query(statement);
     if (query.has_value()) {
         return query;
     } else {
-        QSqlQuery query;
+        QSqlQuery query(db);
         if (!query.prepare(statement)) {
             qCCritical(AKONADISERVER_LOG) << "DATABASE ERROR while PREPARING QUERY:";
             qCCritical(AKONADISERVER_LOG) << "  Error code:" << query.lastError().nativeErrorCode();
@@ -130,7 +155,7 @@ bool Query::exec()
     QTextStream stream(&statement);
     serialize(stream);
 
-    auto query = prepareQuery(statement);
+    auto query = prepareQuery(mDb.database(), statement);
     if (!query) {
         return false;
     }

src/server/storage/qb/query.h

@@ -68,10 +68,22 @@ public:
     /**
      * Returns query error, only valid after exec.
      */
-    QSqlError &error() const;
+    QSqlError error() const;
 
-    QueryResultIterator begin();
-    QueryResultIterator end();
+    QueryResultIterator begin() const;
+    QueryResultIterator end() const;
+
+    /**
+     * Returns the size of the result (number of returned rows)
+     *
+     * Returns -1 if the size cannot be determined or the database does not support it.
+     */
+    int size() const;
+
+    /**
+     * Marks the query as finished, freeing results from the memory
+     */
+    void finish();
 
     /**
       Executes the query, returns true on success.

src/server/storage/qb/queryiterator.h

@@ -21,6 +21,7 @@
 #define AKONADI_SERVER_QUERYITERATOR_H_
 
 #include <QSqlQuery>
+#include <QVariant>
 
 #include <iterator>
 
@@ -35,25 +36,59 @@ class QueryResultIterator;
 class QueryResult
 {
 public:
+    QueryResult(const QueryResult &) = default;
+    QueryResult &operator=(const QueryResult &) = default;
+    QueryResult(QueryResult &&) = default;
+    QueryResult &operator=(QueryResult &&) = default;
+    ~QueryResult() = default;
+
+    bool operator==(const QueryResult &other) const
+    {
+        // TODO: user should never compare two iterators from different queries
+        // but if they do, we should catch it here...
+        return mQuery.isValid() == other.mQuery.isValid()
+                || mQuery.at() == other.mQuery.at();
+    }
+
     auto at(int index) const
     {
-        return mQuery.at(index);
+        return mQuery.isValid() ? mQuery.value(index) : QVariant{};
     }
 
     template<typename T>
     T at(int index) const
     {
-        const auto &value = mQuery.at(index);
+        if (!mQuery.isValid()) {
+            return T{};
+        }
+
+        const auto value = at(index);
         Q_ASSERT(value.canConvert<T>());
         return value.value<T>();
     }
 
 private:
     friend class QueryResultIterator;
-    QueryResult(const QSqlQuery &query)
+    explicit QueryResult(const QSqlQuery &query)
         : mQuery(query)
+    {
+        Q_ASSERT(mQuery.at() == QSql::BeforeFirstRow);
+        mQuery.next(); // move to the first row
+    }
+
+    explicit QueryResult()
     {}
 
+    bool next()
+    {
+        return mQuery.next();
+    }
+
+    bool isValid() const
+    {
+        return mQuery.isValid();
+    }
+
     QSqlQuery mQuery;
 };
 
@@ -70,8 +105,11 @@ public:
 
     using End = bool;
 
-    QueryResultIterator(QueryResultIterator &other) = default;
-    QueryResultIterator &operator=(QueryResultIterator &other) = default;
+    QueryResultIterator(const QueryResultIterator &) = default;
+    QueryResultIterator &operator=(const QueryResultIterator &) = default;
+    QueryResultIterator(QueryResultIterator &&) = default;
+    QueryResultIterator &operator=(QueryResultIterator &&) = default;
+    ~QueryResultIterator() = default;
 
     reference operator*() const
     {
@@ -85,52 +123,47 @@ public:
 
     QueryResultIterator &operator++()
     {
-        if (mQuery.next()) {
-            mResult = QueryResult{mQuery};
-        } else {
-            mEnd = true;
-        }
-
+        mResult.next();
         return *this;
     }
 
     QueryResultIterator operator++(int num)
     {
-        if (mEnd) {
+        if (!mResult.isValid()) {
             return *this;
         }
 
-        const auto prevResult = mResult;
-        for (int i = 0; i < num; ++i) {
-            if (!mQuery.next()) {
-                mEnd = true;
-                return prevResult;
-            }
-        }
+        auto prevResult = mResult;
+        for (int i = 0; i < num && mResult.isValid(); ++i, mResult.next());
 
-        mResult = QueryResult{mQuery};
-        return *this;
+        return QueryResultIterator(std::move(prevResult));
     }
 
-    bool operator==(QueryResultIterator &other) const
+    bool operator==(const QueryResultIterator &other) const
     {
-        return mEnd == other.mEnd || mResult == other.mResult;
+        return mResult == other.mResult;
     }
 
-    inline bool operator!=(QueryResultIterator &other) const
+    inline bool operator!=(const QueryResultIterator &other) const
     {
         return !(*this == other);
     }
 
 private:
-    friend class Qb::Query;
-    explicit QueryResultIterator(QSqlQuery &query);
-    explicit QueryResultIterator(End);
+    friend class Query;
+    explicit QueryResultIterator(const QSqlQuery &query)
+        : mResult(query)
+    {}
+
+    explicit QueryResultIterator(QueryResult &&result)
+        : mResult(std::move(result))
+    {}
+
+    explicit QueryResultIterator(End)
+    {}
 
 private:
-    QSqlQuery mQuery;
     QueryResult mResult;
-    End mEnd = false;
 };
 
 } // namespace Qb

src/server/storage/qb/selectquery.cpp

@@ -115,7 +115,10 @@ QTextStream &OrderByStmt::serialize(QTextStream &stream) const
 
 SelectQuery::SelectQuery(DataStore &db)
     : Query(db)
-{}
+{
+    // Enable forward-only mode in SELECT queries - this is a memory optimization
+    query().setForwardOnly(true);
+}
 
 SelectQuery &SelectQuery::from(const TableStmt &table)
 {
@@ -159,12 +162,24 @@ SelectQuery &SelectQuery::where(const ConditionStmt &cond)
     return *this;
 }
 
+SelectQuery &SelectQuery::where(ConditionStmt &&cond)
+{
+    mWhere = std::move(cond);
+    return *this;
+}
+
 SelectQuery &SelectQuery::having(const ConditionStmt &cond)
 {
     mHaving = cond;
     return *this;
 }
 
+SelectQuery &SelectQuery::having(ConditionStmt &&cond)
+{
+    mHaving = std::move(cond);
+    return *this;
+}
+
 SelectQuery &SelectQuery::orderBy(const QString &column, SortDirection order)
 {
     mOrderBy.push_back(OrderByStmt{column, order});

src/server/storage/qb/selectquery.h

@@ -164,11 +164,13 @@ public:
       Add a WHERE condition
     */
     SelectQuery &where(const ConditionStmt &condition);
+    SelectQuery &where(ConditionStmt &&condition);
 
     /**
       Add a HAVING condition
     */
     SelectQuery &having(const ConditionStmt &condition);
+    SelectQuery &having(ConditionStmt &&condition);
 
     /**
       Add sort column.
@@ -212,7 +214,7 @@ private:
     bool mForUpdate = false;
 };
 
-SelectQuery Select(DataStore &db)
+inline SelectQuery Select(DataStore &db)
 {
     return SelectQuery(db);
 }

src/server/storage/qb/updatequery.h

@@ -58,7 +58,7 @@ private:
     akOptional<ConditionStmt> mCondition;
 };
 
-UpdateQuery Update(DataStore &db)
+inline UpdateQuery Update(DataStore &db)
 {
     return UpdateQuery(db);
 }